Skip to content

Commit efc37e9

Browse files
matloobgopherbot
authored andcommitted
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 <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>
1 parent 98a0311 commit efc37e9

File tree

4 files changed

+64
-8
lines changed

4 files changed

+64
-8
lines changed

src/cmd/go/internal/tool/tool.go

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,29 @@ func loadModTool(ctx context.Context, name string) string {
277277
return ""
278278
}
279279

280+
func builtTool(runAction *work.Action) string {
281+
linkAction := runAction.Deps[0]
282+
if toolN {
283+
// #72824: If -n is set, use the cached path if we can.
284+
// This is only necessary if the binary wasn't cached
285+
// before this invocation of the go command: if the binary
286+
// was cached, BuiltTarget() will be the cached executable.
287+
// It's only in the "first run", where we actually do the build
288+
// and save the result to the cache that BuiltTarget is not
289+
// the cached binary. Ideally, we would set BuiltTarget
290+
// to the cached path even in the first run, but if we
291+
// copy the binary to the cached path, and try to run it
292+
// in the same process, we'll run into the dreaded #22315
293+
// resulting in occasional ETXTBSYs. Instead of getting the
294+
// ETXTBSY and then retrying just don't use the cached path
295+
// on the first run if we're going to actually run the binary.
296+
if cached := linkAction.CachedExecutable(); cached != "" {
297+
return cached
298+
}
299+
}
300+
return linkAction.BuiltTarget()
301+
}
302+
280303
func buildAndRunBuiltinTool(ctx context.Context, toolName, tool string, args []string) {
281304
// Override GOOS and GOARCH for the build to build the tool using
282305
// the same GOOS and GOARCH as this go command.
@@ -288,7 +311,7 @@ func buildAndRunBuiltinTool(ctx context.Context, toolName, tool string, args []s
288311
modload.RootMode = modload.NoRoot
289312

290313
runFunc := func(b *work.Builder, ctx context.Context, a *work.Action) error {
291-
cmdline := str.StringList(a.Deps[0].BuiltTarget(), a.Args)
314+
cmdline := str.StringList(builtTool(a), a.Args)
292315
return runBuiltTool(toolName, nil, cmdline)
293316
}
294317

@@ -300,7 +323,7 @@ func buildAndRunModtool(ctx context.Context, toolName, tool string, args []strin
300323
// Use the ExecCmd to run the binary, as go run does. ExecCmd allows users
301324
// to provide a runner to run the binary, for example a simulator for binaries
302325
// that are cross-compiled to a different platform.
303-
cmdline := str.StringList(work.FindExecCmd(), a.Deps[0].BuiltTarget(), a.Args)
326+
cmdline := str.StringList(work.FindExecCmd(), builtTool(a), a.Args)
304327
// Use same environment go run uses to start the executable:
305328
// the original environment with cfg.GOROOTbin added to the path.
306329
env := slices.Clip(cfg.OrigEnv)

src/cmd/go/internal/work/action.go

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -97,11 +97,12 @@ type Action struct {
9797
CacheExecutable bool // Whether to cache executables produced by link steps
9898

9999
// Generated files, directories.
100-
Objdir string // directory for intermediate objects
101-
Target string // goal of the action: the created package or executable
102-
built string // the actual created package or executable
103-
actionID cache.ActionID // cache ID of action input
104-
buildID string // build ID of action output
100+
Objdir string // directory for intermediate objects
101+
Target string // goal of the action: the created package or executable
102+
built string // the actual created package or executable
103+
cachedExecutable string // the cached executable, if CacheExecutable was set
104+
actionID cache.ActionID // cache ID of action input
105+
buildID string // build ID of action output
105106

106107
VetxOnly bool // Mode=="vet": only being called to supply info about dependencies
107108
needVet bool // Mode=="build": need to fill in vet config
@@ -133,6 +134,10 @@ func (a *Action) BuildID() string { return a.buildID }
133134
// from Target when the result was cached.
134135
func (a *Action) BuiltTarget() string { return a.built }
135136

137+
// CachedExecutable returns the cached executable, if CacheExecutable
138+
// was set and the executable could be cached, and "" otherwise.
139+
func (a *Action) CachedExecutable() string { return a.cachedExecutable }
140+
136141
// An actionQueue is a priority queue of actions.
137142
type actionQueue []*Action
138143

src/cmd/go/internal/work/buildid.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -745,8 +745,9 @@ func (b *Builder) updateBuildID(a *Action, target string) error {
745745
}
746746
outputID, _, err := c.PutExecutable(a.actionID, name+cfg.ExeSuffix, r)
747747
r.Close()
748+
a.cachedExecutable = c.OutputFile(outputID)
748749
if err == nil && cfg.BuildX {
749-
sh.ShowCmd("", "%s # internal", joinUnambiguously(str.StringList("cp", target, c.OutputFile(outputID))))
750+
sh.ShowCmd("", "%s # internal", joinUnambiguously(str.StringList("cp", target, a.cachedExecutable)))
750751
}
751752
}
752753
}
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
[short] skip 'does a build in using an empty cache'
2+
3+
# Start with a fresh cache because we want to verify the behavior
4+
# when the tool hasn't been cached previously.
5+
env GOCACHE=$WORK${/}cache
6+
7+
# Even when the tool hasn't been previously cached but was built and
8+
# saved to the cache in the invocation of 'go tool -n' we should return
9+
# its cached location.
10+
go tool -n foo
11+
stdout $GOCACHE
12+
13+
# And of course we should also return the cached location on subsequent
14+
# runs.
15+
go tool -n foo
16+
stdout $GOCACHE
17+
18+
-- go.mod --
19+
module example.com/foo
20+
21+
go 1.25
22+
23+
tool example.com/foo
24+
-- main.go --
25+
package main
26+
27+
func main() {}

0 commit comments

Comments
 (0)