From a201d2545f8fb007d6af0834959756cd70da0a5a Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Thu, 9 May 2024 00:50:18 -0400 Subject: [PATCH 1/3] refactor: rename is_proc_macro to reflect it checks every targets --- src/cargo/core/resolver/features.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/cargo/core/resolver/features.rs b/src/cargo/core/resolver/features.rs index fdb49c0bf9f..8be2b48a061 100644 --- a/src/cargo/core/resolver/features.rs +++ b/src/cargo/core/resolver/features.rs @@ -426,10 +426,12 @@ pub struct FeatureResolver<'a, 'gctx> { /// If this is `true`, then a non-default `feature_key` needs to be tracked while /// traversing the graph. /// - /// This is only here to avoid calling `is_proc_macro` when all feature - /// options are disabled (because `is_proc_macro` can trigger downloads). - /// This has to be separate from `FeatureOpts.decouple_host_deps` because + /// This is only here to avoid calling [`has_any_proc_macro`] when all feature + /// options are disabled (because [`has_any_proc_macro`] can trigger downloads). + /// This has to be separate from [`FeatureOpts::decouple_host_deps`] because /// `for_host` tracking is also needed for `itarget` to work properly. + /// + /// [`has_any_proc_macro`]: FeatureResolver::has_any_proc_macro track_for_host: bool, /// `dep_name?/feat_name` features that will be activated if `dep_name` is /// ever activated. @@ -490,7 +492,7 @@ impl<'a, 'gctx> FeatureResolver<'a, 'gctx> { let member_features = self.ws.members_with_features(specs, cli_features)?; for (member, cli_features) in &member_features { let fvs = self.fvs_from_requested(member.package_id(), cli_features); - let fk = if self.track_for_host && self.is_proc_macro(member.package_id()) { + let fk = if self.track_for_host && self.has_any_proc_macro(member.package_id()) { // Also activate for normal dependencies. This is needed if the // proc-macro includes other targets (like binaries or tests), // or running in `cargo test`. Note that in a workspace, if @@ -852,7 +854,7 @@ impl<'a, 'gctx> FeatureResolver<'a, 'gctx> { // for various targets which are either specified in the manifest // or on the cargo command-line. let lib_fk = if fk == FeaturesFor::default() { - (self.track_for_host && (dep.is_build() || self.is_proc_macro(dep_id))) + (self.track_for_host && (dep.is_build() || self.has_any_proc_macro(dep_id))) .then(|| FeaturesFor::HostDep) .unwrap_or_default() } else { @@ -957,7 +959,8 @@ impl<'a, 'gctx> FeatureResolver<'a, 'gctx> { } } - fn is_proc_macro(&self, package_id: PackageId) -> bool { + /// Whether the given package has any proc macro target, including proc-macro examples. + fn has_any_proc_macro(&self, package_id: PackageId) -> bool { self.package_set .get_one(package_id) .expect("packages downloaded") From 2b0df9410403935d2cf7b4702cdfe611634201b6 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Thu, 9 May 2024 01:10:49 -0400 Subject: [PATCH 2/3] test: show proc macro example from dep affecting feature unificaiton --- tests/testsuite/features2.rs | 40 ++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/tests/testsuite/features2.rs b/tests/testsuite/features2.rs index 9d696978f91..878324290d9 100644 --- a/tests/testsuite/features2.rs +++ b/tests/testsuite/features2.rs @@ -2652,3 +2652,43 @@ fn dep_with_optional_host_deps_activated() { ) .run(); } + +#[cargo_test] +fn dont_unify_proc_macro_example_from_dependency() { + // See https://github.com/rust-lang/cargo/issues/13726 + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + edition = "2021" + + [dependencies] + pm_helper = { path = "pm_helper" } + "#, + ) + .file("src/lib.rs", "") + .file( + "pm_helper/Cargo.toml", + r#" + [package] + name = "pm_helper" + + [[example]] + name = "pm" + proc-macro = true + crate-type = ["proc-macro"] + "#, + ) + .file("pm_helper/src/lib.rs", "") + .file("pm_helper/examples/pm.rs", "") + .build(); + + p.cargo("check") + .with_status(101) + .with_stderr_contains( + "[..]activated_features for invalid package: features did not find PackageId [..]pm_helper[..]NormalOrDev[..]" + ) + .run(); +} From 0ead10eebf3a92a9734050da2eb3c3aed07f93e3 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Thu, 9 May 2024 01:16:36 -0400 Subject: [PATCH 3/3] fix: proc-macro example from dep no longer affects feature resolution Previously when checking if a dependency is a proc-macro, the v2 feature resolve resolver v2 looks for `proc-macro = true` for every target of the dependency. However, it's impossible to depend on a non-lib target as a proc-macro. This fix switches to only check if `proc-macro = true` for `[lib]` target for a dependency. --- src/cargo/core/resolver/features.rs | 15 ++++++++++++++- tests/testsuite/features2.rs | 10 +++++++--- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/src/cargo/core/resolver/features.rs b/src/cargo/core/resolver/features.rs index 8be2b48a061..807bdc4533a 100644 --- a/src/cargo/core/resolver/features.rs +++ b/src/cargo/core/resolver/features.rs @@ -854,7 +854,7 @@ impl<'a, 'gctx> FeatureResolver<'a, 'gctx> { // for various targets which are either specified in the manifest // or on the cargo command-line. let lib_fk = if fk == FeaturesFor::default() { - (self.track_for_host && (dep.is_build() || self.has_any_proc_macro(dep_id))) + (self.track_for_host && (dep.is_build() || self.has_proc_macro_lib(dep_id))) .then(|| FeaturesFor::HostDep) .unwrap_or_default() } else { @@ -966,4 +966,17 @@ impl<'a, 'gctx> FeatureResolver<'a, 'gctx> { .expect("packages downloaded") .proc_macro() } + + /// Whether the given package is a proc macro lib target. + /// + /// This is useful for checking if a dependency is a proc macro, + /// as it is not possible to depend on a non-lib target as a proc-macro. + fn has_proc_macro_lib(&self, package_id: PackageId) -> bool { + self.package_set + .get_one(package_id) + .expect("packages downloaded") + .library() + .map(|lib| lib.proc_macro()) + .unwrap_or_default() + } } diff --git a/tests/testsuite/features2.rs b/tests/testsuite/features2.rs index 878324290d9..18951457287 100644 --- a/tests/testsuite/features2.rs +++ b/tests/testsuite/features2.rs @@ -2686,9 +2686,13 @@ fn dont_unify_proc_macro_example_from_dependency() { .build(); p.cargo("check") - .with_status(101) - .with_stderr_contains( - "[..]activated_features for invalid package: features did not find PackageId [..]pm_helper[..]NormalOrDev[..]" + .with_stderr( + "\ +[LOCKING] 2 packages to latest compatible versions +[CHECKING] pm_helper v0.0.0 ([CWD]/pm_helper) +[CHECKING] foo v0.0.0 ([CWD]) +[FINISHED] `dev` [..] +", ) .run(); }