Skip to content

Commit e1e8913

Browse files
committed
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. Here is an example of rebuilding bootstrap after a switch from warn->deny: ``` INFO: Downloading and building bootstrap before processing --help command. See src/bootstrap/README.md for help with common commands. Building bootstrap Compiling bootstrap v0.0.0 (/Users/jyn/src/ferrocene3/src/bootstrap) warning: unused variable: `x` --> src/bootstrap/src/core/builder/mod.rs:1792:13 | 1792 | let x: (); | ^ | = note: `#[warn(unused_variables)]` (part of `#[warn(unused)]`) on by default help: if this is intentional, prefix it with an underscore | 1792 | let _x: (); | + help: you might have meant to pattern match on the similarly named constant `_` | 1792 - let x: (); 1792 + let utils::render_tests::_: (); | warning: `bootstrap` (lib) generated 1 warning (run `cargo fix --lib -p bootstrap` to apply 1 suggestion) Finished `dev` profile [unoptimized] target(s) in 5.14s error: warnings are denied by `build.warnings` configuration failed to run: /Users/jyn/src/ferrocene3/build/aarch64-apple-darwin/stage0/bin/cargo build --jobs=default --manifest-path /Users/jyn/src/ferrocene3/src/bootstrap/Cargo.toml -Zroot-dir=/Users/jyn/src/ferrocene3 -Zwarnings ``` building the compiler from scratch with `deny`: https://gist.github.com/jyn514/493ed26c2aa6f61bf47c21e75efa2175 and rebuilding the compiler after switching from deny->warn (note that it reuses the whole cache, there are no invalidations): ``` $ x c compiler Building bootstrap Finished `dev` profile [unoptimized] target(s) in 0.15s Checking stage1 compiler artifacts{rustc-main, rustc_abi, rustc_arena, rustc_ast, rustc_ast_ir, rustc_ast_lowering, rustc_ast_passes, rustc_ast_pretty, rustc_attr_parsing, rustc_baked_icu_data, rustc_borrowck, rustc_builtin_macros, rustc_codegen_llvm, rustc_codegen_ssa, rustc_const_eval, rustc_data_structures, rustc_driver, rustc_driver_impl, rustc_error_codes, rustc_error_messages, rustc_errors, rustc_expand, rustc_feature, rustc_fluent_macro, rustc_fs_util, rustc_graphviz, rustc_hashes, rustc_hir, rustc_hir_analysis, rustc_hir_id, rustc_hir_pretty, rustc_hir_typeck, rustc_incremental, rustc_index, rustc_index_macros, rustc_infer, rustc_interface, rustc_lexer, rustc_lint, rustc_lint_defs, rustc_llvm, rustc_log, rustc_macros, rustc_metadata, rustc_middle, rustc_mir_build, rustc_mir_dataflow, rustc_mir_transform, rustc_monomorphize, rustc_next_trait_solver, rustc_parse, rustc_parse_format, rustc_passes, rustc_pattern_analysis, rustc_privacy, rustc_proc_macro, rustc_public, rustc_public_bridge, rustc_query_impl, rustc_query_system, rustc_resolve, rustc_sanitizers, rustc_serialize, rustc_session, rustc_span, rustc_symbol_mangling, rustc_target, rustc_thread_pool, rustc_trait_selection, rustc_traits, rustc_transmute, rustc_ty_utils, rustc_type_ir, rustc_type_ir_macros, rustc_windows_rc} (stage0 -> stage1, aarch64-apple-darwin) warning: function `foo` is never used --> compiler/rustc_hashes/src/lib.rs:16:4 | 16 | fn foo() {} | ^^^ | = note: `#[warn(dead_code)]` (part of `#[warn(unused)]`) on by default warning: `rustc_hashes` (lib) generated 1 warning Finished `release` profile [optimized + debuginfo] target(s) in 0.53s Build completed successfully in 0:00:08 ```
1 parent 292be5c commit e1e8913

File tree

3 files changed

+31
-22
lines changed

3 files changed

+31
-22
lines changed

src/bootstrap/bootstrap.py

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1113,6 +1113,19 @@ def build_bootstrap_cmd(self, env):
11131113
else:
11141114
env["RUSTFLAGS"] = "-Zallow-features="
11151115

