From 93df0f2c79d4c6430d7c2087eb0ed14447a4ccda Mon Sep 17 00:00:00 2001 From: Jynn Nelson Date: Fri, 31 Oct 2025 11:41:36 -0400 Subject: [PATCH] bootstrap: Use cargo's `build.warnings=deny` rather than -Dwarnings This has two major advantages. First, it makes us less dependent on the rustc shim, which is nice but not super important. More importantly, it gives us much nicer caching properties. Previously, `x build --warnings warn && x build --warnings deny` would rebuild all of bootstrap, and if there were any warnings emitted on the last build of the compiler, they would *not* fail the build, because cargo would cache the output rather than rerunning the shim. After this change, bootstrap rebuilds instantly, and cargo knows that it should fail the build but *without* invalidating the cache. --- src/bootstrap/bootstrap.py | 32 ++++++++++++------------- src/bootstrap/bootstrap_test.py | 10 ++++---- src/bootstrap/src/core/builder/cargo.rs | 11 +++++++-- 3 files changed, 31 insertions(+), 22 deletions(-) diff --git a/src/bootstrap/bootstrap.py b/src/bootstrap/bootstrap.py index 4dd465edb0df9..2e16f2cf27e7b 100644 --- a/src/bootstrap/bootstrap.py +++ b/src/bootstrap/bootstrap.py @@ -1113,6 +1113,19 @@ def build_bootstrap_cmd(self, env): else: env["RUSTFLAGS"] = "-Zallow-features=" + if not os.path.isfile(self.cargo()): + raise Exception("no cargo executable found at `{}`".format(self.cargo())) + args = [ + self.cargo(), + "build", + "--jobs=" + self.jobs, + "--manifest-path", + os.path.join(self.rust_root, "src/bootstrap/Cargo.toml"), + "-Zroot-dir=" + self.rust_root, + ] + # verbose cargo output is very noisy, so only enable it with -vv + args.extend("--verbose" for _ in range(self.verbose - 1)) + target_features = [] if self.get_toml("crt-static", build_section) == "true": target_features += ["+crt-static"] @@ -1131,28 +1144,15 @@ def build_bootstrap_cmd(self, env): else: deny_warnings = self.warnings == "deny" if deny_warnings: - env["RUSTFLAGS"] += " -Dwarnings" + args += ["-Zwarnings"] + env["CARGO_BUILD_WARNINGS"] = "deny" # Add RUSTFLAGS_BOOTSTRAP to RUSTFLAGS for bootstrap compilation. # Note that RUSTFLAGS_BOOTSTRAP should always be added to the end of - # RUSTFLAGS to be actually effective (e.g., if we have `-Dwarnings` in - # RUSTFLAGS, passing `-Awarnings` from RUSTFLAGS_BOOTSTRAP should override it). + # RUSTFLAGS, since that causes RUSTFLAGS_BOOTSTRAP to override RUSTFLAGS. if "RUSTFLAGS_BOOTSTRAP" in env: env["RUSTFLAGS"] += " " + env["RUSTFLAGS_BOOTSTRAP"] - if not os.path.isfile(self.cargo()): - raise Exception("no cargo executable found at `{}`".format(self.cargo())) - args = [ - self.cargo(), - "build", - "--jobs=" + self.jobs, - "--manifest-path", - os.path.join(self.rust_root, "src/bootstrap/Cargo.toml"), - "-Zroot-dir=" + self.rust_root, - ] - # verbose cargo output is very noisy, so only enable it with -vv - args.extend("--verbose" for _ in range(self.verbose - 1)) - if "BOOTSTRAP_TRACING" in env: args.append("--features=tracing") diff --git a/src/bootstrap/bootstrap_test.py b/src/bootstrap/bootstrap_test.py index 9e12982a43dc3..1dbe997d23f3a 100644 --- a/src/bootstrap/bootstrap_test.py +++ b/src/bootstrap/bootstrap_test.py @@ -244,8 +244,10 @@ def test_warnings(self): if toml_warnings is not None: configure_args = ["--set", "rust.deny-warnings=" + toml_warnings] - _, env = self.build_args(configure_args, args=["--warnings=warn"]) - self.assertFalse("-Dwarnings" in env["RUSTFLAGS"]) + args, env = self.build_args(configure_args, args=["--warnings=warn"]) + self.assertFalse("CARGO_BUILD_WARNINGS" in env) + self.assertFalse("-Zwarnings" in args) - _, env = self.build_args(configure_args, args=["--warnings=deny"]) - self.assertTrue("-Dwarnings" in env["RUSTFLAGS"]) + args, env = self.build_args(configure_args, args=["--warnings=deny"]) + self.assertEqual("deny", env["CARGO_BUILD_WARNINGS"]) + self.assertTrue("-Zwarnings" in args) diff --git a/src/bootstrap/src/core/builder/cargo.rs b/src/bootstrap/src/core/builder/cargo.rs index c2029f97391d4..76eb03c8d499d 100644 --- a/src/bootstrap/src/core/builder/cargo.rs +++ b/src/bootstrap/src/core/builder/cargo.rs @@ -459,6 +459,11 @@ impl Builder<'_> { let out_dir = self.stage_out(compiler, mode); cargo.env("CARGO_TARGET_DIR", &out_dir); + // Set this unconditionally. Cargo silently ignores `CARGO_BUILD_WARNINGS` when `-Z + // warnings` isn't present, which is hard to debug, and it's not worth the effort to keep + // them in sync. + cargo.arg("-Zwarnings"); + // Bootstrap makes a lot of assumptions about the artifacts produced in the target // directory. If users override the "build directory" using `build-dir` // (https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#build-dir), then @@ -1159,8 +1164,10 @@ impl Builder<'_> { lint_flags.push("-Wunused_lifetimes"); if self.config.deny_warnings { - lint_flags.push("-Dwarnings"); - rustdocflags.arg("-Dwarnings"); + // We use this instead of `lint_flags` so that we don't have to rebuild all + // workspace dependencies when `deny-warnings` changes, but we still get an error + // immediately instead of having to wait until the next rebuild. + cargo.env("CARGO_BUILD_WARNINGS", "deny"); } rustdocflags.arg("-Wrustdoc::invalid_codeblock_attributes");