Merge pull request #92 from GitoxideLabs/copilot/check-changelog-untracked

Add untracked CHANGELOG.md files to git before commit

Author: Byron

src/command/release/git.rs

@@ -1,4 +1,4 @@
-use std::{convert::TryInto, process::Command};
+use std::{convert::TryInto, path::Path, process::Command};
 
 use anyhow::{anyhow, bail, Context};
 use cargo_metadata::Package;
@@ -7,15 +7,56 @@ use gix::{bstr::ByteSlice, refs, refs::transaction::PreviousValue, Id};
 use super::{tag_name, Options};
 use crate::utils::will;
 
-pub(in crate::command::release_impl) fn commit_changes(
+pub(in crate::command::release_impl) fn commit_changes<'a>(
     message: impl AsRef<str>,
     dry_run: bool,
     empty_commit_possible: bool,
     signoff: bool,
-    ctx: &crate::Context,
-) -> anyhow::Result<Option<Id<'_>>> {
+    changelog_paths: &[impl AsRef<Path>],
+    ctx: &'a crate::Context,
+) -> anyhow::Result<Option<Id<'a>>> {
     // TODO: replace with gitoxide one day
-    let mut cmd = Command::new("git");
+    // Add changelog files that are not yet tracked in git index.
+    // `git commit -am` only stages tracked files, so we need to explicitly add new ones.
+    if !changelog_paths.is_empty() {
+        let index = ctx.repo.open_index()?;
+        let workdir = ctx.repo.workdir().context("Can only work in non-bare repositories")?;
+
+        let untracked_paths: Vec<_> = changelog_paths
+            .iter()
+            .filter_map(|path| {
+                // Convert absolute path to worktree-relative path with forward slashes
+                path.as_ref().strip_prefix(workdir).ok().and_then(|relative_path| {
+                    let relative_path_unix = gix::path::to_unix_separators(gix::path::into_bstr(relative_path));
+                    // Check if the path is NOT tracked in the git index
+                    index
+                        .entry_by_path(relative_path_unix.as_bstr())
+                        .is_none()
+                        .then_some(relative_path)
+                })
+            })
+            .collect();
+
+        if !untracked_paths.is_empty() {
+            let mut git_add = Command::new(gix::path::env::exe_invocation());
+            git_add.args(["add", "--"]);
+            for path in &untracked_paths {
+                git_add.arg(path);
+            }
+            log::trace!("{} run {:?}", will(dry_run), git_add);
+            let output = git_add.output()?;
+            if !dry_run && !output.status.success() {
+                let paths: Vec<_> = untracked_paths.iter().map(|p| p.to_string_lossy()).collect();
+                bail!(
+                    "Failed to add new changelog files to git: {}: {err}",
+                    paths.join(", "),
+                    err = output.stderr.to_str_lossy()
+                );
+            }
+        }
+    }
+
+    let mut cmd = Command::new(gix::path::env::exe_invocation());
     cmd.arg("commit").arg("-am").arg(message.as_ref());
     if empty_commit_possible {
         cmd.arg("--allow-empty");
@@ -96,7 +137,7 @@ pub fn push_tags_and_head(
         return Ok(());
     }
 
-    let mut cmd = Command::new("git");
+    let mut cmd = Command::new(gix::path::env::exe_invocation());
     cmd.arg("push")
         .arg({
             let remote = repo
@@ -139,8 +180,9 @@ mod tests {
         )
         .unwrap();
         let message = "commit message";
+        let empty: &[&std::path::Path] = &[];
         testing_logger::setup();
-        let _ = commit_changes(message, true, false, false, &ctx).unwrap();
+        let _ = commit_changes(message, true, false, false, empty, &ctx).unwrap();
         testing_logger::validate(|captured_logs| {
             assert_eq!(captured_logs.len(), 1);
             assert_eq!(
@@ -162,8 +204,9 @@ mod tests {
         )
         .unwrap();
         let message = "commit message";
+        let empty: &[&std::path::Path] = &[];
         testing_logger::setup();
-        let _ = commit_changes(message, true, false, true, &ctx).unwrap();
+        let _ = commit_changes(message, true, false, true, empty, &ctx).unwrap();
         testing_logger::validate(|captured_logs| {
             assert_eq!(captured_logs.len(), 1);
             assert_eq!(

src/command/release/manifest.rs

@@ -94,6 +94,12 @@ pub(in crate::command::release_impl) fn edit_version_and_fixup_dependent_crates_
 
     preview_changelogs(ctx, &pending_changelogs, opts.clone())?;
 
+    // Collect all changelog paths before they are consumed by commit_locks_and_generate_bail_message
+    let changelog_paths: Vec<std::path::PathBuf> = pending_changelogs
+        .iter()
+        .map(|(_, _, lock)| lock.resource_path().to_owned())
+        .collect();
+
     let bail_message = commit_locks_and_generate_bail_message(
         ctx,
         pending_changelogs,
@@ -103,7 +109,14 @@ pub(in crate::command::release_impl) fn edit_version_and_fixup_dependent_crates_
         opts.clone(),
     )?;
 
-    let res = git::commit_changes(commit_message, dry_run, !made_change, opts.signoff, &ctx.base)?;
+    let res = git::commit_changes(
+        commit_message,
+        dry_run,
+        !made_change,
+        opts.signoff,
+        &changelog_paths,
+        &ctx.base,
+    )?;
     if let Some(bail_message) = bail_message {
         bail!(bail_message);
     } else {
@@ -128,7 +141,7 @@ fn commit_locks_and_generate_bail_message(
         ..
     }: Options,
 ) -> anyhow::Result<Option<String>> {
-    let bail_message_after_commit = if !dry_run {
+    let bail_message = if !dry_run {
         let mut packages_whose_changelogs_need_edits = None;
         let mut packages_which_might_be_fully_generated = None;
         for (idx, (package, _, lock)) in pending_changelogs.into_iter().enumerate() {
@@ -253,7 +266,7 @@ fn commit_locks_and_generate_bail_message(
         }
         None
     };
-    Ok(bail_message_after_commit)
+    Ok(bail_message)
 }
 
 fn preview_changelogs(

src/git/mod.rs

@@ -71,7 +71,7 @@ pub fn change_since_last_release(package: &Package, ctx: &crate::Context) -> any
 }
 
 pub fn assure_clean_working_tree() -> anyhow::Result<()> {
-    let tracked_changed = !Command::new("git")
+    let tracked_changed = !Command::new(gix::path::env::exe_invocation())
         .arg("diff")
         .arg("HEAD")
         .arg("--exit-code")
@@ -82,7 +82,7 @@ pub fn assure_clean_working_tree() -> anyhow::Result<()> {
         bail!("Detected working tree changes. Please commit beforehand as otherwise these would be committed as part of manifest changes, or use --allow-dirty to force it.")
     }
 
-    let untracked = Command::new("git")
+    let untracked = Command::new(gix::path::env::exe_invocation())
         .arg("ls-files")
         .arg("--exclude-standard")
         .arg("--others")
@@ -104,7 +104,11 @@ pub fn remote_url(repo: &gix::Repository) -> anyhow::Result<Option<gix::Url>> {
 }
 
 pub fn author() -> anyhow::Result<gix::actor::Signature> {
-    let stdout = Command::new("git").arg("var").arg("GIT_AUTHOR_IDENT").output()?.stdout;
+    let stdout = Command::new(gix::path::env::exe_invocation())
+        .arg("var")
+        .arg("GIT_AUTHOR_IDENT")
+        .output()?
+        .stdout;
     Ok(gix::actor::SignatureRef::from_bytes::<()>(&stdout)
         .ok()
         .ok_or_else(|| anyhow!("Could not parse author from GIT_AUTHOR_IDENT='{}'", stdout.as_bstr()))?

tests/snapshots/triple-depth-workspace/a-dry-run-success-multi-crate-auto-bump-breaking-change-dependant-publish

@@ -15,6 +15,7 @@
 [INFO ] Up to 3 changelogs would be previewed if the --execute is set and --no-changelog-preview is unset.
 [WARN ] WOULD ask for review after commit as the changelog entry is empty for crates: b, c
 [WARN ] To fix the changelog manually, run: cargo changelog --write c a
+[TRACE] WOULD run "git" "add" "--" "c/CHANGELOG.md"
 [TRACE] WOULD run "git" "commit" "-am" "Bump a v0.9.0, b v0.9.0, c v9.0.0, safety bump 2 crates\n\nSAFETY BUMP: b v0.9.0, c v9.0.0"
 [TRACE] WOULD create tag object a-v0.9.0 with changelog message, first line is: '### Refactor (BREAKING)'
 [TRACE] WOULD create tag object b-v0.9.0 with changelog message, first line is: '### Commit Statistics'

tests/snapshots/triple-depth-workspace/c-dry-run-success-multi-crate-auto-bump-breaking-change

@@ -10,6 +10,7 @@
 [INFO ] Up to 3 changelogs would be previewed if the --execute is set and --no-changelog-preview is unset.
 [WARN ] WOULD ask for review after commit as the changelog entry is empty for crates: a, b
 [WARN ] To fix the changelog manually, run: cargo changelog --write c a
+[TRACE] WOULD run "git" "add" "--" "c/CHANGELOG.md"
 [TRACE] WOULD run "git" "commit" "-am" "Adjusting changelogs prior to release of a v0.8.0, b v0.8.0, c v8.0.0"
 [TRACE] WOULD create tag object a-v0.8.0 with changelog message, first line is: '### Commit Statistics'
 [TRACE] WOULD create tag object b-v0.8.0 with changelog message, first line is: '### Commit Statistics'

tests/snapshots/triple-depth-workspace/c-dry-run-success-multi-crate-auto-bump-minor-change

@@ -10,6 +10,7 @@
 [INFO ] Up to 3 changelogs would be previewed if the --execute is set and --no-changelog-preview is unset.
 [WARN ] WOULD ask for review after commit as the changelog entry is empty for crates: a, b
 [WARN ] To fix the changelog manually, run: cargo changelog --write c a
+[TRACE] WOULD run "git" "add" "--" "c/CHANGELOG.md"
 [TRACE] WOULD run "git" "commit" "-am" "Adjusting changelogs prior to release of a v0.8.0, b v0.8.0, c v8.0.0"
 [TRACE] WOULD create tag object a-v0.8.0 with changelog message, first line is: '### Commit Statistics'
 [TRACE] WOULD create tag object b-v0.8.0 with changelog message, first line is: '### Commit Statistics'

tests/snapshots/triple-depth-workspace/c-dry-run-success-multi-crate-auto-bump-no-change

@@ -10,6 +10,7 @@
 [INFO ] Up to 3 changelogs would be previewed if the --execute is set and --no-changelog-preview is unset.
 [WARN ] WOULD ask for review after commit as the changelog entry is empty for crates: a, b, c
 [WARN ] To fix the changelog manually, run: cargo changelog --write c a
+[TRACE] WOULD run "git" "add" "--" "c/CHANGELOG.md"
 [TRACE] WOULD run "git" "commit" "-am" "Adjusting changelogs prior to release of a v0.8.0, b v0.8.0, c v8.0.0"
 [TRACE] WOULD create tag object a-v0.8.0 with changelog message, first line is: '### Commit Statistics'
 [TRACE] WOULD create tag object b-v0.8.0 with changelog message, first line is: '### Commit Statistics'