Skip to content

Commit 4005bc4

Browse files
borsbors
committed
Auto merge of #4788 - alexcrichton:rename-works, r=matklad
Avoid rebuilding a project when cwd changes This commit is targeted at solving a use case which typically comes up during CI builds -- the `target` directory is cached between builds but the cwd of the build changes over time. For example the following scenario can happen: 1. A project is compiled at `/projects/a`. 2. The `target` directory is cached. 3. A new build is started in `/projects/b`. 4. The previous `target` directory is restored to `/projects/b`. 5. The build start, and Cargo rebuilds everything. The last piece of behavior is indeed unfortunate! Cargo's internal hashing currently isn't that resilient to changing cwd and this PR aims to help improve the situation! The first point of too-much-hashing came up with `Target::src_path`. Each `Target` was hashed and stored for all compilations, and the `src_path` field was an absolute path on the filesystem to the file that needed to be compiled. This path then changed over time when cwd changed, but otherwise everything else remained the same! This commit updates the handling of the `src_path` field to simply ignore it when hashing. Instead the path we actually pass to rustc is later calculated and then passed to the fingerprint calculation. The next problem this fixes is that the dep info files were augmented after creation to have the cwd of the compiler at the time to find the files at a later date. This, unfortunately, would cause issues if the cwd itself changed. Instead the cwd is now left out of dep-info files (they're no longer augmented) and instead the cwd is recalculated when parsing the dep info later. The final problem that this commit fixes is actually an existing issue in Cargo today. Right now you can actually execute `cargo build` from anywhere in a project and Cargo will execute the build. Unfortunately though the argument to rustc was actually different depending on what directory you were in (the compiler was invoked with a path relative to cwd). This path ends up being used for metadata like debuginfo which means that different directories would cause different artifacts to be created, but Cargo wouldn't rerun the compiler! To fix this issue the matter of cwd is now entirely excluded from compilation command lines. Instead rustc is unconditionally invoked with a relative path *if* the path is underneath the workspace root, and otherwise it's invoked as an absolute path (in which case the cwd doesn't matter). Once all these fixes were added up it means that now we can have projects where if you move the entire directory Cargo won't rebuild the original source! Note that this may be a bit of a breaking change, however. This means that the paths in error messages for cargo will no longer be unconditionally relative to the current working directory, but rather relative to the root of the workspace itself. Unfortunately this is moreso of a feature right now rather than a bug, so it may be one that we just have to stomach. Closes #3273
2 parents bc9ffdf + f688e9c commit 4005bc4

File tree

6 files changed

+208
-105
lines changed

6 files changed

+208
-105
lines changed

‎src/cargo/core/manifest.rs

+20-4
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use std::collections::{HashMap, BTreeMap};
22
use std::fmt;
33
use std::path::{PathBuf, Path};
44
use std::rc::Rc;
5+
use std::hash::{Hash, Hasher};
56

