diff --git a/misc/cgo/testplugin/test.bash b/misc/cgo/testplugin/test.bash index 18e3803bf42ae..5ef87625f1a65 100755 --- a/misc/cgo/testplugin/test.bash +++ b/misc/cgo/testplugin/test.bash @@ -14,6 +14,9 @@ fi goos=$(go env GOOS) goarch=$(go env GOARCH) +echo SKIP: golang.org/issue/22571. +exit 0 + function cleanup() { rm -f plugin*.so unnamed*.so iface*.so issue* rm -rf host pkg sub iface diff --git a/src/cmd/dist/deps.go b/src/cmd/dist/deps.go index f0b068da5013f..ffc3b4788c260 100644 --- a/src/cmd/dist/deps.go +++ b/src/cmd/dist/deps.go @@ -280,6 +280,7 @@ var builddeps = map[string][]string{ "cmd/go/internal/test": { "bytes", // cmd/go/internal/test "cmd/go/internal/base", // cmd/go/internal/test + "cmd/go/internal/cache", // cmd/go/internal/test "cmd/go/internal/cfg", // cmd/go/internal/test "cmd/go/internal/cmdflag", // cmd/go/internal/test "cmd/go/internal/load", // cmd/go/internal/test @@ -293,6 +294,7 @@ var builddeps = map[string][]string{ "go/doc", // cmd/go/internal/test "go/parser", // cmd/go/internal/test "go/token", // cmd/go/internal/test + "io", // cmd/go/internal/test "os", // cmd/go/internal/test "os/exec", // cmd/go/internal/test "path", // cmd/go/internal/test diff --git a/src/cmd/go/alldocs.go b/src/cmd/go/alldocs.go index 0124199e181c8..5fbefb7b32ce1 100644 --- a/src/cmd/go/alldocs.go +++ b/src/cmd/go/alldocs.go @@ -724,10 +724,10 @@ // // 'Go test' recompiles each package along with any files with names matching // the file pattern "*_test.go". -// Files whose names begin with "_" (including "_test.go") or "." are ignored. // These additional files can contain test functions, benchmark functions, and // example functions. See 'go help testfunc' for more. // Each listed package causes the execution of a separate test binary. +// Files whose names begin with "_" (including "_test.go") or "." are ignored. // // Test files that declare a package with the suffix "_test" will be compiled as a // separate package, and then linked and run with the main test binary. @@ -735,11 +735,37 @@ // The go tool will ignore a directory named "testdata", making it available // to hold ancillary data needed by the tests. // -// By default, go test needs no arguments. It compiles and tests the package -// with source in the current directory, including tests, and runs the tests. -// -// The package is built in a temporary directory so it does not interfere with the -// non-test installation. +// Go test runs in two different modes: local directory mode when invoked with +// no package arguments (for example, 'go test'), and package list mode when +// invoked with package arguments (for example 'go test math', 'go test ./...', +// and even 'go test .'). +// +// In local directory mode, go test compiles and tests the package sources +// found in the current directory and then runs the resulting test binary. +// In this mode, the test binary runs with standard output and standard error +// connected directly to the go command's own standard output and standard +// error, and test result caching (discussed below) is disabled. +// After the package test finishes, go test prints to standard output a +// summary line showing the test status ('ok' or 'FAIL'), package name, +// and elapsed time. +// +// In package list mode, go test compiles and tests each of the packages +// listed on the command line. If a package test passes, go test prints only +// the final 'ok' summary line. If a package test fails, go test prints the +// full test output. If invoked with the -bench or -v flag, go test prints +// the full output even for passing package tests, in order to display the +// requested benchmark results or verbose logging. In package list mode, +// go test prints all test output and summary lines to standard output. +// +// In package list mode, go test also caches successful package test results. +// If go test has cached a previous test run using the same test binary and +// the same command line consisting entirely of cacheable test flags +// (defined as -cpu, -list, -parallel, -run, -short, and -v), +// go test will redisplay the previous output instead of running the test +// binary again. In the summary line, go test prints '(cached)' in place of +// the elapsed time. To disable test caching, use any test flag or argument +// other than the cacheable flags. The idiomatic way to disable test caching +// explicitly is to use -count=1. // // In addition to the build flags, the flags handled by 'go test' itself are: // diff --git a/src/cmd/go/go_test.go b/src/cmd/go/go_test.go index 6048dc97c581a..23b7920ebc444 100644 --- a/src/cmd/go/go_test.go +++ b/src/cmd/go/go_test.go @@ -4741,3 +4741,96 @@ func TestBuildCache(t *testing.T) { tg.grepStderr(`[\\/]link|gccgo`, "did not run linker") } + +func TestTestCache(t *testing.T) { + if strings.Contains(os.Getenv("GODEBUG"), "gocacheverify") { + t.Skip("GODEBUG gocacheverify") + } + tg := testgo(t) + defer tg.cleanup() + tg.makeTempdir() + tg.setenv("GOPATH", tg.tempdir) + tg.setenv("GOCACHE", filepath.Join(tg.tempdir, "cache")) + + tg.run("test", "-x", "errors") + tg.grepStderr(`[\\/]compile|gccgo`, "did not run compiler") + tg.grepStderr(`[\\/]link|gccgo`, "did not run linker") + tg.grepStderr(`errors\.test`, "did not run test") + + tg.run("test", "-x", "errors") + tg.grepStdout(`ok \terrors\t\(cached\)`, "did not report cached result") + tg.grepStderrNot(`[\\/]compile|gccgo`, "incorrectly ran compiler") + tg.grepStderrNot(`[\\/]link|gccgo`, "incorrectly ran linker") + tg.grepStderrNot(`errors\.test`, "incorrectly ran test") + tg.grepStderrNot("DO NOT USE", "poisoned action status leaked") + + // The -p=1 in the commands below just makes the -x output easier to read. + + t.Log("\n\nINITIAL\n\n") + + tg.tempFile("src/p1/p1.go", "package p1\nvar X = 1\n") + tg.tempFile("src/p2/p2.go", "package p2\nimport _ \"p1\"\nvar X = 1\n") + tg.tempFile("src/t/t1/t1_test.go", "package t\nimport \"testing\"\nfunc Test1(*testing.T) {}\n") + tg.tempFile("src/t/t2/t2_test.go", "package t\nimport _ \"p1\"\nimport \"testing\"\nfunc Test2(*testing.T) {}\n") + tg.tempFile("src/t/t3/t3_test.go", "package t\nimport \"p1\"\nimport \"testing\"\nfunc Test3(t *testing.T) {t.Log(p1.X)}\n") + tg.tempFile("src/t/t4/t4_test.go", "package t\nimport \"p2\"\nimport \"testing\"\nfunc Test4(t *testing.T) {t.Log(p2.X)}") + tg.run("test", "-x", "-v", "-short", "t/...") + + t.Log("\n\nREPEAT\n\n") + + tg.run("test", "-x", "-v", "-short", "t/...") + tg.grepStdout(`ok \tt/t1\t\(cached\)`, "did not cache t1") + tg.grepStdout(`ok \tt/t2\t\(cached\)`, "did not cache t2") + tg.grepStdout(`ok \tt/t3\t\(cached\)`, "did not cache t3") + tg.grepStdout(`ok \tt/t4\t\(cached\)`, "did not cache t4") + tg.grepStderrNot(`[\\/]compile|gccgo`, "incorrectly ran compiler") + tg.grepStderrNot(`[\\/]link|gccgo`, "incorrectly ran linker") + tg.grepStderrNot(`p[0-9]\.test`, "incorrectly ran test") + + t.Log("\n\nCOMMENT\n\n") + + // Changing the program text without affecting the compiled package + // should result in the package being rebuilt but nothing more. + tg.tempFile("src/p1/p1.go", "package p1\nvar X = 01\n") + tg.run("test", "-p=1", "-x", "-v", "-short", "t/...") + tg.grepStdout(`ok \tt/t1\t\(cached\)`, "did not cache t1") + tg.grepStdout(`ok \tt/t2\t\(cached\)`, "did not cache t2") + tg.grepStdout(`ok \tt/t3\t\(cached\)`, "did not cache t3") + tg.grepStdout(`ok \tt/t4\t\(cached\)`, "did not cache t4") + tg.grepStderrNot(`([\\/]compile|gccgo).*t[0-9]_test\.go`, "incorrectly ran compiler") + tg.grepStderrNot(`[\\/]link|gccgo`, "incorrectly ran linker") + tg.grepStderrNot(`t[0-9]\.test.*test\.short`, "incorrectly ran test") + + t.Log("\n\nCHANGE\n\n") + + // Changing the actual package should have limited effects. + tg.tempFile("src/p1/p1.go", "package p1\nvar X = 02\n") + tg.run("test", "-p=1", "-x", "-v", "-short", "t/...") + + // p2 should have been rebuilt. + tg.grepStderr(`([\\/]compile|gccgo).*p2.go`, "did not recompile p2") + + // t1 does not import anything, should not have been rebuilt. + tg.grepStderrNot(`([\\/]compile|gccgo).*t1_test.go`, "incorrectly recompiled t1") + tg.grepStderrNot(`([\\/]link|gccgo).*t1_test`, "incorrectly relinked t1_test") + tg.grepStdout(`ok \tt/t1\t\(cached\)`, "did not cache t/t1") + + // t2 imports p1 and must be rebuilt and relinked, + // but the change should not have any effect on the test binary, + // so the test should not have been rerun. + tg.grepStderr(`([\\/]compile|gccgo).*t2_test.go`, "did not recompile t2") + tg.grepStderr(`([\\/]link|gccgo).*t2\.test`, "did not relink t2_test") + tg.grepStdout(`ok \tt/t2\t\(cached\)`, "did not cache t/t2") + + // t3 imports p1, and changing X changes t3's test binary. + tg.grepStderr(`([\\/]compile|gccgo).*t3_test.go`, "did not recompile t3") + tg.grepStderr(`([\\/]link|gccgo).*t3\.test`, "did not relink t3_test") + tg.grepStderr(`t3\.test.*-test.short`, "did not rerun t3_test") + tg.grepStdoutNot(`ok \tt/t3\t\(cached\)`, "reported cached t3_test result") + + // t4 imports p2, but p2 did not change, so t4 should be relinked, not recompiled, + // and not rerun. + tg.grepStderrNot(`([\\/]compile|gccgo).*t4_test.go`, "incorrectly recompiled t4") + tg.grepStderr(`([\\/]link|gccgo).*t4\.test`, "did not relink t4_test") + tg.grepStdout(`ok \tt/t4\t\(cached\)`, "did not cache t/t4") +} diff --git a/src/cmd/go/internal/test/test.go b/src/cmd/go/internal/test/test.go index 62a5be9ef2df1..7bf24d4a0888b 100644 --- a/src/cmd/go/internal/test/test.go +++ b/src/cmd/go/internal/test/test.go @@ -13,6 +13,7 @@ import ( "go/doc" "go/parser" "go/token" + "io" "os" "os/exec" "path" @@ -26,6 +27,7 @@ import ( "unicode/utf8" "cmd/go/internal/base" + "cmd/go/internal/cache" "cmd/go/internal/cfg" "cmd/go/internal/load" "cmd/go/internal/str" @@ -56,10 +58,10 @@ followed by detailed output for each failed package. 'Go test' recompiles each package along with any files with names matching the file pattern "*_test.go". -Files whose names begin with "_" (including "_test.go") or "." are ignored. These additional files can contain test functions, benchmark functions, and example functions. See 'go help testfunc' for more. Each listed package causes the execution of a separate test binary. +Files whose names begin with "_" (including "_test.go") or "." are ignored. Test files that declare a package with the suffix "_test" will be compiled as a separate package, and then linked and run with the main test binary. @@ -67,11 +69,37 @@ separate package, and then linked and run with the main test binary. The go tool will ignore a directory named "testdata", making it available to hold ancillary data needed by the tests. -By default, go test needs no arguments. It compiles and tests the package -with source in the current directory, including tests, and runs the tests. - -The package is built in a temporary directory so it does not interfere with the -non-test installation. +Go test runs in two different modes: local directory mode when invoked with +no package arguments (for example, 'go test'), and package list mode when +invoked with package arguments (for example 'go test math', 'go test ./...', +and even 'go test .'). + +In local directory mode, go test compiles and tests the package sources +found in the current directory and then runs the resulting test binary. +In this mode, the test binary runs with standard output and standard error +connected directly to the go command's own standard output and standard +error, and test result caching (discussed below) is disabled. +After the package test finishes, go test prints to standard output a +summary line showing the test status ('ok' or 'FAIL'), package name, +and elapsed time. + +In package list mode, go test compiles and tests each of the packages +listed on the command line. If a package test passes, go test prints only +the final 'ok' summary line. If a package test fails, go test prints the +full test output. If invoked with the -bench or -v flag, go test prints +the full output even for passing package tests, in order to display the +requested benchmark results or verbose logging. In package list mode, +go test prints all test output and summary lines to standard output. + +In package list mode, go test also caches successful package test results. +If go test has cached a previous test run using the same test binary and +the same command line consisting entirely of cacheable test flags +(defined as -cpu, -list, -parallel, -run, -short, and -v), +go test will redisplay the previous output instead of running the test +binary again. In the summary line, go test prints '(cached)' in place of +the elapsed time. To disable test caching, use any test flag or argument +other than the cacheable flags. The idiomatic way to disable test caching +explicitly is to use -count=1. ` + strings.TrimSpace(testFlag1) + ` See 'go help testflag' for details. @@ -405,21 +433,22 @@ See the documentation of the testing package for more information. } var ( - testC bool // -c flag - testCover bool // -cover flag - testCoverMode string // -covermode flag - testCoverPaths []string // -coverpkg flag - testCoverPkgs []*load.Package // -coverpkg flag - testO string // -o flag - testProfile bool // some profiling flag - testNeedBinary bool // profile needs to keep binary around - testV bool // -v flag - testTimeout string // -timeout flag - testArgs []string - testBench bool - testList bool - testStreamOutput bool // show output as it is generated - testShowPass bool // show passing output + testC bool // -c flag + testCover bool // -cover flag + testCoverMode string // -covermode flag + testCoverPaths []string // -coverpkg flag + testCoverPkgs []*load.Package // -coverpkg flag + testO string // -o flag + testProfile bool // some profiling flag + testNeedBinary bool // profile needs to keep binary around + testV bool // -v flag + testTimeout string // -timeout flag + testArgs []string + testBench bool + testList bool + testShowPass bool // show passing output + pkgArgs []string + pkgs []*load.Package testKillTimeout = 10 * time.Minute ) @@ -432,14 +461,13 @@ var testMainDeps = []string{ } func runTest(cmd *base.Command, args []string) { - var pkgArgs []string pkgArgs, testArgs = testFlags(args) work.FindExecCmd() // initialize cached result work.InstrumentInit() work.BuildModeInit() - pkgs := load.PackagesForBuild(pkgArgs) + pkgs = load.PackagesForBuild(pkgArgs) if len(pkgs) == 0 { base.Fatalf("no packages to test") } @@ -467,16 +495,6 @@ func runTest(cmd *base.Command, args []string) { // otherwise the output will get mixed. testShowPass = testV || testList - // stream test output (no buffering) when no package has - // been given on the command line (implicit current directory) - // or when benchmarking. - // Also stream if we're showing output anyway with a - // single package under test or if parallelism is set to 1. - // In these cases, streaming the output produces the same result - // as not streaming, just more immediately. - testStreamOutput = len(pkgArgs) == 0 || testBench || - (testShowPass && (len(pkgs) == 1 || cfg.BuildP == 1)) - // For 'go test -i -o x.test', we want to build x.test. Imply -c to make the logic easier. if cfg.BuildI && testO != "" { testC = true @@ -948,12 +966,14 @@ func builderTest(b *work.Builder, p *load.Package) (buildAction, runAction, prin printAction = &work.Action{Mode: "test print (nop)", Package: p, Deps: []*work.Action{runAction}} // nop } else { // run test + c := new(runCache) runAction = &work.Action{ Mode: "test run", - Func: builderRunTest, + Func: c.builderRunTest, Deps: []*work.Action{buildAction}, Package: p, IgnoreFail: true, + TryCache: c.tryCache, } cleanAction := &work.Action{ Mode: "test clean", @@ -1054,11 +1074,34 @@ func declareCoverVars(importPath string, files ...string) map[string]*load.Cover var noTestsToRun = []byte("\ntesting: warning: no tests to run\n") +type runCache struct { + disableCache bool // cache should be disabled for this run + + buf *bytes.Buffer + id1 cache.ActionID + id2 cache.ActionID +} + // builderRunTest is the action for running a test binary. -func builderRunTest(b *work.Builder, a *work.Action) error { - args := str.StringList(work.FindExecCmd(), a.Deps[0].Target, testArgs) - a.TestOutput = new(bytes.Buffer) +func (c *runCache) builderRunTest(b *work.Builder, a *work.Action) error { + if c.buf == nil { + // We did not find a cached result using the link step action ID, + // so we ran the link step. Try again now with the link output + // content ID. The attempt using the action ID makes sure that + // if the link inputs don't change, we reuse the cached test + // result without even rerunning the linker. The attempt using + // the link output (test binary) content ID makes sure that if + // we have different link inputs but the same final binary, + // we still reuse the cached test result. + // c.saveOutput will store the result under both IDs. + c.tryCacheWithID(b, a, a.Deps[0].BuildContentID()) + } + if c.buf != nil { + a.TestOutput = c.buf + return nil + } + args := str.StringList(work.FindExecCmd(), a.Deps[0].Target, testArgs) if cfg.BuildN || cfg.BuildX { b.Showcmd("", "%s", strings.Join(args, " ")) if cfg.BuildN { @@ -1069,6 +1112,7 @@ func builderRunTest(b *work.Builder, a *work.Action) error { if a.Failed { // We were unable to build the binary. a.Failed = false + a.TestOutput = new(bytes.Buffer) fmt.Fprintf(a.TestOutput, "FAIL\t%s [build failed]\n", a.Package.ImportPath) base.SetExitStatus(1) return nil @@ -1078,12 +1122,48 @@ func builderRunTest(b *work.Builder, a *work.Action) error { cmd.Dir = a.Package.Dir cmd.Env = base.EnvForDir(cmd.Dir, cfg.OrigEnv) var buf bytes.Buffer - if testStreamOutput { + if len(pkgArgs) == 0 || testBench { + // Stream test output (no buffering) when no package has + // been given on the command line (implicit current directory) + // or when benchmarking. Allowing stderr to pass through to + // stderr here is a bit of an historical mistake, but now a + // documented one. Except in this case, all output is merged + // to one stream written to stdout. cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr } else { - cmd.Stdout = &buf - cmd.Stderr = &buf + // If we're only running a single package under test + // or if parallelism is set to 1, and if we're displaying + // all output (testShowPass), we can hurry the output along, + // echoing it as soon as it comes in. We still have to copy + // to &buf for caching the result. This special case was + // introduced in Go 1.5 and is intentionally undocumented: + // the rationale is that the exact details of output buffering + // are up to the go command and subject to change. + // NOTE(rsc): Originally this special case also had the effect of + // allowing stderr to pass through to os.Stderr, unlike + // the normal buffering that merges stdout and stderr into stdout. + // This had the truly mysterious result that on a multiprocessor, + // "go test -v math" could print to stderr, + // "go test -v math strings" could not, and + // "go test -p=1 -v math strings" could once again. + // While I'm willing to let the buffer flush timing + // fluctuate based on minor details like this, + // allowing the file descriptor to which output is sent + // to change as well seems like a serious mistake. + // Go 1.10 changed this code to allow the less aggressive + // buffering but still merge all output to standard output. + // I'd really like to remove this special case entirely, + // but it is surely very helpful to see progress being made + // when tests are run on slow single-CPU ARM systems. + if testShowPass && (len(pkgs) == 1 || cfg.BuildP == 1) { + // Write both to stdout and buf, for possible saving + // to cache, and for looking for the "no tests to run" message. + cmd.Stdout = io.MultiWriter(os.Stdout, &buf) + } else { + cmd.Stdout = &buf + } + cmd.Stderr = cmd.Stdout } // If there are any local SWIG dependencies, we want to load @@ -1130,41 +1210,141 @@ func builderRunTest(b *work.Builder, a *work.Action) error { cmd.Process.Signal(base.SignalTrace) select { case err = <-done: - fmt.Fprintf(&buf, "*** Test killed with %v: ran too long (%v).\n", base.SignalTrace, testKillTimeout) + fmt.Fprintf(cmd.Stdout, "*** Test killed with %v: ran too long (%v).\n", base.SignalTrace, testKillTimeout) break Outer case <-time.After(5 * time.Second): } } cmd.Process.Kill() err = <-done - fmt.Fprintf(&buf, "*** Test killed: ran too long (%v).\n", testKillTimeout) + fmt.Fprintf(cmd.Stdout, "*** Test killed: ran too long (%v).\n", testKillTimeout) } tick.Stop() } out := buf.Bytes() + a.TestOutput = &buf t := fmt.Sprintf("%.3fs", time.Since(t0).Seconds()) if err == nil { norun := "" - if testShowPass { - a.TestOutput.Write(out) + if !testShowPass { + buf.Reset() } if bytes.HasPrefix(out, noTestsToRun[1:]) || bytes.Contains(out, noTestsToRun) { norun = " [no tests to run]" } - fmt.Fprintf(a.TestOutput, "ok \t%s\t%s%s%s\n", a.Package.ImportPath, t, coveragePercentage(out), norun) - return nil + fmt.Fprintf(cmd.Stdout, "ok \t%s\t%s%s%s\n", a.Package.ImportPath, t, coveragePercentage(out), norun) + c.saveOutput(a) + } else { + base.SetExitStatus(1) + // If there was test output, assume we don't need to print the exit status. + // Buf there's no test output, do print the exit status. + if len(out) == 0 { + fmt.Fprintf(cmd.Stdout, "%s\n", err) + } + fmt.Fprintf(cmd.Stdout, "FAIL\t%s\t%s\n", a.Package.ImportPath, t) + } + + if cmd.Stdout != &buf { + buf.Reset() // cmd.Stdout was going to os.Stdout already + } + return nil +} + +// tryCache is called just before the link attempt, +// to see if the test result is cached and therefore the link is unneeded. +// It reports whether the result can be satisfied from cache. +func (c *runCache) tryCache(b *work.Builder, a *work.Action) bool { + return c.tryCacheWithID(b, a, a.Deps[0].BuildActionID()) +} + +func (c *runCache) tryCacheWithID(b *work.Builder, a *work.Action, id string) bool { + if len(pkgArgs) == 0 { + // Caching does not apply to "go test", + // only to "go test foo" (including "go test ."). + c.disableCache = true + return false + } + + for _, arg := range testArgs { + i := strings.Index(arg, "=") + if i < 0 || !strings.HasPrefix(arg, "-test.") { + c.disableCache = true + return false + } + switch arg[:i] { + case "-test.cpu", + "-test.list", + "-test.parallel", + "-test.run", + "-test.short", + "-test.v": + // These are cacheable. + // Note that this list is documented above, + // so if you add to this list, update the docs too. + default: + // nothing else is cacheable + c.disableCache = true + return false + } + } + + if cache.Default() == nil { + c.disableCache = true + return false } - base.SetExitStatus(1) - if len(out) > 0 { - a.TestOutput.Write(out) - // assume printing the test binary's exit status is superfluous + h := cache.NewHash("testResult") + fmt.Fprintf(h, "test binary %s args %q execcmd %q", id, testArgs, work.ExecCmd) + // TODO(rsc): How to handle other test dependencies like environment variables or input files? + // We could potentially add new API like testing.UsedEnv(envName string) + // or testing.UsedFile(inputFile string) to let tests declare what external inputs + // they consulted. These could be recorded and rechecked. + // The lookup here would become a two-step lookup: first use the binary+args + // to fetch the list of other inputs, then add the other inputs to produce a + // second key for fetching the results. + // For now, we'll assume that users will use -count=1 (or "go test") to bypass the test result + // cache when modifying those things. + testID := h.Sum() + if c.id1 == (cache.ActionID{}) { + c.id1 = testID } else { - fmt.Fprintf(a.TestOutput, "%s\n", err) + c.id2 = testID } - fmt.Fprintf(a.TestOutput, "FAIL\t%s\t%s\n", a.Package.ImportPath, t) - return nil + // Parse cached result in preparation for changing run time to "(cached)". + // If we can't parse the cached result, don't use it. + data, _ := cache.Default().GetBytes(testID) + if len(data) == 0 || data[len(data)-1] != '\n' { + return false + } + i := bytes.LastIndexByte(data[:len(data)-1], '\n') + 1 + if !bytes.HasPrefix(data[i:], []byte("ok \t")) { + return false + } + j := bytes.IndexByte(data[i+len("ok \t"):], '\t') + if j < 0 { + return false + } + j += i + len("ok \t") + 1 + + // Committed to printing. + c.buf = new(bytes.Buffer) + c.buf.Write(data[:j]) + c.buf.WriteString("(cached)") + for j < len(data) && ('0' <= data[j] && data[j] <= '9' || data[j] == '.' || data[j] == 's') { + j++ + } + c.buf.Write(data[j:]) + return true +} + +func (c *runCache) saveOutput(a *work.Action) { + if c.id1 != (cache.ActionID{}) { + cache.Default().PutNoVerify(c.id1, bytes.NewReader(a.TestOutput.Bytes())) + } + if c.id2 != (cache.ActionID{}) { + cache.Default().PutNoVerify(c.id2, bytes.NewReader(a.TestOutput.Bytes())) + } } // coveragePercentage returns the coverage results (if enabled) for the diff --git a/src/cmd/go/internal/work/action.go b/src/cmd/go/internal/work/action.go index 883c454340585..25a0a96b9837e 100644 --- a/src/cmd/go/internal/work/action.go +++ b/src/cmd/go/internal/work/action.go @@ -68,6 +68,8 @@ type Action struct { triggers []*Action // inverse of deps + TryCache func(*Builder, *Action) bool // callback for cache bypass + // Generated files, directories. Objdir string // directory for intermediate objects Target string // goal of the action: the created package or executable @@ -84,6 +86,15 @@ type Action struct { Failed bool // whether the action failed } +// BuildActionID returns the action ID section of a's build ID. +func (a *Action) BuildActionID() string { return actionID(a.buildID) } + +// BuildContentID returns the content ID section of a's build ID. +func (a *Action) BuildContentID() string { return contentID(a.buildID) } + +// BuildID returns a's build ID. +func (a *Action) BuildID() string { return a.buildID } + // An actionQueue is a priority queue of actions. type actionQueue []*Action diff --git a/src/cmd/go/internal/work/buildid.go b/src/cmd/go/internal/work/buildid.go index 935b638fd99a1..35ef1df885ae1 100644 --- a/src/cmd/go/internal/work/buildid.go +++ b/src/cmd/go/internal/work/buildid.go @@ -93,6 +93,15 @@ import ( const buildIDSeparator = "/" +// actionID returns the action ID half of a build ID. +func actionID(buildID string) string { + i := strings.Index(buildID, buildIDSeparator) + if i < 0 { + return buildID + } + return buildID[:i] +} + // contentID returns the content ID half of a build ID. func contentID(buildID string) string { return buildID[strings.LastIndex(buildID, buildIDSeparator)+1:] @@ -276,6 +285,7 @@ func (b *Builder) useCache(a *Action, p *load.Package, actionHash cache.ActionID // want the package for is to link a binary, and the binary is // already up-to-date, then to avoid a rebuild, report the package // as up-to-date as well. See "Build IDs" comment above. + // TODO(rsc): Rewrite this code to use a TryCache func on the link action. if target != "" && !cfg.BuildA && a.Mode == "build" && len(a.triggers) == 1 && a.triggers[0].Mode == "link" { buildID, err := buildid.ReadFile(target) if err == nil { @@ -306,6 +316,17 @@ func (b *Builder) useCache(a *Action, p *load.Package, actionHash cache.ActionID } } + // Special case for linking a test binary: if the only thing we + // want the binary for is to run the test, and the test result is cached, + // then to avoid the link step, report the link as up-to-date. + // We avoid the nested build ID problem in the previous special case + // by recording the test results in the cache under the action ID half. + if !cfg.BuildA && len(a.triggers) == 1 && a.triggers[0].TryCache != nil && a.triggers[0].TryCache(b, a.triggers[0]) { + a.Target = "DO NOT USE - pseudo-cache Target" + a.built = "DO NOT USE - pseudo-cache built" + return true + } + if b.ComputeStaleOnly { // Invoked during go list only to compute and record staleness. if p := a.Package; p != nil && !p.Stale { @@ -358,9 +379,11 @@ func (b *Builder) useCache(a *Action, p *load.Package, actionHash cache.ActionID // a.buildID to record as the build ID in the resulting package or binary. // updateBuildID computes the final content ID and updates the build IDs // in the binary. -func (b *Builder) updateBuildID(a *Action, target string) error { +func (b *Builder) updateBuildID(a *Action, target string, rewrite bool) error { if cfg.BuildX || cfg.BuildN { - b.Showcmd("", "%s # internal", joinUnambiguously(str.StringList(base.Tool("buildid"), "-w", target))) + if rewrite { + b.Showcmd("", "%s # internal", joinUnambiguously(str.StringList(base.Tool("buildid"), "-w", target))) + } if cfg.BuildN { return nil } @@ -387,17 +410,20 @@ func (b *Builder) updateBuildID(a *Action, target string) error { // Assume the user specified -buildid= to override what we were going to choose. return nil } - w, err := os.OpenFile(target, os.O_WRONLY, 0) - if err != nil { - return err - } - err = buildid.Rewrite(w, matches, newID) - if err != nil { - w.Close() - return err - } - if err := w.Close(); err != nil { - return err + + if rewrite { + w, err := os.OpenFile(target, os.O_WRONLY, 0) + if err != nil { + return err + } + err = buildid.Rewrite(w, matches, newID) + if err != nil { + w.Close() + return err + } + if err := w.Close(); err != nil { + return err + } } // Cache package builds, but not binaries (link steps). @@ -408,6 +434,11 @@ func (b *Builder) updateBuildID(a *Action, target string) error { // Not caching the link step also makes sure that repeated "go run" at least // always rerun the linker, so that they don't get too fast. // (We don't want people thinking go is a scripting language.) + // Note also that if we start caching binaries, then we will + // copy the binaries out of the cache to run them, and then + // that will mean the go process is itself writing a binary + // and then executing it, so we will need to defend against + // ETXTBSY problems as discussed in exec.go and golang.org/issue/22220. if c := cache.Default(); c != nil && a.Mode == "build" { r, err := os.Open(target) if err == nil { diff --git a/src/cmd/go/internal/work/exec.go b/src/cmd/go/internal/work/exec.go index 9daa58577805f..da4b5306e9b09 100644 --- a/src/cmd/go/internal/work/exec.go +++ b/src/cmd/go/internal/work/exec.go @@ -244,7 +244,7 @@ func (b *Builder) buildActionID(a *Action) cache.ActionID { for _, a1 := range a.Deps { p1 := a1.Package if p1 != nil { - fmt.Fprintf(h, "import %s %s\n", p1.ImportPath, a1.buildID) + fmt.Fprintf(h, "import %s %s\n", p1.ImportPath, contentID(a1.buildID)) } } @@ -613,7 +613,7 @@ func (b *Builder) build(a *Action) (err error) { } } - if err := b.updateBuildID(a, objpkg); err != nil { + if err := b.updateBuildID(a, objpkg, true); err != nil { return err } @@ -765,21 +765,23 @@ func (b *Builder) link(a *Action) (err error) { } // Update the binary with the final build ID. - // But if OmitDebug is set, don't, because we set OmitDebug + // But if OmitDebug is set, don't rewrite the binary, because we set OmitDebug // on binaries that we are going to run and then delete. // There's no point in doing work on such a binary. // Worse, opening the binary for write here makes it // essentially impossible to safely fork+exec due to a fundamental // incompatibility between ETXTBSY and threads on modern Unix systems. // See golang.org/issue/22220. + // We still call updateBuildID to update a.buildID, which is important + // for test result caching, but passing rewrite=false (final arg) + // means we don't actually rewrite the binary, nor store the + // result into the cache. // Not calling updateBuildID means we also don't insert these // binaries into the build object cache. That's probably a net win: // less cache space wasted on large binaries we are not likely to // need again. (On the other hand it does make repeated go test slower.) - if !a.Package.Internal.OmitDebug { - if err := b.updateBuildID(a, a.Target); err != nil { - return err - } + if err := b.updateBuildID(a, a.Target, !a.Package.Internal.OmitDebug); err != nil { + return err } a.built = a.Target