bootstrap: Don't trigger an unnecessary LLVM build from check builds

[Zalathar]

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:

rust/src/bootstrap/src/core/build_steps/compile.rs

Lines 1382 to 1396 in 3014e79

	// 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).
	if builder.config.llvm_enabled(target) {
	let building_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 {
	rustc_llvm_env(builder, cargo, target)
	}
	}

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)

[rustbot]

Kobzol is not on the review rotation at the moment.
They may take a while to respond.

[Zalathar]

r? bootstrap

[Zalathar]

Hmm, in addition to those snapshot failures, I'm also seeing LLVM builds in subsequent manual testing. Mysterious.

@rustbot author

[rustbot]

Reminder, once the PR becomes ready for a review, use @rustbot ready.

[Zalathar]

Updated snapshot tests, several of which included the inappropriate LLVM build (diff).

[Zalathar]

Having to deal with PR CI failures for i686-pc-windows-gnu feels like I am being pranked. Can't wait to see that target further demoted to oblivion.

[Zalathar]

@bors2 try jobs=x86_64-pc-windows-gnu

[rust-bors]

⌛ Trying commit bf75472 with merge 635dd0f…

To cancel the try build, run the command @bors2 try cancel.

[rust-bors]

💔 Test failed

• CI ❌

[Zalathar]

@bors2 try jobs=x86_64-mingw-1,x86_64-mingw-2

[rust-bors]

⌛ Trying commit bf75472 with merge ddfea62…

To cancel the try build, run the command @bors2 try cancel.

[Zalathar]

Historical note: The existing stage-sensitive check was introduced back in #110122.

From looking at the context, and considering the many changes that have been made since, I think it should be fine to remove the stage-number check.

[rust-bors]

☀️ Try build successful (CI)
Build commit: ddfea62 (ddfea62a5c617a7e19b1257e31a91974fdde3605, parent: f21fbac535ab2c3bc50db20547f4d48477357103)

[mati865]

Having to deal with PR CI failures for i686-pc-windows-gnu feels like I am being pranked. Can't wait to see that target further demoted to oblivion.

TBH that error would happen on any Windows target other than *-windows-gnullvm ones, so *-windows-msvc would also fail. i686-pc-windows-gnu just happens to be checked by the tidy job.