Skip to content

Commit

Permalink
Fix publishing renamed dependencies to crates.io
Browse files Browse the repository at this point in the history
This commit fixes publishing crates which contain locally renamed dependencies
to crates.io. Previously this lack of information meant that although we could
resolve the crate graph correctly it wouldn't work well with respect to optional
features and optional dependencies. The fix here is to persist this information
into the registry about the crate being renamed in `Cargo.toml`, allowing Cargo
to correctly deduce feature names as it does when it has `Cargo.toml` locally.

A dual side of this commit is to publish this information to crates.io. We'll
want to merge the associated PR (link to come soon) on crates.io first and make
sure that's deployed as well before we stabilize the crate renaming feature.

The index format is updated as well as part of this change. The `name` key for
dependencies is now unconditionally what was written in `Cargo.toml` as the
left-hand-side of the dependency specification. In other words this is the raw
crate name, but only for the local crate. A new key, `package`, is added to
dependencies (and it can be `None`). This key indicates the crates.io package is
being linked against, an represents the `package` key in `Cargo.toml`.

It's important to consider the interaction with older Cargo implementations
which don't support the `package` key in the index. In these situations older
Cargo binaries will likely fail to resolve entirely as the renamed name is
unlikely to exist on crates.io. For example the `futures` crate now has an
optional dependency with the name `futures01` which depends on an older version
of `futures` on crates.io. The string `futures01` will be listed in the index
under the `"name"` key, but no `futures01` crate exists on crates.io so older
Cargo will generate an error. If the crate does exist on crates.io, then even
weirder error messages will likely result.

Closes #5962
  • Loading branch information
alexcrichton committed Sep 7, 2018
1 parent f9926c6 commit e82443d
Show file tree
Hide file tree
Showing 8 changed files with 179 additions and 58 deletions.
16 changes: 8 additions & 8 deletions src/cargo/core/dependency.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ struct Inner {
specified_req: bool,
kind: Kind,
only_match_name: bool,
rename: Option<InternedString>,
explicit_name_in_toml: Option<InternedString>,

optional: bool,
default_features: bool,
Expand Down Expand Up @@ -73,7 +73,7 @@ impl ser::Serialize for Dependency {
uses_default_features: self.uses_default_features(),
features: self.features(),
target: self.platform(),
rename: self.rename().map(|s| s.as_str()),
rename: self.explicit_name_in_toml().map(|s| s.as_str()),
}.serialize(s)
}
}
Expand Down Expand Up @@ -199,7 +199,7 @@ impl Dependency {
default_features: true,
specified_req: false,
platform: None,
rename: None,
explicit_name_in_toml: None,
}),
}
}
Expand Down Expand Up @@ -229,7 +229,7 @@ impl Dependency {
/// foo = { version = "0.1", package = 'bar' }
/// ```
pub fn name_in_toml(&self) -> InternedString {
self.rename().unwrap_or(self.inner.name)
self.explicit_name_in_toml().unwrap_or(self.inner.name)
}

/// The name of the package that this `Dependency` depends on.
Expand Down Expand Up @@ -285,8 +285,8 @@ impl Dependency {
///
/// If the `package` key is used in `Cargo.toml` then this returns the same
/// value as `name_in_toml`.
pub fn rename(&self) -> Option<InternedString> {
self.inner.rename
pub fn explicit_name_in_toml(&self) -> Option<InternedString> {
self.inner.explicit_name_in_toml
}

pub fn set_kind(&mut self, kind: Kind) -> &mut Dependency {
Expand Down Expand Up @@ -330,8 +330,8 @@ impl Dependency {
self
}

pub fn set_rename(&mut self, rename: &str) -> &mut Dependency {
Rc::make_mut(&mut self.inner).rename = Some(InternedString::new(rename));
pub fn set_explicit_name_in_toml(&mut self, name: &str) -> &mut Dependency {
Rc::make_mut(&mut self.inner).explicit_name_in_toml = Some(InternedString::new(name));
self
}

Expand Down
2 changes: 1 addition & 1 deletion src/cargo/core/resolver/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ unable to verify that `{0}` is the same as when the lockfile was generated

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));
.map(|d| d.explicit_name_in_toml().map(|s| s.as_str()).unwrap_or(&crate_name));
let name = names.next().unwrap_or(&crate_name);
for n in names {
if n == name {
Expand Down
1 change: 1 addition & 0 deletions src/cargo/ops/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ fn transmit(
Kind::Development => "dev",
}.to_string(),
registry: dep_registry,
explicit_name_in_toml: dep.explicit_name_in_toml().map(|s| s.to_string()),
})
})
.collect::<CargoResult<Vec<NewCrateDependency>>>()?;
Expand Down
12 changes: 11 additions & 1 deletion src/cargo/sources/registry/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,7 @@ struct RegistryDependency<'a> {
target: Option<Cow<'a, str>>,
kind: Option<Cow<'a, str>>,
registry: Option<Cow<'a, str>>,
package: Option<Cow<'a, str>>,
}

