Mix feature flags into fingerprint/metadata shorthash

Since building dependencies results in different libraries
depending on the feature flags, I added the feature flags
into the short_hash. This solves an issue when multiple crates
share a target directory or multiple targets share a common
library with divergent feature flag choice.

- Only link binaries, build scripts, and top level deps
- Handle dylibs differently (no metadata filename / linking)
- Fingerprint based on link_dst rather than file_stem.

This (sadly) limits the effects of dep caching to things
which don't produce a hard-link. Currently, this is only dependent
library crates. Stil a big win.

Author: nipunn1313

src/cargo/core/manifest.rs

@@ -395,6 +395,13 @@ impl Target {
         }
     }
 
+    pub fn is_dylib(&self) -> bool {
+        match self.kind {
+            TargetKind::Lib(ref libs) => libs.iter().any(|l| *l == LibKind::Dylib),
+            _ => false
+        }
+    }
+
     pub fn linkable(&self) -> bool {
         match self.kind {
             TargetKind::Lib(ref kinds) => {

src/cargo/ops/cargo_clean.rs

@@ -77,9 +77,11 @@ pub fn clean(ws: &Workspace, opts: &CleanOptions) -> CargoResult<()> {
         try!(rm_rf(&layout.proxy().fingerprint(&unit.pkg)));
         try!(rm_rf(&layout.build(&unit.pkg)));
 
-        let root = cx.out_dir(&unit);
-        for (filename, _) in try!(cx.target_filenames(&unit)) {
-            try!(rm_rf(&root.join(&filename)));
+        for (src, link_dst, _) in try!(cx.target_filenames(&unit)) {
+            try!(rm_rf(&src));
+            if let Some(dst) = link_dst {
+                try!(rm_rf(&dst));
+            }
         }
     }
 

src/cargo/ops/cargo_rustc/context.rs

@@ -309,49 +309,55 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
 
     /// Get the metadata for a target in a specific profile
     pub fn target_metadata(&self, unit: &Unit) -> Option<Metadata> {
-        let metadata = unit.target.metadata();
+        // No metadata for dylibs because of a couple issues
+        // - OSX encodes the dylib name in the executable
+        // - Windows rustc multiple files of which we can't easily link all of them
+        if !unit.profile.test && unit.target.is_dylib() {
+            return None;
+        }
+
+        let metadata = unit.target.metadata().cloned().map(|mut m| {
+            if let Some(features) = self.resolve.features(unit.pkg.package_id()) {
+                let mut feat_vec: Vec<&String> = features.iter().collect();
+                feat_vec.sort();
+                for feat in feat_vec {
+                    m.mix(feat);
+                }
+            }
+            m.mix(unit.profile);
+            m
+        });
+        let mut pkg_metadata = {
+            let mut m = unit.pkg.generate_metadata();
+            if let Some(features) = self.resolve.features(unit.pkg.package_id()) {
+                let mut feat_vec: Vec<&String> = features.iter().collect();
+                feat_vec.sort();
+                for feat in feat_vec {
+                    m.mix(feat);
+                }
+            }
+            m.mix(unit.profile);
+            m
+        };
+
         if unit.target.is_lib() && unit.profile.test {
             // Libs and their tests are built in parallel, so we need to make
             // sure that their metadata is different.
-            metadata.cloned().map(|mut m| {
+            metadata.map(|mut m| {
                 m.mix(&"test");
                 m
             })
         } else if unit.target.is_bin() && unit.profile.test {
             // 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 = unit.pkg.generate_metadata();
-            metadata.mix(&format!("bin-{}", unit.target.name()));
-            Some(metadata)
+            pkg_metadata.mix(&format!("bin-{}", unit.target.name()));
+            Some(pkg_metadata)
         } else if unit.pkg.package_id().source_id().is_path() &&
                   !unit.profile.test {
-            // If we're not building a unit test but we're building a path
-            // dependency, then we're likely compiling the "current package" or
-            // some package in a workspace. In this situation we pass no
-            // metadata by default so we'll have predictable
-            // file names like `target/debug/libfoo.{a,so,rlib}` and such.
-            //
-            // Note, though, that the compiler's build system at least wants
-            // path dependencies to have hashes in filenames. To account for
-            // that we have an extra hack here which reads the
-            // `__CARGO_DEFAULT_METADATA` environment variable and creates a
-            // hash in the filename if that's present.
-            //
-            // This environment variable should not be relied on! It's basically
-            // just here for rustbuild. We need a more principled method of
-            // doing this eventually.
-            if unit.target.is_lib() {
-                env::var("__CARGO_DEFAULT_LIB_METADATA").ok().map(|meta| {
-                    let mut metadata = unit.pkg.generate_metadata();
-                    metadata.mix(&meta);
-                    metadata
-                })
-            } else {
-                None
-            }
+            Some(pkg_metadata)
         } else {
-            metadata.cloned()
+            metadata
         }
     }
 
@@ -360,19 +366,57 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
         match self.target_metadata(unit) {
             Some(ref metadata) => format!("{}{}", unit.target.crate_name(),
                                           metadata.extra_filename),
-            None if unit.target.allows_underscores() => {
-                unit.target.name().to_string()
+            None => self.bin_stem(unit),
+        }
+    }
+
+    fn bin_stem(&self, unit: &Unit) -> String {
+        if unit.target.allows_underscores() {
+            unit.target.name().to_string()
+        } else {
+            unit.target.crate_name()
+        }
+    }
+
+    pub fn link_stem(&self, unit: &Unit) -> Option<(PathBuf, String)> {
+        let src_dir = self.out_dir(unit);
+        let bin_stem = self.bin_stem(unit);
+        let file_stem = self.file_stem(unit);
+
+        // We currently only lift files up from the `deps` directory. If
+        // it was compiled into something like `example/` or `doc/` then
+        // we don't want to link it up.
+        if src_dir.ends_with("deps") {
+            // Don't lift up library dependencies
+            if unit.pkg.package_id() != &self.current_package && !unit.target.is_bin() {
+                None
+            } else {
+                Some((
+                    src_dir.parent().unwrap().to_owned(),
+                    if unit.profile.test {file_stem} else {bin_stem},
+                ))
             }
-            None => unit.target.crate_name(),
+        } else if bin_stem == file_stem {
+            None
+        } else if src_dir.ends_with("examples") {
+            Some((src_dir, bin_stem))
+        } else if src_dir.parent().unwrap().ends_with("build") {
+            Some((src_dir, bin_stem))
+        } else {
+            None
         }
     }
 
     /// Return the filenames that the given target for the given profile will
-    /// generate, along with whether you can link against that file (e.g. it's a
-    /// library).
+    /// generate as a list of 3-tuples (filename, link_dst, linkable)
+    /// filename: filename rustc compiles to. (Often has metadata suffix).
+    /// link_dst: Optional file to link/copy the result to (without metadata suffix)
+    /// linkable: Whether possible to link against file (eg it's a library)
     pub fn target_filenames(&self, unit: &Unit)
-                            -> CargoResult<Vec<(String, bool)>> {
+                            -> CargoResult<Vec<(PathBuf, Option<PathBuf>, bool)>> {
+        let out_dir = self.out_dir(unit);
         let stem = self.file_stem(unit);
+        let link_stem = self.link_stem(unit);
         let info = if unit.target.for_host() {
             &self.host_info
         } else {
@@ -386,8 +430,11 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
                 let crate_type = if crate_type == "lib" {"rlib"} else {crate_type};
                 match info.crate_types.get(crate_type) {
                     Some(&Some((ref prefix, ref suffix))) => {
-                        ret.push((format!("{}{}{}", prefix, stem, suffix),
-                                  linkable));
+                        let filename = out_dir.join(format!("{}{}{}", prefix, stem, suffix));
+                        let link_dst = link_stem.clone().map(|(ld, ls)| {
+                            ld.join(format!("{}{}{}", prefix, ls, suffix))
+                        });
+                        ret.push((filename, link_dst, linkable));
                         Ok(())
                     }
                     // not supported, don't worry about it
@@ -429,6 +476,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
                    support any of the output crate types",
                   unit.pkg, self.target_triple());
         }
+        info!("Target filenames: {:?}", ret);
         Ok(ret)
     }
 

src/cargo/ops/cargo_rustc/fingerprint.rs

@@ -49,7 +49,7 @@ pub fn prepare_target<'a, 'cfg>(cx: &mut Context<'a, 'cfg>,
     let _p = profile::start(format!("fingerprint: {} / {}",
                                     unit.pkg.package_id(), unit.target.name()));
     let new = dir(cx, unit);
-    let loc = new.join(&filename(unit));
+    let loc = new.join(&filename(cx, unit));
 
     debug!("fingerprint at: {}", loc.display());
 
@@ -82,8 +82,11 @@ pub fn prepare_target<'a, 'cfg>(cx: &mut Context<'a, 'cfg>,
         missing_outputs = !root.join(unit.target.crate_name())
                                .join("index.html").exists();
     } else {
-        for (filename, _) in try!(cx.target_filenames(unit)) {
-            missing_outputs |= fs::metadata(root.join(filename)).is_err();
+        for (src, link_dst, _) in try!(cx.target_filenames(unit)) {
+            missing_outputs |= fs::metadata(&src).is_err();
+            if let Some(link_dst) = link_dst {
+                missing_outputs |= fs::metadata(link_dst).is_err();
+            }
         }
     }
 
@@ -529,7 +532,7 @@ pub fn dir(cx: &Context, unit: &Unit) -> PathBuf {
 
 /// Returns the (old, new) location for the dep info file of a target.
 pub fn dep_info_loc(cx: &Context, unit: &Unit) -> PathBuf {
-    dir(cx, unit).join(&format!("dep-{}", filename(unit)))
+    dir(cx, unit).join(&format!("dep-{}", filename(cx, unit)))
 }
 
 fn compare_old_fingerprint(loc: &Path, new_fingerprint: &Fingerprint)
@@ -650,7 +653,13 @@ fn mtime_if_fresh<I>(output: &Path, paths: I) -> Option<FileTime>
     }
 }
 
-fn filename(unit: &Unit) -> String {
+fn filename(cx: &Context, unit: &Unit) -> String {
+    // If there exists a link stem, we have to use that since multiple target filenames
+    // may hardlink to the same target stem. If there's no link, we can use the original
+    // file_stem (which can include a suffix)
+    let file_stem = cx.link_stem(unit)
+        .map(|(_path, stem)| stem)
+        .unwrap_or_else(|| cx.file_stem(unit));
     let kind = match *unit.target.kind() {
         TargetKind::Lib(..) => "lib",
         TargetKind::Bin => "bin",
@@ -666,7 +675,7 @@ fn filename(unit: &Unit) -> String {
     } else {
         ""
     };
-    format!("{}{}-{}", flavor, kind, unit.target.name())
+    format!("{}{}-{}", flavor, kind, file_stem)
 }
 
 // The dep-info files emitted by the compiler all have their listed paths

src/cargo/ops/cargo_rustc/layout.rs

@@ -175,7 +175,7 @@ impl<'a> LayoutProxy<'a> {
         } else if unit.target.is_lib() {
             self.deps().to_path_buf()
         } else {
-            self.root().to_path_buf()
+            self.deps().to_path_buf()
         }
     }