diff --git a/src/cargo/core/compiler/artifact.rs b/src/cargo/core/compiler/artifact.rs index 148b0feb109..b1391eccdb0 100644 --- a/src/cargo/core/compiler/artifact.rs +++ b/src/cargo/core/compiler/artifact.rs @@ -1,9 +1,10 @@ /// Generate artifact information from unit dependencies for configuring the compiler environment. use crate::core::compiler::unit_graph::UnitDep; use crate::core::compiler::{Context, CrateType, FileFlavor, Unit}; -use crate::core::TargetKind; +use crate::core::dependency::ArtifactKind; +use crate::core::{Dependency, Target, TargetKind}; use crate::CargoResult; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::ffi::OsString; /// Return all environment variables for the given unit-dependencies @@ -55,3 +56,42 @@ fn unit_artifact_type_name_upper(unit: &Unit) -> &'static str { invalid => unreachable!("BUG: artifacts cannot be of type {:?}", invalid), } } + +/// Given a dependency with an artifact `artifact_dep` and a set of available `targets` +/// of its package, find a target for each kind of artifacts that are to be built. +/// +/// Failure to match any target results in an error mentioning the parent manifests +/// `parent_package` name. +pub(crate) fn match_artifacts_kind_with_targets<'t, 'd>( + artifact_dep: &'d Dependency, + targets: &'t [Target], + parent_package: &str, +) -> CargoResult> { + let mut out = HashSet::new(); + let artifact_requirements = artifact_dep.artifact().expect("artifact present"); + for artifact_kind in artifact_requirements.kinds() { + let mut extend = |kind, filter: &dyn Fn(&&Target) -> bool| { + let mut iter = targets.iter().filter(filter).peekable(); + let found = iter.peek().is_some(); + out.extend(std::iter::repeat(kind).zip(iter)); + found + }; + let found = match artifact_kind { + ArtifactKind::Cdylib => extend(artifact_kind, &|t| t.is_cdylib()), + ArtifactKind::Staticlib => extend(artifact_kind, &|t| t.is_staticlib()), + ArtifactKind::AllBinaries => extend(artifact_kind, &|t| t.is_bin()), + ArtifactKind::SelectedBinary(bin_name) => extend(artifact_kind, &|t| { + t.is_bin() && t.name() == bin_name.as_str() + }), + }; + if !found { + anyhow::bail!( + "dependency `{}` in package `{}` requires a `{}` artifact to be present.", + artifact_dep.name_in_toml(), + parent_package, + artifact_kind + ); + } + } + Ok(out) +} diff --git a/src/cargo/core/compiler/unit_dependencies.rs b/src/cargo/core/compiler/unit_dependencies.rs index 64356d47b4b..68fc1e5196d 100644 --- a/src/cargo/core/compiler/unit_dependencies.rs +++ b/src/cargo/core/compiler/unit_dependencies.rs @@ -19,11 +19,12 @@ use std::collections::{HashMap, HashSet}; use log::trace; +use crate::core::compiler::artifact::match_artifacts_kind_with_targets; use crate::core::compiler::unit_graph::{UnitDep, UnitGraph}; use crate::core::compiler::{ CompileKind, CompileMode, CrateType, RustcTargetData, Unit, UnitInterner, }; -use crate::core::dependency::{Artifact, ArtifactKind, ArtifactTarget, DepKind}; +use crate::core::dependency::{Artifact, ArtifactTarget, DepKind}; use crate::core::profiles::{Profile, Profiles, UnitFor}; use crate::core::resolver::features::{FeaturesFor, ResolvedFeatures}; use crate::core::resolver::Resolve; @@ -557,6 +558,7 @@ fn artifact_targets_to_unit_deps( let ret = match_artifacts_kind_with_targets(dep, artifact_pkg.targets(), parent.pkg.name().as_str())? .into_iter() + .map(|(_artifact_kind, target)| target) .flat_map(|target| { // We split target libraries into individual units, even though rustc is able // to produce multiple kinds in an single invocation for the sole reason that @@ -598,45 +600,6 @@ fn artifact_targets_to_unit_deps( Ok(ret) } -/// Given a dependency with an artifact `artifact_dep` and a set of available `targets` -/// of its package, find a target for each kind of artifacts that are to be built. -/// -/// Failure to match any target results in an error mentioning the parent manifests -/// `parent_package` name. -fn match_artifacts_kind_with_targets<'a>( - artifact_dep: &Dependency, - targets: &'a [Target], - parent_package: &str, -) -> CargoResult> { - let mut out = HashSet::new(); - let artifact_requirements = artifact_dep.artifact().expect("artifact present"); - for artifact_kind in artifact_requirements.kinds() { - let mut extend = |filter: &dyn Fn(&&Target) -> bool| { - let mut iter = targets.iter().filter(filter).peekable(); - let found = iter.peek().is_some(); - out.extend(iter); - found - }; - let found = match artifact_kind { - ArtifactKind::Cdylib => extend(&|t| t.is_cdylib()), - ArtifactKind::Staticlib => extend(&|t| t.is_staticlib()), - ArtifactKind::AllBinaries => extend(&|t| t.is_bin()), - ArtifactKind::SelectedBinary(bin_name) => { - extend(&|t| t.is_bin() && t.name() == bin_name.as_str()) - } - }; - if !found { - anyhow::bail!( - "dependency `{}` in package `{}` requires a `{}` artifact to be present.", - artifact_dep.name_in_toml(), - parent_package, - artifact_kind - ); - } - } - Ok(out) -} - /// Returns the dependencies necessary to document a package. fn compute_deps_doc( unit: &Unit, diff --git a/src/cargo/core/dependency.rs b/src/cargo/core/dependency.rs index b86d2e93194..db4fdd07393 100644 --- a/src/cargo/core/dependency.rs +++ b/src/cargo/core/dependency.rs @@ -566,10 +566,8 @@ impl ser::Serialize for ArtifactKind { S: ser::Serializer, { let out: Cow<'_, str> = match *self { - ArtifactKind::AllBinaries => "bin".into(), - ArtifactKind::Staticlib => "staticlib".into(), - ArtifactKind::Cdylib => "cdylib".into(), ArtifactKind::SelectedBinary(name) => format!("bin:{}", name.as_str()).into(), + _ => self.crate_type().into(), }; out.serialize(s) } @@ -578,15 +576,24 @@ impl ser::Serialize for ArtifactKind { impl fmt::Display for ArtifactKind { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.write_str(match self { - ArtifactKind::Cdylib => "cdylib", - ArtifactKind::Staticlib => "staticlib", - ArtifactKind::AllBinaries => "bin", - ArtifactKind::SelectedBinary(bin_name) => return write!(f, "bin:{}", bin_name), + ArtifactKind::SelectedBinary(bin_name) => return write!(f, "bin:{bin_name}"), + _ => self.crate_type(), }) } } impl ArtifactKind { + /// Returns a string of crate type of the artifact being built. + /// + /// Note that the name of `SelectedBinary` would be dropped and displayed as `bin`. + pub fn crate_type(&self) -> &'static str { + match self { + ArtifactKind::AllBinaries | ArtifactKind::SelectedBinary(_) => "bin", + ArtifactKind::Cdylib => "cdylib", + ArtifactKind::Staticlib => "staticlib", + } + } + fn parse(kind: &str) -> CargoResult { Ok(match kind { "bin" => ArtifactKind::AllBinaries, diff --git a/src/cargo/ops/cargo_output_metadata.rs b/src/cargo/ops/cargo_output_metadata.rs index d8a86eae778..c71cfaa1e2e 100644 --- a/src/cargo/ops/cargo_output_metadata.rs +++ b/src/cargo/ops/cargo_output_metadata.rs @@ -1,8 +1,9 @@ +use crate::core::compiler::artifact::match_artifacts_kind_with_targets; use crate::core::compiler::{CompileKind, RustcTargetData}; use crate::core::dependency::DepKind; use crate::core::package::SerializedPackage; use crate::core::resolver::{features::CliFeatures, HasDevUnits, Resolve}; -use crate::core::{Dependency, Package, PackageId, Workspace}; +use crate::core::{Package, PackageId, Workspace}; use crate::ops::{self, Packages}; use crate::util::interning::InternedString; use crate::util::CargoResult; @@ -81,6 +82,8 @@ struct MetadataResolveNode { #[derive(Serialize)] struct Dep { + // TODO(bindeps): after -Zbindeps gets stabilized, + // mark this field as deprecated in the help manual of cargo-metadata name: InternedString, pkg: PackageId, dep_kinds: Vec, @@ -90,15 +93,27 @@ struct Dep { struct DepKindInfo { kind: DepKind, target: Option, -} -impl From<&Dependency> for DepKindInfo { - fn from(dep: &Dependency) -> DepKindInfo { - DepKindInfo { - kind: dep.kind(), - target: dep.platform().cloned(), - } - } + // vvvvv The fields below are introduced for `-Z bindeps`. + /// What the manifest calls the crate. + /// + /// A renamed dependency will show the rename instead of original name. + // TODO(bindeps): Remove `Option` after -Zbindeps get stabilized. + #[serde(skip_serializing_if = "Option::is_none")] + extern_name: Option, + /// Artifact's crate type, e.g. staticlib, cdylib, bin... + #[serde(skip_serializing_if = "Option::is_none")] + artifact: Option<&'static str>, + /// Equivalent to `{ target = "…" }` in an artifact dependency requirement. + /// + /// * If the target points to a custom target JSON file, the path will be absolute. + /// * If the target is a build assumed target `{ target = "target" }`, it will show as ``. + #[serde(skip_serializing_if = "Option::is_none")] + compile_target: Option, + /// Executable name for an artifact binary dependency. + #[serde(skip_serializing_if = "Option::is_none")] + bin_name: Option, + // ^^^^^ The fields above are introduced for `-Z bindeps`. } /// Builds the resolve graph as it will be displayed to the user. @@ -149,7 +164,7 @@ fn build_resolve_graph( &package_map, &target_data, &requested_kinds, - ); + )?; } // Get a Vec of Packages. let actual_packages = package_map @@ -172,9 +187,9 @@ fn build_resolve_graph_r( package_map: &BTreeMap, target_data: &RustcTargetData<'_>, requested_kinds: &[CompileKind], -) { +) -> CargoResult<()> { if node_map.contains_key(&pkg_id) { - return; + return Ok(()); } // This normalizes the IDs so that they are consistent between the // `packages` array and the `resolve` map. This is a bit of a hack to @@ -193,9 +208,9 @@ fn build_resolve_graph_r( let normalize_id = |id| -> PackageId { *package_map.get_key_value(&id).unwrap().0 }; let features = resolve.features(pkg_id).to_vec(); - let deps: Vec = resolve - .deps(pkg_id) - .filter(|(_dep_id, deps)| { + let deps = { + let mut dep_metadatas = Vec::new(); + let iter = resolve.deps(pkg_id).filter(|(_dep_id, deps)| { if requested_kinds == [CompileKind::Host] { true } else { @@ -204,27 +219,109 @@ fn build_resolve_graph_r( .any(|dep| target_data.dep_platform_activated(dep, *kind)) }) } - }) - .filter_map(|(dep_id, deps)| { - let mut dep_kinds: Vec<_> = deps.iter().map(DepKindInfo::from).collect(); + }); + for (dep_id, deps) in iter { + let mut dep_kinds = Vec::new(); + + let targets = package_map[&dep_id].targets(); + + // Try to get the extern name for lib, or crate name for bins. + let extern_name = |target| { + resolve + .extern_crate_name_and_dep_name(pkg_id, dep_id, target) + .map(|(ext_crate_name, _)| ext_crate_name) + }; + + let lib_target = targets.iter().find(|t| t.is_lib()); + + for dep in deps.iter() { + if let Some(target) = lib_target { + // When we do have a library target, include them in deps if... + let included = match dep.artifact() { + // it is not an artifact dep at all + None => true, + // it is also an artifact dep with `{ …, lib = true }` + Some(a) if a.is_lib() => true, + _ => false, + }; + // TODO(bindeps): Cargo shouldn't have `extern_name` field + // if the user is not using -Zbindeps. + // Remove this condition ` after -Zbindeps gets stabilized. + let extern_name = if dep.artifact().is_some() { + Some(extern_name(target)?) + } else { + None + }; + if included { + dep_kinds.push(DepKindInfo { + kind: dep.kind(), + target: dep.platform().cloned(), + extern_name, + artifact: None, + compile_target: None, + bin_name: None, + }); + } + } + + // No need to proceed if there is no artifact dependency. + let Some(artifact_requirements) = dep.artifact() else { + continue; + }; + + let compile_target = match artifact_requirements.target() { + Some(t) => t + .to_compile_target() + .map(|t| t.rustc_target()) + // Given that Cargo doesn't know which target it should resolve to, + // when an artifact dep is specified with { target = "target" }, + // keep it with a special "" string, + .or_else(|| Some(InternedString::new(""))), + None => None, + }; + + let target_set = + match_artifacts_kind_with_targets(dep, targets, pkg_id.name().as_str())?; + dep_kinds.reserve(target_set.len()); + for (kind, target) in target_set.into_iter() { + dep_kinds.push(DepKindInfo { + kind: dep.kind(), + target: dep.platform().cloned(), + extern_name: extern_name(target).ok(), + artifact: Some(kind.crate_type()), + compile_target, + bin_name: target.is_bin().then(|| target.name().to_string()), + }) + } + } + dep_kinds.sort(); - package_map - .get(&dep_id) - .and_then(|pkg| pkg.targets().iter().find(|t| t.is_lib())) - .and_then(|lib_target| { - resolve - .extern_crate_name_and_dep_name(pkg_id, dep_id, lib_target) - .map(|(ext_crate_name, _)| ext_crate_name) - .ok() - }) - .map(|name| Dep { - name, - pkg: normalize_id(dep_id), + + let pkg = normalize_id(dep_id); + + let dep = match (lib_target, dep_kinds.len()) { + (Some(target), _) => Dep { + name: extern_name(target)?, + pkg, dep_kinds, - }) - }) - .collect(); - let dumb_deps: Vec = deps.iter().map(|dep| normalize_id(dep.pkg)).collect(); + }, + // No lib target exists but contains artifact deps. + (None, 1..) => Dep { + name: InternedString::new(""), + pkg, + dep_kinds, + }, + // No lib or artifact dep exists. + // Ususally this mean parent depending on non-lib bin crate. + (None, _) => continue, + }; + + dep_metadatas.push(dep) + } + dep_metadatas + }; + + let dumb_deps: Vec = deps.iter().map(|dep| dep.pkg).collect(); let to_visit = dumb_deps.clone(); let node = MetadataResolveNode { id: normalize_id(pkg_id), @@ -241,6 +338,8 @@ fn build_resolve_graph_r( package_map, target_data, requested_kinds, - ); + )?; } + + Ok(()) } diff --git a/tests/testsuite/metadata.rs b/tests/testsuite/metadata.rs index 92e63b5f15e..441630bec43 100644 --- a/tests/testsuite/metadata.rs +++ b/tests/testsuite/metadata.rs @@ -1152,17 +1152,17 @@ fn workspace_metadata_with_dependencies_and_resolve() { name = "bar" version = "0.5.0" authors = [] - + [build-dependencies] artifact = { path = "../artifact/", artifact = "bin", target = "target" } bin-only-artifact = { path = "../bin-only-artifact/", artifact = "bin", target = "$ALT_TARGET" } non-artifact = { path = "../non-artifact" } - + [dependencies] artifact = { path = "../artifact/", artifact = ["cdylib", "staticlib", "bin:baz-name"], lib = true, target = "$ALT_TARGET" } bin-only-artifact = { path = "../bin-only-artifact/", artifact = "bin:a-name" } non-artifact = { path = "../non-artifact" } - + [dev-dependencies] artifact = { path = "../artifact/" } non-artifact = { path = "../non-artifact" } @@ -1617,12 +1617,36 @@ fn workspace_metadata_with_dependencies_and_resolve() { { "dependencies": [ "artifact 0.5.0 (path+file://[..]/foo/artifact)", + "bin-only-artifact 0.5.0 (path+file://[..]/foo/bin-only-artifact)", "non-artifact 0.5.0 (path+file://[..]/foo/non-artifact)" ], "deps": [ { "dep_kinds": [ { + "extern_name": "artifact", + "kind": null, + "target": null + }, + { + "artifact": "cdylib", + "compile_target": "wasm32-unknown-unknown", + "extern_name": "artifact", + "kind": null, + "target": null + }, + { + "artifact": "staticlib", + "compile_target": "wasm32-unknown-unknown", + "extern_name": "artifact", + "kind": null, + "target": null + }, + { + "artifact": "bin", + "bin_name": "baz-name", + "compile_target": "wasm32-unknown-unknown", + "extern_name": "baz_name", "kind": null, "target": null }, @@ -1631,6 +1655,18 @@ fn workspace_metadata_with_dependencies_and_resolve() { "target": null }, { + "artifact": "bin", + "bin_name": "bar-name", + "compile_target": "", + "extern_name": "bar_name", + "kind": "build", + "target": null + }, + { + "artifact": "bin", + "bin_name": "baz-name", + "compile_target": "", + "extern_name": "baz_name", "kind": "build", "target": null } @@ -1638,6 +1674,42 @@ fn workspace_metadata_with_dependencies_and_resolve() { "name": "artifact", "pkg": "artifact 0.5.0 (path+file://[..]/foo/artifact)" }, + { + "dep_kinds": [ + { + "artifact": "bin", + "bin_name": "a-name", + "extern_name": "a_name", + "kind": null, + "target": null + }, + { + "artifact": "bin", + "bin_name": "b-name", + "extern_name": "b_name", + "kind": "dev", + "target": null + }, + { + "artifact": "bin", + "bin_name": "a-name", + "compile_target": "wasm32-unknown-unknown", + "extern_name": "a_name", + "kind": "build", + "target": null + }, + { + "artifact": "bin", + "bin_name": "b-name", + "compile_target": "wasm32-unknown-unknown", + "extern_name": "b_name", + "kind": "build", + "target": null + } + ], + "name": "", + "pkg": "bin-only-artifact 0.5.0 (path+file://[..]/foo/bin-only-artifact)" + }, { "dep_kinds": [ { @@ -1778,6 +1850,66 @@ Caused by: .run(); } +#[cargo_test] +fn cargo_metadata_with_invalid_artifact_deps() { + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.5.0" + + [dependencies] + artifact = { path = "artifact", artifact = "bin:notfound" } + "#, + ) + .file("src/lib.rs", "") + .file("artifact/Cargo.toml", &basic_bin_manifest("artifact")) + .file("artifact/src/main.rs", "fn main() {}") + .build(); + + p.cargo("metadata -Z bindeps") + .masquerade_as_nightly_cargo(&["bindeps"]) + .with_status(101) + .with_stderr( + "\ +[WARNING] please specify `--format-version` flag explicitly to avoid compatibility problems +[ERROR] dependency `artifact` in package `foo` requires a `bin:notfound` artifact to be present.", + ) + .run(); +} + +#[cargo_test] +fn cargo_metadata_with_invalid_duplicate_renamed_deps() { + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.5.0" + + [dependencies] + bar = { path = "bar" } + baz = { path = "bar", package = "bar" } + "#, + ) + .file("src/lib.rs", "") + .file("bar/Cargo.toml", &basic_lib_manifest("bar")) + .file("bar/src/lib.rs", "") + .build(); + + p.cargo("metadata") + .with_status(101) + .with_stderr( + "\ +[WARNING] please specify `--format-version` flag explicitly to avoid compatibility problems +[ERROR] the crate `foo v0.5.0 ([..])` depends on crate `bar v0.5.0 ([..])` multiple times with different names", + ) + .run(); +} + const MANIFEST_OUTPUT: &str = r#" { "packages": [{