From 544a5ce48fc616716bbee076f85d5c15fae00232 Mon Sep 17 00:00:00 2001 From: Kaur Kuut Date: Tue, 10 Dec 2024 00:02:00 +0200 Subject: [PATCH 1/3] Add `--must-have-and-exclude-feature` option. --- CHANGELOG.md | 2 + README.md | 9 ++ src/cli.rs | 32 +++- src/features.rs | 4 + src/main.rs | 11 ++ src/process.rs | 35 +++-- .../must_have_and_exclude_feature/Cargo.toml | 7 + .../member1/Cargo.toml | 13 ++ .../member1/src/main.rs | 1 + .../member2/Cargo.toml | 13 ++ .../member2/src/main.rs | 1 + .../member3/Cargo.toml | 12 ++ .../member3/src/main.rs | 1 + tests/long-help.txt | 9 ++ tests/short-help.txt | 1 + tests/test.rs | 137 ++++++++++++++++++ 16 files changed, 274 insertions(+), 14 deletions(-) create mode 100644 tests/fixtures/must_have_and_exclude_feature/Cargo.toml create mode 100644 tests/fixtures/must_have_and_exclude_feature/member1/Cargo.toml create mode 100644 tests/fixtures/must_have_and_exclude_feature/member1/src/main.rs create mode 100644 tests/fixtures/must_have_and_exclude_feature/member2/Cargo.toml create mode 100644 tests/fixtures/must_have_and_exclude_feature/member2/src/main.rs create mode 100644 tests/fixtures/must_have_and_exclude_feature/member3/Cargo.toml create mode 100644 tests/fixtures/must_have_and_exclude_feature/member3/src/main.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index d666cffc..37683b41 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,8 @@ Note: In this file, do not use the hard wrap in the middle of a sentence for com ## [Unreleased] +- Add `--must-have-and-exclude-feature` option. ([#262](https://github.com/taiki-e/cargo-hack/pull/262), thanks @xStrom) + ## [0.6.35] - 2025-02-11 - Performance improvements. diff --git a/README.md b/README.md index c2b234bb..7086622a 100644 --- a/README.md +++ b/README.md @@ -162,6 +162,15 @@ OPTIONS: This flag can only be used together with either --each-feature flag or --feature-powerset flag. + --must-have-and-exclude-feature + Require the specified feature to be present but excluded. + + Exclude the specified feature and all other features which depend on it. + + Exclude packages which don't have the specified feature. + + This is useful for doing no_std testing with --must-have-and-exclude-feature std. + --no-dev-deps Perform without dev-dependencies. diff --git a/src/cli.rs b/src/cli.rs index 758c85cf..a431a2f5 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -37,6 +37,8 @@ pub(crate) struct Args { pub(crate) each_feature: bool, /// --feature-powerset pub(crate) feature_powerset: bool, + /// --must-have-and-exclude-feature + pub(crate) must_have_and_exclude_feature: Option, /// --no-dev-deps pub(crate) no_dev_deps: bool, /// --remove-dev-deps @@ -152,6 +154,7 @@ impl Args { let mut remove_dev_deps = false; let mut each_feature = false; let mut feature_powerset = false; + let mut must_have_and_exclude_feature = None; let mut no_private = false; let mut ignore_private = false; let mut ignore_unknown_features = false; @@ -303,6 +306,9 @@ impl Args { Long("remove-dev-deps") => parse_flag!(remove_dev_deps), Long("each-feature") => parse_flag!(each_feature), Long("feature-powerset") => parse_flag!(feature_powerset), + Long("must-have-and-exclude-feature") => { + parse_opt!(must_have_and_exclude_feature, false); + } Long("at-least-one-of") => at_least_one_of.push(parser.value()?.parse()?), Long("no-private") => parse_flag!(no_private), Long("ignore-private") => parse_flag!(ignore_private), @@ -492,6 +498,8 @@ impl Args { conflicts("--all-features", "--each-feature")?; } else if feature_powerset { conflicts("--all-features", "--feature-powerset")?; + } else if must_have_and_exclude_feature.is_some() { + conflicts("--all-features", "--must-have-and-exclude-feature")?; } } if no_default_features { @@ -520,6 +528,21 @@ impl Args { } } + if let Some(f) = must_have_and_exclude_feature.as_ref() { + if features.contains(f) { + bail!("feature `{f}` specified by both --must-have-and-exclude-feature and --features"); + } + if optional_deps.as_ref().is_some_and(|d| d.contains(f)) { + bail!("feature `{f}` specified by both --must-have-and-exclude-feature and --optional-deps"); + } + if group_features.iter().any(|v| v.matches(f)) { + bail!("feature `{f}` specified by both --must-have-and-exclude-feature and --group-features"); + } + if include_features.contains(f) { + bail!("feature `{f}` specified by both --must-have-and-exclude-feature and --include-features"); + } + } + if subcommand.is_none() { if cargo_args.iter().any(|a| a == "--list") { cmd!(cargo, "--list").run()?; @@ -591,7 +614,8 @@ impl Args { exclude_no_default_features |= !include_features.is_empty(); exclude_all_features |= !include_features.is_empty() || !exclude_features.is_empty() - || !mutually_exclusive_features.is_empty(); + || !mutually_exclusive_features.is_empty() + || must_have_and_exclude_feature.is_some(); exclude_features.extend_from_slice(&features); term::verbose::set(verbose != 0); @@ -613,6 +637,7 @@ impl Args { workspace, each_feature, feature_powerset, + must_have_and_exclude_feature, no_dev_deps, remove_dev_deps, no_private, @@ -758,6 +783,11 @@ const HELP: &[HelpText<'_>] = &[ --feature-powerset flag.", ], ), + ("", "--must-have-and-exclude-feature", "", "Require the specified feature to be present but excluded", &[ + "Exclude the specified feature and all other features which depend on it.", + "Exclude packages which don't have the specified feature.", + "This is useful for doing no_std testing with --must-have-and-exclude-feature std.", + ]), ("", "--no-dev-deps", "", "Perform without dev-dependencies", &[ "Note that this flag removes dev-dependencies from real `Cargo.toml` while cargo-hack is \ running and restores it when finished.", diff --git a/src/features.rs b/src/features.rs index 51adee48..684a3f0f 100644 --- a/src/features.rs +++ b/src/features.rs @@ -84,6 +84,10 @@ impl Features { pub(crate) fn contains(&self, name: &str) -> bool { self.features.iter().any(|f| f == name) } + + pub(crate) fn get(&self, name: &str) -> Option<&Feature> { + self.features.iter().find(|f| *f == name) + } } /// The representation of Cargo feature. diff --git a/src/main.rs b/src/main.rs index 7f0fbb31..cd639feb 100644 --- a/src/main.rs +++ b/src/main.rs @@ -200,9 +200,13 @@ fn determine_kind<'a>( let package = cx.packages(id); let pkg_features = cx.pkg_features(id); + let recursively_exclude_feature = + cx.must_have_and_exclude_feature.as_ref().and_then(|s| pkg_features.get(s)); let filter = |&f: &&Feature| { !cx.exclude_features.iter().any(|s| f == s) && !cx.group_features.iter().any(|g| g.matches(f.name())) + && !recursively_exclude_feature + .is_some_and(|rf| rf.matches_recursive(f.name(), &package.features)) }; let features = if cx.include_features.is_empty() { // TODO @@ -340,10 +344,14 @@ fn determine_package_list(cx: &Context) -> Result>> { ); } } + let has_required_features = |id: &&PackageId| { + cx.must_have_and_exclude_feature.as_ref().is_none_or(|s| cx.pkg_features(id).contains(s)) + }; Ok(if cx.workspace { let ids: Vec<_> = cx .workspace_members() .filter(|id| !cx.exclude.contains(&cx.packages(id).name)) + .filter(has_required_features) .collect(); let multiple_packages = ids.len() > 1; ids.iter().filter_map(|id| determine_kind(cx, id, multiple_packages)).collect() @@ -360,6 +368,7 @@ fn determine_package_list(cx: &Context) -> Result>> { .workspace_members() .filter(|id| cx.package.contains(&cx.packages(id).name)) .filter(|id| !cx.exclude.contains(&cx.packages(id).name)) + .filter(has_required_features) .collect(); let multiple_packages = ids.len() > 1; ids.iter().filter_map(|id| determine_kind(cx, id, multiple_packages)).collect() @@ -367,6 +376,7 @@ fn determine_package_list(cx: &Context) -> Result>> { let ids: Vec<_> = cx .workspace_members() .filter(|id| !cx.exclude.contains(&cx.packages(id).name)) + .filter(has_required_features) .collect(); let multiple_packages = ids.len() > 1; ids.iter().filter_map(|id| determine_kind(cx, id, multiple_packages)).collect() @@ -376,6 +386,7 @@ fn determine_package_list(cx: &Context) -> Result>> { cx.workspace_members() .find(|id| cx.packages(id).name == *current_package) .filter(|id| !cx.exclude.contains(&cx.packages(id).name)) + .filter(has_required_features) .and_then(|id| determine_kind(cx, id, multiple_packages).map(|p| vec![p])) .unwrap_or_default() }) diff --git a/src/process.rs b/src/process.rs index 3eaaf0ca..953614b9 100644 --- a/src/process.rs +++ b/src/process.rs @@ -98,19 +98,28 @@ impl<'a> ProcessBuilder<'a> { } pub(crate) fn append_features_from_args(&mut self, cx: &Context, id: &PackageId) { - if cx.ignore_unknown_features { - self.append_features(cx.features.iter().filter(|&f| { - if cx.pkg_features(id).contains(f) { - true - } else { - // ignored - info!("skipped applying unknown `{f}` feature to {}", cx.packages(id).name); - false - } - })); - } else if !cx.features.is_empty() { - self.append_features(&cx.features); - } + let package = cx.packages(id); + let pkg_features = cx.pkg_features(id); + let recursively_exclude_feature = + cx.must_have_and_exclude_feature.as_ref().and_then(|s| pkg_features.get(s)); + + self.append_features(cx.features.iter().filter(|&f| { + if recursively_exclude_feature + .is_some_and(|rf| rf.matches_recursive(f, &package.features)) + { + info!( + "skipped applying `{f}` feature to {} because it would enable excluded feature `{}`", + package.name, + recursively_exclude_feature.unwrap().name() + ); + false + } else if cx.ignore_unknown_features && !pkg_features.contains(f) { + info!("skipped applying unknown `{f}` feature to {}", package.name); + false + } else { + true + } + })); } /// Gets the comma-separated features list diff --git a/tests/fixtures/must_have_and_exclude_feature/Cargo.toml b/tests/fixtures/must_have_and_exclude_feature/Cargo.toml new file mode 100644 index 00000000..926f6381 --- /dev/null +++ b/tests/fixtures/must_have_and_exclude_feature/Cargo.toml @@ -0,0 +1,7 @@ +[workspace] +resolver = "2" +members = [ + "member1", + "member2", + "member3", +] diff --git a/tests/fixtures/must_have_and_exclude_feature/member1/Cargo.toml b/tests/fixtures/must_have_and_exclude_feature/member1/Cargo.toml new file mode 100644 index 00000000..e37751ab --- /dev/null +++ b/tests/fixtures/must_have_and_exclude_feature/member1/Cargo.toml @@ -0,0 +1,13 @@ +[package] +name = "member1" +version = "0.0.0" + +[features] +default = ["c"] +a = [] +b = [] +c = ["b"] + +[dependencies] + +[dev-dependencies] diff --git a/tests/fixtures/must_have_and_exclude_feature/member1/src/main.rs b/tests/fixtures/must_have_and_exclude_feature/member1/src/main.rs new file mode 100644 index 00000000..f328e4d9 --- /dev/null +++ b/tests/fixtures/must_have_and_exclude_feature/member1/src/main.rs @@ -0,0 +1 @@ +fn main() {} diff --git a/tests/fixtures/must_have_and_exclude_feature/member2/Cargo.toml b/tests/fixtures/must_have_and_exclude_feature/member2/Cargo.toml new file mode 100644 index 00000000..978b3944 --- /dev/null +++ b/tests/fixtures/must_have_and_exclude_feature/member2/Cargo.toml @@ -0,0 +1,13 @@ +[package] +name = "member2" +version = "0.0.0" + +[features] +default = ["a"] +a = [] +b = [] +c = ["b"] + +[dependencies] + +[dev-dependencies] diff --git a/tests/fixtures/must_have_and_exclude_feature/member2/src/main.rs b/tests/fixtures/must_have_and_exclude_feature/member2/src/main.rs new file mode 100644 index 00000000..f328e4d9 --- /dev/null +++ b/tests/fixtures/must_have_and_exclude_feature/member2/src/main.rs @@ -0,0 +1 @@ +fn main() {} diff --git a/tests/fixtures/must_have_and_exclude_feature/member3/Cargo.toml b/tests/fixtures/must_have_and_exclude_feature/member3/Cargo.toml new file mode 100644 index 00000000..6d54b48d --- /dev/null +++ b/tests/fixtures/must_have_and_exclude_feature/member3/Cargo.toml @@ -0,0 +1,12 @@ +[package] +name = "member3" +version = "0.0.0" + +[features] +default = ["c"] +a = [] +c = ["a"] + +[dependencies] + +[dev-dependencies] diff --git a/tests/fixtures/must_have_and_exclude_feature/member3/src/main.rs b/tests/fixtures/must_have_and_exclude_feature/member3/src/main.rs new file mode 100644 index 00000000..f328e4d9 --- /dev/null +++ b/tests/fixtures/must_have_and_exclude_feature/member3/src/main.rs @@ -0,0 +1 @@ +fn main() {} diff --git a/tests/long-help.txt b/tests/long-help.txt index 53023f27..2c7030cc 100644 --- a/tests/long-help.txt +++ b/tests/long-help.txt @@ -132,6 +132,15 @@ OPTIONS: This flag can only be used together with either --each-feature flag or --feature-powerset flag. + --must-have-and-exclude-feature + Require the specified feature to be present but excluded. + + Exclude the specified feature and all other features which depend on it. + + Exclude packages which don't have the specified feature. + + This is useful for doing no_std testing with --must-have-and-exclude-feature std. + --no-dev-deps Perform without dev-dependencies. diff --git a/tests/short-help.txt b/tests/short-help.txt index 5ed21b18..14d5bea0 100644 --- a/tests/short-help.txt +++ b/tests/short-help.txt @@ -31,6 +31,7 @@ OPTIONS: features that don't enable any of the features listed --include-features ... Include only the specified features in the feature combinations instead of package features + --must-have-and-exclude-feature Require the specified feature to be present but excluded --no-dev-deps Perform without dev-dependencies --remove-dev-deps Equivalent to --no-dev-deps flag except for does not restore the original `Cargo.toml` after performed diff --git a/tests/test.rs b/tests/test.rs index 15101088..509650a8 100644 --- a/tests/test.rs +++ b/tests/test.rs @@ -1088,6 +1088,143 @@ fn exclude_all_features_failure() { ); } +#[test] +fn must_have_and_exclude_feature() { + cargo_hack(["check", "--feature-powerset", "--must-have-and-exclude-feature", "b"]) + .assert_success("must_have_and_exclude_feature") + .stderr_contains( + " + running `cargo check --no-default-features` on member1 (1/5) + running `cargo check --no-default-features --features a` on member1 (2/5) + running `cargo check --no-default-features` on member2 (3/5) + running `cargo check --no-default-features --features default` on member2 (4/5) + running `cargo check --no-default-features --features a` on member2 (5/5) + ", + ) + .stderr_not_contains( + " + --all-features + --features b` + --features c` + --features default` on member1 + on member3 + ", + ); + + cargo_hack(["check", "--each-feature", "--must-have-and-exclude-feature", "b"]) + .assert_success("must_have_and_exclude_feature") + .stderr_contains( + " + running `cargo check --no-default-features` on member1 (1/5) + running `cargo check --no-default-features --features a` on member1 (2/5) + running `cargo check --no-default-features` on member2 (3/5) + running `cargo check --no-default-features --features a` on member2 (4/5) + running `cargo check --no-default-features --features default` on member2 (5/5) + ", + ) + .stderr_not_contains( + " + --all-features + --features b` + --features c` + --features default` on member1 + on member3 + ", + ); + + cargo_hack([ + "check", + "--each-feature", + "--must-have-and-exclude-feature", + "b", + "--features", + "default", + ]) + .assert_success("must_have_and_exclude_feature") + .stderr_contains( + " + skipped applying `default` feature to member1 because it would enable excluded feature `b` + running `cargo check --no-default-features` on member1 (1/4) + running `cargo check --no-default-features --features a` on member1 (2/4) + running `cargo check --no-default-features --features default` on member2 (3/4) + running `cargo check --no-default-features --features default,a` on member2 (4/4) + ", + ) + .stderr_not_contains( + " + --all-features + --features b` + --features c` + --features default` on member1 + on member3 + " + ); + + cargo_hack([ + "check", + "--each-feature", + "--must-have-and-exclude-feature", + "b", + "--features", + "default", + "--ignore-unknown-features", + ]) + .assert_success("must_have_and_exclude_feature") + .stderr_contains( + " + skipped applying `default` feature to member1 because it would enable excluded feature `b` + running `cargo check --no-default-features` on member1 (1/4) + running `cargo check --no-default-features --features a` on member1 (2/4) + running `cargo check --no-default-features --features default` on member2 (3/4) + running `cargo check --no-default-features --features default,a` on member2 (4/4) + ", + ) + .stderr_not_contains( + " + --all-features + --features b` + --features c` + --features default` on member1 + on member3 + " + ); +} + +#[test] +fn must_have_and_exclude_feature_failure() { + cargo_hack([ + "check", + "--each-feature", + "--must-have-and-exclude-feature", + "b", + "--features", + "b", + ]) + .assert_failure("must_have_and_exclude_feature") + .stderr_contains( + "feature `b` specified by both --must-have-and-exclude-feature and --features", + ); + + cargo_hack([ + "check", + "--each-feature", + "--must-have-and-exclude-feature", + "b", + "--include-features", + "b", + ]) + .assert_failure("must_have_and_exclude_feature") + .stderr_contains( + "feature `b` specified by both --must-have-and-exclude-feature and --include-features", + ); + + cargo_hack(["check", "--must-have-and-exclude-feature", "b", "--all-features"]) + .assert_failure("must_have_and_exclude_feature") + .stderr_contains( + "--all-features may not be used together with --must-have-and-exclude-feature", + ); +} + #[test] fn each_feature_all() { cargo_hack(["check", "--each-feature", "--workspace"]).assert_success("real").stderr_contains( From 6335ad94e54bbfed5385f57ac827a8ed8bec897f Mon Sep 17 00:00:00 2001 From: Kaur Kuut Date: Tue, 10 Dec 2024 18:15:46 +0200 Subject: [PATCH 2/3] Satisfy tidy lint. --- tests/fixtures/must_have_and_exclude_feature/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/fixtures/must_have_and_exclude_feature/Cargo.toml b/tests/fixtures/must_have_and_exclude_feature/Cargo.toml index 926f6381..f567c306 100644 --- a/tests/fixtures/must_have_and_exclude_feature/Cargo.toml +++ b/tests/fixtures/must_have_and_exclude_feature/Cargo.toml @@ -3,5 +3,5 @@ resolver = "2" members = [ "member1", "member2", - "member3", + "member3", ] From 717b9264d5588f89d62c1c9596ab016f225d3fd8 Mon Sep 17 00:00:00 2001 From: Kaur Kuut Date: Tue, 10 Dec 2024 18:21:53 +0200 Subject: [PATCH 3/3] Replace Option::is_none_or with Option::is_some_and for MSRV. --- src/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main.rs b/src/main.rs index cd639feb..a49e2b4e 100644 --- a/src/main.rs +++ b/src/main.rs @@ -345,7 +345,7 @@ fn determine_package_list(cx: &Context) -> Result>> { } } let has_required_features = |id: &&PackageId| { - cx.must_have_and_exclude_feature.as_ref().is_none_or(|s| cx.pkg_features(id).contains(s)) + !cx.must_have_and_exclude_feature.as_ref().is_some_and(|s| !cx.pkg_features(id).contains(s)) }; Ok(if cx.workspace { let ids: Vec<_> = cx