From 641530a93de3c7f3b5c8edeecb8f3fd826dc22fd Mon Sep 17 00:00:00 2001 From: Aatif Syed Date: Wed, 10 Apr 2024 01:19:34 +0100 Subject: [PATCH 1/6] mark: main From caf3fcaf18b16a4889e5c4c09d48f04255bd3832 Mon Sep 17 00:00:00 2001 From: Aatif Syed Date: Wed, 10 Apr 2024 01:20:06 +0100 Subject: [PATCH 2/6] feat: use FeatureName and PackageName --- Cargo.toml | 1 + src/lib.rs | 19 +++++++++++-------- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 8138918a..a095eb84 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -12,6 +12,7 @@ rust-version = "1.56.0" [dependencies] camino = { version = "1.0.7", features = ["serde1"] } cargo-platform = "0.1.2" +cargo-util-schemas = "0.2.0" derive_builder = { version = "0.12", optional = true } semver = { version = "1.0.7", features = ["serde"] } serde = { version = "1.0.136", features = ["derive"] } diff --git a/src/lib.rs b/src/lib.rs index 4c951faa..929dab58 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -94,6 +94,7 @@ pub use camino; pub use semver; use semver::Version; +use cargo_util_schemas::manifest::{FeatureName, PackageName}; #[cfg(feature = "builder")] pub use dependency::DependencyBuilder; pub use dependency::{Dependency, DependencyKind}; @@ -292,7 +293,7 @@ pub struct Node { /// Features enabled on the crate #[serde(default)] - pub features: Vec, + pub features: Vec, } #[derive(Clone, Serialize, Deserialize, Debug)] @@ -303,7 +304,7 @@ pub struct Node { pub struct NodeDep { /// The name of the dependency's library target. /// If the crate was renamed, it is the new name. - pub name: String, + pub name: String, // TODO(aatifsyed): should this be PackageName? /// Package ID (opaque unique identifier) pub pkg: PackageId, /// The kinds of dependencies. @@ -347,7 +348,7 @@ pub struct DepKindInfo { pub struct Package { /// The [`name` field](https://doc.rust-lang.org/cargo/reference/manifest.html#the-name-field) as given in the `Cargo.toml` // (We say "given in" instead of "specified in" since the `name` key cannot be inherited from the workspace.) - pub name: String, + pub name: PackageName, /// The [`version` field](https://doc.rust-lang.org/cargo/reference/manifest.html#the-version-field) as specified in the `Cargo.toml` pub version: Version, /// The [`authors` field](https://doc.rust-lang.org/cargo/reference/manifest.html#the-authors-field) as specified in the `Cargo.toml` @@ -357,7 +358,7 @@ pub struct Package { pub id: PackageId, /// The source of the package, e.g. /// crates.io or `None` for local projects. - pub source: Option, + pub source: Option, // TODO(aatifsyed): should this be RegistryName? /// The [`description` field](https://doc.rust-lang.org/cargo/reference/manifest.html#the-description-field) as specified in the `Cargo.toml` pub description: Option, /// List of dependencies of this particular package @@ -371,7 +372,7 @@ pub struct Package { /// Targets provided by the crate (lib, bin, example, test, ...) pub targets: Vec, /// Features provided by the crate, mapped to the features required by that feature. - pub features: BTreeMap>, + pub features: BTreeMap>, /// Path containing the `Cargo.toml` pub manifest_path: Utf8PathBuf, /// The [`categories` field](https://doc.rust-lang.org/cargo/reference/manifest.html#the-categories-field) as specified in the `Cargo.toml` @@ -525,7 +526,7 @@ pub struct Target { #[serde(rename = "required-features")] /// This target is built only if these features are enabled. /// It doesn't apply to `lib` targets. - pub required_features: Vec, + pub required_features: Vec, /// Path to the main source file of the target pub src_path: Utf8PathBuf, /// Rust edition for this target @@ -776,7 +777,7 @@ pub enum CargoOpt { /// Run cargo with `--no-default-features` NoDefaultFeatures, /// Run cargo with `--features ` - SomeFeatures(Vec), + SomeFeatures(Vec), } /// A builder for configurating `cargo metadata` invocation. @@ -876,7 +877,9 @@ impl MetadataCommand { /// ``` pub fn features(&mut self, features: CargoOpt) -> &mut MetadataCommand { match features { - CargoOpt::SomeFeatures(features) => self.features.extend(features), + CargoOpt::SomeFeatures(features) => self + .features + .extend(features.into_iter().map(FeatureName::into_inner)), CargoOpt::NoDefaultFeatures => { assert!( !self.no_default_features, From 88f9967720af2dd341f36e894d3ed30ac9543eae Mon Sep 17 00:00:00 2001 From: Aatif Syed Date: Wed, 10 Apr 2024 01:32:56 +0100 Subject: [PATCH 3/6] test: fix tests --- tests/selftest.rs | 4 +-- tests/test_samples.rs | 60 ++++++++++++++++++++++++++++--------------- 2 files changed, 41 insertions(+), 23 deletions(-) diff --git a/tests/selftest.rs b/tests/selftest.rs index e6dadf77..2ad1ce4a 100644 --- a/tests/selftest.rs +++ b/tests/selftest.rs @@ -17,7 +17,7 @@ fn metadata() { let metadata = MetadataCommand::new().no_deps().exec().unwrap(); let this = &metadata.packages[0]; - assert_eq!(this.name, "cargo_metadata"); + assert_eq!(this.name.as_str(), "cargo_metadata"); assert_eq!(this.targets.len(), 3); let lib = this @@ -130,7 +130,7 @@ fn metadata_deps() { .expect("Did not find ourselves"); let this = &metadata[this_id]; - assert_eq!(this.name, "cargo_metadata"); + assert_eq!(this.name.as_str(), "cargo_metadata"); let workspace_packages = metadata.workspace_packages(); assert_eq!(workspace_packages.len(), 1); diff --git a/tests/test_samples.rs b/tests/test_samples.rs index 2f406c5b..a41d10e4 100644 --- a/tests/test_samples.rs +++ b/tests/test_samples.rs @@ -8,6 +8,7 @@ use cargo_metadata::{ workspace_default_members_is_missing, ArtifactDebuginfo, CargoOpt, DependencyKind, Edition, Message, Metadata, MetadataCommand, }; +use cargo_util_schemas::manifest::FeatureName; /// Output from oldest version ever supported (1.24). /// @@ -71,7 +72,7 @@ fn old_minimal() { let meta: Metadata = serde_json::from_str(JSON_OLD_MINIMAL).unwrap(); assert_eq!(meta.packages.len(), 1); let pkg = &meta.packages[0]; - assert_eq!(pkg.name, "foo"); + assert_eq!(pkg.name.as_str(), "foo"); assert_eq!(pkg.version, semver::Version::parse("0.1.0").unwrap()); assert_eq!(pkg.authors.len(), 0); assert_eq!(pkg.id.to_string(), "foo 0.1.0 (path+file:///foo)"); @@ -143,6 +144,14 @@ macro_rules! sorted { }}; } +macro_rules! features { + ($($feat:expr),* $(,)?) => { + ::std::vec![ + $(::cargo_util_schemas::manifest::FeatureName::new(String::from($feat)).unwrap()),* + ] + }; +} + fn cargo_version() -> semver::Version { let output = std::process::Command::new("cargo") .arg("-V") @@ -214,7 +223,11 @@ fn all_the_fields() { } assert_eq!(meta.packages.len(), 9); - let all = meta.packages.iter().find(|p| p.name == "all").unwrap(); + let all = meta + .packages + .iter() + .find(|p| p.name.as_str() == "all") + .unwrap(); assert_eq!(all.version, semver::Version::parse("0.1.0").unwrap()); assert_eq!(all.authors, vec!["Jane Doe "]); assert!(all.id.to_string().contains("all")); @@ -335,7 +348,7 @@ fn all_the_fields() { assert!(!otherbin.doc); let reqfeat = get_file_name!("reqfeat.rs"); - assert_eq!(reqfeat.required_features, vec!["feat2"]); + assert_eq!(reqfeat.required_features, features!["feat2"]); let ex1 = get_file_name!("ex1.rs"); assert_eq!(ex1.kind, vec!["example".into()]); @@ -410,14 +423,14 @@ fn all_the_fields() { .iter() .find(|n| n.id.to_string().contains("bitflags")) .unwrap(); - assert_eq!(bitflags.features, vec!["default"]); + assert_eq!(bitflags.features, features!["default"]); let featdep = resolve .nodes .iter() .find(|n| n.id.to_string().contains("featdep")) .unwrap(); - assert_eq!(featdep.features, vec!["i128"]); + assert_eq!(featdep.features, features!["i128"]); let all = resolve .nodes @@ -442,7 +455,10 @@ fn all_the_fields() { .find(|d| d.name == "different_name") .unwrap(); assert!(namedep.pkg.to_string().contains("namedep")); - assert_eq!(sorted!(all.features), vec!["bitflags", "default", "feat1"]); + assert_eq!( + sorted!(all.features), + features!["bitflags", "default", "feat1"] + ); let bdep = all.deps.iter().find(|d| d.name == "bdep").unwrap(); assert_eq!(bdep.dep_kinds.len(), 1); @@ -553,7 +569,11 @@ fn current_dir() { .current_dir("tests/all/namedep") .exec() .unwrap(); - let namedep = meta.packages.iter().find(|p| p.name == "namedep").unwrap(); + let namedep = meta + .packages + .iter() + .find(|p| p.name.as_str() == "namedep") + .unwrap(); assert!(namedep.name.starts_with("namedep")); } @@ -582,7 +602,7 @@ Evil proc macro was here! fn advanced_feature_configuration() { fn build_features &mut MetadataCommand>( func: F, - ) -> Vec { + ) -> Vec { let mut meta = MetadataCommand::new(); let meta = meta.manifest_path("tests/all/Cargo.toml"); @@ -604,26 +624,23 @@ fn advanced_feature_configuration() { let default_features = build_features(|meta| meta); assert_eq!( sorted!(default_features), - vec!["bitflags", "default", "feat1"] + features!["bitflags", "default", "feat1"] ); // Manually specify the same default features let manual_features = build_features(|meta| { meta.features(CargoOpt::NoDefaultFeatures) - .features(CargoOpt::SomeFeatures(vec![ - "feat1".into(), - "bitflags".into(), - ])) + .features(CargoOpt::SomeFeatures(features!["feat1", "bitflags",])) }); - assert_eq!(sorted!(manual_features), vec!["bitflags", "feat1"]); + assert_eq!(sorted!(manual_features), features!["bitflags", "feat1"]); // Multiple SomeFeatures is same as one longer SomeFeatures let manual_features = build_features(|meta| { meta.features(CargoOpt::NoDefaultFeatures) - .features(CargoOpt::SomeFeatures(vec!["feat1".into()])) - .features(CargoOpt::SomeFeatures(vec!["feat2".into()])) + .features(CargoOpt::SomeFeatures(features!["feat1"])) + .features(CargoOpt::SomeFeatures(features!["feat2"])) }); - assert_eq!(sorted!(manual_features), vec!["feat1", "feat2"]); + assert_eq!(sorted!(manual_features), features!["feat1", "feat2"]); // No features + All features == All features let all_features = build_features(|meta| { @@ -632,12 +649,12 @@ fn advanced_feature_configuration() { }); assert_eq!( sorted!(all_features), - vec!["bitflags", "default", "feat1", "feat2"] + features!["bitflags", "default", "feat1", "feat2"] ); // The '--all-features' flag supersedes other feature flags let all_flag_variants = build_features(|meta| { - meta.features(CargoOpt::SomeFeatures(vec!["feat2".into()])) + meta.features(CargoOpt::SomeFeatures(features!["feat2"])) .features(CargoOpt::NoDefaultFeatures) .features(CargoOpt::AllFeatures) }); @@ -659,7 +676,7 @@ fn basic_workspace_root_package_exists() { .manifest_path("tests/basic_workspace/Cargo.toml") .exec() .unwrap(); - assert_eq!(meta.root_package().unwrap().name, "ex_bin"); + assert_eq!(meta.root_package().unwrap().name.as_str(), "ex_bin"); // Now with no_deps, it should still work exactly the same let meta = MetadataCommand::new() .manifest_path("tests/basic_workspace/Cargo.toml") @@ -669,7 +686,8 @@ fn basic_workspace_root_package_exists() { assert_eq!( meta.root_package() .expect("workspace root still exists when no_deps used") - .name, + .name + .as_str(), "ex_bin" ); } From f501cdbee3ceeb186e9b6687e2875395e23e4dff Mon Sep 17 00:00:00 2001 From: Aatif Syed Date: Wed, 10 Apr 2024 01:37:42 +0100 Subject: [PATCH 4/6] refactor: builder accepts strings --- src/lib.rs | 6 ++---- tests/test_samples.rs | 27 +++++++++++++++++---------- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 929dab58..1c0f8f79 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -777,7 +777,7 @@ pub enum CargoOpt { /// Run cargo with `--no-default-features` NoDefaultFeatures, /// Run cargo with `--features ` - SomeFeatures(Vec), + SomeFeatures(Vec), } /// A builder for configurating `cargo metadata` invocation. @@ -877,9 +877,7 @@ impl MetadataCommand { /// ``` pub fn features(&mut self, features: CargoOpt) -> &mut MetadataCommand { match features { - CargoOpt::SomeFeatures(features) => self - .features - .extend(features.into_iter().map(FeatureName::into_inner)), + CargoOpt::SomeFeatures(features) => self.features.extend(features), CargoOpt::NoDefaultFeatures => { assert!( !self.no_default_features, diff --git a/tests/test_samples.rs b/tests/test_samples.rs index a41d10e4..0a1b1235 100644 --- a/tests/test_samples.rs +++ b/tests/test_samples.rs @@ -602,7 +602,7 @@ Evil proc macro was here! fn advanced_feature_configuration() { fn build_features &mut MetadataCommand>( func: F, - ) -> Vec { + ) -> Vec { let mut meta = MetadataCommand::new(); let meta = meta.manifest_path("tests/all/Cargo.toml"); @@ -617,30 +617,37 @@ fn advanced_feature_configuration() { .find(|n| !n.features.is_empty()) .unwrap(); - all.features.clone() + all.features + .clone() + .into_iter() + .map(FeatureName::into_inner) + .collect() } // Default behavior; tested above let default_features = build_features(|meta| meta); assert_eq!( sorted!(default_features), - features!["bitflags", "default", "feat1"] + vec!["bitflags", "default", "feat1"] ); // Manually specify the same default features let manual_features = build_features(|meta| { meta.features(CargoOpt::NoDefaultFeatures) - .features(CargoOpt::SomeFeatures(features!["feat1", "bitflags",])) + .features(CargoOpt::SomeFeatures(vec![ + "feat1".into(), + "bitflags".into(), + ])) }); - assert_eq!(sorted!(manual_features), features!["bitflags", "feat1"]); + assert_eq!(sorted!(manual_features), vec!["bitflags", "feat1"]); // Multiple SomeFeatures is same as one longer SomeFeatures let manual_features = build_features(|meta| { meta.features(CargoOpt::NoDefaultFeatures) - .features(CargoOpt::SomeFeatures(features!["feat1"])) - .features(CargoOpt::SomeFeatures(features!["feat2"])) + .features(CargoOpt::SomeFeatures(vec!["feat1".into()])) + .features(CargoOpt::SomeFeatures(vec!["feat2".into()])) }); - assert_eq!(sorted!(manual_features), features!["feat1", "feat2"]); + assert_eq!(sorted!(manual_features), vec!["feat1", "feat2"]); // No features + All features == All features let all_features = build_features(|meta| { @@ -649,12 +656,12 @@ fn advanced_feature_configuration() { }); assert_eq!( sorted!(all_features), - features!["bitflags", "default", "feat1", "feat2"] + vec!["bitflags", "default", "feat1", "feat2"] ); // The '--all-features' flag supersedes other feature flags let all_flag_variants = build_features(|meta| { - meta.features(CargoOpt::SomeFeatures(features!["feat2"])) + meta.features(CargoOpt::SomeFeatures(vec!["feat2".into()])) .features(CargoOpt::NoDefaultFeatures) .features(CargoOpt::AllFeatures) }); From 3a28fe244b58a60d559d2680df5133ad94c08748 Mon Sep 17 00:00:00 2001 From: Aatif Syed Date: Fri, 26 Apr 2024 20:05:32 +0200 Subject: [PATCH 5/6] build!: bump rust-version to 1.75.0 --- .github/workflows/main.yml | 2 +- Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 27c8c4e4..05b25b0a 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -31,7 +31,7 @@ jobs: - rust: stable - rust: beta - rust: nightly - - rust: 1.56.0 + - rust: 1.75.0 steps: - uses: actions/checkout@v2 - name: Install rust diff --git a/Cargo.toml b/Cargo.toml index a095eb84..e7ad9c2a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -7,7 +7,7 @@ description = "structured access to the output of `cargo metadata`" license = "MIT" readme = "README.md" edition = "2018" -rust-version = "1.56.0" +rust-version = "1.75.0" [dependencies] camino = { version = "1.0.7", features = ["serde1"] } From 074a36c4daa6cb74e46b6e7dbfe187647dbefe4b Mon Sep 17 00:00:00 2001 From: Aatif Syed Date: Thu, 23 May 2024 12:02:31 +0200 Subject: [PATCH 6/6] refactor: NodeDep::name: PackageName --- src/lib.rs | 5 +++-- tests/test_samples.rs | 12 ++++++------ 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 1c0f8f79..63fc3d5e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -304,7 +304,7 @@ pub struct Node { pub struct NodeDep { /// The name of the dependency's library target. /// If the crate was renamed, it is the new name. - pub name: String, // TODO(aatifsyed): should this be PackageName? + pub name: PackageName, /// Package ID (opaque unique identifier) pub pkg: PackageId, /// The kinds of dependencies. @@ -358,7 +358,8 @@ pub struct Package { pub id: PackageId, /// The source of the package, e.g. /// crates.io or `None` for local projects. - pub source: Option, // TODO(aatifsyed): should this be RegistryName? + // Note that this is NOT the same as cargo_util_schemas::RegistryName + pub source: Option, /// The [`description` field](https://doc.rust-lang.org/cargo/reference/manifest.html#the-description-field) as specified in the `Cargo.toml` pub description: Option, /// List of dependencies of this particular package diff --git a/tests/test_samples.rs b/tests/test_samples.rs index 0a1b1235..d35edd16 100644 --- a/tests/test_samples.rs +++ b/tests/test_samples.rs @@ -439,10 +439,10 @@ fn all_the_fields() { .unwrap(); assert_eq!(all.dependencies.len(), 8); assert_eq!(all.deps.len(), 8); - let newname = all.deps.iter().find(|d| d.name == "newname").unwrap(); + let newname = all.deps.iter().find(|d| &*d.name == "newname").unwrap(); assert!(newname.pkg.to_string().contains("oldname")); // Note the underscore here. - let path_dep = all.deps.iter().find(|d| d.name == "path_dep").unwrap(); + let path_dep = all.deps.iter().find(|d| &*d.name == "path_dep").unwrap(); assert!(path_dep.pkg.to_string().contains("path-dep")); assert_eq!(path_dep.dep_kinds.len(), 1); let kind = &path_dep.dep_kinds[0]; @@ -452,7 +452,7 @@ fn all_the_fields() { let namedep = all .deps .iter() - .find(|d| d.name == "different_name") + .find(|d| &*d.name == "different_name") .unwrap(); assert!(namedep.pkg.to_string().contains("namedep")); assert_eq!( @@ -460,19 +460,19 @@ fn all_the_fields() { features!["bitflags", "default", "feat1"] ); - let bdep = all.deps.iter().find(|d| d.name == "bdep").unwrap(); + let bdep = all.deps.iter().find(|d| &*d.name == "bdep").unwrap(); assert_eq!(bdep.dep_kinds.len(), 1); let kind = &bdep.dep_kinds[0]; assert_eq!(kind.kind, DependencyKind::Build); assert!(kind.target.is_none()); - let devdep = all.deps.iter().find(|d| d.name == "devdep").unwrap(); + let devdep = all.deps.iter().find(|d| &*d.name == "devdep").unwrap(); assert_eq!(devdep.dep_kinds.len(), 1); let kind = &devdep.dep_kinds[0]; assert_eq!(kind.kind, DependencyKind::Development); assert!(kind.target.is_none()); - let windep = all.deps.iter().find(|d| d.name == "windep").unwrap(); + let windep = all.deps.iter().find(|d| &*d.name == "windep").unwrap(); assert_eq!(windep.dep_kinds.len(), 1); let kind = &windep.dep_kinds[0]; assert_eq!(kind.kind, DependencyKind::Normal);