[go] cmd/go: ensure that go.mod and go.sum remain consistent after `go get -tool`
Ian Alexander (Gerrit)
Ian Alexander voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
- Michael Matloob
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Matloob (Gerrit)
Michael Matloob voted and added 1 comment![]( "Open in Gerrit")
Votes added by Michael Matloob
| Code-Review | +1 |
1 comment
mg, err := modload.LoadModGraph(moduleLoaderState, ctx, "")
if err != nil {
toolchain.SwitchOrFatal(moduleLoaderState, ctx, err)
}
r.buildList = mg.BuildList()
r.buildListVersion = make(map[string]string, len(r.buildList))
for _, m := range r.buildList {
r.buildListVersion[m.Path] = m.Version
}
}can we do `r.updateBuildList(loaderstate, ctx, nil)` instead?
Related details
- Ian Alexander
- Michael Matloob
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Matloob (Gerrit)
Michael Matloob voted Code-Review+2![]( "Open in Gerrit")
| Code-Review | +2 |
Related details
- Ian Alexander
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Alexander (Gerrit)
Ian Alexander uploaded new patchset![]( "Open in Gerrit")
- Code-Review: +1 by Michael Matloob, +2 by Michael Matloob
- TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI
Related details
- Ian Alexander
- Michael Matloob
- Michael Matloob
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Alexander (Gerrit)
Ian Alexander added 1 comment![]( "Open in Gerrit")
mg, err := modload.LoadModGraph(moduleLoaderState, ctx, "")
if err != nil {
toolchain.SwitchOrFatal(moduleLoaderState, ctx, err)
}
r.buildList = mg.BuildList()
r.buildListVersion = make(map[string]string, len(r.buildList))
for _, m := range r.buildList {
r.buildListVersion[m.Path] = m.Version
}
}can we do `r.updateBuildList(loaderstate, ctx, nil)` instead?
Updated the code and test as per our discussion. Before submission, we will need to fix the test so that it will run without accessing the network.
Related details
- Michael Matloob
- Michael Matloob
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Alexander (Gerrit)
Ian Alexander voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Ian Alexander (Gerrit)
Ian Alexander uploaded new patchset![]( "Open in Gerrit")
- Ian Alexander
- Michael Matloob
- Michael Matloob
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Alexander (Gerrit)
Ian Alexander uploaded new patchset![]( "Open in Gerrit")
Related details
- Ian Alexander
- Michael Matloob
- Michael Matloob
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Alexander (Gerrit)
Ian Alexander uploaded new patchset![]( "Open in Gerrit")
- TryBots-Pass: LUCI-TryBot-Result-1 by Go LUCI
Related details
- Ian Alexander
- Michael Matloob
- Michael Matloob
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Alexander (Gerrit)
Ian Alexander voted and added 1 comment![]( "Open in Gerrit")
Votes added by Ian Alexander
| Commit-Queue | +1 |
1 comment
mg, err := modload.LoadModGraph(moduleLoaderState, ctx, "")
if err != nil {
toolchain.SwitchOrFatal(moduleLoaderState, ctx, err)
}
r.buildList = mg.BuildList()
r.buildListVersion = make(map[string]string, len(r.buildList))
for _, m := range r.buildList {
r.buildListVersion[m.Path] = m.Version
}
}Ian Alexandercan we do `r.updateBuildList(loaderstate, ctx, nil)` instead?
Updated the code and test as per our discussion. Before submission, we will need to fix the test so that it will run without accessing the network.
- Michael Matloob
- Michael Matloob
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
naved juwale (Gerrit)
naved juwale added 1 comment![]( "Open in Gerrit")
You are a highly experienced code reviewer specializing in Git patches. Your
task is to analyze the provided Git patch (`patch`) and provide comprehensive
feedback. Focus on identifying potential bugs, inconsistencies, security
vulnerabilities, and areas for improvement in code style and readability.
Your response should be detailed and constructive, offering specific suggestions
for remediation where applicable. Prioritize clarity and conciseness in your
feedback.
# Step by Step Instructions
1. Read the provided `patch` carefully. Understand the changes it introduces to the codebase.
2. Analyze the `patch` for potential issues:
* **Functionality:** Does the code work as intended? Are there any bugs or unexpected behavior?
* **Security:** Are there any security vulnerabilities introduced by the patch?
* **Style:** Does the code adhere to the project's coding style guidelines? Is it readable and maintainable?
* **Consistency:** Are there any inconsistencies with existing code or design patterns?
* **Testing:** Does the patch include sufficient tests to cover the changes?
3. Formulate concise and constructive feedback for each identified issue. Provide specific suggestions for remediation where possible.
4. Summarize your findings in a clear and organized manner. Prioritize critical issues over minor ones.
5. Review the feedback written so far. Is the feedback comprehensive and sufficiently detailed?
If not, go back to step 2, focusing on any areas that require further analysis or clarification.
If yes, proceed to step 6.
6. Output the complete review.
Patch:
"""
From eee3f38e76eacb81883f30b00c202b98203c9020 Mon Sep 17 00:00:00 2001
From: Ian Alexander <ji...@google.com>
Date: Tue, 24 Feb 2026 12:00:40 -0500
Subject: [PATCH] cmd/go: ensure go.mod and go.sum are consistent after `go get -tool`
The issue was that `go get -tool` could trigger module upgrades (due to
the tool's own requirements) that were not correctly captured by the
final consistency check in `checkPackageProblems`. This happened because
`updateTools` was being called after `checkPackageProblems`, and even if
moved earlier, it failed to update the resolver's internal build list
representation. This left some incidentally upgraded modules (like
github.com/go-logr/logr in the gonzo module) without their corresponding
zip sums in go.sum, causing subsequent builds to fail.
The fix involves:
1. Moving the updateTools call before checkPackageProblems in runGet.
2. Ensuring the resolver's buildList and buildListVersion are
explicitly refreshed from the updated module graph after updateTools is
called.
This ensures that checkPackageProblems correctly identifies all modules
that were upgraded during the `go get -tool` operation and fetches the
necessary checksums, maintaining module consistency.
A test and associated necessary vcs-test configuration are added to
prevent regressions in the future.
Fixes #74691.
Change-Id: I1a7e22a35132bcbee2ceac1ff7fc190666db965b
---
diff --git a/src/cmd/go/internal/modget/get.go b/src/cmd/go/internal/modget/get.go
index b731ccc..196a220 100644
--- a/src/cmd/go/internal/modget/get.go
+++ b/src/cmd/go/internal/modget/get.go
@@ -402,14 +402,14 @@
}
}
+ if *getTool {
+ updateTools(moduleLoaderState, ctx, r, queries, &opts)
+ }
+
// If a workspace applies, checkPackageProblems will switch to the workspace
// using modload.EnterWorkspace when doing the final load, and then switch back.
r.checkPackageProblems(moduleLoaderState, ctx, pkgPatterns)
- if *getTool {
- updateTools(moduleLoaderState, ctx, queries, &opts)
- }
-
// Everything succeeded. Update go.mod.
oldReqs := reqsFromGoMod(modload.ModFile(moduleLoaderState))
@@ -433,7 +433,7 @@
}
}
-func updateTools(loaderstate *modload.State, ctx context.Context, queries []*query, opts *modload.WriteOpts) {
+func updateTools(loaderstate *modload.State, ctx context.Context, r *resolver, queries []*query, opts *modload.WriteOpts) {
pkgOpts := modload.PackageOpts{
VendorModulesInGOROOTSrc: true,
LoadTests: *getT,
@@ -457,6 +457,16 @@
opts.AddTools = append(opts.AddTools, m.Pkgs...)
}
}
+
+ mg, err := modload.LoadModGraph(loaderstate, ctx, "")
+ if err != nil {
+ toolchain.SwitchOrFatal(loaderstate, ctx, err)
+ }
+ r.buildList = mg.BuildList()
+ r.buildListVersion = make(map[string]string, len(r.buildList))
+ for _, m := range r.buildList {
+ r.buildListVersion[m.Path] = m.Version
+ }
}
// parseArgs parses command-line arguments and reports errors.
diff --git a/src/cmd/go/testdata/mod/github.com_go-logr_logr_v1.4.1.txt b/src/cmd/go/testdata/mod/github.com_go-logr_logr_v1.4.1.txt
new file mode 100644
index 0000000..c6b9adc
--- /dev/null
+++ b/src/cmd/go/testdata/mod/github.com_go-logr_logr_v1.4.1.txt
@@ -0,0 +1,14 @@
+module github.com/go-logr/lo...@v1.4.1
+
+-- .mod --
+module github.com/go-logr/logr
+
+go 1.18
+-- .info --
+{"Version":"v1.4.1","Time":"2023-12-21T15:57:58Z","Origin":{"VCS":"git","URL":"https://github.com/go-logr/logr","Hash":"dcdc3f2cd12e8a5c4e2a6712d6958c90e2e5bd98","Ref":"refs/tags/v1.4.1"}}
+-- go.mod --
+module github.com/go-logr/logr
+
+go 1.18
+-- logr.go --
+package logr
diff --git a/src/cmd/go/testdata/mod/github.com_go-logr_logr_v1.4.3.txt b/src/cmd/go/testdata/mod/github.com_go-logr_logr_v1.4.3.txt
new file mode 100644
index 0000000..fdef27e
--- /dev/null
+++ b/src/cmd/go/testdata/mod/github.com_go-logr_logr_v1.4.3.txt
@@ -0,0 +1,14 @@
+module github.com/go-logr/lo...@v1.4.3
+
+-- .mod --
+module github.com/go-logr/logr
+
+go 1.18
+-- .info --
+{"Version":"v1.4.3","Time":"2025-05-19T04:56:57Z","Origin":{"VCS":"git","URL":"https://github.com/go-logr/logr","Hash":"38a1c47ef633fa6b2eee6b8f2e1371ba8626e557","Ref":"refs/tags/v1.4.3"}}
+-- go.mod --
+module github.com/go-logr/logr
+
+go 1.18
+-- logr.go --
+package logr
diff --git a/src/cmd/go/testdata/mod/github.com_golangci_golangci-lint_v2_v2.10.1.txt b/src/cmd/go/testdata/mod/github.com_golangci_golangci-lint_v2_v2.10.1.txt
new file mode 100644
index 0000000..379214e
--- /dev/null
+++ b/src/cmd/go/testdata/mod/github.com_golangci_golangci-lint_v2_v2.10.1.txt
@@ -0,0 +1,20 @@
+module github.com/golangci/golangci-lint/v2@latest
+
+-- .mod --
+module github.com/golangci/golangci-lint/v2
+
+// The minimum Go version must always be latest-1.
+// This version should never be changed outside of the PR to add the support of newer Go version.
+// Only golangci-lint maintainers are allowed to change it.
+go 1.25.0
+
+require github.com/securego/gosec/v2 v2.23.0
+-- .info --
+{"Version":"v2.10.1"}
+-- cmd/golangci-lint/main.go --
+package main
+
+import _ "github.com/securego/gosec/v2/rules"
+
+func main()
+
diff --git a/src/cmd/go/testdata/mod/github.com_securego_gosec_v2_v2.23.0.txt b/src/cmd/go/testdata/mod/github.com_securego_gosec_v2_v2.23.0.txt
new file mode 100644
index 0000000..eab81ad
--- /dev/null
+++ b/src/cmd/go/testdata/mod/github.com_securego_gosec_v2_v2.23.0.txt
@@ -0,0 +1,28 @@
+module github.com/securego/gosec/v...@v2.23.0
+
+-- .mod --
+module github.com/securego/gosec/v2
+
+require (
+ example.com v1.0.0
+)
+
+require (
+ github.com/go-logr/logr v1.4.3 // indirect
+)
+
+go 1.25.0
+-- .info --
+{"Version":"v2.23.0","Time":"2026-02-10T14:47:11Z","Origin":{"VCS":"git","URL":"https://github.com/securego/gosec","Hash":"398ad549bbf1a51dc978fd966169f660c59774de","Ref":"refs/tags/v2.23.0"}}
+-- go.mod --
+module github.com/securego/gosec/v2
+
+require example.com v1.0.0
+
+go 1.25.0
+-- rules/rules_a.go --
+package rules
+
+import _ "example.com" // pull in a pre-module-pruning module (doesn't declare go version in go.mod)
+
+
diff --git a/src/cmd/go/testdata/mod/k8s.io_klog_v2_v2.130.1.txt b/src/cmd/go/testdata/mod/k8s.io_klog_v2_v2.130.1.txt
new file mode 100644
index 0000000..477baec
--- /dev/null
+++ b/src/cmd/go/testdata/mod/k8s.io_klog_v2_v2.130.1.txt
@@ -0,0 +1,20 @@
+Using requirements of .k8s.io/klog/v2 v2.130.1
+
+-- .mod --
+module k8s.io/klog/v2
+
+go 1.21
+
+require github.com/go-logr/logr v1.4.1
+-- .info --
+{"Version":"v2.130.1"}
+-- go.mod --
+module k8s.io/klog/v2
+
+go 1.21
+
+require github.com/go-logr/logr v1.4.1
+-- foo.go --
+package foo
+
+import _ "github.com/go-logr/logr"
diff --git a/src/cmd/go/testdata/script/get_tool_issue74691.txt b/src/cmd/go/testdata/script/get_tool_issue74691.txt
new file mode 100644
index 0000000..db34680
--- /dev/null
+++ b/src/cmd/go/testdata/script/get_tool_issue74691.txt
@@ -0,0 +1,16 @@
+# Regression test for https://go.dev/issue/74691.
+
+go mod init gonzo
+go get k8s.io/klog/v...@v2.130.1
+go mod tidy
+go build ./...
+go get -tool github.com/golangci/golangci-lint/v2/cmd/golangci-lint
+go build ./...
+
+-- main.go --
+package main
+import (
+ _ "k8s.io/klog/v2"
+)
+func main() {}
+
"""
IMPORTANT NOTE: Start directly with the output, do not output any delimiters.
Take a Deep Breath, read the instructions again, read the inputs again. Each instruction is crucial and must be executed with utmost care and attention to detail.
Review:
Related details
- Ian Alexander
- Michael Matloob
- Michael Matloob
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
naved juwale (Gerrit)
naved juwale added 1 comment![]( "Open in Gerrit")
You are a Git commit message expert, tasked with improving the quality and clarity of commit messages.
Your goal is to generate a well-structured and informative commit message based on a provided Git patch.
The commit message must adhere to a specific style guide, focusing on conciseness, clarity, and a professional tone.
You will use the patch's diff to understand the changes, summarizing complex diffs and focusing on the intent and impact of the changes.
You should paraphrase any provided bug summaries to explain the problem that was fixed.
Your output must be a single Markdown code block containing only the complete commit message (title and body),
formatted according to the provided specifications.
# Step by Step Instructions
1. **Analyze the Patch:** Carefully examine the provided `patch` to understand the changes made to the codebase.
Identify the key modifications, focusing on their intent and impact. Summarize complex changes concisely.
2. **Review Existing Commit Message:** Read the commit message included in the `patch`. Note its strengths and weaknesses.
Identify areas for improvement in clarity, conciseness, and adherence to the style guide.
3. **Refine the Title:** Craft a concise and informative commit title (under 60 characters) using sentence case and the imperative mood.
The title should accurately reflect the primary change implemented in the patch.
4. **Develop the Body:** Write a detailed body for the commit message, explaining the "what" and "why" of the changes.
Use the information gathered in Step 1 to describe the intent and impact of the modifications.
Structure the body using paragraphs, blank lines, and bullet points as needed for clarity. Wrap lines to approximately 72 characters.
5. **Ensure Style Compliance:** Verify that the commit message (title and body) adheres to all requirements outlined in the provided
"Commit Message Requirements" section. This includes checking for sentence case, imperative mood, line wrapping, and the exclusion of testing information.
6. **Format the Output:** Enclose the complete commit message (title and body) within a single Markdown code block.
Ensure there is one blank line separating the title and the body.
7. **Review and Iterate (Loop Instruction):** Review the complete commit message. Is it clear, concise, and informative?
Does it accurately reflect the changes made in the patch and adhere to the style guide? If not, return to Step 3 or Step 4 to make improvements. If satisfied, proceed to Step 8.
8. **Output the Commit Message:** Output the final, formatted commit message as a single Markdown code block.
Commit Message:
Ian Alexander (Gerrit)
Ian Alexander uploaded new patchset![]( "Open in Gerrit")
- TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI
Related details
- Ian Alexander
- Michael Matloob
- Michael Matloob
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Alexander (Gerrit)
Ian Alexander voted Commit-Queue+1![]( "Open in Gerrit")
- Michael Matloob
- Michael Matloob
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Matloob (Gerrit)
Michael Matloob voted and added 2 comments![]( "Open in Gerrit")
Votes added by Michael Matloob
| Code-Review | +1 |
2 comments
module github.com/go-logr/lo...@v1.4.1Could we add comments to all the new modules we're adding to mention that we've stripped down the set of requirements (from the original upstream requirements) for the purposes of the test get_tool_issue_74691?
import (
_ "k8s.io/klog/v2"
)Related details
- Ian Alexander
- Michael Matloob
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Matloob (Gerrit)
Michael Matloob voted Code-Review+2![]( "Open in Gerrit")
| Code-Review | +2 |
Related details
- Ian Alexander
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |