Closed
Conversation
When bootstrapping Rust, the `-j N` flag was passed to CMake, which was then forwarded to Ninja. This prevents the jobserver from being used, and as a result leads to oversubscription when Rust is just one of the many packages built as part of a larger software stack. Since Cargo and the Rust compiler have long supported the jobserver, it would be good if also bootstrapping Rust itself would participate in the protocol, leading to composable parallelism. This change allows bootstrapping to respect an existing FIFO based jobserver. Old pipe based jobservers are not supported, because they are brittle: currently the Python scripts in bootstrap do not inherit the file descriptors, but do pass on `MAKEFLAGS`. Because Ninja only supports FIFO based jobservers, it's better to focus on new jobservers only. In summary: * Bootstrap Cargo passes `MAKEFLAGS` verbatim to subprocesses if it advertises a FIFO style jobserver, otherwise it unsets it. * `llvm.rs` does not pass `-j` to `cmake` when a FIFO style jobserver is set in `MAKEFLAGS. * Bootstrap Cargo no longer unsets `MKFLAGS`: from git blame, GNU Make considered it a historical artifact back in 1992, and it is never read by GNU Make, it's only set for backwards compatibility. Signed-off-by: Harmen Stoppels <me@harmenstoppels.nl>
And `install src` with `build.docs = false`.
`use _` is never valid, so it should not be suggested as a similar name.
Rather than "an"
…observer-protocol, r=clubby789 bootstrap: respect POSIX jobserver When bootstrapping Rust, the `-j N` flag was passed to CMake, which was then forwarded to Ninja. This prevents the jobserver from being used, and as a result leads to oversubscription when Rust is just one of the many packages built as part of a larger software stack. Since Cargo and the Rust compiler have long supported the jobserver, it would be good if also bootstrapping Rust itself would participate in the protocol, leading to composable parallelism. This change allows bootstrapping to respect an existing FIFO based jobserver. Old pipe based jobservers are not supported, because they are brittle: currently the Python scripts in bootstrap do not inherit the file descriptors, but do pass on `MAKEFLAGS`, which has lead to errors like "invalid file descriptor" in the past. Because Ninja only supports FIFO based jobservers, it's better to focus on new jobservers only, which shouldn't suffer from the "invalid file descriptor" issue. In summary: * Bootstrap Cargo passes `MAKEFLAGS` verbatim to subprocesses if it advertises a FIFO style jobserver, otherwise it unsets it. This ensures subprocesses respect the jobserver during bootstrap. * `llvm.rs` does not pass `-j` to `cmake` when a FIFO style jobserver is set in `MAKEFLAGS`. This ensures Ninja respects the jobserver. * Bootstrap Cargo no longer unsets `MKFLAGS`: from git blame, GNU Make considered it a historical artifact back in 1992, and it is never read by GNU Make, it's only set for backwards compatibility in case sub-Makefiles read it. --- I've tested this with the [Spack package manager](https://github.com/spack/spack) starting the POSIX jobserver, building node.js and rust in parallel with `-j16`, which looks like this: ```console $ pstree 382710 python3─┬─python3 └─python3─┬─python3─┬─make───make───6*[ccache───g++───cc1plus] │ └─{python3} └─python3─┬─python3.11───bootstrap───cmake───ninja-build───10*[sh───ccache───g++───cc1plus] └─{python3} ``` As you can see there are 10 `g++` processes running for rust, and `6` for node.js, and with a mix of `make` and `ninja` as build tools :). (The only violation I see now is `rust-lld`, but I think that'll be fixed with the LLVM 23 release)
DOC: do not link to "nightly" in Iterator::by_ref() docstring I happened to see that the link in the docstring https://doc.rust-lang.org/std/iter/trait.Iterator.html#method.by_ref points to the "nightly" docs instead of "stable". I'm not sure if my fix actually works because I didn't get `cargo doc` to run on my local computer.
…docs, r=clubby789
Add bootstrap snapshot tests for {`install`, `install src`}
And `install src` with `build.docs = false`.
This is mostly to get coverage for the baseline to make it easier to review rust-lang#150845.
…ercote Clarify some variable names in the query proc-macro These are some cleanups to the `rustc_macros::query`, extracted from rust-lang#152833. r? nnethercote
Fix typo in doc for core::mem::type_info::Struct
…ort-suggestion, r=Kivooeo resolve: do not suggest `_` for unresolved imports Fix invalid unresolved-import suggestion when a module contains an item named `_`. `use _` is never valid syntax, so `_` should not be suggested as a similar name. Closes rust-lang#152812
std::ops::ControlFlow - use "a" before `Result` Rather than "an"
rustbot
added
A-query-system
Member
Author
|
@bors r+ rollup=never p=5 |
Contributor
rust-bors
bot
added
S-waiting-on-bors
This comment has been minimized.
This comment has been minimized.
rust-bors bot
pushed a commit
that referenced
this pull request
Feb 20, 2026
Rollup of 7 pull requests Successful merges: - #152057 (bootstrap: respect POSIX jobserver) - #152818 (DOC: do not link to "nightly" in Iterator::by_ref() docstring) - #152840 (Add bootstrap snapshot tests for {`install`, `install src`}) - #152846 (Clarify some variable names in the query proc-macro) - #152858 (Fix typo in doc for core::mem::type_info::Struct) - #152861 (resolve: do not suggest `_` for unresolved imports) - #152873 (std::ops::ControlFlow - use "a" before `Result`)
Member
|
The current CI attempt has a cursed Apple runner (which might finish) and a cursed Linux runner (which seems unlikely to finish), so letting this go for the full six hours is probably not worthwhile. |
Member
Author
|
Yeah. I'll kill this and replace it with a rollup encompassing the additional PR approved in the meantime. @bors r- |
Contributor
|
📋 Only unclosed PRs can be unapproved. |
rust-bors
bot
added
S-waiting-on-review
Contributor
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
9 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.
Successful merges:
install,install src} #152840 (Add bootstrap snapshot tests for {install,install src})_for unresolved imports #152861 (resolve: do not suggest_for unresolved imports)Result#152873 (std::ops::ControlFlow - use "a" beforeResult)r? @ghost
Create a similar rollup