From: Alex Crichton <alex@alexcrichton.com>
Date: Thu, 5 Oct 2017 22:11:25 +0000 (-0700)
Subject: Fix [patch] causing updates with a virtual manifest
X-Git-Tag: archive/raspbian/0.35.0-2+rpi1~3^2^2^2^2^2^2^2~22^2~7^2^2
X-Git-Url: https://dgit.raspbian.org/?a=commitdiff_plain;h=f0a52633b06011ff811c41cc99b35456d67e023d;p=cargo.git

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 4ca01c49a..3f185bc80 100644
--- 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 c333d4b59..db8bd372d 100644
--- 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] [..]
+"));
+}