From 2db5b45dcd093e157113d58b0b9d3d74bef2a08c Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 7 Aug 2018 12:11:10 +0300 Subject: [PATCH 1/2] Move extern_crate_name to Resolve --- src/cargo/core/compiler/build_context/mod.rs | 24 +-------------- src/cargo/core/resolver/resolve.rs | 31 ++++++++++++++++++-- 2 files changed, 30 insertions(+), 25 deletions(-) diff --git a/src/cargo/core/compiler/build_context/mod.rs b/src/cargo/core/compiler/build_context/mod.rs index c0cc12e7019..ed65d6fc3e7 100644 --- a/src/cargo/core/compiler/build_context/mod.rs +++ b/src/cargo/core/compiler/build_context/mod.rs @@ -92,29 +92,7 @@ impl<'a, 'cfg> BuildContext<'a, 'cfg> { } pub fn extern_crate_name(&self, unit: &Unit<'a>, dep: &Unit<'a>) -> CargoResult { - let deps = { - let a = unit.pkg.package_id(); - let b = dep.pkg.package_id(); - if a == b { - &[] - } else { - self.resolve.dependencies_listed(a, b) - } - }; - - let crate_name = dep.target.crate_name(); - let mut names = deps.iter() - .map(|d| d.rename().map(|s| s.as_str()).unwrap_or(&crate_name)); - let name = names.next().unwrap_or(&crate_name); - for n in names { - if n == name { - continue - } - bail!("multiple dependencies listed for the same crate must \ - all have the same name, but the dependency on `{}` \ - is listed as having different names", dep.pkg.package_id()); - } - Ok(name.to_string()) + self.resolve.extern_crate_name(unit.pkg.package_id(), dep.pkg.package_id(), dep.target) } /// Whether a dependency should be compiled for the host or target platform, diff --git a/src/cargo/core/resolver/resolve.rs b/src/cargo/core/resolver/resolve.rs index 975be4de714..2aad009a143 100644 --- a/src/cargo/core/resolver/resolve.rs +++ b/src/cargo/core/resolver/resolve.rs @@ -4,7 +4,7 @@ use std::iter::FromIterator; use url::Url; -use core::{Dependency, PackageId, PackageIdSpec, Summary}; +use core::{Dependency, PackageId, PackageIdSpec, Summary, Target}; use util::Graph; use util::errors::CargoResult; use util::graph::{Edges, Nodes}; @@ -213,7 +213,34 @@ unable to verify that `{0}` is the same as when the lockfile was generated &self.metadata } - pub fn dependencies_listed(&self, from: &PackageId, to: &PackageId) -> &[Dependency] { + pub fn extern_crate_name( + &self, + from: &PackageId, + to: &PackageId, + to_target: &Target, + ) -> CargoResult { + let deps = if from == to { + &[] + } else { + self.dependencies_listed(from, to) + }; + + let crate_name = to_target.crate_name(); + let mut names = deps.iter() + .map(|d| d.rename().map(|s| s.as_str()).unwrap_or(&crate_name)); + let name = names.next().unwrap_or(&crate_name); + for n in names { + if n == name { + continue + } + bail!("multiple dependencies listed for the same crate must \ + all have the same name, but the dependency on `{}` \ + is listed as having different names", to); + } + Ok(name.to_string()) + } + + fn dependencies_listed(&self, from: &PackageId, to: &PackageId) -> &[Dependency] { // We've got a dependency on `from` to `to`, but this dependency edge // may be affected by [replace]. If the `to` package is listed as the // target of a replacement (aka the key of a reverse replacement map) From 39b1f752f6609c760cb908650b45da148354d0a7 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 7 Aug 2018 12:37:29 +0300 Subject: [PATCH 2/2] Add rename info to Cargo metadata Previously, `cargo metadata` exposed dependencies info as a graph of package id without any additional information on edges. However, we do want to add some data to edges, including for example, package renames and `cfg` info. Internally, the edges info is represented as a `Vec`. We could have exposed that directly, but that risks exposing and ossifying an implementation details. So instead we collapse a `Vec` to a single JSON object, which at the moment contains `id` and `rename` info only. It should be possible to add additional fields to that object backwards compatibly. Such representation does not correspond directly to any internal Cargo data structure, but hopefully it shouldn't be to hard to maintain. This representation also does not quite correspond to the "real world", where dependencies are between crate (cargo targets), and not packages. However, because each dep edge is a JSON object, adding a target filter should be possible, which would handle dev-, build-, and potential future bin-specific dependencies backwards-compatibly. --- src/cargo/ops/cargo_output_metadata.rs | 46 ++++-- tests/testsuite/metadata.rs | 216 ++++++++++++++++++++++++- 2 files changed, 246 insertions(+), 16 deletions(-) diff --git a/src/cargo/ops/cargo_output_metadata.rs b/src/cargo/ops/cargo_output_metadata.rs index 50cfd8188ea..fe2443d403e 100644 --- a/src/cargo/ops/cargo_output_metadata.rs +++ b/src/cargo/ops/cargo_output_metadata.rs @@ -1,7 +1,7 @@ use serde::ser; use core::resolver::Resolve; -use core::{Package, PackageId, Workspace}; +use core::{Package, PackageId, Workspace, PackageSet}; use ops::{self, Packages}; use util::CargoResult; @@ -18,7 +18,7 @@ pub struct OutputMetadataOptions { /// Loads the manifest, resolves the dependencies of the project to the concrete /// used versions - considering overrides - and writes all dependencies in a JSON /// format to stdout. -pub fn output_metadata(ws: &Workspace, opt: &OutputMetadataOptions) -> CargoResult { +pub fn output_metadata<'a>(ws: &'a Workspace, opt: &OutputMetadataOptions) -> CargoResult> { if opt.version != VERSION { bail!( "metadata version {} not supported, only {} is currently supported", @@ -33,7 +33,7 @@ pub fn output_metadata(ws: &Workspace, opt: &OutputMetadataOptions) -> CargoResu } } -fn metadata_no_deps(ws: &Workspace, _opt: &OutputMetadataOptions) -> CargoResult { +fn metadata_no_deps<'a>(ws: &'a Workspace, _opt: &OutputMetadataOptions) -> CargoResult> { Ok(ExportInfo { packages: ws.members().cloned().collect(), workspace_members: ws.members().map(|pkg| pkg.package_id().clone()).collect(), @@ -44,7 +44,7 @@ fn metadata_no_deps(ws: &Workspace, _opt: &OutputMetadataOptions) -> CargoResult }) } -fn metadata_full(ws: &Workspace, opt: &OutputMetadataOptions) -> CargoResult { +fn metadata_full<'a>(ws: &'a Workspace, opt: &OutputMetadataOptions) -> CargoResult> { let specs = Packages::All.to_package_id_specs(ws)?; let deps = ops::resolve_ws_precisely( ws, @@ -54,18 +54,18 @@ fn metadata_full(ws: &Workspace, opt: &OutputMetadataOptions) -> CargoResult>>()?; Ok(ExportInfo { packages, workspace_members: ws.members().map(|pkg| pkg.package_id().clone()).collect(), resolve: Some(MetadataResolve { - resolve, + resolve: (package_set, resolve), root: ws.current_opt().map(|pkg| pkg.package_id().clone()), }), target_directory: ws.target_dir().display().to_string(), @@ -75,10 +75,10 @@ fn metadata_full(ws: &Workspace, opt: &OutputMetadataOptions) -> CargoResult { packages: Vec, workspace_members: Vec, - resolve: Option, + resolve: Option>, target_directory: String, version: u32, workspace_root: String, @@ -88,20 +88,27 @@ pub struct ExportInfo { /// The one from lockfile does not fit because it uses a non-standard /// format for `PackageId`s #[derive(Serialize)] -struct MetadataResolve { +struct MetadataResolve<'a> { #[serde(rename = "nodes", serialize_with = "serialize_resolve")] - resolve: Resolve, + resolve: (PackageSet<'a>, Resolve), root: Option, } -fn serialize_resolve(resolve: &Resolve, s: S) -> Result +fn serialize_resolve((package_set, resolve): &(PackageSet, Resolve), s: S) -> Result where S: ser::Serializer, { + #[derive(Serialize)] + struct Dep<'a> { + name: Option, + pkg: &'a PackageId + } + #[derive(Serialize)] struct Node<'a> { id: &'a PackageId, dependencies: Vec<&'a PackageId>, + deps: Vec>, features: Vec<&'a str>, } @@ -109,7 +116,18 @@ where .iter() .map(|id| Node { id, - dependencies: resolve.deps(id).map(|p| p.0).collect(), + dependencies: resolve.deps(id).map(|(pkg, _deps)| pkg).collect(), + deps: resolve.deps(id) + .map(|(pkg, _deps)| { + let name = package_set.get(pkg).ok() + .and_then(|pkg| pkg.targets().iter().find(|t| t.is_lib())) + .and_then(|lib_target| { + resolve.extern_crate_name(id, pkg, lib_target).ok() + }); + + Dep { name, pkg } + }) + .collect(), features: resolve.features_sorted(id), })) } diff --git a/tests/testsuite/metadata.rs b/tests/testsuite/metadata.rs index 46134f4e40d..db3545dd0da 100644 --- a/tests/testsuite/metadata.rs +++ b/tests/testsuite/metadata.rs @@ -55,6 +55,7 @@ fn cargo_metadata_simple() { "nodes": [ { "dependencies": [], + "deps": [], "features": [], "id": "foo 0.5.0 (path+file:[..]foo)" } @@ -148,6 +149,7 @@ crate-type = ["lib", "staticlib"] "nodes": [ { "dependencies": [], + "deps": [], "features": [], "id": "foo 0.5.0 (path+file:[..]foo)" } @@ -231,6 +233,7 @@ optional_feat = [] "nodes": [ { "dependencies": [], + "deps": [], "features": [ "default", "default_feat" @@ -411,6 +414,9 @@ fn cargo_metadata_with_deps_and_version() { "dependencies": [ "bar 0.0.1 (registry+[..])" ], + "deps": [ + { "name": "bar", "pkg": "bar 0.0.1 (registry+[..])" } + ], "features": [], "id": "foo 0.5.0 (path+file:[..]foo)" }, @@ -418,11 +424,15 @@ fn cargo_metadata_with_deps_and_version() { "dependencies": [ "baz 0.0.1 (registry+[..])" ], + "deps": [ + { "name": "baz", "pkg": "baz 0.0.1 (registry+[..])" } + ], "features": [], "id": "bar 0.0.1 (registry+[..])" }, { "dependencies": [], + "deps": [], "features": [], "id": "baz 0.0.1 (registry+[..])" } @@ -506,7 +516,8 @@ name = "ex" { "id": "foo 0.1.0 (path+file:[..]foo)", "features": [], - "dependencies": [] + "dependencies": [], + "deps": [] } ] }, @@ -588,7 +599,8 @@ crate-type = ["rlib", "dylib"] { "id": "foo 0.1.0 (path+file:[..]foo)", "features": [], - "dependencies": [] + "dependencies": [], + "deps": [] } ] }, @@ -688,11 +700,13 @@ fn workspace_metadata() { "nodes": [ { "dependencies": [], + "deps": [], "features": [], "id": "baz 0.5.0 (path+file:[..]baz)" }, { "dependencies": [], + "deps": [], "features": [], "id": "bar 0.5.0 (path+file:[..]bar)" } @@ -1127,6 +1141,7 @@ fn cargo_metadata_path_to_cargo_toml_project() { "nodes": [ { "dependencies": [], + "deps": [], "features": [], "id": "bar 0.5.0 ([..])" } @@ -1207,6 +1222,7 @@ fn package_edition_2018() { "nodes": [ { "dependencies": [], + "deps": [], "features": [], "id": "foo 0.1.0 (path+file:[..])" } @@ -1302,6 +1318,7 @@ fn target_edition_2018() { "nodes": [ { "dependencies": [], + "deps": [], "features": [], "id": "foo 0.1.0 (path+file:[..])" } @@ -1319,3 +1336,198 @@ fn target_edition_2018() { ), ); } + +#[test] +fn rename_dependency() { + Package::new("bar", "0.1.0").publish(); + Package::new("bar", "0.2.0").publish(); + + let p = project() + .file( + "Cargo.toml", + r#" + cargo-features = ["rename-dependency"] + + [project] + name = "foo" + version = "0.0.1" + authors = [] + + [dependencies] + bar = { version = "0.1.0" } + baz = { version = "0.2.0", package = "bar" } + "#, + ) + .file("src/lib.rs", "extern crate bar; extern crate baz;") + .build(); + + assert_that( + p.cargo("metadata").masquerade_as_nightly_cargo(), + execs().with_json(r#" +{ + "packages": [ + { + "authors": [], + "categories": [], + "dependencies": [ + { + "features": [], + "kind": null, + "name": "bar", + "optional": false, + "rename": null, + "req": "^0.1.0", + "source": "registry+https://github.com/rust-lang/crates.io-index", + "target": null, + "uses_default_features": true + }, + { + "features": [], + "kind": null, + "name": "bar", + "optional": false, + "rename": "baz", + "req": "^0.2.0", + "source": "registry+https://github.com/rust-lang/crates.io-index", + "target": null, + "uses_default_features": true + } + ], + "description": null, + "edition": "2015", + "features": {}, + "id": "foo 0.0.1[..]", + "keywords": [], + "license": null, + "license_file": null, + "manifest_path": "[..]", + "metadata": null, + "name": "foo", + "readme": null, + "repository": null, + "source": null, + "targets": [ + { + "crate_types": [ + "lib" + ], + "edition": "2015", + "kind": [ + "lib" + ], + "name": "foo", + "src_path": "[..]" + } + ], + "version": "0.0.1" + }, + { + "authors": [], + "categories": [], + "dependencies": [], + "description": null, + "edition": "2015", + "features": {}, + "id": "bar 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", + "keywords": [], + "license": null, + "license_file": null, + "manifest_path": "[..]", + "metadata": null, + "name": "bar", + "readme": null, + "repository": null, + "source": "registry+https://github.com/rust-lang/crates.io-index", + "targets": [ + { + "crate_types": [ + "lib" + ], + "edition": "2015", + "kind": [ + "lib" + ], + "name": "bar", + "src_path": "[..]" + } + ], + "version": "0.1.0" + }, + { + "authors": [], + "categories": [], + "dependencies": [], + "description": null, + "edition": "2015", + "features": {}, + "id": "bar 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)", + "keywords": [], + "license": null, + "license_file": null, + "manifest_path": "[..]", + "metadata": null, + "name": "bar", + "readme": null, + "repository": null, + "source": "registry+https://github.com/rust-lang/crates.io-index", + "targets": [ + { + "crate_types": [ + "lib" + ], + "edition": "2015", + "kind": [ + "lib" + ], + "name": "bar", + "src_path": "[..]" + } + ], + "version": "0.2.0" + } + ], + "resolve": { + "nodes": [ + { + "dependencies": [], + "deps": [], + "features": [], + "id": "bar 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)" + }, + { + "dependencies": [], + "deps": [], + "features": [], + "id": "bar 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)" + }, + { + "dependencies": [ + "bar 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", + "bar 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)" + ], + "deps": [ + { + "name": "bar", + "pkg": "bar 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)" + }, + { + "name": "baz", + "pkg": "bar 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)" + } + ], + "features": [], + "id": "foo 0.0.1[..]" + } + ], + "root": "foo 0.0.1[..]" + }, + "target_directory": "[..]", + "version": 1, + "workspace_members": [ + "foo 0.0.1[..]" + ], + "workspace_root": "[..]" +}"#, + ), + ); +}