src/cmd/go/internal/tool/tool.go | 27 +++++++++++++++++++++++++-- src/cmd/go/internal/work/action.go | 15 ++++++++++----- src/cmd/go/internal/work/buildid.go | 3 ++- src/cmd/go/testdata/script/tool_n_issue72824.txt | 27 +++++++++++++++++++++++++++ diff --git a/src/cmd/go/internal/tool/tool.go b/src/cmd/go/internal/tool/tool.go index 16e1a4f47f49c036e449587ec32b0c41f493db9f..120ef5339bede00d0a20f7479e449abcc4d785e2 100644 --- a/src/cmd/go/internal/tool/tool.go +++ b/src/cmd/go/internal/tool/tool.go @@ -277,6 +277,29 @@ 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 @@ // user happens to be in. 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 @@ runFunc := func(b *work.Builder, ctx context.Context, a *work.Action) error { // 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 2426720021007543bfc81f4325848e8bef6bdc6b..ecc3337131cbfb8c24655008435cb49c334cebaa 100644 --- a/src/cmd/go/internal/work/action.go +++ b/src/cmd/go/internal/work/action.go @@ -97,11 +97,12 @@ 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 @@ -132,6 +133,10 @@ // BuiltTarget returns the actual file that was built. This differs // 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 0bf9ba1781b0f746f2448d831a500623925fad0a..c272131c774fa2c721546bdef06acc0b4c87b9f0 100644 --- a/src/cmd/go/internal/work/buildid.go +++ b/src/cmd/go/internal/work/buildid.go @@ -745,8 +745,9 @@ name = a.Package.DefaultExecName() } 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 0000000000000000000000000000000000000000..0c90fce290d286cb782fcf1ac3e0b7385eb9a5dc --- /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