From 79942fea1bb7a480f6eebd6cd1f8a04ac9f45a0b Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 17 Jan 2018 11:33:25 -0800 Subject: [PATCH] Implement renaming dependencies in the manifest This commit implements a new unstable feature for manifests which allows renaming a crate when depending on it. For example you can do: ```toml cargo-features = ["dependencies-as"] ... [dependencies] foo = "0.1" bar = { version = "0.1", registry = "custom", package = "foo" } baz = { git = "https://github.com/foo/bar", package = "foo" } ``` Here three crates will be imported but they'll be made available to the Rust source code via the names `foo` (crates.io), `bar` (the custom registry), and `baz` (the git dependency). The *package* name, however, will be `foo` for all of them. In other words the git repository here would be searched for a crate called `foo`. For example: ```rust extern crate foo; // crates.io extern crate bar; // registry `custom` extern crate baz; // git repository ``` The intention here is to enable a few use cases: * Enable depending on the same named crate from different registries * Allow depending on multiple versions of a crate from one registry * Removing the need for `extern crate foo as bar` syntactically in Rust source Currently I don't think we're ready to stabilize this so it's just a nightly feature, but I'm hoping we can continue to iterate on it! --- src/cargo/core/dependency.rs | 13 ++++ src/cargo/core/features.rs | 3 + src/cargo/ops/cargo_rustc/mod.rs | 34 ++++++--- src/cargo/util/toml/mod.rs | 80 +++++++++++++-------- tests/metadata.rs | 6 +- tests/rename-deps.rs | 120 +++++++++++++++++++++++++++++++ 6 files changed, 216 insertions(+), 40 deletions(-) create mode 100644 tests/rename-deps.rs diff --git a/src/cargo/core/dependency.rs b/src/cargo/core/dependency.rs index ca944dc19ad..aa24554e771 100644 --- a/src/cargo/core/dependency.rs +++ b/src/cargo/core/dependency.rs @@ -27,6 +27,7 @@ struct Inner { specified_req: bool, kind: Kind, only_match_name: bool, + rename: Option, optional: bool, default_features: bool, @@ -49,6 +50,7 @@ struct SerializedDependency<'a> { source: &'a SourceId, req: String, kind: Kind, + rename: Option<&'a str>, optional: bool, uses_default_features: bool, @@ -69,6 +71,7 @@ impl ser::Serialize for Dependency { uses_default_features: self.uses_default_features(), features: self.features(), target: self.platform(), + rename: self.rename(), }.serialize(s) } } @@ -182,6 +185,7 @@ impl Dependency { default_features: true, specified_req: false, platform: None, + rename: None, }), } } @@ -221,6 +225,10 @@ impl Dependency { self.inner.platform.as_ref() } + pub fn rename(&self) -> Option<&str> { + self.inner.rename.as_ref().map(|s| &**s) + } + pub fn set_kind(&mut self, kind: Kind) -> &mut Dependency { Rc::make_mut(&mut self.inner).kind = kind; self @@ -261,6 +269,11 @@ impl Dependency { self } + pub fn set_rename(&mut self, rename: &str) -> &mut Dependency { + Rc::make_mut(&mut self.inner).rename = Some(rename.to_string()); + self + } + /// Lock this dependency to depending on the specified package id pub fn lock_to(&mut self, id: &PackageId) -> &mut Dependency { assert_eq!(self.inner.source_id, *id.source_id()); diff --git a/src/cargo/core/features.rs b/src/cargo/core/features.rs index 323366b9bce..74437f7c998 100644 --- a/src/cargo/core/features.rs +++ b/src/cargo/core/features.rs @@ -159,6 +159,9 @@ features! { // Using epochs [unstable] epoch: bool, + + // Renaming a package in the manifest via the `package` key + [unstable] rename_dependency: bool, } } diff --git a/src/cargo/ops/cargo_rustc/mod.rs b/src/cargo/ops/cargo_rustc/mod.rs index b3294857608..ec97e81b2cc 100644 --- a/src/cargo/ops/cargo_rustc/mod.rs +++ b/src/cargo/ops/cargo_rustc/mod.rs @@ -947,12 +947,12 @@ Cargo.toml. This warning might turn into a hard error in the future.", } } - for unit in dep_targets { - if unit.profile.run_custom_build { - cmd.env("OUT_DIR", &cx.build_script_out_dir(&unit)); + for dep in dep_targets { + if dep.profile.run_custom_build { + cmd.env("OUT_DIR", &cx.build_script_out_dir(&dep)); } - if unit.target.linkable() && !unit.profile.doc { - link_to(cmd, cx, &unit)?; + if dep.target.linkable() && !dep.profile.doc { + link_to(cmd, cx, &unit, &dep)?; } } @@ -960,15 +960,31 @@ Cargo.toml. This warning might turn into a hard error in the future.", fn link_to<'a, 'cfg>(cmd: &mut ProcessBuilder, cx: &mut Context<'a, 'cfg>, - unit: &Unit<'a>) -> CargoResult<()> { - for &(ref dst, _, file_type) in cx.target_filenames(unit)?.iter() { + current: &Unit<'a>, + dep: &Unit<'a>) -> CargoResult<()> { + for &(ref dst, _, file_type) in cx.target_filenames(dep)?.iter() { if file_type != TargetFileType::Linkable { continue } let mut v = OsString::new(); - v.push(&unit.target.crate_name()); + + // Unfortunately right now Cargo doesn't have a great way to get a + // 1:1 mapping of entries in `dependencies()` to the actual crate + // we're depending on. Instead we're left to do some guesswork here + // to figure out what `Dependency` the `dep` unit corresponds to in + // `current` to see if we're renaming it. + // + // This I believe mostly works out for now, but we'll likely want + // to tighten up this in the future. + let name = current.pkg.dependencies() + .iter() + .filter(|d| d.matches_ignoring_source(dep.pkg.summary())) + .filter_map(|d| d.rename()) + .next(); + + v.push(name.unwrap_or(&dep.target.crate_name())); v.push("="); - v.push(cx.out_dir(unit)); + v.push(cx.out_dir(dep)); v.push(&path::MAIN_SEPARATOR.to_string()); v.push(&dst.file_name().unwrap()); cmd.arg("--extern").arg(&v); diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index c33aaef93e9..633f2bf1ff8 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -142,7 +142,7 @@ type TomlBenchTarget = TomlTarget; #[serde(untagged)] pub enum TomlDependency { Simple(String), - Detailed(DetailedTomlDependency) + Detailed(DetailedTomlDependency), } impl<'de> de::Deserialize<'de> for TomlDependency { @@ -193,6 +193,7 @@ pub struct DetailedTomlDependency { default_features: Option, #[serde(rename = "default_features")] default_features2: Option, + package: Option, } #[derive(Debug, Deserialize, Serialize)] @@ -917,16 +918,28 @@ impl TomlDependency { cx: &mut Context, kind: Option) -> CargoResult { - let details = match *self { - TomlDependency::Simple(ref version) => DetailedTomlDependency { - version: Some(version.clone()), - .. Default::default() - }, - TomlDependency::Detailed(ref details) => details.clone(), - }; + match *self { + TomlDependency::Simple(ref version) => { + DetailedTomlDependency { + version: Some(version.clone()), + ..Default::default() + }.to_dependency(name, cx, kind) + } + TomlDependency::Detailed(ref details) => { + details.to_dependency(name, cx, kind) + } + } + } +} - if details.version.is_none() && details.path.is_none() && - details.git.is_none() { +impl DetailedTomlDependency { + fn to_dependency(&self, + name: &str, + cx: &mut Context, + kind: Option) + -> CargoResult { + if self.version.is_none() && self.path.is_none() && + self.git.is_none() { let msg = format!("dependency ({}) specified without \ providing a local path, Git repository, or \ version to use. This will be considered an \ @@ -934,11 +947,11 @@ impl TomlDependency { cx.warnings.push(msg); } - if details.git.is_none() { + if self.git.is_none() { let git_only_keys = [ - (&details.branch, "branch"), - (&details.tag, "tag"), - (&details.rev, "rev") + (&self.branch, "branch"), + (&self.tag, "tag"), + (&self.rev, "rev") ]; for &(key, key_name) in &git_only_keys { @@ -951,7 +964,7 @@ impl TomlDependency { } } - let registry_id = match details.registry { + let registry_id = match self.registry { Some(ref registry) => { cx.features.require(Feature::alternative_registries())?; SourceId::alt_registry(cx.config, registry)? @@ -960,10 +973,10 @@ impl TomlDependency { }; let new_source_id = match ( - details.git.as_ref(), - details.path.as_ref(), - details.registry.as_ref(), - details.registry_index.as_ref(), + self.git.as_ref(), + self.path.as_ref(), + self.registry.as_ref(), + self.registry_index.as_ref(), ) { (Some(_), _, Some(_), _) | (Some(_), _, _, Some(_))=> bail!("dependency ({}) specification is ambiguous. \ @@ -978,7 +991,7 @@ impl TomlDependency { cx.warnings.push(msg) } - let n_details = [&details.branch, &details.tag, &details.rev] + let n_details = [&self.branch, &self.tag, &self.rev] .iter() .filter(|d| d.is_some()) .count(); @@ -990,9 +1003,9 @@ impl TomlDependency { cx.warnings.push(msg) } - let reference = details.branch.clone().map(GitReference::Branch) - .or_else(|| details.tag.clone().map(GitReference::Tag)) - .or_else(|| details.rev.clone().map(GitReference::Rev)) + let reference = self.branch.clone().map(GitReference::Branch) + .or_else(|| self.tag.clone().map(GitReference::Tag)) + .or_else(|| self.rev.clone().map(GitReference::Rev)) .unwrap_or_else(|| GitReference::Branch("master".to_string())); let loc = git.to_url()?; SourceId::for_git(&loc, reference)? @@ -1023,24 +1036,33 @@ impl TomlDependency { (None, None, None, None) => SourceId::crates_io(cx.config)?, }; - let version = details.version.as_ref().map(|v| &v[..]); + let (pkg_name, rename) = match self.package { + Some(ref s) => (&s[..], Some(name)), + None => (name, None), + }; + + let version = self.version.as_ref().map(|v| &v[..]); let mut dep = match cx.pkgid { Some(id) => { - Dependency::parse(name, version, &new_source_id, + Dependency::parse(pkg_name, version, &new_source_id, id, cx.config)? } None => Dependency::parse_no_deprecated(name, version, &new_source_id)?, }; - dep.set_features(details.features.unwrap_or_default()) - .set_default_features(details.default_features - .or(details.default_features2) + dep.set_features(self.features.clone().unwrap_or_default()) + .set_default_features(self.default_features + .or(self.default_features2) .unwrap_or(true)) - .set_optional(details.optional.unwrap_or(false)) + .set_optional(self.optional.unwrap_or(false)) .set_platform(cx.platform.clone()) .set_registry_id(®istry_id); if let Some(kind) = kind { dep.set_kind(kind); } + if let Some(rename) = rename { + cx.features.require(Feature::rename_dependency())?; + dep.set_rename(rename); + } Ok(dep) } } diff --git a/tests/metadata.rs b/tests/metadata.rs index 71c3545ea95..b3c99fd1746 100644 --- a/tests/metadata.rs +++ b/tests/metadata.rs @@ -193,7 +193,8 @@ fn cargo_metadata_with_deps_and_version() { "req": "^0.0.1", "source": "registry+[..]", "target": null, - "uses_default_features": true + "uses_default_features": true, + "rename": null } ], "features": {}, @@ -228,7 +229,8 @@ fn cargo_metadata_with_deps_and_version() { "req": "*", "source": "registry+[..]", "target": null, - "uses_default_features": true + "uses_default_features": true, + "rename": null } ], "features": {}, diff --git a/tests/rename-deps.rs b/tests/rename-deps.rs new file mode 100644 index 00000000000..f144acca27a --- /dev/null +++ b/tests/rename-deps.rs @@ -0,0 +1,120 @@ +extern crate cargo; +extern crate cargotest; +extern crate hamcrest; + +use cargotest::support::{project, execs}; +use cargotest::support::registry::Package; +use cargotest::ChannelChanger; +use hamcrest::assert_that; + +#[test] +fn gated() { + let p = project("foo") + .file("Cargo.toml", r#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + + [dependencies] + bar = { package = "foo", version = "0.1" } + "#) + .file("src/lib.rs", "") + .build(); + + assert_that(p.cargo("build").masquerade_as_nightly_cargo(), + execs().with_status(101) + .with_stderr("\ +error: failed to parse manifest at `[..]` + +Caused by: + feature `rename-dependency` is required + +consider adding `cargo-features = [\"rename-dependency\"]` to the manifest +")); + + let p = project("bar") + .file("Cargo.toml", r#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + + [dependencies] + bar = { version = "0.1", package = "baz" } + "#) + .file("src/lib.rs", "") + .build(); + + assert_that(p.cargo("build").masquerade_as_nightly_cargo(), + execs().with_status(101) + .with_stderr("\ +error: failed to parse manifest at `[..]` + +Caused by: + feature `rename-dependency` is required + +consider adding `cargo-features = [\"rename-dependency\"]` to the manifest +")); +} + +#[test] +fn rename_dependency() { + Package::new("bar", "0.1.0").publish(); + Package::new("bar", "0.2.0").publish(); + + let p = project("foo") + .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("build").masquerade_as_nightly_cargo(), + execs().with_status(0)); +} + +#[test] +fn rename_with_different_names() { + let p = project("foo") + .file("Cargo.toml", r#" + cargo-features = ["rename-dependency"] + + [project] + name = "foo" + version = "0.0.1" + authors = [] + + [dependencies] + baz = { path = "bar", package = "bar" } + "#) + .file("src/lib.rs", " + extern crate baz; + ") + .file("bar/Cargo.toml", r#" + [project] + name = "bar" + version = "0.0.1" + authors = [] + + [lib] + name = "random_name" + "#) + .file("bar/src/lib.rs", "") + .build(); + + assert_that(p.cargo("build").masquerade_as_nightly_cargo(), + execs().with_status(0)); +}