]> Cypherpunks repositories - gostls13.git/commitdiff
[release-branch.go1.25] cmd/go: always return the cached path from go tool -n
authorMichael Matloob <matloob@golang.org>
Fri, 18 Jul 2025 18:14:16 +0000 (14:14 -0400)
committerGopher Robot <gobot@golang.org>
Mon, 28 Jul 2025 17:58:29 +0000 (10:58 -0700)
If we're running go tool -n always return the cached path of the tool.
We can't always use the cached path when running the tool because if we
copied the tool to the cached location in the same process and then try
to run it we'll run into #22315, producing spurious ETXTBSYs.

Fixes #72824

Change-Id: I81f23773b9028f955ccc97453627ae4f2573814b
Reviewed-on: https://go-review.googlesource.com/c/go/+/688895
Auto-Submit: Michael Matloob <matloob@golang.org>
Reviewed-by: Michael Matloob <matloob@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
(cherry picked from commit efc37e97c0f358f3cffe7ca2b78c743470345189)
Reviewed-on: https://go-review.googlesource.com/c/go/+/690895
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Bypass: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
src/cmd/go/internal/tool/tool.go
src/cmd/go/internal/work/action.go
src/cmd/go/internal/work/buildid.go
src/cmd/go/testdata/script/tool_n_issue72824.txt [new file with mode: 0644]

index 16e1a4f47f49c036e449587ec32b0c41f493db9f..120ef5339bede00d0a20f7479e449abcc4d785e2 100644 (file)
@@ -277,6 +277,29 @@ func loadModTool(ctx context.Context, name string) string {
        return ""
 }
 
+func builtTool(runAction *work.Action) string {
+       linkAction := runAction.Deps[0]
+       if toolN {
+               // #72824: If -n is set, use the cached path if we can.
+               // This is only necessary if the binary wasn't cached
+               // before this invocation of the go command: if the binary
+               // was cached, BuiltTarget() will be the cached executable.
+               // It's only in the "first run", where we actually do the build
+               // and save the result to the cache that BuiltTarget is not
+               // the cached binary. Ideally, we would set BuiltTarget
+               // to the cached path even in the first run, but if we
+               // copy the binary to the cached path, and try to run it
+               // in the same process, we'll run into the dreaded #22315
+               // resulting in occasional ETXTBSYs. Instead of getting the
+               // ETXTBSY and then retrying just don't use the cached path
+               // on the first run if we're going to actually run the binary.
+               if cached := linkAction.CachedExecutable(); cached != "" {
+                       return cached
+               }
+       }
+       return linkAction.BuiltTarget()
+}
+
 func buildAndRunBuiltinTool(ctx context.Context, toolName, tool string, args []string) {
        // Override GOOS and GOARCH for the build to build the tool using
        // the same GOOS and GOARCH as this go command.
@@ -288,7 +311,7 @@ func buildAndRunBuiltinTool(ctx context.Context, toolName, tool string, args []s
        modload.RootMode = modload.NoRoot
 
        runFunc := func(b *work.Builder, ctx context.Context, a *work.Action) error {
-               cmdline := str.StringList(a.Deps[0].BuiltTarget(), a.Args)
+               cmdline := str.StringList(builtTool(a), a.Args)
                return runBuiltTool(toolName, nil, cmdline)
        }
 
@@ -300,7 +323,7 @@ func buildAndRunModtool(ctx context.Context, toolName, tool string, args []strin
                // Use the ExecCmd to run the binary, as go run does. ExecCmd allows users
                // to provide a runner to run the binary, for example a simulator for binaries
                // that are cross-compiled to a different platform.
-               cmdline := str.StringList(work.FindExecCmd(), a.Deps[0].BuiltTarget(), a.Args)
+               cmdline := str.StringList(work.FindExecCmd(), builtTool(a), a.Args)
                // Use same environment go run uses to start the executable:
                // the original environment with cfg.GOROOTbin added to the path.
                env := slices.Clip(cfg.OrigEnv)
index 2426720021007543bfc81f4325848e8bef6bdc6b..ecc3337131cbfb8c24655008435cb49c334cebaa 100644 (file)
@@ -97,11 +97,12 @@ type Action struct {
        CacheExecutable bool // Whether to cache executables produced by link steps
 
        // Generated files, directories.
-       Objdir   string         // directory for intermediate objects
-       Target   string         // goal of the action: the created package or executable
-       built    string         // the actual created package or executable
-       actionID cache.ActionID // cache ID of action input
-       buildID  string         // build ID of action output
+       Objdir           string         // directory for intermediate objects
+       Target           string         // goal of the action: the created package or executable
+       built            string         // the actual created package or executable
+       cachedExecutable string         // the cached executable, if CacheExecutable was set
+       actionID         cache.ActionID // cache ID of action input
+       buildID          string         // build ID of action output
 
        VetxOnly  bool       // Mode=="vet": only being called to supply info about dependencies
        needVet   bool       // Mode=="build": need to fill in vet config
@@ -133,6 +134,10 @@ func (a *Action) BuildID() string { return a.buildID }
 // from Target when the result was cached.
 func (a *Action) BuiltTarget() string { return a.built }
 
+// CachedExecutable returns the cached executable, if CacheExecutable
+// was set and the executable could be cached, and "" otherwise.
+func (a *Action) CachedExecutable() string { return a.cachedExecutable }
+
 // An actionQueue is a priority queue of actions.
 type actionQueue []*Action
 
index 0bf9ba1781b0f746f2448d831a500623925fad0a..c272131c774fa2c721546bdef06acc0b4c87b9f0 100644 (file)
@@ -745,8 +745,9 @@ func (b *Builder) updateBuildID(a *Action, target string) error {
                        }
                        outputID, _, err := c.PutExecutable(a.actionID, name+cfg.ExeSuffix, r)
                        r.Close()
+                       a.cachedExecutable = c.OutputFile(outputID)
                        if err == nil && cfg.BuildX {
-                               sh.ShowCmd("", "%s # internal", joinUnambiguously(str.StringList("cp", target, c.OutputFile(outputID))))
+                               sh.ShowCmd("", "%s # internal", joinUnambiguously(str.StringList("cp", target, a.cachedExecutable)))
                        }
                }
        }
diff --git a/src/cmd/go/testdata/script/tool_n_issue72824.txt b/src/cmd/go/testdata/script/tool_n_issue72824.txt
new file mode 100644 (file)
index 0000000..0c90fce
--- /dev/null
@@ -0,0 +1,27 @@
+[short] skip 'does a build in using an empty cache'
+
+# Start with a fresh cache because we want to verify the behavior
+# when the tool hasn't been cached previously.
+env GOCACHE=$WORK${/}cache
+
+# Even when the tool hasn't been previously cached but was built and
+# saved to the cache in the invocation of 'go tool -n' we should return
+# its cached location.
+go tool -n foo
+stdout $GOCACHE
+
+# And of course we should also return the cached location on subsequent
+# runs.
+go tool -n foo
+stdout $GOCACHE
+
+-- go.mod --
+module example.com/foo
+
+go 1.25
+
+tool example.com/foo
+-- main.go --
+package main
+
+func main() {}
\ No newline at end of file