Skip to content

Commit

Permalink
Mix feature flags into fingerprint/metadata shorthash
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
nipunn1313 committed Nov 7, 2016
1 parent 14a28af commit 530f2dd
Show file tree
Hide file tree
Showing 18 changed files with 301 additions and 154 deletions.
7 changes: 7 additions & 0 deletions src/cargo/core/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
8 changes: 5 additions & 3 deletions src/cargo/ops/cargo_clean.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}
}

Expand Down
124 changes: 86 additions & 38 deletions src/cargo/ops/cargo_rustc/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

Expand All @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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)
}

Expand Down
21 changes: 15 additions & 6 deletions src/cargo/ops/cargo_rustc/fingerprint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());

Expand Down Expand Up @@ -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();
}
}
}

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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",
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/ops/cargo_rustc/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
}

Expand Down
Loading

0 comments on commit 530f2dd

Please sign in to comment.