From: bors Date: Thu, 6 Oct 2016 00:40:11 +0000 (-0700) Subject: Auto merge of #3136 - alexcrichton:warn-bad-override, r=brson X-Git-Tag: archive/raspbian/0.35.0-2+rpi1~3^2^2^2^2^2^2^2~22^2~13^2~55 X-Git-Url: https://dgit.raspbian.org/?a=commitdiff_plain;h=a713da1867463a898b962a3f3e15ab407d8713ef;p=cargo.git Auto merge of #3136 - alexcrichton:warn-bad-override, r=brson Warn about path overrides that won't work Cargo has a long-standing [bug] in path overrides where they will cause spurious rebuilds of crates in the crate graph. This can be very difficult to diagnose and be incredibly frustrating as well. Unfortunately, though, it's behavior that fundamentally can't be fixed in Cargo. The crux of the problem happens when a `path` replacement changes something about the list of dependencies of the crate that it's replacing. This alteration to the list of dependencies *cannot be tracked by Cargo* as the lock file was previously emitted. In the best case this ends up causing random recompiles. In the worst case it cause infinite registry updates that always result in recompiles. A solution to this intention, changing the dependency graph of an overridden dependency, was [implemented] with the `[replace]` feature in Cargo awhile back. With that in mind, this commit implements a *warning* whenever a bad dependency replacement is detected. The message here is pretty wordy, but it's intended to convey that you should switch to using `[replace]` for a more robust impelmentation, and it can also give security to anyone using `path` overrides that if they get past this warning everything should work as intended. [bug]: https://github.com/rust-lang/cargo/issues/2041 [implemented]: http://doc.crates.io/specifying-dependencies.html#overriding-dependencies Closes #2041 --- a713da1867463a898b962a3f3e15ab407d8713ef diff --cc src/cargo/core/registry.rs index 99d9db83f,ecaac9815..79089ee4d --- a/src/cargo/core/registry.rs +++ b/src/cargo/core/registry.rs @@@ -194,31 -192,30 +192,32 @@@ impl<'cfg> PackageRegistry<'cfg> 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 if - // possible. This is where the concept of a lockfile comes into play. - // - // If a summary points at a package id which was previously locked, then we - // override the summary's id itself, as well as all dependencies, to be - // rewritten to the locked versions. This will transform the summary's - // source to a precise source (listed in the locked version) as well as - // transforming all of the dependencies from range requirements on imprecise - // sources to exact requirements on precise sources. - // - // If a summary does not point at a package id which was previously locked, - // we still want to avoid updating as many dependencies as possible to keep - // the graph stable. In this case we map all of the summary's dependencies - // to be rewritten to a locked version wherever possible. If we're unable to - // map a dependency though, we just pass it on through. - fn lock(&self, summary: Summary) -> Summary { + /// This function is used to transform a summary to another locked summary + /// if possible. This is where the concept of a lockfile comes into play. + /// + /// If a summary points at a package id which was previously locked, then we + /// override the summary's id itself, as well as all dependencies, to be + /// rewritten to the locked versions. This will transform the summary's + /// source to a precise source (listed in the locked version) as well as + /// transforming all of the dependencies from range requirements on + /// imprecise sources to exact requirements on precise sources. + /// + /// If a summary does not point at a package id which was previously locked, + /// or if any dependencies were added and don't have a previously listed + /// version, we still want to avoid updating as many dependencies as + /// possible to keep the graph stable. In this case we map all of the + /// summary's dependencies to be rewritten to a locked version wherever + /// possible. If we're unable to map a dependency though, we just pass it on + /// through. + pub fn lock(&self, summary: Summary) -> Summary { let pair = self.locked.get(summary.source_id()).and_then(|map| { map.get(summary.name()) }).and_then(|vec| {