Repository: google/comprehensive-rust
File: xtask/src/main.rs (Rust)

Code before:
fn install_tools() -> Result<(), DynError> {
    println!("Installing project tools...");

    let install_args: Vec<Vec<&str>> = vec![
        // The --locked flag is important for reproducible builds. It also
        // avoids breakage due to skews between mdbook and mdbook-svgbob.
        vec!["mdbook", "--locked", "--version", "0.4.44"],
        vec!["mdbook-svgbob", "--locked", "--version", "0.2.1"],
        vec!["mdbook-pandoc", "--locked", "--version", "0.9.3"],
        vec!["mdbook-i18n-helpers", "--locked", "--version", "0.3.5"],
        vec!["i18n-report", "--locked", "--version", "0.2.0"],
        // These packages are located in this repository
        vec!["--path", "mdbook-exerciser", "--locked"],
        vec!["--path", "mdbook-course", "--locked"],
    ];

    for args in &install_args {
        let status = Command::new(env!("CARGO"))
            .arg("install")
            .args(args)
            .status()
            .expect("Failed to execute cargo install");

        if !status.success() {
            let error_message = format!(
                "Command 'cargo install {}' exited with status code: {}",
                args.join(" "),
                status.code().unwrap()
            );
            return Err(Box::from(error_message));
        }
    }

    Ok(())
}

fn get_help_string(task: Option<&str>) -> String {
    if let Some(t) = task {
        format!(
            "Unrecognized task '{t}'. Available tasks:

install-tools            Installs the tools the project depends on."
        )
    } else {
        "Missing task. To execute a task run `cargo xtask [task]`.".to_string()
    }
}


Review comment (suggestion):
I'm surprised it should be necessary with this case? I would have expected Clap to be able to handle it for us?

We can merge this PR now, but could you look into using a Rust enum to represent the available commands? I'm pretty sure Clap can be told that a command line argument must be a valid enum variant — and so it can return an error earlier if people use an unrecognized name.

I think there is an example of what I mean here: https://docs.rs/clap/latest/clap/_cookbook/cargo_example_derive/index.html

We want each xtask (such as `install-tools`) to become a sub-command.

Code after:
use clap::Parser;
use std::{env, process::Command};

fn main() -> Result<()> {
    if let Err(e) = execute_task() {
        eprintln!("{e}");
        std::process::exit(-1);
    }
    Ok(())
}

#[derive(Parser, Debug)]
#[command(
    about = "Binary for executing tasks within the Comprehensive Rust project"
)]
struct Args {
    #[arg(required = true, help = "The task to execute")]
    task: String,
}

fn execute_task() -> Result<()> {
    let task = Args::parse().task;
    match task.as_str() {
        "install-tools" => install_tools()?,
        _ => {
            return Err(anyhow!(unrecognized_task_string(task.as_str())));
        }
    }
    Ok(())
}

fn install_tools() -> Result<()> {
    println!("Installing project tools...");

    let install_args = vec![
        // The --locked flag is important for reproducible builds. It also
        // avoids breakage due to skews between mdbook and mdbook-svgbob.
        vec!["mdbook", "--locked", "--version", "0.4.44"],
        vec!["mdbook-svgbob", "--locked", "--version", "0.2.1"],
        vec!["mdbook-pandoc", "--locked", "--version", "0.9.3"],
        vec!["mdbook-i18n-helpers", "--locked", "--version", "0.3.5"],
        vec!["i18n-report", "--locked", "--version", "0.2.0"],
        // These packages are located in this repository
        vec!["--path", "mdbook-exerciser", "--locked"],
        vec!["--path", "mdbook-course", "--locked"],
    ];

    for args in &install_args {
        let status = Command::new(env!("CARGO"))
            .arg("install")
            .args(args)