Support multiple --target flags on the CLI
#8167
Conversation
|
r? @ehuss (rust_highfive has picked a reviewer for you, use r? to override) |
|
Hm, so if I'm interpreting this right, we would expect that e.g. rustbuild would (eventually) start using this to parallelize the std-building on our dist-various builders, right? That would (I imagine) see some pretty good improvement in CI times, especially on the many-core builders we're getting with GHA. I haven't actually read the PR to determine if that's true, though. |
|
@Mark-Simulacrum I'd be pretty surprised if rustbuild could take advantage of this tbh, although it's perhaps not completely out of the question. It would likely take a large-ish refactoring because there'd no longer be a |
There was a problem hiding this comment.
Looks like there are a couple nightly-only tests that need to be fixed.
I noticed that it does not handle multiple --target flags with the same value very well (Cargo hangs acquiring the file lock). Perhaps BuildConfig::requested_kinds should be a set? Or maybe there should be some deduplication in from_requested_targets?
Oops I remember thinking I should write a test and handle that and then I must have gotten distracted before implementing a fix for that... |
8709171 to
25c25ec
Compare
|
Updated! Thanks for the keen eye |
|
☔ The latest upstream changes (presumably #8177) made this pull request unmergeable. Please resolve the merge conflicts. |
This commit refactors the internals of Cargo to no longer have a singular `--target` flag (and singular `requested_target` kind throught) but to instead have a list. The semantics of multiple `--target` flags is to build the selected targets for each of the input `--target` flag inputs. For now this is gated behind `-Zmultitarget` as an unstable features, since I'm not entirely sure this is the interface we want. In general it'd be great if we had a way to simply specify `Unit` structures of what to build on the CLI, but we're in general very far away from that, so I figured that this is probably sufficient at least for testing for now. cc rust-lang#8156
25c25ec to
3fd2814
Compare
|
👍 |
|
📌 Commit 3fd2814 has been approved by |
|
☀️ Test successful - checks-azure |
|
Woah this is unexpectedly efficient. |
Fix doctests not running with --target=HOST. There was a regression in #8167 where `cargo test --target=$HOST` stopped running doctests. This caused doctests to silently stop running in rust-lang/rust (rust-lang/rust#73286). This PR restores the original behavior where `--target=$HOST` behaves as-if it is a normal host test. There was a discussion about this at #8167 (review), but I think I let it slip through the cracks.
This commit fixes a (five year old!) regression in `cargo metadata` where if `--filter-platform` isn't explicitly specified it will accidentally read `$CARGO_BUILD_TARGET` (or `build.target` configuration) and use that as the default `--filter-platform`. The reason for this is that the calculation for targets changed in rust-lang#8167 and while the shared function makes sense for other commands such as `cargo build` the targets have a different meaning in `cargo metadata` so a slightly different set of functionality is desired. This commit fixes the issue by introducing a new constructor for the list of `CompileKind` variants where the fallback of "if nothing is specified" is explicitly chosen.
This commit fixes a (five year old!) regression in `cargo metadata` where if `--filter-platform` isn't explicitly specified it will accidentally read `$CARGO_BUILD_TARGET` (or `build.target` configuration) and use that as the default `--filter-platform`. The reason for this is that the calculation for targets changed in #8167 and while the shared function makes sense for other commands such as `cargo build` the targets have a different meaning in `cargo metadata` so a slightly different set of functionality is desired. This commit fixes the issue by introducing a new constructor for the list of `CompileKind` variants where the fallback of "if nothing is specified" is explicitly chosen. Closes #15270 <!-- Thanks for submitting a pull request 🎉! Here are some tips for you: * If this is your first contribution, read "Cargo Contribution Guide" first: https://doc.crates.io/contrib/ * Run `cargo fmt --all` to format your code changes. * Small commits and pull requests are always preferable and easy to review. * If your idea is large and needs feedback from the community, read how: https://doc.crates.io/contrib/process/#working-on-large-features * Cargo takes care of compatibility. Read our design principles: https://doc.crates.io/contrib/design.html * When changing help text of cargo commands, follow the steps to generate docs: https://github.com/rust-lang/cargo/tree/master/src/doc#building-the-man-pages * If your PR is not finished, set it as "draft" PR or add "WIP" in its title. * It's ok to use the CI resources to test your PR, but please don't abuse them. ### What does this PR try to resolve? Explain the motivation behind this change. A clear overview along with an in-depth explanation are helpful. You can use `Fixes #<issue number>` to associate this PR to an existing issue. ### How should we test and review this PR? Demonstrate how you test this change and guide reviewers through your PR. With a smooth review process, a pull request usually gets reviewed quicker. If you don't know how to write and run your tests, please read the guide: https://doc.crates.io/contrib/tests ### Additional information Other information you want to mention in this PR, such as prior arts, future extensions, an unresolved problem, or a TODO list. -->
This commit refactors the internals of Cargo to no longer have a
singular
--targetflag (and singularrequested_targetkind throught)but to instead have a list. The semantics of multiple
--targetflagsis to build the selected targets for each of the input
--targetflaginputs.
For now this is gated behind
-Zmultitargetas an unstable features,since I'm not entirely sure this is the interface we want. In general
it'd be great if we had a way to simply specify
Unitstructures ofwhat to build on the CLI, but we're in general very far away from that,
so I figured that this is probably sufficient at least for testing for
now.
cc #8156