development/cargo
development/cargo
1
0
Fork 0
mirror of https://github.com/rust-lang/cargo synced 2024-10-14 03:32:39 +00:00
Code Issues Packages Projects Releases Wiki Activity

Auto merge of #5811 - alexcrichton:rename-crate-feature-names, r=ehuss

Browse source 
Use listed dependency name for feature names

This commit updates the implementation of renamed dependencies to use the listed
name of a dependency in Cargo.toml for the name of the associated feature,
rather than using the package name. This'll allow disambiguating between
different packages of the same name and was the intention all along!

Closes #5753
This commit is contained in:
bors 2018-07-31 18:18:17 +00:00
parent 4b95e8c50e 5295caddd5
commit d9feff2b88
16 changed files with 236 additions and 67 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
src/cargo/core/compiler/build_context/mod.rs
View file

@ -104,7 +104,7 @@ impl<'a, 'cfg> BuildContext<'a, 'cfg> {
let crate_name = dep.target.crate_name();
let mut names = deps.iter()
.map(|d| d.rename().unwrap_or(&crate_name));
.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 {

2
src/cargo/core/compiler/context/unit_dependencies.rs
View file

@ -118,7 +118,7 @@ fn compute_deps<'a, 'cfg>(
// If the dependency is optional, then we're only activating it
// if the corresponding feature was activated
if dep.is_optional() && !bcx.resolve.features(id).contains(&*dep.name()) {
if dep.is_optional() && !bcx.resolve.features(id).contains(&*dep.name_in_toml()) {
return false;
}

64
src/cargo/core/dependency.rs
View file

@ -28,7 +28,7 @@ struct Inner {
specified_req: bool,
kind: Kind,
only_match_name: bool,
rename: Option<String>,
rename: Option<InternedString>,
optional: bool,
default_features: bool,
@ -65,7 +65,7 @@ impl ser::Serialize for Dependency {
S: ser::Serializer,
{
SerializedDependency {
name: &*self.name(),
name: &*self.package_name(),
source: self.source_id(),
req: self.version_req().to_string(),
kind: self.kind(),
@ -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(),
rename: self.rename().map(|s| s.as_str()),
}.serialize(s)
}
}
@ -208,7 +208,49 @@ impl Dependency {
&self.inner.req
}
pub fn name(&self) -> InternedString {
/// This is the name of this `Dependency` as listed in `Cargo.toml`.
///
/// Or in other words, this is what shows up in the `[dependencies]` section
/// on the left hand side. This is **not** the name of the package that's
/// being depended on as the dependency can be renamed. For that use
/// `package_name` below.
///
/// Both of the dependencies below return `foo` for `name_in_toml`:
///
/// ```toml
/// [dependencies]
/// foo = "0.1"
/// ```
///
/// and ...
///
/// ```toml
/// [dependencies]
/// foo = { version = "0.1", package = 'bar' }
/// ```
pub fn name_in_toml(&self) -> InternedString {
self.rename().unwrap_or(self.inner.name)
}
/// The name of the package that this `Dependency` depends on.
///
/// Usually this is what's written on the left hand side of a dependencies
/// section, but it can also be renamed via the `package` key.
///
/// Both of the dependencies below return `foo` for `package_name`:
///
/// ```toml
/// [dependencies]
/// foo = "0.1"
/// ```
///
/// and ...
///
/// ```toml
/// [dependencies]
/// bar = { version = "0.1", package = 'foo' }
/// ```
pub fn package_name(&self) -> InternedString {
self.inner.name
}
@ -239,8 +281,12 @@ impl Dependency {
self.inner.platform.as_ref()
}
pub fn rename(&self) -> Option<&str> {
self.inner.rename.as_ref().map(|s| &**s)
/// The renamed name of this dependency, if any.
///
/// 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 set_kind(&mut self, kind: Kind) -> &mut Dependency {
@ -285,7 +331,7 @@ impl Dependency {
}
pub fn set_rename(&mut self, rename: &str) -> &mut Dependency {
Rc::make_mut(&mut self.inner).rename = Some(rename.to_string());
Rc::make_mut(&mut self.inner).rename = Some(InternedString::new(rename));
self
}
@ -295,7 +341,7 @@ impl Dependency {
assert!(self.inner.req.matches(id.version()));
trace!(
"locking dep from `{}` with `{}` at {} to {}",
self.name(),
self.package_name(),
self.version_req(),
self.source_id(),
id
@ -346,7 +392,7 @@ impl Dependency {
/// Returns true if the package (`sum`) can fulfill this dependency request.
pub fn matches_ignoring_source(&self, id: &PackageId) -> bool {
self.name() == id.name() && self.version_req().matches(id.version())
self.package_name() == id.name() && self.version_req().matches(id.version())
}
/// Returns true if the package (`id`) can fulfill this dependency request.

28
src/cargo/core/registry.rs
View file

@ -197,7 +197,7 @@ impl<'cfg> PackageRegistry<'cfg> {
// of summaries which should be the same length as `deps` above.
let unlocked_summaries = deps.iter()
.map(|dep| {
debug!("registring a patch for `{}` with `{}`", url, dep.name());
debug!("registring a patch for `{}` with `{}`", url, dep.package_name());
// Go straight to the source for resolving `dep`. Load it as we
// normally would and then ask it directly for the list of summaries
@ -207,7 +207,7 @@ impl<'cfg> PackageRegistry<'cfg> {
format_err!(
"failed to load source for a dependency \
on `{}`",
dep.name()
dep.package_name()
)
})?;
@ -223,14 +223,14 @@ impl<'cfg> PackageRegistry<'cfg> {
"patch for `{}` in `{}` did not resolve to any crates. If this is \
unexpected, you may wish to consult: \
https://github.com/rust-lang/cargo/issues/4678",
dep.name(),
dep.package_name(),
url
),
};
if summaries.next().is_some() {
bail!(
"patch for `{}` in `{}` resolved to more than one candidate",
dep.name(),
dep.package_name(),
url
)
}
@ -238,7 +238,7 @@ impl<'cfg> PackageRegistry<'cfg> {
bail!(
"patch for `{}` in `{}` points to the same source, but \
patches must point to different sources",
dep.name(),
dep.package_name(),
url
);
}
@ -306,7 +306,7 @@ impl<'cfg> PackageRegistry<'cfg> {
fn query_overrides(&mut self, dep: &Dependency) -> CargoResult<Option<Summary>> {
for s in self.overrides.iter() {
let src = self.sources.get_mut(s).unwrap();
let dep = Dependency::new_override(&*dep.name(), s);
let dep = Dependency::new_override(&*dep.package_name(), s);
let mut results = src.query_vec(&dep)?;
if !results.is_empty() {
return Ok(Some(results.remove(0)));
@ -369,21 +369,21 @@ http://doc.crates.io/specifying-dependencies.html#overriding-dependencies
modified to not match the previously resolved version\n\n\
{}",
override_summary.package_id().name(),
dep.name(),
dep.package_name(),
boilerplate
);
self.source_config.config().shell().warn(&msg)?;
return Ok(());
}
if let Some(id) = real_deps.get(0) {
if let Some(dep) = real_deps.get(0) {
let msg = format!(
"\
path override for crate `{}` has altered the original list of
dependencies; the dependency on `{}` was removed\n\n
{}",
override_summary.package_id().name(),
id.name(),
dep.package_name(),
boilerplate
);
self.source_config.config().shell().warn(&msg)?;
@ -439,7 +439,7 @@ impl<'cfg> Registry for PackageRegistry<'cfg> {
with `{}`, \
looking at sources",
patches.len(),
dep.name(),
dep.package_name(),
dep.source_id(),
dep.version_req()
);
@ -451,7 +451,7 @@ impl<'cfg> Registry for PackageRegistry<'cfg> {
format_err!(
"failed to load source for a dependency \
on `{}`",
dep.name()
dep.package_name()
)
})?;
@ -542,7 +542,7 @@ fn lock(locked: &LockedMap, patches: &HashMap<Url, Vec<PackageId>>, summary: Sum
None => summary,
};
summary.map_dependencies(|dep| {
trace!("\t{}/{}/{}", dep.name(), dep.version_req(), dep.source_id());
trace!("\t{}/{}/{}", dep.package_name(), dep.version_req(), dep.source_id());
// If we've got a known set of overrides for this summary, then
// one of a few cases can arise:
@ -578,7 +578,7 @@ fn lock(locked: &LockedMap, patches: &HashMap<Url, Vec<PackageId>>, summary: Sum
// If anything does then we lock it to that and move on.
let v = locked
.get(dep.source_id())
.and_then(|map| map.get(&*dep.name()))
.and_then(|map| map.get(&*dep.package_name()))
.and_then(|vec| vec.iter().find(|&&(ref id, _)| dep.matches_id(id)));
if let Some(&(ref id, _)) = v {
trace!("\tsecond hit on {}", id);
@ -592,7 +592,7 @@ fn lock(locked: &LockedMap, patches: &HashMap<Url, Vec<PackageId>>, summary: Sum
let v = patches.get(dep.source_id().url()).map(|vec| {
let dep2 = dep.clone();
let mut iter = vec.iter().filter(move |p| {
dep2.name() == p.name() && dep2.version_req().matches(p.version())
dep2.matches_ignoring_source(p)
});
(iter.next(), iter)
});

2
src/cargo/core/resolver/conflict_cache.rs
View file

@ -81,7 +81,7 @@ impl ConflictCache {
.entry(dep.clone())
.or_insert_with(Vec::new);
if !past.contains(con) {
trace!("{} adding a skip {:?}", dep.name(), con);
trace!("{} adding a skip {:?}", dep.package_name(), con);
past.push(con.clone());
for c in con.keys() {
self.dep_from_pid

22
src/cargo/core/resolver/context.rs
View file

@ -128,7 +128,7 @@ impl Context {
pub fn prev_active(&self, dep: &Dependency) -> &[Summary] {
self.activations
.get(&(dep.name(), dep.source_id().clone()))
.get(&(dep.package_name(), dep.source_id().clone()))
.map(|v| &v[..])
.unwrap_or(&[])
}
@ -178,19 +178,19 @@ impl Context {
for dep in deps {
// Skip optional dependencies, but not those enabled through a
// feature
if dep.is_optional() && !reqs.deps.contains_key(&dep.name()) {
if dep.is_optional() && !reqs.deps.contains_key(&dep.name_in_toml()) {
continue;
}
// So we want this dependency. Move the features we want from
// `feature_deps` to `ret` and register ourselves as using this
// name.
let base = reqs.deps.get(&dep.name()).unwrap_or(&default_dep);
used_features.insert(dep.name());
let base = reqs.deps.get(&dep.name_in_toml()).unwrap_or(&default_dep);
used_features.insert(dep.name_in_toml());
let always_required = !dep.is_optional()
&& !s
.dependencies()
.iter()
.any(|d| d.is_optional() && d.name() == dep.name());
.any(|d| d.is_optional() && d.name_in_toml() == dep.name_in_toml());
if always_required && base.0 {
self.warnings.push(format!(
"Package `{}` does not have feature `{}`. It has a required dependency \
@ -198,7 +198,7 @@ impl Context {
This is currently a warning to ease the transition, but it will become an \
error in the future.",
s.package_id(),
dep.name()
dep.name_in_toml()
));
}
let mut base = base.1.clone();
@ -220,8 +220,8 @@ impl Context {
let remaining = reqs
.deps
.keys()
.filter(|&s| !used_features.contains(s))
.map(|s| s.as_str())
.cloned()
.filter(|s| !used_features.contains(s))
.collect::<Vec<_>>();
if !remaining.is_empty() {
let features = remaining.join(", ");
@ -298,10 +298,10 @@ fn build_requirements<'a, 'b: 'a>(
all_features: true, ..
} => {
for key in s.features().keys() {
reqs.require_feature(InternedString::new(&key))?;
reqs.require_feature(*key)?;
}
for dep in s.dependencies().iter().filter(|d| d.is_optional()) {
reqs.require_dependency(dep.name());
reqs.require_dependency(dep.name_in_toml());
}
}
Method::Required {
@ -389,7 +389,7 @@ impl<'r> Requirements<'r> {
for fv in self
.summary
.features()
.get(&feat)
.get(feat.as_str())
.expect("must be a valid feature")
{
match *fv {

26
src/cargo/core/resolver/mod.rs
View file

@ -263,14 +263,14 @@ fn activate_deps_loop(
"{}[{}]>{} {} candidates",
parent.name(),
cur,
dep.name(),
dep.package_name(),
candidates.len()
);
trace!(
"{}[{}]>{} {} prev activations",
parent.name(),
cur,
dep.name(),
dep.package_name(),
cx.prev_active(&dep).len()
);
@ -307,7 +307,7 @@ fn activate_deps_loop(
// It's our job here to backtrack, if possible, and find a
// different candidate to activate. If we can't find any
// candidates whatsoever then it's time to bail entirely.
trace!("{}[{}]>{} -- no candidates", parent.name(), cur, dep.name());
trace!("{}[{}]>{} -- no candidates", parent.name(), cur, dep.package_name());
// Use our list of `conflicting_activations` to add to our
// global list of past conflicting activations, effectively
@ -400,7 +400,7 @@ fn activate_deps_loop(
"{}[{}]>{} trying {}",
parent.name(),
cur,
dep.name(),
dep.package_name(),
candidate.summary.version()
);
let res = activate(&mut cx, registry, Some((&parent, &dep)), candidate, &method);
@ -551,7 +551,7 @@ fn activate_deps_loop(
"{}[{}]>{} skipping {} ",
parent.name(),
cur,
dep.name(),
dep.package_name(),
pid.version()
);
false
@ -855,7 +855,7 @@ fn activation_error(
) -> CargoError {
let graph = cx.graph();
if !candidates.is_empty() {
let mut msg = format!("failed to select a version for `{}`.", dep.name());
let mut msg = format!("failed to select a version for `{}`.", dep.package_name());
msg.push_str("\n ... required by ");
msg.push_str(&describe_path(&graph.path_to_top(parent.package_id())));
@ -881,7 +881,7 @@ fn activation_error(
for &(p, r) in links_errors.iter() {
if let ConflictReason::Links(ref link) = *r {
msg.push_str("\n\nthe package `");
msg.push_str(&*dep.name());
msg.push_str(&*dep.package_name());
msg.push_str("` links to the native library `");
msg.push_str(link);
msg.push_str("`, but it conflicts with a previous package which links to `");
@ -900,11 +900,11 @@ fn activation_error(
msg.push_str("\n\nthe package `");
msg.push_str(&*p.name());
msg.push_str("` depends on `");
msg.push_str(&*dep.name());
msg.push_str(&*dep.package_name());
msg.push_str("`, with features: `");
msg.push_str(features);
msg.push_str("` but `");
msg.push_str(&*dep.name());
msg.push_str(&*dep.package_name());
msg.push_str("` does not have these features.\n");
}
// p == parent so the full path is redundant.
@ -923,7 +923,7 @@ fn activation_error(
}
msg.push_str("\n\nfailed to select a version for `");
msg.push_str(&*dep.name());
msg.push_str(&*dep.package_name());
msg.push_str("` which could resolve this conflict");
return format_err!("{}", msg);
@ -964,7 +964,7 @@ fn activation_error(
location searched: {}\n\
versions found: {}\n",
dep.version_req(),
dep.name(),
dep.package_name(),
dep.source_id(),
versions
);
@ -993,14 +993,14 @@ fn activation_error(
candidates.dedup();
let mut candidates: Vec<_> = candidates
.iter()
.map(|n| (lev_distance(&*new_dep.name(), &*n), n))
.map(|n| (lev_distance(&*new_dep.package_name(), &*n), n))
.filter(|&(d, _)| d < 4)
.collect();
candidates.sort_by_key(|o| o.0);
let mut msg = format!(
"no matching package named `{}` found\n\
location searched: {}\n",
dep.name(),
dep.package_name(),
dep.source_id()
);
if !candidates.is_empty() {

4
src/cargo/core/resolver/types.rs
View file

@ -63,7 +63,7 @@ impl<'a> RegistryQueryer<'a> {
None => continue,
Some(replacement) => replacement,
};
debug!("found an override for {} {}", dep.name(), dep.version_req());
debug!("found an override for {} {}", dep.package_name(), dep.version_req());
let mut summaries = self.registry.query_vec(dep, false)?.into_iter();
let s = summaries.next().ok_or_else(|| {
@ -117,7 +117,7 @@ impl<'a> RegistryQueryer<'a> {
}
for dep in summary.dependencies() {
debug!("\t{} => {}", dep.name(), dep.version_req());
debug!("\t{} => {}", dep.package_name(), dep.version_req());
}
candidate.replace = replace;

9
src/cargo/core/summary.rs
View file

@ -41,17 +41,18 @@ impl Summary {
) -> CargoResult<Summary>
where K: Borrow<str> + Ord + Display {
for dep in dependencies.iter() {
if !namespaced_features && features.get(&*dep.name()).is_some() {
let feature = dep.name_in_toml();
if !namespaced_features && features.get(&*feature).is_some() {
bail!(
"Features and dependencies cannot have the \
same name: `{}`",
dep.name()
feature
)
}
if dep.is_optional() && !dep.is_transitive() {
bail!(
"Dev-dependencies are not allowed to be optional: `{}`",
dep.name()
feature
)
}
}
@ -147,7 +148,7 @@ where K: Borrow<str> + Ord + Display {
let mut dep_map = HashMap::new();
for dep in dependencies.iter() {
dep_map
.entry(dep.name().as_str())
.entry(dep.name_in_toml())
.or_insert_with(Vec::new)
.push(dep);
}

2
src/cargo/ops/cargo_package.rs
View file

@ -145,7 +145,7 @@ fn verify_dependencies(pkg: &Package) -> CargoResult<()> {
"all path dependencies must have a version specified \
when packaging.\ndependency `{}` does not specify \
a version.",
dep.name()
dep.name_in_toml()
)
}
}

13
src/cargo/ops/registry.rs
View file

@ -108,7 +108,7 @@ fn verify_dependencies(pkg: &Package, registry_src: &SourceId) -> CargoResult<()
"all path dependencies must have a version specified \
when publishing.\ndependency `{}` does not specify \
a version",
dep.name()
dep.package_name()
)
}
} else if dep.source_id() != registry_src {
@ -119,7 +119,10 @@ fn verify_dependencies(pkg: &Package, registry_src: &SourceId) -> CargoResult<()
bail!("crates cannot be published to crates.io with dependencies sourced from other\n\
registries either publish `{}` on crates.io or pull it into this repository\n\
and specify it with a path and version\n\
(crate `{}` is pulled from {})", dep.name(), dep.name(), dep.source_id());
(crate `{}` is pulled from {})",
dep.package_name(),
dep.package_name(),
dep.source_id());
}
} else {
bail!(
@ -128,8 +131,8 @@ fn verify_dependencies(pkg: &Package, registry_src: &SourceId) -> CargoResult<()
specify a crates.io version as a dependency or pull it into this \
repository and specify it with a path and version\n(crate `{}` has \
repository path `{}`)",
dep.name(),
dep.name(),
dep.package_name(),
dep.package_name(),
dep.source_id()
);
}
@ -164,7 +167,7 @@ fn transmit(
Ok(NewCrateDependency {
optional: dep.is_optional(),
default_features: dep.uses_default_features(),
name: dep.name().to_string(),
name: dep.package_name().to_string(),
features: dep.features().iter().map(|s| s.to_string()).collect(),
version_req: dep.version_req().to_string(),
target: dep.platform().map(|s| s.to_string()),

2
src/cargo/ops/resolve.rs
View file

@ -522,7 +522,7 @@ fn register_previous_locks<'a>(
"poisoning {} because {} looks like it changed {}",
dep.source_id(),
member.package_id(),
dep.name()
dep.package_name()
);
for id in resolve
.iter()

7
src/cargo/sources/registry/index.rs
View file

@ -269,7 +269,8 @@ impl<'cfg> RegistryIndex<'cfg> {
f: &mut FnMut(Summary),
) -> CargoResult<()> {
let source_id = self.source_id.clone();
let summaries = self.summaries(dep.name().as_str(), load)?;
let name = dep.package_name().as_str();
let summaries = self.summaries(name, load)?;
let summaries = summaries
.iter()
.filter(|&&(_, yanked)| dep.source_id().precise().is_some() || !yanked)
@ -281,8 +282,8 @@ impl<'cfg> RegistryIndex<'cfg> {
// this source, `<p_req>` is the version installed and `<f_req> is the
// version requested (argument to `--precise`).
let summaries = summaries.filter(|s| match source_id.precise() {
Some(p) if p.starts_with(&*dep.name()) && p[dep.name().len()..].starts_with('=') => {
let mut vers = p[dep.name().len() + 1..].splitn(2, "->");
Some(p) if p.starts_with(name) && p[name.len()..].starts_with('=') => {
let mut vers = p[name.len() + 1..].splitn(2, "->");
if dep
.version_req()
.matches(&Version::parse(vers.next().unwrap()).unwrap())

2
src/cargo/util/toml/mod.rs
View file

@ -872,7 +872,7 @@ impl TomlManifest {
{
let mut names_sources = BTreeMap::new();
for dep in &deps {
let name = dep.rename().unwrap_or_else(|| dep.name().as_str());
let name = dep.name_in_toml();
let prev = names_sources.insert(name.to_string(), dep.source_id());
if prev.is_some() && prev != Some(dep.source_id()) {
bail!(

18
src/doc/src/reference/unstable.md
View file

@ -99,6 +99,24 @@ extern crate bar; // registry `custom`
extern crate baz; // git repository
```
Note that if you have an optional dependency like:
```toml
[dependencies]
foo = { version = "0.1", package = 'bar', optional = true }
```
you're depending on the crate `bar` from crates.io, but your crate has a `foo`
feature instead of a `bar` feature. That is, names of features take after the
name of the dependency, not the package name, when renamed.
Enabling transitive dependencies works similarly, for example we could add the
following to the above manifest:
```toml
[features]
log-debug = ['foo/log-debug'] # using 'bar/log-debug' would be an error!
```
### publish-lockfile
* Original Issue: [#2263](https://github.com/rust-lang/cargo/issues/2263)

100
tests/testsuite/rename_deps.rs
View file

@ -364,3 +364,103 @@ fn can_run_doc_tests() {
)),
);
}
#[test]
fn features_still_work() {
Package::new("foo", "0.1.0").publish();
Package::new("bar", "0.1.0").publish();
let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "test"
version = "0.1.0"
authors = []
[dependencies]
p1 = { path = 'a', features = ['b'] }
p2 = { path = 'b' }
"#,
)
.file("src/lib.rs", "")
.file(
"a/Cargo.toml",
r#"
cargo-features = ["rename-dependency"]
[package]
name = "p1"
version = "0.1.0"
authors = []
[dependencies]
b = { version = "0.1", package = "foo", optional = true }
"#,
)
.file("a/src/lib.rs", "extern crate b;")
.file(
"b/Cargo.toml",
r#"
cargo-features = ["rename-dependency"]
[package]
name = "p2"
version = "0.1.0"
authors = []
[dependencies]
b = { version = "0.1", package = "bar", optional = true }
[features]
default = ['b']
"#,
)
.file("b/src/lib.rs", "extern crate b;")
.build();
assert_that(
p.cargo("build -v").masquerade_as_nightly_cargo(),
execs().with_status(0),
);
}
#[test]
fn features_not_working() {
Package::new("foo", "0.1.0").publish();
Package::new("bar", "0.1.0").publish();
let p = project()
.file(
"Cargo.toml",
r#"
cargo-features = ["rename-dependency"]
[package]
name = "test"
version = "0.1.0"
authors = []
[dependencies]
a = { path = 'a', package = 'p1', optional = true }
[features]
default = ['p1']
"#,
)
.file("src/lib.rs", "")
.file("a/Cargo.toml", &basic_manifest("p1", "0.1.0"))
.build();
assert_that(
p.cargo("build -v").masquerade_as_nightly_cargo(),
execs()
.with_status(101)
.with_stderr("\
error: failed to parse manifest at `[..]`
Caused by:
Feature `default` includes `p1` which is neither a dependency nor another feature
")
);
}