Stable metadata hashes across workspaces

src/cargo/core/package_id.rs

@@ -2,6 +2,7 @@ use std::cmp::Ordering;
 use std::fmt::{self, Formatter};
 use std::hash::Hash;
 use std::hash;
+use std::path::Path;
 use std::sync::Arc;
 
 use semver;
@@ -131,6 +132,20 @@ impl PackageId {
             }),
         }
     }
+
+    pub fn stable_hash<'a>(&'a self, workspace: &'a Path) -> PackageIdStableHash<'a> {
+        PackageIdStableHash(&self, workspace)
+    }
+}
+
+pub struct PackageIdStableHash<'a>(&'a PackageId, &'a Path);
+
+impl<'a> Hash for PackageIdStableHash<'a> {
+    fn hash<S: hash::Hasher>(&self, state: &mut S) {
+        self.0.inner.name.hash(state);
+        self.0.inner.version.hash(state);
+        self.0.inner.source_id.stable_hash(self.1, state);
+    }
 }
 
 impl fmt::Display for PackageId {

src/cargo/core/source.rs

@@ -1,7 +1,7 @@
 use std::cmp::{self, Ordering};
 use std::collections::hash_map::{HashMap, Values, IterMut};
 use std::fmt::{self, Formatter};
-use std::hash;
+use std::hash::{self, Hash};
 use std::path::Path;
 use std::sync::Arc;
 use std::sync::atomic::{AtomicBool, ATOMIC_BOOL_INIT};
@@ -297,6 +297,17 @@ impl SourceId {
         }
         self.inner.url.to_string() == CRATES_IO
     }
+
+    pub fn stable_hash<S: hash::Hasher>(&self, workspace: &Path, into: &mut S) {
+        if self.is_path() {
+            if let Ok(p) = self.inner.url.to_file_path().unwrap().strip_prefix(workspace) {
+                self.inner.kind.hash(into);
+                p.to_str().unwrap().hash(into);
+                return
+            }
+        }
+        self.hash(into)
+    }
 }
 
 impl PartialEq for SourceId {
@@ -417,7 +428,7 @@ impl Ord for SourceIdInner {
 // The hash of SourceId is used in the name of some Cargo folders, so shouldn't
 // vary. `as_str` gives the serialisation of a url (which has a spec) and so
 // insulates against possible changes in how the url crate does hashing.
-impl hash::Hash for SourceId {
+impl Hash for SourceId {
     fn hash<S: hash::Hasher>(&self, into: &mut S) {
         self.inner.kind.hash(into);
         match *self.inner {

src/cargo/ops/cargo_rustc/context.rs

@@ -407,7 +407,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
         let name = unit.pkg.package_id().name();
         match self.target_metadata(unit) {
             Some(meta) => format!("{}-{}", name, meta),
-            None => format!("{}-{}", name, util::short_hash(unit.pkg)),
+            None => format!("{}-{}", name, self.target_short_hash(unit)),
         }
     }
 
@@ -426,6 +426,13 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
         self.build_config.requested_target.as_ref().map(|s| &s[..])
     }
 
+    /// Get the short hash based only on the PackageId
+    /// Used for the metadata when target_metadata returns None
+    pub fn target_short_hash(&self, unit: &Unit) -> String {
+        let hashable = unit.pkg.package_id().stable_hash(self.ws.root());
+        util::short_hash(&hashable)
+    }
+
     /// Get the metadata for a target in a specific profile
     /// We build to the path: "{filename}-{target_metadata}"
     /// We use a linking step to link/copy to a predictable filename
@@ -459,7 +466,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
 
         // Unique metadata per (name, source, version) triple. This'll allow us
         // to pull crates from anywhere w/o worrying about conflicts
-        unit.pkg.package_id().hash(&mut hasher);
+        unit.pkg.package_id().stable_hash(self.ws.root()).hash(&mut hasher);
 
         // Add package properties which map to environment variables
         // exposed by Cargo

src/cargo/ops/cargo_rustc/mod.rs

@@ -12,7 +12,7 @@ use core::{Package, PackageId, PackageSet, Target, Resolve};
 use core::{Profile, Profiles, Workspace};
 use core::shell::ColorChoice;
 use util::{self, ProcessBuilder, machine_message};
-use util::{Config, internal, profile, join_paths, short_hash};
+use util::{Config, internal, profile, join_paths};
 use util::errors::{CargoResult, CargoResultExt};
 use util::Freshness;
 
@@ -791,7 +791,7 @@ fn build_base_args(cx: &mut Context,
             cmd.arg("-C").arg(&format!("extra-filename=-{}", m));
         }
         None => {
-            cmd.arg("-C").arg(&format!("metadata={}", short_hash(unit.pkg)));
+            cmd.arg("-C").arg(&format!("metadata={}", cx.target_short_hash(unit)));
         }
     }
 

tests/build.rs

@@ -1,4 +1,5 @@
 extern crate cargo;
+#[macro_use]
 extern crate cargotest;
 extern crate hamcrest;
 extern crate tempdir;
@@ -3271,3 +3272,32 @@ fn no_bin_in_src_with_lib() {
                 execs().with_status(101)
                        .with_stderr_contains(r#"[ERROR] couldn't read "[..]main.rs"[..]"#));
 }
+
+#[test]
+fn same_metadata_different_directory() {
+    // A top-level crate built in two different workspaces should have the
+    // same metadata hash.
+    let p = project("foo1")
+        .file("Cargo.toml", &basic_bin_manifest("foo"))
+        .file("src/foo.rs", &main_file(r#""i am foo""#, &[]));
+    let output = t!(String::from_utf8(
+        t!(p.cargo_process("build").arg("-v").exec_with_output())
+            .stderr,
+    ));
+    let metadata = output
+        .split_whitespace()
+        .filter(|arg| arg.starts_with("metadata="))
+        .next()
+        .unwrap();
+
+    let p = project("foo2")
+        .file("Cargo.toml", &basic_bin_manifest("foo"))
+        .file("src/foo.rs", &main_file(r#""i am foo""#, &[]));
+
+    assert_that(
+        p.cargo_process("build").arg("-v"),
+        execs().with_status(0).with_stderr_contains(
+            format!("[..]{}[..]", metadata),
+        ),
+    );
+}

tests/path.rs

@@ -1,4 +1,5 @@
 extern crate cargo;
+#[macro_use]
 extern crate cargotest;
 extern crate hamcrest;
 
@@ -808,6 +809,8 @@ fn custom_target_no_rebuild() {
             authors = []
             [dependencies]
             a = { path = "a" }
+            [workspace]
+            members = ["a", "b"]
         "#)
         .file("src/lib.rs", "")
         .file("a/Cargo.toml", r#"
@@ -835,9 +838,10 @@ fn custom_target_no_rebuild() {
 [FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
 "));
 
+    t!(fs::rename(p.root().join("target"), p.root().join("target_moved")));
     assert_that(p.cargo("build")
                  .arg("--manifest-path=b/Cargo.toml")
-                 .env("CARGO_TARGET_DIR", "target"),
+                 .env("CARGO_TARGET_DIR", "target_moved"),
                 execs().with_status(0)
                        .with_stderr("\
 [COMPILING] b v0.5.0 ([..])