From: Michael Matloob Date: Fri, 18 Jul 2025 18:14:16 +0000 (-0400) Subject: [release-branch.go1.25] cmd/go: always return the cached path from go tool -n X-Git-Tag: go1.25rc3~4 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=c95d3093ca7f7b66453dbe67b4d210ffa5d83a9a;p=gostls13.git [release-branch.go1.25] cmd/go: always return the cached path from go tool -n 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 Reviewed-by: Michael Matloob Reviewed-by: Michael Pratt LUCI-TryBot-Result: Go LUCI (cherry picked from commit efc37e97c0f358f3cffe7ca2b78c743470345189) Reviewed-on: https://go-review.googlesource.com/c/go/+/690895 Reviewed-by: Dmitri Shuralyov TryBot-Bypass: Dmitri Shuralyov Reviewed-by: Dmitri Shuralyov --- diff --git a/src/cmd/go/internal/tool/tool.go b/src/cmd/go/internal/tool/tool.go index 16e1a4f47f..120ef5339b 100644 --- a/src/cmd/go/internal/tool/tool.go +++ b/src/cmd/go/internal/tool/tool.go @@ -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) diff --git a/src/cmd/go/internal/work/action.go b/src/cmd/go/internal/work/action.go index 2426720021..ecc3337131 100644 --- a/src/cmd/go/internal/work/action.go +++ b/src/cmd/go/internal/work/action.go @@ -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 diff --git a/src/cmd/go/internal/work/buildid.go b/src/cmd/go/internal/work/buildid.go index 0bf9ba1781..c272131c77 100644 --- a/src/cmd/go/internal/work/buildid.go +++ b/src/cmd/go/internal/work/buildid.go @@ -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 index 0000000000..0c90fce290 --- /dev/null +++ b/src/cmd/go/testdata/script/tool_n_issue72824.txt @@ -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