Skip to content

Commit 20cb59b

Browse files
committed
bootstrap: respect modern 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`. 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>
1 parent 79a1e77 commit 20cb59b

File tree

2 files changed

+20
-4
lines changed

2 files changed

+20
-4
lines changed

src/bootstrap/src/core/build_steps/llvm.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -773,7 +773,15 @@ fn configure_cmake(
773773
.define("CMAKE_CXX_COMPILER", sanitize_cc(&cxx))
774774
.define("CMAKE_ASM_COMPILER", sanitize_cc(&cc));
775775

776-
cfg.build_arg("-j").build_arg(builder.jobs().to_string());
776+
// If we are running under a FIFO jobserver, we should not pass -j to CMake; otherwise it
777+
// overrides the jobserver settings and can lead to oversubscription.
778+
let has_modern_jobserver = env::var("MAKEFLAGS")
779+
.map(|flags| flags.contains("--jobserver-auth=fifo:"))
780+
.unwrap_or(false);
781+
782+
if !has_modern_jobserver {
783+
cfg.build_arg("-j").build_arg(builder.jobs().to_string());
784+
}
777785
let mut cflags = ccflags.cflags.clone();
778786
// FIXME(madsmtm): Allow `cmake-rs` to select flags by itself by passing
779787
// our flags via `.cflag`/`.cxxflag` instead.

src/bootstrap/src/core/builder/cargo.rs

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -547,9 +547,17 @@ impl Builder<'_> {
547547
assert_eq!(target, compiler.host);
548548
}
549549

550-
// Remove make-related flags to ensure Cargo can correctly set things up
551-
cargo.env_remove("MAKEFLAGS");
552-
cargo.env_remove("MFLAGS");
550+
// Bootstrap only supports modern FIFO jobservers. Older pipe-based jobservers can run into
551+
// "invalid file descriptor" errors, as the jobserver file descriptors are not inherited by
552+
// scripts like bootstrap.py, while the environment variable is propagated. So, we pass
553+
// MAKEFLAGS only if we detect a FIFO jobserver, otherwise we clear it.
554+
let has_modern_jobserver = env::var("MAKEFLAGS")
555+
.map(|flags| flags.contains("--jobserver-auth=fifo:"))
556+
.unwrap_or(false);
557+
558+
if !has_modern_jobserver {
559+
cargo.env_remove("MAKEFLAGS");
560+
}
553561

554562
cargo
555563
}

0 commit comments

Comments
 (0)