Skip to content

Commit 1b0c024

Browse files
authored
Rollup merge of rust-lang#74046 - ehuss:deny-warnings-caching, r=Mark-Simulacrum
Fix caching issue when building tools. This fixes a problem with tool builds not being cached properly. rust-lang#73297 changed it so that Clippy will participate in the "deny warnings" setting. Unfortunately this causes a problem because Clippy shares the build directory with other tools which do not participate in "deny warnings". Because Cargo does not independently cache artifacts based on different RUSTFLAGS settings, it causes all the shared dependencies to get rebuilt if Clippy ever gets built. The solution here is to stop using RUSTFLAGS, and just sneak the settings in through the rustc wrapper. Cargo won't know about the different settings, so it will not bust the cache. This should be safe since lint settings on dependencies are ignored. This is how things used to work in the past before rust-lang#64316. Alternate solutions: * Treat Clippy as a "submodule" and don't enforce warnings on it. This was the behavior before rust-lang#73297. The consequence is that if a warning sneaks into clippy, that the clippy maintainers will need to fix it when they sync clippy back to the clippy repo. * Just deny warnings on all tools (removing the in-tree/submodule distinction). This is tempting, but with some issues (cc rust-lang#52336): * Adding or changing warnings in rustc can be difficult to land because tools have to be updated if they trip the warning. In practice, this isn't too bad. Cargo (and rustfmt) already runs with `deny(warnings)`, so this has been the de-facto standard already (although they do not use the extra lints like `unused_lifetimes`). * Teach Cargo to add flags to the workspace members, but not dependencies. * Teach Cargo to add flags without fingerprinting them? * Teach Cargo to independently cache different RUSTFLAGS artifacts (this was [reverted](rust-lang/cargo#7417) due to complications). This would also unnecessarily rebuild dependencies, but would avoid cache thrashing. * Teach Cargo about lint settings. Closes rust-lang#74016
2 parents 51a8ce1 + 310c97b commit 1b0c024

File tree

2 files changed

+18
-4
lines changed

2 files changed

+18
-4
lines changed

src/bootstrap/bin/rustc.rs

+4
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,10 @@ fn main() {
7676
cmd.env("RUST_BACKTRACE", "1");
7777
}
7878

79+
if let Ok(lint_flags) = env::var("RUSTC_LINT_FLAGS") {
80+
cmd.args(lint_flags.split_whitespace());
81+
}
82+
7983
if target.is_some() {
8084
// The stage0 compiler has a special sysroot distinct from what we
8185
// actually downloaded, so we just always pass the `--sysroot` option,

src/bootstrap/builder.rs

+14-4
Original file line numberDiff line numberDiff line change
@@ -1130,22 +1130,32 @@ impl<'a> Builder<'a> {
11301130
cargo.env("RUSTC_VERBOSE", self.verbosity.to_string());
11311131

11321132
if source_type == SourceType::InTree {
1133+
let mut lint_flags = Vec::new();
11331134
// When extending this list, add the new lints to the RUSTFLAGS of the
11341135
// build_bootstrap function of src/bootstrap/bootstrap.py as well as
11351136
// some code doesn't go through this `rustc` wrapper.
1136-
rustflags.arg("-Wrust_2018_idioms");
1137-
rustflags.arg("-Wunused_lifetimes");
1137+
lint_flags.push("-Wrust_2018_idioms");
1138+
lint_flags.push("-Wunused_lifetimes");
11381139

11391140
if self.config.deny_warnings {
1140-
rustflags.arg("-Dwarnings");
1141+
lint_flags.push("-Dwarnings");
11411142
}
11421143

11431144
// FIXME(#58633) hide "unused attribute" errors in incremental
11441145
// builds of the standard library, as the underlying checks are
11451146
// not yet properly integrated with incremental recompilation.
11461147
if mode == Mode::Std && compiler.stage == 0 && self.config.incremental {
1147-
rustflags.arg("-Aunused-attributes");
1148+
lint_flags.push("-Aunused-attributes");
11481149
}
1150+
// This does not use RUSTFLAGS due to caching issues with Cargo.
1151+
// Clippy is treated as an "in tree" tool, but shares the same
1152+
// cache as other "submodule" tools. With these options set in
1153+
// RUSTFLAGS, that causes *every* shared dependency to be rebuilt.
1154+
// By injecting this into the rustc wrapper, this circumvents
1155+
// Cargo's fingerprint detection. This is fine because lint flags
1156+
// are always ignored in dependencies. Eventually this should be
1157+
// fixed via better support from Cargo.
1158+
cargo.env("RUSTC_LINT_FLAGS", lint_flags.join(" "));
11491159
}
11501160

11511161
if let Mode::Rustc | Mode::Codegen = mode {

0 commit comments

Comments
 (0)