cargo
cargo copied to clipboard
Change precedence of `build-override`
See the original PR here https://github.com/rust-lang/cargo/pull/9382
If you specify both
build-overrideandpackage."*"then the glob profile has higher precedence than thebuild-overrideone which is much more specific. In practice this means that very often thebuild-overrideis just completely ignored and all the proc macro crates get compiled without thebuild-overrideactually doing anything. This usually results indebugbuilds taking much longer than they should, often even longer thanreleasebuilds. This Pull Request fixes this precedence issue.
This includes those changes along with the necessary documentation changes. Additionally, the build-override defaults (and not just overrides supplied by the user) are now applied before package."*" overrides.
Fixes #9351
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.
Squashed :slightly_smiling_face:
@bors r+
:pushpin: Commit ede065f5bbbf6dac4a520469949ec0e97a7d2d92 has been approved by weihanglo
:hourglass: Testing commit ede065f5bbbf6dac4a520469949ec0e97a7d2d92 with merge d012044dadf58209d47b1aa93581ed0dadb9dc91...
@bors r-
:broken_heart: Test failed - checks-actions
@rfcbot merge
It makes sense to me as a more specific overrides taking higher precedence than glob one. Though this changes the old behaviour, need the team to take a look.
Team member @weihanglo has proposed to merge this. The next step is review by the rest of the tagged team members:
- [x] @Eh2406
- [ ] @ehuss
- [ ] @epage
- [ ] @joshtriplett
- [x] @weihanglo
Concerns:
- future-globs (https://github.com/rust-lang/cargo/pull/10787#issuecomment-1170646225)
- intuition (https://github.com/rust-lang/cargo/pull/10787#issuecomment-1170647445)
Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!
See this document for info about what commands tagged team members can give me.
@rfcbot concern future-globs
This changes the precedence from
- Profiles in
.cargo/configfiles (using same order as below). profile.dev.package.<name>profile.dev.package.*profile.dev.build-overrideprofile.dev
to
- Profiles in
.cargo/configfiles (using same order as below). profile.dev.package.<name>profile.dev.build-overrideprofile.dev.package.*profile.dev
With the original, it can really be simplified to
- Profiles in
.cargo/configfiles (using same order as below). profile.dev.package.<glob>profile.dev.build-overrideprofile.dev
Where our glob syntax only allows * without anything before or after.
What is the likelihood we'd expand our glob syntax? What would this change look like after expanding our glob syntax? Would we want literals to come first, then build-override then any glob? Or would we want all non naked * globs to come before `build-override? Or would we not want this change after expanding the glob syntax?
@rfcbot concern intuition
When writing up the previous concern, I realized that there is also an intuition problem. While the original issue said that * is expected to be lower than build-override within a specific profile configuration, I suspect users would not expect <name> to be at a different precedence level than * and that this would lead to confusion. While we are documenting the behavior, that is likely to be later in the development cycle and more frustrating.
This might be a bit of a sidetrack, but IMO it is already a bit unintuitive that workspace members are excluded from *.
What is the likelihood we'd expand our glob syntax?
With crates having arbitrary names I find it difficult to imagine a more complex glob being reliable. Maybe if it was limited to workspace members? Some way to configure multiple crates at once seems like it would be useful (unless profile.dev.package.<name> already has a way to list crates that I'm not aware of).
I suspect users would not expect
<name>to be at a different precedence level than*and that this would lead to confusion. While we are documenting the behavior, that is likely to be later in the development cycle and more frustrating.
This makes me realize the current changes would still leave no way to specifically configure <name> as a regular dependency without also affecting its configuration when used as a build dependency. But if build-override precedence is simply pushed up further then there is no way to specifically configure particular build dependencies. :thinking:
I wasn't aware that .cargo/config can provide a higher precedence. That seems like a decent (albeit hacky) way to work around the problem these changes are trying to address.
With crates having arbitrary names I find it difficult to imagine a more complex glob being reliable.
Brainstorming hat: Besides any *derive*, https://github.com/rust-lang/rfcs/pull/3243# might provide opportunities for more structured naming for globbing.
This makes me realize the current changes would still leave no way to specifically configure
<name>as a regular dependency without also affecting its configuration when used as a build dependency. But ifbuild-overrideprecedence is simply pushed up further then there is no way to specifically configure particular build dependencies.
It seems like we would need a whole separate .build-override.package.
:sunny: Try build successful - checks-actions
Build commit: d012044dadf58209d47b1aa93581ed0dadb9dc91 (d012044dadf58209d47b1aa93581ed0dadb9dc91)
With the concerns pointed out by @epage, I'm not really happy with the current solution.
For my own purposes, it seems like in the short term it would be better to just target specific build-time crates with configuration and/or see if I can use .cargo/config profiles to get a higher precedence.
What would the steps be to move forward with designing an alternative? (happy to discuss further on zulip if that would be ideal)
The initial idea that comes to mind is being able to specify build-override."*" and build-override.<name>. With higher precedence than any of the package. sections. But that might be a bit too ad hoc and would yield two options with the same effect: build-override."*" and just build-override.
@rfcbot close
Thanks @Imberflur for your efforts. We're still seeking a solution to #9351.
@rfcbot cancel
@weihanglo proposal cancelled.
The Cargo team doesn't have a bandwidth to lead this change by ourselves. That is to say, if @Imberflur or someone wants things to move forward, you must be the driver for the design! We're still open to discuss on either Zulip or GitHub issue (#9351).
Closing for now. Thank you!