Skip to content

Commit

Permalink
Auto merge of #6734 - ehuss:fingerprint-build-path-only, r=alexcrichton
Browse files Browse the repository at this point in the history
Fingerprint build script deps only for path packages.

#6720 introduced some protection that if there is a build script, and two commands are used (such as `cargo build` then `cargo test`), the second command would correctly get rebuilt. However, the way it was implemented relies on mtimes working correctly. A common use case is to cache built dependencies in Docker, and Docker zeros the nanoseconds from mtime when the image is saved. This caused all packages that had build scripts to get rebuilt when the Docker image runs.

The solution here is to only use the #6720 protection for local (path) packages. This runs under the assumption that mtimes need to work for those anyways.

The consequence is that the scenario in #6720 will no longer work for detecting changes in registry dependencies with build scripts. Fixing that final edge case is nontrivial. Since it is unlikely to happen often, I figure this workaround should be sufficient for now.

cc rust-lang/rust-playground#469
cc rust-lang/rust#59061
  • Loading branch information
bors committed Mar 11, 2019
2 parents d74d879 + 5e7b50a commit dd76122
Showing 1 changed file with 40 additions and 27 deletions.
67 changes: 40 additions & 27 deletions src/cargo/core/compiler/fingerprint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -480,36 +480,49 @@ fn calculate<'a, 'cfg>(
let local = if use_dep_info(unit) {
let dep_info = dep_info_loc(cx, unit);
let mtime = dep_info_mtime_if_fresh(unit.pkg, &dep_info)?;
LocalFingerprint::mtime(cx.files().target_root(), mtime, &dep_info)
let mut local = vec![LocalFingerprint::mtime(
cx.files().target_root(),
mtime,
&dep_info,
)];
// Include the fingerprint of the build script.
//
// This is not included for dependencies (Precalculated below) because
// Docker zeros the nanosecond part of the mtime when the image is
// saved, which prevents built dependencies from being cached.
// This has the consequence that if a dependency needs to be rebuilt
// (such as an environment variable tracked via rerun-if-env-changed),
// and you run two separate commands (`build` then `test`), the second
// command will erroneously think it is fresh.
// See: https://github.com/rust-lang/cargo/issues/6733
local.extend(
cx.dep_targets(unit)
.iter()
.filter(|u| u.mode.is_run_custom_build())
.map(|dep| {
// If the build script is overridden, use the override info as
// the override. Otherwise, use the last invocation time of
// the build script. If the build script re-runs during this
// run, dirty propagation within the JobQueue will ensure that
// this gets invalidated. This is only here to catch the
// situation when cargo is run a second time for another
// target that wasn't built previously (such as `cargo build`
// then `cargo test`).
build_script_override_fingerprint(cx, unit).unwrap_or_else(|| {
let ts_path = cx
.files()
.build_script_run_dir(dep)
.join("invoked.timestamp");
let ts_path_mtime = paths::mtime(&ts_path).ok();
LocalFingerprint::mtime(cx.files().target_root(), ts_path_mtime, &ts_path)
})
}),
);
local
} else {
let fingerprint = pkg_fingerprint(&cx.bcx, unit.pkg)?;
LocalFingerprint::Precalculated(fingerprint)
vec![LocalFingerprint::Precalculated(fingerprint)]
};
let mut local = vec![local];
// Include fingerprint for any build scripts this unit requires.
local.extend(
cx.dep_targets(unit)
.iter()
.filter(|u| u.mode.is_run_custom_build())
.map(|dep| {
// If the build script is overridden, use the override info as
// the override. Otherwise, use the last invocation time of
// the build script. If the build script re-runs during this
// run, dirty propagation within the JobQueue will ensure that
// this gets invalidated. This is only here to catch the
// situation when cargo is run a second time for another
// target that wasn't built previously (such as `cargo build`
// then `cargo test`).
build_script_override_fingerprint(cx, unit).unwrap_or_else(|| {
let ts_path = cx
.files()
.build_script_run_dir(dep)
.join("invoked.timestamp");
let ts_path_mtime = paths::mtime(&ts_path).ok();
LocalFingerprint::mtime(cx.files().target_root(), ts_path_mtime, &ts_path)
})
}),
);

let extra_flags = if unit.mode.is_doc() {
bcx.rustdocflags_args(unit)?
Expand Down

0 comments on commit dd76122

Please sign in to comment.