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 <matloob@golang.org>
diff --git a/src/cmd/go/go_test.go b/src/cmd/go/go_test.go
index 641cab8..39e387b 100644
--- a/src/cmd/go/go_test.go
+++ b/src/cmd/go/go_test.go
@@ -199,6 +199,7 @@
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).
diff --git a/src/cmd/go/internal/work/exec.go b/src/cmd/go/internal/work/exec.go
index d781ad2..dbe31a6 100644
--- a/src/cmd/go/internal/work/exec.go
+++ b/src/cmd/go/internal/work/exec.go
@@ -8,11 +8,6 @@
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 @@
"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 @@
return nil
}
+ if err := allowInstall(a); err != nil {
+ return err
+ }
+
// make target directory
dir, _ := filepath.Split(a.Target)
if dir != "" {
@@ -1192,6 +1197,10 @@
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) 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 @@
}
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 @@
// 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 @@
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 @@
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 @@
return nil
}
+ if err := allowInstall(a); err != nil {
+ return err
+ }
+
dir, _ := filepath.Split(a.Target)
if dir != "" {
if err := b.Mkdir(dir); err != nil {
diff --git a/src/cmd/go/internal/work/testgo.go b/src/cmd/go/internal/work/testgo.go
index 3e623c6..931f49a 100644
--- a/src/cmd/go/internal/work/testgo.go
+++ b/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))
+ }
+ }
}
diff --git a/src/cmd/go/script_test.go b/src/cmd/go/script_test.go
index 87afb6a..ebadce8 100644
--- a/src/cmd/go/script_test.go
+++ b/src/cmd/go/script_test.go
@@ -126,6 +126,7 @@
"GOPROXY=" + proxyURL,
"GOPRIVATE=",
"GOROOT=" + testGOROOT,
+ "TESTGO_GOROOT=" + testGOROOT,
"GOSUMDB=" + testSumDBVerifierKey,
"GONOPROXY=",
"GONOSUMDB=",
diff --git a/src/cmd/go/testdata/script/README b/src/cmd/go/testdata/script/README
index f4c92e6..e22ddca 100644
--- a/src/cmd/go/testdata/script/README
+++ b/src/cmd/go/testdata/script/README
@@ -34,6 +34,7 @@
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
diff --git a/src/cmd/go/testdata/script/get_update_all.txt b/src/cmd/go/testdata/script/get_update_all.txt
index 1f2f5bf..d0b9860 100644
--- a/src/cmd/go/testdata/script/get_update_all.txt
+++ b/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
\ No newline at end of file
+go get -u -n .../
+! stderr 'duplicate loads of' # make sure old packages are removed from cache
