From e1ba760fd04c1c94d05d4fa8069110c606786cbc Mon Sep 17 00:00:00 2001 From: Jake Goulding Date: Fri, 15 Jul 2016 16:55:29 -0400 Subject: [PATCH 1/4] Allow moving files into the future as well --- tests/cargotest/support/paths.rs | 38 +++++++++++++++++++++++--------- 1 file changed, 27 insertions(+), 11 deletions(-) diff --git a/tests/cargotest/support/paths.rs b/tests/cargotest/support/paths.rs index 62e3b548d2d..bf3b6d446cc 100644 --- a/tests/cargotest/support/paths.rs +++ b/tests/cargotest/support/paths.rs @@ -58,7 +58,17 @@ pub fn home() -> PathBuf { pub trait CargoPathExt { fn rm_rf(&self); fn mkdir_p(&self); - fn move_into_the_past(&self); + + fn move_into_the_past(&self) { + self.move_in_time(|sec, nsec| (sec - 3600, nsec)) + } + + fn move_into_the_future(&self) { + self.move_in_time(|sec, nsec| (sec + 3600, nsec)) + } + + fn move_in_time(&self, F) + where F: Fn(u64, u32) -> (u64, u32); } impl CargoPathExt for Path { @@ -91,31 +101,37 @@ impl CargoPathExt for Path { }) } - fn move_into_the_past(&self) { + fn move_in_time(&self, travel_amount: F) + where F: Fn(u64, u32) -> ((u64, u32)), + { if self.is_file() { - time_travel(self); + time_travel(self, &travel_amount); } else { - recurse(self, &self.join("target")); + recurse(self, &self.join("target"), &travel_amount); } - fn recurse(p: &Path, bad: &Path) { + fn recurse(p: &Path, bad: &Path, travel_amount: &F) + where F: Fn(u64, u32) -> ((u64, u32)), + { if p.is_file() { - time_travel(p) + time_travel(p, travel_amount) } else if !p.starts_with(bad) { for f in t!(fs::read_dir(p)) { let f = t!(f).path(); - recurse(&f, bad); + recurse(&f, bad, travel_amount); } } } - fn time_travel(path: &Path) { + fn time_travel(path: &Path, travel_amount: &F) + where F: Fn(u64, u32) -> ((u64, u32)), + { let stat = t!(path.metadata()); let mtime = FileTime::from_last_modification_time(&stat); - let newtime = mtime.seconds_relative_to_1970() - 3600; - let nanos = mtime.nanoseconds(); - let newtime = FileTime::from_seconds_since_1970(newtime, nanos); + + let (sec, nsec) = travel_amount(mtime.seconds_relative_to_1970(), mtime.nanoseconds()); + let newtime = FileTime::from_seconds_since_1970(sec, nsec); // Sadly change_file_times has a failure mode where a readonly file // cannot have its times changed on windows. From a7c457e8033c2b0d0b107ed2f14d39315393aff2 Mon Sep 17 00:00:00 2001 From: Jake Goulding Date: Fri, 15 Jul 2016 10:31:06 -0400 Subject: [PATCH 2/4] Add tests --- tests/freshness.rs | 65 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/tests/freshness.rs b/tests/freshness.rs index f5173bebcd5..ae627548679 100644 --- a/tests/freshness.rs +++ b/tests/freshness.rs @@ -360,3 +360,68 @@ fn same_build_dir_cached_packages() { [COMPILING] a2 v0.0.1 ({dir}/a2) ", dir = p.url()))); } + +#[test] +fn no_rebuild_if_build_artifacts_move_backwards_in_time() { + let p = project("backwards_in_time") + .file("Cargo.toml", r#" + [package] + name = "backwards_in_time" + version = "0.0.1" + authors = [] + + [dependencies] + a = { path = "a" } + "#) + .file("src/lib.rs", "") + .file("a/Cargo.toml", r#" + [package] + name = "a" + version = "0.0.1" + authors = [] + "#) + .file("a/src/lib.rs", ""); + + assert_that(p.cargo_process("build"), + execs().with_status(0)); + + p.root().move_into_the_past(); + p.root().join("target").move_into_the_past(); + + assert_that(p.cargo("build").env("RUST_LOG", ""), + execs().with_status(0).with_stdout("").with_stderr("")); +} + +#[test] +fn rebuild_if_build_artifacts_move_forward_in_time() { + let p = project("forwards_in_time") + .file("Cargo.toml", r#" + [package] + name = "forwards_in_time" + version = "0.0.1" + authors = [] + + [dependencies] + a = { path = "a" } + "#) + .file("src/lib.rs", "") + .file("a/Cargo.toml", r#" + [package] + name = "a" + version = "0.0.1" + authors = [] + "#) + .file("a/src/lib.rs", ""); + + assert_that(p.cargo_process("build"), + execs().with_status(0)); + + p.root().move_into_the_future(); + p.root().join("target").move_into_the_future(); + + assert_that(p.cargo("build").env("RUST_LOG", ""), + execs().with_status(0).with_stdout("").with_stderr("\ +[COMPILING] a v0.0.1 ([..]) +[COMPILING] forwards_in_time v0.0.1 ([..]) +")); +} From bcbe0c0b00b29ab301b58cf0045a99f35917bdf2 Mon Sep 17 00:00:00 2001 From: Jake Goulding Date: Thu, 14 Jul 2016 19:17:08 -0400 Subject: [PATCH 3/4] Files modified in the past should not trigger a fingerprint change Previously, we would bail from the fingerprint computation if the old and new mtimes had changed in *any* way. This caused some issues of rebuilding with filesystems that do not preserve nanosecond granularity (#2874). --- src/cargo/ops/cargo_rustc/fingerprint.rs | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/src/cargo/ops/cargo_rustc/fingerprint.rs b/src/cargo/ops/cargo_rustc/fingerprint.rs index 2bef5a90774..4b16a4a2ee6 100644 --- a/src/cargo/ops/cargo_rustc/fingerprint.rs +++ b/src/cargo/ops/cargo_rustc/fingerprint.rs @@ -172,13 +172,21 @@ impl Fingerprint { a, b) } } - (&LocalFingerprint::MtimeBased(ref a, ref ap), - &LocalFingerprint::MtimeBased(ref b, ref bp)) => { - let a = a.0.lock().unwrap(); - let b = b.0.lock().unwrap(); - if *a != *b { - bail!("mtime based components have changed: {:?} != {:?}, \ - paths are {:?} and {:?}", *a, *b, ap, bp) + (&LocalFingerprint::MtimeBased(ref on_disk_mtime, ref ap), + &LocalFingerprint::MtimeBased(ref previously_built_mtime, ref bp)) => { + let on_disk_mtime = on_disk_mtime.0.lock().unwrap(); + let previously_built_mtime = previously_built_mtime.0.lock().unwrap(); + + let should_rebuild = match (*on_disk_mtime, *previously_built_mtime) { + (None, None) => false, + (Some(_), None) | (None, Some(_)) => true, + (Some(on_disk), Some(previously_built)) => on_disk > previously_built, + }; + + if should_rebuild { + bail!("mtime based components have changed: previously {:?} now {:?}, \ + paths are {:?} and {:?}", + *previously_built_mtime, *on_disk_mtime, ap, bp) } } _ => bail!("local fingerprint type has changed"), From db6db929b183357e98ea113384c33ea90f618958 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 2 Nov 2016 09:18:30 -0700 Subject: [PATCH 4/4] Fix some tests from the previous merge --- tests/freshness.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/freshness.rs b/tests/freshness.rs index 63a488cd3b9..03baf550ee5 100644 --- a/tests/freshness.rs +++ b/tests/freshness.rs @@ -396,10 +396,11 @@ fn no_rebuild_if_build_artifacts_move_backwards_in_time() { execs().with_status(0)); p.root().move_into_the_past(); - p.root().join("target").move_into_the_past(); - assert_that(p.cargo("build").env("RUST_LOG", ""), - execs().with_status(0).with_stdout("").with_stderr("")); + assert_that(p.cargo("build"), + execs().with_status(0).with_stdout("").with_stderr("\ +[FINISHED] [..] +")); } #[test] @@ -427,11 +428,11 @@ fn rebuild_if_build_artifacts_move_forward_in_time() { execs().with_status(0)); p.root().move_into_the_future(); - p.root().join("target").move_into_the_future(); assert_that(p.cargo("build").env("RUST_LOG", ""), execs().with_status(0).with_stdout("").with_stderr("\ [COMPILING] a v0.0.1 ([..]) [COMPILING] forwards_in_time v0.0.1 ([..]) +[FINISHED] [..] ")); }