From 8388b31c5bb1478defee2963c6caae69348a7e1a Mon Sep 17 00:00:00 2001 From: Zalathar Date: Fri, 18 Jul 2025 14:02:00 +1000 Subject: [PATCH 1/3] Split some multi-snapshot tests to make blessing easier When a snapshot test fails, it only emits a `.pending-snap` file for the first snapshot assertion that actually failed, because subsequent assertions aren't executed. That makes it cumbersome to re-bless tests that contain multiple snapshot assertions. --- src/bootstrap/src/core/builder/tests.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/bootstrap/src/core/builder/tests.rs b/src/bootstrap/src/core/builder/tests.rs index 51a906496923b..e71edbb8bc5fb 100644 --- a/src/bootstrap/src/core/builder/tests.rs +++ b/src/bootstrap/src/core/builder/tests.rs @@ -712,7 +712,11 @@ mod snapshot { [build] llvm [build] rustc 0 -> rustc 1 "); + } + #[test] + fn build_rustc_no_explicit_stage() { + let ctx = TestCtx::new(); insta::assert_snapshot!( ctx.config("build") .path("rustc") @@ -1299,7 +1303,11 @@ mod snapshot { [check] rustc 0 -> cranelift 1 [check] rustc 0 -> gcc 1 "); + } + #[test] + fn check_rustc_no_explicit_stage() { + let ctx = TestCtx::new(); insta::assert_snapshot!( ctx.config("check") .path("rustc") From 7a217e1ba51b486cd7d56eb684ae44dc3a44e1b3 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Wed, 16 Jul 2025 17:16:15 +1000 Subject: [PATCH 2/3] Don't trigger an LLVM build from check builds using the stage 0 compiler --- src/bootstrap/src/core/build_steps/compile.rs | 29 ++++++++++++------- src/bootstrap/src/core/builder/tests.rs | 8 ----- 2 files changed, 19 insertions(+), 18 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/compile.rs b/src/bootstrap/src/core/build_steps/compile.rs index 09bb2e35bdaa2..9d29b1320b115 100644 --- a/src/bootstrap/src/core/build_steps/compile.rs +++ b/src/bootstrap/src/core/build_steps/compile.rs @@ -1323,7 +1323,7 @@ pub fn rustc_cargo_env( builder: &Builder<'_>, cargo: &mut Cargo, target: TargetSelection, - build_stage: u32, + _build_stage: u32, ) { // Set some configuration variables picked up by build scripts and // the compiler alike @@ -1379,18 +1379,24 @@ pub fn rustc_cargo_env( cargo.rustflag("--cfg=llvm_enzyme"); } - // Note that this is disabled if LLVM itself is disabled or we're in a check - // build. If we are in a check build we still go ahead here presuming we've - // detected that LLVM is already built and good to go which helps prevent - // busting caches (e.g. like #71152). + // These conditionals represent a tension between three forces: + // - For non-check builds, we need to define some LLVM-related environment + // variables, requiring LLVM to have been built. + // - For check builds, we want to avoid building LLVM if possible. + // - Check builds and non-check builds should have the same environment if + // possible, to avoid unnecessary rebuilds due to cache-busting. + // + // Therefore we try to avoid building LLVM for check builds, but only if + // building LLVM would be expensive. If "building" LLVM is cheap + // (i.e. it's already built or is downloadable), we prefer to maintain a + // consistent environment between check and non-check builds. if builder.config.llvm_enabled(target) { - let building_is_expensive = + let building_llvm_is_expensive = crate::core::build_steps::llvm::prebuilt_llvm_config(builder, target, false) .should_build(); - // `top_stage == stage` might be false for `check --stage 1`, if we are building the stage 1 compiler - let can_skip_build = builder.kind == Kind::Check && builder.top_stage == build_stage; - let should_skip_build = building_is_expensive && can_skip_build; - if !should_skip_build { + + let skip_llvm = (builder.kind == Kind::Check) && building_llvm_is_expensive; + if !skip_llvm { rustc_llvm_env(builder, cargo, target) } } @@ -1407,6 +1413,9 @@ pub fn rustc_cargo_env( /// Pass down configuration from the LLVM build into the build of /// rustc_llvm and rustc_codegen_llvm. +/// +/// Note that this has the side-effect of _building LLVM_, which is sometimes +/// unwanted (e.g. for check builds). fn rustc_llvm_env(builder: &Builder<'_>, cargo: &mut Cargo, target: TargetSelection) { if builder.config.is_rust_llvm(target) { cargo.env("LLVM_RUSTLLVM", "1"); diff --git a/src/bootstrap/src/core/builder/tests.rs b/src/bootstrap/src/core/builder/tests.rs index e71edbb8bc5fb..97e7e512ce1a9 100644 --- a/src/bootstrap/src/core/builder/tests.rs +++ b/src/bootstrap/src/core/builder/tests.rs @@ -1298,7 +1298,6 @@ mod snapshot { ctx.config("check") .path("compiler") .render_steps(), @r" - [build] llvm [check] rustc 0 -> rustc 1 [check] rustc 0 -> cranelift 1 [check] rustc 0 -> gcc 1 @@ -1312,7 +1311,6 @@ mod snapshot { ctx.config("check") .path("rustc") .render_steps(), @r" - [build] llvm [check] rustc 0 -> rustc 1 "); } @@ -1332,7 +1330,6 @@ mod snapshot { .path("compiler") .stage(1) .render_steps(), @r" - [build] llvm [check] rustc 0 -> rustc 1 [check] rustc 0 -> cranelift 1 [check] rustc 0 -> gcc 1 @@ -1464,7 +1461,6 @@ mod snapshot { .paths(&["library", "compiler"]) .args(&args) .render_steps(), @r" - [build] llvm [check] rustc 0 -> rustc 1 [check] rustc 0 -> cranelift 1 [check] rustc 0 -> gcc 1 @@ -1478,7 +1474,6 @@ mod snapshot { ctx.config("check") .path("miri") .render_steps(), @r" - [build] llvm [check] rustc 0 -> rustc 1 [check] rustc 0 -> Miri 1 "); @@ -1499,7 +1494,6 @@ mod snapshot { .path("miri") .stage(1) .render_steps(), @r" - [build] llvm [check] rustc 0 -> rustc 1 [check] rustc 0 -> Miri 1 "); @@ -1552,7 +1546,6 @@ mod snapshot { ctx.config("check") .path("rustc_codegen_cranelift") .render_steps(), @r" - [build] llvm [check] rustc 0 -> rustc 1 [check] rustc 0 -> cranelift 1 [check] rustc 0 -> gcc 1 @@ -1566,7 +1559,6 @@ mod snapshot { ctx.config("check") .path("rust-analyzer") .render_steps(), @r" - [build] llvm [check] rustc 0 -> rustc 1 [check] rustc 0 -> rust-analyzer 1 "); From a028ca5f9bd771e825e5c8c3c55f938aea89985e Mon Sep 17 00:00:00 2001 From: Zalathar Date: Wed, 16 Jul 2025 20:53:29 +1000 Subject: [PATCH 3/3] Clean up an unused compiler-stage parameter --- src/bootstrap/src/core/build_steps/check.rs | 2 +- src/bootstrap/src/core/build_steps/compile.rs | 11 +++-------- src/bootstrap/src/core/build_steps/test.rs | 4 ++-- 3 files changed, 6 insertions(+), 11 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/check.rs b/src/bootstrap/src/core/build_steps/check.rs index 3278b55305c8b..0aec3cf7ca4b3 100644 --- a/src/bootstrap/src/core/build_steps/check.rs +++ b/src/bootstrap/src/core/build_steps/check.rs @@ -350,7 +350,7 @@ impl Step for CodegenBackend { cargo .arg("--manifest-path") .arg(builder.src.join(format!("compiler/rustc_codegen_{backend}/Cargo.toml"))); - rustc_cargo_env(builder, &mut cargo, target, build_compiler.stage); + rustc_cargo_env(builder, &mut cargo, target); let _guard = builder.msg_check(format!("rustc_codegen_{backend}"), target, None); diff --git a/src/bootstrap/src/core/build_steps/compile.rs b/src/bootstrap/src/core/build_steps/compile.rs index 9d29b1320b115..fc5bf6dc354ea 100644 --- a/src/bootstrap/src/core/build_steps/compile.rs +++ b/src/bootstrap/src/core/build_steps/compile.rs @@ -1316,15 +1316,10 @@ pub fn rustc_cargo( cargo.env("RUSTC_WRAPPER", ccache); } - rustc_cargo_env(builder, cargo, target, build_compiler.stage); + rustc_cargo_env(builder, cargo, target); } -pub fn rustc_cargo_env( - builder: &Builder<'_>, - cargo: &mut Cargo, - target: TargetSelection, - _build_stage: u32, -) { +pub fn rustc_cargo_env(builder: &Builder<'_>, cargo: &mut Cargo, target: TargetSelection) { // Set some configuration variables picked up by build scripts and // the compiler alike cargo @@ -1674,7 +1669,7 @@ impl Step for CodegenBackend { cargo .arg("--manifest-path") .arg(builder.src.join(format!("compiler/rustc_codegen_{backend}/Cargo.toml"))); - rustc_cargo_env(builder, &mut cargo, target, compiler.stage); + rustc_cargo_env(builder, &mut cargo, target); // Ideally, we'd have a separate step for the individual codegen backends, // like we have in tests (test::CodegenGCC) but that would require a lot of restructuring. diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index 9e7ea5c115f19..b7dd2b778bb66 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -3386,7 +3386,7 @@ impl Step for CodegenCranelift { cargo .arg("--manifest-path") .arg(builder.src.join("compiler/rustc_codegen_cranelift/build_system/Cargo.toml")); - compile::rustc_cargo_env(builder, &mut cargo, target, compiler.stage); + compile::rustc_cargo_env(builder, &mut cargo, target); // Avoid incremental cache issues when changing rustc cargo.env("CARGO_BUILD_INCREMENTAL", "false"); @@ -3518,7 +3518,7 @@ impl Step for CodegenGCC { cargo .arg("--manifest-path") .arg(builder.src.join("compiler/rustc_codegen_gcc/build_system/Cargo.toml")); - compile::rustc_cargo_env(builder, &mut cargo, target, compiler.stage); + compile::rustc_cargo_env(builder, &mut cargo, target); add_cg_gcc_cargo_flags(&mut cargo, &gcc); // Avoid incremental cache issues when changing rustc