cargo
cargo copied to clipboard
Support per pkg target for `-Zbuild-std`
Fixes #9451.
cc @alexcrichton
To review this PR, you can look at the commit summaries and descriptions.
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @ehuss (or someone else) soon.
Please see the contribution instructions for more information.
r? @alexcrichton
:umbrella: The latest upstream changes (presumably #9992) made this pull request unmergeable. Please resolve the merge conflicts.
Coming back to this, I don't believe build-std currently has a problem with proc-macro and buildscript builds confusing host-defined std with custom-built std. These lines:
https://github.com/rust-lang/cargo/blob/c9a8199e927192d4f03f73a14f97faa29c9df956/src/cargo/core/compiler/unit_dependencies.rs#L159-L168
.. already avoid attaching custom std deps for host compile kinds. This means that if a user did not specify the compile kind to use for a specific unit, it will just use the host-defined std. Only when specifying a target, through per-pkg-target or --target is when build-std will build a std for that specific target and attach std to those crates that requested those.
:umbrella: The latest upstream changes (presumably #10434) made this pull request unmergeable. Please resolve the merge conflicts.
Hi! @fee1-dead I'm really looking forward to see this change, we really need it in https://github.com/aya-rs/aya ecosystem. Could you please rebase it?
r? @ehuss @rustbot ready
There seems to be a discussion up at https://github.com/rust-lang/cargo/pull/10330#discussion_r794815070 that doesn't appear to be resolved. I don't think the --target flag requirement can just be removed without major effort. There's a bit more discussion at https://github.com/rust-lang/wg-cargo-std-aware/issues/25.
Also, similar to #10405, I don't think this is a complete solution since per-package targets might come from registry dependencies. Those won't get std built correctly.
@rustbot author
:umbrella: The latest upstream changes (presumably #10129) made this pull request unmergeable. Please resolve the merge conflicts.
Okay so I did some testing and found that no --target basically works the same as --target $HOST. I have updated some tests to make sure that this is indeed true for proc macro crates to use sysroot crates instead of the custom built one.
@rustbot ready
:umbrella: The latest upstream changes (presumably #9892) made this pull request unmergeable. Please resolve the merge conflicts.
@ehuss I am unfamiliar with how artifact dependencies work and I would like this to land. Since these two are both unstable features now, could we note the certain incompatibilities and just merge it for now for further testing?
Hm, I was perhaps not understanding the intent of the per-package-target feature. I did not realize that it doesn't work with dependencies at all. That doesn't match the motivation for the original issue (#7004), so I'm not really clear how per-package-target is intended to be used. If you or others can put some clear use cases in the tracking issue, that would be helpful.
As for moving forward with this, I think that may be possible. There are still issues around resolution, features, and target-info collection. But those aren't new.
Just some general feedback from a quick look:
- The restriction on specifying
--targetcan't be removed at this time. - The use of
package_set.packages()to fetch a list of root packages of interest seems fragile. I'd like to see a more principled way of being explicit that "these are all of the kinds of the root set". - It isn't clear why there is a change related to
--build-plan
Interaction with artifact dependencies can be ruled with requirement for each artifact dep to always have its target specified. Implicit behavior here can always be added later and is unclear now (should it be inherited from dependent/workspace/the package itself???)