diff --git a/src/cargo/core/compiler/custom_build.rs b/src/cargo/core/compiler/custom_build.rs index fad916d0443..c413ddcb66a 100644 --- a/src/cargo/core/compiler/custom_build.rs +++ b/src/cargo/core/compiler/custom_build.rs @@ -332,6 +332,7 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoRes state.build_plan(invocation_name, cmd.clone(), Arc::new(Vec::new())); } else { state.running(&cmd); + let timestamp = paths::get_current_filesystem_time(&output_file)?; let output = if extra_verbose { let prefix = format!("[{} {}] ", id.name(), id.version()); state.capture_output(&cmd, Some(prefix), true) @@ -354,6 +355,7 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoRes // state informing what variables were discovered via our script as // well. paths::write(&output_file, &output.stdout)?; + filetime::set_file_times(output_file, timestamp, timestamp)?; paths::write(&err_file, &output.stderr)?; paths::write(&root_output_file, util::path2bytes(&script_out_dir)?)?; let parsed_output = diff --git a/src/cargo/core/compiler/fingerprint.rs b/src/cargo/core/compiler/fingerprint.rs index 62daeed05fa..0b34da51fd7 100644 --- a/src/cargo/core/compiler/fingerprint.rs +++ b/src/cargo/core/compiler/fingerprint.rs @@ -761,23 +761,25 @@ where } }; - // Note that equal mtimes are considered "stale". For filesystems with - // not much timestamp precision like 1s this is a conservative approximation + // TODO: fix #5918 + // Note that equal mtimes should be considered "stale". For filesystems with + // not much timestamp precision like 1s this is would be 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! + // a build starts. 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 + // a file is updated at most 10ms after a build starts then Cargo may not // pick up the build changes. // - // All in all, the equality check here is a conservative assumption that, + // All in all, an equality check here would be 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 { + // Unfortunately this became problematic when (in #6484) cargo switch to more accurately + // measuring the start time of builds. + if mtime2 > mtime { info!("stale: {} -- {} vs {}", path.display(), mtime2, mtime); true } else { diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index 1c6f47b41e4..735b8f0e17c 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -292,6 +292,7 @@ fn rustc<'a, 'cfg>( } state.running(&rustc); + let timestamp = paths::get_current_filesystem_time(&dep_info_loc)?; if json_messages { exec.exec_json( rustc, @@ -334,6 +335,7 @@ fn rustc<'a, 'cfg>( rustc_dep_info_loc.display() )) })?; + filetime::set_file_times(dep_info_loc, timestamp, timestamp)?; } Ok(()) diff --git a/src/cargo/util/paths.rs b/src/cargo/util/paths.rs index 7cb6e8adc65..7ecd4160386 100644 --- a/src/cargo/util/paths.rs +++ b/src/cargo/util/paths.rs @@ -197,6 +197,18 @@ pub fn mtime(path: &Path) -> CargoResult { Ok(FileTime::from_last_modification_time(&meta)) } +/// get `FileTime::from_system_time(SystemTime::now());` using the exact clock that this file system is using. +pub fn get_current_filesystem_time(path: &Path) -> CargoResult { + // note that if `FileTime::from_system_time(SystemTime::now());` is determined to be sufficient, + // then this can be removed. + let timestamp = path.with_file_name("invoked.timestamp"); + write( + ×tamp, + b"This file has an mtime of when this was started.", + )?; + Ok(mtime(×tamp)?) +} + #[cfg(unix)] pub fn path2bytes(path: &Path) -> CargoResult<&[u8]> { use std::os::unix::prelude::*; diff --git a/tests/testsuite/freshness.rs b/tests/testsuite/freshness.rs index 1d07baa4c27..38a434b605a 100644 --- a/tests/testsuite/freshness.rs +++ b/tests/testsuite/freshness.rs @@ -1,5 +1,7 @@ -use std::fs::{self, File}; +use std::fs::{self, File, OpenOptions}; use std::io::prelude::*; +use std::net::TcpListener; +use std::thread; use crate::support::paths::CargoPathExt; use crate::support::registry::Package; @@ -1304,6 +1306,9 @@ fn bust_patched_dep() { .build(); p.cargo("build").run(); + if is_coarse_mtime() { + sleep_ms(1000); + } File::create(&p.root().join("reg1new/src/lib.rs")).unwrap(); if is_coarse_mtime() { @@ -1332,3 +1337,110 @@ fn bust_patched_dep() { ) .run(); } + +#[test] +fn rebuild_on_mid_build_file_modification() { + let server = TcpListener::bind("127.0.0.1:0").unwrap(); + let addr = server.local_addr().unwrap(); + + let p = project() + .file( + "Cargo.toml", + r#" + [workspace] + members = ["root", "proc_macro_dep"] + "#, + ) + .file( + "root/Cargo.toml", + r#" + [project] + name = "root" + version = "0.1.0" + authors = [] + + [dependencies] + proc_macro_dep = { path = "../proc_macro_dep" } + "#, + ) + .file( + "root/src/lib.rs", + r#" + #[macro_use] + extern crate proc_macro_dep; + + #[derive(Noop)] + pub struct X; + "#, + ) + .file( + "proc_macro_dep/Cargo.toml", + r#" + [project] + name = "proc_macro_dep" + version = "0.1.0" + authors = [] + + [lib] + proc-macro = true + "#, + ) + .file( + "proc_macro_dep/src/lib.rs", + &format!( + r#" + extern crate proc_macro; + + use std::io::Read; + use std::net::TcpStream; + use proc_macro::TokenStream; + + #[proc_macro_derive(Noop)] + pub fn noop(_input: TokenStream) -> TokenStream {{ + let mut stream = TcpStream::connect("{}").unwrap(); + let mut v = Vec::new(); + stream.read_to_end(&mut v).unwrap(); + "".parse().unwrap() + }} + "#, + addr + ), + ) + .build(); + let root = p.root(); + + let t = thread::spawn(move || { + let socket = server.accept().unwrap().0; + sleep_ms(1000); + let mut file = OpenOptions::new() + .write(true) + .append(true) + .open(root.join("root/src/lib.rs")) + .unwrap(); + writeln!(file, "// modified").expect("Failed to append to root sources"); + drop(file); + drop(socket); + drop(server.accept().unwrap()); + }); + + p.cargo("build") + .with_stderr( + "\ +[COMPILING] proc_macro_dep v0.1.0 ([..]/proc_macro_dep) +[COMPILING] root v0.1.0 ([..]/root) +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..] +", + ) + .run(); + + p.cargo("build") + .with_stderr( + "\ +[COMPILING] root v0.1.0 ([..]/root) +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..] +", + ) + .run(); + + t.join().ok().unwrap(); +} diff --git a/tests/testsuite/path.rs b/tests/testsuite/path.rs index 8e46a604b23..2c818c9cdf1 100644 --- a/tests/testsuite/path.rs +++ b/tests/testsuite/path.rs @@ -356,6 +356,7 @@ fn deep_dependencies_trigger_rebuild() { // // We base recompilation off mtime, so sleep for at least a second to ensure // that this write will change the mtime. + sleep_ms(1000); File::create(&p.root().join("baz/src/baz.rs")) .unwrap() .write_all(br#"pub fn baz() { println!("hello!"); }"#) @@ -372,6 +373,7 @@ fn deep_dependencies_trigger_rebuild() { .run(); // Make sure an update to bar doesn't trigger baz + sleep_ms(1000); File::create(&p.root().join("bar/src/bar.rs")) .unwrap() .write_all(