chore: xtask refactor. (#231)

Signed-off-by: Andrew Lilley Brinker <[email protected]>

xtask/Cargo.toml

@@ -12,7 +12,8 @@ edition.workspace = true
 [dependencies]
 anyhow = "1.0.80"
 cargo_metadata = "0.18.1"
-clap = "4.5.1"
+clap = { version = "4.5.1", features = ["derive"] }
+derive_more = { version = "1.0.0", features = ["display"] }
 env_logger = "0.11.2"
 log = "0.4.20"
 pathbuf = "1.0.0"

xtask/src/cli.rs

@@ -1,91 +1,82 @@
 //! The `cargo xtask` Command Line Interface (CLI).
 
-use clap::{arg, builder::PossibleValue, value_parser, ArgMatches, Command, ValueEnum};
-use std::fmt::{Display, Formatter, Result as FmtResult};
+use clap::{arg, Parser as _};
 
 /// Define the CLI and parse arguments from the command line.
-pub fn args() -> ArgMatches {
-    Command::new("xtask")
-        .about("Task runner for the OmniBOR Rust workspace")
-        .help_expected(true)
-        .subcommand(
-            Command::new("release")
-                .about("Release a new version of a workspace crate")
-                .arg(
-                    arg!(-c --crate <CRATE>)
-                        .required(true)
-                        .value_parser(value_parser!(Crate))
-                        .help("the crate to release"),
-                )
-                .arg(
-                    arg!(-b --bump <BUMP>)
-                        .required(true)
-                        .value_parser(value_parser!(Bump))
-                        .help("the version to bump"),
-                )
-                .arg(
-                    arg!(-x - -execute)
-                        .required(false)
-                        .default_value("false")
-                        .value_parser(value_parser!(bool))
-                        .help("not a dry run, actually execute the release"),
-                )
-                .arg(
-                    arg!(--"allow-dirty")
-                        .required(false)
-                        .default_value("false")
-                        .value_parser(value_parser!(bool))
-                        .help("allow Git worktree to be dirty"),
-                ),
-        )
-        .get_matches()
+pub fn args() -> Cli {
+    Cli::parse()
 }
 
-/// The crate to release; can be "gitoid" or "omnibor"
-#[derive(Debug, Clone, Copy)]
-pub enum Crate {
-    /// The `gitoid` crate, found in the `gitoid` folder.
-    GitOid,
-
-    /// The `omnibor` crate, found in the `omnibor` folder.
-    OmniBor,
-
-    /// The `omnibor-cli` crate, found in the `omnibor-cli` folder.
-    OmniBorCli,
+#[derive(Debug, clap::Parser)]
+pub struct Cli {
+    #[clap(subcommand)]
+    pub subcommand: Subcommand,
 }
 
-impl Crate {
-    /// Get the name of the crate, as it should be shown in the CLI
-    /// and as it exists on the filesystem.
-    pub fn name(&self) -> &'static str {
-        match self {
-            Crate::GitOid => "gitoid",
-            Crate::OmniBor => "omnibor",
-            Crate::OmniBorCli => "omnibor-cli",
-        }
-    }
+#[derive(Debug, Clone, clap::Subcommand)]
+pub enum Subcommand {
+    /// Release a new version of a crate.
+    ///
+    /// Runs the following steps:
+    ///
+    /// (1) Verifies external tool dependencies are installed,
+    /// (2) Verifies that Git worktree is ready (unless `--allow-dirty`),
+    /// (3) Verifies you're on the `main` branch,
+    /// (4) Verifies that `git-cliff` agrees about the version bump,
+    /// (5) Generates the `CHANGELOG.md`,
+    /// (6) Commits the `CHANGELOG.md`,
+    /// (7) Runs a dry-run `cargo release` (unless `--execute`).
+    ///
+    /// Unless `--execute`, all steps will be rolled back after completion
+    /// of the pipeline. All previously-executed steps will also be rolled back
+    /// if a prior step fails.
+    ///
+    /// Note that this *does not* account for:
+    ///
+    /// (1) Running more than one instance of this command at the same time,
+    /// (2) Running other programs which may interfere with relevant state (like
+    /// Git repo state) at the same time,
+    /// (3) Terminating the program prematurely, causing rollback to fail.
+    ///
+    /// It is your responsibility to cleanup manually if any of the above
+    /// situations arise.
+    Release(ReleaseArgs),
 }
 
-impl Display for Crate {
-    fn fmt(&self, f: &mut Formatter<'_>) -> FmtResult {
-        write!(f, "{}", self.name())
-    }
+#[derive(Debug, Clone, clap::Args)]
+pub struct ReleaseArgs {
+    /// The crate to release.
+    #[arg(short = 'c', long = "crate", value_name = "CRATE")]
+    pub krate: Crate,
+
+    /// The version to bump.
+    #[arg(short = 'b', long = "bump")]
+    pub bump: Bump,
+
+    /// Not a dry-run, actually execute the release.
+    #[arg(short = 'x', long = "execute")]
+    pub execute: bool,
+
+    /// Allow Git worktree to be dirty.
+    #[arg(short = 'd', long = "allow-dirty")]
+    pub allow_dirty: bool,
 }
 
-// This is needed for `clap` to be able to parse string values
-// into this `Crate` enum.
-impl ValueEnum for Crate {
-    fn value_variants<'a>() -> &'a [Self] {
-        &[Crate::GitOid, Crate::OmniBor, Crate::OmniBorCli]
-    }
+/// The crate to release; can be "gitoid" or "omnibor"
+#[derive(Debug, Clone, Copy, derive_more::Display, clap::ValueEnum)]
+pub enum Crate {
+    /// The `gitoid` crate, found in the `gitoid` folder.
+    Gitoid,
+
+    /// The `omnibor` crate, found in the `omnibor` folder.
+    Omnibor,
 
-    fn to_possible_value(&self) -> Option<PossibleValue> {
-        Some(PossibleValue::new(self.name()))
-    }
+    /// The `omnibor-cli` crate, found in the `omnibor-cli` folder.
+    OmniborCli,
 }
 
 /// The version to bump; can be "major", "minor", or "patch"
-#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, derive_more::Display, clap::ValueEnum)]
 pub enum Bump {
     /// Bump the major version.
     Major,
@@ -96,28 +87,3 @@ pub enum Bump {
     /// Bump the patch version.
     Patch,
 }
-
-impl Display for Bump {
-    fn fmt(&self, f: &mut Formatter<'_>) -> FmtResult {
-        match self {
-            Bump::Major => write!(f, "major"),
-            Bump::Minor => write!(f, "minor"),
-            Bump::Patch => write!(f, "patch"),
-        }
-    }
-}
-// This is needed for `clap` to be able to parse string values
-// into this `Bump` enum.
-impl ValueEnum for Bump {
-    fn value_variants<'a>() -> &'a [Self] {
-        &[Bump::Major, Bump::Minor, Bump::Patch]
-    }
-
-    fn to_possible_value(&self) -> Option<PossibleValue> {
-        Some(match self {
-            Bump::Major => PossibleValue::new("major"),
-            Bump::Minor => PossibleValue::new("minor"),
-            Bump::Patch => PossibleValue::new("patch"),
-        })
-    }
-}

xtask/src/main.rs

@@ -16,9 +16,8 @@ fn main() -> ExitCode {
 
     let args = cli::args();
 
-    let res = match args.subcommand() {
-        Some(("release", args)) => release::run(args),
-        Some(_) | None => Ok(()),
+    let res = match args.subcommand {
+        cli::Subcommand::Release(args) => release::run(&args),
     };
 
     if let Err(err) = res {

xtask/src/pipeline.rs

@@ -41,7 +41,7 @@ where
     }
 
     /// Force rollback at the end of the pipeline, regardless of outcome.
-    pub fn force_rollback(&mut self) {
+    pub fn plan_forced_rollback(&mut self) {
         self.force_rollback = true;
     }
 
@@ -93,6 +93,18 @@ macro_rules! step {
     }};
 }
 
+/// Construct a pipeline of steps each implementing the `Step` trait.
+#[macro_export]
+macro_rules! pipeline {
+    ( $($step:expr),* ) => {{
+        Pipeline::new([
+            $(
+                $crate::step!($step)
+            ),*
+        ])
+    }};
+}
+
 /// A pipeline step which mutates the environment and can be undone.
 pub trait Step {
     /// The name of the step, to report to the user.

xtask/src/release.rs

@@ -1,13 +1,12 @@
 //! The `cargo xtask release` subcommand.
 
 use crate::{
-    cli::{Bump, Crate},
+    cli::{Bump, Crate, ReleaseArgs},
+    pipeline,
     pipeline::{Pipeline, Step},
-    step,
 };
 use anyhow::{anyhow, bail, Context as _, Result};
 use cargo_metadata::{Metadata, MetadataCommand, Package};
-use clap::ArgMatches;
 use pathbuf::pathbuf;
 use semver::Version;
 use serde::{de::DeserializeOwned, Deserialize};
@@ -20,27 +19,11 @@ use toml::from_str as from_toml_str;
 use xshell::{cmd, Shell};
 
 /// Run the release command.
-pub fn run(args: &ArgMatches) -> Result<()> {
-    let krate: Crate = *args
-        .get_one("crate")
-        .ok_or_else(|| anyhow!("'--crate' is a required argument"))?;
-
-    let bump: Bump = *args
-        .get_one("bump")
-        .ok_or_else(|| anyhow!("'--bump' is a required argument"))?;
-
-    let execute: bool = *args
-        .get_one("execute")
-        .expect("--execute has a default value, so it should never be missing");
-
-    let allow_dirty: bool = *args
-        .get_one("allow-dirty")
-        .expect("--allow-dirty has a default value, so it should never be missing");
-
+pub fn run(args: &ReleaseArgs) -> Result<()> {
     log::info!(
         "running 'release', bumping the {} version number for crate '{}'",
-        bump,
-        krate
+        args.bump,
+        args.krate
     );
 
     let workspace_metadata = MetadataCommand::new().exec()?;
@@ -50,34 +33,36 @@ pub fn run(args: &ArgMatches) -> Result<()> {
         .clone()
         .into_std_path_buf();
 
-    let pkg = find_pkg(&workspace_metadata, krate)
+    let pkg = find_pkg(&workspace_metadata, args.krate)
         .ok_or_else(|| anyhow!("failed to find package in workspace"))?;
 
-    let mut pipeline = Pipeline::new([
-        step!(CheckDependencies),
-        step!(CheckGitReady { allow_dirty }),
-        step!(CheckGitBranch),
-        step!(CheckChangelogVersionBump {
+    let mut pipeline = pipeline!(
+        CheckDependencies,
+        CheckGitReady {
+            allow_dirty: args.allow_dirty
+        },
+        CheckGitBranch,
+        CheckChangelogVersionBump {
             workspace_root: workspace_root.clone(),
             crate_version: pkg.version.clone(),
-            krate,
-            bump,
-        }),
-        step!(GenerateChangelog {
+            krate: args.krate,
+            bump: args.bump,
+        },
+        GenerateChangelog {
             workspace_root: workspace_root.clone(),
-            krate
-        }),
-        step!(CommitChangelog { krate }),
-        step!(ReleaseCrate {
-            krate,
-            bump,
-            execute
-        }),
-    ]);
+            krate: args.krate
+        },
+        CommitChangelog { krate: args.krate },
+        ReleaseCrate {
+            krate: args.krate,
+            bump: args.bump,
+            execute: args.execute
+        }
+    );
 
     // If we're not executing, force a rollback at the end.
-    if execute.not() {
-        pipeline.force_rollback();
+    if args.execute.not() {
+        pipeline.plan_forced_rollback();
     }
 
     pipeline.run()
@@ -88,7 +73,7 @@ fn find_pkg(workspace_metadata: &Metadata, krate: Crate) -> Option<&Package> {
     for id in &workspace_metadata.workspace_members {
         let pkg = &workspace_metadata[id];
 
-        if pkg.name == krate.name() {
+        if pkg.name == krate.to_string() {
             return Some(pkg);
         }
     }
@@ -218,7 +203,7 @@ impl CheckChangelogVersionBump {
     }
 
     fn include(&self) -> PathBuf {
-        pathbuf![self.krate.name(), "*"]
+        pathbuf![&self.krate.to_string(), "*"]
     }
 }
 
@@ -289,11 +274,11 @@ impl GenerateChangelog {
     }
 
     fn include(&self) -> PathBuf {
-        pathbuf![self.krate.name(), "*"]
+        pathbuf![&self.krate.to_string(), "*"]
     }
 
     fn output(&self) -> PathBuf {
-        pathbuf![self.krate.name(), "CHANGELOG.md"]
+        pathbuf![&self.krate.to_string(), "CHANGELOG.md"]
     }
 }
 
@@ -332,7 +317,7 @@ struct CommitChangelog {
 
 impl CommitChangelog {
     fn commit_msg(&self) -> String {
-        format!("chore: update `{}` crate CHANGELOG.md", self.krate.name())
+        format!("chore: update `{}` crate CHANGELOG.md", self.krate)
     }
 }
 
@@ -344,7 +329,7 @@ impl Step for CommitChangelog {
     fn run(&mut self) -> Result<()> {
         let sh = Shell::new()?;
         let msg = self.commit_msg();
-        let changelog = pathbuf![self.krate.name(), "CHANGELOG.md"];
+        let changelog = pathbuf![&self.krate.to_string(), "CHANGELOG.md"];
         cmd!(sh, "git add {changelog}").run()?;
         cmd!(sh, "git commit --signoff -m {msg}").run()?;
         Ok(())
@@ -377,7 +362,7 @@ impl Step for ReleaseCrate {
 
     fn run(&mut self) -> Result<()> {
         let sh = Shell::new()?;
-        let krate = self.krate.name();
+        let krate = self.krate.to_string();
         let bump = self.bump.to_string();
 
         let execute = if self.execute {