Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid rebuilding a project when cwd changes #4788

Merged
merged 2 commits into from
Dec 12, 2017
Merged

Avoid rebuilding a project when cwd changes #4788

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8647a87
Avoid rebuilding a project when cwd changes
alexcrichton Dec 6, 2017
f688e9c
Change Cargo's own dep-file format
alexcrichton Dec 8, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 20 additions & 4 deletions src/cargo/core/manifest.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use std::collections::{HashMap, BTreeMap};
use std::fmt;
use std::path::{PathBuf, Path};
use std::rc::Rc;
use std::hash::{Hash, Hasher};

use semver::Version;
use serde::ser;
Expand Down Expand Up @@ -199,7 +200,11 @@ pub struct Profiles {
pub struct Target {
kind: TargetKind,
name: String,
src_path: PathBuf,
// Note that the `src_path` here is excluded from the `Hash` implementation
// as it's absolute currently and is otherwise a little too brittle for
// causing rebuilds. Instead the hash for the path that we send to the
// compiler is handled elsewhere.
src_path: NonHashedPathBuf,
required_features: Option<Vec<String>>,
tested: bool,
benched: bool,
Expand All @@ -209,6 +214,17 @@ pub struct Target {
for_host: bool,
}

#[derive(Clone, PartialEq, Eq, Debug)]
struct NonHashedPathBuf {
path: PathBuf,
}

impl Hash for NonHashedPathBuf {
fn hash<H: Hasher>(&self, _: &mut H) {
// ...
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A slightly more elaborte design here would be to introduce a notion of explicitly relative path here and track path relative to workspace root and to other workspace members outside the root. Sort of like

struct PathRoots {
  // a number of anchors, such as current workspace dir, a particular workspace member, users's home directory, 
  // CARGO_HOME, etc. This structure is "global" for cargo and is stored in Workspace/config.
  roots: Vec::<PathBuf>, 
}

struct RelativePath {
  root_idx: usize // index in the global PathRoots
  rel_path: PathBuf
}

impl RelativePath {
   fn to_path(&self, roots: &PathRoots) -> PathBuf {
    roots[self.root_idx].join(&self.rel_path)
  }
}

Not sure that we need this, but this can cope with workspaces which are not entirely under a workspace root. (Or perhaps it was a mistake to allow non-subdirectory members?).


#[derive(Serialize)]
struct SerializedTarget<'a> {
/// Is this a `--bin bin`, `--lib`, `--example ex`?
Expand All @@ -227,7 +243,7 @@ impl ser::Serialize for Target {
kind: &self.kind,
crate_types: self.rustc_crate_types(),
name: &self.name,
src_path: &self.src_path,
src_path: &self.src_path.path,
}.serialize(s)
}
}
Expand Down Expand Up @@ -370,7 +386,7 @@ impl Target {
Target {
kind: TargetKind::Bin,
name: String::new(),
src_path: src_path,
src_path: NonHashedPathBuf { path: src_path },
required_features: None,
doc: false,
doctest: false,
Expand Down Expand Up @@ -459,7 +475,7 @@ impl Target {

pub fn name(&self) -> &str { &self.name }
pub fn crate_name(&self) -> String { self.name.replace("-", "_") }
pub fn src_path(&self) -> &Path { &self.src_path }
pub fn src_path(&self) -> &Path { &self.src_path.path }
pub fn required_features(&self) -> Option<&Vec<String>> { self.required_features.as_ref() }
pub fn kind(&self) -> &TargetKind { &self.kind }
pub fn tested(&self) -> bool { self.tested }
Expand Down
178 changes: 110 additions & 68 deletions src/cargo/ops/cargo_rustc/fingerprint.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
use std::env;
use std::fs::{self, File, OpenOptions};
use std::fs;
use std::hash::{self, Hasher};
use std::io::prelude::*;
use std::io::{BufReader, SeekFrom};
use std::path::{Path, PathBuf};
use std::sync::{Arc, Mutex};

Expand Down Expand Up @@ -99,8 +97,9 @@ pub fn prepare_target<'a, 'cfg>(cx: &mut Context<'a, 'cfg>,
}

let allow_failure = unit.profile.rustc_args.is_some();
let target_root = cx.target_root().to_path_buf();
let write_fingerprint = Work::new(move |_| {
match fingerprint.update_local() {
match fingerprint.update_local(&target_root) {
Ok(()) => {}
Err(..) if allow_failure => return Ok(()),
Err(e) => return Err(e)
Expand Down Expand Up @@ -139,6 +138,7 @@ pub struct Fingerprint {
features: String,
target: u64,
profile: u64,
path: u64,
#[serde(serialize_with = "serialize_deps", deserialize_with = "deserialize_deps")]
deps: Vec<(String, Arc<Fingerprint>)>,
local: Vec<LocalFingerprint>,
Expand All @@ -165,6 +165,7 @@ fn deserialize_deps<'de, D>(d: D) -> Result<Vec<(String, Arc<Fingerprint>)>, D::
rustc: 0,
target: 0,
profile: 0,
path: 0,
local: vec![LocalFingerprint::Precalculated(String::new())],
features: String::new(),
deps: Vec::new(),
Expand All @@ -181,15 +182,27 @@ enum LocalFingerprint {
EnvBased(String, Option<String>),
}

impl LocalFingerprint {
fn mtime(root: &Path, mtime: Option<FileTime>, path: &Path)
-> LocalFingerprint
{
let mtime = MtimeSlot(Mutex::new(mtime));
assert!(path.is_absolute());
let path = path.strip_prefix(root).unwrap_or(path);
LocalFingerprint::MtimeBased(mtime, path.to_path_buf())
}
}

struct MtimeSlot(Mutex<Option<FileTime>>);

impl Fingerprint {
fn update_local(&self) -> CargoResult<()> {
fn update_local(&self, root: &Path) -> CargoResult<()> {
let mut hash_busted = false;
for local in self.local.iter() {
match *local {
LocalFingerprint::MtimeBased(ref slot, ref path) => {
let meta = fs::metadata(path)
let path = root.join(path);
let meta = fs::metadata(&path)
.chain_err(|| {
internal(format!("failed to stat `{}`", path.display()))
})?;
Expand Down Expand Up @@ -227,11 +240,14 @@ impl Fingerprint {
if self.target != old.target {
bail!("target configuration has changed")
}
if self.path != old.path {
bail!("path to the compiler has changed")
}
if self.profile != old.profile {
bail!("profile configuration has changed")
}
if self.rustflags != old.rustflags {
return Err(internal("RUSTFLAGS has changed"))
bail!("RUSTFLAGS has changed")
}
if self.local.len() != old.local.len() {
bail!("local lens changed");
Expand Down Expand Up @@ -294,13 +310,14 @@ impl hash::Hash for Fingerprint {
rustc,
ref features,
target,
path,
profile,
ref deps,
ref local,
memoized_hash: _,
ref rustflags,
} = *self;
(rustc, features, target, profile, local, rustflags).hash(h);
(rustc, features, target, path, profile, local, rustflags).hash(h);

h.write_usize(deps.len());
for &(ref name, ref fingerprint) in deps {
Expand Down Expand Up @@ -375,8 +392,8 @@ fn calculate<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>)
// And finally, calculate what our own local fingerprint is
let local = if use_dep_info(unit) {
let dep_info = dep_info_loc(cx, unit);
let mtime = dep_info_mtime_if_fresh(&dep_info)?;
LocalFingerprint::MtimeBased(MtimeSlot(Mutex::new(mtime)), dep_info)
let mtime = dep_info_mtime_if_fresh(unit.pkg, &dep_info)?;
LocalFingerprint::mtime(cx.target_root(), mtime, &dep_info)
} else {
let fingerprint = pkg_fingerprint(cx, unit.pkg)?;
LocalFingerprint::Precalculated(fingerprint)
Expand All @@ -392,6 +409,9 @@ fn calculate<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>)
rustc: util::hash_u64(&cx.config.rustc()?.verbose_version),
target: util::hash_u64(&unit.target),
profile: util::hash_u64(&unit.profile),
// Note that .0 is hashed here, not .1 which is the cwd. That doesn't
// actually affect the output artifact so there's no need to hash it.
path: util::hash_u64(&super::path_args(cx, unit).0),
features: format!("{:?}", cx.resolve.features_sorted(unit.pkg.package_id())),
deps: deps,
local: vec![local],
Expand Down Expand Up @@ -443,6 +463,7 @@ pub fn prepare_build_cmd<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>)
rustc: 0,
target: 0,
profile: 0,
path: 0,
features: String::new(),
deps: Vec::new(),
local: local,
Expand All @@ -464,16 +485,17 @@ pub fn prepare_build_cmd<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>)
// build script.
let state = Arc::clone(&cx.build_state);
let key = (unit.pkg.package_id().clone(), unit.kind);
let root = unit.pkg.root().to_path_buf();
let pkg_root = unit.pkg.root().to_path_buf();
let target_root = cx.target_root().to_path_buf();
let write_fingerprint = Work::new(move |_| {
if let Some(output_path) = output_path {
let outputs = state.outputs.lock().unwrap();
let outputs = &outputs[&key];
if !outputs.rerun_if_changed.is_empty() ||
!outputs.rerun_if_env_changed.is_empty() {
let deps = BuildDeps::new(&output_path, Some(outputs));
fingerprint.local = local_fingerprints_deps(&deps, &root);
fingerprint.update_local()?;
fingerprint.local = local_fingerprints_deps(&deps, &target_root, &pkg_root);
fingerprint.update_local(&target_root)?;
}
}
write_fingerprint(&loc, &fingerprint)
Expand Down Expand Up @@ -516,18 +538,19 @@ fn build_script_local_fingerprints<'a, 'cfg>(cx: &mut Context<'a, 'cfg>,
// Ok so now we're in "new mode" where we can have files listed as
// dependencies as well as env vars listed as dependencies. Process them all
// here.
Ok((local_fingerprints_deps(deps, unit.pkg.root()), Some(output)))
Ok((local_fingerprints_deps(deps, cx.target_root(), unit.pkg.root()), Some(output)))
}

fn local_fingerprints_deps(deps: &BuildDeps, root: &Path) -> Vec<LocalFingerprint> {
fn local_fingerprints_deps(deps: &BuildDeps, target_root: &Path, pkg_root: &Path)
-> Vec<LocalFingerprint>
{
debug!("new local fingerprints deps");
let mut local = Vec::new();
if !deps.rerun_if_changed.is_empty() {
let output = &deps.build_script_output;
let deps = deps.rerun_if_changed.iter().map(|p| root.join(p));
let deps = deps.rerun_if_changed.iter().map(|p| pkg_root.join(p));
let mtime = mtime_if_fresh(output, deps);
let mtime = MtimeSlot(Mutex::new(mtime));
local.push(LocalFingerprint::MtimeBased(mtime, output.clone()));
local.push(LocalFingerprint::mtime(target_root, mtime, output));
}

for var in deps.rerun_if_env_changed.iter() {
Expand Down Expand Up @@ -584,51 +607,34 @@ fn log_compare(unit: &Unit, compare: &CargoResult<()>) {
};
info!("fingerprint error for {}: {}", unit.pkg, ce);

for cause in ce.iter() {
for cause in ce.iter().skip(1) {
info!(" cause: {}", cause);
}
}

// Parse the dep-info into a list of paths
pub fn parse_dep_info(dep_info: &Path) -> CargoResult<Option<Vec<PathBuf>>> {
macro_rules! fs_try {
($e:expr) => (match $e { Ok(e) => e, Err(..) => return Ok(None) })
}
let mut f = BufReader::new(fs_try!(File::open(dep_info)));
// see comments in append_current_dir for where this cwd is manifested from.
let mut cwd = Vec::new();
if fs_try!(f.read_until(0, &mut cwd)) == 0 {
return Ok(None)
}
let cwd = util::bytes2path(&cwd[..cwd.len()-1])?;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Am I correct that new versions of Cargo will fail to parse old depinfo, which is actually OK, because it'll cause only a rebuild, and not a build failure?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm I'm not actually sure what will happen but the hashes are all changing so I don't think newer cargo will use the same files from older cargo.

let line = match f.lines().next() {
Some(Ok(line)) => line,
_ => return Ok(None),
pub fn parse_dep_info(pkg: &Package, dep_info: &Path)
-> CargoResult<Option<Vec<PathBuf>>>
{
let data = match paths::read_bytes(dep_info) {
Ok(data) => data,
Err(_) => return Ok(None),
};
let pos = line.find(": ").ok_or_else(|| {
internal(format!("dep-info not in an understood format: {}",
dep_info.display()))
})?;
let deps = &line[pos + 2..];

let mut paths = Vec::new();
let mut deps = deps.split(' ').map(|s| s.trim()).filter(|s| !s.is_empty());
while let Some(s) = deps.next() {
let mut file = s.to_string();
while file.ends_with('\\') {
file.pop();
file.push(' ');
file.push_str(deps.next().ok_or_else(|| {
internal("malformed dep-info format, trailing \\".to_string())
})?);
}
paths.push(cwd.join(&file));
let paths = data.split(|&x| x == 0)
.filter(|x| !x.is_empty())
.map(|p| util::bytes2path(p).map(|p| pkg.root().join(p)))
.collect::<Result<Vec<_>, _>>()?;
if paths.len() == 0 {
Ok(None)
} else {
Ok(Some(paths))
}
Ok(Some(paths))
}

fn dep_info_mtime_if_fresh(dep_info: &Path) -> CargoResult<Option<FileTime>> {
if let Some(paths) = parse_dep_info(dep_info)? {
fn dep_info_mtime_if_fresh(pkg: &Package, dep_info: &Path)
-> CargoResult<Option<FileTime>>
{
if let Some(paths) = parse_dep_info(pkg, dep_info)? {
Ok(mtime_if_fresh(dep_info, paths.iter()))
} else {
Ok(None)
Expand Down Expand Up @@ -704,19 +710,55 @@ fn filename<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> String {
format!("{}{}-{}", flavor, kind, file_stem)
}

// The dep-info files emitted by the compiler all have their listed paths
// relative to whatever the current directory was at the time that the compiler
// was invoked. As the current directory may change over time, we need to record
// what that directory was at the beginning of the file so we can know about it
// next time.
pub fn append_current_dir(path: &Path, cwd: &Path) -> CargoResult<()> {
debug!("appending {} <- {}", path.display(), cwd.display());
let mut f = OpenOptions::new().read(true).write(true).open(path)?;
let mut contents = Vec::new();
f.read_to_end(&mut contents)?;
f.seek(SeekFrom::Start(0))?;
f.write_all(util::path2bytes(cwd)?)?;
f.write_all(&[0])?;
f.write_all(&contents)?;
/// Parses the dep-info file coming out of rustc into a Cargo-specific format.
///
/// This function will parse `rustc_dep_info` as a makefile-style dep info to
/// learn about the all files which a crate depends on. This is then
/// re-serialized into the `cargo_dep_info` path in a Cargo-specific format.
///
/// The `pkg_root` argument here is the absolute path to the directory
/// containing `Cargo.toml` for this crate that was compiled. The paths listed
/// in the rustc dep-info file may or may not be absolute but we'll want to
/// consider all of them relative to the `root` specified.
///
/// The `rustc_cwd` argument is the absolute path to the cwd of the compiler
/// when it was invoked.
///
/// The serialized Cargo format will contain a list of files, all of which are
/// relative if they're under `root`. or absolute if they're elsewehre.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, weren't depinfo files, produced by Cargo, used by some other tools? #3557

So, this change will break such clients :(

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh sure!

I think though (at least the intention) that these files are just those in .fingerprint, so the ones like target/debug/foo.d should still be present unmangled from rustc itself.

pub fn translate_dep_info(rustc_dep_info: &Path,
cargo_dep_info: &Path,
pkg_root: &Path,
rustc_cwd: &Path) -> CargoResult<()> {
let contents = paths::read(rustc_dep_info)?;
let line = match contents.lines().next() {
Some(line) => line,
None => return Ok(()),
};
let pos = line.find(": ").ok_or_else(|| {
internal(format!("dep-info not in an understood format: {}",
rustc_dep_info.display()))
})?;
let deps = &line[pos + 2..];

let mut new_contents = Vec::new();
let mut deps = deps.split(' ').map(|s| s.trim()).filter(|s| !s.is_empty());
while let Some(s) = deps.next() {
let mut file = s.to_string();
while file.ends_with('\\') {
file.pop();
file.push(' ');
file.push_str(deps.next().ok_or_else(|| {
internal("malformed dep-info format, trailing \\".to_string())
})?);
}

let absolute = rustc_cwd.join(file);
let path = absolute.strip_prefix(pkg_root).unwrap_or(&absolute);
new_contents.extend(util::path2bytes(path)?);
new_contents.push(0);
}
paths::write(cargo_dep_info, &new_contents)?;
Ok(())
}

Loading