Stabilize named profiles

[ehuss]

This stabilizes the named profiles feature. As an overview of what this does, it allows specifying custom named profiles, such as:

[profile.release-lto]
inherits = "release"
lto = true

And enables the use of the --profile CLI option to choose a profile by name.

Another key change here is that cargo now only uses a single profile per command. Previously, some commands such as cargo test would use a mix of profiles based on which package and target were being built.

Summary of new behavior

• Profiles can now have arbitrary names. New profiles require the inherits key.
• The --profile flag is now available on all build commands.
• The CompileMode is no longer considered for choosing the profile, only one profile will be used. Previously, building a test, benchmark, or doctest would use the test or bench profile, and all dependencies would use the dev/release profiles. This change is done to arguably make it easier to understand, and to possibly give more desired and intuitive behavior.
• The test profile now inherits settings from the dev profile (and bench from release).

Deviations from the original RFC and implementation

• The original RFC indicated that --all-targets without --profile would retain the old behavior where it would use different profiles for different targets. However, the implementation uses a single profile, to avoid confusion and to keep things simple.
• The dir-name key is not exposed to the user. The implementation is retained to handle mapping of built-in profile names (test/dev→debug, bench→release). This can be exposed in the future if necessary.

Notes about this PR

• Fixed an issue where the duplicate package override check would randomly return matches for inherited profiles like test.
• I left some of the old, vestigial code behind to possibly make it easier to revert this PR if necessary. If this does land, I think it can be eventually removed (code using Feature::named_profiles and various things using named_profiles_enabled).
• Added target to reserved list, just because.
• Adds a warning if --release is combined with --profile in cargo rustc, check, or fix. The --release flag was being ignored.

Hazards and concerns

• This has had very little real-world testing.
• Custom profile directories may conflict with other things in the target directory. We have reserved profile names that currently conflict (such as doc or package). However, they can still collide with target names. This also presents a hazard if Cargo ever wants to add new things to that top directory. We decided to proceed with this because:
  • We currently have no plans to add new built-in profiles.
  • We have reserved several profile names (including anything starting with "cargo"), and the profile name syntax is deliberately limited (so cargo is still free to add . prefixed hidden directories).
  • A user creating a profile that collides with a target name resides in the "don't do that" territory. Also, that shouldn't be catastrophic, as the directories are still somewhat organized differently.
• Artifacts may no longer be shared in some circumstances. This can lead to substantially increased target directory sizes (and build times), particularly if the test profile is not the same as the dev profile. Previously, dependencies would use the dev profile for both. If the user wants to retain the old behavior, they can use an override like [profile.test.package."*"] and set the same settings as dev.
• This may break existing workflows. It is possible, though unlikely, that changes to the profile settings will cause changes to how things build in such a way to break behavior.
  • Another example is using something like cargo build to prime a cache that is used for cargo test, and there is a custom test profile, the cache will no longer be primed.
• The legacy behavior with cargo rustc, cargo check, and cargo fix may be confusing. We may in the future consider something like a --mode flag to formalize that behavior.
• The PROFILE environment variable in build scripts may cause confusion or cause problems since it only sets release or debug. Some people may be using that to determine if --release should be used for a recursive cargo invocation. Currently I noted in the documentation that it shouldn't be used. However, I think it could be reasonable to maybe add a separate environment variable (PROFILE_NAME?) that exposes the actual profile used. We felt that changing the existing value could cause too much breakage (and the mapping of debug→dev is a little awkward).

Closes #6988

[rust-highfive]

r? @Eh2406

(rust-highfive has picked a reviewer for you, use r? to override)

[cbeck88]

The original RFC indicated that --all-targets without --profile would retain the old behavior where it would use different profiles for different targets. However, the implementation uses a single profile, to avoid confusion and to keep things simple.

As a user, this sounds great -- I had no idea that cargo did not use the same profile for the whole build, and it feels like a problematic behavior. For example in a benchmark there may a bench-specific compiler flags that are passed (and these would be set in the bench profile), but then if those flags don't apply to the dependencies then the profiler may not be able to work properly with all the code that is generated.

[alexcrichton]

Personally I feel that the hazards/concerns here are acceptable and we've done our due diligence, thanks @ehuss for putting this all together!

@rfcbot fcp merge

[rfcbot]

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

• @Eh2406
• @alexcrichton
• @ehuss
• @joshtriplett

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]

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

[rfcbot]

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.

This will be merged soon.

[alexcrichton]

🎊
@bors: r+

[bors]

📌 Commit 595384f has been approved by alexcrichton

[bors]

⌛ Testing commit 595384f with merge ec38c84...

[bors]

☀️ Test successful - checks-actions
Approved by: alexcrichton
Pushing ec38c84 to master...

[danielhenrymantilla]

@ehuss what was the reason for removing this neat diagram?

I imagine that with named profiles the things it mentioned might not have been 100% accurate anymore, but I found it to be a nice resource to link to when people where confused by the "shallowness" of --cfg test, for instance. Nowadays I find it hard to lead them to a clear representation of this situation wherein the unit tested lib, or its integration tests, are technically different crates from the original lib, due to this --cfg test difference, whereas the deps just don't get that flag.

I think it would be very nice if a similar (albeit updated as needed) diagram could be added back to the reference 🙂

[ehuss]

Yea, the diagram is no longer accurate in terms of how profiles work. However, it definitely sounds useful to include something similar to show these internal relationships. I'll put it on my list of documentation work, I've been thinking about expanding this somehow.

[danielhenrymantilla]

Great!