feat(manifest)!: implement feature-metadata RFC3416 by AudaciousAxiom · Pull Request #15056 · rust-lang/cargo

AudaciousAxiom · 2025-01-12T17:43:16Z

What does this PR try to resolve?

This PR implements RFC3416: feature-metadata.
It introduces an alternate syntax to define crate features in manifests: instead of simply being an array of features the feature enables, it can now be a table with, for now, one required array key, enables, which is equivalent to the existing array. This lays the groundwork for later supporting additional keys in that table, to provide feature metadata.

Related to #14157

How should we test and review this PR?

Integrations tests are included in this PR, including for the normalized manifest used for publishing on a crate registry.

Additional information

The JSON schema of the manifest was updated using:

cargo test -p cargo-util-schemas --features unstable-schema -- 'dump_manifest_schema'

Questions

Should there be a Cargo unstable feature for this change?
This PR of course introduces a breaking change in the cargo-util-schemas crate, is there anything to do regarding this in this PR?
Should this PR also attempt to update core::Summary or should this be left to future implementations of RFCs providing other keys (e.g., doc)?

rustbot · 2025-01-12T17:43:22Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @epage (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

@rustbot author: the review is finished, PR author should check the comments and take action accordingly
@rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

AudaciousAxiom · 2025-01-12T17:44:35Z

+
+impl FeatureDefinition {
+    /// Returns the features that this feature enables.
+    pub fn enables(&self) -> &[String] {


This method could also return an impl Iterator<Item = String> if preferred.

AudaciousAxiom · 2025-01-12T17:47:39Z

+            let feature_array = feature_deps
+                .enables()
+                .iter()
+                .filter(|feature_dep| {
+                    let feature_value = FeatureValue::new(InternedString::new(feature_dep));
+                    match feature_value {
+                        FeatureValue::Dep { dep_name }
+                        | FeatureValue::DepFeature { dep_name, .. } => {
+                            let k = &manifest::PackageName::new(dep_name.to_string()).unwrap();
+                            dep_name_set.contains(k)
+                        }
+                        _ => true,
                    }
-                    _ => true,
-                }
-            });
+                })
+                .cloned()
+                .collect();
+            *feature_deps = FeatureDefinition::Array(feature_array);


For compatibility, this uses the array syntax for generating the normalized manifest, even when the table syntax is used by authors (see the corresponding integration test).

weihanglo · 2025-01-12T18:42:45Z

