Skip to content

Commit 2beb173

Browse files
rscrsc
committed
all: respect $GO_GCFLAGS during run.bash
If the go install doesn't use the same flags as the main build it can overwrite the installed standard library, leading to flakiness and slow future tests. Force uses of 'go install' etc to propagate $GO_GCFLAGS or disable them entirely, to avoid problems. As I understand it, the main place this happens is the ssacheck builder. If there are other uses that need to run some of the now-disabled tests we can reenable fixed tests in followup CLs. Change-Id: Ib860a253539f402f8a96a3c00ec34f0bbf137c9a Reviewed-on: https://go-review.googlesource.com/74470 Reviewed-by: David Crawshaw <crawshaw@golang.org>
1 parent 99be9cc commit 2beb173

File tree

6 files changed

+40
-10
lines changed

6 files changed

+40
-10
lines changed

‎src/cmd/dist/test.go‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -578,7 +578,12 @@ func (t *tester) registerTests() {
578578
if t.hasBash() && t.cgoEnabled && goos != "android" && goos != "darwin" {
579579
t.registerTest("testgodefs", "../misc/cgo/testgodefs", "./test.bash")
580580
}
581-
if t.cgoEnabled {
581+
582+
// Don't run these tests with $GO_GCFLAGS because most of them
583+
// assume that they can run "go install" with no -gcflags and not
584+
// recompile the entire standard library. If make.bash ran with
585+
// special -gcflags, that's not true.
586+
if t.cgoEnabled && gogcflags == "" {
582587
if t.cgoTestSOSupported() {
583588
t.tests = append(t.tests, distTest{
584589
name: "testso",

‎src/cmd/go/go_test.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,13 @@ var testCC string
8686
// The TestMain function creates a go command for testing purposes and
8787
// deletes it after the tests have been run.
8888
func TestMain(m *testing.M) {
89+
if os.Getenv("GO_GCFLAGS") != "" {
90+
fmt.Fprintf(os.Stderr, "testing: warning: no tests to run\n") // magic string for cmd/go
91+
fmt.Printf("cmd/go test is not compatible with $GO_GCFLAGS being set\n")
92+
fmt.Printf("SKIP\n")
93+
return
94+
}
95+
8996
if canRun {
9097
args := []string{"build", "-tags", "testgo", "-o", "testgo" + exeSuffix}
9198
if race.Enabled {

‎src/cmd/internal/goobj/goobj_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ func buildGoobj() error {
133133
if err != nil {
134134
return err
135135
}
136-
cmd := exec.Command(gotool, "install", "mycgo")
136+
cmd := exec.Command(gotool, "install", "-gcflags="+os.Getenv("GO_GCFLAGS"), "mycgo")
137137
cmd.Env = append(os.Environ(), "GOPATH="+gopath)
138138
out, err = cmd.CombinedOutput()
139139
if err != nil {

‎src/internal/testenv/testenv.go‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,13 @@ func Builder() string {
3333
// HasGoBuild reports whether the current system can build programs with ``go build''
3434
// and then run them with os.StartProcess or exec.Command.
3535
func HasGoBuild() bool {
36+
if os.Getenv("GO_GCFLAGS") != "" {
37+
// It's too much work to require every caller of the go command
38+
// to pass along "-gcflags="+os.Getenv("GO_GCFLAGS").
39+
// For now, if $GO_GCFLAGS is set, report that we simply can't
40+
// run go build.
41+
return false
42+
}
3643
switch runtime.GOOS {
3744
case "android", "nacl":
3845
return false
@@ -48,6 +55,9 @@ func HasGoBuild() bool {
4855
// and then run them with os.StartProcess or exec.Command.
4956
// If not, MustHaveGoBuild calls t.Skip with an explanation.
5057
func MustHaveGoBuild(t testing.TB) {
58+
if os.Getenv("GO_GCFLAGS") != "" {
59+
t.Skipf("skipping test: 'go build' not compatible with setting $GO_GCFLAGS")
60+
}
5161
if !HasGoBuild() {
5262
t.Skipf("skipping test: 'go build' not available on %s/%s", runtime.GOOS, runtime.GOARCH)
5363
}

‎src/runtime/crash_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -140,14 +140,14 @@ var (
140140
func checkStaleRuntime(t *testing.T) {
141141
staleRuntimeOnce.Do(func() {
142142
// 'go run' uses the installed copy of runtime.a, which may be out of date.
143-
out, err := testenv.CleanCmdEnv(exec.Command(testenv.GoToolPath(t), "list", "-f", "{{.Stale}}", "runtime")).CombinedOutput()
143+
out, err := testenv.CleanCmdEnv(exec.Command(testenv.GoToolPath(t), "list", "-gcflags="+os.Getenv("GO_GCFLAGS"), "-f", "{{.Stale}}", "runtime")).CombinedOutput()
144144
if err != nil {
145145
staleRuntimeErr = fmt.Errorf("failed to execute 'go list': %v\n%v", err, string(out))
146146
return
147147
}
148148
if string(out) != "false\n" {
149149
t.Logf("go list -f {{.Stale}} runtime:\n%s", out)
150-
out, err := testenv.CleanCmdEnv(exec.Command(testenv.GoToolPath(t), "list", "-f", "{{.StaleReason}}", "runtime")).CombinedOutput()
150+
out, err := testenv.CleanCmdEnv(exec.Command(testenv.GoToolPath(t), "list", "-gcflags="+os.Getenv("GO_GCFLAGS"), "-f", "{{.StaleReason}}", "runtime")).CombinedOutput()
151151
if err != nil {
152152
t.Logf("go list -f {{.StaleReason}} failed: %v", err)
153153
}

‎test/run.go‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -417,6 +417,14 @@ func (ctxt *context) match(name string) bool {
417417

418418
func init() { checkShouldTest() }
419419

420+
// goGcflags returns the -gcflags argument to use with go build / go run.
421+
// This must match the flags used for building the standard libary,
422+
// or else the commands will rebuild any needed packages (like runtime)
423+
// over and over.
424+
func goGcflags() string {
425+
return "-gcflags=" + os.Getenv("GO_GCFLAGS")
426+
}
427+
420428
// run runs a test.
421429
func (t *test) run() {
422430
start := time.Now()
@@ -701,7 +709,7 @@ func (t *test) run() {
701709
}
702710

703711
case "build":
704-
_, err := runcmd("go", "build", "-o", "a.exe", long)
712+
_, err := runcmd("go", "build", goGcflags(), "-o", "a.exe", long)
705713
if err != nil {
706714
t.err = err
707715
}
@@ -766,7 +774,7 @@ func (t *test) run() {
766774
case "buildrun": // build binary, then run binary, instead of go run. Useful for timeout tests where failure mode is infinite loop.
767775
// TODO: not supported on NaCl
768776
useTmp = true
769-
cmd := []string{"go", "build", "-o", "a.exe"}
777+
cmd := []string{"go", "build", goGcflags(), "-o", "a.exe"}
770778
if *linkshared {
771779
cmd = append(cmd, "-linkshared")
772780
}
@@ -791,7 +799,7 @@ func (t *test) run() {
791799

792800
case "run":
793801
useTmp = false
794-
cmd := []string{"go", "run"}
802+
cmd := []string{"go", "run", goGcflags()}
795803
if *linkshared {
796804
cmd = append(cmd, "-linkshared")
797805
}
@@ -812,7 +820,7 @@ func (t *test) run() {
812820
<-rungatec
813821
}()
814822
useTmp = false
815-
cmd := []string{"go", "run"}
823+
cmd := []string{"go", "run", goGcflags()}
816824
if *linkshared {
817825
cmd = append(cmd, "-linkshared")
818826
}
@@ -827,7 +835,7 @@ func (t *test) run() {
827835
t.err = fmt.Errorf("write tempfile:%s", err)
828836
return
829837
}
830-
cmd = []string{"go", "run"}
838+
cmd = []string{"go", "run", goGcflags()}
831839
if *linkshared {
832840
cmd = append(cmd, "-linkshared")
833841
}
@@ -843,7 +851,7 @@ func (t *test) run() {
843851

844852
case "errorcheckoutput":
845853
useTmp = false
846-
cmd := []string{"go", "run"}
854+
cmd := []string{"go", "run", goGcflags()}
847855
if *linkshared {
848856
cmd = append(cmd, "-linkshared")
849857
}

0 commit comments

Comments
 (0)