Fix [patch] causing updates with a virtual manifest

This commit fixes the [patch] implementation to avoid causing spurious updates
of the registry when specified inside of a virtual manifest via a path
dependency that was otherwise outside of the workspace.

Cargo previously attempted to learn about path dependencies through the
configuration of the workspace by looking at members, their path dependencies,
and their replace sections. The replace/patch sections, however, only matter at
the root of the workspace and those weren't looked at! This commit fixes this
problem by explicitly looking at the workspace's replace/patch sections and
avoiding looking at other manifest replace/patch which shouldn't matter.

Closes #4552

diff --git a/src/cargo/core/resolver/encode.rs b/src/cargo/core/resolver/encode.rs
index 4ca01c49a3178434c9408261ccda17ec74d4417b..3f185bc8055b844702eb15b7555b1ae4c118b841 100644 (file)
--- a/src/cargo/core/resolver/encode.rs
+++ b/src/cargo/core/resolver/encode.rs
@@ -5,7 +5,7 @@ use std::str::FromStr;
 use serde::ser;
 use serde::de;
 
-use core::{Package, PackageId, SourceId, Workspace};
+use core::{Package, PackageId, SourceId, Workspace, Dependency};
 use util::{Graph, Config, internal};
 use util::errors::{CargoResult, CargoResultExt, CargoError};
 
@@ -60,7 +60,12 @@ impl EncodableResolve {
                 let id = match pkg.source.as_ref().or(path_deps.get(&pkg.name)) {
                     // We failed to find a local package in the workspace.
                     // It must have been removed and should be ignored.
-                    None => continue,
+                    None => {
+                        debug!("path dependency now missing {} v{}",
+                               pkg.name,
+                               pkg.version);
+                        continue
+                    }
                     Some(source) => PackageId::new(&pkg.name, &pkg.version, source)?
                 };
 
@@ -199,34 +204,49 @@ fn build_path_deps(ws: &Workspace) -> HashMap<String, SourceId> {
         visited.insert(member.package_id().source_id().clone());
     }
     for member in members.iter() {
-        build(member, ws.config(), &mut ret, &mut visited);
+        build_pkg(member, ws.config(), &mut ret, &mut visited);
+    }
+    for (_, deps) in ws.root_patch() {
+        for dep in deps {
+            build_dep(dep, ws.config(), &mut ret, &mut visited);
+        }
+    }
+    for &(_, ref dep) in ws.root_replace() {
+        build_dep(dep, ws.config(), &mut ret, &mut visited);
     }
 
     return ret;
 
-    fn build(pkg: &Package,
-             config: &Config,
-             ret: &mut HashMap<String, SourceId>,
-             visited: &mut HashSet<SourceId>) {
-        let replace = pkg.manifest().replace().iter().map(|p| &p.1);
-        let patch = pkg.manifest().patch().values().flat_map(|v| v);
-        let deps = pkg.dependencies()
-                      .iter()
-                      .chain(replace)
-                      .chain(patch)
-                      .map(|d| d.source_id())
-                      .filter(|id| !visited.contains(id) && id.is_path())
-                      .filter_map(|id| id.url().to_file_path().ok())
-                      .map(|path| path.join("Cargo.toml"))
-                      .filter_map(|path| Package::for_path(&path, config).ok())
-                      .collect::<Vec<_>>();
-        for pkg in deps {
-            ret.insert(pkg.name().to_string(),
-                       pkg.package_id().source_id().clone());
-            visited.insert(pkg.package_id().source_id().clone());
-            build(&pkg, config, ret, visited);
+    fn build_pkg(pkg: &Package,
+                 config: &Config,
+                 ret: &mut HashMap<String, SourceId>,
+                 visited: &mut HashSet<SourceId>) {
+        for dep in pkg.dependencies() {
+            build_dep(dep, config, ret, visited);
         }
     }
+
+    fn build_dep(dep: &Dependency,
+                 config: &Config,
+                 ret: &mut HashMap<String, SourceId>,
+                 visited: &mut HashSet<SourceId>) {
+        let id = dep.source_id();
+        if visited.contains(id) || !id.is_path() {
+            return
+        }
+        let path = match id.url().to_file_path() {
+            Ok(p) => p.join("Cargo.toml"),
+            Err(_) => return,
+        };
+        let pkg = match Package::for_path(&path, config) {
+            Ok(p) => p,
+            Err(_) => return,
+        };
+        ret.insert(pkg.name().to_string(),
+                   pkg.package_id().source_id().clone());
+        visited.insert(pkg.package_id().source_id().clone());
+        build_pkg(&pkg, config, ret, visited);
+    }
 }
 
 impl Patch {

diff --git a/tests/patch.rs b/tests/patch.rs
index c333d4b5991cf950cdbec0861b283f488e81d323..db8bd372dd8e657420ebd7ff4d0b71cd3c2ab081 100644 (file)
--- a/tests/patch.rs
+++ b/tests/patch.rs
@@ -742,3 +742,41 @@ Caused by:
   to different sources
 "));
 }
+
+#[test]
+fn patch_in_virtual() {
+    Package::new("foo", "0.1.0").publish();
+
+    let p = project("bar")
+        .file("Cargo.toml", r#"
+            [workspace]
+            members = ["bar"]
+
+            [patch.crates-io]
+            foo = { path = "foo" }
+        "#)
+        .file("foo/Cargo.toml", r#"
+            [package]
+            name = "foo"
+            version = "0.1.0"
+            authors = []
+        "#)
+        .file("foo/src/lib.rs", r#""#)
+        .file("bar/Cargo.toml", r#"
+            [package]
+            name = "bar"
+            version = "0.1.0"
+            authors = []
+
+            [dependencies]
+            foo = "0.1"
+        "#)
+        .file("bar/src/lib.rs", r#""#);
+
+    assert_that(p.cargo_process("build"),
+                execs().with_status(0));
+    assert_that(p.cargo("build"),
+                execs().with_status(0).with_stderr("\
+[FINISHED] [..]
+"));
+}