From 4e5af28150543b388334de720d1b93c1fb712031 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 13 Dec 2024 10:31:53 -0600 Subject: [PATCH] refactor(schema): Group TomlManifest fields to clarify requires_package I found a bug in the manifest parser and figured this would help make it more obvious. Since I was already changing the order, I figure I'm make things a little more logical (user-facing first, implementtion details later) --- crates/cargo-util-schemas/src/manifest/mod.rs | 20 +++++++------ src/cargo/util/toml/mod.rs | 26 ++++++++--------- tests/testsuite/features_namespaced.rs | 16 +++++------ tests/testsuite/publish.rs | 28 +++++++++---------- tests/testsuite/weak_dep_features.rs | 8 +++--- 5 files changed, 50 insertions(+), 48 deletions(-) diff --git a/crates/cargo-util-schemas/src/manifest/mod.rs b/crates/cargo-util-schemas/src/manifest/mod.rs index efb71d8ce..bfb23495c 100644 --- a/crates/cargo-util-schemas/src/manifest/mod.rs +++ b/crates/cargo-util-schemas/src/manifest/mod.rs @@ -35,11 +35,13 @@ use crate::schema::TomlValueWrapper; #[serde(rename_all = "kebab-case")] #[cfg_attr(feature = "unstable-schema", derive(schemars::JsonSchema))] pub struct TomlManifest { - // when adding new fields, be sure to check whether `requires_package` should disallow them pub cargo_features: Option>, + + // Update `requires_package` when adding new package-specific fields pub package: Option>, pub project: Option>, - pub profile: Option, + pub badges: Option>>, + pub features: Option>>, pub lib: Option, pub bin: Option>, pub example: Option>, @@ -52,14 +54,14 @@ pub struct TomlManifest { pub build_dependencies: Option>, #[serde(rename = "build_dependencies")] pub build_dependencies2: Option>, - pub features: Option>>, pub target: Option>, - pub replace: Option>, - pub patch: Option>>, - pub workspace: Option, - pub badges: Option>>, pub lints: Option, + pub workspace: Option, + pub profile: Option, + pub patch: Option>>, + pub replace: Option>, + /// Report unused keys (see also nested `_unused_keys`) /// Note: this is populated by the caller, rather than automatically #[serde(skip)] @@ -69,6 +71,8 @@ pub struct TomlManifest { impl TomlManifest { pub fn requires_package(&self) -> impl Iterator { [ + self.badges.as_ref().map(|_| "badges"), + self.features.as_ref().map(|_| "features"), self.lib.as_ref().map(|_| "lib"), self.bin.as_ref().map(|_| "bin"), self.example.as_ref().map(|_| "example"), @@ -79,9 +83,7 @@ impl TomlManifest { self.build_dependencies() .as_ref() .map(|_| "build-dependencies"), - self.features.as_ref().map(|_| "features"), self.target.as_ref().map(|_| "target"), - self.badges.as_ref().map(|_| "badges"), self.lints.as_ref().map(|_| "lints"), ] .into_iter() diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index ab96cdd03..0a52c4f0c 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -279,7 +279,8 @@ fn normalize_toml( cargo_features: original_toml.cargo_features.clone(), package: None, project: None, - profile: original_toml.profile.clone(), + badges: None, + features: None, lib: None, bin: None, example: None, @@ -290,13 +291,12 @@ fn normalize_toml( dev_dependencies2: None, build_dependencies: None, build_dependencies2: None, - features: None, target: None, - replace: original_toml.replace.clone(), - patch: None, - workspace: original_toml.workspace.clone(), - badges: None, lints: None, + workspace: original_toml.workspace.clone(), + profile: original_toml.profile.clone(), + patch: None, + replace: original_toml.replace.clone(), _unused_keys: Default::default(), }; @@ -2820,9 +2820,11 @@ fn prepare_toml_for_publish( let all = |_d: &manifest::TomlDependency| true; let mut manifest = manifest::TomlManifest { + cargo_features: me.cargo_features.clone(), package: Some(package), project: None, - profile: me.profile.clone(), + badges: me.badges.clone(), + features: me.features.clone(), lib, bin, example, @@ -2837,7 +2839,6 @@ fn prepare_toml_for_publish( dev_dependencies2: None, build_dependencies: map_deps(gctx, me.build_dependencies(), all)?, build_dependencies2: None, - features: me.features.clone(), target: match me.target.as_ref().map(|target_map| { target_map .iter() @@ -2863,12 +2864,11 @@ fn prepare_toml_for_publish( Some(Err(e)) => return Err(e), None => None, }, - replace: None, - patch: None, - workspace: None, - badges: me.badges.clone(), - cargo_features: me.cargo_features.clone(), lints: me.lints.clone(), + workspace: None, + profile: me.profile.clone(), + patch: None, + replace: None, _unused_keys: Default::default(), }; strip_features(&mut manifest); diff --git a/tests/testsuite/features_namespaced.rs b/tests/testsuite/features_namespaced.rs index 796a31edc..d04625a07 100644 --- a/tests/testsuite/features_namespaced.rs +++ b/tests/testsuite/features_namespaced.rs @@ -1006,6 +1006,9 @@ homepage = "https://example.com/" readme = false license = "MIT" +[features] +feat = ["opt-dep1"] + [lib] name = "foo" path = "src/lib.rs" @@ -1018,9 +1021,6 @@ optional = true version = "1.0" optional = true -[features] -feat = ["opt-dep1"] - "##]], )], ); @@ -1143,6 +1143,11 @@ homepage = "https://example.com/" readme = false license = "MIT" +[features] +feat1 = [] +feat2 = ["dep:bar"] +feat3 = ["feat2"] + [lib] name = "foo" path = "src/lib.rs" @@ -1151,11 +1156,6 @@ path = "src/lib.rs" version = "1.0" optional = true -[features] -feat1 = [] -feat2 = ["dep:bar"] -feat3 = ["feat2"] - "##]], )], ); diff --git a/tests/testsuite/publish.rs b/tests/testsuite/publish.rs index 51ab6cfda..4bcb91944 100644 --- a/tests/testsuite/publish.rs +++ b/tests/testsuite/publish.rs @@ -2015,6 +2015,20 @@ readme = false license = "MIT" repository = "foo" +[features] +foo_feature = [ + "normal-only/cat", + "build-only/cat", + "normal-and-dev/cat", + "target-normal-only/cat", + "target-build-only/cat", + "target-normal-and-dev/cat", + "optional-dep-feature/cat", + "dep:optional-namespaced", + "optional-renamed-dep-feature10/cat", + "dep:optional-renamed-namespaced10", +] + [[bin]] name = "foo" path = "src/main.rs" @@ -2057,20 +2071,6 @@ features = ["cat"] version = "1.0" features = ["cat"] -[features] -foo_feature = [ - "normal-only/cat", - "build-only/cat", - "normal-and-dev/cat", - "target-normal-only/cat", - "target-build-only/cat", - "target-normal-and-dev/cat", - "optional-dep-feature/cat", - "dep:optional-namespaced", - "optional-renamed-dep-feature10/cat", - "dep:optional-renamed-namespaced10", -] - [target."cfg(unix)".dependencies.target-normal-and-dev] version = "1.0" features = ["cat"] diff --git a/tests/testsuite/weak_dep_features.rs b/tests/testsuite/weak_dep_features.rs index b0997414c..583f67c8b 100644 --- a/tests/testsuite/weak_dep_features.rs +++ b/tests/testsuite/weak_dep_features.rs @@ -650,6 +650,10 @@ homepage = "https://example.com/" readme = false license = "MIT" +[features] +feat1 = [] +feat2 = ["bar?/feat"] + [lib] name = "foo" path = "src/lib.rs" @@ -658,10 +662,6 @@ path = "src/lib.rs" version = "1.0" optional = true -[features] -feat1 = [] -feat2 = ["bar?/feat"] - "##]], )], );