Skip to content
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.

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

Bootstraping ignores make jobserver #116101

Open
jefferyto opened this issue Sep 23, 2023 · 7 comments
Open

Bootstraping ignores make jobserver #116101

jefferyto opened this issue Sep 23, 2023 · 7 comments
Labels
C-bug Category: This is a bug. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-cargo Relevant to the cargo team, which will review and decide on the PR/issue.

Comments

@jefferyto
Copy link
Contributor

I'm currently working on enhancing the Rust package (currently 1.72.0) in OpenWrt (the build system is based on buildroot). After ensuring the x.py command is prefixed with + and that MAKEFLAGS is set correctly, bootstraping continues to use all of my CPUs instead of the -jN jobs from the make command line.

I have tried patching out these lines that remove MAKEFLAGS from the environment for cargo; the MAKEFLAGS value does reach cargo but cargo continues to set up its own jobserver instead.

I can see that removing MAKEFLAGS from the environment was deliberate (as noted in #50629 (comment)), but it would be great if building rustc can cooperate with the rest of the build system by using the same jobserver.

@jefferyto jefferyto added the C-bug Category: This is a bug. label Sep 23, 2023
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Sep 23, 2023
@albertlarsan68 albertlarsan68 added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Sep 23, 2023
@onur-ozkan onur-ozkan self-assigned this Sep 23, 2023
@saethlin saethlin removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Sep 23, 2023
@onur-ozkan
Copy link
Member

onur-ozkan commented Sep 23, 2023

I can see that removing MAKEFLAGS from the environment was deliberate (as noted in #50629 (comment)), but it would be great if building rustc can cooperate with the rest of the build system by using the same jobserver.

Unfortunately, due to numerious problems achieving this 100% during the bootstrapping process is not possible. However, there is a workaround provided in this PR: #116108, which allows you to set the cargo jobs to the value provided to Make using -jN, similar to how it's done with x.py -jN.

bootstraping continues to use all of my CPUs instead of the -jN jobs from the make command line.

The PR linked above should resolve this problem.

@jefferyto
Copy link
Contributor Author

However, there is a workaround provided in this PR: #116108, which allows you to set the cargo jobs to the value provided to Make using -jN, similar to how it's done with x.py -jN.

Thanks but that doesn't solve the issue. When I do a build in OpenWrt, I am often building many packages, not just Rust. If we just give the same -jN value to Rust, say -j4, then the overall build system will think there are 4 jobs available and Rust will also think there are 4 jobs available. Then potentially the overall build system will be building 3 other packages and Rust will be using 4 jobs, i.e. total of 7 jobs. This is why Rust should use the same jobserver as make.

@onur-ozkan
Copy link
Member

I thought you were only building rust from make, I see now. Thanks for clarification.

I have tried patching out these lines that remove MAKEFLAGS from the environment for cargo; the MAKEFLAGS value does reach cargo but cargo continues to set up its own jobserver instead.

I assume the patch was removing those lines, right?

@jefferyto
Copy link
Contributor Author

I assume the patch was removing those lines, right?

Yes. I removed the lines from bare_cargo() not rustdoc_cmd() though (the commit you referenced in #116108 (comment) was for rustdoc_cmd()); I assume rustdoc_cmd() is only invoked when building rustdoc.

@onur-ozkan
Copy link
Member

Then this

the MAKEFLAGS value does reach cargo but cargo continues to set up its own jobserver instead.

means the real issue is more related with cargo itself.

@onur-ozkan onur-ozkan added T-cargo Relevant to the cargo team, which will review and decide on the PR/issue. and removed T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Sep 24, 2023
@onur-ozkan onur-ozkan removed their assignment Sep 24, 2023
@onur-ozkan onur-ozkan added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Sep 24, 2023
@onur-ozkan
Copy link
Member

onur-ozkan commented Sep 24, 2023

We can remove

rust/src/bootstrap/builder.rs

Lines 1246 to 1248 in 70a7fe1

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

lines once

the MAKEFLAGS value does reach cargo but cargo continues to set up its own jobserver instead.

is fixed from cargo side(I haven't tested it, so I added the needs-triage label).

@jefferyto
Copy link
Contributor Author

means the real issue is more related with cargo itself.

I'm able to have cargo use the same make jobserver (to build other Rust packages) after rustc is compiled/installed (cargo is called directly from the Makefile), so I would say that bootstrap is at least involved in this issue, if not the main cause.

@onur-ozkan onur-ozkan added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Sep 24, 2023
@fmease fmease removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Sep 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-cargo Relevant to the cargo team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants