Skip to content

bootstrap: respect POSIX jobserver #152057

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rust-bors merged 1 commit into from
Feb 20, 2026
+20 −4
Merged

bootstrap: respect POSIX jobserver #152057

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion src/bootstrap/src/core/build_steps/llvm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -773,7 +773,15 @@ fn configure_cmake(
.define("CMAKE_CXX_COMPILER", sanitize_cc(&cxx))
.define("CMAKE_ASM_COMPILER", sanitize_cc(&cc));

cfg.build_arg("-j").build_arg(builder.jobs().to_string());
// If we are running under a FIFO jobserver, we should not pass -j to CMake; otherwise it
// overrides the jobserver settings and can lead to oversubscription.
let has_modern_jobserver = env::var("MAKEFLAGS")
.map(|flags| flags.contains("--jobserver-auth=fifo:"))
.unwrap_or(false);

if !has_modern_jobserver {
cfg.build_arg("-j").build_arg(builder.jobs().to_string());
}
let mut cflags = ccflags.cflags.clone();
// FIXME(madsmtm): Allow `cmake-rs` to select flags by itself by passing
// our flags via `.cflag`/`.cxxflag` instead.
Expand Down
14 changes: 11 additions & 3 deletions src/bootstrap/src/core/builder/cargo.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -547,9 +547,17 @@ impl Builder<'_> {
assert_eq!(target, compiler.host);
}

// Remove make-related flags to ensure Cargo can correctly set things up
cargo.env_remove("MAKEFLAGS");
cargo.env_remove("MFLAGS");
Copy link
Member

@bjorn3 bjorn3 Feb 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was intentional because of #56090 (comment). I don't know if that issue has been fixed since or not.

Copy link
Contributor

@petrochenkov petrochenkov Feb 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jobserver-rs has much better diagnostics now, even if it's not fixed now it will be more clear what's going on.

Copy link
Contributor Author

@haampie haampie Feb 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll have a look. Possibly I could split the PR in two; the bootstrap - cmake - ninja-build - ... process tree doesn't have cargo in it.

Copy link
Contributor Author

@haampie haampie Feb 4, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I quickly looked at it. Few observations:

The "bad file descriptor" issue applies only to old style jobservers that work with pipes. The issue happens when a parent process either closes the file descriptors explicitly or makes them non-inheritable, yet continues to advertise them in the MAKEFLAGS environment variable to child processes.

As far as I can see, sccache closes file descriptors explicitly, but for a valid reason: they start a daemon the first time you run sccache with has a lifetime (heh) longer than the jobserver. Afterwards, they start their own jobserver, and I suppose they set their own MAKEFLAGS too then.

Another source of this issue is possibly the Python scripts in this repo. Python defaults to making file descriptors non-inheritable (PEP 445). So, to support an old style jobserver, subprocess.Popen(..., close_fds=False) should be used in bootstrap.py in this repo.

However, given that (a) ninja only supports the new style jobserver, and (b) the sccache logic about closing fds doesn't run with the new style jobserver, I don't think it's worth trying to support the old jobserver protocol.

So, my proposal is, if MAKEFLAGS contains the string --jobserver-auth=fifo: (new style jobserver):

  1. Pass MAKEFLAGS verbatim to child processes
  2. Avoid passing -j N to cmake to ensure cmake and ninja respect the jobserver

That would be the minimal change, and there shouldn't be any bad file descriptor errors.

Copy link
Contributor Author

@haampie haampie Feb 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've pushed a change, I don't think there should be any concerns related to "bad file descriptor" issues now.

// Bootstrap only supports modern FIFO jobservers. Older pipe-based jobservers can run into
// "invalid file descriptor" errors, as the jobserver file descriptors are not inherited by
// scripts like bootstrap.py, while the environment variable is propagated. So, we pass
// MAKEFLAGS only if we detect a FIFO jobserver, otherwise we clear it.
let has_modern_jobserver = env::var("MAKEFLAGS")
.map(|flags| flags.contains("--jobserver-auth=fifo:"))
.unwrap_or(false);

if !has_modern_jobserver {
cargo.env_remove("MAKEFLAGS");
}

cargo
}
Expand Down
Loading