diff --git a/src/cargo/core/registry.rs b/src/cargo/core/registry.rs index 99d9db83f95..79089ee4d7e 100644 --- a/src/cargo/core/registry.rs +++ b/src/cargo/core/registry.rs @@ -1,4 +1,4 @@ -use std::collections::{HashSet, HashMap}; +use std::collections::HashMap; use core::{Source, SourceId, SourceMap, Summary, Dependency, PackageId, Package}; use core::PackageSet; @@ -188,17 +188,16 @@ impl<'cfg> PackageRegistry<'cfg> { } fn query_overrides(&mut self, dep: &Dependency) - -> CargoResult> { - let mut seen = HashSet::new(); - let mut ret = Vec::new(); + -> CargoResult> { for s in self.overrides.iter() { let src = self.sources.get_mut(s).unwrap(); let dep = Dependency::new_override(dep.name(), s); - ret.extend(try!(src.query(&dep)).into_iter().filter(|s| { - seen.insert(s.name().to_string()) - })); + let mut results = try!(src.query(&dep)); + if results.len() > 0 { + return Ok(Some(results.remove(0))) + } } - Ok(ret) + Ok(None) } /// This function is used to transform a summary to another locked summary @@ -271,25 +270,89 @@ impl<'cfg> PackageRegistry<'cfg> { } }) } + + fn warn_bad_override(&self, + override_summary: &Summary, + real_summary: &Summary) -> CargoResult<()> { + let real = real_summary.package_id(); + let map = try!(self.locked.get(real.source_id()).chain_error(|| { + human(format!("failed to find lock source of {}", real)) + })); + let list = try!(map.get(real.name()).chain_error(|| { + human(format!("failed to find lock name of {}", real)) + })); + let &(_, ref real_deps) = try!(list.iter().find(|&&(ref id, _)| { + real == id + }).chain_error(|| { + human(format!("failed to find lock version of {}", real)) + })); + let mut real_deps = real_deps.clone(); + + let boilerplate = "\ +This is currently allowed but is known to produce buggy behavior with spurious +recompiles and changes to the crate graph. Path overrides unfortunately were +never intended to support this feature, so for now this message is just a +warning. In the future, however, this message will become a hard error. + +To change the dependency graph via an override it's recommended to use the +`[replace]` feature of Cargo instead of the path override feature. This is +documented online at the url below for more information. + +http://doc.crates.io/specifying-dependencies.html#overriding-dependencies +"; + + for dep in override_summary.dependencies() { + if let Some(i) = real_deps.iter().position(|id| dep.matches_id(id)) { + real_deps.remove(i); + continue + } + let msg = format!("\ + path override for crate `{}` has altered the original list of\n\ + dependencies; the dependency on `{}` was either added or\n\ + modified to not match the previously resolved version\n\n\ + {}", override_summary.package_id().name(), dep.name(), boilerplate); + try!(self.source_config.config().shell().warn(&msg)); + return Ok(()) + } + + for id in real_deps { + 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(), boilerplate); + try!(self.source_config.config().shell().warn(&msg)); + return Ok(()) + } + + Ok(()) + } } impl<'cfg> Registry for PackageRegistry<'cfg> { fn query(&mut self, dep: &Dependency) -> CargoResult> { - let overrides = try!(self.query_overrides(&dep)); - - let ret = if overrides.is_empty() { - // Ensure the requested source_id is loaded - try!(self.ensure_loaded(dep.source_id(), Kind::Normal).chain_error(|| { - human(format!("failed to load source for a dependency \ - on `{}`", dep.name())) - })); - - match self.sources.get_mut(dep.source_id()) { - Some(src) => try!(src.query(&dep)), - None => Vec::new(), + // Ensure the requested source_id is loaded + try!(self.ensure_loaded(dep.source_id(), Kind::Normal).chain_error(|| { + human(format!("failed to load source for a dependency \ + on `{}`", dep.name())) + })); + + let override_summary = try!(self.query_overrides(&dep)); + let real_summaries = match self.sources.get_mut(dep.source_id()) { + Some(src) => Some(try!(src.query(&dep))), + None => None, + }; + + let ret = match (override_summary, real_summaries) { + (Some(candidate), Some(summaries)) => { + if summaries.len() != 1 { + bail!("found an override with a non-locked list"); + } + try!(self.warn_bad_override(&candidate, &summaries[0])); + vec![candidate] } - } else { - overrides + (Some(_), None) => bail!("override found but no real ones"), + (None, Some(summaries)) => summaries, + (None, None) => Vec::new(), }; // post-process all returned summaries to ensure that we lock all diff --git a/src/cargo/sources/config.rs b/src/cargo/sources/config.rs index 01205ff51b7..ac254f147ec 100644 --- a/src/cargo/sources/config.rs +++ b/src/cargo/sources/config.rs @@ -62,6 +62,10 @@ impl<'cfg> SourceConfigMap<'cfg> { Ok(base) } + pub fn config(&self) -> &'cfg Config { + self.config + } + pub fn load(&self, id: &SourceId) -> CargoResult> { debug!("loading: {}", id); let mut name = match self.id2name.get(id) { diff --git a/tests/overrides.rs b/tests/overrides.rs index 816efd53c1d..c6d8a96e446 100644 --- a/tests/overrides.rs +++ b/tests/overrides.rs @@ -708,3 +708,70 @@ fn no_override_self() { assert_that(p.cargo_process("build").arg("--verbose"), execs().with_status(0)); } + +#[test] +fn broken_path_override_warns() { + Package::new("foo", "0.1.0").publish(); + Package::new("foo", "0.2.0").publish(); + + let p = project("local") + .file("Cargo.toml", r#" + [package] + name = "local" + version = "0.0.1" + authors = [] + + [dependencies] + a = { path = "a1" } + "#) + .file("src/lib.rs", "") + .file("a1/Cargo.toml", r#" + [package] + name = "a" + version = "0.0.1" + authors = [] + + [dependencies] + foo = "0.1" + "#) + .file("a1/src/lib.rs", "") + .file("a2/Cargo.toml", r#" + [package] + name = "a" + version = "0.0.1" + authors = [] + + [dependencies] + foo = "0.2" + "#) + .file("a2/src/lib.rs", "") + .file(".cargo/config", r#" + paths = ["a2"] + "#); + + assert_that(p.cargo_process("build"), + execs().with_status(0) + .with_stderr("\ +[UPDATING] [..] +warning: path override for crate `a` has altered the original list of +dependencies; the dependency on `foo` was either added or +modified to not match the previously resolved version + +This is currently allowed but is known to produce buggy behavior with spurious +recompiles and changes to the crate graph. Path overrides unfortunately were +never intended to support this feature, so for now this message is just a +warning. In the future, however, this message will become a hard error. + +To change the dependency graph via an override it's recommended to use the +`[replace]` feature of Cargo instead of the path override feature. This is +documented online at the url below for more information. + +http://doc.crates.io/specifying-dependencies.html#overriding-dependencies + +[DOWNLOADING] [..] +[COMPILING] [..] +[COMPILING] [..] +[COMPILING] [..] +[FINISHED] [..] +")); +}