Repository: taiki-e/cargo-hack (language: rust)

Issue:
cargo hack ... --features X requires feature X on all workspace crates
Running cargo-hack on a workspace requesting a specific feature to be enabled will require the feature to be enabled on every crate, and will cause an error when a crate doesn't have that feature.

It's unclear if this is the intended behavior, so this might be a feature request, or it might not be and then this might be a bug.

I expected when I use the `--features X` option that it would apply to the crates that have the feature and not to the ones that don't have it.

For example, if a repo has multiple crates, some with a testutils feature, and the developer wishes to run all tests with the testutils feature enabled, you might think the following command would work:
```
cargo hack test --features testutils
```

But it will result in:
```
info: running `cargo test --features testutils` on crate_a (2/14)
error: none of the selected packages contains these features: testutils
```


Discussion / hints:
You need to use --ignore-unknown-features flag

https://github.com/taiki-e/cargo-hack#--ignore-unknown-features

> Skip passing `--features` to `cargo` if that feature does not exist.


Ahh. Thank you. Sorry for the noise! 
This error happens when using `--group-features` regardless of whether `--ignore-unknown-features` is passed, so it seems like this issue isn't fully resolved. I don't see an existing issue for this, so reopening this one.

For a command that looks like:
```
cargo hack --feature-powerset --ignore-unknown-features --group-features testutils,serde --group-features hostfn_log_fmt_values,hostfn_create_x_from_ed25519 check
```

Displays a warning like:
```
warning: --ignore-unknown-features for --group-features is not fully implemented and may not work as intended
```

Displays an error like:
```
info: running `cargo check --no-default-features --features testutils,serde` on soroban-test-wasms (10/40)
error: none of the selected packages contains these features: serde, testutils
```

Fix patch:
diff --git a/README.md b/README.md
index 60cfe34..abb0211 100644
--- a/README.md
+++ b/README.md
@@ -175,7 +175,7 @@ OPTIONS:
         --ignore-unknown-features
             Skip passing --features flag to `cargo` if that feature does not exist in the package.
 
-            This flag can only be used together with either --features or --include-features.
+            This flag can be used with --features, --include-features, or --group-features.
 
         --rust-version
             Perform commands on `package.rust-version`.
diff --git a/src/cli.rs b/src/cli.rs
index 59d8bf1..bb33a90 100644
--- a/src/cli.rs
+++ b/src/cli.rs
@@ -395,13 +395,6 @@ impl Args {
                     "--ignore-unknown-features for --include-features is not fully implemented and may not work as intended"
                 );
             }
