Use a shell for lifecycle commands and improve hook errors

README.md

@@ -108,7 +108,7 @@ hyperfine -S 'my-command > output-${HYPERFINE_ITERATION}.log'
 
 ### Intermediate shell
 
-By default, commands are executed directly, without an intermediate shell (`--shell=none`).
+By default, benchmarked commands, including warmup runs, are executed directly, without an intermediate shell (`--shell=none`).
 Arguments are split using shell-like quoting, so quoted arguments containing spaces are supported.
 Shell syntax such as pipes, redirections, environment-variable expansion, `*`, and `~` is not interpreted.
 This avoids shell startup overhead and the noise from correcting for it, especially for fast commands
@@ -125,10 +125,11 @@ You can also select a specific shell with `--shell <SHELL>`:
 hyperfine --shell zsh 'for i in {1..10000}; do echo test; done'
 ```
 
-The shell setting applies to all commands, including `--setup`, `--prepare`, `--conclude`, and `--cleanup`.
-If any of these commands need shell syntax, enable a shell explicitly.
+The `--setup`, `--prepare`, `--conclude`, and `--cleanup` commands always run through a shell.
+They use the shell selected by `--shell`, or the platform shell (`sh` on Unix, `cmd.exe` on Windows)
+when `--shell` is omitted or set to `none`.
 
-When a shell is enabled, hyperfine *corrects for the shell spawning time*. It runs the shell with an
+When benchmarked commands use a shell, hyperfine *corrects for the shell spawning time*. It runs the shell with an
 empty command multiple times to measure its startup time, then subtracts this time from each
 time measurement.
 

doc/hyperfine.1

@@ -194,14 +194,16 @@ possible parameter combinations.
 .HP
 \fB\-\-shell\fR \fISHELL\fP
 .IP
-Set the shell to use for executing commands (default: none). This can be
-the name or the path to the shell executable, or a full command line
+Set the shell to use for executing benchmarked commands, including warmup runs
+(default: none). This can be the name or the path to the shell executable,
+or a full command line
 like "bash \fB\-\-norc\fR". It can also be set to "default" to explicitly
 select the platform shell (sh on Unix, cmd.exe on Windows). Finally, this can also be
-set to "none" to disable the shell. In this case, commands will be
+set to "none" to disable the shell. In this case, benchmarked commands will be
 executed directly. They can still have arguments, but more complex
 things like "sleep 0.1; sleep 0.2" are not possible without a shell.
-This option also applies to setup, prepare, conclude, and cleanup commands.
+Setup, prepare, conclude, and cleanup commands always use a shell: the selected
+shell, or the platform shell when this option is omitted or set to "none".
 .HP
 \fB\-S\fR
 .IP

src/benchmark/executor.rs

@@ -40,6 +40,21 @@ pub trait Executor {
         output_policy: &CommandOutputPolicy,
     ) -> Result<Measurement>;
 
+    /// Run a lifecycle command through a shell, always treating failures as errors.
+    fn run_intermediate_command_and_measure(
+        &self,
+        command: &Command<'_>,
+        iteration: BenchmarkIteration,
+        output_policy: &CommandOutputPolicy,
+    ) -> Result<Measurement> {
+        self.run_command_and_measure(
+            command,
+            iteration,
+            Some(CmdFailureAction::RaiseError),
+            output_policy,
+        )
+    }
+
     /// Perform a calibration of this executor. For example,
     /// when running commands through a shell, we need to
     /// measure the shell spawning time separately in order
@@ -153,6 +168,18 @@ impl Executor for RawExecutor<'_> {
         )
     }
 
+    fn run_intermediate_command_and_measure(
+        &self,
+        command: &Command<'_>,
+        iteration: BenchmarkIteration,
+        output_policy: &CommandOutputPolicy,
+    ) -> Result<Measurement> {
+        // Keep shell startup in the hook's duration: raw benchmarks have no
+        // calibrated shell overhead to add back when estimating the run count.
+        ShellExecutor::new(&Shell::platform_default(), self.options)
+            .run_intermediate_command_and_measure(command, iteration, output_policy)
+    }
+
     fn calibrate(&mut self) -> Result<()> {
         Ok(())
     }

src/benchmark/mod.rs

@@ -11,9 +11,7 @@ use crate::benchmark::benchmark_result::Parameter;
 use crate::benchmark::executor::BenchmarkIteration;
 use crate::benchmark::measurement::{Measurement, Measurements};
 use crate::command::Command;
-use crate::options::{
-    CmdFailureAction, CommandOutputPolicy, ExecutorKind, Options, OutputStyleOption,
-};
+use crate::options::{CommandOutputPolicy, ExecutorKind, Options, OutputStyleOption};
 use crate::outlier_detection::OUTLIER_THRESHOLD;
 use crate::output::console_writeln;
 use crate::output::progress_bar::{
@@ -25,7 +23,7 @@ use crate::parameter::ParameterNameAndValue;
 use crate::quantity::{self, const_time_from_seconds, Time, Zero};
 use benchmark_result::BenchmarkResult;
 
-use anyhow::{ensure, Context, Result};
+use anyhow::{anyhow, ensure, Result};
 use colored::*;
 
 use self::executor::Executor;
@@ -64,22 +62,14 @@ impl<'a> Benchmark<'a> {
         iteration: executor::BenchmarkIteration,
     ) -> Result<Measurement> {
         self.executor
-            .run_command_and_measure(
-                command,
-                iteration,
-                Some(CmdFailureAction::RaiseError),
-                output_policy,
-            )
-            .with_context(|| {
-                let ignore_failure = if cfg!(windows) {
-                    " || exit /b 0"
+            .run_intermediate_command_and_measure(command, iteration, output_policy)
+            .map_err(|error| {
+                let hint = if cfg!(windows) {
+                    "Append ' || exit /b 0' to the command if this failure can be ignored."
                 } else {
-                    " || true"
+                    "Append ' || true' to the command if this failure can be ignored."
                 };
-                format!(
-                    "The {kind} command failed. If this failure can be ignored, \
-                     use '-S'/'--shell=default' and append '{ignore_failure}' to the command"
-                )
+                anyhow!("The {kind} command failed: {error:#} {hint}")
             })
     }
 

src/cli.rs

@@ -236,14 +236,15 @@ fn build_command() -> Command {
                 .value_name("SHELL")
                 .overrides_with("shell")
                 .value_hint(ValueHint::CommandString)
-                .help("Set the shell to use for executing commands (default: none). This can be the \
-                       name or the path to the shell executable, or a full command line \
+                .help("Set the shell to use for executing benchmarked commands, including warmup runs \
+                       (default: none). This can be the name or the path to the shell executable, or a full command line \
                        like \"bash --norc\". It can also be set to \"default\" to explicitly select \
                        the platform shell (sh on Unix, cmd.exe on Windows). It can also be set to \
-                       \"none\" to disable the shell. In this case, commands will be executed \
+                       \"none\" to disable the shell. In this case, benchmarked commands will be executed \
                        directly. They can still have arguments, but more complex things like \
-                       \"sleep 0.1; sleep 0.2\" are not possible without a shell. This option also \
-                       applies to setup, prepare, conclude, and cleanup commands.")
+                       \"sleep 0.1; sleep 0.2\" are not possible without a shell. Setup, prepare, conclude, \
+                       and cleanup commands always use a shell: the selected shell, or the platform \
+                       shell when this option is omitted or set to \"none\".")
         )
         .arg(
             Arg::new("default-shell")

tests/integration_tests.rs

@@ -318,7 +318,7 @@ fn fails_for_unknown_setup_command() {
         .arg("echo test")
         .assert()
         .failure()
-        .stderr(predicate::str::contains("The setup command failed."));
+        .stderr(predicate::str::contains("The setup command failed:"));
 }
 
 #[test]
@@ -330,7 +330,7 @@ fn fails_for_unknown_cleanup_command() {
         .arg("echo test")
         .assert()
         .failure()
-        .stderr(predicate::str::contains("The cleanup command failed."));
+        .stderr(predicate::str::contains("The cleanup command failed:"));
 }
 
 #[test]
@@ -341,7 +341,7 @@ fn fails_for_unknown_prepare_command() {
         .arg("echo test")
         .assert()
         .failure()
-        .stderr(predicate::str::contains("The preparation command failed."));
+        .stderr(predicate::str::contains("The preparation command failed:"));
 }
 
 #[test]
@@ -352,40 +352,7 @@ fn fails_for_unknown_conclude_command() {
         .arg("echo test")
         .assert()
         .failure()
-        .stderr(predicate::str::contains("The conclusion command failed."));
-}
-
-#[cfg(unix)]
-#[test]
-fn hook_errors_preserve_causes_and_explain_shell_syntax() {
-    for hook in ["--setup", "--prepare", "--conclude", "--cleanup"] {
-        for (command, cause) in [
-            (
-                "some-nonexisting-program-b5d9574198b7e4b12a71fa4747c0a577",
-                "No such file or directory",
-            ),
-            ("printf 'unclosed", "missing closing quote"),
-            ("false", "terminated with non-zero exit code 1"),
-        ] {
-            let assert = hyperfine()
-                .args(["--runs=1", hook, command, "true"])
-                .assert()
-                .failure()
-                .stderr(predicate::str::contains(command))
-                .stderr(predicate::str::contains(cause))
-                .stderr(predicate::str::contains("'-S'/'--shell=default'"))
-                .stderr(predicate::str::contains("append ' || true'"))
-                .stderr(predicate::str::contains("--ignore-failure").not());
-            if command != "false" {
-                assert.stderr(predicate::str::contains("non-zero exit code").not());
-            }
-        }
-
-        hyperfine()
-            .args(["-S", "--runs=1", hook, "false || true", "true"])
-            .assert()
-            .success();
-    }
+        .stderr(predicate::str::contains("The conclusion command failed:"));
 }
 
 #[cfg(unix)]
@@ -505,9 +472,14 @@ fn runs_commands_using_user_defined_shell() {
         .arg("--show-output")
         .arg("--shell")
         .arg("echo 'custom_shell' '--shell-arg'")
+        .arg("--setup=echo setup")
+        .arg("--prepare=echo prepare")
+        .arg("--conclude=echo conclude")
+        .arg("--cleanup=echo cleanup")
         .arg("echo benchmark")
         .assert()
         .success()
+        .stdout(predicate::str::contains("custom_shell --shell-arg").count(5))
         .stdout(
             predicate::str::contains("custom_shell --shell-arg -c echo benchmark").or(
                 predicate::str::contains("custom_shell --shell-arg /C echo benchmark"),