Skip to content

Commit

Permalink
feat(manifest)!: implement feature-metadata RFC3416
Browse files Browse the repository at this point in the history
  • Loading branch information
AudaciousAxiom committed Jan 12, 2025
1 parent b414a91 commit af99add
Show file tree
Hide file tree
Showing 4 changed files with 112 additions and 32 deletions.
61 changes: 59 additions & 2 deletions crates/cargo-util-schemas/src/manifest/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ pub struct TomlManifest {
pub package: Option<Box<TomlPackage>>,
pub project: Option<Box<TomlPackage>>,
pub badges: Option<BTreeMap<String, BTreeMap<String, String>>>,
pub features: Option<BTreeMap<FeatureName, Vec<String>>>,
pub features: Option<BTreeMap<FeatureName, FeatureDefinition>>,
pub lib: Option<TomlLibTarget>,
pub bin: Option<Vec<TomlBinTarget>>,
pub example: Option<Vec<TomlExampleTarget>>,
Expand Down Expand Up @@ -110,7 +110,7 @@ impl TomlManifest {
.or(self.build_dependencies2.as_ref())
}

pub fn features(&self) -> Option<&BTreeMap<FeatureName, Vec<String>>> {
pub fn features(&self) -> Option<&BTreeMap<FeatureName, FeatureDefinition>> {
self.features.as_ref()
}

Expand Down Expand Up @@ -1521,6 +1521,63 @@ impl TomlPlatform {
}
}

/// Definition of a feature.
#[derive(Clone, Debug, Serialize)]
#[cfg_attr(feature = "unstable-schema", derive(schemars::JsonSchema))]
#[serde(untagged)]
pub enum FeatureDefinition {
/// Features that this feature enables.
Array(Vec<String>),
/// Metadata of this feature.
Metadata(FeatureMetadata),
}

// Implementing `Deserialize` manually allows for a better error message when the `enables` key is
// missing.
impl<'de> de::Deserialize<'de> for FeatureDefinition {
fn deserialize<D>(d: D) -> Result<FeatureDefinition, D::Error>
where
D: de::Deserializer<'de>,
{
UntaggedEnumVisitor::new()
.seq(|seq| {
seq.deserialize::<Vec<String>>()
.map(FeatureDefinition::Array)
})
.map(|seq| {
seq.deserialize::<FeatureMetadata>()
.map(FeatureDefinition::Metadata)
})
.deserialize(d)
}
}

impl FeatureDefinition {
/// Returns the features that this feature enables.
pub fn enables(&self) -> &[String] {
match self {
Self::Array(features) => features,
Self::Metadata(FeatureMetadata {
enables: features, ..
}) => features,
}
}
}

/// Metadata of a feature.
#[derive(Clone, Debug, Deserialize, Serialize)]
#[cfg_attr(feature = "unstable-schema", derive(schemars::JsonSchema))]
pub struct FeatureMetadata {
/// Features that this feature enables.
pub enables: Vec<String>,

/// This is here to provide a way to see the "unused manifest keys" when deserializing
#[serde(skip_serializing)]
#[serde(flatten)]
#[cfg_attr(feature = "unstable-schema", schemars(skip))]
pub _unused_keys: BTreeMap<String, toml::Value>,
}

#[derive(Serialize, Debug, Clone)]
#[cfg_attr(feature = "unstable-schema", derive(schemars::JsonSchema))]
pub struct InheritableLints {
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/ops/registry/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -539,7 +539,7 @@ pub(crate) fn prepare_transmit(
.map(|(feat, values)| {
(
feat.to_string(),
values.iter().map(|fv| fv.to_string()).collect(),
values.enables().iter().map(|fv| fv.to_string()).collect(),
)
})
.collect::<BTreeMap<String, Vec<String>>>(),
Expand Down
60 changes: 47 additions & 13 deletions src/cargo/util/toml/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ use anyhow::{anyhow, bail, Context as _};
use cargo_platform::Platform;
use cargo_util::paths;
use cargo_util_schemas::manifest::{
self, PackageName, PathBaseName, TomlDependency, TomlDetailedDependency, TomlManifest,
self, FeatureDefinition, FeatureMetadata, FeatureName, PackageName, PathBaseName,
TomlDependency, TomlDetailedDependency, TomlManifest,
};
use cargo_util_schemas::manifest::{RustVersion, StringOrBool};
use itertools::Itertools;
Expand Down Expand Up @@ -708,8 +709,8 @@ fn default_readme_from_package_root(package_root: &Path) -> Option<String> {

#[tracing::instrument(skip_all)]
fn normalize_features(
original_features: Option<&BTreeMap<manifest::FeatureName, Vec<String>>>,
) -> CargoResult<Option<BTreeMap<manifest::FeatureName, Vec<String>>>> {
original_features: Option<&BTreeMap<manifest::FeatureName, FeatureDefinition>>,
) -> CargoResult<Option<BTreeMap<manifest::FeatureName, FeatureDefinition>>> {
let Some(normalized_features) = original_features.cloned() else {
return Ok(None);
};
Expand Down Expand Up @@ -1296,6 +1297,8 @@ pub fn to_real_manifest(
}
}

validate_feature_definitions(original_toml.features.as_ref(), warnings)?;

validate_dependencies(original_toml.dependencies.as_ref(), None, None, warnings)?;
validate_dependencies(
original_toml.dev_dependencies(),
Expand Down Expand Up @@ -1497,7 +1500,7 @@ pub fn to_real_manifest(
.map(|(k, v)| {
(
InternedString::new(k),
v.iter().map(InternedString::from).collect(),
v.enables().iter().map(InternedString::from).collect(),
)
})
.collect(),
Expand Down Expand Up @@ -1764,6 +1767,30 @@ fn to_virtual_manifest(
Ok(manifest)
}

fn validate_feature_definitions(
features: Option<&BTreeMap<FeatureName, FeatureDefinition>>,
warnings: &mut Vec<String>,
) -> CargoResult<()> {
let Some(features) = features else {
return Ok(());
};

for (feature, feature_definition) in features {
match feature_definition {
FeatureDefinition::Array(..) => {}
FeatureDefinition::Metadata(FeatureMetadata { _unused_keys, .. }) => {
warnings.extend(
_unused_keys
.keys()
.map(|k| format!("unused manifest key: `features.{feature}.{k}`")),
);
}
}
}

Ok(())
}

#[tracing::instrument(skip_all)]
fn validate_dependencies(
original_deps: Option<&BTreeMap<manifest::PackageName, manifest::InheritableDependency>>,
Expand Down Expand Up @@ -2902,16 +2929,23 @@ fn prepare_toml_for_publish(
};

features.values_mut().for_each(|feature_deps| {
feature_deps.retain(|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)
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);
});
}

Expand Down
21 changes: 5 additions & 16 deletions tests/testsuite/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2362,7 +2362,7 @@ fn unused_keys_in_feature_metadata() {
edition = "2015"
[features]
foo = { enables = ["bar"], foobar = false }
foo = { enables = ["bar"], a = false, b = true }
bar = []
"#,
)
Expand All @@ -2371,7 +2371,8 @@ fn unused_keys_in_feature_metadata() {

p.cargo("check")
.with_stderr_data(str![[r#"
[WARNING] unused manifest key: `features.foo.foobar`
[WARNING] unused manifest key: `features.foo.a`
[WARNING] unused manifest key: `features.foo.b`
[CHECKING] foo v0.0.0 ([ROOT]/foo)
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
Expand All @@ -2395,22 +2396,10 @@ fn normalize_feature_metadata() {
c = { enables = ["a", "b"] }
"#,
)
.file(
"src/main.rs",
r#"
#[cfg(feature = "a")]
fn a() {}
#[cfg(feature = "b")]
fn b() {}
fn main() {
a();
b();
}
"#,
)
.file("src/main.rs", "")
.build();

p.cargo("package --no-verify -v").run();
p.cargo("package --no-verify").run();
let f = File::open(&p.root().join("target/package/foo-0.0.0.crate")).unwrap();
let normalized_manifest = str![[r#"
# THIS FILE IS AUTOMATICALLY GENERATED BY CARGO
Expand Down

0 comments on commit af99add

Please sign in to comment.