From 6c4069b4b5d312f3013c8f3328d0327964942d31 Mon Sep 17 00:00:00 2001 From: Urgau Date: Fri, 1 Dec 2023 21:55:16 +0100 Subject: [PATCH 1/3] Add test showing the buggy behavior of check-cfg and the fingerprint --- tests/testsuite/check_cfg.rs | 50 ++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/tests/testsuite/check_cfg.rs b/tests/testsuite/check_cfg.rs index 57d5f805363..951c9101bb9 100644 --- a/tests/testsuite/check_cfg.rs +++ b/tests/testsuite/check_cfg.rs @@ -141,6 +141,56 @@ fn features_with_namespaced_features() { .run(); } +#[cargo_test(nightly, reason = "--check-cfg is unstable")] +fn features_fingerprint() { + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.1.0" + + [features] + f_a = [] + f_b = [] + "#, + ) + .file("src/lib.rs", "#[cfg(feature = \"f_b\")] fn entry() {}") + .build(); + + p.cargo("check -v -Zcheck-cfg") + .masquerade_as_nightly_cargo(&["check-cfg"]) + .with_stderr_contains(x!("rustc" => "cfg" of "feature" with "f_a" "f_b")) + .with_stderr_does_not_contain("[..]unexpected_cfgs[..]") + .run(); + + p.cargo("check -v -Zcheck-cfg") + .masquerade_as_nightly_cargo(&["check-cfg"]) + .with_stderr_does_not_contain("[..]rustc[..]") + .run(); + + p.change_file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.1.0" + + [features] + f_a = [] + "#, + ); + + p.cargo("check -v -Zcheck-cfg") + .masquerade_as_nightly_cargo(&["check-cfg"]) + // this is a bug, rustc should be called again + .with_stderr_does_not_contain("[..]rustc[..]") + // and should show a warning from the unexpected_cfgs lint + .with_stderr_does_not_contain("[..]unexpected_cfgs[..]") + .run(); +} + #[cargo_test(nightly, reason = "--check-cfg is unstable")] fn well_known_names_values() { let p = project() From 7d1fc45a293fa7de7602f246fb05283f37f4bea8 Mon Sep 17 00:00:00 2001 From: Urgau Date: Sun, 19 Nov 2023 18:56:39 +0100 Subject: [PATCH 2/3] Include declared list of features in fingerprint for -Zcheck-cfg Otherwise changing (add, removing, ...) features from the `[features]` table would not cause rustc to be called with the new `--check-cfg`, showing potentially outdated warnings. --- .../core/compiler/fingerprint/dirty_reason.rs | 7 +++++++ src/cargo/core/compiler/fingerprint/mod.rs | 21 +++++++++++++++++++ tests/testsuite/check_cfg.rs | 10 +++++---- 3 files changed, 34 insertions(+), 4 deletions(-) diff --git a/src/cargo/core/compiler/fingerprint/dirty_reason.rs b/src/cargo/core/compiler/fingerprint/dirty_reason.rs index 363aab7e624..38b3fc4411b 100644 --- a/src/cargo/core/compiler/fingerprint/dirty_reason.rs +++ b/src/cargo/core/compiler/fingerprint/dirty_reason.rs @@ -15,6 +15,10 @@ pub enum DirtyReason { old: String, new: String, }, + DeclaredFeaturesChanged { + old: String, + new: String, + }, TargetConfigurationChanged, PathToSourceChanged, ProfileConfigurationChanged, @@ -141,6 +145,9 @@ impl DirtyReason { DirtyReason::FeaturesChanged { .. } => { s.dirty_because(unit, "the list of features changed") } + DirtyReason::DeclaredFeaturesChanged { .. } => { + s.dirty_because(unit, "the list of declared features changed") + } DirtyReason::TargetConfigurationChanged => { s.dirty_because(unit, "the target configuration changed") } diff --git a/src/cargo/core/compiler/fingerprint/mod.rs b/src/cargo/core/compiler/fingerprint/mod.rs index b1040be41b8..ac8cdba2668 100644 --- a/src/cargo/core/compiler/fingerprint/mod.rs +++ b/src/cargo/core/compiler/fingerprint/mod.rs @@ -65,6 +65,7 @@ //! Target Name | ✓ | ✓ //! TargetKind (bin/lib/etc.) | ✓ | ✓ //! Enabled Features | ✓ | ✓ +//! Declared Features | ✓ | //! Immediate dependency’s hashes | ✓[^1] | ✓ //! [`CompileKind`] (host/target) | ✓ | ✓ //! __CARGO_DEFAULT_LIB_METADATA[^4] | | ✓ @@ -572,6 +573,8 @@ pub struct Fingerprint { rustc: u64, /// Sorted list of cfg features enabled. features: String, + /// Sorted list of all the declared cfg features. + declared_features: String, /// Hash of the `Target` struct, including the target name, /// package-relative source path, edition, etc. target: u64, @@ -876,6 +879,7 @@ impl Fingerprint { profile: 0, path: 0, features: String::new(), + declared_features: String::new(), deps: Vec::new(), local: Mutex::new(Vec::new()), memoized_hash: Mutex::new(None), @@ -922,6 +926,12 @@ impl Fingerprint { new: self.features.clone(), }; } + if self.declared_features != old.declared_features { + return DirtyReason::DeclaredFeaturesChanged { + old: old.declared_features.clone(), + new: self.declared_features.clone(), + }; + } if self.target != old.target { return DirtyReason::TargetConfigurationChanged; } @@ -1200,6 +1210,7 @@ impl hash::Hash for Fingerprint { let Fingerprint { rustc, ref features, + ref declared_features, target, path, profile, @@ -1215,6 +1226,7 @@ impl hash::Hash for Fingerprint { ( rustc, features, + declared_features, target, path, profile, @@ -1431,6 +1443,7 @@ fn calculate_normal(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult>(); Ok(Fingerprint { rustc: util::hash_u64(&cx.bcx.rustc().verbose_version), target: util::hash_u64(&unit.target), @@ -1439,6 +1452,14 @@ fn calculate_normal(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult "cfg" of "feature" with "f_a")) + // and that we indeed found a new warning from the unexpected_cfgs lint + .with_stderr_contains("[..]unexpected_cfgs[..]") .run(); } From f32f43d879ed5b0b02a88916c98bd24069c79145 Mon Sep 17 00:00:00 2001 From: Urgau Date: Fri, 1 Dec 2023 22:05:17 +0100 Subject: [PATCH 3/3] Avoid fingerprint invalidation when reordering the declared features --- src/cargo/core/compiler/fingerprint/mod.rs | 4 +++- tests/testsuite/check_cfg.rs | 19 +++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/cargo/core/compiler/fingerprint/mod.rs b/src/cargo/core/compiler/fingerprint/mod.rs index ac8cdba2668..e1737a8b657 100644 --- a/src/cargo/core/compiler/fingerprint/mod.rs +++ b/src/cargo/core/compiler/fingerprint/mod.rs @@ -1443,7 +1443,9 @@ fn calculate_normal(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult>(); + let mut declared_features = unit.pkg.summary().features().keys().collect::>(); + declared_features.sort(); // to avoid useless rebuild if the user orders it's features + // differently Ok(Fingerprint { rustc: util::hash_u64(&cx.bcx.rustc().verbose_version), target: util::hash_u64(&unit.target), diff --git a/tests/testsuite/check_cfg.rs b/tests/testsuite/check_cfg.rs index ba1858e985f..059becac0be 100644 --- a/tests/testsuite/check_cfg.rs +++ b/tests/testsuite/check_cfg.rs @@ -165,6 +165,25 @@ fn features_fingerprint() { .with_stderr_does_not_contain("[..]unexpected_cfgs[..]") .run(); + p.cargo("check -v -Zcheck-cfg") + .masquerade_as_nightly_cargo(&["check-cfg"]) + .with_stderr_does_not_contain("[..]rustc[..]") + .run(); + + // checking that re-ordering the features does not invalid the fingerprint + p.change_file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.1.0" + + [features] + f_b = [] + f_a = [] + "#, + ); + p.cargo("check -v -Zcheck-cfg") .masquerade_as_nightly_cargo(&["check-cfg"]) .with_stderr_does_not_contain("[..]rustc[..]")