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 8e7106d2a20555d68a39252eb821729a4263f8bf..dc22aad3ed05bdf523950232021cc99a9adc876a 100644 (file)
--- a/src/cmd/dist/test.go
+++ b/src/cmd/dist/test.go
@@ -100,11 +100,11 @@ func (t *tester) run() {
 
        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 @@ func (t *tester) run() {
        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 @@ func (t *tester) run() {
 
        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 @@ func (t *tester) run() {
                        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 @@ func short() string {
        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 @@ func (t *tester) registerTests() {
                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 @@ func (t *tester) registerTests() {
                                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 @@ func (t *tester) supportedBuildmode(mode string) bool {
                return false
 
        default:
-               log.Fatalf("internal error: unknown buildmode %s", mode)
+               fatalf("internal error: unknown buildmode %s", mode)
                return false
        }
 }
@@ -1190,7 +1187,7 @@ func (t *tester) runPending(nextTest *distTest) {
                checkNotStale("go", "std")
        }
        if t.failed && !t.keepGoing {
-               log.Fatal("FAILED")
+               fatalf("FAILED")
        }
 
        if dt := nextTest; dt != nil {
@@ -1468,7 +1465,7 @@ func (t *tester) makeGOROOTUnwritable() (undo func()) {
                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)
                }
        }