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 5bfc6a8c8b7e9ae101b765e2006c927c7f171637..a0cf0bac081e5a9bf0da29ea8f2521646303df61 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_else(|| 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] [..]
+"));
+}