Test for bad path overrides with summaries

Bad path overrides are currently detected to issue warnings in cases where path
overrides are not suitable and have exhibited buggy behavior in the past.
Unfortunately though it looks like some false positives are being issued,
causing unnecessary confusion about `paths` overrides.

This commit fixes the detection of these "bad path overrides" by comparing
`Summary` dependencies (what's written down in `Cargo.toml`) rather than
comparing the `Cargo.toml` of the override with `Cargo.lock`. We're guaranteed
that the package we're overridding has already been resolved into `Cargo.lock`,
so we know that if the two `Cargo.toml` files are equivalent we'll continue
with the same crate graph.

I'm not actually entirely sure why I originally thought it'd be better to go
through the `Cargo.lock` comparison route. Unfortunately that doesn't take into
account optional deps which aren't in `Cargo.lock` but are in `Cargo.toml` of
the override, causing the false positive. This method, however, simply ensures
that the two dependency lists are the same.

Closes #3313

src/cargo/core/dependency.rs

@@ -11,7 +11,7 @@ use util::{CargoError, CargoResult, Cfg, CfgExpr, ChainError, human, Config};
 
 /// Information about a dependency requested by a Cargo manifest.
 /// Cheap to copy.
-#[derive(PartialEq, Clone ,Debug)]
+#[derive(PartialEq, Clone, Debug)]
 pub struct Dependency {
     inner: Rc<DependencyInner>,
 }

src/cargo/core/registry.rs

@@ -290,23 +290,7 @@ impl<'cfg> PackageRegistry<'cfg> {
     fn warn_bad_override(&self,
                          override_summary: &Summary,
                          real_summary: &Summary) -> CargoResult<()> {
-        let real = real_summary.package_id();
-        // If we don't have a locked variant then things are going quite wrong
-        // at this point, but we've already emitted a warning, so don't worry
-        // about it.
-        let map = match self.locked.get(real.source_id()) {
-            Some(map) => map,
-            None => return Ok(()),
-        };
-        let list = map.get(real.name()).chain_error(|| {
-            human(format!("failed to find lock name of {}", real))
-        })?;
-        let &(_, ref real_deps) = 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 mut real_deps = real_summary.dependencies().iter().collect::<Vec<_>>();
 
         let boilerplate = "\
 This is currently allowed but is known to produce buggy behavior with spurious
@@ -322,7 +306,7 @@ 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)) {
+            if let Some(i) = real_deps.iter().position(|d| dep == *d) {
                 real_deps.remove(i);
                 continue
             }

tests/overrides.rs

@@ -1013,3 +1013,93 @@ fn replace_to_path_dep() {
     assert_that(p.cargo_process("build"),
                 execs().with_status(0));
 }
+
+#[test]
+fn paths_ok_with_optional() {
+    Package::new("bar", "0.1.0").publish();
+
+    let p = project("local")
+        .file("Cargo.toml", r#"
+            [package]
+            name = "local"
+            version = "0.0.1"
+            authors = []
+
+            [dependencies]
+            foo = { path = "foo" }
+        "#)
+        .file("src/lib.rs", "")
+        .file("foo/Cargo.toml", r#"
+            [package]
+            name = "foo"
+            version = "0.1.0"
+            authors = []
+
+            [dependencies]
+            bar = { version = "0.1", optional = true }
+        "#)
+        .file("foo/src/lib.rs", "")
+        .file("foo2/Cargo.toml", r#"
+            [package]
+            name = "foo"
+            version = "0.1.0"
+            authors = []
+
+            [dependencies]
+            bar = { version = "0.1", optional = true }
+        "#)
+        .file("foo2/src/lib.rs", "")
+        .file(".cargo/config", r#"
+            paths = ["foo2"]
+        "#);
+
+    assert_that(p.cargo_process("build"),
+                execs().with_status(0).with_stderr("\
+[COMPILING] foo v0.1.0 ([..]foo2)
+[COMPILING] local v0.0.1 ([..])
+[FINISHED] [..]
+"));
+}
+
+#[test]
+fn paths_add_optional_bad() {
+    Package::new("bar", "0.1.0").publish();
+
+    let p = project("local")
+        .file("Cargo.toml", r#"
+            [package]
+            name = "local"
+            version = "0.0.1"
+            authors = []
+
+            [dependencies]
+            foo = { path = "foo" }
+        "#)
+        .file("src/lib.rs", "")
+        .file("foo/Cargo.toml", r#"
+            [package]
+            name = "foo"
+            version = "0.1.0"
+            authors = []
+        "#)
+        .file("foo/src/lib.rs", "")
+        .file("foo2/Cargo.toml", r#"
+            [package]
+            name = "foo"
+            version = "0.1.0"
+            authors = []
+
+            [dependencies]
+            bar = { version = "0.1", optional = true }
+        "#)
+        .file("foo2/src/lib.rs", "")
+        .file(".cargo/config", r#"
+            paths = ["foo2"]
+        "#);
+
+    assert_that(p.cargo_process("build"),
+                execs().with_status(0).with_stderr_contains("\
+warning: path override for crate `foo` has altered the original list of
+dependencies; the dependency on `bar` was either added or\
+"));
+}