Auto merge of #9437 - In-line:unknown-feature-resolver-1, r=ehuss

Fix bug when with resolver = "1" non-virtual package was allowing unknown features
rust-lang · May 24, 2021 · 07340c9 · 07340c9
2 parents 6ca6dd0 + 61b762b
commit 07340c9
Show file tree

Hide file tree

Showing 2 changed files with 97 additions and 40 deletions.
diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs
@@ -1224,7 +1224,10 @@ impl<'cfg> Workspace<'cfg> {
                     //   really mean anything (either the member is built or it isn't).
                     // * With `--features nonmember?/feat`, cwd_features will
                     //   handle processing it correctly.
-                    let is_member = self.members().any(|member| member.name() == *dep_name);
+                    let is_member = self.members().any(|member| {
+                        // Check if `dep_name` is member of the workspace, but isn't associated with current package.
+                        self.current_opt() != Some(member) && member.name() == *dep_name
+                    });
                     if is_member && specs.iter().any(|spec| spec.name() == *dep_name) {
                         member_specific_features
                             .entry(*dep_name)
@@ -1237,49 +1240,57 @@ impl<'cfg> Workspace<'cfg> {
             }
         }
 
-        let ms = self.members().filter_map(|member| {
-            let member_id = member.package_id();
-            match self.current_opt() {
-                // The features passed on the command-line only apply to
-                // the "current" package (determined by the cwd).
-                Some(current) if member_id == current.package_id() => {
-                    let feats = CliFeatures {
-                        features: Rc::new(cwd_features.clone()),
-                        all_features: cli_features.all_features,
-                        uses_default_features: cli_features.uses_default_features,
-                    };
-                    Some((member, feats))
-                }
-                _ => {
-                    // Ignore members that are not enabled on the command-line.
-                    if specs.iter().any(|spec| spec.matches(member_id)) {
-                        // -p for a workspace member that is not the "current"
-                        // one.
-                        //
-                        // The odd behavior here is due to backwards
-                        // compatibility. `--features` and
-                        // `--no-default-features` used to only apply to the
-                        // "current" package. As an extension, this allows
-                        // member-name/feature-name to set member-specific
-                        // features, which should be backwards-compatible.
+        let ms: Vec<_> = self
+            .members()
+            .filter_map(|member| {
+                let member_id = member.package_id();
+                match self.current_opt() {
+                    // The features passed on the command-line only apply to
+                    // the "current" package (determined by the cwd).
+                    Some(current) if member_id == current.package_id() => {
                         let feats = CliFeatures {
-                            features: Rc::new(
-                                member_specific_features
-                                    .remove(member.name().as_str())
-                                    .unwrap_or_default(),
-                            ),
-                            uses_default_features: true,
+                            features: Rc::new(cwd_features.clone()),
                             all_features: cli_features.all_features,
+                            uses_default_features: cli_features.uses_default_features,
                         };
                         Some((member, feats))
-                    } else {
-                        // This member was not requested on the command-line, skip.
-                        None
+                    }
+                    _ => {
+                        // Ignore members that are not enabled on the command-line.
+                        if specs.iter().any(|spec| spec.matches(member_id)) {
+                            // -p for a workspace member that is not the "current"
+                            // one.
+                            //
+                            // The odd behavior here is due to backwards
+                            // compatibility. `--features` and
+                            // `--no-default-features` used to only apply to the
+                            // "current" package. As an extension, this allows
+                            // member-name/feature-name to set member-specific
+                            // features, which should be backwards-compatible.
+                            let feats = CliFeatures {
+                                features: Rc::new(
+                                    member_specific_features
+                                        .remove(member.name().as_str())
+                                        .unwrap_or_default(),
+                                ),
+                                uses_default_features: true,
+                                all_features: cli_features.all_features,
+                            };
+                            Some((member, feats))
+                        } else {
+                            // This member was not requested on the command-line, skip.
+                            None
+                        }
                     }
                 }
-            }
-        });
-        ms.collect()
+            })
+            .collect();
+
+        // If any member specific features were not removed while iterating over members
+        // some features will be ignored.
+        assert!(member_specific_features.is_empty());
+
+        ms
     }
 }
 

diff --git a/tests/testsuite/package_features.rs b/tests/testsuite/package_features.rs
@@ -65,13 +65,11 @@ fn virtual_no_default_features() {
         .run();
 
     p.cargo("check --features foo")
-        .masquerade_as_nightly_cargo()
         .with_status(101)
         .with_stderr("[ERROR] none of the selected packages contains these features: foo")
         .run();
 
     p.cargo("check --features a/dep1,b/f1,b/f2,f2")
-        .masquerade_as_nightly_cargo()
         .with_status(101)
         .with_stderr("[ERROR] none of the selected packages contains these features: b/f2, f2")
         .run();
@@ -272,6 +270,48 @@ fn other_member_from_current() {
         .run();
 }
 
+#[cargo_test]
+fn feature_default_resolver() {
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+            [package]
+            name = "a"
+            version = "0.1.0"
+
+            [features]
+            test = []
+            "#,
+        )
+        .file(
+            "src/main.rs",
+            r#"
+                fn main() {
+                    if cfg!(feature = "test") {
+                        println!("feature set");
+                    }
+                }
+            "#,
+        )
+        .build();
+
+    p.cargo("check --features testt")
+        .with_status(101)
+        .with_stderr("[ERROR] Package `a[..]` does not have the feature `testt`")
+        .run();
+
+    p.cargo("run --features test")
+        .with_status(0)
+        .with_stdout("feature set")
+        .run();
+
+    p.cargo("run --features a/test")
+        .with_status(101)
+        .with_stderr("[ERROR] package `a[..]` does not have a dependency named `a`")
+        .run();
+}
+
 #[cargo_test]
 fn virtual_member_slash() {
     // member slash feature syntax
@@ -458,6 +498,12 @@ fn resolver1_member_features() {
         .cwd("member2")
         .with_stdout("m1-feature set")
         .run();
+
+    p.cargo("check -p member1 --features member1/m2-feature")
+        .cwd("member2")
+        .with_status(101)
+        .with_stderr("[ERROR] Package `member1[..]` does not have the feature `m2-feature`")
+        .run();
 }
 
 #[cargo_test]