Fix spurious rebuilds when switching source paths

src/cargo/core/package.rs

@@ -4,18 +4,11 @@ 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};
-use core::source::{SourceId, Source};
+use core::source::Source;
 
 /// Informations about a package that is available somewhere in the file system.
 ///
@@ -27,8 +20,6 @@ pub struct Package {
     manifest: Manifest,
     // The root of the package
     manifest_path: PathBuf,
-    // Where this package came from
-    source_id: SourceId,
 }
 
 #[derive(RustcEncodable)]
@@ -60,12 +51,10 @@ impl Encodable for Package {
 
 impl Package {
     pub fn new(manifest: Manifest,
-               manifest_path: &Path,
-               source_id: &SourceId) -> Package {
+               manifest_path: &Path) -> Package {
         Package {
             manifest: manifest,
             manifest_path: manifest_path.to_path_buf(),
-            source_id: source_id.clone(),
         }
     }
 
@@ -82,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 {
@@ -100,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)
+        }
     }
 }
 

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 }

src/cargo/core/registry.rs

@@ -19,6 +19,13 @@ impl Registry for Vec<Summary> {
     }
 }
 
+impl Registry for Vec<Package> {
+    fn query(&mut self, dep: &Dependency) -> CargoResult<Vec<Summary>> {
+        Ok(self.iter().filter(|pkg| dep.matches(pkg.summary()))
+               .map(|pkg| pkg.summary().clone()).collect())
+    }
+}
+
 /// This structure represents a registry of known packages. It internally
 /// contains a number of `Box<Source>` instances which are used to load a
 /// `Package` from.

src/cargo/core/source.rs

@@ -12,7 +12,7 @@ use url::Url;
 use core::{Summary, Package, PackageId, Registry, Dependency};
 use sources::{PathSource, GitSource, RegistrySource};
 use sources::git;
-use util::{human, Config, CargoResult, CargoError, ToUrl};
+use util::{human, Config, CargoResult, ToUrl};
 
 /// A Source finds and downloads remote packages based on names and
 /// versions.
@@ -61,8 +61,6 @@ pub enum GitReference {
     Rev(String),
 }
 
-type Error = Box<CargoError + Send>;
-
 /// Unique identifier for a source of packages.
 #[derive(Clone, Eq, Debug)]
 pub struct SourceId {

src/cargo/ops/cargo_package.rs

@@ -175,7 +175,7 @@ fn run_verify(config: &Config, pkg: &Package, tar: &Path)
     });
     let mut new_manifest = pkg.manifest().clone();
     new_manifest.set_summary(new_summary.override_id(new_pkgid));
-    let new_pkg = Package::new(new_manifest, &manifest_path, &new_src);
+    let new_pkg = Package::new(new_manifest, &manifest_path);
 
     // Now that we've rewritten all our path dependencies, compile it!
     try!(ops::compile_pkg(&new_pkg, None, &ops::CompileOptions {

src/cargo/ops/cargo_read_manifest.rs

@@ -30,7 +30,7 @@ pub fn read_package(path: &Path, source_id: &SourceId, config: &Config)
     let (manifest, nested) =
         try!(read_manifest(&data, layout, source_id, config));
 
-    Ok((Package::new(manifest, path, source_id), nested))
+    Ok((Package::new(manifest, path), nested))
 }
 
 pub fn read_packages(path: &Path, source_id: &SourceId, config: &Config)

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 {

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))
     }
 }
 

src/cargo/sources/path.rs

@@ -59,7 +59,7 @@ impl<'cfg> PathSource<'cfg> {
         }
     }
 
-    pub fn read_packages(&self) -> CargoResult<Vec<Package>> {
+    fn read_packages(&self) -> CargoResult<Vec<Package>> {
         if self.updated {
             Ok(self.packages.clone())
         } else if self.id.is_path() && self.id.precise().is_some() {
@@ -272,10 +272,7 @@ impl<'cfg> Debug for PathSource<'cfg> {
 
 impl<'cfg> Registry for PathSource<'cfg> {
     fn query(&mut self, dep: &Dependency) -> CargoResult<Vec<Summary>> {
-        let mut summaries: Vec<Summary> = self.packages.iter()
-                                              .map(|p| p.summary().clone())
-                                              .collect();
-        summaries.query(dep)
+        self.packages.query(dep)
     }
 }
 

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)));
             }
         }
 

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)));
+});