+    for (feature, feature_definition) in features {
+        match feature_definition {
+            FeatureDefinition::Array(..) => {}
+            FeatureDefinition::Metadata(FeatureMetadata { _unused_keys, .. }) => {


haven't got time into full review, though I think the meta field should be behind a nightly feature flag.

Should there be a Cargo unstable feature for this change?

Yes. Probably behind a cargo-feature "feature-metadata".

This PR of course introduces a breaking change in the cargo-util-schemas crate, is there anything to do regarding this in this PR?

Could add a doc comment on relevant field/variant indicating it is unstable/nightly only.

cargo/crates/cargo-util-schemas/src/manifest/mod.rs

Lines 936 to 937 in f15df8f

/// Unstable feature `-Ztrim-paths`.

pub trim_paths: Option<TomlTrimPaths>,

There is a CI job checking if a member crate needs a version bump. It didn't warn you so I assume it has already been bumped in this release cycle. You do not need to do anything.

Should this PR also attempt to update core::Summary or should this be left to future implementations of RFCs providing other keys (e.g., doc)?

Summary is more like a thing for dependency resolution. I think we revisit it in the future. Regardless, see epage's comment #14157 (comment) that the feature itself is not particularly useful until other RFC gets merged. Anyway, thanks for the contribution!

epage · 2025-01-13T12:57:46Z

What is your motivation for moving this forward?

In #14157 (comment) I was wondering if this was worth it without a feature depending on it.

AudaciousAxiom · 2025-01-13T21:50:20Z

What is your motivation for moving this forward?

I have recently published featurecomb, a crate that allows to define relations between Cargo features in manifests (e.g., mutual exclusion, or a feature requiring another one to be enabled), and to enforce these relations through a proc-macro that reads the manifest and generates #[cfg]-gated compile_error! statements.

I wanted to see how much of this could be introduced into Cargo (I am aware of other existing effort in that direction), and the first step for that was to tackle feature-metadata.
(The most challenging open question I think would be that enforcement cannot be done through the manifest only, even if fully implemented in Cargo, at the very least because of course Cargo is not mandatory for compiling Rust code and it would therefore not be possible to rely on these relations being actually enforced without a dedicated attribute/macro in the source code itself, as is the case with featurecomb.)

I have introduced a feature-metadata unstable feature which can be dynamically enabled through cargo-features. However, I realize now that the same cannot be done for the cargo-util-schemas crate (we can only document the FeatureDefinition::Metadata variant as unstable). So, unless there is a way to dynamically feature-gate the change in the schema, and to not cause unnecessary churn to the dependents of cargo-util-schemas, I agree it may be best not to move forward on this PR for now, until other keys are introduced. Future implementations of these could hopefully use this PR as a base.

epage · 2025-01-13T21:55:28Z

I wanted to see how much of this could be introduced into Cargo (I am aware of other existing effort in that direction), and the first step for that was to tackle feature-metadata.

Interesting effort. Some cautions though

Trying to add mutually exclusive support for the existing feature system is likely a dead end because features must be additive
This would created "unused manifest fields" which are not subject to our compatibility guarantees. I'd recommend finding a different way to declare the relevant information

I have introduced a feature-metadata unstable feature which can be dynamically enabled through cargo-features. However, I realize now that the same cannot be done for the cargo-util-schemas crate (we can only document the FeatureDefinition::Metadata variant as unstable). So, unless there is a way to dynamically feature-gate the change in the schema, and to not cause unnecessary churn to the dependents of cargo-util-schemas, I agree it may be best not to move forward on this PR for now, until other keys are introduced. Future implementations of these could hopefully use this PR as a base.

cargo-util-schemas does not get feature gated. We do that in cargo/utils/toml/mod.rs or later.

syphar · 2026-08-27T01:37:07Z

I want to invest some time to drive progress on the whole "feature documentation" topic,

And I see we already have some code :)

@AudaciousAxiom it looks like this PR stalled, is that something I can help with or even take over?

rustbot · 2026-08-27T20:02:23Z

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

AudaciousAxiom · 2026-08-27T20:29:55Z

I've rebased and squashed the existing fixup commits; the new fixup commit is for the differences in tests, which are just the slightly changed Cargo output.

@syphar I haven't been through the previous PR comments yet; feel free to point out any comment that still needs addressing or new changes needed due to the modifications of the codebase in the meantime.

weihanglo

Thanks for picking it up! Looks generally good. Feel free to reorganize and rebase your commit history.

View changes since this review

weihanglo · 2026-08-29T20:10:30Z

+...
+
+...
+
+...


why multiple multiline wildcards?

I think I was under the impression that might otherwise result in some false negatives; I've replaced them with a single wildcard.

AudaciousAxiom · 2026-08-30T08:35:27Z

@weihanglo Thanks for the review! I've fixed the commit history, moved the test commit after the implementation commit as I see you want each commit to pass the tests, and added the changes from the review as fixup commits. I'll simply autosquash before merging.

weihanglo · 2026-08-30T12:35:51Z

@weihanglo Thanks for the review! I've fixed the commit history, moved the test commit after the implementation commit as I see you want each commit to pass the tests, and added the changes from the review as fixup commits. I'll simply autosquash before merging.

Thanks, though rebasing is easier for us to review. Under most circumstances, we don't need contributors to keep the edit history.

Also, the paragraph you quote actually prefers test commits placed before fix commits, so that the git diff can show the behavior change. See those examples in that note, or some newers examples like #17356 and #17337

weihanglo

Looks good.

Wonder whether you are open to do git cleanup, or I can edit history and do that for you before merging.

View changes since this review

weihanglo · 2026-08-30T12:46:28Z

+```
+
+This is equivalent to the array-of-strings syntax.
+Support for other keys should be added later.


Look at other unstable for example min-publish-age, we add some doc examples that is meant to be copied verbatim when stabilization.

Above just FYI, docs updates don't block this PR merge.

View changes since the review

Thanks for pointing that out; I didn't include this in this round to not further delay the merge if you want it now.

I've opened #17446 to update the documentation when this eventually stabilizes; I don't think this can be really be added as a doc snippet to the unstable feature docs as this is not purely additive.

We can't really merge #17446 now but when stabilizing.
Thanks it for doing it promptly :)

AudaciousAxiom · 2026-08-30T13:47:41Z

Thanks, though rebasing is easier for us to review. Under most circumstances, we don't need contributors to keep the edit history.

Thanks, I'll keep that in mind, this really depends on the project.

Also, the paragraph you quote actually prefers test commits placed before fix commits, so that the git diff can show the behavior change. See those examples in that note, or some newers examples like #17356 and #17337

I did look, but all of these are bug fixes; it makes sense to me to first acknowledge the bug with tests, and then fix it and update the tests atomically, but I wasn't sure what to do for this PR, as this is a purely additive feature; adding the tests first wouldn't allow them to pass.

Wonder whether you are open to do git cleanup, or I can edit history and do that for you before merging.

I've autosquashed the fixup commits; feel free to further edit the commits as you want.

weihanglo · 2026-08-31T00:42:24Z

I did look, but all of these are bug fixes; it makes sense to me to first acknowledge the bug with tests, and then fix it and update the tests atomically, but I wasn't sure what to do for this PR, as this is a purely additive feature; adding the tests first wouldn't allow them to pass.

I tweaked a bit the tests. Thanks for the contribution anyway!

Update cargo submodule 26 commits in e8cb624d5701824f46a2ec5873cfd59ee3d2f66c..b2e9d5f9db3fb1c454ab84f10c16508984a266e2 2026-08-22 00:23:45 +0000 to 2026-09-02 14:49:16 +0000 - fix(parser): Resolve theoretical use-after-free (rust-lang/cargo#17428) - fix(trim-paths)!: remove default scope from release profile (rust-lang/cargo#17424) - fix(git): Use git's 429 retry, when available (rust-lang/cargo#17422) - Avoid passing search path (-L) args when they are passed as --extern (rust-lang/cargo#17410) - chore(deps): update crate-ci/typos action to v1.50.0 (rust-lang/cargo#17417) - test: Move -Z onto its own line (rust-lang/cargo#17416) - chore(triagebot): enable `@rustbot merge/delegate` (rust-lang/cargo#17415) - Micro-optimize two package dir functions (rust-lang/cargo#17413) - perf: Do not build SBOM if user has not set build.sbom (rust-lang/cargo#17412) - feat(manifest)!: implement feature-metadata RFC3416 (rust-lang/cargo#15056) - Cargo profiling improvements (rust-lang/cargo#17411) - test(git): Remove gix override run in CI and the mode in code (rust-lang/cargo#17405) - perf(git): Reduce extra work when using git-cli (rust-lang/cargo#17406) - feat(resolver): Stabilize min-publish-age (rust-lang/cargo#17335) - fix(git): Remove ref status update when showing progress (rust-lang/cargo#17400) - revert: refactor: move sysroot lookup to GlobalContext (rust-lang/cargo#17401) - fix(run): Printing a new line to avoid overwriting error code after \r (rust-lang/cargo#17373) - fix(trim-paths): custom workspace-relative member paths remap (rust-lang/cargo#17366) - fix(home): rustdoc lint (rust-lang/cargo#17394) - feat(diag): Stabilize cargo-lints (rust-lang/cargo#17298) - chore(deps): Update partial_ref to v0.3.4 (rust-lang/cargo#17392) - refactor: remove ad-hoc `subslice_range` (rust-lang/cargo#17390) - docs(changelog): move build-dir new layout to Changed (rust-lang/cargo#17387) - chore(deps): update msrv (1 version) to v1.98 (rust-lang/cargo#17386) - docs: Use mdbook admonitions (rust-lang/cargo#17384) - chore(ci): exclude resolver-tests from intra doc link checks (rust-lang/cargo#17385)

rustbot assigned epage Jan 12, 2025

rustbot added A-interacts-with-crates.io Area: interaction with registries A-manifest Area: Cargo.toml issues Command-publish S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 12, 2025

AudaciousAxiom commented Jan 12, 2025

View reviewed changes

AudaciousAxiom force-pushed the feat/rfc-impl-feature-metadata branch from 790a0e8 to 338281e Compare January 12, 2025 17:49

AudaciousAxiom marked this pull request as ready for review January 12, 2025 18:04

weihanglo reviewed Jan 12, 2025

View reviewed changes

rustbot added A-documenting-cargo-itself Area: Cargo's documentation A-unstable Area: nightly unstable support labels Jan 13, 2025

This comment has been minimized.

Sign in to view

AudaciousAxiom force-pushed the feat/rfc-impl-feature-metadata branch from 905820c to 9b4843c Compare August 27, 2026 20:02

weihanglo reviewed Aug 29, 2026

View reviewed changes

AudaciousAxiom force-pushed the feat/rfc-impl-feature-metadata branch 2 times, most recently from 4c785a0 to 7016b1c Compare August 30, 2026 08:28

weihanglo approved these changes Aug 30, 2026

View reviewed changes

weihanglo reviewed Aug 30, 2026

View reviewed changes

AudaciousAxiom force-pushed the feat/rfc-impl-feature-metadata branch from 7016b1c to fcb04c6 Compare August 30, 2026 13:41

test(feature-metadata): test valid and invalid feature metadata

f05a5f8

feat(manifest)!: implement feature-metadata RFC3416

5110662

weihanglo force-pushed the feat/rfc-impl-feature-metadata branch from fcb04c6 to 5110662 Compare August 31, 2026 00:40

weihanglo enabled auto-merge August 31, 2026 00:42

weihanglo added this pull request to the merge queue Aug 31, 2026

Merged via the queue into rust-lang:master with commit 6895274 Aug 31, 2026
28 checks passed

rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 31, 2026

weihanglo mentioned this pull request Aug 31, 2026

Update cargo submodule rust-lang/rust#161789

Merged

rustbot added this to the 1.100.0 milestone Sep 2, 2026

AudaciousAxiom deleted the feat/rfc-impl-feature-metadata branch September 3, 2026 19:44

weihanglo mentioned this pull request Sep 6, 2026

Tracking Issue for feature-metadata #14157

Open

AudaciousAxiom mentioned this pull request Sep 6, 2026

docs(book): document the table syntax of features #17446

Draft

	/// Unstable feature `-Ztrim-paths`.
	pub trim_paths: Option<TomlTrimPaths>,

+              ...
+              ...
+              ...

Uh oh!

Uh oh!

Conversation

AudaciousAxiom commented Jan 12, 2025 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR try to resolve?

How should we test and review this PR?

Additional information

Questions

Uh oh!

rustbot commented Jan 12, 2025

Uh oh!

AudaciousAxiom Jan 12, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

AudaciousAxiom Jan 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

weihanglo Jan 12, 2025

Choose a reason for hiding this comment

Uh oh!

weihanglo Jan 13, 2025

Choose a reason for hiding this comment

Uh oh!

epage commented Jan 13, 2025

Uh oh!

AudaciousAxiom commented Jan 13, 2025

Uh oh!

epage commented Jan 13, 2025

Uh oh!

This comment has been minimized.

syphar commented Aug 27, 2026

Uh oh!

rustbot commented Aug 27, 2026

Uh oh!

AudaciousAxiom commented Aug 27, 2026

Uh oh!

weihanglo left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

weihanglo Aug 29, 2026

Choose a reason for hiding this comment

Uh oh!

AudaciousAxiom Aug 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AudaciousAxiom commented Aug 30, 2026

Uh oh!

weihanglo commented Aug 30, 2026

Uh oh!

weihanglo left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

weihanglo Aug 30, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AudaciousAxiom Aug 30, 2026

Choose a reason for hiding this comment

Uh oh!

AudaciousAxiom Sep 6, 2026

Choose a reason for hiding this comment

Uh oh!

weihanglo Sep 6, 2026

Choose a reason for hiding this comment

Uh oh!

AudaciousAxiom commented Aug 30, 2026

Uh oh!

weihanglo commented Aug 31, 2026

Uh oh!

Uh oh!

Reviewers

AudaciousAxiom commented Jan 12, 2025 •

edited by rustbot

Loading

AudaciousAxiom Jan 12, 2025 •

edited

Loading

weihanglo left a comment •

edited by rustbot

Loading

AudaciousAxiom Aug 30, 2026 •

edited

Loading

weihanglo left a comment •

edited by rustbot

Loading

weihanglo Aug 30, 2026 •

edited by rustbot

Loading