From b7ecbaf2243996b955f58a0289924a52a94eb689 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 21 Aug 2018 16:13:11 +0200 Subject: [PATCH 1/3] fix cargo not doing anything when the input and output mtimes are equal That's a problem as the input may have changed in that same second but after the output got generated! --- src/cargo/core/compiler/fingerprint.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/cargo/core/compiler/fingerprint.rs b/src/cargo/core/compiler/fingerprint.rs index e3a70638134..c2352602dd4 100644 --- a/src/cargo/core/compiler/fingerprint.rs +++ b/src/cargo/core/compiler/fingerprint.rs @@ -744,7 +744,11 @@ where return true; } }; - if mtime2 > mtime { + // Equal mtimes could mean that the input was changed in that same second, + // but *after* the output was generated. So this means they are stale. + // In theory, cargo is using nanosecond precision throughout so this + // should not make a difference -- but it was necessary to fix #5918. + if mtime2 >= mtime { info!("stale: {} -- {} vs {}", path.display(), mtime2, mtime); true } else { From 08843a045775855d90b451d6c46d68075beaafd9 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 21 Aug 2018 18:14:45 +0200 Subject: [PATCH 2/3] attempt to fix test suite race on mac --- tests/testsuite/build_script.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/testsuite/build_script.rs b/tests/testsuite/build_script.rs index 2bd9d9a5baf..d118f431db0 100644 --- a/tests/testsuite/build_script.rs +++ b/tests/testsuite/build_script.rs @@ -2545,6 +2545,7 @@ fn rebuild_only_on_explicit_paths() { sleep_ms(1000); File::create(p.root().join("foo")).unwrap(); File::create(p.root().join("bar")).unwrap(); + sleep_ms(1000); // make sure the to-be-created outfile has a timestamp distinct from the infiles // now the exist, so run once, catch the mtime, then shouldn't run again println!("run with"); From b1fbafd066982f4d741c0ec7692aa5a41ec2d489 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 22 Aug 2018 20:55:14 +0200 Subject: [PATCH 3/3] update comment based on further research --- src/cargo/core/compiler/fingerprint.rs | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/src/cargo/core/compiler/fingerprint.rs b/src/cargo/core/compiler/fingerprint.rs index c2352602dd4..eace5217c91 100644 --- a/src/cargo/core/compiler/fingerprint.rs +++ b/src/cargo/core/compiler/fingerprint.rs @@ -744,10 +744,23 @@ where return true; } }; - // Equal mtimes could mean that the input was changed in that same second, - // but *after* the output was generated. So this means they are stale. - // In theory, cargo is using nanosecond precision throughout so this - // should not make a difference -- but it was necessary to fix #5918. + + // Note that equal mtimes are considered "stale". For filesystems with + // not much timestamp precision like 1s this is a conservative approximation + // to handle the case where a file is modified within the same second after + // a build finishes. We want to make sure that incremental rebuilds pick that up! + // + // For filesystems with nanosecond precision it's been seen in the wild that + // its "nanosecond precision" isn't really nanosecond-accurate. It turns out that + // kernels may cache the current time so files created at different times actually + // list the same nanosecond precision. Some digging on #5919 picked up that the + // kernel caches the current time between timer ticks, which could mean that if + // a file is updated at most 10ms after a build finishes then Cargo may not + // pick up the build changes. + // + // All in all, the equality check here is a conservative assumption that, + // if equal, files were changed just after a previous build finished. + // It's hoped this doesn't cause too many issues in practice! if mtime2 >= mtime { info!("stale: {} -- {} vs {}", path.display(), mtime2, mtime); true