67
use semver::Version;
78
use serde::ser;
@@ -199,7 +200,11 @@ pub struct Profiles {
199200
pub struct Target {
200201
kind: TargetKind,
201202
name: String,
202-
src_path: PathBuf,
203+
// Note that the `src_path` here is excluded from the `Hash` implementation
204+
// as it's absolute currently and is otherwise a little too brittle for
205+
// causing rebuilds. Instead the hash for the path that we send to the
206+
// compiler is handled elsewhere.
207+
src_path: NonHashedPathBuf,
203208
required_features: Option<Vec<String>>,
204209
tested: bool,
205210
benched: bool,
@@ -209,6 +214,17 @@ pub struct Target {
209214
for_host: bool,
210215
}
211216

217+
#[derive(Clone, PartialEq, Eq, Debug)]
218+
struct NonHashedPathBuf {
219+
path: PathBuf,
220+
}
221+
222+
impl Hash for NonHashedPathBuf {
223+
fn hash<H: Hasher>(&self, _: &mut H) {
224+
// ...
225+
}
226+
}
227+
212228
#[derive(Serialize)]
213229
struct SerializedTarget<'a> {
214230
/// Is this a `--bin bin`, `--lib`, `--example ex`?
@@ -227,7 +243,7 @@ impl ser::Serialize for Target {
227243
kind: &self.kind,
228244
crate_types: self.rustc_crate_types(),
229245
name: &self.name,
230-
src_path: &self.src_path,
246+
src_path: &self.src_path.path,
231247
}.serialize(s)
232248
}
233249
}
@@ -370,7 +386,7 @@ impl Target {
370386
Target {
371387
kind: TargetKind::Bin,
372388
name: String::new(),
373-
src_path: src_path,
389+
src_path: NonHashedPathBuf { path: src_path },
374390
required_features: None,
375391
doc: false,
376392
doctest: false,
@@ -459,7 +475,7 @@ impl Target {
459475

460476
pub fn name(&self) -> &str { &self.name }
461477
pub fn crate_name(&self) -> String { self.name.replace("-", "_") }
462-
pub fn src_path(&self) -> &Path { &self.src_path }
478+
pub fn src_path(&self) -> &Path { &self.src_path.path }
463479
pub fn required_features(&self) -> Option<&Vec<String>> { self.required_features.as_ref() }
464480
pub fn kind(&self) -> &TargetKind { &self.kind }
465481
pub fn tested(&self) -> bool { self.tested }

‎src/cargo/ops/cargo_rustc/fingerprint.rs

+110-68
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
use std::env;
2-
use std::fs::{self, File, OpenOptions};
2+
use std::fs;
33
use std::hash::{self, Hasher};
4-
use std::io::prelude::*;
5-
use std::io::{BufReader, SeekFrom};
64
use std::path::{Path, PathBuf};
75
use std::sync::{Arc, Mutex};
86

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

10199
let allow_failure = unit.profile.rustc_args.is_some();
100+
let target_root = cx.target_root().to_path_buf();
102101
let write_fingerprint = Work::new(move |_| {
103-
match fingerprint.update_local() {
102+
match fingerprint.update_local(&target_root) {
104103
Ok(()) => {}
105104
Err(..) if allow_failure => return Ok(()),
106105
Err(e) => return Err(e)
@@ -139,6 +138,7 @@ pub struct Fingerprint {
139138
features: String,
140139
target: u64,
141140
profile: u64,
141+
path: u64,
142142
#[serde(serialize_with = "serialize_deps", deserialize_with = "deserialize_deps")]
143143
deps: Vec<(String, Arc<Fingerprint>)>,
144144
local: Vec<LocalFingerprint>,
@@ -165,6 +165,7 @@ fn deserialize_deps<'de, D>(d: D) -> Result<Vec<(String, Arc<Fingerprint>)>, D::
165165
rustc: 0,
166166
target: 0,
167167
profile: 0,
168+
path: 0,
168169
local: vec![LocalFingerprint::Precalculated(String::new())],
169170
features: String::new(),
170171
deps: Vec::new(),
@@ -181,15 +182,27 @@ enum LocalFingerprint {
181182
EnvBased(String, Option<String>),
182183
}
183184

185+
impl LocalFingerprint {
186+
fn mtime(root: &Path, mtime: Option<FileTime>, path: &Path)
187+
-> LocalFingerprint
188+
{
189+
let mtime = MtimeSlot(Mutex::new(mtime));
190+
assert!(path.is_absolute());
191+
let path = path.strip_prefix(root).unwrap_or(path);
192+
LocalFingerprint::MtimeBased(mtime, path.to_path_buf())
193+
}
194+
}
195+
184196
struct MtimeSlot(Mutex<Option<FileTime>>);
185197

186198
impl Fingerprint {
187-
fn update_local(&self) -> CargoResult<()> {
199+
fn update_local(&self, root: &Path) -> CargoResult<()> {
188200
let mut hash_busted = false;
189201
for local in self.local.iter() {
190202
match *local {
191203
LocalFingerprint::MtimeBased(ref slot, ref path) => {
192-
let meta = fs::metadata(path)
204+
let path = root.join(path);
205+
let meta = fs::metadata(&path)
193206
.chain_err(|| {
194207
internal(format!("failed to stat `{}`", path.display()))
195208
})?;
@@ -227,11 +240,14 @@ impl Fingerprint {
227240
if self.target != old.target {
228241
bail!("target configuration has changed")
229242
}
243+
if self.path != old.path {
244+
bail!("path to the compiler has changed")
245+
}
230246
if self.profile != old.profile {
231247
bail!("profile configuration has changed")
232248
}
233249
if self.rustflags != old.rustflags {
234-
return Err(internal("RUSTFLAGS has changed"))
250+
bail!("RUSTFLAGS has changed")
235251
}
236252
if self.local.len() != old.local.len() {
237253
bail!("local lens changed");
@@ -294,13 +310,14 @@ impl hash::Hash for Fingerprint {
294310
rustc,
295311
ref features,
296312
target,
313+
path,
297314
profile,
298315
ref deps,
299316
ref local,
300317
memoized_hash: _,
301318
ref rustflags,
302319
} = *self;
303-
(rustc, features, target, profile, local, rustflags).hash(h);
320+
(rustc, features, target, path, profile, local, rustflags).hash(h);
304321

305322
h.write_usize(deps.len());
306323
for &(ref name, ref fingerprint) in deps {
@@ -375,8 +392,8 @@ fn calculate<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>)
375392
// And finally, calculate what our own local fingerprint is
376393
let local = if use_dep_info(unit) {
377394
let dep_info = dep_info_loc(cx, unit);
378-
let mtime = dep_info_mtime_if_fresh(&dep_info)?;
379-
LocalFingerprint::MtimeBased(MtimeSlot(Mutex::new(mtime)), dep_info)
395+
let mtime = dep_info_mtime_if_fresh(unit.pkg, &dep_info)?;
396+
LocalFingerprint::mtime(cx.target_root(), mtime, &dep_info)
380397
} else {
381398
let fingerprint = pkg_fingerprint(cx, unit.pkg)?;
382399
LocalFingerprint::Precalculated(fingerprint)
@@ -392,6 +409,9 @@ fn calculate<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>)
392409
rustc: util::hash_u64(&cx.config.rustc()?.verbose_version),
393410
target: util::hash_u64(&unit.target),
394411
profile: util::hash_u64(&unit.profile),
412+
// Note that .0 is hashed here, not .1 which is the cwd. That doesn't
413+
// actually affect the output artifact so there's no need to hash it.
414+
path: util::hash_u64(&super::path_args(cx, unit).0),
395415
features: format!("{:?}", cx.resolve.features_sorted(unit.pkg.package_id())),
396416
deps: deps,
397417
local: vec![local],
@@ -443,6 +463,7 @@ pub fn prepare_build_cmd<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>)
443463
rustc: 0,
444464
target: 0,
445465
profile: 0,
466+
path: 0,
446467
features: String::new(),
447468
deps: Vec::new(),
448469
local: local,
@@ -464,16 +485,17 @@ pub fn prepare_build_cmd<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>)
464485
// build script.
465486
let state = Arc::clone(&cx.build_state);
466487
let key = (unit.pkg.package_id().clone(), unit.kind);
467-
let root = unit.pkg.root().to_path_buf();
488+
let pkg_root = unit.pkg.root().to_path_buf();
489+
let target_root = cx.target_root().to_path_buf();
468490
let write_fingerprint = Work::new(move |_| {
469491
if let Some(output_path) = output_path {
470492
let outputs = state.outputs.lock().unwrap();
471493
let outputs = &outputs[&key];
472494
if !outputs.rerun_if_changed.is_empty() ||
473495
!outputs.rerun_if_env_changed.is_empty() {
474496
let deps = BuildDeps::new(&output_path, Some(outputs));
475-
fingerprint.local = local_fingerprints_deps(&deps, &root);
476-
fingerprint.update_local()?;
497+
fingerprint.local = local_fingerprints_deps(&deps, &target_root, &pkg_root);
498+
fingerprint.update_local(&target_root)?;
477499
}
478500
}
479501
write_fingerprint(&loc, &fingerprint)
@@ -516,18 +538,19 @@ fn build_script_local_fingerprints<'a, 'cfg>(cx: &mut Context<'a, 'cfg>,
516538
// Ok so now we're in "new mode" where we can have files listed as
517539
// dependencies as well as env vars listed as dependencies. Process them all
518540
// here.
519-
Ok((local_fingerprints_deps(deps, unit.pkg.root()), Some(output)))
541+
Ok((local_fingerprints_deps(deps, cx.target_root(), unit.pkg.root()), Some(output)))
520542
}
521543

522-
fn local_fingerprints_deps(deps: &BuildDeps, root: &Path) -> Vec<LocalFingerprint> {
544+
fn local_fingerprints_deps(deps: &BuildDeps, target_root: &Path, pkg_root: &Path)
545+
-> Vec<LocalFingerprint>
546+
{
523547
debug!("new local fingerprints deps");
524548
let mut local = Vec::new();
525549
if !deps.rerun_if_changed.is_empty() {
526550
let output = &deps.build_script_output;
527-
let deps = deps.rerun_if_changed.iter().map(|p| root.join(p));
551+
let deps = deps.rerun_if_changed.iter().map(|p| pkg_root.join(p));
528552
let mtime = mtime_if_fresh(output, deps);
529-
let mtime = MtimeSlot(Mutex::new(mtime));
530-
local.push(LocalFingerprint::MtimeBased(mtime, output.clone()));
553+
local.push(LocalFingerprint::mtime(target_root, mtime, output));
531554
}
532555

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

587-
for cause in ce.iter() {
610+
for cause in ce.iter().skip(1) {
588611
info!(" cause: {}", cause);
589612
}
590613
}
591614

592615
// Parse the dep-info into a list of paths
593-
pub fn parse_dep_info(dep_info: &Path) -> CargoResult<Option<Vec<PathBuf>>> {
594-
macro_rules! fs_try {
595-
($e:expr) => (match $e { Ok(e) => e, Err(..) => return Ok(None) })
596-
}
597-
let mut f = BufReader::new(fs_try!(File::open(dep_info)));
598-
// see comments in append_current_dir for where this cwd is manifested from.
599-
let mut cwd = Vec::new();
600-
if fs_try!(f.read_until(0, &mut cwd)) == 0 {
601-
return Ok(None)
602-
}
603-
let cwd = util::bytes2path(&cwd[..cwd.len()-1])?;
604-
let line = match f.lines().next() {
605-
Some(Ok(line)) => line,
606-
_ => return Ok(None),
616+
pub fn parse_dep_info(pkg: &Package, dep_info: &Path)
617+
-> CargoResult<Option<Vec<PathBuf>>>
618+
{
619+
let data = match paths::read_bytes(dep_info) {
620+
Ok(data) => data,
621+
Err(_) => return Ok(None),
607622
};
608-
let pos = line.find(": ").ok_or_else(|| {
609-
internal(format!("dep-info not in an understood format: {}",
610-
dep_info.display()))
611-
})?;
612-
let deps = &line[pos + 2..];
613-
614-
let mut paths = Vec::new();
615-
let mut deps = deps.split(' ').map(|s| s.trim()).filter(|s| !s.is_empty());
616-
while let Some(s) = deps.next() {
617-
let mut file = s.to_string();
618-
while file.ends_with('\\') {
619-
file.pop();
620-
file.push(' ');
621-
file.push_str(deps.next().ok_or_else(|| {
622-
internal("malformed dep-info format, trailing \\".to_string())
623-
})?);
624-
}
625-
paths.push(cwd.join(&file));
623+
let paths = data.split(|&x| x == 0)
624+
.filter(|x| !x.is_empty())
625+
.map(|p| util::bytes2path(p).map(|p| pkg.root().join(p)))
626+
.collect::<Result<Vec<_>, _>>()?;
627+
if paths.len() == 0 {
628+
Ok(None)
629+
} else {
630+
Ok(Some(paths))
626631
}
627-
Ok(Some(paths))
628632
}
629633

630-
fn dep_info_mtime_if_fresh(dep_info: &Path) -> CargoResult<Option<FileTime>> {
631-
if let Some(paths) = parse_dep_info(dep_info)? {
634+
fn dep_info_mtime_if_fresh(pkg: &Package, dep_info: &Path)
635+
-> CargoResult<Option<FileTime>>
636+
{
637+
if let Some(paths) = parse_dep_info(pkg, dep_info)? {
632638
Ok(mtime_if_fresh(dep_info, paths.iter()))
633639
} else {
634640
Ok(None)
@@ -704,19 +710,55 @@ fn filename<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> String {
704710
format!("{}{}-{}", flavor, kind, file_stem)
705711
}
706712

707-
// The dep-info files emitted by the compiler all have their listed paths
708-
// relative to whatever the current directory was at the time that the compiler
709-
// was invoked. As the current directory may change over time, we need to record
710-
// what that directory was at the beginning of the file so we can know about it
711-
// next time.
712-
pub fn append_current_dir(path: &Path, cwd: &Path) -> CargoResult<()> {
713-
debug!("appending {} <- {}", path.display(), cwd.display());
714-
let mut f = OpenOptions::new().read(true).write(true).open(path)?;
715-
let mut contents = Vec::new();
716-
f.read_to_end(&mut contents)?;
717-
f.seek(SeekFrom::Start(0))?;
718-
f.write_all(util::path2bytes(cwd)?)?;
719-
f.write_all(&[0])?;
720-
f.write_all(&contents)?;
713+
/// Parses the dep-info file coming out of rustc into a Cargo-specific format.
714+
///
715+
/// This function will parse `rustc_dep_info` as a makefile-style dep info to
716+
/// learn about the all files which a crate depends on. This is then
717+
/// re-serialized into the `cargo_dep_info` path in a Cargo-specific format.
718+
///
719+
/// The `pkg_root` argument here is the absolute path to the directory
720+
/// containing `Cargo.toml` for this crate that was compiled. The paths listed
721+
/// in the rustc dep-info file may or may not be absolute but we'll want to
722+
/// consider all of them relative to the `root` specified.
723+
///
724+
/// The `rustc_cwd` argument is the absolute path to the cwd of the compiler
725+
/// when it was invoked.
726+
///
727+
/// The serialized Cargo format will contain a list of files, all of which are
728+
/// relative if they're under `root`. or absolute if they're elsewehre.
729+
pub fn translate_dep_info(rustc_dep_info: &Path,
730+
cargo_dep_info: &Path,
731+
pkg_root: &Path,
732+
rustc_cwd: &Path) -> CargoResult<()> {
733+
let contents = paths::read(rustc_dep_info)?;
734+
let line = match contents.lines().next() {
735+
Some(line) => line,
736+
None => return Ok(()),
737+
};
738+
let pos = line.find(": ").ok_or_else(|| {
739+
internal(format!("dep-info not in an understood format: {}",
740+
rustc_dep_info.display()))
741+
})?;
742+
let deps = &line[pos + 2..];
743+
744+
let mut new_contents = Vec::new();
745+
let mut deps = deps.split(' ').map(|s| s.trim()).filter(|s| !s.is_empty());
746+
while let Some(s) = deps.next() {
747+
let mut file = s.to_string();
748+
while file.ends_with('\\') {
749+
file.pop();
750+
file.push(' ');
751+
file.push_str(deps.next().ok_or_else(|| {
752+
internal("malformed dep-info format, trailing \\".to_string())
753+
})?);
754+
}
755+
756+
let absolute = rustc_cwd.join(file);
757+
let path = absolute.strip_prefix(pkg_root).unwrap_or(&absolute);
758+
new_contents.extend(util::path2bytes(path)?);
759+
new_contents.push(0);
760+
}
761+
paths::write(cargo_dep_info, &new_contents)?;
721762
Ok(())
722763
}
764+

0 commit comments

Comments
 (0)