From 4160a71d4206e11d2122d1d9520c55a5b95c2085 Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Fri, 15 Nov 2019 12:26:09 -0500 Subject: [PATCH] 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 TryBot-Result: Gobot Gobot Reviewed-by: Brad Fitzpatrick --- src/cmd/dist/test.go | 31 ++++++++++++++----------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/src/cmd/dist/test.go b/src/cmd/dist/test.go index 8e7106d2a2..dc22aad3ed 100644 --- 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) } } -- 2.50.0