Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement renaming dependencies in the manifest #4953

Merged
merged 1 commit into from
Feb 19, 2018
Merged

Implement renaming dependencies in the manifest #4953

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions src/cargo/core/dependency.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ struct Inner {
specified_req: bool,
kind: Kind,
only_match_name: bool,
rename: Option<String>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd call it package, to be consistent with the package in the surface syntax, and to make it more clear in which direction renaming goes (I.e, in theory you can have a crate foo which is renamed to pacakge baz, or, the other way around, a package baz which is renamed to crate foo).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh this is slightly different through b/c it's not actually the package, but rather what the dependency is renaming the crate to

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, even though there's a one-to-one correspondence with the package attribute, rename probably makes more sense in code (perhaps with a comment?)... that's my thought, anyway.


optional: bool,
default_features: bool,
Expand All @@ -49,6 +50,7 @@ struct SerializedDependency<'a> {
source: &'a SourceId,
req: String,
kind: Kind,
rename: Option<&'a str>,

optional: bool,
uses_default_features: bool,
Expand All @@ -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)
}
}
Expand Down Expand Up @@ -182,6 +185,7 @@ impl Dependency {
default_features: true,
specified_req: false,
platform: None,
rename: None,
}),
}
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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());
Expand Down
3 changes: 3 additions & 0 deletions src/cargo/core/features.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,9 @@ features! {

// Using epochs
[unstable] epoch: bool,

// Renaming a package in the manifest via the `package` key
[unstable] rename_dependency: bool,
}
}

Expand Down
34 changes: 25 additions & 9 deletions src/cargo/ops/cargo_rustc/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -947,28 +947,44 @@ 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)?;
}
}

return Ok(());

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()));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we make this .unwrap_or an utility method of something? Like fn crate_name(&self) -> String, which deals with as and, probably, with translating - to _ as well.

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);
Expand Down
80 changes: 51 additions & 29 deletions src/cargo/util/toml/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -193,6 +193,7 @@ pub struct DetailedTomlDependency {
default_features: Option<bool>,
#[serde(rename = "default_features")]
default_features2: Option<bool>,
package: Option<String>,
}

#[derive(Debug, Deserialize, Serialize)]
Expand Down Expand Up @@ -917,28 +918,40 @@ impl TomlDependency {
cx: &mut Context,
kind: Option<Kind>)
-> CargoResult<Dependency> {
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<Kind>)
-> CargoResult<Dependency> {
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 \
error in future versions", name);
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 {
Expand All @@ -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)?
Expand All @@ -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. \
Expand All @@ -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();
Expand All @@ -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)?
Expand Down Expand Up @@ -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(&registry_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)
}
}
Expand Down
6 changes: 4 additions & 2 deletions tests/metadata.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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": {},
Expand Down Expand Up @@ -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": {},
Expand Down
Loading