From 51e11e310c36d3322335756fe2ea109c99d6ed50 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Fri, 12 Jun 2020 11:06:26 -0400 Subject: [PATCH] Avoid prematurely recording toolstates When we're running with dry_run enabled (i.e. all builds do this initially), we're guaranteed to save of a toolstate of TestFail for tools that aren't tested. In practice, we do test tools as well, so for those tools we would initially record them as being TestPass, and then later on re-record the correct state after actually testing them. However, this would not work well if the build failed for whatever reason (e.g. panicking in bootstrap, or as was the case in 73097, clippy failing to test successfully), we would just go on believing that things passed when they in practice did not. This commit also adjusts saving toolstate to never record clippy explicitly (otherwise, it would be recorded when building it); eventually that'll likely move to other tools as well but not yet. This is deemed simpler than checking everywhere we generically save toolstate. We also move clippy out of the "toolstate" no-fail-fast build into a separate x.py invocation; this should no longer be technically required but provides the nice state of letting us check toolstate for all tools and only then check clippy (giving full results on every build). --- src/bootstrap/toolstate.rs | 12 ++++++++++++ src/ci/docker/x86_64-gnu-tools/checktools.sh | 4 +++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/bootstrap/toolstate.rs b/src/bootstrap/toolstate.rs index 6c219cee01ee5..8740393288c48 100644 --- a/src/bootstrap/toolstate.rs +++ b/src/bootstrap/toolstate.rs @@ -272,6 +272,18 @@ impl Builder<'_> { /// `rust.save-toolstates` in `config.toml`. If unspecified, nothing will be /// done. The file is updated immediately after this function completes. pub fn save_toolstate(&self, tool: &str, state: ToolState) { + // If we're in a dry run setting we don't want to save toolstates as + // that means if we e.g. panic down the line it'll look like we tested + // everything (but we actually haven't). + if self.config.dry_run { + return; + } + // Toolstate isn't tracked for clippy, but since most tools do, we avoid + // checking in all the places we could save toolstate and just do so + // here. + if tool == "clippy-driver" { + return; + } if let Some(ref path) = self.config.save_toolstates { if let Some(parent) = path.parent() { // Ensure the parent directory always exists diff --git a/src/ci/docker/x86_64-gnu-tools/checktools.sh b/src/ci/docker/x86_64-gnu-tools/checktools.sh index c9d1cb21da8b0..b4b23a245e0aa 100755 --- a/src/ci/docker/x86_64-gnu-tools/checktools.sh +++ b/src/ci/docker/x86_64-gnu-tools/checktools.sh @@ -14,11 +14,13 @@ python3 "$X_PY" test --no-fail-fast \ src/doc/rust-by-example \ src/doc/embedded-book \ src/doc/edition-guide \ - src/tools/clippy \ src/tools/rls \ src/tools/rustfmt \ src/tools/miri \ set -e +# debugging: print out the saved toolstates +cat /tmp/toolstate/toolstates.json python3 "$X_PY" test check-tools +python3 "$X_PY" test src/tools/clippy