Skip to content

Commit

Permalink
fix(fingerprint): include bin target if it is also an artifact dep
Browse files Browse the repository at this point in the history
  • Loading branch information
weihanglo committed Nov 9, 2022
1 parent 58e86d4 commit 347523e
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 22 deletions.
32 changes: 18 additions & 14 deletions src/cargo/core/compiler/fingerprint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1254,20 +1254,24 @@ fn calculate(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Arc<Fingerpri
/// Calculate a fingerprint for a "normal" unit, or anything that's not a build
/// script. This is an internal helper of `calculate`, don't call directly.
fn calculate_normal(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Fingerprint> {
// Recursively calculate the fingerprint for all of our dependencies.
//
// Skip fingerprints of binaries because they don't actually induce a
// recompile, they're just dependencies in the sense that they need to be
// built.
//
// Create Vec since mutable cx is needed in closure.
let deps = Vec::from(cx.unit_deps(unit));
let mut deps = deps
.into_iter()
.filter(|dep| !dep.unit.target.is_bin())
.map(|dep| DepFingerprint::new(cx, unit, &dep))
.collect::<CargoResult<Vec<_>>>()?;
deps.sort_by(|a, b| a.pkg_id.cmp(&b.pkg_id));
let deps = {
// Recursively calculate the fingerprint for all of our dependencies.
//
// Skip fingerprints of binaries because they don't actually induce a
// recompile, they're just dependencies in the sense that they need to be
// built. The only exception here are artifact dependencies,
// which is an actual dependency that needs a recompile.
//
// Create Vec since mutable cx is needed in closure.
let deps = Vec::from(cx.unit_deps(unit));
let mut deps = deps
.into_iter()
.filter(|dep| !dep.unit.target.is_bin() || dep.unit.artifact.is_true())
.map(|dep| DepFingerprint::new(cx, unit, &dep))
.collect::<CargoResult<Vec<_>>>()?;
deps.sort_by(|a, b| a.pkg_id.cmp(&b.pkg_id));
deps
};

// Afterwards calculate our own fingerprint information.
let target_root = target_root(cx);
Expand Down
14 changes: 6 additions & 8 deletions tests/testsuite/artifact_dep.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2279,7 +2279,7 @@ fn build_script_features_for_shared_dependency() {

#[cargo_test]
fn calc_bin_artifact_fingerprint() {
// This records the WRONG behaviour. See rust-lang/cargo#10527
// See rust-lang/cargo#10527
let p = project()
.file(
"Cargo.toml",
Expand Down Expand Up @@ -2316,29 +2316,27 @@ fn calc_bin_artifact_fingerprint() {
.run();

p.change_file("bar/src/main.rs", r#"fn main() { println!("bar") }"#);
// Change in bin artifact but not propagated to parent fingerprint.
// This is WRONG!
// Change in artifact bin dep `bar` propagates to `foo`, triggering recompile.
p.cargo("check -v -Z bindeps")
.masquerade_as_nightly_cargo(&["bindeps"])
.with_stderr(
"\
[COMPILING] bar v0.5.0 ([CWD]/bar)
[RUNNING] `rustc --crate-name bar [..]`
[FRESH] foo v0.1.0 ([CWD])
[CHECKING] foo v0.1.0 ([CWD])
[RUNNING] `rustc --crate-name foo [..]`
[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
",
)
.run();

// Only run the second time can parent fingerpint perceive the change.
// This is WRONG!
// All units are fresh. No recompile.
p.cargo("check -v -Z bindeps")
.masquerade_as_nightly_cargo(&["bindeps"])
.with_stderr(
"\
[FRESH] bar v0.5.0 ([CWD]/bar)
[CHECKING] foo v0.1.0 ([CWD])
[RUNNING] `rustc --crate-name foo [..]`
[FRESH] foo v0.1.0 ([CWD])
[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
",
)
Expand Down

0 comments on commit 347523e

Please sign in to comment.