impl<'a> RegistryDependency<'a> {
Expand All @@ -289,6 +290,7 @@ impl<'a> RegistryDependency<'a> {
target,
kind,
registry,
package,
} = self;

let id = if let Some(registry) = registry {
Expand All @@ -297,7 +299,15 @@ impl<'a> RegistryDependency<'a> {
default.clone()
};

let mut dep = Dependency::parse_no_deprecated(&name, Some(&req), &id)?;

let mut dep = Dependency::parse_no_deprecated(
package.as_ref().unwrap_or(&name),
Some(&req),
&id,
)?;
if package.is_some() {
dep.set_explicit_name_in_toml(&name);
}
let kind = match kind.as_ref().map(|s| &s[..]).unwrap_or("") {
"dev" => Kind::Development,
"build" => Kind::Build,
Expand Down
26 changes: 13 additions & 13 deletions src/cargo/util/toml/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1239,7 +1239,7 @@ impl TomlDependency {
impl DetailedTomlDependency {
fn to_dependency(
&self,
name: &str,
name_in_toml: &str,
cx: &mut Context,
kind: Option<Kind>,
) -> CargoResult<Dependency> {
Expand All @@ -1249,7 +1249,7 @@ impl DetailedTomlDependency {
providing a local path, Git repository, or \
version to use. This will be considered an \
error in future versions",
name
name_in_toml
);
cx.warnings.push(msg);
}
Expand All @@ -1266,7 +1266,7 @@ impl DetailedTomlDependency {
let msg = format!(
"key `{}` is ignored for dependency ({}). \
This will be considered an error in future versions",
key_name, name
key_name, name_in_toml
);
cx.warnings.push(msg)
}
Expand All @@ -1290,20 +1290,20 @@ impl DetailedTomlDependency {
(Some(_), _, Some(_), _) | (Some(_), _, _, Some(_)) => bail!(
"dependency ({}) specification is ambiguous. \
Only one of `git` or `registry` is allowed.",
name
name_in_toml
),
(_, _, Some(_), Some(_)) => bail!(
"dependency ({}) specification is ambiguous. \
Only one of `registry` or `registry-index` is allowed.",
name
name_in_toml
),
(Some(git), maybe_path, _, _) => {
if maybe_path.is_some() {
let msg = format!(
"dependency ({}) specification is ambiguous. \
Only one of `git` or `path` is allowed. \
This will be considered an error in future versions",
name
name_in_toml
);
cx.warnings.push(msg)
}
Expand All @@ -1318,7 +1318,7 @@ impl DetailedTomlDependency {
"dependency ({}) specification is ambiguous. \
Only one of `branch`, `tag` or `rev` is allowed. \
This will be considered an error in future versions",
name
name_in_toml
);
cx.warnings.push(msg)
}
Expand Down Expand Up @@ -1359,15 +1359,15 @@ impl DetailedTomlDependency {
(None, None, None, None) => SourceId::crates_io(cx.config)?,
};

let (pkg_name, rename) = match self.package {
Some(ref s) => (&s[..], Some(name)),
None => (name, None),
let (pkg_name, explicit_name_in_toml) = match self.package {
Some(ref s) => (&s[..], Some(name_in_toml)),
None => (name_in_toml, None),
};

let version = self.version.as_ref().map(|v| &v[..]);
let mut dep = match cx.pkgid {
Some(id) => Dependency::parse(pkg_name, version, &new_source_id, id, cx.config)?,
None => Dependency::parse_no_deprecated(name, version, &new_source_id)?,
None => Dependency::parse_no_deprecated(pkg_name, version, &new_source_id)?,
};
dep.set_features(self.features.iter().flat_map(|x| x))
.set_default_features(
Expand All @@ -1381,9 +1381,9 @@ impl DetailedTomlDependency {
if let Some(kind) = kind {
dep.set_kind(kind);
}
if let Some(rename) = rename {
if let Some(name_in_toml) = explicit_name_in_toml {
cx.features.require(Feature::rename_dependency())?;
dep.set_rename(rename);
dep.set_explicit_name_in_toml(name_in_toml);
}
Ok(dep)
}
Expand Down
5 changes: 4 additions & 1 deletion src/crates-io/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,10 @@ pub struct NewCrateDependency {
pub version_req: String,
pub target: Option<String>,
pub kind: String,
#[serde(skip_serializing_if = "Option::is_none")] pub registry: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")]
pub registry: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")]
pub explicit_name_in_toml: Option<String>,
}

#[derive(Deserialize)]
Expand Down
56 changes: 55 additions & 1 deletion tests/testsuite/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use cargo::util::paths::remove_dir_all;
use support::cargo_process;
use support::git;
use support::paths::{self, CargoPathExt};
use support::registry::{self, Package};
use support::registry::{self, Package, Dependency};
use support::{basic_manifest, project};
use url::Url;

Expand Down Expand Up @@ -1711,3 +1711,57 @@ fn git_init_templatedir_missing() {
p.cargo("build").run();
p.cargo("build").run();
}

#[test]
fn rename_deps_and_features() {
Package::new("foo", "0.1.0")
.file("src/lib.rs", "pub fn f1() {}")
.publish();
Package::new("foo", "0.2.0")
.file("src/lib.rs", "pub fn f2() {}")
.publish();
Package::new("bar", "0.2.0")
.add_dep(Dependency::new("foo01", "0.1.0").package("foo").optional(true))
.add_dep(Dependency::new("foo02", "0.2.0").package("foo"))
.feature("another", &["foo01"])
.file(
"src/lib.rs",
r#"
extern crate foo02;
#[cfg(feature = "foo01")]
extern crate foo01;
pub fn foo() {
foo02::f2();
#[cfg(feature = "foo01")]
foo01::f1();
}
"#,
)
.publish();

let p = project()
.file(
"Cargo.toml",
r#"
[project]
name = "a"
version = "0.5.0"
authors = []
[dependencies]
bar = "0.2"
"#,
).file(
"src/main.rs",
"
extern crate bar;
fn main() { bar::foo(); }
",
)
.build();

p.cargo("build").run();
p.cargo("build --features bar/foo01").run();
p.cargo("build --features bar/another").run();
}
Loading

0 comments on commit e82443d

Please sign in to comment.