From a5b7bcb1e4c160738b37a9e63e5fec08e4830951 Mon Sep 17 00:00:00 2001 From: Andrew Lilley Brinker Date: Tue, 5 Mar 2024 15:31:02 -0800 Subject: [PATCH] feat: Progress implementing `release` for `xtask` (#151) This commit moves closer to a full implementation of the `release` subcommand for `cargo xtask`. This includes some improvements to the underlying pipeline-handling code, as well as improvements to the release workflow itself. There are now more checks run before executing anything, as well as guaranteed rollback, and implementation of the changelog generation is underway. Signed-off-by: Andrew Lilley Brinker --- Cargo.toml | 80 ++++++++++++++ xtask/Cargo.toml | 4 +- xtask/src/cli.rs | 20 ++-- xtask/src/pipeline.rs | 87 ++++++++------- xtask/src/release.rs | 239 ++++++++++++++++++++++++++++++++++-------- 5 files changed, 344 insertions(+), 86 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 16d31c2..bb0878f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -30,3 +30,83 @@ lto = "thin" [workspace.metadata.release] pre-release-commit-message = "chore: Release" + + +# git-cliff ~ configuration +# https://git-cliff.org/docs/configuration + +[workspace.metadata.git-cliff.changelog] +# changelog header +header = """ +# Changelog\n +All notable changes to this project will be documented in this file. + +The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), +and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).\n +""" +# template for the changelog body +# https://keats.github.io/tera/docs/#introduction +body = """ +{% if version -%} + ## [{{ version | trim_start_matches(pat="v") }}] - {{ timestamp | date(format="%Y-%m-%d") }} +{% else -%} + ## [Unreleased] +{% endif -%} +{% for group, commits in commits | group_by(attribute="group") %} + ### {{ group | upper_first }} + {% for commit in commits %} + - {{ commit.message | upper_first }}\ + {% endfor %} +{% endfor %}\n +""" +# template for the changelog footer +footer = """ +{% for release in releases -%} + {% if release.version -%} + {% if release.previous.version -%} + [{{ release.version | trim_start_matches(pat="v") }}]: \ + https://github.com/{{ remote.github.owner }}/{{ remote.github.repo }}\ + /compare/{{ release.previous.version }}..{{ release.version }} + {% endif -%} + {% else -%} + [unreleased]: https://github.com/{{ remote.github.owner }}/{{ remote.github.repo }}\ + /compare/{{ release.previous.version }}..HEAD + {% endif -%} +{% endfor %} + +""" +# remove the leading and trailing whitespace from the templates +trim = true + +[workspace.metadata.git-cliff.git] +# parse the commits based on https://www.conventionalcommits.org +conventional_commits = true +# filter out the commits that are not conventional +filter_unconventional = true +# process each line of a commit as an individual commit +split_commits = false +# regex for parsing and grouping commits +commit_parsers = [ + { message = "^.*: add", group = "Added" }, + { message = "^.*: support", group = "Added" }, + { message = "^.*: remove", group = "Removed" }, + { message = "^.*: delete", group = "Removed" }, + { message = "^test", group = "Fixed" }, + { message = "^fix", group = "Fixed" }, + { message = "^.*: fix", group = "Fixed" }, + { message = "^.*", group = "Changed" }, +] +# protect breaking changes from being skipped due to matching a skipping commit_parser +protect_breaking_commits = false +# filter out the commits that are not matched by commit parsers +filter_commits = true +# regex for matching git tags +tag_pattern = "v[0-9].*" +# regex for skipping tags +skip_tags = "v0.1.0-beta.1" +# regex for ignoring tags +ignore_tags = "" +# sort the tags topologically +topo_order = false +# sort the commits inside sections by oldest/newest order +sort_commits = "oldest" \ No newline at end of file diff --git a/xtask/Cargo.toml b/xtask/Cargo.toml index dd12091..9a08d92 100644 --- a/xtask/Cargo.toml +++ b/xtask/Cargo.toml @@ -11,8 +11,10 @@ edition.workspace = true [dependencies] anyhow = "1.0.80" +cargo_metadata = "0.18.1" clap = "4.5.1" -duct = "0.13.7" env_logger = "0.11.2" log = "0.4.20" +pathbuf = "1.0.0" which = "6.0.0" +xshell = "0.2.5" diff --git a/xtask/src/cli.rs b/xtask/src/cli.rs index 29684cb..599bb13 100644 --- a/xtask/src/cli.rs +++ b/xtask/src/cli.rs @@ -23,6 +23,7 @@ pub fn args() -> ArgMatches { .arg( arg!(--execute) .required(false) + .default_value("false") .value_parser(value_parser!(bool)) .help("not a dry run, actually execute the release"), ), @@ -37,25 +38,28 @@ pub enum Crate { OmniBor, } -impl Display for Crate { - fn fmt(&self, f: &mut Formatter<'_>) -> FmtResult { +impl Crate { + pub fn name(&self) -> &'static str { match self { - Crate::GitOid => write!(f, "gitoid"), - Crate::OmniBor => write!(f, "omnibor"), + Crate::GitOid => "gitoid", + Crate::OmniBor => "omnibor", } } } +impl Display for Crate { + fn fmt(&self, f: &mut Formatter<'_>) -> FmtResult { + write!(f, "{}", self.name()) + } +} + impl ValueEnum for Crate { fn value_variants<'a>() -> &'a [Self] { &[Crate::GitOid, Crate::OmniBor] } fn to_possible_value(&self) -> Option { - Some(match self { - Crate::GitOid => PossibleValue::new("gitoid"), - Crate::OmniBor => PossibleValue::new("omnibor"), - }) + Some(PossibleValue::new(self.name())) } } diff --git a/xtask/src/pipeline.rs b/xtask/src/pipeline.rs index 9118171..2496d92 100644 --- a/xtask/src/pipeline.rs +++ b/xtask/src/pipeline.rs @@ -1,32 +1,45 @@ -use anyhow::{bail, Error, Result}; +use anyhow::{anyhow, bail, Error, Result}; use std::error::Error as StdError; use std::fmt::{Display, Formatter, Result as FmtResult}; use std::iter::Iterator; use std::result::Result as StdResult; -/// A mutable-reference [`Step`]` trait object. -pub type DynStep<'step> = &'step mut dyn Step; - -/// Run a pipeline of steps in order, rolling back if needed. -/// -/// The type signature here is a little funky, but it just means that -/// it takes as a parameter something which can be turned into an owning -/// iterator over mutable references to Step trait objects. -/// -/// This lets the user call it with just a plain array of trait objects, -/// also assisted by the `step!` macro. -pub fn run<'step, I, It>(steps: I) -> Result<()> +pub struct Pipeline where - It: Iterator>, - I: IntoIterator, IntoIter = It>, + It: Iterator, { - fn inner<'step>(steps: impl Iterator>) -> Result<()> { + steps: It, + force_rollback: bool, +} + +impl Pipeline +where + It: Iterator, +{ + /// Construct a new pipeline. + pub fn new(steps: I) -> Self + where + I: IntoIterator, + { + Pipeline { + steps: steps.into_iter(), + force_rollback: false, + } + } + + /// Force rollback at the end of the pipeline, regardless of outcome. + pub fn force_rollback(&mut self) { + self.force_rollback = true; + } + + /// Run the pipeline. + pub fn run(self) -> Result<()> { let mut forward_err = None; let mut completed_steps = Vec::new(); // Run the steps forward. - for step in steps { - if let Err(forward) = forward(step) { + for mut step in self.steps { + if let Err(forward) = forward(step.as_mut()) { forward_err = Some(forward); completed_steps.push(step); break; @@ -35,10 +48,12 @@ where completed_steps.push(step); } - // If forward had an error, initiate rollback. - if let Some(forward_err) = forward_err { - for step in completed_steps { - if let Err(backward_err) = backward(step) { + // If we're forcing rollback or forward had an error, initiate rollback. + if self.force_rollback || forward_err.is_some() { + let forward_err = forward_err.unwrap_or_else(StepError::forced_rollback); + + for mut step in completed_steps { + if let Err(backward_err) = backward(step.as_mut()) { bail!(PipelineError::rollback(forward_err, backward_err)); } } @@ -48,14 +63,15 @@ where Ok(()) } - - inner(steps.into_iter()) } +/// A Boxed [`Step`]` trait object. +pub type DynStep = Box; + #[macro_export] macro_rules! step { ( $step:expr ) => {{ - &mut $step as &mut dyn Step + Box::new($step) as Box }}; } @@ -95,11 +111,8 @@ pub trait Step { /// Note that this trait does _not_ ensure graceful shutdown if /// you cancel an operation with a kill signal before the `undo` /// operation can complete. - fn undo(&mut self) -> Result<()>; - - /// Check if a step mutates the environment, so undo might be skipped. - fn can_skip_undo(&self) -> bool { - false + fn undo(&mut self) -> Result<()> { + Ok(()) } } @@ -115,11 +128,6 @@ fn forward(step: &mut dyn Step) -> StdResult<(), StepError> { /// Helper function to run a step backward and convert the error to [`StepError`] fn backward(step: &mut dyn Step) -> StdResult<(), StepError> { - if step.can_skip_undo() { - log::info!("skipping rollback for step '{}'", step.name()); - return Ok(()); - } - log::info!("rolling back step '{}'", step.name()); step.undo().map_err(|error| StepError { @@ -207,6 +215,15 @@ struct StepError { error: Error, } +impl StepError { + fn forced_rollback() -> Self { + StepError { + name: "forced-rollback", + error: anyhow!("forced rollback"), + } + } +} + impl Display for StepError { fn fmt(&self, f: &mut Formatter<'_>) -> FmtResult { write!(f, "step '{}' failed", self.name) diff --git a/xtask/src/release.rs b/xtask/src/release.rs index c0ed340..73c870b 100644 --- a/xtask/src/release.rs +++ b/xtask/src/release.rs @@ -1,47 +1,58 @@ use crate::cli::{Bump, Crate}; -use crate::pipeline::{self, Step}; +use crate::pipeline::{Pipeline, Step}; use crate::step; -use anyhow::{anyhow, Context as _, Result}; +use anyhow::{anyhow, bail, Error, Result}; +use cargo_metadata::MetadataCommand; use clap::ArgMatches; - -/* - # Run `git-cliff` to generate a changelog. - # Commit the changelog w/ commit msg in Conventional Commit fmt. - # Run `cargo-release` to release the new version. - # If anything fails, rollback prior steps in reverse order. - # Probably good for each step to have a "do" and "undo" operation. - # - # ... In fact I'll probably write this in Rust lol. - - # Need: - # - # - git - # - git-cliff - # - cargo - # - cargo-release -*/ +use pathbuf::pathbuf; +use std::ops::Not as _; +use std::path::PathBuf; +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"); + log::info!( "running 'release', bumping the {} number for crate '{}'", bump, krate ); - pipeline::run([ + let workspace_root = workspace_root()?; + + let mut pipeline = Pipeline::new([ step!(CheckDependencies), - step!(GenerateChangelog), - step!(CommitChangelog), - step!(ReleaseCrate), - ]) + step!(CheckGitReady), + step!(CheckGitBranch), + step!(GenerateChangelog { + workspace_root, + krate + }), + step!(CommitChangelog { krate }), + step!(ReleaseCrate { + krate, + bump, + execute + }), + ]); + + // If we're not executing, force a rollback at the end. + if execute.not() { + pipeline.force_rollback(); + } + + pipeline.run() } struct CheckDependencies; @@ -52,39 +63,151 @@ impl Step for CheckDependencies { } fn run(&mut self) -> Result<()> { - check_cmd("git")?; - check_cmd("git-cliff")?; - check_cmd("cargo")?; - check_cmd("cargo-release")?; + let mut missing_cmds = Vec::new(); + + check_cmd(&mut missing_cmds, "git"); + check_cmd(&mut missing_cmds, "git-cliff"); + check_cmd(&mut missing_cmds, "cargo"); + check_cmd(&mut missing_cmds, "cargo-release"); + + if missing_cmds.is_empty().not() { + let commands = missing_cmds + .iter() + .map(|i| i.name) + .collect::>() + .join(", "); + bail!( + "missing commands: {}; please install before continuing", + commands + ); + } + Ok(()) } +} + +struct CheckGitReady; + +impl Step for CheckGitReady { + fn name(&self) -> &'static str { + "check-git-ready" + } + + fn run(&mut self) -> Result<()> { + let sh = Shell::new()?; + + // 1. Make sure the index is up to date (ignore errors). + let _ = cmd!(sh, "git update-index -q --ignore-submodules --refresh") + .quiet() + .run(); + + // 2. Check for unstaged changes in the working tree. + if cmd!(sh, "git diff-files --quiet --ignore-submodules") + .quiet() + .run() + .is_err() + { + bail!( + "unstaged changes detected in the working tree; commit or stash before continuing" + ); + } + + // 3. Check for uncommitted changes in the index. + if cmd!( + sh, + "git diff-index --cached --quiet HEAD --ignore-submodules" + ) + .quiet() + .run() + .is_err() + { + bail!("uncommitted changes detected in the index; commit or stash before continuing"); + } - fn undo(&mut self) -> Result<()> { Ok(()) } +} + +struct CheckGitBranch; + +impl Step for CheckGitBranch { + fn name(&self) -> &'static str { + "check-git-branch" + } + + fn run(&mut self) -> Result<()> { + let sh = Shell::new()?; + + let current_branch = cmd!(sh, "git rev-parse --abbrev-ref HEAD").quiet().read()?; - fn can_skip_undo(&self) -> bool { - true + if current_branch != "main" { + bail!( + "release must be run on the main branch; change branch to 'main' before continuing" + ); + } + + Ok(()) } } -struct GenerateChangelog; +struct GenerateChangelog { + workspace_root: PathBuf, + krate: Crate, +} + +impl GenerateChangelog { + fn config(&self) -> PathBuf { + pathbuf!["Cargo.toml"] + } + + fn include(&self) -> PathBuf { + pathbuf![self.krate.name(), "*"] + } + + fn output(&self) -> PathBuf { + pathbuf![self.krate.name(), "CHANGELOG.md"] + } +} impl Step for GenerateChangelog { fn name(&self) -> &'static str { "generate-changelog" } + // Generate the CHANGELOG file for the specified crate. fn run(&mut self) -> Result<()> { + let sh = Shell::new()?; + let config = self.config(); + let include = self.include(); + let output = self.output(); + sh.change_dir(&self.workspace_root); + cmd!( + sh, + "git cliff --config {config} --include-path {include} -o {output}" + ) + .quiet() + .run()?; Ok(()) } + // Delete the generated CHANGELOG file. fn undo(&mut self) -> Result<()> { + let sh = Shell::new()?; + let output = self.output(); + cmd!(sh, "rm {output}").quiet().run()?; Ok(()) } } -struct CommitChangelog; +struct CommitChangelog { + krate: Crate, +} + +impl CommitChangelog { + fn commit_msg(&self) -> String { + format!("chore: update `{}` crate CHANGELOG.md", self.krate.name()) + } +} impl Step for CommitChangelog { fn name(&self) -> &'static str { @@ -92,15 +215,36 @@ 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"]; + cmd!(sh, "git add {changelog}").quiet().run()?; + cmd!(sh, "git commit --signoff -m \"{msg}\"") + .quiet() + .run()?; Ok(()) } fn undo(&mut self) -> Result<()> { + let sh = Shell::new()?; + let last_msg = cmd!(sh, "git log -1 --pretty=%s").quiet().read()?; + let expected_msg = self.commit_msg(); + + if last_msg != expected_msg { + bail!("last commit isn't CHANGELOG commit; aborting to avoid breaking git history"); + } + + cmd!(sh, "git reset --hard HEAD~1").quiet().run()?; Ok(()) } } -struct ReleaseCrate; +#[allow(unused)] +struct ReleaseCrate { + krate: Crate, + bump: Bump, + execute: bool, +} impl Step for ReleaseCrate { fn name(&self) -> &'static str { @@ -108,17 +252,28 @@ impl Step for ReleaseCrate { } fn run(&mut self) -> Result<()> { - Ok(()) + bail!("not yet implemented"); } +} - fn undo(&mut self) -> Result<()> { - Ok(()) +/// Check if a command exists on the command line. +fn check_cmd(missing_cmds: &mut Vec, name: &'static str) { + if let Err(err) = which::which(name) { + let err = anyhow!(err); + missing_cmds.push(MissingCmd { name, err }); } } -/// Check if a command exists on the command line. -fn check_cmd(name: &str) -> Result<()> { - which::which(name) - .map(|_| ()) - .context(format!("failed to find command '{}'", name)) +#[derive(Debug)] +struct MissingCmd { + name: &'static str, + #[allow(unused)] + err: Error, +} + +// Figure out the root of the current Cargo workspace. +fn workspace_root() -> Result { + let metadata = MetadataCommand::new().exec()?; + let root = metadata.workspace_root.into_std_path_buf(); + Ok(root) }