From 6829bf7ae0af59e779a75acfd2ad300148df5f71 Mon Sep 17 00:00:00 2001 From: Ethan Brierley Date: Fri, 6 Sep 2024 08:22:23 +0100 Subject: [PATCH 1/4] test: public dependencies don't show up in `cargo-metadata` --- tests/testsuite/metadata.rs | 197 ++++++++++++++++++++++++++++++++++++ 1 file changed, 197 insertions(+) diff --git a/tests/testsuite/metadata.rs b/tests/testsuite/metadata.rs index e76796c0514..d8187a907b4 100644 --- a/tests/testsuite/metadata.rs +++ b/tests/testsuite/metadata.rs @@ -627,6 +627,203 @@ fn cargo_metadata_with_deps_and_version() { .run(); } +/// The `public` field should not show up in `cargo metadata` output if `-Zpublic-dependency` +/// is not enabled +#[cargo_test] +fn cargo_metadata_public_private_dependencies_disabled() { + let p = project() + .file("src/foo.rs", "") + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.5.0" + authors = [] + license = "MIT" + description = "foo" + + [[bin]] + name = "foo" + + [dependencies] + bar = { version = "*", public = false } + foobar = { version = "*", public = true } + baz = "*" + "#, + ) + .build(); + Package::new("bar", "0.0.1").publish(); + Package::new("foobar", "0.0.2").publish(); + Package::new("baz", "0.0.3").publish(); + + p.cargo("metadata -q --format-version 1") + .with_stdout_data( + str![[r#" +{ + "metadata": null, + "packages": [ + { + "name": "bar", + "...": "{...}" + }, + { + "name": "baz", + "...": "{...}" + }, + { + "name": "foo", + "dependencies": [ + { + "features": [], + "kind": null, + "name": "bar", + "optional": false, + "registry": null, + "rename": null, + "req": "*", + "source": "registry+https://github.com/rust-lang/crates.io-index", + "target": null, + "uses_default_features": true + }, + { + "features": [], + "kind": null, + "name": "baz", + "optional": false, + "registry": null, + "rename": null, + "req": "*", + "source": "registry+https://github.com/rust-lang/crates.io-index", + "target": null, + "uses_default_features": true + }, + { + "features": [], + "kind": null, + "name": "foobar", + "optional": false, + "registry": null, + "rename": null, + "req": "*", + "source": "registry+https://github.com/rust-lang/crates.io-index", + "target": null, + "uses_default_features": true + } + ], + "...": "{...}" + }, + { + "name": "foobar", + "...": "{...}" + } + ], + "...": "{...}" +} +"#]] + .is_json(), + ) + .run(); +} + +#[cargo_test] +fn cargo_metadata_public_private_dependencies_enabled() { + let p = project() + .file("src/foo.rs", "") + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.5.0" + authors = [] + license = "MIT" + description = "foo" + + [[bin]] + name = "foo" + + [dependencies] + bar = { version = "*", public = false } + foobar = { version = "*", public = true } + baz = "*" + "#, + ) + .build(); + Package::new("bar", "0.0.1").publish(); + Package::new("foobar", "0.0.2").publish(); + Package::new("baz", "0.0.3").publish(); + + p.cargo("metadata -q --format-version 1 -Zpublic-dependency") + .masquerade_as_nightly_cargo(&["public-dependency"]) + .with_stdout_data( + str![[r#" +{ + "metadata": null, + "packages": [ + { + "name": "bar", + "...": "{...}" + }, + { + "name": "baz", + "...": "{...}" + }, + { + "name": "foo", + "dependencies": [ + { + "features": [], + "kind": null, + "name": "bar", + "optional": false, + "registry": null, + "rename": null, + "req": "*", + "source": "registry+https://github.com/rust-lang/crates.io-index", + "target": null, + "uses_default_features": true + }, + { + "features": [], + "kind": null, + "name": "baz", + "optional": false, + "registry": null, + "rename": null, + "req": "*", + "source": "registry+https://github.com/rust-lang/crates.io-index", + "target": null, + "uses_default_features": true + }, + { + "features": [], + "kind": null, + "name": "foobar", + "optional": false, + "registry": null, + "rename": null, + "req": "*", + "source": "registry+https://github.com/rust-lang/crates.io-index", + "target": null, + "uses_default_features": true + } + ], + "...": "{...}" + }, + { + "name": "foobar", + "...": "{...}" + } + ], + "...": "{...}" +} +"#]] + .is_json(), + ) + .run(); +} + #[cargo_test] fn example() { let p = project() From ac7d8c17ffdbade8cee1c29f1daa0d268a4eb867 Mon Sep 17 00:00:00 2001 From: Ethan Brierley Date: Fri, 6 Sep 2024 08:22:23 +0100 Subject: [PATCH 2/4] refactor: intorduce `Dependency::serialized` replacing `Serialize` impl This change introduces a new method, `Dependency::serialized` which replaces the direct `Serialize` implementation on `Dependency`. This matches the pattern used by `Package` with its `Package::serialized`, and will enable us to influence the serialization format with additional arguments. I replaced borrowed types in `SerializedDependency` with owned variants to satisfy the borrow checker. This matches `SerializedPackage` and shouldn't be an issue since `Dependency` is cheap to copy. --- src/cargo/core/dependency.rs | 55 ++++++++++++++++-------------------- src/cargo/core/mod.rs | 2 +- src/cargo/core/package.rs | 12 ++++++-- 3 files changed, 34 insertions(+), 35 deletions(-) diff --git a/src/cargo/core/dependency.rs b/src/cargo/core/dependency.rs index 12c48411983..be999c59a64 100644 --- a/src/cargo/core/dependency.rs +++ b/src/cargo/core/dependency.rs @@ -52,52 +52,28 @@ struct Inner { } #[derive(Serialize)] -struct SerializedDependency<'a> { - name: &'a str, +pub struct SerializedDependency { + name: InternedString, source: SourceId, req: String, kind: DepKind, - rename: Option<&'a str>, + rename: Option, optional: bool, uses_default_features: bool, - features: &'a [InternedString], + features: Vec, #[serde(skip_serializing_if = "Option::is_none")] - artifact: Option<&'a Artifact>, - target: Option<&'a Platform>, + artifact: Option, + target: Option, /// The registry URL this dependency is from. /// If None, then it comes from the default registry (crates.io). - registry: Option<&'a str>, + registry: Option, /// The file system path for a local path dependency. #[serde(skip_serializing_if = "Option::is_none")] path: Option, } -impl ser::Serialize for Dependency { - fn serialize(&self, s: S) -> Result - where - S: ser::Serializer, - { - let registry_id = self.registry_id(); - SerializedDependency { - name: &*self.package_name(), - source: self.source_id(), - req: self.version_req().to_string(), - kind: self.kind(), - optional: self.is_optional(), - uses_default_features: self.uses_default_features(), - features: self.features(), - target: self.platform(), - rename: self.explicit_name_in_toml().map(|s| s.as_str()), - registry: registry_id.as_ref().map(|sid| sid.url().as_str()), - path: self.source_id().local_path(), - artifact: self.artifact(), - } - .serialize(s) - } -} - #[derive(PartialEq, Eq, Hash, Ord, PartialOrd, Clone, Debug, Copy)] pub enum DepKind { Normal, @@ -182,6 +158,23 @@ impl Dependency { } } + pub fn serialized(&self) -> SerializedDependency { + SerializedDependency { + name: self.package_name(), + source: self.source_id(), + req: self.version_req().to_string(), + kind: self.kind(), + optional: self.is_optional(), + uses_default_features: self.uses_default_features(), + features: self.features().to_vec(), + target: self.inner.platform.clone(), + rename: self.explicit_name_in_toml(), + registry: self.registry_id().as_ref().map(|sid| sid.url().to_string()), + path: self.source_id().local_path(), + artifact: self.inner.artifact.clone(), + } + } + pub fn version_req(&self) -> &OptVersionReq { &self.inner.req } diff --git a/src/cargo/core/mod.rs b/src/cargo/core/mod.rs index bdc9b2f4fe6..e857ca35dad 100644 --- a/src/cargo/core/mod.rs +++ b/src/cargo/core/mod.rs @@ -1,4 +1,4 @@ -pub use self::dependency::Dependency; +pub use self::dependency::{Dependency, SerializedDependency}; pub use self::features::{CliUnstable, Edition, Feature, Features}; pub use self::manifest::{EitherManifest, VirtualManifest}; pub use self::manifest::{Manifest, Target, TargetKind}; diff --git a/src/cargo/core/package.rs b/src/cargo/core/package.rs index 7dd412154f7..4495aa245ec 100644 --- a/src/cargo/core/package.rs +++ b/src/cargo/core/package.rs @@ -22,7 +22,9 @@ use crate::core::compiler::{CompileKind, RustcTargetData}; use crate::core::dependency::DepKind; use crate::core::resolver::features::ForceAllTargets; use crate::core::resolver::{HasDevUnits, Resolve}; -use crate::core::{Dependency, Manifest, PackageId, PackageIdSpec, SourceId, Target}; +use crate::core::{ + Dependency, Manifest, PackageId, PackageIdSpec, SerializedDependency, SourceId, Target, +}; use crate::core::{Summary, Workspace}; use crate::sources::source::{MaybePackage, SourceMap}; use crate::util::cache_lock::{CacheLock, CacheLockMode}; @@ -73,7 +75,7 @@ pub struct SerializedPackage { license_file: Option, description: Option, source: SourceId, - dependencies: Vec, + dependencies: Vec, targets: Vec, features: BTreeMap>, manifest_path: PathBuf, @@ -224,7 +226,11 @@ impl Package { license_file: manmeta.license_file.clone(), description: manmeta.description.clone(), source: summary.source_id(), - dependencies: summary.dependencies().to_vec(), + dependencies: summary + .dependencies() + .iter() + .map(Dependency::serialized) + .collect(), targets, features, manifest_path: self.manifest_path().to_path_buf(), From fe9600675eefef88c19d2ee1e1835c598c0299d3 Mon Sep 17 00:00:00 2001 From: Ethan Brierley Date: Fri, 6 Sep 2024 08:22:23 +0100 Subject: [PATCH 3/4] feat: include public/private dependency status in `cargo metadata` output --- src/bin/cargo/commands/read_manifest.rs | 5 ++++- src/cargo/core/dependency.rs | 21 +++++++++++++++++++-- src/cargo/core/package.rs | 15 ++++++++++----- src/cargo/ops/cargo_output_metadata.rs | 7 +++++-- tests/testsuite/metadata.rs | 3 +++ 5 files changed, 41 insertions(+), 10 deletions(-) diff --git a/src/bin/cargo/commands/read_manifest.rs b/src/bin/cargo/commands/read_manifest.rs index b86bbf795bc..692b79d1cbb 100644 --- a/src/bin/cargo/commands/read_manifest.rs +++ b/src/bin/cargo/commands/read_manifest.rs @@ -15,6 +15,9 @@ Deprecated, use `cargo metadata --no-deps` instead.\ pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult { let ws = args.workspace(gctx)?; - gctx.shell().print_json(&ws.current()?.serialized())?; + gctx.shell().print_json( + &ws.current()? + .serialized(gctx.cli_unstable(), ws.unstable_features()), + )?; Ok(()) } diff --git a/src/cargo/core/dependency.rs b/src/cargo/core/dependency.rs index be999c59a64..2ec2aa1a328 100644 --- a/src/cargo/core/dependency.rs +++ b/src/cargo/core/dependency.rs @@ -9,7 +9,7 @@ use std::sync::Arc; use tracing::trace; use crate::core::compiler::{CompileKind, CompileTarget}; -use crate::core::{PackageId, SourceId, Summary}; +use crate::core::{CliUnstable, Feature, Features, PackageId, SourceId, Summary}; use crate::util::errors::CargoResult; use crate::util::interning::InternedString; use crate::util::OptVersionReq; @@ -72,6 +72,12 @@ pub struct SerializedDependency { /// The file system path for a local path dependency. #[serde(skip_serializing_if = "Option::is_none")] path: Option, + + /// `public` flag is unset if `-Zpublic-dependency` is not enabled + /// + /// Once that feature is stabilized, `public` will not need to be `Option` + #[serde(skip_serializing_if = "Option::is_none")] + public: Option, } #[derive(PartialEq, Eq, Hash, Ord, PartialOrd, Clone, Debug, Copy)] @@ -158,7 +164,11 @@ impl Dependency { } } - pub fn serialized(&self) -> SerializedDependency { + pub fn serialized( + &self, + unstable_flags: &CliUnstable, + features: &Features, + ) -> SerializedDependency { SerializedDependency { name: self.package_name(), source: self.source_id(), @@ -172,6 +182,13 @@ impl Dependency { registry: self.registry_id().as_ref().map(|sid| sid.url().to_string()), path: self.source_id().local_path(), artifact: self.inner.artifact.clone(), + public: if unstable_flags.public_dependency + || features.is_enabled(Feature::public_dependency()) + { + Some(self.inner.public) + } else { + None + }, } } diff --git a/src/cargo/core/package.rs b/src/cargo/core/package.rs index 4495aa245ec..ac1bcdc5cb7 100644 --- a/src/cargo/core/package.rs +++ b/src/cargo/core/package.rs @@ -23,7 +23,8 @@ use crate::core::dependency::DepKind; use crate::core::resolver::features::ForceAllTargets; use crate::core::resolver::{HasDevUnits, Resolve}; use crate::core::{ - Dependency, Manifest, PackageId, PackageIdSpec, SerializedDependency, SourceId, Target, + CliUnstable, Dependency, Features, Manifest, PackageId, PackageIdSpec, SerializedDependency, + SourceId, Target, }; use crate::core::{Summary, Workspace}; use crate::sources::source::{MaybePackage, SourceMap}; @@ -190,7 +191,11 @@ impl Package { self.targets().iter().any(|t| t.is_example() || t.is_bin()) } - pub fn serialized(&self) -> SerializedPackage { + pub fn serialized( + &self, + unstable_flags: &CliUnstable, + cargo_features: &Features, + ) -> SerializedPackage { let summary = self.manifest().summary(); let package_id = summary.package_id(); let manmeta = self.manifest().metadata(); @@ -205,7 +210,7 @@ impl Package { .cloned() .collect(); // Convert Vec to Vec - let features = summary + let crate_features = summary .features() .iter() .map(|(k, v)| { @@ -229,10 +234,10 @@ impl Package { dependencies: summary .dependencies() .iter() - .map(Dependency::serialized) + .map(|dep| dep.serialized(unstable_flags, cargo_features)) .collect(), targets, - features, + features: crate_features, manifest_path: self.manifest_path().to_path_buf(), metadata: self.manifest().custom_metadata().cloned(), authors: manmeta.authors.clone(), diff --git a/src/cargo/ops/cargo_output_metadata.rs b/src/cargo/ops/cargo_output_metadata.rs index 1aadc05d77a..246636d1f60 100644 --- a/src/cargo/ops/cargo_output_metadata.rs +++ b/src/cargo/ops/cargo_output_metadata.rs @@ -33,7 +33,10 @@ pub fn output_metadata(ws: &Workspace<'_>, opt: &OutputMetadataOptions) -> Cargo ); } let (packages, resolve) = if opt.no_deps { - let packages = ws.members().map(|pkg| pkg.serialized()).collect(); + let packages = ws + .members() + .map(|pkg| pkg.serialized(ws.gctx().cli_unstable(), ws.unstable_features())) + .collect(); (packages, None) } else { let (packages, resolve) = build_resolve_graph(ws, opt)?; @@ -178,7 +181,7 @@ fn build_resolve_graph( let actual_packages = package_map .into_iter() .filter_map(|(pkg_id, pkg)| node_map.get(&pkg_id).map(|_| pkg)) - .map(|pkg| pkg.serialized()) + .map(|pkg| pkg.serialized(ws.gctx().cli_unstable(), ws.unstable_features())) .collect(); let mr = MetadataResolve { diff --git a/tests/testsuite/metadata.rs b/tests/testsuite/metadata.rs index d8187a907b4..c7bf2af5040 100644 --- a/tests/testsuite/metadata.rs +++ b/tests/testsuite/metadata.rs @@ -777,6 +777,7 @@ fn cargo_metadata_public_private_dependencies_enabled() { "kind": null, "name": "bar", "optional": false, + "public": false, "registry": null, "rename": null, "req": "*", @@ -789,6 +790,7 @@ fn cargo_metadata_public_private_dependencies_enabled() { "kind": null, "name": "baz", "optional": false, + "public": false, "registry": null, "rename": null, "req": "*", @@ -801,6 +803,7 @@ fn cargo_metadata_public_private_dependencies_enabled() { "kind": null, "name": "foobar", "optional": false, + "public": true, "registry": null, "rename": null, "req": "*", From 37834942bbbd87b7a618589c2f7494818e0c2d15 Mon Sep 17 00:00:00 2001 From: Ethan Brierley Date: Fri, 6 Sep 2024 19:18:41 +0100 Subject: [PATCH 4/4] docs: include new `public` flag in metadata schema --- src/doc/man/cargo-metadata.md | 7 ++++++- src/doc/man/generated_txt/cargo-metadata.txt | 7 ++++++- src/doc/src/commands/cargo-metadata.md | 7 ++++++- src/etc/man/cargo-metadata.1 | 7 ++++++- 4 files changed, 24 insertions(+), 4 deletions(-) diff --git a/src/doc/man/cargo-metadata.md b/src/doc/man/cargo-metadata.md index 0205a61e865..431725c352e 100644 --- a/src/doc/man/cargo-metadata.md +++ b/src/doc/man/cargo-metadata.md @@ -123,7 +123,12 @@ The JSON output has the following format: If not specified or null, the dependency is from the default registry (crates.io). */ - "registry": null + "registry": null, + /* (unstable) Boolean flag of whether or not this is a pulbic + dependency. This field is only present when + `-Zpublic-dependency` is enabled. + */ + "public": false } ], /* Array of Cargo targets. */ diff --git a/src/doc/man/generated_txt/cargo-metadata.txt b/src/doc/man/generated_txt/cargo-metadata.txt index 939826531f6..0fb8b8ad820 100644 --- a/src/doc/man/generated_txt/cargo-metadata.txt +++ b/src/doc/man/generated_txt/cargo-metadata.txt @@ -119,7 +119,12 @@ OUTPUT FORMAT If not specified or null, the dependency is from the default registry (crates.io). */ - "registry": null + "registry": null, + /* (unstable) Boolean flag of whether or not this is a pulbic + dependency. This field is only present when + `-Zpublic-dependency` is enabled. + */ + "public": false } ], /* Array of Cargo targets. */ diff --git a/src/doc/src/commands/cargo-metadata.md b/src/doc/src/commands/cargo-metadata.md index d788235cbfb..52068e223c5 100644 --- a/src/doc/src/commands/cargo-metadata.md +++ b/src/doc/src/commands/cargo-metadata.md @@ -123,7 +123,12 @@ The JSON output has the following format: If not specified or null, the dependency is from the default registry (crates.io). */ - "registry": null + "registry": null, + /* (unstable) Boolean flag of whether or not this is a pulbic + dependency. This field is only present when + `-Zpublic-dependency` is enabled. + */ + "public": false } ], /* Array of Cargo targets. */ diff --git a/src/etc/man/cargo-metadata.1 b/src/etc/man/cargo-metadata.1 index 059b66d45da..eb0e274d19e 100644 --- a/src/etc/man/cargo-metadata.1 +++ b/src/etc/man/cargo-metadata.1 @@ -125,7 +125,12 @@ The JSON output has the following format: If not specified or null, the dependency is from the default registry (crates.io). */ - "registry": null + "registry": null, + /* (unstable) Boolean flag of whether or not this is a pulbic + dependency. This field is only present when + `\-Zpublic\-dependency` is enabled. + */ + "public": false } ], /* Array of Cargo targets. */