1116+
if not os.path.isfile(self.cargo()):
1117+
raise Exception("no cargo executable found at `{}`".format(self.cargo()))
1118+
args = [
1119+
self.cargo(),
1120+
"build",
1121+
"--jobs=" + self.jobs,
1122+
"--manifest-path",
1123+
os.path.join(self.rust_root, "src/bootstrap/Cargo.toml"),
1124+
"-Zroot-dir=" + self.rust_root,
1125+
]
1126+
# verbose cargo output is very noisy, so only enable it with -vv
1127+
args.extend("--verbose" for _ in range(self.verbose - 1))
1128+
11161129
target_features = []
11171130
if self.get_toml("crt-static", build_section) == "true":
11181131
target_features += ["+crt-static"]
@@ -1131,28 +1144,15 @@ def build_bootstrap_cmd(self, env):
11311144
else:
11321145
deny_warnings = self.warnings == "deny"
11331146
if deny_warnings:
1134-
env["RUSTFLAGS"] += " -Dwarnings"
1147+
args += ["-Zwarnings"]
1148+
env["CARGO_BUILD_WARNINGS"] = "deny"
11351149

11361150
# Add RUSTFLAGS_BOOTSTRAP to RUSTFLAGS for bootstrap compilation.
11371151
# Note that RUSTFLAGS_BOOTSTRAP should always be added to the end of
1138-
# RUSTFLAGS to be actually effective (e.g., if we have `-Dwarnings` in
1139-
# RUSTFLAGS, passing `-Awarnings` from RUSTFLAGS_BOOTSTRAP should override it).
1152+
# RUSTFLAGS, since that causes RUSTFLAGS_BOOTSTRAP to override RUSTFLAGS.
11401153
if "RUSTFLAGS_BOOTSTRAP" in env:
11411154
env["RUSTFLAGS"] += " " + env["RUSTFLAGS_BOOTSTRAP"]
11421155

1143-
if not os.path.isfile(self.cargo()):
1144-
raise Exception("no cargo executable found at `{}`".format(self.cargo()))
1145-
args = [
1146-
self.cargo(),
1147-
"build",
1148-
"--jobs=" + self.jobs,
1149-
"--manifest-path",
1150-
os.path.join(self.rust_root, "src/bootstrap/Cargo.toml"),
1151-
"-Zroot-dir=" + self.rust_root,
1152-
]
1153-
# verbose cargo output is very noisy, so only enable it with -vv
1154-
args.extend("--verbose" for _ in range(self.verbose - 1))
1155-
11561156
if "BOOTSTRAP_TRACING" in env:
11571157
args.append("--features=tracing")
11581158

src/bootstrap/bootstrap_test.py

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -244,8 +244,10 @@ def test_warnings(self):
244244
if toml_warnings is not None:
245245
configure_args = ["--set", "rust.deny-warnings=" + toml_warnings]
246246

247-
_, env = self.build_args(configure_args, args=["--warnings=warn"])
248-
self.assertFalse("-Dwarnings" in env["RUSTFLAGS"])
247+
args, env = self.build_args(configure_args, args=["--warnings=warn"])
248+
self.assertFalse("CARGO_BUILD_WARNINGS" in env)
249+
self.assertFalse("-Zwarnings" in args)
249250

250-
_, env = self.build_args(configure_args, args=["--warnings=deny"])
251-
self.assertTrue("-Dwarnings" in env["RUSTFLAGS"])
251+
args, env = self.build_args(configure_args, args=["--warnings=deny"])
252+
self.assertEqual("deny", env["CARGO_BUILD_WARNINGS"])
253+
self.assertTrue("-Zwarnings" in args)

src/bootstrap/src/core/builder/cargo.rs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -459,6 +459,11 @@ impl Builder<'_> {
459459
let out_dir = self.stage_out(compiler, mode);
460460
cargo.env("CARGO_TARGET_DIR", &out_dir);
461461

462+
// Set this unconditionally. Cargo silently ignores `CARGO_BUILD_WARNINGS` when `-Z
463+
// warnings` isn't present, which is hard to debug, and it's not worth the effort to keep
464+
// them in sync.
465+
cargo.arg("-Zwarnings");
466+
462467
// Bootstrap makes a lot of assumptions about the artifacts produced in the target
463468
// directory. If users override the "build directory" using `build-dir`
464469
// (https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#build-dir), then
@@ -1159,8 +1164,10 @@ impl Builder<'_> {
11591164
lint_flags.push("-Wunused_lifetimes");
11601165

11611166
if self.config.deny_warnings {
1162-
lint_flags.push("-Dwarnings");
1163-
rustdocflags.arg("-Dwarnings");
1167+
// We use this instead of `lint_flags` so that we don't have to rebuild all
1168+
// workspace dependencies when `deny-warnings` changes, but we still get an error
1169+
// immediately instead of having to wait until the next rebuild.
1170+
cargo.env("CARGO_BUILD_WARNINGS", "deny");
11641171
}
11651172

11661173
rustdocflags.arg("-Wrustdoc::invalid_codeblock_attributes");

0 commit comments

Comments
 (0)