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

Issue:
Keep going on failure?
Thanks for this tool, especially the feature powerset function is *very* handy!

I was wondering if there could be a mode like `make -k` which errors are recorded, but all runs will still be completed, and failing combinations reported at the end.


Discussion / hints:

Fix patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 3ec4baa6..69a10280 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -12,7 +12,9 @@ Note: In this file, do not use the hard wrap in the middle of a sentence for com
 
 - Fix an error when using old cargo with a dependency graph containing 2021 edition crates. ([#138](https://github.com/taiki-e/cargo-hack/pull/138))
 
-- Support omitting lower bound of --version-range. ([#139](https://github.com/taiki-e/cargo-hack/pull/139))
+- Support omitting lower bound of `--version-range`. ([#139](https://github.com/taiki-e/cargo-hack/pull/139))
+
+- Add `--keep-going` flag. ([#140](https://github.com/taiki-e/cargo-hack/pull/140))
 
 ## [0.5.8] - 2021-10-13
 
diff --git a/README.md b/README.md
index 150ae2cd..ef61dc6e 100644
--- a/README.md
+++ b/README.md
@@ -165,6 +165,9 @@ OPTIONS:
 
             This flag can only be used together with --version-range flag.
 
+        --keep-going
+            Keep going on failure.
+
     -v, --verbose
             Use verbose output.
 
diff --git a/src/cli.rs b/src/cli.rs
index db4962d2..707301b7 100644
--- a/src/cli.rs
+++ b/src/cli.rs
@@ -40,6 +40,8 @@ pub(crate) struct Args<'a> {
     pub(crate) clean_per_run: bool,
     /// --clean-per-version
     pub(crate) clean_per_version: bool,
+    /// --keep-going
+    pub(crate) keep_going: bool,
     /// --version-range
     pub(crate) version_range: Option<&'a str>,
     /// --version-step
@@ -130,6 +132,7 @@ pub(crate) fn parse_args<'a>(raw: &'a [String], cargo: &OsStr) -> Result<Args<'a
     let mut ignore_unknown_features = false;
     let mut clean_per_run = false;
     let mut clean_per_version = false;
+    let mut keep_going = false;
     let mut version_range = None;
     let mut version_step = None;
 
@@ -261,6 +264,7 @@ pub(crate) fn parse_args<'a>(raw: &'a [String], cargo: &OsStr) -> Result<Args<'a
                 "--include-deps-features" => parse_flag!(include_deps_features),
                 "--clean-per-run" => parse_flag!(clean_per_run),
                 "--clean-per-version" => parse_flag!(clean_per_version),
+                "--keep-going" => parse_flag!(keep_going),
                 "--ignore-unknown-features" => parse_flag!(ignore_unknown_features),
                 // allow multiple uses
                 "--verbose" | "-v" | "-vv" => {
@@ -496,6 +500,7 @@ pub(crate) fn parse_args<'a>(raw: &'a [String], cargo: &OsStr) -> Result<Args<'a
         optional_deps,
         clean_per_run,
         clean_per_version,
+        keep_going,
         include_features: include_features.into_iter().map(Into::into).collect(),
         include_deps_features,
         version_range,
@@ -648,6 +653,7 @@ const HELP: &[HelpText<'_>] = &[
         "Note that dependencies artifacts will also be removed.",
         "This flag can only be used together with --version-range flag.",
     ]),
+    ("", "--keep-going", "", "Keep going on failure", &[]),
     ("-v", "--verbose", "", "Use verbose output", &[]),
     ("", "--color", "<WHEN>", "Coloring: auto, always, never", &[
         "This flag will be propagated to cargo.",
diff --git a/src/features.rs b/src/features.rs
index 2a617d6e..8f744332 100644
--- a/src/features.rs
+++ b/src/features.rs
@@ -3,10 +3,7 @@ use std::{
     slice,
 };
 
-use crate::{
-    metadata::{Dependency, Metadata},
-    PackageId,
-};
+use crate::{metadata::Metadata, PackageId};
 
 pub(crate) struct Features {
     features: Vec<Feature>,
@@ -24,7 +21,7 @@ impl Features {
         for name in package.features.keys() {
             features.push(name.into());
         }
-        for name in package.dependencies.iter().filter_map(Dependency::as_feature) {
+        for name in package.optional_deps() {
             features.push(name.into());
         }
         let len = [package.features.len(), features.len()];
diff --git a/src/main.rs b/src/main.rs
index 52088855..2a9ddca2 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -25,7 +25,10 @@ mod restore;
 mod rustup;
 mod version;
 
-use std::fmt::Write;
+use std::{
+    collections::BTreeMap,
+    fmt::{self, Write},
+};
 
 use anyhow::{bail, Result};
 
@@ -61,6 +64,7 @@ fn exec_on_workspace(cx: &Context<'_>) -> Result<()> {
 
     let mut progress = Progress::default();
     let packages = determine_package_list(cx, &mut progress)?;
+    let mut keep_going = KeepGoing::default();
     if let Some(range) = &cx.version_range {
         progress.total *= range.len();
         let line = cmd!("cargo");
@@ -97,17 +101,22 @@ fn exec_on_workspace(cx: &Context<'_>) -> Result<()> {
             let mut line = line.clone();
             line.leading_arg(toolchain);
             line.apply_context(cx);
-            packages
-                .iter()
-                .try_for_each(|(id, kind)| exec_on_package(cx, id, kind, &line, &mut progress))
-        })
+            packages.iter().try_for_each(|(id, kind)| {
+                exec_on_package(cx, id, kind, &line, &mut progress, &mut keep_going)
+            })
+        })?;
     } else {
         let mut line = cx.cargo();
         line.apply_context(cx);
-        packages
-            .iter()
-            .try_for_each(|(id, kind)| exec_on_package(cx, id, kind, &line, &mut progress))
+        packages.iter().try_for_each(|(id, kind)| {
+            exec_on_package(cx, id, kind, &line, &mut progress, &mut keep_going)
+        })?;
+    }
+    if keep_going.count > 0 {
+        eprintln!();
+        error!("{}", keep_going);
     }
+    Ok(())
 }
 
 #[derive(Default)]
@@ -189,7 +198,9 @@ fn determine_kind<'a>(cx: &'a Context<'_>, id: &PackageId, progress: &mut Progre
         } else {
             progress.total += features.len()
                 + !cx.exclude_no_default_features as usize
-                + !cx.exclude_all_features as usize;
+                + (!cx.exclude_all_features
+                    && package.features.len() + package.optional_deps().count() > 1)
+                    as usize;
             Kind::Each { features }
         }
     } else if cx.feature_powerset {
@@ -202,7 +213,9 @@ fn determine_kind<'a>(cx: &'a Context<'_>, id: &PackageId, progress: &mut Progre
             // -1: the first element of a powerset is `[]`
             progress.total += features.len() - 1
                 + !cx.exclude_no_default_features as usize
-                + !cx.exclude_all_features as usize;
+                + (!cx.exclude_all_features
+                    && package.features.len() + package.optional_deps().count() > 1)
+                    as usize;
             Kind::Powerset { features }
         }
     } else {
@@ -259,6 +272,7 @@ fn exec_on_package(
     kind: &Kind<'_>,
     line: &ProcessBuilder<'_>,
     progress: &mut Progress,
+    keep_going: &mut KeepGoing,
 ) -> Result<()> {
     if let Kind::SkipAsPrivate = kind {
         return Ok(());
@@ -278,11 +292,11 @@ fn exec_on_package(
 
         fs::write(&package.manifest_path, new)?;
 
-        exec_actual(cx, id, kind, &mut line, progress)?;
+        exec_actual(cx, id, kind, &mut line, progress, keep_going)?;
 
         handle.close()
     } else {
-        exec_actual(cx, id, kind, &mut line, progress)
+        exec_actual(cx, id, kind, &mut line, progress, keep_going)
     }
 }
 
@@ -292,13 +306,14 @@ fn exec_actual(
     kind: &Kind<'_>,
     line: &mut ProcessBuilder<'_>,
     progress: &mut Progress,
+    keep_going: &mut KeepGoing,
 ) -> Result<()> {
     match kind {
         Kind::NoSubcommand => return Ok(()),
         Kind::SkipAsPrivate => unreachable!(),
         Kind::Normal => {
             // only run with default features
-            return exec_cargo(cx, id, line, progress);
+            return exec_cargo(cx, id, line, progress, keep_going);
         }
         Kind::Each { .. } | Kind::Powerset { .. } => {}
     }
@@ -316,29 +331,30 @@ fn exec_actual(
 
     if !cx.exclude_no_default_features {
         // run with no default features if the package has other features
-        exec_cargo(cx, id, &mut line, progress)?;
+        exec_cargo(cx, id, &mut line, progress, keep_going)?;
     }
 
     match kind {
         Kind::Each { features } => {
-            features
-                .iter()
-                .try_for_each(|f| exec_cargo_with_features(cx, id, &line, progress, Some(f)))?;
+            features.iter().try_for_each(|f| {
+                exec_cargo_with_features(cx, id, &line, progress, keep_going, Some(f))
+            })?;
         }
         Kind::Powerset { features } => {
             // The first element of a powerset is `[]` so it should be skipped.
-            features
-                .iter()
-                .skip(1)
-                .try_for_each(|f| exec_cargo_with_features(cx, id, &line, progress, f))?;
+            features.iter().skip(1).try_for_each(|f| {
+                exec_cargo_with_features(cx, id, &line, progress, keep_going, f)
+            })?;
         }
         _ => unreachable!(),
     }
 
-    if !cx.exclude_all_features {
+    let pkg = cx.packages(id);
+    if !cx.exclude_all_features && pkg.features.len() + pkg.optional_deps().count() > 1 {
         // run with all features
+        // https://github.com/taiki-e/cargo-hack/issues/42
         line.arg("--all-features");
-        exec_cargo(cx, id, &mut line, progress)?;
+        exec_cargo(cx, id, &mut line, progress, keep_going)?;
     }
 
     Ok(())
@@ -349,11 +365,33 @@ fn exec_cargo_with_features(
     id: &PackageId,
     line: &ProcessBuilder<'_>,
     progress: &mut Progress,
+    keep_going: &mut KeepGoing,
     features: impl IntoIterator<Item = impl AsRef<str>>,
 ) -> Result<()> {
     let mut line = line.clone();
     line.append_features(features);
-    exec_cargo(cx, id, &mut line, progress)
+    exec_cargo(cx, id, &mut line, progress, keep_going)
+}
+
+#[derive(Default)]
+struct KeepGoing {
+    count: u64,
+    failed_commands: BTreeMap<String, Vec<String>>,
+}
+
+impl fmt::Display for KeepGoing {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        writeln!(f, "failed to run {} commands", self.count)?;
+        writeln!(f)?;
+        writeln!(f, "failed commands:")?;
+        for (pkg, commands) in &self.failed_commands {
+            writeln!(f, "    {}:", pkg)?;
+            for cmd in commands {
+                writeln!(f, "        {}", cmd)?;
+            }
+        }
+        Ok(())
+    }
 }
 
 fn exec_cargo(
@@ -361,7 +399,34 @@ fn exec_cargo(
     id: &PackageId,
     line: &mut ProcessBuilder<'_>,
     progress: &mut Progress,
+    keep_going: &mut KeepGoing,
 ) -> Result<()> {
+    let res = exec_cargo_inner(cx, id, line, progress);
+    if cx.keep_going {
+        if let Err(e) = res {
+            error!("{:#}", e);
+            keep_going.count = keep_going.count.saturating_add(1);
+            let name = cx.packages(id).name.clone();
+            if !keep_going.failed_commands.contains_key(&name) {
+                keep_going.failed_commands.insert(name.clone(), vec![]);
+            }
+            keep_going.failed_commands.get_mut(&name).unwrap().push(format!("{:#}", line));
+        }
+        Ok(())
+    } else {
+        res
+    }
+}
+
+fn exec_cargo_inner(
+    cx: &Context<'_>,
+    id: &PackageId,
+    line: &mut ProcessBuilder<'_>,
+    progress: &mut Progress,
+) -> Result<()> {
+    if progress.count != 0 {
+        eprintln!();
+    }
     progress.count += 1;
 
     if cx.clean_per_run {
diff --git a/src/metadata.rs b/src/metadata.rs
index 5aa58efd..2c6b99b5 100644
--- a/src/metadata.rs
+++ b/src/metadata.rs
@@ -295,6 +295,10 @@ impl Package {
             },
         }))
     }
+
+    pub(crate) fn optional_deps(&self) -> impl Iterator<Item = &str> + '_ {
+        self.dependencies.iter().filter_map(Dependency::as_feature)
+    }
 }
 
 /// A dependency of the main crate.
diff --git a/src/process.rs b/src/process.rs
index cbe12bfd..6bba4428 100644
--- a/src/process.rs
+++ b/src/process.rs
@@ -44,11 +44,6 @@ pub(crate) struct ProcessBuilder<'a> {
     /// This list always has a trailing comma if it is not empty.
     // cargo less than Rust 1.38 cannot handle multiple '--features' flags, so it creates another String.
     features: String,
-
-    /// `true` to include full program path in display.
-    display_program_path: bool,
-    /// `true` to include manifest path in display.
-    display_manifest_path: bool,
 }
 
 impl<'a> ProcessBuilder<'a> {
@@ -61,8 +56,6 @@ impl<'a> ProcessBuilder<'a> {
             leading_args: Vec::new(),
             args: Vec::new(),
             features: String::new(),
-            display_program_path: term::verbose(),
-            display_manifest_path: term::verbose(),
         }
     }
 
@@ -116,12 +109,6 @@ impl<'a> ProcessBuilder<'a> {
         }
     }
 
-    /// Enables all display-related flags.
-    fn display_all(&mut self) {
-        self.display_program_path = true;
-        self.display_manifest_path = true;
-    }
-
     /// Gets the comma-separated features list
     fn get_features(&self) -> &str {
         // drop a trailing comma if it is not empty.
@@ -132,15 +119,13 @@ impl<'a> ProcessBuilder<'a> {
     /// status to an error.
     pub(crate) fn run(&mut self) -> Result<()> {
         let status = self.build().status().with_context(|| {
-            self.display_all();
-            ProcessError::new(&format!("could not execute process {}", self), None, None)
+            ProcessError::new(&format!("could not execute process {:#}", self), None, None)
         })?;
         if status.success() {
             Ok(())
         } else {
-            self.display_all();
             Err(ProcessError::new(
-                &format!("process didn't exit successfully: {}", self),
+                &format!("process didn't exit successfully: {:#}", self),
                 Some(status),
                 None,
             )
@@ -152,15 +137,13 @@ impl<'a> ProcessBuilder<'a> {
     /// output, or an error if non-zero exit status.
     pub(crate) fn run_with_output(&mut self) -> Result<Output> {
         let output = self.build().output().with_context(|| {
-            self.display_all();
-            ProcessError::new(&format!("could not execute process {}", self), None, None)
+            ProcessError::new(&format!("could not execute process {:#}", self), None, None)
         })?;
         if output.status.success() {
             Ok(output)
         } else {
-            self.display_all();
             Err(ProcessError::new(
-                &format!("process didn't exit successfully: {}", self),
+                &format!("process didn't exit successfully: {:#}", self),
                 Some(output.status),
                 Some(&output),
             )
@@ -171,10 +154,8 @@ impl<'a> ProcessBuilder<'a> {
     /// Executes a process, captures its stdio output, returning the captured
     /// standard output as a `String`.
     pub(crate) fn read(&mut self) -> Result<String> {
-        let mut output = String::from_utf8(self.run_with_output()?.stdout).with_context(|| {
-            self.display_all();
-            format!("failed to parse output from {}", self)
-        })?;
+        let mut output = String::from_utf8(self.run_with_output()?.stdout)
+            .with_context(|| format!("failed to parse output from {:#}", self))?;
         while output.ends_with('\n') || output.ends_with('\r') {
             output.pop();
         }
@@ -204,7 +185,7 @@ impl fmt::Display for ProcessBuilder<'_> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         write!(f, "`")?;
 
-        if self.display_program_path {
+        if f.alternate() || term::verbose() {
             write!(f, "{}", self.program.to_string_lossy())?;
         } else {
             write!(f, "{}", Path::new(&*self.program).file_stem().unwrap().to_string_lossy())?;
@@ -223,7 +204,7 @@ impl fmt::Display for ProcessBuilder<'_> {
             if arg == "--manifest-path" {
                 let path = Path::new(args.next().unwrap());
                 // Displaying `--manifest-path` is redundant.
-                if self.display_manifest_path {
+                if f.alternate() || term::verbose() {
                     let path = env::current_dir()
                         .ok()
                         .and_then(|cwd| path.strip_prefix(&cwd).ok())


Test patch:
diff --git a/tests/fixtures/keep_going/Cargo.toml b/tests/fixtures/keep_going/Cargo.toml
new file mode 100644
index 00000000..ea75c9ea
--- /dev/null
+++ b/tests/fixtures/keep_going/Cargo.toml
@@ -0,0 +1,13 @@
+[package]
+name = "keep_going"
+version = "0.0.0"
+publish = false
+
+[workspace]
+
+[features]
+a = []
+
+[dependencies]
+
+[dev-dependencies]
diff --git a/tests/fixtures/keep_going/src/main.rs b/tests/fixtures/keep_going/src/main.rs
new file mode 100644
index 00000000..e57f5e62
--- /dev/null
+++ b/tests/fixtures/keep_going/src/main.rs
@@ -0,0 +1,6 @@
+#[cfg(feature = "a")]
+compile_error!("`a` feature specified");
+#[cfg(not(feature = "a"))]
+compile_error!("`a` feature not specified");
+
+fn main() {}
diff --git a/tests/long-help.txt b/tests/long-help.txt
index 8fb12f20..2812cb0a 100644
--- a/tests/long-help.txt
+++ b/tests/long-help.txt
@@ -143,6 +143,9 @@ OPTIONS:
 
             This flag can only be used together with --version-range flag.
 
+        --keep-going
+            Keep going on failure.
+
     -v, --verbose
             Use verbose output.
 
diff --git a/tests/short-help.txt b/tests/short-help.txt
index 71f5c13c..85ab02a8 100644
--- a/tests/short-help.txt
+++ b/tests/short-help.txt
@@ -38,6 +38,7 @@ OPTIONS:
         --clean-per-run                  Remove artifacts for that package before running the
                                          command
         --clean-per-version              Remove artifacts per Rust version
+        --keep-going                     Keep going on failure
     -v, --verbose                        Use verbose output
         --color <WHEN>                   Coloring: auto, always, never
     -h, --help                           Prints help information
diff --git a/tests/test.rs b/tests/test.rs
index 12adabd4..0e2e0119 100644
--- a/tests/test.rs
+++ b/tests/test.rs
@@ -1344,3 +1344,23 @@ fn clean_per_version_failure() {
         .assert_failure("real")
         .stderr_contains("--clean-per-version can only be used together with --version-range");
 }
+
+#[test]
+fn keep_going() {
+    cargo_hack(["check", "--each-feature", "--keep-going"])
+        .assert_failure("keep_going")
+        .stderr_contains(format!(
+            "
+            running `cargo check --no-default-features` on keep_going (1/2)
+            `a` feature not specified
+            running `cargo check --no-default-features --features a` on keep_going (2/2)
+            `a` feature specified
+            failed to run 2 commands
+            failed commands:
+            keep_going:
+            cargo{0} check --manifest-path Cargo.toml --no-default-features`
+            cargo{0} check --manifest-path Cargo.toml --no-default-features --features a`
+            ",
+            env::consts::EXE_SUFFIX
+        ));
+}