refactor cargo xtasks (#2907)

* improve error handling
* show executed command lines
* reduce code duplication (run_command, run_mdbook_command)

xtask/src/main.rs

@@ -19,18 +19,14 @@
 //! `cargo xtask install-tools` and the logic defined here will install
 //! the tools.
 
-use anyhow::{Ok, Result, anyhow};
+use anyhow::{Context, Result, anyhow};
 use clap::{Parser, Subcommand};
 use std::env;
 use std::path::{Path, PathBuf};
-use std::process::{Command, Stdio};
+use std::process::Command;
 
 fn main() -> Result<()> {
-    if let Err(e) = execute_task() {
+    execute_task()
-        eprintln!("{e}");
-        std::process::exit(-1);
-    }
-    Ok(())
 }
 
 #[derive(Parser)]
@@ -83,11 +79,28 @@ enum Task {
 fn execute_task() -> Result<()> {
     let cli = Cli::parse();
     match cli.task {
-        Task::InstallTools => install_tools()?,
+        Task::InstallTools => install_tools(),
-        Task::WebTests { dir } => run_web_tests(dir)?,
+        Task::WebTests { dir } => run_web_tests(dir),
-        Task::RustTests => run_rust_tests()?,
+        Task::RustTests => run_rust_tests(),
-        Task::Serve { language, output } => start_web_server(language, output)?,
+        Task::Serve { language, output } => start_web_server(language, output),
-        Task::Build { language, output } => build(language, output)?,
+        Task::Build { language, output } => build(language, output),
+    }
+}
+
+/// Executes a command and returns an error if it fails.
+fn run_command(cmd: &mut Command) -> Result<()> {
+    let command_display = format!("{cmd:?}");
+    println!("> {command_display}");
+    let status = cmd
+        .status()
+        .with_context(|| format!("Failed to execute command: {command_display}"))?;
+    if !status.success() {
+        let exit_description = if let Some(code) = status.code() {
+            format!("exited with status code: {}", code)
+        } else {
+            "was terminated by a signal".to_string()
+        };
+        return Err(anyhow!("Command `{command_display}` {exit_description}"));
     }
     Ok(())
 }
@@ -97,155 +110,126 @@ fn install_tools() -> Result<()> {
 
     const PINNED_NIGHTLY: &str = "nightly-2025-09-01";
 
+    // Install rustup components
     let rustup_steps = [
         ["toolchain", "install", "--profile", "minimal", PINNED_NIGHTLY],
         ["component", "add", "rustfmt", "--toolchain", PINNED_NIGHTLY],
     ];
-
     for args in rustup_steps {
-        let status = std::process::Command::new("rustup").args(args).status()?;
+        let mut cmd = Command::new("rustup");
-        if !status.success() {
+        cmd.args(args);
-            return Err(anyhow!(
+        run_command(&mut cmd)?;
-                "Command 'rustup {}' failed with status {:?}",
-                args.join(" "),
-                status.code()
-            ));
-        }
     }
 
-    let path_to_mdbook_exerciser =
+    let cargo = env!("CARGO");
-        Path::new(env!("CARGO_WORKSPACE_DIR")).join("mdbook-exerciser");
+    // The --locked flag is important for reproducible builds.
-    let path_to_mdbook_course =
+    let tools = [
-        Path::new(env!("CARGO_WORKSPACE_DIR")).join("mdbook-course");
+        ("mdbook", "0.4.52"),
-
+        ("mdbook-svgbob", "0.2.2"),
-    let install_args = vec![
+        ("mdbook-pandoc", "0.10.4"),
-        // The --locked flag is important for reproducible builds. It also
+        ("mdbook-i18n-helpers", "0.3.6"),
-        // avoids breakage due to skews between mdbook and mdbook-svgbob.
+        ("i18n-report", "0.2.0"),
-        vec!["mdbook", "--locked", "--version", "0.4.52"],
+        ("mdbook-linkcheck2", "0.9.1"),
-        vec!["mdbook-svgbob", "--locked", "--version", "0.2.2"],
-        vec!["mdbook-pandoc", "--locked", "--version", "0.10.4"],
-        vec!["mdbook-i18n-helpers", "--locked", "--version", "0.3.6"],
-        vec!["i18n-report", "--locked", "--version", "0.2.0"],
-        vec!["mdbook-linkcheck2", "--locked", "--version", "0.9.1"],
-        // Mdbook-exerciser and mdbook-course are located in this repository.
-        // To make it possible to install them from any directory we need to
-        // specify their path from the workspace root.
-        vec!["--path", path_to_mdbook_exerciser.to_str().unwrap(), "--locked"],
-        vec!["--path", path_to_mdbook_course.to_str().unwrap(), "--locked"],
     ];
 
-    for args in &install_args {
+    for (tool, version) in tools {
-        let status = Command::new(env!("CARGO"))
+        let mut cmd = Command::new(cargo);
-            .arg("install")
+        cmd.args(["install", tool, "--version", version, "--locked"]);
-            .args(args)
+        run_command(&mut cmd)?;
-            .status()
+    }
-            .expect("Failed to execute cargo install");
 
-        if !status.success() {
+    // Install local tools from the workspace.
-            let error_message = format!(
+    let workspace_dir = Path::new(env!("CARGO_WORKSPACE_DIR"));
-                "Command 'cargo install {}' exited with status code: {}",
+    let local_tools = ["mdbook-exerciser", "mdbook-course"];
-                args.join(" "),
+    for tool in local_tools {
-                status.code().unwrap()
+        let mut cmd = Command::new(cargo);
-            );
+        cmd.args(["install", "--path"])
-            return Err(anyhow!(error_message));
+            .arg(workspace_dir.join(tool))
-        }
+            .arg("--locked");
+        run_command(&mut cmd)?;
     }
 
     // Uninstall original linkcheck if currently installed (see issue no 2773)
-    uninstall_mdbook_linkcheck();
+    uninstall_mdbook_linkcheck()?;
 
     Ok(())
 }
 
-fn uninstall_mdbook_linkcheck() {
+fn uninstall_mdbook_linkcheck() -> Result<()> {
     println!("Uninstalling old mdbook-linkcheck if installed...");
     let output = Command::new(env!("CARGO"))
-        .arg("uninstall")
+        .args(["uninstall", "mdbook-linkcheck"])
-        .arg("mdbook-linkcheck")
         .output()
-        .expect("Failed to execute cargo uninstall mdbook-linkcheck");
+        .context("Failed to execute `cargo uninstall mdbook-linkcheck`")?;
 
     if !output.status.success() {
-        if String::from_utf8_lossy(&output.stderr)
+        let stderr = String::from_utf8_lossy(&output.stderr);
-            .into_owned()
+        // This specific error is OK, it just means the package wasn't installed.
-            .contains("did not match any packages")
+        if !stderr.contains("did not match any packages") {
-        {
+            return Err(anyhow!(
-            println!("mdbook-linkcheck not installed. Continuing...");
+                "Failed to uninstall `mdbook-linkcheck`.\n--- stderr:\n{stderr}"
-        } else {
+            ));
-            eprintln!(
-                "An error occurred during uninstallation of mdbook-linkcheck:\n{:#?}",
-                String::from_utf8_lossy(&output.stderr)
-            );
         }
+        println!("mdbook-linkcheck not installed. Continuing...");
     }
+    Ok(())
 }
 
 fn run_web_tests(dir: Option<PathBuf>) -> Result<()> {
     println!("Running web tests...");
+    let workspace_dir = Path::new(env!("CARGO_WORKSPACE_DIR"));
 
     let absolute_dir = dir.map(|d| d.canonicalize()).transpose()?;
 
     if let Some(d) = &absolute_dir {
         println!("Refreshing slide lists...");
-        let path_to_refresh_slides_script = Path::new("tests")
+        let refresh_slides_script = Path::new("tests")
             .join("src")
             .join("slides")
             .join("create-slide.list.sh");
-        let status = Command::new(path_to_refresh_slides_script)
+        let mut cmd = Command::new(&refresh_slides_script);
-            .current_dir(Path::new(env!("CARGO_WORKSPACE_DIR")))
+        cmd.current_dir(workspace_dir).arg(d);
-            .arg(d)
+        run_command(&mut cmd)?;
-            .status()
-            .expect("Failed to execute create-slide.list.sh");
-
-        if !status.success() {
-            let error_message = format!(
-                "Command 'cargo xtask web-tests' exited with status code: {}",
-                status.code().unwrap()
-            );
-            return Err(anyhow!(error_message));
-        }
     }
 
-    let path_to_tests_dir = Path::new(env!("CARGO_WORKSPACE_DIR")).join("tests");
+    let tests_dir = workspace_dir.join("tests");
-    let mut command = Command::new("npm");
+    let mut cmd = Command::new("npm");
-    command.current_dir(path_to_tests_dir.to_str().unwrap());
+    cmd.current_dir(tests_dir).arg("test");
-    command.arg("test");
 
     if let Some(d) = absolute_dir {
-        command.env("TEST_BOOK_DIR", d);
+        cmd.env("TEST_BOOK_DIR", d);
     }
-    let status = command.status().expect("Failed to execute npm test");
+    run_command(&mut cmd)
-
-    if !status.success() {
-        let error_message = format!(
-            "Command 'cargo xtask web-tests' exited with status code: {}",
-            status.code().unwrap()
-        );
-        return Err(anyhow!(error_message));
-    }
-
-    Ok(())
 }
 
 fn run_rust_tests() -> Result<()> {
     println!("Running rust tests...");
-    let path_to_workspace_root = Path::new(env!("CARGO_WORKSPACE_DIR"));
+    let workspace_root = Path::new(env!("CARGO_WORKSPACE_DIR"));
 
-    let status = Command::new("mdbook")
+    let mut cmd = Command::new("mdbook");
-        .current_dir(path_to_workspace_root.to_str().unwrap())
+    cmd.current_dir(workspace_root).arg("test");
-        .arg("test")
+    run_command(&mut cmd)
-        .status()
+}
-        .expect("Failed to execute mdbook test");
 
-    if !status.success() {
+fn run_mdbook_command(
-        let error_message = format!(
+    subcommand: &str,
-            "Command 'cargo xtask rust-tests' exited with status code: {}",
+    language: Option<String>,
-            status.code().unwrap()
+    output_arg: Option<PathBuf>,
-        );
+) -> Result<()> {
-        return Err(anyhow!(error_message));
+    let workspace_root = Path::new(env!("CARGO_WORKSPACE_DIR"));
+
+    let mut cmd = Command::new("mdbook");
+    cmd.current_dir(workspace_root).arg(subcommand);
+
+    if let Some(language) = &language {
+        println!("Language: {language}");
+        cmd.env("MDBOOK_BOOK__LANGUAGE", language);
     }
 
-    Ok(())
+    cmd.arg("-d");
+    cmd.arg(get_output_dir(language, output_arg));
+
+    run_command(&mut cmd)
 }
 
 fn start_web_server(
@@ -253,58 +237,12 @@ fn start_web_server(
     output_arg: Option<PathBuf>,
 ) -> Result<()> {
     println!("Starting web server ...");
-    let path_to_workspace_root = Path::new(env!("CARGO_WORKSPACE_DIR"));
+    run_mdbook_command("serve", language, output_arg)
-
-    let mut command = Command::new("mdbook");
-    command.current_dir(path_to_workspace_root.to_str().unwrap());
-    command.arg("serve");
-
-    if let Some(language) = &language {
-        println!("Language: {}", &language);
-        command.env("MDBOOK_BOOK__LANGUAGE", &language);
-    }
-
-    command.arg("-d");
-    command.arg(get_output_dir(language, output_arg));
-
-    let status = command.status().expect("Failed to execute mdbook serve");
-
-    if !status.success() {
-        let error_message = format!(
-            "Command 'cargo xtask serve' exited with status code: {}",
-            status.code().unwrap()
-        );
-        return Err(anyhow!(error_message));
-    }
-    Ok(())
 }
 
 fn build(language: Option<String>, output_arg: Option<PathBuf>) -> Result<()> {
     println!("Building course...");
-    let path_to_workspace_root = Path::new(env!("CARGO_WORKSPACE_DIR"));
+    run_mdbook_command("build", language, output_arg)
-
-    let mut command = Command::new("mdbook");
-    command.current_dir(path_to_workspace_root.to_str().unwrap());
-    command.arg("build");
-
-    if let Some(language) = &language {
-        println!("Language: {}", &language);
-        command.env("MDBOOK_BOOK__LANGUAGE", language);
-    }
-
-    command.arg("-d");
-    command.arg(get_output_dir(language, output_arg));
-
-    let status = command.status().expect("Failed to execute mdbook build");
-
-    if !status.success() {
-        let error_message = format!(
-            "Command 'cargo xtask build' exited with status code: {}",
-            status.code().unwrap()
-        );
-        return Err(anyhow!(error_message));
-    }
-    Ok(())
 }
 
 fn get_output_dir(language: Option<String>, output_arg: Option<PathBuf>) -> PathBuf {