Skip to content

Commit a0bebff

Browse files
Bryan C. Millsgopherbot
Bryan C. Mills
authored andcommitted
cmd/dist: consistently use $GOROOT/bin/go instead of just "go"
Also remove existing special cases that transform "go" into gorootBinGo, because they make debugging and code-reviews more difficult: log messages that don't include the full path can mask bugs like #31567, and the reader of the code has to trace through the various call chains to verify that the correct "go" is being used. Instead, we can make the use of the correct "go" command plainly obvious in the code by using one consistent name for it. (Prior to this CL, we had three different names for it: gorootBinGo, "go", and cmdGo. Now we have only one. Updates #31567. Change-Id: Ia9ff27e5e800c79af5a4e9f2803c9ea5ccafbf35 Reviewed-on: https://go-review.googlesource.com/c/go/+/452678 Reviewed-by: Michael Matloob <[email protected]> Reviewed-by: Austin Clements <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Auto-Submit: Bryan Mills <[email protected]> Run-TryBot: Bryan Mills <[email protected]>
1 parent 13c70b1 commit a0bebff

File tree

3 files changed

+19
-27
lines changed

3 files changed

+19
-27
lines changed

src/cmd/dist/build.go

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1405,7 +1405,6 @@ func cmdbootstrap() {
14051405
setNoOpt()
14061406
goldflags = os.Getenv("GO_LDFLAGS") // we were using $BOOT_GO_LDFLAGS until now
14071407
goBootstrap := pathf("%s/go_bootstrap", tooldir)
1408-
cmdGo := pathf("%s/go", gorootBin)
14091408
if debug {
14101409
run("", ShowOutput|CheckExit, pathf("%s/compile", tooldir), "-V=full")
14111410
copyfile(pathf("%s/compile1", tooldir), pathf("%s/compile", tooldir), writeExec)
@@ -1488,8 +1487,8 @@ func cmdbootstrap() {
14881487
goInstall(toolenv, goBootstrap, "cmd")
14891488
checkNotStale(nil, goBootstrap, "std")
14901489
checkNotStale(toolenv, goBootstrap, "cmd")
1491-
checkNotStale(nil, cmdGo, "std")
1492-
checkNotStale(toolenv, cmdGo, "cmd")
1490+
checkNotStale(nil, gorootBinGo, "std")
1491+
checkNotStale(toolenv, gorootBinGo, "cmd")
14931492

14941493
timelog("build", "target toolchain")
14951494
if vflag > 0 {
@@ -1507,8 +1506,8 @@ func cmdbootstrap() {
15071506
checkNotStale(toolenv, goBootstrap, append(toolchain, "runtime/internal/sys")...)
15081507
checkNotStale(nil, goBootstrap, "std")
15091508
checkNotStale(toolenv, goBootstrap, "cmd")
1510-
checkNotStale(nil, cmdGo, "std")
1511-
checkNotStale(toolenv, cmdGo, "cmd")
1509+
checkNotStale(nil, gorootBinGo, "std")
1510+
checkNotStale(toolenv, gorootBinGo, "cmd")
15121511
if debug {
15131512
run("", ShowOutput|CheckExit, pathf("%s/compile", tooldir), "-V=full")
15141513
checkNotStale(toolenv, goBootstrap, append(toolchain, "runtime/internal/sys")...)
@@ -1542,7 +1541,7 @@ func cmdbootstrap() {
15421541
os.Setenv("GOOS", gohostos)
15431542
os.Setenv("GOARCH", gohostarch)
15441543
os.Setenv("CC", compilerEnvLookup("CC", defaultcc, gohostos, gohostarch))
1545-
goCmd(nil, cmdGo, "build", "-o", pathf("%s/go_%s_%s_exec%s", gorootBin, goos, goarch, exe), wrapperPath)
1544+
goCmd(nil, gorootBinGo, "build", "-o", pathf("%s/go_%s_%s_exec%s", gorootBin, goos, goarch, exe), wrapperPath)
15461545
// Restore environment.
15471546
// TODO(elias.naur): support environment variables in goCmd?
15481547
os.Setenv("GOOS", goos)

src/cmd/dist/test.go

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ func (t *tester) run() {
149149
if t.rebuild {
150150
t.out("Building packages and commands.")
151151
// Force rebuild the whole toolchain.
152-
goInstall(toolenv, "go", append([]string{"-a"}, toolchain...)...)
152+
goInstall(toolenv, gorootBinGo, append([]string{"-a"}, toolchain...)...)
153153
}
154154

155155
if !t.listMode {
@@ -166,10 +166,10 @@ func (t *tester) run() {
166166
// to break if we don't automatically refresh things here.
167167
// Rebuilding is a shortened bootstrap.
168168
// See cmdbootstrap for a description of the overall process.
169-
goInstall(toolenv, "go", toolchain...)
170-
goInstall(toolenv, "go", toolchain...)
171-
goInstall(toolenv, "go", "cmd")
172-
goInstall(nil, "go", "std")
169+
goInstall(toolenv, gorootBinGo, toolchain...)
170+
goInstall(toolenv, gorootBinGo, toolchain...)
171+
goInstall(toolenv, gorootBinGo, "cmd")
172+
goInstall(nil, gorootBinGo, "std")
173173
} else {
174174
// The Go builder infrastructure should always begin running tests from a
175175
// clean, non-stale state, so there is no need to rebuild the world.
@@ -179,15 +179,15 @@ func (t *tester) run() {
179179
// The cache used by dist when building is different from that used when
180180
// running dist test, so rebuild (but don't install) std and cmd to make
181181
// sure packages without install targets are cached so they are not stale.
182-
goCmd(toolenv, "go", "build", "cmd") // make sure dependencies of targets are cached
183-
goCmd(nil, "go", "build", "std")
184-
checkNotStale(nil, "go", "std")
182+
goCmd(toolenv, gorootBinGo, "build", "cmd") // make sure dependencies of targets are cached
183+
goCmd(nil, gorootBinGo, "build", "std")
184+
checkNotStale(nil, gorootBinGo, "std")
185185
if builder != "aix-ppc64" {
186186
// The aix-ppc64 builder for some reason does not have deterministic cgo
187187
// builds, so "cmd" is stale. Fortunately, most of the tests don't care.
188188
// TODO(#56896): remove this special case once the builder supports
189189
// determistic cgo builds.
190-
checkNotStale(toolenv, "go", "cmd")
190+
checkNotStale(toolenv, gorootBinGo, "cmd")
191191
}
192192
}
193193
}
@@ -300,7 +300,7 @@ func (t *tester) maybeLogMetadata() error {
300300
//
301301
// TODO(prattmic): If we split dist bootstrap and dist test then this
302302
// could be simplified to directly use internal/sysinfo here.
303-
return t.dirCmd(filepath.Join(goroot, "src/cmd/internal/metadata"), "go", []string{"run", "main.go"}).Run()
303+
return t.dirCmd(filepath.Join(goroot, "src/cmd/internal/metadata"), gorootBinGo, []string{"run", "main.go"}).Run()
304304
}
305305

306306
// goTest represents all options to a "go test" command. The final command will
@@ -1077,9 +1077,6 @@ func flattenCmdline(cmdline []interface{}) (bin string, args []string) {
10771077
}
10781078

10791079
bin = list[0]
1080-
if bin == "go" {
1081-
bin = gorootBinGo
1082-
}
10831080
return bin, list[1:]
10841081
}
10851082

@@ -1354,7 +1351,7 @@ func (t *tester) registerCgoTests() {
13541351
// running in parallel with earlier tests, or if it has some other reason
13551352
// for needing the earlier tests to be done.
13561353
func (t *tester) runPending(nextTest *distTest) {
1357-
checkNotStale(nil, "go", "std")
1354+
checkNotStale(nil, gorootBinGo, "std")
13581355
worklist := t.worklist
13591356
t.worklist = nil
13601357
for _, w := range worklist {
@@ -1412,7 +1409,7 @@ func (t *tester) runPending(nextTest *distTest) {
14121409
log.Printf("Failed: %v", w.err)
14131410
t.failed = true
14141411
}
1415-
checkNotStale(nil, "go", "std")
1412+
checkNotStale(nil, gorootBinGo, "std")
14161413
}
14171414
if t.failed && !t.keepGoing {
14181415
fatalf("FAILED")
@@ -1612,7 +1609,7 @@ func (t *tester) testDirTest(dt *distTest, shard, shards int) error {
16121609
os.Remove(runtest.exe)
16131610
})
16141611

1615-
cmd := t.dirCmd("test", "go", "build", "-o", runtest.exe, "run.go")
1612+
cmd := t.dirCmd("test", gorootBinGo, "build", "-o", runtest.exe, "run.go")
16161613
setEnv(cmd, "GOOS", gohostos)
16171614
setEnv(cmd, "GOARCH", gohostarch)
16181615
runtest.err = cmd.Run()

src/cmd/dist/util.go

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -75,11 +75,7 @@ func runEnv(dir string, mode int, env []string, cmd ...string) string {
7575
errprintf("run: %s\n", strings.Join(cmd, " "))
7676
}
7777

78-
bin := cmd[0]
79-
if bin == "go" {
80-
bin = gorootBinGo
81-
}
82-
xcmd := exec.Command(bin, cmd[1:]...)
78+
xcmd := exec.Command(cmd[0], cmd[1:]...)
8379
if env != nil {
8480
xcmd.Env = append(os.Environ(), env...)
8581
}

0 commit comments

Comments
 (0)