Skip to content

Commit

Permalink
Merge #64
Browse files Browse the repository at this point in the history 
64: Exclude features specified by --features flag from feature combinations r=taiki-e a=taiki-e

Fixes #60

Co-authored-by: Taiki Endo <te316e89@gmail.com>
  • Loading branch information
@bors @taiki-e
bors[bot] and taiki-e committed Oct 16, 2020
2 parents 1ae2e29 + dee6e9a commit 301df44
Show file tree
Hide file tree
Showing 6 changed files with 117 additions and 60 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ This project adheres to [Semantic Versioning](https://semver.org).

* Add `--skip-all-features` flag. See [#42][42] for details.

* Fix an issue where using `--features` with `--each-feature` or `--feature-powerset` together would result in the same feature combination being performed multiple times.

[42]: https://github.com/taiki-e/cargo-hack/pull/42

## [0.3.14] - 2020-10-10
Expand Down
80 changes: 51 additions & 29 deletions src/cli.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,6 @@ pub(crate) struct Args {
pub(crate) each_feature: bool,
/// --feature-powerset
pub(crate) feature_powerset: bool,
/// --skip <FEATURES>...
pub(crate) skip: Vec<String>,
/// --no-dev-deps
pub(crate) no_dev_deps: bool,
/// --remove-dev-deps
Expand All @@ -194,17 +192,25 @@ pub(crate) struct Args {
pub(crate) ignore_unknown_features: bool,
/// --optional-deps [DEPS]...
pub(crate) optional_deps: Option<Vec<String>>,
/// --skip-no-default-features
pub(crate) skip_no_default_features: bool,
/// --skip-all-features
pub(crate) skip_all_features: bool,
/// --clean-per-run
pub(crate) clean_per_run: bool,
/// -v, --verbose, -vv
pub(crate) verbose: bool,
/// --depth <NUM>
pub(crate) depth: Option<usize>,

/// --no-default-features
pub(crate) no_default_features: bool,
/// -v, --verbose, -vv
pub(crate) verbose: bool,

// Note: These values are not always exactly the same as the input.
// Error messages should not assume that these options have been specified.
/// --skip <FEATURES>...
pub(crate) skip: Vec<String>,
/// --skip-no-default-features
pub(crate) skip_no_default_features: bool,
/// --skip-all-features
pub(crate) skip_all_features: bool,

// flags that will be propagated to cargo
/// --features <FEATURES>...
pub(crate) features: Vec<String>,
Expand Down Expand Up @@ -283,9 +289,12 @@ pub(crate) fn args(coloring: &mut Option<Coloring>) -> Result<Option<Args>> {
let mut skip_no_default_features = false;
let mut skip_all_features = false;
let mut clean_per_run = false;
let mut verbose = false;
let mut depth = None;

let mut verbose = false;
let mut no_default_features = false;
let mut all_features = false;

let res = (|| -> Result<()> {
while let Some(arg) = args.next() {
// stop at `--`
Expand All @@ -306,20 +315,14 @@ pub(crate) fn args(coloring: &mut Option<Coloring>) -> Result<Option<Args>> {
}

macro_rules! parse_opt {
($opt:ident, $propagate:expr, $pat:expr, $help:expr) => {
($opt:ident, $pat:expr, $help:expr) => {
if arg == $pat {
if $opt.is_some() {
return Err(multi_arg($help, subcommand.as_ref()));
}
let next =
args.next().ok_or_else(|| req_arg($help, subcommand.as_ref()))?;
if $propagate {
$opt = Some(next.clone());
leading.push(arg);
leading.push(next);
} else {
$opt = Some(next);
}
$opt = Some(next);
continue;
} else if arg.starts_with(concat!($pat, "=")) {
if $opt.is_some() {
Expand All @@ -330,9 +333,6 @@ pub(crate) fn args(coloring: &mut Option<Coloring>) -> Result<Option<Args>> {
.nth(1)
.ok_or_else(|| req_arg($help, subcommand.as_ref()))?;
$opt = Some(next.to_string());
if $propagate {
leading.push(arg);
}
continue;
}
};
Expand Down Expand Up @@ -383,13 +383,15 @@ pub(crate) fn args(coloring: &mut Option<Coloring>) -> Result<Option<Args>> {
($flag:ident) => {
if mem::replace(&mut $flag, true) {
return Err(multi_arg(&arg, subcommand.as_ref()));
} else {
continue;
}
};
}

parse_opt!(manifest_path, false, "--manifest-path", "--manifest-path <PATH>");
parse_opt!(depth, false, "--depth", "--depth <NUM>");
parse_opt!(color, true, "--color", "--color <WHEN>");
parse_opt!(manifest_path, "--manifest-path", "--manifest-path <PATH>");
parse_opt!(depth, "--depth", "--depth <NUM>");
parse_opt!(color, "--color", "--color <WHEN>");

parse_multi_opt!(package, false, true, "--package", "--package <SPEC>...");
parse_multi_opt!(package, false, true, "-p", "--package <SPEC>...");
Expand All @@ -416,6 +418,7 @@ pub(crate) fn args(coloring: &mut Option<Coloring>) -> Result<Option<Args>> {
if let Some(arg) = workspace.replace(arg) {
return Err(multi_arg(&arg, subcommand.as_ref()));
}
continue;
}
"--no-dev-deps" => parse_flag!(no_dev_deps),
"--remove-dev-deps" => parse_flag!(remove_dev_deps),
Expand All @@ -430,9 +433,16 @@ pub(crate) fn args(coloring: &mut Option<Coloring>) -> Result<Option<Args>> {
"--ignore-non-exist-features was removed, use --ignore-unknown-features instead"
),
// allow multiple uses
"--verbose" | "-v" | "-vv" => verbose = true,
_ => leading.push(arg),
"--verbose" | "-v" | "-vv" => {
verbose = true;
continue;
}
"--no-default-features" => no_default_features = true,
"--all-features" => all_features = true,
_ => {}
}

leading.push(arg);
}

Ok(())
Expand Down Expand Up @@ -517,6 +527,12 @@ pub(crate) fn args(coloring: &mut Option<Coloring>) -> Result<Option<Args>> {
if each_feature && feature_powerset {
bail!("--each-feature may not be used together with --feature-powerset");
}
if all_features && each_feature {
bail!("--all-features may not be used together with --each-feature");
}
if all_features && feature_powerset {
bail!("--all-features may not be used together with --feature-powerset");
}

if subcommand.is_none() {
if leading.iter().any(|a| a == "--list") {
Expand Down Expand Up @@ -546,6 +562,9 @@ For more information try --help
)
}

skip_no_default_features |= no_default_features;
skip.extend_from_slice(&features);

Ok(Some(Args {
leading_args: leading,
trailing_args: args.collect::<Vec<_>>(),
Expand All @@ -558,20 +577,23 @@ For more information try --help
workspace: workspace.is_some(),
each_feature,
feature_powerset,
skip,
no_dev_deps,
remove_dev_deps,
ignore_private,
ignore_unknown_features,
optional_deps,
skip_no_default_features,
skip_all_features,
clean_per_run,
depth,

no_default_features,
verbose,

skip,
skip_no_default_features,
skip_all_features,

features,
color,
verbose,
}))
}

Expand Down
2 changes: 1 addition & 1 deletion src/main.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ fn exec_on_package(
Ok(())
} else {
let mut line = line.clone();
line.features(args, package);
line.append_features_from_args(args, package);

line.arg("--manifest-path");
line.arg(&package.manifest_path);
Expand Down
4 changes: 3 additions & 1 deletion src/package.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,9 @@ pub(crate) fn exec(
exec_cargo(args, package, &mut line, info, true)?;
}

line.arg("--no-default-features");
if !args.no_default_features {
line.arg("--no-default-features");
}

if !args.skip_no_default_features {
// run with no default features if the package has other features
Expand Down
4 changes: 1 addition & 3 deletions src/process.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,7 @@ impl<'a> ProcessBuilder<'a> {
}
}

/// (chainable) Adds `--features` flag to the args list.
pub(crate) fn features(&mut self, args: &Args, package: &Package) -> &mut Self {
pub(crate) fn append_features_from_args(&mut self, args: &Args, package: &Package) {
if args.ignore_unknown_features {
self.append_features(args.features.iter().filter(|f| {
if package.features.get(*f).is_some()
Expand All @@ -86,7 +85,6 @@ impl<'a> ProcessBuilder<'a> {
} else if !args.features.is_empty() {
self.append_features(&args.features);
}
self
}

/// (chainable) Adds `arg` to the args list.
Expand Down
85 changes: 59 additions & 26 deletions tests/test.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -527,27 +527,25 @@ fn each_feature() {

// with other feature
cargo_hack()
.args(&["check", "--each-feature", "--features=a"])
.args(&["check", "--each-feature", "--features", "a"])
.current_dir(test_dir("tests/fixtures/real"))
.output()
.unwrap()
.assert_success()
.assert_stderr_contains("running `cargo check --features a` on real (1/6)")
.assert_stderr_contains("running `cargo check --features a` on real (1/5)")
.assert_stderr_contains(
"running `cargo check --no-default-features --features a` on real (2/6)",
"running `cargo check --no-default-features --features a` on real (2/5)",
)
.assert_stderr_contains(
"running `cargo check --no-default-features --features a,a` on real (3/6)",
"running `cargo check --no-default-features --features a,b` on real (3/5)",
)
.assert_stderr_contains(
"running `cargo check --no-default-features --features a,b` on real (4/6)",
"running `cargo check --no-default-features --features a,c` on real (4/5)",
)
.assert_stderr_contains(
"running `cargo check --no-default-features --features a,c` on real (5/6)",
"running `cargo check --no-default-features --all-features --features a` on real (5/5)",
)
.assert_stderr_contains(
"running `cargo check --no-default-features --all-features --features a` on real (6/6)",
);
.assert_stderr_not_contains("--features a,a");
}

#[test]
Expand Down Expand Up @@ -587,37 +585,28 @@ fn feature_powerset() {

// with other feature
cargo_hack()
.args(&["check", "--feature-powerset", "--features=a"])
.args(&["check", "--feature-powerset", "--features", "a"])
.current_dir(test_dir("tests/fixtures/real"))
.output()
.unwrap()
.assert_success()
.assert_stderr_contains("running `cargo check --features a` on real (1/10)")
.assert_stderr_contains("running `cargo check --no-default-features --features a` on real (2/10)")
.assert_stderr_contains(
"running `cargo check --no-default-features --features a,a` on real (3/10)",
)
.assert_stderr_contains(
"running `cargo check --no-default-features --features a,b` on real (4/10)",
)
.assert_stderr_contains("running `cargo check --features a` on real (1/6)")
.assert_stderr_contains(
"running `cargo check --no-default-features --features a,c` on real (6/10)",
"running `cargo check --no-default-features --features a` on real (2/6)",
)
.assert_stderr_contains(
"running `cargo check --no-default-features --features a,a,b` on real (5/10)",
"running `cargo check --no-default-features --features a,b` on real (3/6)",
)
.assert_stderr_contains(
"running `cargo check --no-default-features --features a,a,c` on real (7/10)",
"running `cargo check --no-default-features --features a,c` on real (4/6)",
)
.assert_stderr_contains(
"running `cargo check --no-default-features --features a,b,c` on real (8/10)",
"running `cargo check --no-default-features --features a,b,c` on real (5/6)",
)
.assert_stderr_contains(
"running `cargo check --no-default-features --features a,a,b,c` on real (9/10)",
"running `cargo check --no-default-features --all-features --features a` on real (6/6)",
)
.assert_stderr_contains(
"running `cargo check --no-default-features --all-features --features a` on real (10/10)",
);
.assert_stderr_not_contains("--features a,a");
}

#[test]
Expand Down Expand Up @@ -1165,3 +1154,47 @@ fn verbose() {
SEPARATOR
));
}

#[test]
fn propagate() {
cargo_hack()
.args(&["check", "--features", "a"])
.current_dir(test_dir("tests/fixtures/real"))
.output()
.unwrap()
.assert_success()
.assert_stderr_contains("--features a");

cargo_hack()
.args(&["check", "--no-default-features"])
.current_dir(test_dir("tests/fixtures/real"))
.output()
.unwrap()
.assert_success()
.assert_stderr_contains("--no-default-features");

cargo_hack()
.args(&["check", "--all-features"])
.current_dir(test_dir("tests/fixtures/real"))
.output()
.unwrap()
.assert_success()
.assert_stderr_contains("--all-features");

cargo_hack()
.args(&["check", "--color", "auto"])
.current_dir(test_dir("tests/fixtures/real"))
.output()
.unwrap()
.assert_success()
.assert_stderr_contains("`cargo check --color auto`");

// --verbose does not be propagated
cargo_hack()
.args(&["check", "--verbose"])
.current_dir(test_dir("tests/fixtures/real"))
.output()
.unwrap()
.assert_success()
.assert_stderr_not_contains("--verbose");
}

0 comments on commit 301df44

Please sign in to comment.