Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rebuild on mid build file modification #6484

Merged
merged 7 commits into from
Jan 7, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/cargo/core/compiler/custom_build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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 =
Expand Down
16 changes: 9 additions & 7 deletions src/cargo/core/compiler/fingerprint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 2 additions & 0 deletions src/cargo/core/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -334,6 +335,7 @@ fn rustc<'a, 'cfg>(
rustc_dep_info_loc.display()
))
})?;
filetime::set_file_times(dep_info_loc, timestamp, timestamp)?;
}

Ok(())
Expand Down
12 changes: 12 additions & 0 deletions src/cargo/util/paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,18 @@ pub fn mtime(path: &Path) -> CargoResult<FileTime> {
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<FileTime> {
// 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(
&timestamp,
b"This file has an mtime of when this was started.",
)?;
Ok(mtime(&timestamp)?)
}

#[cfg(unix)]
pub fn path2bytes(path: &Path) -> CargoResult<&[u8]> {
use std::os::unix::prelude::*;
Expand Down
114 changes: 113 additions & 1 deletion tests/testsuite/freshness.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -1281,6 +1283,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() {
Expand Down Expand Up @@ -1309,3 +1314,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();
}
2 changes: 2 additions & 0 deletions tests/testsuite/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!"); }"#)
Expand All @@ -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(
Expand Down