From 58e86d4d94496cb51accd233c5cfc42baaef56b4 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Wed, 9 Nov 2022 16:23:09 +0000 Subject: [PATCH 1/2] test(bindeps): recompile when bin target is also artifact dep This records the WRONG behaviour, which parent fingerpint cannot detect change of a dependency if it is a bin target and also a artifact dep. --- tests/testsuite/artifact_dep.rs | 68 +++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/tests/testsuite/artifact_dep.rs b/tests/testsuite/artifact_dep.rs index 6f41dd21a04..74752435c3b 100644 --- a/tests/testsuite/artifact_dep.rs +++ b/tests/testsuite/artifact_dep.rs @@ -2276,3 +2276,71 @@ fn build_script_features_for_shared_dependency() { .masquerade_as_nightly_cargo(&["bindeps"]) .run(); } + +#[cargo_test] +fn calc_bin_artifact_fingerprint() { + // This records the WRONG behaviour. See rust-lang/cargo#10527 + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.1.0" + resolver = "2" + + [dependencies] + bar = { path = "bar/", artifact = "bin" } + "#, + ) + .file( + "src/main.rs", + r#" + fn main() { + let _b = include_bytes!(env!("CARGO_BIN_FILE_BAR")); + } + "#, + ) + .file("bar/Cargo.toml", &basic_bin_manifest("bar")) + .file("bar/src/main.rs", r#"fn main() { println!("foo") }"#) + .build(); + p.cargo("check -Z bindeps") + .masquerade_as_nightly_cargo(&["bindeps"]) + .with_stderr( + "\ +[COMPILING] bar v0.5.0 ([CWD]/bar) +[CHECKING] foo v0.1.0 ([CWD]) +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..] +", + ) + .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! + 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]) +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..] +", + ) + .run(); + + // Only run the second time can parent fingerpint perceive the change. + // This is WRONG! + 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 [..]` +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..] +", + ) + .run(); +} From 347523e163e8c823fb2e5f425ef43ca5a39ffdc8 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Wed, 9 Nov 2022 16:09:19 +0000 Subject: [PATCH 2/2] fix(fingerprint): include bin target if it is also an artifact dep --- src/cargo/core/compiler/fingerprint.rs | 32 +++++++++++++++----------- tests/testsuite/artifact_dep.rs | 14 +++++------ 2 files changed, 24 insertions(+), 22 deletions(-) diff --git a/src/cargo/core/compiler/fingerprint.rs b/src/cargo/core/compiler/fingerprint.rs index 22a07a601fc..e147ac03d86 100644 --- a/src/cargo/core/compiler/fingerprint.rs +++ b/src/cargo/core/compiler/fingerprint.rs @@ -1254,20 +1254,24 @@ fn calculate(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult, unit: &Unit) -> CargoResult { - // 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::>>()?; - 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::>>()?; + 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); diff --git a/tests/testsuite/artifact_dep.rs b/tests/testsuite/artifact_dep.rs index 74752435c3b..0665df649fd 100644 --- a/tests/testsuite/artifact_dep.rs +++ b/tests/testsuite/artifact_dep.rs @@ -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", @@ -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 [..] ", )