Auto merge of #3246 - alexcrichton:mtimes-in-the-past, r=alexcrichton

Continuation of #2880

Just applied a patch to help fix the tests.

src/cargo/ops/cargo_rustc/fingerprint.rs

@@ -191,13 +191,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"),

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<F>(&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<F>(&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<F>(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<F>(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.

tests/freshness.rs

@@ -370,3 +370,69 @@ fn same_build_dir_cached_packages() {
 [FINISHED] debug [unoptimized + debuginfo] target(s) in [..]
 ", 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();
+
+    assert_that(p.cargo("build"),
+                execs().with_status(0).with_stdout("").with_stderr("\
+[FINISHED] [..]
+"));
+}
+
+#[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();
+
+    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] [..]
+"));
+}