Fix checking of LLVM prebuilt status - #161475
Merged
Merged
Conversation
Collaborator
|
This PR changes how LLVM is built. Consider updating src/bootstrap/download-ci-llvm-stamp. |
rustbot
added
S-waiting-on-review
This comment has been minimized.
This comment has been minimized.
…p building LLVM Because when we do e.g. `x check library`, during that invocation we build the compiler, and that needs LLVM.
Kobzol
force-pushed
the
bootstrap-llvm-check-fix
branch
from
August 21, 2026 19:21
bef82f4 to
e0c2819
Compare
rustbot
added
the
A-LLVM
This comment has been minimized.
This comment has been minimized.
Member
Author
|
I pushed the commit from #161466 and PR CI is green, so it looks like this PR indeed fixes the problem. |
Member
|
Can we land this fix first, then do the LLVM submodule update separately? EDIT: oh I see that's what you intended in #161466 (comment) indeed |
jieyouxu
approved these changes
Aug 22, 2026
Member
There was a problem hiding this comment.
Thanks, r=me after dropping the LLVM submodule commit
@rustbot author
| let building_llvm_is_expensive = prebuilt_llvm_output(builder, target).is_none(); | ||
|
|
||
| let skip_llvm = (builder.kind == Kind::Check) && building_llvm_is_expensive; | ||
| let skip_llvm = (cargo.kind() == Kind::Check) && building_llvm_is_expensive; |
Member
There was a problem hiding this comment.
Remark: hm, in some sense the "overall" (?) build kind (tied to the cli invocation) is not the same as the "local" build kind (in the case of using cargo, which check/build etc. is used). I.e. the over build kind matters only for the final "step", but it may be multiple local "build" kind steps leading up to the final step.
I wonder if there are ways to make this logic more clear but yeah.
rustbot
added
S-waiting-on-author
Kobzol
force-pushed
the
bootstrap-llvm-check-fix
branch
from
August 22, 2026 11:15
e0c2819 to
83a54b3
Compare
Member
Author
|
@bors r=jieyouxu rollup=iffy |
Contributor
rust-bors
Bot
added
S-waiting-on-bors
rust-bors Bot
pushed a commit
that referenced
this pull request
Aug 22, 2026
…uwer Rollup of 9 pull requests Successful merges: - #161475 (Fix checking of LLVM prebuilt status) - #161269 (Remove `impl DebugWithContext for BorrowckDomain`) - #161419 (Doclink to `char::REPLACEMENT_CHARACTER` instead of `std::char::REPLACEMENT_CHARACTER`.) - #161428 (Install cargo tools with locked dependencies) - #161502 (Clean up a few `_inner` functions after `ensure_sufficient_stack`) - #161507 (Remove useless `!fields.is_empty()` check) - #161512 (enable next solver in Miri) - #161516 (Revert #161236 (Download auto jobs in citool in parallel)) - #161518 (rename `T-libs-api` to `T-libs` in issue templates)
rust-bors Bot
pushed a commit
that referenced
this pull request
Aug 22, 2026
Rollup merge of #161475 - Kobzol:bootstrap-llvm-check-fix, r=jieyouxu Fix checking of LLVM prebuilt status This regressed in #160916. I'll start from the end. There was a pre-existing bug (fixed by the second commit of this PR), where if we do `x check library`, we have `builder.kind == Kind::Check`, but we are actually building things (like the compiler) during that bootstrap invocation. But bootstrap was only checking the builder kind before, and in that case it would skip building LLVM, *unless* it was already built locally previously. On PR CI, and perhaps always (because build steps executed during check likely only occur during `x check library`, which requires *building* the compiler anyway), the LLVM was built locally anyway, so this bug was hidden away. This also removes an unnecessary LLVM build when running Clippy on the rustc_private tools. However, after #160916, this was no longer case, because it stopped treating locally built LLVM as being prebuilt (which, in and of itself, is kinda a bug). Because when we check the compiler, we want to avoid building (and checking out!) LLVM. Before #160916, bootstrap considered a *previously locally built* LLVM to be available as a prebuilt `llvm-config`, and in that case configured `LLVM_CONFIG` for `rustc_llvm`. Because all PR CI bootstrap invocations that do `check` actually built LLVM prior doing a build, this worked, somehow, but broke after my PR. Should unblock #161466. r? jieyouxu
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This regressed in #160916.
I'll start from the end. There was a pre-existing bug (fixed by the second commit of this PR), where if we do
x check library, we havebuilder.kind == Kind::Check, but we are actually building things (like the compiler) during that bootstrap invocation. But bootstrap was only checking the builder kind before, and in that case it would skip building LLVM, unless it was already built locally previously. On PR CI, and perhaps always (because build steps executed during check likely only occur duringx check library, which requires building the compiler anyway), the LLVM was built locally anyway, so this bug was hidden away. This also removes an unnecessary LLVM build when running Clippy on the rustc_private tools.However, after #160916, this was no longer case, because it stopped treating locally built LLVM as being prebuilt (which, in and of itself, is kinda a bug). Because when we check the compiler, we want to avoid building (and checking out!) LLVM. Before #160916, bootstrap considered a previously locally built LLVM to be available as a prebuilt
llvm-config, and in that case configuredLLVM_CONFIGforrustc_llvm. Because all PR CI bootstrap invocations that docheckactually built LLVM prior doing a build, this worked, somehow, but broke after my PR.Should unblock #161466.
r? jieyouxu