cargo icon indicating copy to clipboard operation
cargo copied to clipboard

Tracking Issue for feature-metadata

Open epage opened this issue 1 year ago • 2 comments

Summary

RFC: #3416 Implementation: #NNNN Documentation: https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#feature-metadata

Extend the schema for feature activations to be a table, not just an array, so more fields can be added in the future.

Unresolved Issues

No response

Future Extensions

  • https://github.com/rust-lang/rfcs/pull/3485
  • https://github.com/rust-lang/rfcs/pull/3486
  • https://github.com/rust-lang/rfcs/pull/3487

About tracking issues

Tracking issues are used to record the overall progress of implementation. They are also used as hubs connecting to other relevant issues, e.g., bugs or open design questions. A tracking issue is however not meant for large scale discussion, questions, or bug reports about a feature. Instead, open a dedicated issue for the specific matter and add the relevant feature gate label.

epage avatar Jun 27 '24 02:06 epage

The docs link u posted (https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#feature-metadata) seems obsolete since there is no section called feature-metadata in that page

amab8901 avatar Aug 17 '24 19:08 amab8901

@amab8901 it is just a placeholder because it hasn't yet implemented

weihanglo avatar Aug 17 '24 19:08 weihanglo

I think I found the place in the cargo source code that needs to be changed to implement this feature: image

The Vec<String> should probably be replaced with BTreeMap<String,String>. At least that's my first thought. But then I realize it will prob break the old syntax. So now I'm thinking maybe we need some kind of generic? Or perhaps some kind of match statement to deal with Vec vs BTreeMap separately?

amab8901 avatar Aug 24 '24 19:08 amab8901

That section of code is an internal data structure to track cargo adds command-line. Cargo.tomls definition is in crates/cargo-util-schemas. An untagged enum would need to be used, much like we have "simple" and "detailed" dependencies. All of this would need to be behind an unstable feature as mentioned behind our contributor guide. We'd also need to make sure that people not using this syntax aren't switched to it on cargo publish so we don't force everyone to upgrade cargo.

With all that said, I'm unsure how much benefit there is on moving this forward until we have fields to take advantage of it.

epage avatar Aug 25 '24 01:08 epage

do u have an RFC for the fields that we are waiting for?

amab8901 avatar Aug 25 '24 10:08 amab8901

See future extensions in the PR description.

weihanglo avatar Aug 25 '24 13:08 weihanglo

My understanding is that you first need to solve this RFC before you can add those fields. Otherwise, when you try to add those fields, the compiler will complain and say "this is invalid syntax in Cargo.toml !"

amab8901 avatar Aug 25 '24 14:08 amab8901

If you are looking to put up a PR, I think the thing that could be changed now is to add a new unstable feature that allows parsing the alternate syntax for feature dependencies:

[features]
foo = []
bar = ["foo"]
# This should be the same as the above
bar = { enables = ["foo"] }

This was accepted with RFC3416. The other attributes (e.g. doc) can't yet be added since they haven't yet been accepted, but it will be easy to expand to them once this groundwork is in.

tgross35 avatar Aug 25 '24 19:08 tgross35

This was accepted with RFC3416. The other attributes (e.g. doc) can't yet be added since they haven't yet been accepted, but it will be easy to expand to them once this groundwork is in.

Note that I said

With all that said, I'm unsure how much benefit there is on moving this forward until we have fields to take advantage of it.

The important thing for the other RFCs was metadata RFC being approved.

epage avatar Aug 26 '24 14:08 epage