cmd: remove support for GOROOT_FINAL

Fixes #62047

Change-Id: If7811c1eb9073fb09b7006076998f8b2e1810bfb
Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-longtest,gotip-windows-amd64-longtest
Reviewed-on: https://go-review.googlesource.com/c/go/+/539975
Auto-Submit: Dmitri Shuralyov <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Bryan Mills <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>

Author: iwdgo

doc/next/3-tools.md

@@ -2,5 +2,11 @@
 
 ### Go command {#go-command}
 
+Setting the `GOROOT_FINAL` environment variable no longer has an effect
+([#62047](https://go.dev/issue/62047)).
+Distributions that install the `go` command to a location other than
+`$GOROOT/bin/go` should install a symlink instead of relocating
+or copying the `go` binary.
+
 ### Cgo {#cgo}
 

src/cmd/addr2line/addr2line_test.go

@@ -109,32 +109,18 @@ func testAddr2Line(t *testing.T, dbgExePath, addr string) {
 	srcPath = filepath.FromSlash(srcPath)
 	fi2, err := os.Stat(srcPath)
 
-	// If GOROOT_FINAL is set and srcPath is not the file we expect, perhaps
-	// srcPath has had GOROOT_FINAL substituted for GOROOT and GOROOT hasn't been
-	// moved to its final location yet. If so, try the original location instead.
-	if gorootFinal := os.Getenv("GOROOT_FINAL"); gorootFinal != "" &&
-		(os.IsNotExist(err) || (err == nil && !os.SameFile(fi1, fi2))) {
-		// srcPath is clean, but GOROOT_FINAL itself might not be.
-		// (See https://golang.org/issue/41447.)
-		gorootFinal = filepath.Clean(gorootFinal)
-
-		if strings.HasPrefix(srcPath, gorootFinal) {
-			fi2, err = os.Stat(runtime.GOROOT() + strings.TrimPrefix(srcPath, gorootFinal))
-		}
-	}
-
 	if err != nil {
 		t.Fatalf("Stat failed: %v", err)
 	}
 	if !os.SameFile(fi1, fi2) {
 		t.Fatalf("addr2line_test.go and %s are not same file", srcPath)
 	}
-	if srcLineNo != "138" {
-		t.Fatalf("line number = %v; want 138", srcLineNo)
+	if want := "124"; srcLineNo != want {
+		t.Fatalf("line number = %v; want %s", srcLineNo, want)
 	}
 }
 
-// This is line 137. The test depends on that.
+// This is line 123. The test depends on that.
 func TestAddr2Line(t *testing.T) {
 	testenv.MustHaveGoBuild(t)
 

src/cmd/dist/build.go

@@ -40,7 +40,6 @@ var (
 	goppc64          string
 	goriscv64        string
 	goroot           string
-	goroot_final     string
 	goextlinkenabled string
 	gogcflags        string // For running built compiler
 	goldflags        string
@@ -127,12 +126,6 @@ func xinit() {
 	// All exec calls rewrite "go" into gorootBinGo.
 	gorootBinGo = pathf("%s/bin/go", goroot)
 
-	b = os.Getenv("GOROOT_FINAL")
-	if b == "" {
-		b = goroot
-	}
-	goroot_final = b
-
 	b = os.Getenv("GOOS")
 	if b == "" {
 		b = gohostos
@@ -245,7 +238,6 @@ func xinit() {
 	os.Setenv("GOPPC64", goppc64)
 	os.Setenv("GORISCV64", goriscv64)
 	os.Setenv("GOROOT", goroot)
-	os.Setenv("GOROOT_FINAL", goroot_final)
 
 	// Set GOBIN to GOROOT/bin. The meaning of GOBIN has drifted over time
 	// (see https://go.dev/issue/3269, https://go.dev/cl/183058,
@@ -1879,10 +1871,7 @@ func banner() {
 	xprintf("Installed Go for %s/%s in %s\n", goos, goarch, goroot)
 	xprintf("Installed commands in %s\n", gorootBin)
 
-	if !xsamefile(goroot_final, goroot) {
-		// If the files are to be moved, don't check that gobin
-		// is on PATH; assume they know what they are doing.
-	} else if gohostos == "plan9" {
+	if gohostos == "plan9" {
 		// Check that GOROOT/bin is bound before /bin.
 		pid := strings.Replace(readfile("#c/pid"), " ", "", -1)
 		ns := fmt.Sprintf("/proc/%s/ns", pid)
@@ -1907,12 +1896,6 @@ func banner() {
 			xprintf("*** You need to add %s to your PATH.\n", gorootBin)
 		}
 	}
-
-	if !xsamefile(goroot_final, goroot) {
-		xprintf("\n"+
-			"The binaries expect %s to be copied or moved to %s\n",
-			goroot, goroot_final)
-	}
 }
 
 // Version prints the Go version.

src/cmd/go/alldocs.go

@@ -90,10 +90,6 @@ func tooSlow(t *testing.T, reason string) {
 // (temp) directory.
 var testGOROOT string
 
-// testGOROOT_FINAL is the GOROOT_FINAL with which the test binary is assumed to
-// have been built.
-var testGOROOT_FINAL = os.Getenv("GOROOT_FINAL")
-
 var testGOCACHE string
 
 var testGo string
@@ -223,10 +219,6 @@ func TestMain(m *testing.M) {
 		}
 		testGOROOT = goEnv("GOROOT")
 		os.Setenv("TESTGO_GOROOT", testGOROOT)
-		// Ensure that GOROOT is set explicitly.
-		// Otherwise, if the toolchain was built with GOROOT_FINAL set but has not
-		// yet been moved to its final location, programs that invoke runtime.GOROOT
-		// may accidentally use the wrong path.
 		os.Setenv("GOROOT", testGOROOT)
 
 		// The whole GOROOT/pkg tree was installed using the GOHOSTOS/GOHOSTARCH

src/cmd/go/go_test.go

@@ -199,7 +199,6 @@ func SetGOROOT(goroot string, isTestGo bool) {
 		GOROOTpkg = filepath.Join(goroot, "pkg")
 		GOROOTsrc = filepath.Join(goroot, "src")
 	}
-	GOROOT_FINAL = findGOROOT_FINAL(goroot)
 
 	installedGOOS = runtime.GOOS
 	installedGOARCH = runtime.GOARCH
@@ -398,8 +397,6 @@ var (
 	GOROOTpkg string
 	GOROOTsrc string
 
-	GOROOT_FINAL string
-
 	GOBIN      = Getenv("GOBIN")
 	GOMODCACHE = envOr("GOMODCACHE", gopathDir("pkg/mod"))
 
@@ -532,16 +529,6 @@ func findGOROOT(env string) string {
 	return def
 }
 
-func findGOROOT_FINAL(goroot string) string {
-	// $GOROOT_FINAL is only for use during make.bash
-	// so it is not settable using go/env, so we use os.Getenv here.
-	def := goroot
-	if env := os.Getenv("GOROOT_FINAL"); env != "" {
-		def = filepath.Clean(env)
-	}
-	return def
-}
-
 // isSameDir reports whether dir1 and dir2 are the same directory.
 func isSameDir(dir1, dir2 string) bool {
 	if dir1 == dir2 {

src/cmd/go/internal/cfg/cfg.go

@@ -645,11 +645,6 @@ Special-purpose environment variables:
 		See src/internal/goexperiment/flags.go for currently valid values.
 		Warning: This variable is provided for the development and testing
 		of the Go toolchain itself. Use beyond that purpose is unsupported.
-	GOROOT_FINAL
-		The root of the installed Go tree, when it is
-		installed in a location other than where it is built.
-		File names in stack traces are rewritten from GOROOT to
-		GOROOT_FINAL.
 	GO_EXTLINK_ENABLED
 		Whether the linker should use external linking mode
 		when using -linkmode=auto with code that uses cgo.

src/cmd/go/internal/help/helpdoc.go

@@ -258,9 +258,12 @@ func (b *Builder) buildActionID(a *Action) cache.ActionID {
 		// when building things in GOROOT.
 		//
 		// The C compiler does not, but for packages in GOROOT we rewrite the path
-		// as though -trimpath were set, so that we don't invalidate the build cache
-		// (and especially any precompiled C archive files) when changing
-		// GOROOT_FINAL. (See https://go.dev/issue/50183.)
+		// as though -trimpath were set. This used to be so that we did not invalidate
+		// the build cache (and especially precompiled archive files) when changing
+		// GOROOT_FINAL, but we no longer ship precompiled archive files as of Go 1.20
+		// (https://go.dev/issue/47257) and no longer support GOROOT_FINAL
+		// (https://go.dev/issue/62047).
+		// TODO(bcmills): Figure out whether this behavior is still useful.
 		//
 		// b.WorkDir is always either trimmed or rewritten to
 		// the literal string "/tmp/go-build".
@@ -1403,11 +1406,11 @@ func (b *Builder) printLinkerConfig(h io.Writer, p *load.Package) {
 			fmt.Fprintf(h, "GOEXPERIMENT=%q\n", cfg.CleanGOEXPERIMENT)
 		}
 
-		// The linker writes source file paths that say GOROOT_FINAL, but
-		// only if -trimpath is not specified (see ld() in gc.go).
-		gorootFinal := cfg.GOROOT_FINAL
+		// The linker writes source file paths that refer to GOROOT,
+		// but only if -trimpath is not specified (see [gctoolchain.ld] in gc.go).
+		gorootFinal := cfg.GOROOT
 		if cfg.BuildTrimpath {
-			gorootFinal = trimPathGoRootFinal
+			gorootFinal = ""
 		}
 		fmt.Fprintf(h, "GOROOT=%s\n", gorootFinal)
 
@@ -2130,7 +2133,7 @@ func (b *Builder) ccompile(a *Action, outfile string, flags []string, file strin
 	file = mkAbs(p.Dir, file)
 	outfile = mkAbs(p.Dir, outfile)
 
-	// Elide source directory paths if -trimpath or GOROOT_FINAL is set.
+	// Elide source directory paths if -trimpath is set.
 	// This is needed for source files (e.g., a .c file in a package directory).
 	// TODO(golang.org/issue/36072): cgo also generates files with #line
 	// directives pointing to the source directory. It should not generate those

src/cmd/go/internal/work/exec.go

@@ -29,9 +29,6 @@ import (
 // Tests can override this by setting $TESTGO_TOOLCHAIN_VERSION.
 var ToolchainVersion = runtime.Version()
 
-// The 'path' used for GOROOT_FINAL when -trimpath is specified
-const trimPathGoRootFinal string = "$GOROOT"
-
 // The Go toolchain.
 
 type gcToolchain struct{}
@@ -669,8 +666,11 @@ func (gcToolchain) ld(b *Builder, root *Action, targetPath, importcfg, mainpkg s
 	}
 
 	env := []string{}
+	// When -trimpath is used, GOROOT is cleared
 	if cfg.BuildTrimpath {
-		env = append(env, "GOROOT_FINAL="+trimPathGoRootFinal)
+		env = append(env, "GOROOT=")
+	} else {
+		env = append(env, "GOROOT="+cfg.GOROOT)
 	}
 	return b.Shell(root).run(dir, root.Package.ImportPath, env, cfg.BuildToolexec, base.Tool("link"), "-o", targetPath, "-importcfg", importcfg, ldflags, mainpkg)
 }

src/cmd/go/internal/work/gc.go

@@ -223,7 +223,6 @@ func scriptEnv(srv *vcstest.Server, srvCertFile string) ([]string, error) {
 		"GOPROXY=" + proxyURL,
 		"GOPRIVATE=",
 		"GOROOT=" + testGOROOT,
-		"GOROOT_FINAL=" + testGOROOT_FINAL, // causes spurious rebuilds and breaks the "stale" built-in if not propagated
 		"GOTRACEBACK=system",
 		"TESTGONETWORK=panic", // allow only local connections by default; the [net] condition resets this
 		"TESTGO_GOROOT=" + testGOROOT,

src/cmd/go/script_test.go

@@ -51,7 +51,6 @@ func scriptConditions() map[string]script.Cond {
 	add("GOEXPERIMENT", script.PrefixCondition("GOEXPERIMENT <suffix> is enabled", hasGoexperiment))
 	add("go-builder", script.BoolCondition("GO_BUILDER_NAME is non-empty", testenv.Builder() != ""))
 	add("link", lazyBool("testenv.HasLink()", testenv.HasLink))
-	add("mismatched-goroot", script.Condition("test's GOROOT_FINAL does not match the real GOROOT", isMismatchedGoroot))
 	add("msan", sysCondition("-msan", platform.MSanSupported, true))
 	add("mustlinkext", script.Condition("platform always requires external linking", mustLinkExt))
 	add("net", script.PrefixCondition("can connect to external network host <suffix>", hasNet))
@@ -85,14 +84,6 @@ func ccIs(s *script.State, want string) (bool, error) {
 	return cfg.DefaultCC(GOOS, GOARCH) == want, nil
 }
 
-func isMismatchedGoroot(s *script.State) (bool, error) {
-	gorootFinal, _ := s.LookupEnv("GOROOT_FINAL")
-	if gorootFinal == "" {
-		gorootFinal, _ = s.LookupEnv("GOROOT")
-	}
-	return gorootFinal != testGOROOT, nil
-}
-
 func sysCondition(flag string, f func(goos, goarch string) bool, needsCgo bool) script.Cond {
 	return script.Condition(
 		"GOOS/GOARCH supports "+flag,

src/cmd/go/scriptconds_test.go

@@ -120,7 +120,6 @@ Scripts also have access to other environment variables, including:
 	GOPATH=$WORK/gopath
 	GOPROXY=<local module proxy serving from cmd/go/testdata/mod>
 	GOROOT=<actual GOROOT>
-	GOROOT_FINAL=<actual GOROOT_FINAL>
 	TESTGO_GOROOT=<GOROOT used to build cmd/go, for use in tests that may change GOROOT>
 	HOME=/no-home
 	PATH=<actual PATH>

src/cmd/go/scriptreadme_test.go

@@ -35,7 +35,6 @@ Scripts also have access to other environment variables, including:
 	GOPATH=$WORK/gopath
 	GOPROXY=<local module proxy serving from cmd/go/testdata/mod>
 	GOROOT=<actual GOROOT>
-	GOROOT_FINAL=<actual GOROOT_FINAL>
 	TESTGO_GOROOT=<GOROOT used to build cmd/go, for use in tests that may change GOROOT>
 	HOME=/no-home
 	PATH=<actual PATH>
@@ -402,8 +401,6 @@ The available conditions are:
 	GO_BUILDER_NAME is non-empty
 [link]
 	testenv.HasLink()
-[mismatched-goroot]
-	test's GOROOT_FINAL does not match the real GOROOT
 [msan]
 	GOOS/GOARCH supports -msan
 [mustlinkext]