Skip to content

Commit

Permalink
Clear out memoized hashes before building crates
Browse files Browse the repository at this point in the history
Build script updates during execution can change the memoized hash of a
`Fingerprint`, and while previously we cleared out a single build
script's memoized hash we forgot to clear out everything that depended
on it as well. This commit pessimistically clears out all `Fingerprint`
memoized hashes just before building to ensure that during the build
everything has the most up-to-date view of the world, and when build
scripts change fingerprints everything that depends on them won't have
run yet.

Closes #7362
  • Loading branch information
alexcrichton committed Sep 17, 2019
1 parent 13bc9a1 commit c3868bb
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 1 deletion.
10 changes: 10 additions & 0 deletions src/cargo/core/compiler/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,16 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
super::compile(&mut self, &mut queue, &mut plan, unit, exec, force_rebuild)?;
}

// Now that we've got the full job queue and we've done all our
// fingerprint analysis to determine what to run, bust all the memoized
// fingerprint hashes to ensure that during the build they all get the
// most up-to-date values. In theory we only need to bust hashes that
// transitively depend on a dirty build script, but it shouldn't matter
// that much for performance anyway.
for fingerprint in self.fingerprints.values() {
fingerprint.clear_memoized();
}

// Now that we've figured out everything that we're going to do, do it!
queue.execute(&mut self, &mut plan)?;

Expand Down
11 changes: 10 additions & 1 deletion src/cargo/core/compiler/fingerprint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,6 @@ pub fn prepare_target<'a, 'cfg>(
// hobble along if it happens to return `Some`.
if let Some(new_local) = (gen_local)(&deps, None)? {
*fingerprint.local.lock().unwrap() = new_local;
*fingerprint.memoized_hash.lock().unwrap() = None;
}

write_fingerprint(&loc, &fingerprint)
Expand Down Expand Up @@ -604,6 +603,16 @@ impl Fingerprint {
}
}

/// For performance reasons fingerprints will memoize their own hash, but
/// there's also internal mutability with its `local` field which can
/// change, for example with build scripts, during a build.
///
/// This method can be used to bust all memoized hashes just before a build
/// to ensure that after a build completes everything is up-to-date.
pub fn clear_memoized(&self) {
*self.memoized_hash.lock().unwrap() = None;
}

fn hash(&self) -> u64 {
if let Some(s) = *self.memoized_hash.lock().unwrap() {
return s;
Expand Down
71 changes: 71 additions & 0 deletions tests/testsuite/freshness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2049,3 +2049,74 @@ fn move_target_directory_with_path_deps() {
.with_stderr("[FINISHED] [..]")
.run();
}

#[cargo_test]
fn rerun_if_changes() {
let p = project()
.file(
"build.rs",
r#"
fn main() {
println!("cargo:rerun-if-env-changed=FOO");
if std::env::var("FOO").is_ok() {
println!("cargo:rerun-if-env-changed=BAR");
}
}
"#,
)
.file("src/lib.rs", "")
.build();

p.cargo("build").run();
p.cargo("build").with_stderr("[FINISHED] [..]").run();

p.cargo("build -v")
.env("FOO", "1")
.with_stderr(
"\
[COMPILING] foo [..]
[RUNNING] `[..]build-script-build`
[RUNNING] `rustc [..]
[FINISHED] [..]
",
)
.run();
p.cargo("build")
.env("FOO", "1")
.with_stderr("[FINISHED] [..]")
.run();

p.cargo("build -v")
.env("FOO", "1")
.env("BAR", "1")
.with_stderr(
"\
[COMPILING] foo [..]
[RUNNING] `[..]build-script-build`
[RUNNING] `rustc [..]
[FINISHED] [..]
",
)
.run();
p.cargo("build")
.env("FOO", "1")
.env("BAR", "1")
.with_stderr("[FINISHED] [..]")
.run();

p.cargo("build -v")
.env("BAR", "2")
.with_stderr(
"\
[COMPILING] foo [..]
[RUNNING] `[..]build-script-build`
[RUNNING] `rustc [..]
[FINISHED] [..]
",
)
.run();
p.cargo("build")
.env("BAR", "2")
.with_stderr("[FINISHED] [..]")
.run();
}

0 comments on commit c3868bb

Please sign in to comment.