From d75a867395e983c0a420ef3fecf30c98897560d7 Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Tue, 22 Nov 2022 11:40:42 -0500 Subject: [PATCH] cmd/dist: skip rebuilding packages during builder testing Since packages in "std" no longer have install targets, checking them for staleness is somewhat meaningless: if they are not cached they will be rebuilt anyway, and without installed archives against which we can compare them the staleness check will not detect builder skew. It would still be meaningful to check "cmd" for staleness, but (especially on sharded VM-based builders) that is a fairly expensive operation relative to its benefit. If we are really interested in detecting builder skew and/or build reproducibility, we could instead add a "misc" test (similar to "misc/reboot", or perhaps even a part of that test) that verifies that bootstrapped binaries are reproducible. For #57734. Updates #47257. Updates #56896. Change-Id: I8683ee81aefe8fb59cce9484453df9729bdc587c Reviewed-on: https://go-review.googlesource.com/c/go/+/452775 Run-TryBot: Bryan Mills TryBot-Result: Gopher Robot Auto-Submit: Bryan Mills Reviewed-by: Austin Clements --- src/cmd/dist/test.go | 42 ++++++++++-------------------------------- 1 file changed, 10 insertions(+), 32 deletions(-) diff --git a/src/cmd/dist/test.go b/src/cmd/dist/test.go index 0cd5113a607..bdf389fea45 100644 --- a/src/cmd/dist/test.go +++ b/src/cmd/dist/test.go @@ -154,41 +154,21 @@ func (t *tester) run() { if !t.listMode { if builder := os.Getenv("GO_BUILDER_NAME"); builder == "" { - // Complete rebuild bootstrap, even with -no-rebuild. + // Ensure that installed commands are up to date, even with -no-rebuild, + // so that tests that run commands end up testing what's actually on disk. // If everything is up-to-date, this is a no-op. - // If everything is not up-to-date, the first checkNotStale - // during the test process will kill the tests, so we might - // as well install the world. - // Now that for example "go install cmd/compile" does not - // also install runtime (you need "go install -i cmd/compile" - // for that), it's easy for previous workflows like - // "rebuild the compiler and then run run.bash" - // to break if we don't automatically refresh things here. - // Rebuilding is a shortened bootstrap. + // We first build the toolchain twice to allow it to converge, + // as when we first bootstrap. // See cmdbootstrap for a description of the overall process. + // + // On the builders, we skip this step: we assume that 'dist test' is + // already using the result of a clean build, and because of test sharding + // and virtualization we usually start with a clean GOCACHE, so we would + // end up rebuilding large parts of the standard library that aren't + // otherwise relevant to the actual set of packages under test. goInstall(toolenv, gorootBinGo, toolchain...) goInstall(toolenv, gorootBinGo, toolchain...) goInstall(toolenv, gorootBinGo, "cmd") - goInstall(nil, gorootBinGo, "std") - } else { - // The Go builder infrastructure should always begin running tests from a - // clean, non-stale state, so there is no need to rebuild the world. - // Instead, we can just check that it is not stale, which may be less - // expensive (and is also more likely to catch bugs in the builder - // implementation). - // The cache used by dist when building is different from that used when - // running dist test, so rebuild (but don't install) std and cmd to make - // sure packages without install targets are cached so they are not stale. - goCmd(toolenv, gorootBinGo, "build", "cmd") // make sure dependencies of targets are cached - goCmd(nil, gorootBinGo, "build", "std") - checkNotStale(nil, gorootBinGo, "std") - if builder != "aix-ppc64" { - // The aix-ppc64 builder for some reason does not have deterministic cgo - // builds, so "cmd" is stale. Fortunately, most of the tests don't care. - // TODO(#56896): remove this special case once the builder supports - // determistic cgo builds. - checkNotStale(toolenv, gorootBinGo, "cmd") - } } } @@ -1361,7 +1341,6 @@ func (t *tester) registerCgoTests() { // running in parallel with earlier tests, or if it has some other reason // for needing the earlier tests to be done. func (t *tester) runPending(nextTest *distTest) { - checkNotStale(nil, gorootBinGo, "std") worklist := t.worklist t.worklist = nil for _, w := range worklist { @@ -1419,7 +1398,6 @@ func (t *tester) runPending(nextTest *distTest) { log.Printf("Failed: %v", w.err) t.failed = true } - checkNotStale(nil, gorootBinGo, "std") } if t.failed && !t.keepGoing { fatalf("FAILED")