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

Stabilize profile-overrides. #7591

Merged
merged 1 commit into from
Dec 2, 2019

Conversation

ehuss
Copy link
Contributor

@ehuss ehuss commented Nov 15, 2019

This stabilizes the profile-overrides feature. This was proposed in RFC 2282 and implemented in #5384. Tracking issue is rust-lang/rust#48683.

This is intended to land in 1.41 which will reach the stable channel on Jan 30th.

This includes a new documentation chapter on profiles. See the "Overrides" section in profiles.md file for details on what is being stabilized.

Note: The config-profile and named-profiles features are still unstable.

Closes #6214

Concerns

  • There is some risk that build-override may be confusing with the proposed future dedicated build profile. There is some more discussion about the differences at Tracking issue for RFC 2282 - Cargo profile dependencies rust#48683 (comment). I don't expect it to be a significant drawback. If we proceed later with a dedicated build profile, I think we can handle explaining the differences in the documentation. (The linked PR is designed to work with profile-overrides.)
  • I don't anticipate any unexpected interactions with config-profiles or named-profiles.
  • Some of the syntax like "*" or build-override may not be immediately obvious what it means without reading the documentation. Nobody suggested any alternatives, though.
  • Configuring overrides for multiple packages is currently a pain, as you have to repeat the settings separately for each package. I propose that we can extend the syntax in the future to allow a comma-separated list of package names to alleviate this concern if it is deemed worthwhile.
  • The user may not know which packages to override, particularly for some dependencies that are split across multiple crates. I think, since this is an advanced feature, the user will likely be comfortable with using things like cargo tree to understand what needs to be overridden. There is some discussion in the tracking issue about automatically including a package's dependencies, but this is somewhat complex.
  • There is some possibly confusing interaction with the test/bench profile. Because dependencies are always built with the dev/release profiles, overridding test/bench usually does not have an effect (unless specifying a workspace member that is being tested/benched). Overriding test/bench was previously prohibited, but was relaxed when named profiles were added.
  • We may want to allow overriding the panic, lto, and rpath settings in the future. I can imagine a case where someone has a large workspace, and wants to override those settings for just one package in the workspace. They are currently not allowed because it doesn't make sense to change those settings for rlibs, and panic usually needs to be in sync for the whole profile.
  • There are some strange interactions with dylib crates detailed in Symbol names may not correctly account for optimization level differences between crates rust#64319. A fix was attempted, but later reverted. Since dylib crates are rare (this mostly applied to libstd), and a workaround was implemented for build-std (it no longer builds a dylib), I'm not too worried about this.
  • The interaction with share-generics can be quite confusing (see Using profile-overrides results in both optimized and unoptimized versions of the same crate in linked executable  rust#63484). I added a section in the docs that tries to address this concern. It's also possible future versions of rustc may handle this better.
  • The new documentation duplicates some of the information in the rustc book. I think it's fine, as there are subtle differences, and it avoids needing to flip back and forth between the two books to learn what the settings do.

@ehuss ehuss added the T-cargo Team: Cargo label Nov 15, 2019
@rust-highfive
Copy link

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 15, 2019
@ehuss
Copy link
Contributor Author

ehuss commented Nov 15, 2019

@rfcbot fcp merge

@rfcbot
Copy link
Collaborator

rfcbot commented Nov 15, 2019

Team member @ehuss has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

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 rfcbot added proposed-final-comment-period An FCP proposal has started, but not yet signed off. disposition-merge FCP with intent to merge labels Nov 15, 2019
@bors
Copy link
Collaborator

bors commented Nov 16, 2019

☔ The latest upstream changes (presumably #7595) made this pull request unmergeable. Please resolve the merge conflicts.

@ehuss ehuss force-pushed the stabilize-profile-overrides branch from 9c993a9 to dda81d3 Compare November 16, 2019 01:14
@rfcbot rfcbot added final-comment-period FCP — a period for last comments before action is taken and removed proposed-final-comment-period An FCP proposal has started, but not yet signed off. labels Nov 18, 2019
@rfcbot
Copy link
Collaborator

rfcbot commented Nov 18, 2019

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot added finished-final-comment-period FCP complete and removed final-comment-period FCP — a period for last comments before action is taken labels Nov 28, 2019
@rfcbot
Copy link
Collaborator

rfcbot commented Nov 28, 2019

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

The RFC will be merged soon.

@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Collaborator

bors commented Dec 2, 2019

📌 Commit dda81d3 has been approved by alexcrichton

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 2, 2019

Overrides cannot specify the `panic`, `lto`, or `rpath` settings.

#### Overrides and generics
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alexcrichton Just wanted to check if you had a chance to read this section. I wasn't confident it is correct or clear or goes into too much or too little detail. Particularly the last paragraph.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heh good catch!

Looks accurate to me though. The only thing I might recommend is a "if you've got questions and/or are confused about this, please let us know" style comment. Basically I just want to make sure folks know they can contact us if they're weirded out.

@bors
Copy link
Collaborator

bors commented Dec 2, 2019

⌛ Testing commit dda81d3 with merge b35bb1b...

bors added a commit that referenced this pull request Dec 2, 2019
Stabilize profile-overrides.

This stabilizes the profile-overrides feature. This was proposed in [RFC 2282](rust-lang/rfcs#2282) and implemented in #5384. Tracking issue is rust-lang/rust#48683.

This is intended to land in 1.41 which will reach the stable channel on Jan 30th.

This includes a new documentation chapter on profiles. See the ["Overrides" section](https://github.com/rust-lang/cargo/blob/9c993a92ce33f219aaaed963bef51fc0f6a7677a/src/doc/src/reference/profiles.md#overrides) in `profiles.md` file for details on what is being stabilized.

Note: The `config-profile` and `named-profiles` features are still unstable.

Closes #6214

**Concerns**
- There is some risk that `build-override` may be confusing with the [proposed future dedicated `build` profile](#6577). There is some more discussion about the differences at rust-lang/rust#48683 (comment). I don't expect it to be a significant drawback. If we proceed later with a dedicated `build` profile, I think we can handle explaining the differences in the documentation. (The linked PR is designed to work with profile-overrides.)
- I don't anticipate any unexpected interactions with `config-profiles` or `named-profiles`.
- Some of the syntax like `"*"` or `build-override` may not be immediately obvious what it means without reading the documentation. Nobody suggested any alternatives, though.
- Configuring overrides for multiple packages is currently a pain, as you have to repeat the settings separately for each package. I propose that we can extend the syntax in the future to allow a comma-separated list of package names to alleviate this concern if it is deemed worthwhile.
- The user may not know which packages to override, particularly for some dependencies that are split across multiple crates. I think, since this is an advanced feature, the user will likely be comfortable with using things like `cargo tree` to understand what needs to be overridden. There is [some discussion](rust-lang/rust#48683 (comment)) in the tracking issue about automatically including a package's dependencies, but this is somewhat complex.
- There is some possibly confusing interaction with the test/bench profile. Because dependencies are always built with the dev/release profiles, overridding test/bench usually does not have an effect (unless specifying a workspace member that is being tested/benched). Overriding test/bench was previously prohibited, but was relaxed when named profiles were added.
- We may want to allow overriding the `panic`, `lto`, and `rpath` settings in the future. I can imagine a case where someone has a large workspace, and wants to override those settings for just one package in the workspace. They are currently not allowed because it doesn't make sense to change those settings for rlibs, and `panic` usually needs to be in sync for the whole profile.
- There are some strange interactions with `dylib` crates detailed in rust-lang/rust#64319. A fix was attempted, but later reverted. Since `dylib` crates are rare (this mostly applied to `libstd`), and a workaround was implemented for `build-std` (it no longer builds a dylib), I'm not too worried about this.
- The interaction with `share-generics` can be quite confusing (see rust-lang/rust#63484). I added a section in the docs that tries to address this concern. It's also possible future versions of `rustc` may handle this better.
- The new documentation duplicates some of the information in the rustc book.  I think it's fine, as there are subtle differences, and it avoids needing to flip back and forth between the two books to learn what the settings do.
@bors
Copy link
Collaborator

bors commented Dec 2, 2019

☀️ Test successful - checks-azure
Approved by: alexcrichton
Pushing b35bb1b to master...

@bors bors merged commit dda81d3 into rust-lang:master Dec 2, 2019
bors added a commit to rust-lang/rust that referenced this pull request Dec 4, 2019
Update cargo

11 commits in 750cb1482e4d0e74822cded7ab8b3c677ed8b041..626f0f40efd32e6b3dbade50cd53fdfaa08446ba
2019-11-23 23:06:36 +0000 to 2019-12-03 16:53:04 +0000
- Change some texts to links in README (rust-lang/cargo#7652)
- Update config and environment variable docs. (rust-lang/cargo#7650)
- Stop ignoring .rs.bk files; rustfmt hasn't generated them in years (rust-lang/cargo#7647)
- Various contributing docs updates. (rust-lang/cargo#7642)
- Stabilize profile-overrides. (rust-lang/cargo#7591)
- Update comment about ResolveVersion default version. (rust-lang/cargo#7637)
- Update tests for slight wording change in rustdoc error message. (rust-lang/cargo#7641)
- Remove dep_targets. (rust-lang/cargo#7626)
- vendor: don't use canonical path in .cargo/config (rust-lang/cargo#7629)
- Minor testsuite organization. (rust-lang/cargo#7628)
- Remove failing plugin tests. (rust-lang/cargo#7630)
@jhpratt
Copy link
Member

jhpratt commented Jan 31, 2020

Question: if I have a crate that supports versions before 1.41, this is perfectly safe to use, yes? As in, it'll be ignored on 1.40 and before, but do the correct thing from now on?

@est31
Copy link
Member

est31 commented Jan 31, 2020

@jhpratt on really old cargo versions before the feature was implemented, there won't be an error. On versions like 1.40 where the feature was unstable, there will be an error unfortunately. See this link for details.

@jhpratt
Copy link
Member

jhpratt commented Jan 31, 2020

Welp, worth asking. Was hoping to provide a slight compile time improvement to the time crate, given certain build-time functionality.

Let's hope that can happen at some point! 🙂

netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Feb 17, 2020
Version 1.41.0 (2020-01-30)
===========================

Language
--------

- [You can now pass type parameters to foreign items when implementing
  traits.][65879] E.g. You can now write `impl<T> From<Foo> for Vec<T> {}`.
- [You can now arbitrarily nest receiver types in the `self` position.][64325] E.g. you can
  now write `fn foo(self: Box<Box<Self>>) {}`. Previously only `Self`, `&Self`,
  `&mut Self`, `Arc<Self>`, `Rc<Self>`, and `Box<Self>` were allowed.
- [You can now use any valid identifier in a `format_args` macro.][66847]
  Previously identifiers starting with an underscore were not allowed.
- [Visibility modifiers (e.g. `pub`) are now syntactically allowed on trait items and
  enum variants.][66183] These are still rejected semantically, but
  can be seen and parsed by procedural macros and conditional compilation.

Compiler
--------

- [Rustc will now warn if you have unused loop `'label`s.][66325]
- [Removed support for the `i686-unknown-dragonfly` target.][67255]
- [Added tier 3 support\* for the `riscv64gc-unknown-linux-gnu` target.][66661]
- [You can now pass an arguments file passing the `@path` syntax
  to rustc.][66172] Note that the format differs somewhat from what is
  found in other tooling; please see [the documentation][argfile-docs] for
  more information.
- [You can now provide `--extern` flag without a path, indicating that it is
  available from the search path or specified with an `-L` flag.][64882]

\* Refer to Rust's [platform support page][forge-platform-support] for more
information on Rust's tiered platform support.

[argfile-docs]: https://doc.rust-lang.org/nightly/rustc/command-line-arguments.html#path-load-command-line-flags-from-a-path

Libraries
---------

- [The `core::panic` module is now stable.][66771] It was already stable
  through `std`.
- [`NonZero*` numerics now implement `From<NonZero*>` if it's a smaller integer
  width.][66277] E.g. `NonZeroU16` now implements `From<NonZeroU8>`.
- [`MaybeUninit<T>` now implements `fmt::Debug`.][65013]

Stabilized APIs
---------------

- [`Result::map_or`]
- [`Result::map_or_else`]
- [`std::rc::Weak::weak_count`]
- [`std::rc::Weak::strong_count`]
- [`std::sync::Weak::weak_count`]
- [`std::sync::Weak::strong_count`]

Cargo
-----

- [Cargo will now document all the private items for binary crates
  by default.][cargo/7593]
- [`cargo-install` will now reinstall the package if it detects that it is out
  of date.][cargo/7560]
- [Cargo.lock now uses a more git friendly format that should help to reduce
  merge conflicts.][cargo/7579]
- [You can now override specific dependencies's build settings][cargo/7591] E.g.
  `[profile.dev.overrides.image] opt-level = 2` sets the `image` crate's
  optimisation level to `2` for debug builds. You can also use
  `[profile.<profile>.build_overrides]` to override build scripts and
  their dependencies.

Misc
----

- [You can now specify `edition` in documentation code blocks to compile the block
  for that edition.][66238] E.g. `edition2018` tells rustdoc that the code sample
  should be compiled the 2018 edition of Rust.
- [You can now provide custom themes to rustdoc with `--theme`, and check the
  current theme with `--check-theme`.][54733]
- [You can use `#[cfg(doc)]` to compile an item when building documentation.][61351]

Compatibility Notes
-------------------

- [As previously announced 1.41.0 will be the last tier 1 release for 32-bit
  Apple targets.][apple-32bit-drop] This means that the source code is still
  available to build, but the targets are no longer being tested and release
  binaries for those platforms will no longer be distributed by the Rust project.
  Please refer to the linked blog post for more information.

[54733]: rust-lang/rust#54733
[61351]: rust-lang/rust#61351
[67255]: rust-lang/rust#67255
[66661]: rust-lang/rust#66661
[66771]: rust-lang/rust#66771
[66847]: rust-lang/rust#66847
[66238]: rust-lang/rust#66238
[66277]: rust-lang/rust#66277
[66325]: rust-lang/rust#66325
[66172]: rust-lang/rust#66172
[66183]: rust-lang/rust#66183
[65879]: rust-lang/rust#65879
[65013]: rust-lang/rust#65013
[64882]: rust-lang/rust#64882
[64325]: rust-lang/rust#64325
[cargo/7560]: rust-lang/cargo#7560
[cargo/7579]: rust-lang/cargo#7579
[cargo/7591]: rust-lang/cargo#7591
[cargo/7593]: rust-lang/cargo#7593
[`Result::map_or_else`]: https://doc.rust-lang.org/std/result/enum.Result.html#method.map_or_else
[`Result::map_or`]: https://doc.rust-lang.org/std/result/enum.Result.html#method.map_or
[`std::rc::Weak::weak_count`]: https://doc.rust-lang.org/std/rc/struct.Weak.html#method.weak_count
[`std::rc::Weak::strong_count`]: https://doc.rust-lang.org/std/rc/struct.Weak.html#method.strong_count
[`std::sync::Weak::weak_count`]: https://doc.rust-lang.org/std/sync/struct.Weak.html#method.weak_count
[`std::sync::Weak::strong_count`]: https://doc.rust-lang.org/std/sync/struct.Weak.html#method.strong_count
[apple-32bit-drop]: https://blog.rust-lang.org/2020/01/03/reducing-support-for-32-bit-apple-targets.html
bors bot added a commit to rust-embedded/book that referenced this pull request Mar 4, 2020
230: Updated documentation on profile-overrides r=therealprof a=irwineffect

Profile-overrides has now been stabilized (rust-lang/cargo#7591), so the book has been updated to point to the stable documentation.

During stabilization, syntax was changed to use "package" instead of "overrides" (rust-lang/cargo#7504), documentation updated to match.

Co-authored-by: James Irwin <irwineffect@gmail.com>
richardeoin added a commit to richardeoin/cortex-m-rtic that referenced this pull request Jun 18, 2020
profile-overrides landed in 1.41 rust-lang/cargo#7591

I appreciate that the intention may be to maintain support for earlier rust versions if the profile-overrides keys are deleted from the README. But if the crate throws an error when compiled with its MSRV compiler, that in my opinion defeats the purpose of the MSRV.
@ehuss ehuss added this to the 1.41.0 milestone Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
disposition-merge FCP with intent to merge finished-final-comment-period FCP complete S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-cargo Team: Cargo
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Docs are missing possible values for LTO
7 participants