From 5be58d876c02e7b18873e894121f9693a3388331 Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Fri, 18 Sep 2020 16:51:54 +0900 Subject: [PATCH] Allow only specified optional dependencies to be considered as features --- src/cli.rs | 49 +++++++++++++++++++++++++++++--------------- src/main.rs | 1 + src/package.rs | 21 ++++++++----------- tests/long-help.txt | 8 +++++--- tests/short-help.txt | 6 +++--- tests/test.rs | 48 +++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 99 insertions(+), 34 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index dc9e8bee..a5d09f17 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -15,7 +15,7 @@ const HELP: &[(&str, &str, &str, &[&str])] = &[ ("", "--workspace", "Perform command for all packages in the workspace", &[]), ("", "--exclude ...", "Exclude packages from the check", &[]), ("", "--manifest-path ", "Path to Cargo.toml", &[]), - ("", "--features ", "Space-separated list of features to activate", &[]), + ("", "--features ...", "Space-separated list of features to activate", &[]), ( "", "--each-feature", @@ -28,10 +28,11 @@ const HELP: &[(&str, &str, &str, &[&str])] = &[ "Perform for the feature powerset which includes --no-default-features flag and default features of the package", &[], ), - ("", "--optional-deps", "Use optional dependencies as features", &[ + ("", "--optional-deps [DEPS]...", "Use optional dependencies as features", &[ + "If DEPS are not specified, all optional dependencies are considered as features.", "This flag can only be used with either --each-feature flag or --feature-powerset flag.", ]), - ("", "--skip ", "Space-separated list of features to skip", &[ + ("", "--skip ...", "Space-separated list of features to skip", &[ "To skip run of default feature, using value `--skip default`.", "This flag can only be used with either --each-feature flag or --feature-powerset flag.", ]), @@ -181,8 +182,8 @@ pub(crate) struct Args { pub(crate) ignore_private: bool, /// --ignore-unknown-features, (--ignore-non-exist-features) pub(crate) ignore_unknown_features: bool, - /// --optional-deps - pub(crate) optional_deps: bool, + /// --optional-deps [DEPS]... + pub(crate) optional_deps: Option>, /// --skip-no-default-features pub(crate) skip_no_default_features: bool, /// --clean-per-run @@ -205,7 +206,7 @@ pub(crate) enum Coloring { } impl Coloring { - pub(crate) fn color_choice(color: Option) -> ColorChoice { + pub(crate) fn color_choice(color: Option) -> ColorChoice { match color { Some(Coloring::Auto) | None => ColorChoice::Auto, Some(Coloring::Always) => ColorChoice::Always, @@ -236,7 +237,7 @@ impl FromStr for Coloring { } pub(crate) fn args(coloring: &mut Option) -> Result> { - let mut args = env::args(); + let mut args = env::args().peekable(); let _ = args.next(); // executable name match &args.next() { Some(a) if a == "hack" => {} @@ -256,6 +257,7 @@ pub(crate) fn args(coloring: &mut Option) -> Result> { let mut exclude = Vec::new(); let mut features = Vec::new(); let mut skip = Vec::new(); + let mut optional_deps = None; let mut workspace = None; let mut no_dev_deps = false; @@ -265,7 +267,6 @@ pub(crate) fn args(coloring: &mut Option) -> Result> { let mut ignore_private = false; let mut ignore_unknown_features = false; let mut ignore_non_exist_features = false; - let mut optional_deps = false; let mut skip_no_default_features = false; let mut clean_per_run = false; @@ -322,8 +323,11 @@ pub(crate) fn args(coloring: &mut Option) -> Result> { } macro_rules! parse_multi_opt { - ($v:ident, $allow_split:expr, $pat:expr, $help:expr) => { + ($v:ident, $allow_split:expr, $require_value:expr, $pat:expr, $help:expr) => { if arg == $pat { + if !$require_value && args.peek().map_or(true, |s| s.starts_with('-')) { + continue; + } let arg = args.next().ok_or_else(|| req_arg($help, subcommand.as_ref()))?; if $allow_split { if arg.contains(',') { @@ -370,11 +374,25 @@ pub(crate) fn args(coloring: &mut Option) -> Result> { parse_opt!(manifest_path, false, "--manifest-path", "--manifest-path "); parse_opt!(color, true, "--color", "--color "); - parse_multi_opt!(package, false, "--package", "--package "); - parse_multi_opt!(package, false, "-p", "--package "); - parse_multi_opt!(exclude, false, "--exclude", "--exclude "); - parse_multi_opt!(features, true, "--features", "--features "); - parse_multi_opt!(skip, true, "--skip", "--skip "); + parse_multi_opt!(package, false, true, "--package", "--package ..."); + parse_multi_opt!(package, false, true, "-p", "--package ..."); + parse_multi_opt!(exclude, false, true, "--exclude", "--exclude ..."); + parse_multi_opt!(features, true, true, "--features", "--features ..."); + parse_multi_opt!(skip, true, true, "--skip", "--skip ..."); + + if arg.starts_with("--optional-deps") { + if optional_deps.is_some() { + return Err(multi_arg(&arg, subcommand.as_ref())); + } + let optional_deps = optional_deps.get_or_insert_with(Vec::new); + parse_multi_opt!( + optional_deps, + true, + false, + "--optional-deps", + "--optional-deps [DEPS]..." + ); + } match arg.as_str() { "--workspace" | "--all" => { @@ -387,7 +405,6 @@ pub(crate) fn args(coloring: &mut Option) -> Result> { "--each-feature" => parse_flag!(each_feature), "--feature-powerset" => parse_flag!(feature_powerset), "--ignore-private" => parse_flag!(ignore_private), - "--optional-deps" => parse_flag!(optional_deps), "--skip-no-default-features" => parse_flag!(skip_no_default_features), "--clean-per-run" => parse_flag!(clean_per_run), "--ignore-unknown-features" => { @@ -434,7 +451,7 @@ pub(crate) fn args(coloring: &mut Option) -> Result> { if !each_feature && !feature_powerset { if !skip.is_empty() { bail!("--skip can only be used with either --each-feature or --feature-powerset"); - } else if optional_deps { + } else if optional_deps.is_some() { bail!( "--optional-deps can only be used with either --each-feature or --feature-powerset" ); diff --git a/src/main.rs b/src/main.rs index bc8c1a54..e5e2835a 100644 --- a/src/main.rs +++ b/src/main.rs @@ -127,6 +127,7 @@ fn exec_on_package( } else { let mut line = line.clone(); line.features(args, package); + line.arg("--manifest-path"); line.arg(&package.manifest_path); diff --git a/src/package.rs b/src/package.rs index 05f95d3d..733c3107 100644 --- a/src/package.rs +++ b/src/package.rs @@ -1,6 +1,9 @@ use std::{ffi::OsStr, fmt::Write, ops::Deref}; -use crate::{metadata, Args, Info, Manifest, ProcessBuilder, Result}; +use crate::{ + metadata::{self, Dependency}, + Args, Info, Manifest, ProcessBuilder, Result, +}; pub(crate) struct Package<'a> { package: &'a metadata::Package, @@ -61,17 +64,11 @@ impl<'a> Kind<'a> { let features = package.features.keys().filter(|f| *f != "default" && !args.skip.contains(f)); - let opt_deps = if args.optional_deps { - Some( - package - .dependencies - .iter() - .filter_map(|dep| dep.as_feature()) - .filter(|f| !args.skip.contains(f)), - ) - } else { - None - }; + let opt_deps = args.optional_deps.as_ref().map(|opt_deps| { + package.dependencies.iter().filter_map(Dependency::as_feature).filter(move |f| { + !args.skip.contains(f) && (opt_deps.is_empty() || opt_deps.contains(f)) + }) + }); if args.each_feature { let features: Vec<_> = if let Some(opt_deps) = opt_deps { diff --git a/tests/long-help.txt b/tests/long-help.txt index 3f947275..3f3e0b48 100644 --- a/tests/long-help.txt +++ b/tests/long-help.txt @@ -23,7 +23,7 @@ OPTIONS: --manifest-path Path to Cargo.toml. - --features + --features ... Space-separated list of features to activate. --each-feature @@ -32,12 +32,14 @@ OPTIONS: --feature-powerset Perform for the feature powerset which includes --no-default-features flag and default features of the package. - --optional-deps + --optional-deps [DEPS]... Use optional dependencies as features. + If DEPS are not specified, all optional dependencies are considered as features. + This flag can only be used with either --each-feature flag or --feature-powerset flag. - --skip + --skip ... Space-separated list of features to skip. To skip run of default feature, using value `--skip default`. diff --git a/tests/short-help.txt b/tests/short-help.txt index e0880ed1..f41d81fe 100644 --- a/tests/short-help.txt +++ b/tests/short-help.txt @@ -13,11 +13,11 @@ OPTIONS: --workspace Perform command for all packages in the workspace --exclude ... Exclude packages from the check --manifest-path Path to Cargo.toml - --features Space-separated list of features to activate + --features ... Space-separated list of features to activate --each-feature Perform for each feature which includes --no-default-features flag and default features of the package --feature-powerset Perform for the feature powerset which includes --no-default-features flag and default features of the package - --optional-deps Use optional dependencies as features - --skip Space-separated list of features to skip + --optional-deps [DEPS]... Use optional dependencies as features + --skip ... Space-separated list of features to skip --skip-no-default-features Skip run of just --no-default-features flag --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 7cd33cec..a48ab106 100644 --- a/tests/test.rs +++ b/tests/test.rs @@ -28,9 +28,11 @@ impl Output { fn stdout(&self) -> Cow<'_, str> { String::from_utf8_lossy(&self.stdout) } + fn stderr(&self) -> Cow<'_, str> { String::from_utf8_lossy(&self.stderr) } + fn assert_success(&self) -> &Self { if !self.status.success() { panic!( @@ -51,6 +53,7 @@ impl Output { } self } + fn assert_stderr_contains(&self, pat: &str) -> &Self { if !self.stderr().contains(pat) { panic!( @@ -61,6 +64,7 @@ impl Output { } self } + fn assert_stderr_not_contains(&self, pat: &str) -> &Self { if self.stderr().contains(pat) { panic!( @@ -71,6 +75,7 @@ impl Output { } self } + fn assert_stdout_contains(&self, pat: &str) -> &Self { if !self.stdout().contains(pat) { panic!( @@ -81,6 +86,7 @@ impl Output { } self } + fn assert_stdout_not_contains(&self, pat: &str) -> &Self { if self.stdout().contains(pat) { panic!( @@ -829,6 +835,48 @@ fn optional_deps() { .assert_stderr_contains( "running `cargo check --no-default-features --features renemed` on optional_deps (4/4)", ); + + cargo_hack() + .args(&["check", "--each-feature", "--optional-deps", "real"]) + .current_dir(test_dir("tests/fixtures/optional_deps")) + .output() + .unwrap() + .assert_success() + .assert_stderr_contains("running `cargo check` on optional_deps (1/3)") + .assert_stderr_contains( + "running `cargo check --no-default-features` on optional_deps (2/3)", + ) + .assert_stderr_contains( + "running `cargo check --no-default-features --features real` on optional_deps (3/3)", + ) + .assert_stderr_not_contains( + "running `cargo check --no-default-features --features renemed` on optional_deps", + ); + + cargo_hack() + .args(&["check", "--each-feature", "--optional-deps=renemed"]) + .current_dir(test_dir("tests/fixtures/optional_deps")) + .output() + .unwrap() + .assert_success() + .assert_stderr_contains("running `cargo check` on optional_deps (1/3)") + .assert_stderr_contains( + "running `cargo check --no-default-features` on optional_deps (2/3)", + ) + .assert_stderr_not_contains( + "running `cargo check --no-default-features --features real` on optional_deps", + ) + .assert_stderr_contains( + "running `cargo check --no-default-features --features renemed` on optional_deps (3/3)", + ); + + cargo_hack() + .args(&["check", "--each-feature", "--optional-deps="]) + .current_dir(test_dir("tests/fixtures/optional_deps")) + .output() + .unwrap() + .assert_success() + .assert_stderr_contains("running `cargo check` on optional_deps (1/1)"); } #[test]