cmd/go/internal/work: disallow testgo binary from installing to GOROOT

Installing to GOROOT makes tests non-parallelizable, since each test
depends on the installed contents of GOROOT already being up-to-date
and may reasonably assume that those contents do not change over the
course of the test.

Fixes #37573
Updates #30316

Change-Id: I2afe95ad11347bee3bb7c2d77a657db6d691cf05
Reviewed-on: https://go-review.googlesource.com/c/go/+/225897
Reviewed-by: Michael Matloob <[email protected]>

Author: Bryan C. Mills

src/cmd/go/go_test.go

@@ -199,6 +199,7 @@ func TestMain(m *testing.M) {
 			return strings.TrimSpace(string(out))
 		}
 		testGOROOT = goEnv("GOROOT")
+		os.Setenv("TESTGO_GOROOT", testGOROOT)
 
 		// The whole GOROOT/pkg tree was installed using the GOHOSTOS/GOHOSTARCH
 		// toolchain (installed in GOROOT/pkg/tool/GOHOSTOS_GOHOSTARCH).

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

@@ -8,11 +8,6 @@ package work
 
 import (
 	"bytes"
-	"cmd/go/internal/base"
-	"cmd/go/internal/cache"
-	"cmd/go/internal/cfg"
-	"cmd/go/internal/load"
-	"cmd/go/internal/str"
 	"encoding/json"
 	"errors"
 	"fmt"
@@ -30,6 +25,12 @@ import (
 	"strings"
 	"sync"
 	"time"
+
+	"cmd/go/internal/base"
+	"cmd/go/internal/cache"
+	"cmd/go/internal/cfg"
+	"cmd/go/internal/load"
+	"cmd/go/internal/str"
 )
 
 // actionList returns the list of actions in the dag rooted at root
@@ -490,6 +491,10 @@ func (b *Builder) build(a *Action) (err error) {
 		return nil
 	}
 
+	if err := allowInstall(a); err != nil {
+		return err
+	}
+
 	// make target directory
 	dir, _ := filepath.Split(a.Target)
 	if dir != "" {
@@ -1192,6 +1197,10 @@ func (b *Builder) link(a *Action) (err error) {
 		return err
 	}
 
+	if err := allowInstall(a); err != nil {
+		return err
+	}
+
 	// make target directory
 	dir, _ := filepath.Split(a.Target)
 	if dir != "" {
@@ -1366,6 +1375,10 @@ func (b *Builder) getPkgConfigFlags(p *load.Package) (cflags, ldflags []string,
 }
 
 func (b *Builder) installShlibname(a *Action) error {
+	if err := allowInstall(a); err != nil {
+		return err
+	}
+
 	// TODO: BuildN
 	a1 := a.Deps[0]
 	err := ioutil.WriteFile(a.Target, []byte(filepath.Base(a1.Target)+"\n"), 0666)
@@ -1416,6 +1429,10 @@ func (b *Builder) linkShared(a *Action) (err error) {
 	}
 	defer b.flushOutput(a)
 
+	if err := allowInstall(a); err != nil {
+		return err
+	}
+
 	if err := b.Mkdir(a.Objdir); err != nil {
 		return err
 	}
@@ -1481,8 +1498,12 @@ func BuildInstallFunc(b *Builder, a *Action) (err error) {
 		// advertise it by touching the mtimes (usually the libraries are up
 		// to date).
 		if !a.buggyInstall && !b.IsCmdList {
-			now := time.Now()
-			os.Chtimes(a.Target, now, now)
+			if cfg.BuildN {
+				b.Showcmd("", "touch %s", a.Target)
+			} else if err := allowInstall(a); err == nil {
+				now := time.Now()
+				os.Chtimes(a.Target, now, now)
+			}
 		}
 		return nil
 	}
@@ -1493,6 +1514,9 @@ func BuildInstallFunc(b *Builder, a *Action) (err error) {
 		a.built = a1.built
 		return nil
 	}
+	if err := allowInstall(a); err != nil {
+		return err
+	}
 
 	if err := b.Mkdir(a.Objdir); err != nil {
 		return err
@@ -1522,6 +1546,13 @@ func BuildInstallFunc(b *Builder, a *Action) (err error) {
 	return b.moveOrCopyFile(a.Target, a1.built, perm, false)
 }
 
+// allowInstall returns a non-nil error if this invocation of the go command is
+// allowed to install a.Target.
+//
+// (The build of cmd/go running under its own test is forbidden from installing
+// to its original GOROOT.)
+var allowInstall = func(*Action) error { return nil }
+
 // cleanup removes a's object dir to keep the amount of
 // on-disk garbage down in a large build. On an operating system
 // with aggressive buffering, cleaning incrementally like
@@ -1685,6 +1716,10 @@ func (b *Builder) installHeader(a *Action) error {
 		return nil
 	}
 
+	if err := allowInstall(a); err != nil {
+		return err
+	}
+
 	dir, _ := filepath.Split(a.Target)
 	if dir != "" {
 		if err := b.Mkdir(dir); err != nil {

src/cmd/go/internal/work/testgo.go

@@ -8,10 +8,41 @@
 
 package work
 
-import "os"
+import (
+	"cmd/go/internal/cfg"
+	"cmd/go/internal/search"
+	"fmt"
+	"os"
+	"path/filepath"
+	"runtime"
+)
 
 func init() {
 	if v := os.Getenv("TESTGO_VERSION"); v != "" {
 		runtimeVersion = v
 	}
+
+	if testGOROOT := os.Getenv("TESTGO_GOROOT"); testGOROOT != "" {
+		// Disallow installs to the GOROOT from which testgo was built.
+		// Installs to other GOROOTs — such as one set explicitly within a test — are ok.
+		allowInstall = func(a *Action) error {
+			if cfg.BuildN {
+				return nil
+			}
+
+			rel := search.InDir(a.Target, testGOROOT)
+			if rel == "" {
+				return nil
+			}
+
+			callerPos := ""
+			if _, file, line, ok := runtime.Caller(1); ok {
+				if shortFile := search.InDir(file, filepath.Join(testGOROOT, "src")); shortFile != "" {
+					file = shortFile
+				}
+				callerPos = fmt.Sprintf("%s:%d: ", file, line)
+			}
+			return fmt.Errorf("%stestgo must not write to GOROOT (installing to %s)", callerPos, filepath.Join("GOROOT", rel))
+		}
+	}
 }

src/cmd/go/script_test.go

@@ -126,6 +126,7 @@ func (ts *testScript) setup() {
 		"GOPROXY=" + proxyURL,
 		"GOPRIVATE=",
 		"GOROOT=" + testGOROOT,
+		"TESTGO_GOROOT=" + testGOROOT,
 		"GOSUMDB=" + testSumDBVerifierKey,
 		"GONOPROXY=",
 		"GONOSUMDB=",

src/cmd/go/testdata/script/README

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

src/cmd/go/testdata/script/get_update_all.txt

@@ -3,5 +3,5 @@
 
 [!net] skip
 
-go get -u .../
-! stderr 'duplicate loads of' # make sure old packages are removed from cache
+go get -u -n .../
+! stderr 'duplicate loads of' # make sure old packages are removed from cache