Fix spurious rebuilds when switching source paths

The method of creating package ids in Cargo means that all sub-crates of a main
repo have the same package id, which encodes the path it came from. This means
that if the "root crate" switches, the package id for all dependencies will
change, causing an alteration in package id hashes, causing recompiles.

This commit alters a few points of hashing to ensure that whenever a package is
being hashed for freshness the *source root* of the crate is used instead of the
root of the main crate. This cause the hashes to be consistent across compiles,
regardless of the root package.

Closes #1694

diff --git a/src/cargo/core/package.rs b/src/cargo/core/package.rs
index 593190162687336a74675d671493557477294835..f71f5025e5d1f45f6ed189e086d6455db319613b 100644 (file)
--- a/src/cargo/core/package.rs
+++ b/src/cargo/core/package.rs
@@ -4,14 +4,7 @@ use std::slice;
 use std::path::{Path, PathBuf};
 use semver::Version;
 
-use core::{
-    Dependency,
-    Manifest,
-    PackageId,
-    Registry,
-    Target,
-    Summary,
-};
+use core::{Dependency, Manifest, PackageId, Registry, Target, Summary, Metadata};
 use core::dependency::SerializedDependency;
 use util::{CargoResult, graph};
 use rustc_serialize::{Encoder,Encodable};
