cmd/dist: fix GOROOT permissions on failure
While running various tests for #28387, I keep ending up with an
unwritable GOROOT after a failure.
While the unwritable GOROOT is a fairly exotic condition (normally
only happens on builders), it's somewhat annoying when debugging, so
I'm switching all of the log.Fatal* call sites to use the existing
fatalf function, which supports general atexit-like cleanup.
Updates #28387
Change-Id: I473cda7eacd9ad82bdeab647766373126dc7390e
Reviewed-on: https://go-review.googlesource.com/c/go/+/207341
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
diff --git a/src/cmd/dist/test.go b/src/cmd/dist/test.go
index 8e7106d..dc22aad 100644
--- a/src/cmd/dist/test.go
+++ b/src/cmd/dist/test.go
@@ -100,11 +100,11 @@
slurp, err := exec.Command("go", "env", "CGO_ENABLED").Output()
if err != nil {
- log.Fatalf("Error running go env CGO_ENABLED: %v", err)
+ fatalf("Error running go env CGO_ENABLED: %v", err)
}
t.cgoEnabled, _ = strconv.ParseBool(strings.TrimSpace(string(slurp)))
if flag.NArg() > 0 && t.runRxStr != "" {
- log.Fatalf("the -run regular expression flag is mutually exclusive with test name arguments")
+ fatalf("the -run regular expression flag is mutually exclusive with test name arguments")
}
t.runNames = flag.Args()
@@ -154,7 +154,7 @@
if s := os.Getenv("GO_TEST_TIMEOUT_SCALE"); s != "" {
t.timeoutScale, err = strconv.Atoi(s)
if err != nil {
- log.Fatalf("failed to parse $GO_TEST_TIMEOUT_SCALE = %q as integer: %v", s, err)
+ fatalf("failed to parse $GO_TEST_TIMEOUT_SCALE = %q as integer: %v", s, err)
}
}
@@ -187,18 +187,17 @@
for _, name := range t.runNames {
if !t.isRegisteredTestName(name) {
- log.Fatalf("unknown test %q", name)
+ fatalf("unknown test %q", name)
}
}
// On a few builders, make GOROOT unwritable to catch tests writing to it.
- restoreGOROOT := func() {}
if strings.HasPrefix(os.Getenv("GO_BUILDER_NAME"), "linux-") {
if os.Getuid() == 0 {
// Don't bother making GOROOT unwritable:
// we're running as root, so permissions would have no effect.
} else {
- restoreGOROOT = t.makeGOROOTUnwritable()
+ xatexit(t.makeGOROOTUnwritable())
}
}
@@ -214,21 +213,19 @@
if t.keepGoing {
log.Printf("Failed: %v", err)
} else {
- restoreGOROOT()
- log.Fatalf("Failed: %v", err)
+ fatalf("Failed: %v", err)
}
}
}
t.runPending(nil)
- restoreGOROOT()
timelog("end", "dist test")
if t.failed {
fmt.Println("\nFAILED")
- os.Exit(1)
+ xexit(1)
} else if incomplete[goos+"/"+goarch] {
fmt.Println("\nFAILED (incomplete port)")
- os.Exit(1)
+ xexit(1)
} else if t.partial {
fmt.Println("\nALL TESTS PASSED (some were excluded)")
} else {
@@ -262,7 +259,7 @@
if v := os.Getenv("GO_TEST_SHORT"); v != "" {
short, err := strconv.ParseBool(v)
if err != nil {
- log.Fatalf("invalid GO_TEST_SHORT %q: %v", v, err)
+ fatalf("invalid GO_TEST_SHORT %q: %v", v, err)
}
if !short {
return "-short=false"
@@ -433,7 +430,7 @@
cmd.Stderr = new(bytes.Buffer)
all, err := cmd.Output()
if err != nil {
- log.Fatalf("Error running go list std cmd: %v:\n%s", err, cmd.Stderr)
+ fatalf("Error running go list std cmd: %v:\n%s", err, cmd.Stderr)
}
pkgs := strings.Fields(string(all))
for _, pkg := range pkgs {
@@ -545,7 +542,7 @@
err := cmd.Run()
if rerr := os.Rename(moved, goroot); rerr != nil {
- log.Fatalf("failed to restore GOROOT: %v", rerr)
+ fatalf("failed to restore GOROOT: %v", rerr)
}
return err
},
@@ -995,7 +992,7 @@
return false
default:
- log.Fatalf("internal error: unknown buildmode %s", mode)
+ fatalf("internal error: unknown buildmode %s", mode)
return false
}
}
@@ -1190,7 +1187,7 @@
checkNotStale("go", "std")
}
if t.failed && !t.keepGoing {
- log.Fatal("FAILED")
+ fatalf("FAILED")
}
if dt := nextTest; dt != nil {
@@ -1468,7 +1465,7 @@
if err != nil {
dirs = dirs[i:] // Only undo what we did so far.
undo()
- log.Fatalf("failed to make GOROOT read-only: %v", err)
+ fatalf("failed to make GOROOT read-only: %v", err)
}
}
