RFC: Remove implicit features in a new edition #3491
Conversation
epage
added
the
T-cargo
| ```toml | ||
| [features] | ||
| gif = ["dep:giffy"] | ||
|
|
||
| [dependencies] | ||
| giffy = { version = "0.11.1", optional = true } | ||
| ``` |
There was a problem hiding this comment.
I assume gif = ["dep:gif"] (matching names) also works? if so, maybe this should be explicitly mentioned or even used as the example to avoid people from thinking that this would not be allowed?
There was a problem hiding this comment.
Yes, it does.
I wrote it from the perspective of someone reading it in 5 years where there is no historical baggage around assuming dependencies and features are in the same namespace.
|
I just would like to thank you for pursuing this feature |
|
🦗 ok, let's FCP this @rfcbot fcp merge |
|
Team member @epage 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
added
proposed-final-comment-period
| In addition to `cfg(feature = "gif")` syntax, you can reference this as | ||
| `cfg(feature = "giffy")` in the code though users may get confused at error | ||
| messages when used on a public API. |
There was a problem hiding this comment.
I'm a little confused by this. Is this suggesting that cargo starts setting a --cfg feature=giffy? That's not how it works today (there is no explicit cfg that can be used for the dependency name).
There was a problem hiding this comment.
Ok, I had misunderstood; I had though we were setting a cfg for dependencies. Removed it in fa464d1
| For existing editions, `cargo` will warn when parsing a workspace member's | ||
| package when an optional dependency is not referenced via `dep:` in the | ||
| features ([rust-lang/cargo#9088](https://github.com/rust-lang/cargo/issues/9088)) using the | ||
| planned warning control system ([rust-lang/cargo#12235](https://github.com/rust-lang/cargo/issues/12235)). |
There was a problem hiding this comment.
I think it might be helpful to write down a contingency plan if that warning system is not available before the edition needs to be ready. I don't know what the timeline for either the warning system or the edition looks like, so I'm uncertain if the two can line up. But my suggestion would be to add some text along the lines of:
If the user-controllable warning system isn't implemented in time for the next edition,
cargo fix --editionwill still automatically fixCargo.tomlto migrate it to the next edition (just potentially using a less generalized implementation specific for this RFC).
There was a problem hiding this comment.
Based on our discussion in the team meeting, I re-worked this to focus on the primary end-user affect we are going for (cargo fix --edition) in b1c02ae.
rfcbot
added
the
final-comment-period
|
🔔 This is now entering its final comment period, as per the review above. 🔔 |
rfcbot
removed
the
proposed-final-comment-period
rfcbot
added
finished-final-comment-period
|
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. |
|
Huzzah! The @rust-lang/cargo team has decided to accept this RFC. To track further discussion, subscribe to the tracking issue here: |
Rendered
This would resolve issues like rust-lang/cargo#12173