Skip to content

fix(fingerpring): do not touch intermediate artifacts #7050

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Jun 21, 2019
Merged

fix(fingerpring): do not touch intermediate artifacts #7050

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9aa7a4d
test(freshness): check that updating dependency mtime does not rebuild
schomatis Jun 21, 2019
78927e6
fix(fingerpring): do not touch intermediate artifacts
schomatis Jun 17, 2019
8343fb7
test(freshness): remove `simple_deps_cleaner_does_not_rebuild`
schomatis Jun 21, 2019
009876a
fix(fingerprint): rustfmt
schomatis Jun 21, 2019
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
14 changes: 2 additions & 12 deletions src/cargo/core/compiler/fingerprint.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -756,12 +756,7 @@ impl Fingerprint {
/// dependencies up to this unit as well. This function assumes that the
/// unit starts out as `FsStatus::Stale` and then it will optionally switch
/// it to `UpToDate` if it can.
fn check_filesystem(
&mut self,
pkg_root: &Path,
target_root: &Path,
mtime_on_use: bool,
) -> CargoResult<()> {
fn check_filesystem(&mut self, pkg_root: &Path, target_root: &Path) -> CargoResult<()> {
assert!(!self.fs_status.up_to_date());

let mut mtimes = HashMap::new();
Expand All @@ -781,10 +776,6 @@ impl Fingerprint {
return Ok(());
}
};
if mtime_on_use {
let t = FileTime::from_system_time(SystemTime::now());
filetime::set_file_times(output, t, t)?;
}
assert!(mtimes.insert(output.clone(), mtime).is_none());
}

Expand Down Expand Up @@ -1024,8 +1015,7 @@ fn calculate<'a, 'cfg>(
// After we built the initial `Fingerprint` be sure to update the
// `fs_status` field of it.
let target_root = target_root(cx, unit);
let mtime_on_use = cx.bcx.config.cli_unstable().mtime_on_use;
fingerprint.check_filesystem(unit.pkg.root(), &target_root, mtime_on_use)?;
fingerprint.check_filesystem(unit.pkg.root(), &target_root)?;

let fingerprint = Arc::new(fingerprint);
cx.fingerprints.insert(*unit, Arc::clone(&fingerprint));
Expand Down
50 changes: 3 additions & 47 deletions tests/testsuite/freshness.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1170,31 +1170,8 @@ fn changing_rustflags_is_cached() {
.run();
}

fn simple_deps_cleaner(mut dir: PathBuf, timestamp: filetime::FileTime) {
// Cargo is experimenting with letting outside projects develop some
// limited forms of GC for target_dir. This is one of the forms.
// Specifically, Cargo is updating the mtime of files in
// target/profile/deps each time it uses the file.
// So a cleaner can remove files older then a time stamp without
// effecting any builds that happened since that time stamp.
let mut cleand = false;
dir.push("deps");
for dep in fs::read_dir(&dir).unwrap() {
let dep = dep.unwrap();
if filetime::FileTime::from_last_modification_time(&dep.metadata().unwrap()) <= timestamp {
fs::remove_file(dep.path()).unwrap();
println!("remove: {:?}", dep.path());
cleand = true;
}
}
assert!(
cleand,
"called simple_deps_cleaner, but there was nothing to remove"
);
}

#[cargo_test]
fn simple_deps_cleaner_does_not_rebuild() {
fn update_dependency_mtime_does_not_rebuild() {
let p = project()
.file(
"Cargo.toml",
Expand All @@ -1212,9 +1189,6 @@ fn simple_deps_cleaner_does_not_rebuild() {
.file("bar/src/lib.rs", "")
.build();

p.cargo("build -Z mtime-on-use")
.masquerade_as_nightly_cargo()
.run();
p.cargo("build -Z mtime-on-use")
.masquerade_as_nightly_cargo()
.env("RUSTFLAGS", "-C target-cpu=native")
Expand All @@ -1225,36 +1199,18 @@ fn simple_deps_cleaner_does_not_rebuild() {
[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]",
)
.run();
if is_coarse_mtime() {
sleep_ms(1000);
}
let timestamp = filetime::FileTime::from_system_time(SystemTime::now());
if is_coarse_mtime() {
sleep_ms(1000);
}
// This does not make new files, but it does update the mtime.
p.cargo("build -Z mtime-on-use")
// This does not make new files, but it does update the mtime of the dependency.
p.cargo("build -p bar -Z mtime-on-use")
.masquerade_as_nightly_cargo()
.env("RUSTFLAGS", "-C target-cpu=native")
.with_stderr("[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]")
.run();
simple_deps_cleaner(p.target_debug_dir(), timestamp);
// This should not recompile!
p.cargo("build -Z mtime-on-use")
.masquerade_as_nightly_cargo()
.env("RUSTFLAGS", "-C target-cpu=native")
.with_stderr("[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]")
.run();
// But this should be cleaned and so need a rebuild
p.cargo("build -Z mtime-on-use")
.masquerade_as_nightly_cargo()
.with_stderr(
"\
[COMPILING] bar v0.0.1 ([..])
[COMPILING] foo v0.0.1 ([..])
[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]",
)
.run();
}

fn fingerprint_cleaner(mut dir: PathBuf, timestamp: filetime::FileTime) {
Expand Down