-            if !group_features.is_empty() {
-                let _guard = term::warn::scoped(false);
-                // TODO: implement
-                warn!(
-                    "--ignore-unknown-features for --group-features is not fully implemented and may not work as intended"
-                );
-            }
         }
         if !each_feature && !feature_powerset {
             if optional_deps.is_some() {
@@ -799,7 +792,7 @@ const HELP: &[HelpText<'_>] = &[
         "--ignore-unknown-features",
         "",
         "Skip passing --features flag to `cargo` if that feature does not exist in the package",
-        &["This flag can only be used together with either --features or --include-features."],
+        &["This flag can be used with --features, --include-features, or --group-features."],
     ),
     (
         "",
diff --git a/src/main.rs b/src/main.rs
index 9668c81..ca7c4f0 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -20,7 +20,7 @@ mod rustup;
 mod version;
 
 use std::{
-    collections::BTreeMap,
+    collections::{BTreeMap, HashSet},
     env,
     fmt::{self, Write},
     str::FromStr,
@@ -155,6 +155,7 @@ enum Kind<'a> {
     Powerset { features: Vec<Vec<&'a Feature>> },
 }
 
+#[allow(clippy::redundant_closure_for_method_calls)]
 fn determine_kind<'a>(
     cx: &'a Context,
     id: &'a PackageId,
@@ -214,7 +215,29 @@ fn determine_kind<'a>(
         }
 
         if !cx.group_features.is_empty() {
-            features.extend(cx.group_features.iter());
+            if cx.ignore_unknown_features {
+                let all_valid_features: HashSet<_> = pkg_features
+                    .normal()
+                    .iter()
+                    .chain(pkg_features.optional_deps())
+                    .flat_map(|f| f.as_group())
+                    .map(|f| f.as_str())
+                    .collect();
+                features.extend(cx.group_features.iter().filter(|&f| {
+                    let all_valid =
+                        f.as_group().iter().all(|f| all_valid_features.contains(f.as_str()));
+                    if !all_valid {
+                        info!(
+                            "skipped applying group `{}` to {}",
+                            f.as_group().join(","),
+                            package.name
+                        );
+                    }
+                    all_valid
+                }));
+            } else {
+                features.extend(cx.group_features.iter());
+            }
         }
 
         features


Test patch:
diff --git a/tests/long-help.txt b/tests/long-help.txt
index 6b8da66..5435036 100644
--- a/tests/long-help.txt
+++ b/tests/long-help.txt
@@ -145,7 +145,7 @@ OPTIONS:
         --ignore-unknown-features
             Skip passing --features flag to `cargo` if that feature does not exist in the package.
 
-            This flag can only be used together with either --features or --include-features.
+            This flag can be used with --features, --include-features, or --group-features.
 
         --rust-version
             Perform commands on `package.rust-version`.
diff --git a/tests/test.rs b/tests/test.rs
index c9f49b9..8f5b736 100644
--- a/tests/test.rs
+++ b/tests/test.rs
@@ -421,6 +421,21 @@ fn ignore_unknown_features() {
             ",
         )
         .stderr_not_contains("skipped applying unknown `f` feature to member2");
+
+    cargo_hack([
+        "check",
+        "--ignore-unknown-features",
+        "--feature-powerset",
+        "--group-features=a,missing",
+    ])
+    .assert_success("virtual")
+    .stderr_contains(
+        "
+            skipped applying group `a,missing` to member1
+            skipped applying group `a,missing` to member2
+            ",
+    )
+    .stderr_not_contains("skipped applying unknown `missing` feature to member2");
 }
 
 #[test]
@@ -448,21 +463,6 @@ fn ignore_unknown_features_failure() {
         work as intended
         ",
     );
-
-    cargo_hack([
-        "check",
-        "--ignore-unknown-features",
-        "--feature-powerset",
-        "--group-features",
-        "a,b",
-    ])
-    .assert_success("real")
-    .stderr_contains(
-        "
-        --ignore-unknown-features for --group-features is not fully implemented and may not \
-        work as intended
-        ",
-    );
 }
 
 #[test]
@@ -650,6 +650,33 @@ fn powerset_deduplication() {
             ",
         );
 
+    // with --group-features and --ignore-unknown-features
+    cargo_hack([
+        "check",
+        "--feature-powerset",
+        "--ignore-unknown-features",
+        "--group-features",
+        "b,d,not_found",
+    ])
+    .assert_success2("powerset_deduplication", require)
+    .stderr_contains(
+        "
+            info: skipped applying group `b,d,not_found` to deduplication
+            info: running `cargo check --no-default-features` on deduplication (1/5)
+            info: running `cargo check --no-default-features --features a` on deduplication (2/5)
+            info: running `cargo check --no-default-features --features c` on deduplication (3/5)
+            info: running `cargo check --no-default-features --features e` on deduplication (4/5)
+            info: running `cargo check --no-default-features --features c,e` on deduplication (5/5)
+            ",
+    )
+    .stderr_not_contains(
+        "
+            a,b,d
+            e,b,d
+            features b,d
+            ",
+    );
+
     // with --group-features + --optional-deps
     cargo_hack(["check", "--feature-powerset", "--group-features", "b,d", "--optional-deps"])
         .assert_success2("powerset_deduplication", require)