Auto merge of #144011 - Zalathar:check-compiler-no-llvm, r=<try>

bootstrap: Don't trigger an LLVM build from check builds using the stage 0 compiler

Coming back to r-l/r development after a few weeks away, I found a major regression in my dev workflows: running `x check compiler` (either manually or via rust-analyzer) would have the side-effect of building LLVM, even though that shouldn't be necessary.

For my main build directory this would be a minor annoyance, but for my separate rust-analyzer build directory it's a huge problem because it causes a completely separate build of LLVM, which takes a long time and should be completely unnecessary.

---

After some investigation, I tracked down the problem to the `can_skip_build` check in this code:

https://github.com/rust-lang/rust/blob/3014e79f9c8d5510ea7b3a3b70d171d0948b1e96/src/bootstrap/src/core/build_steps/compile.rs#L1382-L1396

Historically, this would skip the LLVM build for stage 0 check builds. But after the recent stage 0 std redesign and some associated check stage renumbering (e.g. #143048), the condition `builder.top_stage == build_stage` is now false, because `top_stage` is 1 (due to the renumbering) but `build_stage` is 0 (because a “stage 1” non-library check build still uses the stage 0 compiler).

---

Because this is a critical contributor roadblock for me, I have tried to fix this in a relatively narrow way. It's possible that all of this surrounding logic could be greatly simplified (especially in light of the stage redesign changes), but I didn't want this fix to be held back by scope creep.

---

(Zulip thread: https://rust-lang.zulipchat.com/#narrow/channel/326414-t-infra.2Fbootstrap/topic/Bootstrap.20incorrectly.20building.20LLVM.20for.20check.20builds/near/528991035)

r? Kobzol
try-job: x86_64-mingw-1
try-job: x86_64-mingw-2

Author: rust-bors[bot]

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);
 

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
@@ -1384,13 +1379,12 @@ pub fn rustc_cargo_env(
     // detected that LLVM is already built and good to go which helps prevent
     // busting caches (e.g. like #71152).
     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)
         }
     }
@@ -1665,7 +1659,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.

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

src/bootstrap/src/core/builder/tests.rs

@@ -630,6 +630,46 @@ fn any_debug() {
     assert_eq!(x.downcast_ref::<MyStruct>(), Some(&MyStruct { x: 7 }));
 }
 
+#[test]
+fn test_check_compiler_doesnt_build_llvm() {
+    // Checking the stage 1 compiler uses the bootstrap compiler, and therefore
+    // should not need to build LLVM.
+    {
+        let config = configure_with_args(
+            &["check", "compiler", "--stage=1"],
+            &[TEST_TRIPLE_1],
+            &[TEST_TRIPLE_1],
+        );
+        let mut cache = run_build(&config.paths.clone(), config);
+
+        assert!(!cache.contains::<llvm::Llvm>());
+    }
+
+    // Checking the stage 1 library uses the stage 1 compiler, so it will need LLVM.
+    {
+        let config = configure_with_args(
+            &["check", "library", "--stage=1"],
+            &[TEST_TRIPLE_1],
+            &[TEST_TRIPLE_1],
+        );
+        let mut cache = run_build(&config.paths.clone(), config);
+
+        assert!(cache.contains::<llvm::Llvm>());
+    }
+
+    // Checking the stage 2 compiler uses the stage 1 compiler, so it will need LLVM.
+    {
+        let config = configure_with_args(
+            &["check", "compiler", "--stage=2"],
+            &[TEST_TRIPLE_1],
+            &[TEST_TRIPLE_1],
+        );
+        let mut cache = run_build(&config.paths.clone(), config);
+
+        assert!(cache.contains::<llvm::Llvm>());
+    }
+}
+
 /// These tests use insta for snapshot testing.
 /// See bootstrap's README on how to bless the snapshots.
 mod snapshot {
@@ -1294,7 +1334,6 @@ mod snapshot {
             ctx.config("check")
                 .path("compiler")
                 .render_steps(), @r"
-        [build] llvm <host>
         [check] rustc 0 <host> -> rustc 1 <host>
         [check] rustc 0 <host> -> cranelift 1 <host>
         [check] rustc 0 <host> -> gcc 1 <host>
@@ -1304,7 +1343,6 @@ mod snapshot {
             ctx.config("check")
                 .path("rustc")
                 .render_steps(), @r"
-        [build] llvm <host>
         [check] rustc 0 <host> -> rustc 1 <host>
         ");
     }
@@ -1324,7 +1362,6 @@ mod snapshot {
                 .path("compiler")
                 .stage(1)
                 .render_steps(), @r"
-        [build] llvm <host>
         [check] rustc 0 <host> -> rustc 1 <host>
         [check] rustc 0 <host> -> cranelift 1 <host>
         [check] rustc 0 <host> -> gcc 1 <host>
@@ -1456,7 +1493,6 @@ mod snapshot {
                 .paths(&["library", "compiler"])
                 .args(&args)
                 .render_steps(), @r"
-        [build] llvm <host>
         [check] rustc 0 <host> -> rustc 1 <host>
         [check] rustc 0 <host> -> cranelift 1 <host>
         [check] rustc 0 <host> -> gcc 1 <host>
@@ -1470,7 +1506,6 @@ mod snapshot {
             ctx.config("check")
                 .path("miri")
                 .render_steps(), @r"
-        [build] llvm <host>
         [check] rustc 0 <host> -> rustc 1 <host>
         [check] rustc 0 <host> -> Miri 1 <host>
         ");
@@ -1491,7 +1526,6 @@ mod snapshot {
                 .path("miri")
                 .stage(1)
                 .render_steps(), @r"
-        [build] llvm <host>
         [check] rustc 0 <host> -> rustc 1 <host>
         [check] rustc 0 <host> -> Miri 1 <host>
         ");
@@ -1544,7 +1578,6 @@ mod snapshot {
             ctx.config("check")
                 .path("rustc_codegen_cranelift")
                 .render_steps(), @r"
-        [build] llvm <host>
         [check] rustc 0 <host> -> rustc 1 <host>
         [check] rustc 0 <host> -> cranelift 1 <host>
         [check] rustc 0 <host> -> gcc 1 <host>
@@ -1558,7 +1591,6 @@ mod snapshot {
             ctx.config("check")
                 .path("rust-analyzer")
                 .render_steps(), @r"
-        [build] llvm <host>
         [check] rustc 0 <host> -> rustc 1 <host>
         [check] rustc 0 <host> -> rust-analyzer 1 <host>
         ");