@@ -78,6 +71,10 @@ impl Package {
     pub fn has_custom_build(&self) -> bool {
         self.targets().iter().any(|t| t.is_custom_build())
     }
+
+    pub fn generate_metadata(&self) -> Metadata {
+        self.package_id().generate_metadata(self.root())
+    }
 }
 
 impl fmt::Display for Package {
@@ -96,7 +93,15 @@ impl Eq for Package {}
 
 impl hash::Hash for Package {
     fn hash<H: hash::Hasher>(&self, into: &mut H) {
-        self.package_id().hash(into)
+        // We want to be sure that a path-based package showing up at the same
+        // location always has the same hash. To that effect we don't hash the
+        // vanilla package ID if we're a path, but instead feed in our own root
+        // path.
+        if self.package_id().source_id().is_path() {
+            (0, self.root(), self.name(), self.package_id().version()).hash(into)
+        } else {
+            (1, self.package_id()).hash(into)
+        }
     }
 }
 

diff --git a/src/cargo/core/package_id.rs b/src/cargo/core/package_id.rs
index f585a0137c9e4da8f51191eb4abb98ae076e7c25..25bfc2796ad61a01525f62d2eea9f133229da4f3 100644 (file)
--- a/src/cargo/core/package_id.rs
+++ b/src/cargo/core/package_id.rs
@@ -3,6 +3,7 @@ use std::error::Error;
 use std::fmt::{self, Formatter};
 use std::hash::Hash;
 use std::hash;
+use std::path::Path;
 use std::sync::Arc;
 
 use regex::Regex;
@@ -135,10 +136,13 @@ impl PackageId {
     pub fn version(&self) -> &semver::Version { &self.inner.version }
     pub fn source_id(&self) -> &SourceId { &self.inner.source_id }
 
-    pub fn generate_metadata(&self) -> Metadata {
-        let metadata = short_hash(
-            &(&self.inner.name, self.inner.version.to_string(),
-              &self.inner.source_id));
+    pub fn generate_metadata(&self, source_root: &Path) -> Metadata {
+        // See comments in Package::hash for why we have this test
+        let metadata = if self.inner.source_id.is_path() {
+            short_hash(&(0, &self.inner.name, &self.inner.version, source_root))
+        } else {
+            short_hash(&(1, self))
+        };
         let extra_filename = format!("-{}", metadata);
 
         Metadata { metadata: metadata, extra_filename: extra_filename }

diff --git a/src/cargo/ops/cargo_rustc/context.rs b/src/cargo/ops/cargo_rustc/context.rs
index 9ea3dfc0bb020f3bfdd70b8afc1e9961f4f33936..4a1284074ae05feab8f9751331292752478a7998 100644 (file)
--- a/src/cargo/ops/cargo_rustc/context.rs
+++ b/src/cargo/ops/cargo_rustc/context.rs
@@ -275,7 +275,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
             // Make sure that the name of this test executable doesn't
             // conflict with a library that has the same name and is
             // being tested
-            let mut metadata = pkg.package_id().generate_metadata();
+            let mut metadata = pkg.generate_metadata();
             metadata.mix(&format!("bin-{}", target.name()));
             Some(metadata)
         } else if pkg.package_id() == self.resolve.root() && !profile.test {

diff --git a/src/cargo/ops/cargo_rustc/layout.rs b/src/cargo/ops/cargo_rustc/layout.rs
index 58c644a82e8d4fc4250862cf33a7135025556909..5ab7bb5a4fd8aee452ed56c43ad1550f7e5b7c91 100644 (file)
--- a/src/cargo/ops/cargo_rustc/layout.rs
+++ b/src/cargo/ops/cargo_rustc/layout.rs
@@ -130,7 +130,7 @@ impl Layout {
     }
 
     fn pkg_dir(&self, pkg: &Package) -> String {
-        format!("{}-{}", pkg.name(), short_hash(pkg.package_id()))
+        format!("{}-{}", pkg.name(), short_hash(pkg))
     }
 }
 

diff --git a/src/cargo/util/toml.rs b/src/cargo/util/toml.rs
index 080e0ce34af7257cec12ce6242fd2d29ce8eb0f0..2bb1a1ff1f7175a8ded593eaec3a256f96271e11 100644 (file)
--- a/src/cargo/util/toml.rs
+++ b/src/cargo/util/toml.rs
@@ -380,7 +380,7 @@ impl TomlManifest {
         }
 
         let pkgid = try!(project.to_package_id(source_id));
-        let metadata = pkgid.generate_metadata();
+        let metadata = pkgid.generate_metadata(&layout.root);
 
         // If we have no lib at all, use the inferred lib if available
         // If we have a lib with a path, we're done
@@ -427,8 +427,8 @@ impl TomlManifest {
 
         for bin in bins.iter() {
             if blacklist.iter().find(|&x| *x == bin.name) != None {
-                return Err(human(&format!("the binary target name `{}` is forbidden", 
-                                          bin.name)));
+                return Err(human(&format!("the binary target name `{}` is \
+                                           forbidden", bin.name)));
             }
         }
 

diff --git a/tests/test_cargo_compile_path_deps.rs b/tests/test_cargo_compile_path_deps.rs
index 608a1b3a34cbc72ee268cb19d5e9da1da071add0..13586953a39df5905aaae2cb951b9afc41924a13 100644 (file)
--- a/tests/test_cargo_compile_path_deps.rs
+++ b/tests/test_cargo_compile_path_deps.rs
@@ -749,3 +749,47 @@ test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured
 
 ", COMPILING, p.url(), COMPILING, p.url())));
 });
+
+test!(custom_target_no_rebuild {
+    let p = project("foo")
+        .file("Cargo.toml", r#"
+            [project]
+            name = "foo"
+            version = "0.5.0"
+            authors = []
+            [dependencies]
+            a = { path = "a" }
+        "#)
+        .file("src/lib.rs", "")
+        .file("a/Cargo.toml", r#"
+            [project]
+            name = "a"
+            version = "0.5.0"
+            authors = []
+        "#)
+        .file("a/src/lib.rs", "")
+        .file("b/Cargo.toml", r#"
+            [project]
+            name = "b"
+            version = "0.5.0"
+            authors = []
+            [dependencies]
+            a = { path = "../a" }
+        "#)
+        .file("b/src/lib.rs", "");
+    p.build();
+    assert_that(p.cargo("build"),
+                execs().with_status(0)
+                       .with_stdout(&format!("\
+{compiling} a v0.5.0 ([..])
+{compiling} foo v0.5.0 ([..])
+", compiling = COMPILING)));
+
+    assert_that(p.cargo("build")
+                 .arg("--manifest-path=b/Cargo.toml")
+                 .env("CARGO_TARGET_DIR", "target"),
+                execs().with_status(0)
+                       .with_stdout(&format!("\
+{compiling} b v0.5.0 ([..])
+", compiling = COMPILING)));
+});