From 52b8c52ad969ae143632cd5e2a6805bce33e217d Mon Sep 17 00:00:00 2001 From: "Arnaud (Martient) Leherpeur" Date: Mon, 21 Sep 2026 09:02:13 +0200 Subject: [PATCH 1/2] ci(release): fail the release when Cargo.toml disagrees with the tag v1.10.0 was tagged on a tree whose Cargo.toml still said 1.9.1, so every published binary reported the old version to --version and self-update kept offering the same release forever. A verify-version job now compares the Cargo.toml version with the tag name and both the release creation and the build matrix wait on it, so a mismatched tag fails before any release or asset is published. --- .github/workflows/cargo-release.yml | 22 ++++++++++++++++- tests/release_workflow_tests.rs | 38 +++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 1 deletion(-) create mode 100644 tests/release_workflow_tests.rs diff --git a/.github/workflows/cargo-release.yml b/.github/workflows/cargo-release.yml index 2e79572..6d48257 100644 --- a/.github/workflows/cargo-release.yml +++ b/.github/workflows/cargo-release.yml @@ -13,7 +13,27 @@ on: - "v[0-9]+.[0-9]+.[0-9]+-*" jobs: + verify-version: + name: Verify Cargo.toml matches the tag + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v5 + + - name: Compare Cargo.toml version with tag + shell: bash + run: | + VERSION=$(grep -m1 'version\s*=' Cargo.toml | cut -d'"' -f2 | cut -d"'" -f2) + TAG="$GITHUB_REF_NAME" + if [ "v$VERSION" != "$TAG" ]; then + echo "::error::Tag $TAG points at a tree whose Cargo.toml says $VERSION." + echo "The published binaries would report $VERSION to --version, and self-update would keep offering $TAG forever." + echo "Bump Cargo.toml to ${TAG#v} on the release branch, then move or recreate the tag." + exit 1 + fi + echo "Cargo.toml version $VERSION matches tag $TAG" + create-release: + needs: verify-version runs-on: ubuntu-latest outputs: is_prerelease: ${{ steps.release_type.outputs.is_prerelease }} @@ -54,7 +74,7 @@ jobs: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} release: - needs: create-release + needs: [verify-version, create-release] name: Release for ${{ matrix.target }} runs-on: ${{ matrix.os }} diff --git a/tests/release_workflow_tests.rs b/tests/release_workflow_tests.rs new file mode 100644 index 0000000..d9a395e --- /dev/null +++ b/tests/release_workflow_tests.rs @@ -0,0 +1,38 @@ +//! The release workflow is the last place a version mistake can be caught before +//! binaries reach users, so the guard itself is worth pinning down. + +use std::fs; +use std::path::Path; + +fn release_workflow() -> String { + let path = Path::new(env!("CARGO_MANIFEST_DIR")).join(".github/workflows/cargo-release.yml"); + fs::read_to_string(&path).unwrap_or_else(|e| panic!("cannot read {}: {e}", path.display())) +} + +#[test] +fn release_workflow_verifies_cargo_version_against_the_tag() { + let workflow = release_workflow(); + + assert!( + workflow.contains("verify-version:"), + "the release workflow must carry a job that compares Cargo.toml with the tag" + ); + assert!( + workflow.contains(r#"if [ "v$VERSION" != "$TAG" ]; then"#), + "the guard must compare the Cargo.toml version with the tag name" + ); +} + +#[test] +fn every_release_job_waits_for_the_version_guard() { + let workflow = release_workflow(); + + assert!( + workflow.contains("needs: verify-version"), + "create-release must not publish a release before the version guard passes" + ); + assert!( + workflow.contains("needs: [verify-version, create-release]"), + "the build matrix must not upload assets before the version guard passes" + ); +} From bad3eb4c792bb77147b6e10dfa66479de18c047f Mon Sep 17 00:00:00 2001 From: "Arnaud (Martient) Leherpeur" Date: Mon, 21 Sep 2026 09:02:20 +0200 Subject: [PATCH 2/2] feat(tag)!: bump version files by default when tagging Tagging without --bump-files left the version files stale, which is how a release can be cut whose binaries report the previous version. Version files are now bumped and committed unless --no-bump-files is passed; --bump-files is still accepted so existing scripts keep working, and passing both is an error. Two bugs surfaced by making this the default are fixed alongside it: - version files that already carried the new version were reported as updated, and the empty commit that followed aborted the whole tag operation - multi-package repositories staged the bumped files but never committed them, so their tags pointed at a commit with the old versions --- docs/src/content/docs/reference/tag.mdx | 15 +- src/cli/commands/tag.rs | 103 ++++++++-- src/version/mod.rs | 16 +- tests/tag_bump_files_tests.rs | 255 ++++++++++++++++++++++++ 4 files changed, 362 insertions(+), 27 deletions(-) create mode 100644 tests/tag_bump_files_tests.rs diff --git a/docs/src/content/docs/reference/tag.mdx b/docs/src/content/docs/reference/tag.mdx index ef8e0e7..cfdf36e 100644 --- a/docs/src/content/docs/reference/tag.mdx +++ b/docs/src/content/docs/reference/tag.mdx @@ -27,8 +27,8 @@ committy --non-interactive tag --publish --confirm-publish # Pre-release flow committy --non-interactive tag --prerelease --prerelease-suffix beta -# Bump versions in config files when tagging -committy --non-interactive tag --bump-files +# Tag without touching version files +committy --non-interactive tag --no-bump-files ``` ## Flags (TagCommand) @@ -40,7 +40,13 @@ committy --non-interactive tag --bump-files Confirm creation without prompting (interactive path). If omitted, you'll be prompted. - __--bump-files__ (alias: `-b`) - Auto-bump registered version files to the new version and commit them. + Bump registered version files to the new version and commit them. This is the default, + so the flag is only needed for compatibility with existing scripts. + +- __--no-bump-files__ + Leave version files untouched. Note that the tag then points at a commit whose + `Cargo.toml`/`package.json` still carries the previous version, so binaries built from + it report the old version. - __--output__ `` Output format. Defaults to `text`. @@ -77,7 +83,8 @@ committy --non-interactive tag --bump-files - When `--name` is provided: creates that tag locally. - Non-interactive without `--name`: calculates the new tag from commit log using regex rules in config and options above, then creates it locally. - `--publish --confirm-publish` is required before the tag or any version-bump commit is pushed to `origin`. -- `--bump-files`: updates common version files and commits them (message: `chore: bump version to `). Remote push still requires `--publish --confirm-publish`. +- Version files are bumped by default: common version files are updated and committed (message: `chore: bump version to `) before the tag is created, so the tag contains the new version. Pass `--no-bump-files` to opt out. Remote push still requires `--publish --confirm-publish`. +- In multi-package repositories the same applies per package; the bump commit message lists each package as `@`. - Fetch behavior: default is fetch unless `--no-fetch`; `--fetch` forces fetch. ## JSON output diff --git a/src/cli/commands/tag.rs b/src/cli/commands/tag.rs index 1579b11..05f2c5f 100644 --- a/src/cli/commands/tag.rs +++ b/src/cli/commands/tag.rs @@ -34,10 +34,16 @@ pub struct TagCommand { #[structopt( short = "b", long = "bump-files", - help = "Want to auto bump the config to the new version (y/N)" + help = "Bump version files to the new version (enabled by default)" )] bump_config_files: bool, + #[structopt( + long = "no-bump-files", + help = "Do not touch version files when tagging" + )] + no_bump_config_files: bool, + #[structopt(flatten)] tag_options: git::TagGeneratorOptions, @@ -80,6 +86,12 @@ impl Command for TagCommand { )); } + if self.bump_config_files && self.no_bump_config_files { + return Err(CliError::InputError( + "Use either --bump-files or --no-bump-files, not both".to_string(), + )); + } + let default_repo_path = PathBuf::from("."); let effective_repo_path = if self.repo_path != default_repo_path { self.repo_path.clone() @@ -120,7 +132,7 @@ impl Command for TagCommand { // Explicit tag name provided - use legacy flow let version_manager = git::TagGenerator::new( tag_options.clone(), - self.bump_config_files, + self.should_bump_files(), git_command_config.clone(), ); version_manager.create_and_push_tag(&version_manager.open_repository()?, name)?; @@ -152,7 +164,7 @@ impl Command for TagCommand { // In non-interactive mode, auto-calculate and act based on options let mut version_manager = git::TagGenerator::new( tag_options.clone(), - self.bump_config_files, + self.should_bump_files(), git_command_config.clone(), ); version_manager.run()?; @@ -186,7 +198,7 @@ impl Command for TagCommand { } let mut version_manager = git::TagGenerator::new( tag_options, - self.bump_config_files, + self.should_bump_files(), git_command_config.clone(), ); version_manager.run()?; @@ -218,7 +230,7 @@ impl Command for TagCommand { "is_pre_release", Value::from(version_manager.is_pre_release), ), - ("allow_bump_files", Value::from(self.bump_config_files)), + ("allow_bump_files", Value::from(self.should_bump_files())), ]), )) { @@ -238,6 +250,12 @@ impl Command for TagCommand { } impl TagCommand { + /// Version files are bumped unless the caller opts out with `--no-bump-files`. + /// `--bump-files` is still accepted so existing scripts keep working. + fn should_bump_files(&self) -> bool { + !self.no_bump_config_files + } + /// Execute multi-package tag operation fn execute_multi_package( &self, @@ -298,13 +316,23 @@ impl TagCommand { return Ok(()); } - // Step 5: Update version files per package - if self.bump_config_files { - self.apply_version_updates(repo_path, &version_updates)?; - info!( - "✅ Updated version files for {} package(s)", - version_updates.len() - ); + // Step 5: Update version files per package, and commit them so the tag we + // create below actually points at the bumped versions. + if self.should_bump_files() { + let updated_files = self.apply_version_updates(repo_path, &version_updates)?; + if !updated_files.is_empty() { + info!( + "✅ Updated version files for {} package(s)", + version_updates.len() + ); + self.commit_version_updates( + repo_path, + &updated_files, + &version_updates, + git_command_config, + )?; + info!("✅ Committed version changes"); + } } // Step 6: Update dependencies if requested @@ -488,7 +516,8 @@ impl TagCommand { &self, repo_path: &Path, updates: &[crate::versioning::manager::VersionUpdate], - ) -> Result<(), CliError> { + ) -> Result, CliError> { + let mut updated_files = Vec::new(); for update in updates { // Find package config to get version file let config = RepositoryConfig::try_load(repo_path) @@ -516,16 +545,54 @@ impl TagCommand { let updated = content.replace(&update.old_version, &update.new_version); + // Nothing to stage or commit when the file already carries the new version. + if updated == content { + continue; + } + fs::write(&version_file, updated) .map_err(|e| CliError::Generic(format!("Failed to write version file: {}", e)))?; - // Stage the updated file - if let Err(e) = git::stage_file(&version_file) { - debug!("Failed to stage {}: {}", version_file.display(), e); - } + updated_files.push(version_file); } - Ok(()) + Ok(updated_files) + } + + /// Stage and commit the bumped version files so the tags created afterwards + /// point at a commit that actually contains them. + fn commit_version_updates( + &self, + repo_path: &Path, + updated_files: &[PathBuf], + updates: &[crate::versioning::manager::VersionUpdate], + git_command_config: &git::GitCommandConfig, + ) -> Result<(), CliError> { + let relative_files: Vec = updated_files + .iter() + .map(|file| { + file.strip_prefix(repo_path) + .unwrap_or(file) + .display() + .to_string() + }) + .collect(); + let mut add_args = vec!["add", "--"]; + add_args.extend(relative_files.iter().map(String::as_str)); + git::run_git( + repo_path, + &add_args, + "stage version updates", + git_command_config, + )?; + + let summary = updates + .iter() + .map(|update| format!("{}@{}", update.package_name, update.new_version)) + .collect::>() + .join(", "); + let message = format!("chore: bump version to {summary}"); + git::commit_changes_in_with_config(repo_path, &message, false, git_command_config) } /// Apply dependency updates to other packages diff --git a/src/version/mod.rs b/src/version/mod.rs index 15e6e39..df25eac 100644 --- a/src/version/mod.rs +++ b/src/version/mod.rs @@ -18,9 +18,12 @@ impl VersionFile { }) } - pub fn update_version(&self, new_version: &str) -> Result<(), CliError> { + /// Rewrites the version in this file, returning `true` when the contents + /// actually changed. A file that already carries `new_version` is left alone + /// so callers do not try to commit an empty diff. + pub fn update_version(&self, new_version: &str) -> Result { if !self.path.exists() { - return Ok(()); // Skip if file doesn't exist + return Ok(false); // Skip if file doesn't exist } let content = std::fs::read_to_string(&self.path).map_err(CliError::IoError)?; @@ -30,9 +33,13 @@ impl VersionFile { .replace_all(&content, &self.format.replace("{}", version_without_v)) .to_string(); + if new_content == content { + return Ok(false); + } + std::fs::write(&self.path, new_content).map_err(CliError::IoError)?; - Ok(()) + Ok(true) } } @@ -114,8 +121,7 @@ impl VersionManager { let mut updated_files = Vec::new(); for file in &self.version_files { - if file.path.exists() { - file.update_version(new_version)?; + if file.path.exists() && file.update_version(new_version)? { updated_files.push(file.path.clone()); } } diff --git a/tests/tag_bump_files_tests.rs b/tests/tag_bump_files_tests.rs new file mode 100644 index 0000000..9516202 --- /dev/null +++ b/tests/tag_bump_files_tests.rs @@ -0,0 +1,255 @@ +mod common; + +use git2::{Repository, Signature}; +use predicates::prelude::*; +use std::fs; +use std::path::Path; +use tempfile::tempdir; + +/// A repository on `master` with a Cargo.toml and one feature commit, so the tag +/// command has something to calculate a bump from. +fn setup_cargo_repo() -> tempfile::TempDir { + let dir = tempdir().unwrap(); + let repo = Repository::init(dir.path()).unwrap(); + + let mut config = repo.config().unwrap(); + config.set_str("user.name", "Test User").unwrap(); + config.set_str("user.email", "test@example.com").unwrap(); + + fs::write( + dir.path().join("Cargo.toml"), + "[package]\nname = \"demo\"\nversion = \"0.0.1\"\nedition = \"2021\"\n", + ) + .unwrap(); + + let signature = Signature::now("Test User", "test@example.com").unwrap(); + let mut index = repo.index().unwrap(); + index.add_path(Path::new("Cargo.toml")).unwrap(); + index.write().unwrap(); + let tree_id = index.write_tree().unwrap(); + let tree = repo.find_tree(tree_id).unwrap(); + repo.commit( + Some("HEAD"), + &signature, + &signature, + "feat: add the demo package", + &tree, + &[], + ) + .unwrap(); + + drop(tree); + dir +} + +fn cargo_version(dir: &Path) -> String { + let content = fs::read_to_string(dir.join("Cargo.toml")).unwrap(); + content + .lines() + .find_map(|line| line.strip_prefix("version = ")) + .unwrap() + .trim_matches('"') + .to_string() +} + +fn head_message(repo: &Repository) -> String { + repo.head() + .unwrap() + .peel_to_commit() + .unwrap() + .message() + .unwrap() + .trim() + .to_string() +} + +#[test] +fn tagging_bumps_version_files_by_default() { + let dir = setup_cargo_repo(); + + let mut cmd = common::committy_cmd(); + cmd.current_dir(dir.path()) + .arg("--non-interactive") + .arg("tag"); + cmd.assert().success(); + + let repo = Repository::open(dir.path()).unwrap(); + let mut tags = Vec::new(); + repo.tag_foreach(|_, name| { + tags.push(String::from_utf8_lossy(name).replace("refs/tags/", "")); + true + }) + .unwrap(); + assert_eq!(tags.len(), 1, "expected exactly one tag, got {tags:?}"); + let tag = &tags[0]; + + // The version file carries the tagged version, not the stale one. + assert_ne!(cargo_version(dir.path()), "0.0.1"); + assert_eq!(cargo_version(dir.path()), tag.trim_start_matches('v')); + + // And the bump is committed, so a build from the tag reports the new version. + assert!( + head_message(&repo).starts_with("chore: bump version to"), + "expected a bump commit at HEAD, got {:?}", + head_message(&repo) + ); + let tagged_commit = repo + .revparse_single(&format!("refs/tags/{tag}")) + .unwrap() + .peel_to_commit() + .unwrap(); + assert_eq!( + tagged_commit.id(), + repo.head().unwrap().peel_to_commit().unwrap().id(), + "the tag must point at the bump commit" + ); +} + +#[test] +fn no_bump_files_leaves_version_files_untouched() { + let dir = setup_cargo_repo(); + + let mut cmd = common::committy_cmd(); + cmd.current_dir(dir.path()) + .arg("--non-interactive") + .arg("tag") + .arg("--no-bump-files"); + cmd.assert().success(); + + assert_eq!(cargo_version(dir.path()), "0.0.1"); + + let repo = Repository::open(dir.path()).unwrap(); + assert_eq!(head_message(&repo), "feat: add the demo package"); +} + +#[test] +fn bump_files_flag_is_still_accepted() { + let dir = setup_cargo_repo(); + + let mut cmd = common::committy_cmd(); + cmd.current_dir(dir.path()) + .arg("--non-interactive") + .arg("tag") + .arg("--bump-files"); + cmd.assert().success(); + + assert_ne!(cargo_version(dir.path()), "0.0.1"); +} + +#[test] +fn conflicting_bump_flags_are_rejected() { + let dir = setup_cargo_repo(); + + let mut cmd = common::committy_cmd(); + cmd.current_dir(dir.path()) + .arg("--non-interactive") + .arg("tag") + .arg("--bump-files") + .arg("--no-bump-files"); + cmd.assert().failure().stderr(predicate::str::contains( + "Use either --bump-files or --no-bump-files, not both", + )); + + assert_eq!(cargo_version(dir.path()), "0.0.1"); +} + +/// Multi-package repositories used to stage the bumped version files without +/// committing them, so the tags landed on a commit that still carried the old +/// versions — the same defect that shipped a v1.10.0 tarball reporting 1.9.1. +#[test] +fn multi_package_bump_is_committed_before_tagging() { + let dir = tempdir().unwrap(); + let repo = Repository::init(dir.path()).unwrap(); + + let mut config = repo.config().unwrap(); + config.set_str("user.name", "Test User").unwrap(); + config.set_str("user.email", "test@example.com").unwrap(); + + fs::create_dir_all(dir.path().join("alpha")).unwrap(); + fs::write( + dir.path().join("alpha/Cargo.toml"), + "[package]\nname = \"alpha\"\nversion = \"0.0.1\"\n", + ) + .unwrap(); + fs::create_dir_all(dir.path().join(".committy")).unwrap(); + fs::write( + dir.path().join(".committy/config.toml"), + r#"[repository] +name = "workspace" +type = "multi-package" + +[versioning] +strategy = "independent" + +[[packages]] +name = "alpha" +type = "rust-cargo" +path = "alpha" +version_file = "Cargo.toml" +version_field = "package.version" +"#, + ) + .unwrap(); + + let signature = Signature::now("Test User", "test@example.com").unwrap(); + let mut index = repo.index().unwrap(); + index + .add_all(["."], git2::IndexAddOption::DEFAULT, None) + .unwrap(); + index.write().unwrap(); + let tree_id = index.write_tree().unwrap(); + let tree = repo.find_tree(tree_id).unwrap(); + repo.commit( + Some("HEAD"), + &signature, + &signature, + "feat(alpha): add the alpha package", + &tree, + &[], + ) + .unwrap(); + drop(tree); + + let mut cmd = common::committy_cmd(); + cmd.current_dir(dir.path()) + .arg("--non-interactive") + .arg("tag"); + cmd.assert().success(); + + let repo = Repository::open(dir.path()).unwrap(); + let head = repo.head().unwrap().peel_to_commit().unwrap(); + assert!( + head_message(&repo).starts_with("chore: bump version to alpha@"), + "expected a bump commit at HEAD, got {:?}", + head_message(&repo) + ); + + // The bumped file is in the tagged tree, not just in the working directory. + let mut tags = Vec::new(); + repo.tag_foreach(|_, name| { + tags.push(String::from_utf8_lossy(name).replace("refs/tags/", "")); + true + }) + .unwrap(); + assert_eq!(tags.len(), 1, "expected exactly one tag, got {tags:?}"); + let tagged_commit = repo + .revparse_single(&format!("refs/tags/{}", tags[0])) + .unwrap() + .peel_to_commit() + .unwrap(); + assert_eq!(tagged_commit.id(), head.id()); + + let version = tags[0].trim_start_matches("alpha-v").to_string(); + let blob = tagged_commit + .tree() + .unwrap() + .get_path(Path::new("alpha/Cargo.toml")) + .unwrap() + .to_object(&repo) + .unwrap(); + let content = String::from_utf8(blob.as_blob().unwrap().content().to_vec()).unwrap(); + assert!( + content.contains(&format!("version = \"{version}\"")), + "tagged tree still carries the old version: {content}" + ); +}