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

Support inheriting jobserver when using cargo run #12597

Closed
petrochenkov opened this issue Aug 30, 2023 · 3 comments · Fixed by #12776
Closed

Support inheriting jobserver when using cargo run #12597

petrochenkov opened this issue Aug 30, 2023 · 3 comments · Fixed by #12776
Labels
A-jobserver Area: jobserver, concurrency, parallelism C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` Command-run S-needs-team-input Status: Needs input from team on whether/how to proceed.

Comments

@petrochenkov
Copy link
Contributor

Problem

Sometimes commands run through cargo run actually perform build actions for a larger build system.

For example in #10511 (comment) the scenario is running something like cargo run -p binding_generator which generates headers to be used during the build later.

And in rust-lang/rust#113730 (comment) cargo-miri is set as target.runner and runs as a part of cargo run (and calls build tools like cargo and rustc recursively).

Proposed Solution

In all cases like these cargo shouldn't close its jobserver file descriptors (or other handles), but pass them to the tool being run instead.

I think doing this under an options would be fine.
The default for such option can be decided upon later (probably kept being false, which is equivalent to the current behavior).

Notes

No response

@petrochenkov petrochenkov added C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` S-triage Status: This issue is waiting on initial triage. labels Aug 30, 2023
@weihanglo weihanglo added E-easy Experience: Easy I-nominated-to-discuss To be discussed during issue triage on the next Cargo team meeting A-jobserver Area: jobserver, concurrency, parallelism S-needs-team-input Status: Needs input from team on whether/how to proceed. and removed S-triage Status: This issue is waiting on initial triage. E-easy Experience: Easy labels Aug 30, 2023
@weihanglo weihanglo removed the I-nominated-to-discuss To be discussed during issue triage on the next Cargo team meeting label Sep 26, 2023
@RalfJung
Copy link
Member

In all cases like these cargo shouldn't close its jobserver file descriptors (or other handles), but pass them to the tool being run instead.

From what I understood in the PR, this does not seem to be what the PR does. The PR only makes it so that if cargo uses an external jobserver, that one is now propagated to cargo run as well.

So maybe the issue should be reopened?

@weihanglo
Copy link
Member

From reading this issue title, and the context of #10511 (comment), I made an assumption that miri (or some other tool alike) would like to inherit existing jobserver from the environment, exactly as how Cargo treats external commands today since #10511. #12776 made it so. It's my fault that didn't request more details from the issue author.

The line between compilation and execution is not always clear in cargo, like --keep-going flag. What is kept going, rustc or the final binary run? 🤯

That said, if this doesn't resolve the miri issue, I am sorry 😞. I would suggest opening another issue to clarify the workflow and expectation, and we could consider a piped-based jobserver by default (see PR description in #12776).

@RalfJung
Copy link
Member

RalfJung commented Jan 22, 2024

Well, it's less of an issue for Miri than a peculiar interaction of how cargo-miri works (a big pile of hacks to get cargo to do what we need it to do) and how cargo works, that was noticed as part of rust-lang/rust#113730. I'm not aware of this causing actual problems for Miri users.

#12776 doesn't change that interaction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-jobserver Area: jobserver, concurrency, parallelism C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` Command-run S-needs-team-input Status: Needs input from team on whether/how to proceed.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants