Load [replace] sections from lock files

alexcrichton · alexcrichton · commit 155dee54575e · 2016-10-20T19:02:38.000-07:00
This commit fixes a bug in Cargo where path-based [replace] dependencies were accidentally not loaded from lock files. This meant that even with a lock file some compilations could accidentally become nondeterministic. The fix here is to just look at all path dependencies, even those specified through [replace] Closes #3216
diff --git a/src/cargo/core/registry.rs b/src/cargo/core/registry.rs
@@ -165,6 +165,10 @@ impl<'cfg> PackageRegistry<'cfg> {
     }
 
     pub fn register_lock(&mut self, id: PackageId, deps: Vec<PackageId>) {
+        trace!("register_lock: {}", id);
+        for dep in deps.iter() {
+            trace!("\t-> {}", dep);
+        }
         let sub_map = self.locked.entry(id.source_id().clone())
                                  .or_insert(HashMap::new());
         let sub_vec = sub_map.entry(id.name().to_string())
@@ -224,12 +228,17 @@ impl<'cfg> PackageRegistry<'cfg> {
             vec.iter().find(|&&(ref id, _)| id == summary.package_id())
         });
 
+        trace!("locking summary of {}", summary.package_id());
+
         // Lock the summary's id if possible
         let summary = match pair {
             Some(&(ref precise, _)) => summary.override_id(precise.clone()),
             None => summary,
         };
         summary.map_dependencies(|dep| {
+            trace!("\t{}/{}/{}", dep.name(), dep.version_req(),
+                   dep.source_id());
+
             // If we've got a known set of overrides for this summary, then
             // one of a few cases can arise:
             //
@@ -252,6 +261,7 @@ impl<'cfg> PackageRegistry<'cfg> {
             if let Some(&(_, ref locked_deps)) = pair {
                 let locked = locked_deps.iter().find(|id| dep.matches_id(id));
                 if let Some(locked) = locked {
+                    trace!("\tfirst hit on {}", locked);
                     return dep.lock_to(locked)
                 }
             }
@@ -265,8 +275,14 @@ impl<'cfg> PackageRegistry<'cfg> {
                 vec.iter().find(|&&(ref id, _)| dep.matches_id(id))
             });
             match v {
-                Some(&(ref id, _)) => dep.lock_to(id),
-                None => dep
+                Some(&(ref id, _)) => {
+                    trace!("\tsecond hit on {}", id);
+                    dep.lock_to(id)
+                }
+                None => {
+                    trace!("\tremaining unlocked");
+                    dep
+                }
             }
         })
     }
diff --git a/src/cargo/core/resolver/encode.rs b/src/cargo/core/resolver/encode.rs
@@ -185,8 +185,10 @@ fn build_path_deps(ws: &Workspace) -> HashMap<String, SourceId> {
     fn build(pkg: &Package,
              config: &Config,
              ret: &mut HashMap<String, SourceId>) {
+        let replace = pkg.manifest().replace();
         let deps = pkg.dependencies()
                       .iter()
+                      .chain(replace.iter().map(|p| &p.1))
                       .filter(|d| !ret.contains_key(d.name()))
                       .map(|d| d.source_id())
                       .filter(|id| id.is_path())
diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs
@@ -864,6 +864,7 @@ impl<'a> Context<'a> {
                 None => return Ok(Candidate { summary: summary, replace: None }),
                 Some(replacement) => replacement,
             };
+            debug!("found an override for {} {}", dep.name(), dep.version_req());
 
             let mut summaries = try!(registry.query(dep)).into_iter();
             let s = try!(summaries.next().chain_error(|| {
@@ -903,6 +904,10 @@ impl<'a> Context<'a> {
                       matched_spec, spec, summary.package_id());
             }
 
+            for dep in summary.dependencies() {
+                debug!("\t{} => {}", dep.name(), dep.version_req());
+            }
+
             Ok(Candidate { summary: summary, replace: replace })
         }).collect()
     }
diff --git a/src/cargo/ops/resolve.rs b/src/cargo/ops/resolve.rs
@@ -77,6 +77,7 @@ pub fn resolve_with_previous<'a>(registry: &mut PackageRegistry,
     //    to the previously resolved version if the dependency listed
     //    still matches the locked version.
     if let Some(r) = previous {
+        trace!("previous: {:?}", r);
         for node in r.iter().filter(|p| keep(p, to_avoid, &to_avoid_sources)) {
             let deps = r.deps_not_replaced(node)
                         .filter(|p| keep(p, to_avoid, &to_avoid_sources))
diff --git a/tests/overrides.rs b/tests/overrides.rs
@@ -775,3 +775,103 @@ http://doc.crates.io/specifying-dependencies.html#overriding-dependencies
 [FINISHED] [..]
 "));
 }
+
+#[test]
+fn override_an_override() {
+    Package::new("chrono", "0.2.0").dep("serde", "< 0.9").publish();
+    Package::new("serde", "0.7.0")
+        .file("src/lib.rs", "pub fn serde07() {}")
+        .publish();
+    Package::new("serde", "0.8.0")
+        .file("src/lib.rs", "pub fn serde08() {}")
+        .publish();
+
+    let p = project("local")
+        .file("Cargo.toml", r#"
+            [package]
+            name = "local"
+            version = "0.0.1"
+            authors = []
+
+            [dependencies]
+            chrono = "0.2"
+            serde = "0.8"
+
+            [replace]
+            "chrono:0.2.0" = { path = "chrono" }
+            "serde:0.8.0" = { path = "serde" }
+        "#)
+        .file("Cargo.lock", r#"
+            [root]
+            name = "local"
+            version = "0.0.1"
+            dependencies = [
+             "chrono 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
+             "serde 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)",
+            ]
+
+            [[package]]
+            name = "chrono"
+            version = "0.2.0"
+            source = "registry+https://github.com/rust-lang/crates.io-index"
+            replace = "chrono 0.2.0"
+
+            [[package]]
+            name = "chrono"
+            version = "0.2.0"
+            dependencies = [
+             "serde 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)",
+            ]
+
+            [[package]]
+            name = "serde"
+            version = "0.7.0"
+            source = "registry+https://github.com/rust-lang/crates.io-index"
+
+            [[package]]
+            name = "serde"
+            version = "0.8.0"
+            source = "registry+https://github.com/rust-lang/crates.io-index"
+            replace = "serde 0.8.0"
+
+            [[package]]
+            name = "serde"
+            version = "0.8.0"
+        "#)
+        .file("src/lib.rs", "
+            extern crate chrono;
+            extern crate serde;
+
+            pub fn local() {
+                chrono::chrono();
+                serde::serde08_override();
+            }
+        ")
+        .file("chrono/Cargo.toml", r#"
+            [package]
+            name = "chrono"
+            version = "0.2.0"
+            authors = []
+
+            [dependencies]
+            serde = "< 0.9"
+        "#)
+        .file("chrono/src/lib.rs", "
+            extern crate serde;
+            pub fn chrono() {
+                serde::serde07();
+            }
+        ")
+        .file("serde/Cargo.toml", r#"
+            [package]
+            name = "serde"
+            version = "0.8.0"
+            authors = []
+        "#)
+        .file("serde/src/lib.rs", "
+            pub fn serde08_override() {}
+        ");
+
+    assert_that(p.cargo_process("build").arg("-v"),
+                execs().with_status(0));
+}
