refactor(stack): extract shared git helpers into crate::git (#1555)

Every per-command stack module (drop, fixup, reword, reorder,
move, squash, checkout, sync, list, note, edit, setup, new, push)
maintained its own slight variant of `shell_quote`,
`resolve_repo_toplevel`, `spawn_rebase`, `run_git_capture`, and
`run_git_silent`. Three of the newer leaf modules
(`change_type`, `notes_push`, `replay`) additionally set
`LC_ALL=C/LANG=C/LANGUAGE=C` because they parse English git error
messages; the per-command duplicates didn't, which would have
been a flake waiting to happen on non-English systems.

Consolidate into `crates/mergify-stack/src/git.rs`:

- `git_cmd(repo_dir)` — base `Command` with `-C <dir>` (when
  supplied) and the forced C locale.
- `run_git_capture(repo_dir, args)` — trimmed stdout or
  `CliError::Generic` with the captured stderr.
- `run_git_silent(repo_dir, args)` — same error mapping,
  output discarded.
- `resolve_repo_toplevel(repo_dir)` — `git rev-parse
  --show-toplevel` as a `PathBuf`.
- `spawn_rebase(repo_dir, base, sequence_editor)` — interactive
  rebase with the optional `GIT_SEQUENCE_EDITOR` env (the
  `None` arm is what `stack edit`'s "fall through to the user's
  editor" path used).
- `shell_quote(value)` — POSIX shell single-quote escape.

Every consumer module updated to import from `crate::git`; the
duplicated function bodies (~50-100 lines per module) deleted.
Side effects:

- The previously locale-unaware per-command helpers now force
  `LC_ALL=C` like the leaf modules already did. Downstream call
  sites that match git error messages by English substring
  (`couldn't find remote ref`, etc.) now work consistently across
  locales — a latent bug fix.
- Call-site signature updates: `spawn_rebase` now takes
  `Option<&str>` for the editor (was `&str` in most consumers,
  `Option<&str>` in `edit`); `commands::sync`'s `run_git` /
  `spawn_rebase_with_editor` got renamed to the shared
  `run_git_silent` / `spawn_rebase`; `commands::push`'s
  `run_git_silent` signature changed from `&Path` to
  `Option<&Path>`.

Addresses Copilot's review feedback on PR #1518 (stack fixup)
about helper duplication across drop/edit/fixup/... — the
extraction unifies all 13 consumer modules in one commit.

All 241 mergify-stack tests + the rest of the workspace pass.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Depends-On: #1554

Author: jd

crates/mergify-stack/src/change_type.rs

@@ -22,7 +22,9 @@
 
 use std::io::Write;
 use std::path::Path;
-use std::process::{Command, Stdio};
+use std::process::Stdio;
+
+use crate::git::{git_cmd, run_git_capture, run_git_silent};
 
 use mergify_core::CliError;
 use serde::{Deserialize, Serialize};
@@ -169,50 +171,6 @@ pub fn fetch_old_pr_heads(
     run_git_silent(repo_dir, &args)
 }
 
-fn git_cmd(repo_dir: Option<&Path>) -> Command {
-    let mut cmd = Command::new("git");
-    if let Some(dir) = repo_dir {
-        cmd.arg("-C").arg(dir);
-    }
-    // Force C locale: callers may match git error messages by
-    // English substring, which breaks under translated locales.
-    // Mirrors `utils.subprocess_env` on the Python side.
-    cmd.env("LC_ALL", "C").env("LANG", "C").env("LANGUAGE", "C");
-    cmd
-}
-
-fn run_git_capture(repo_dir: Option<&Path>, args: &[&str]) -> Result<String, CliError> {
-    let output = git_cmd(repo_dir)
-        .args(args)
-        .output()
-        .map_err(|e| CliError::Generic(format!("failed to spawn `git {}`: {e}", args.join(" "))))?;
-    if !output.status.success() {
-        let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string();
-        return Err(CliError::Generic(if stderr.is_empty() {
-            format!("`git {}` failed", args.join(" "))
-        } else {
-            stderr
-        }));
-    }
-    String::from_utf8(output.stdout).map_err(|e| {
-        CliError::Generic(format!("`git {}` output is not UTF-8: {e}", args.join(" ")))
-    })
-}
-
-fn run_git_silent(repo_dir: Option<&Path>, args: &[&str]) -> Result<(), CliError> {
-    let status = git_cmd(repo_dir)
-        .args(args)
-        .status()
-        .map_err(|e| CliError::Generic(format!("failed to spawn `git {}`: {e}", args.join(" "))))?;
-    if !status.success() {
-        return Err(CliError::Generic(format!(
-            "`git {}` exited {status}",
-            args.join(" ")
-        )));
-    }
-    Ok(())
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;

crates/mergify-stack/src/commands/checkout.rs

@@ -19,8 +19,9 @@
 //!    head, `git checkout -b <local>` on it, set upstream
 //!    tracking to the root's base.
 
-use std::path::{Path, PathBuf};
-use std::process::Command;
+use std::path::Path;
+
+use crate::git::run_git_silent as run_git;
 
 use mergify_core::CliError;
 use mergify_core::HttpClient;
@@ -224,46 +225,6 @@ fn summary_from(pull: &Value) -> Result<PullSummary, CliError> {
     })
 }
 
-fn run_git(repo_dir: Option<&Path>, args: &[&str]) -> Result<(), CliError> {
-    let mut cmd = Command::new("git");
-    if let Some(dir) = repo_dir {
-        cmd.arg("-C").arg(dir);
-    }
-    cmd.args(args);
-    let status = cmd
-        .status()
-        .map_err(|e| CliError::Generic(format!("failed to spawn `git {}`: {e}", args.join(" "))))?;
-    if !status.success() {
-        return Err(CliError::Generic(format!(
-            "`git {}` exited {status}",
-            args.join(" ")
-        )));
-    }
-    Ok(())
-}
-
-// Unused but kept so future commands (sync, list) can reuse the
-// same toplevel-resolution.
-#[allow(dead_code)]
-fn resolve_repo_toplevel(repo_dir: Option<&Path>) -> Result<PathBuf, CliError> {
-    let mut cmd = Command::new("git");
-    if let Some(dir) = repo_dir {
-        cmd.arg("-C").arg(dir);
-    }
-    cmd.args(["rev-parse", "--show-toplevel"]);
-    let output = cmd
-        .output()
-        .map_err(|e| CliError::Generic(format!("spawn git rev-parse: {e}")))?;
-    if !output.status.success() {
-        return Err(CliError::Generic(
-            "git rev-parse --show-toplevel failed".into(),
-        ));
-    }
-    Ok(PathBuf::from(
-        String::from_utf8_lossy(&output.stdout).trim().to_string(),
-    ))
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;

crates/mergify-stack/src/commands/drop.rs

@@ -8,12 +8,12 @@
 //! <SHA,SHA,…>`, which deletes the targeted `pick` lines from the
 //! todo before git replays the rebase.
 
-use std::path::{Path, PathBuf};
-use std::process::Command;
+use std::path::Path;
 
 use mergify_core::CliError;
 
 use crate::change_id;
+use crate::git::{resolve_repo_toplevel, run_git_capture, shell_quote, spawn_rebase};
 use crate::local_commits::{self, LocalCommit};
 use crate::trunk;
 
@@ -91,7 +91,7 @@ pub fn run(opts: &Options<'_>) -> Result<Outcome, CliError> {
 
     let shas: Vec<String> = resolved.iter().map(|c| c.sha.clone()).collect();
     let editor = build_sequence_editor(opts.mergify_binary, &shas);
-    spawn_rebase(&repo_dir, &base, &editor)?;
+    spawn_rebase(&repo_dir, &base, Some(&editor))?;
     Ok(Outcome::Dropped { dropped: resolved })
 }
 
@@ -140,55 +140,6 @@ fn build_sequence_editor(binary: &Path, shas: &[String]) -> String {
     format!("{bin} _internal rebase-todo-rewrite --action drop --shas {shas}")
 }
 
-fn shell_quote(value: &str) -> String {
-    let escaped = value.replace('\'', "'\\''");
-    format!("'{escaped}'")
-}
-
-fn resolve_repo_toplevel(repo_dir: Option<&Path>) -> Result<PathBuf, CliError> {
-    let raw = run_git_capture(repo_dir, &["rev-parse", "--show-toplevel"])?;
-    Ok(PathBuf::from(raw))
-}
-
-fn spawn_rebase(repo_dir: &Path, base: &str, sequence_editor: &str) -> Result<(), CliError> {
-    let status = Command::new("git")
-        .arg("-C")
-        .arg(repo_dir)
-        .args(["rebase", "-i", base])
-        .env("GIT_SEQUENCE_EDITOR", sequence_editor)
-        .status()
-        .map_err(|e| CliError::Generic(format!("failed to spawn `git rebase -i`: {e}")))?;
-    if !status.success() {
-        return Err(CliError::Generic(format!(
-            "`git rebase -i {base}` exited {status}"
-        )));
-    }
-    Ok(())
-}
-
-fn run_git_capture(repo_dir: Option<&Path>, args: &[&str]) -> Result<String, CliError> {
-    let mut cmd = Command::new("git");
-    if let Some(dir) = repo_dir {
-        cmd.arg("-C").arg(dir);
-    }
-    cmd.args(args);
-    let output = cmd
-        .output()
-        .map_err(|e| CliError::Generic(format!("failed to spawn `git {}`: {e}", args.join(" "))))?;
-    if !output.status.success() {
-        let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string();
-        return Err(CliError::Generic(if stderr.is_empty() {
-            format!("`git {}` failed", args.join(" "))
-        } else {
-            stderr
-        }));
-    }
-    let stdout = String::from_utf8(output.stdout).map_err(|e| {
-        CliError::Generic(format!("`git {}` output is not UTF-8: {e}", args.join(" ")))
-    })?;
-    Ok(stdout.trim_end().to_string())
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;

crates/mergify-stack/src/commands/edit.rs

@@ -9,12 +9,12 @@
 //! binary's `_internal rebase-todo-rewrite` subcommand, set as
 //! `GIT_SEQUENCE_EDITOR` before spawning `git rebase -i <base>`.
 
-use std::path::{Path, PathBuf};
-use std::process::Command;
+use std::path::Path;
 
 use mergify_core::CliError;
 
 use crate::change_id;
+use crate::git::{resolve_repo_toplevel, run_git_capture, shell_quote, spawn_rebase};
 use crate::local_commits;
 use crate::trunk;
 
@@ -142,67 +142,6 @@ fn build_sequence_editor(binary: &Path, sha: &str) -> String {
     format!("{bin} _internal rebase-todo-rewrite --action edit --sha {sha}")
 }
 
-fn shell_quote(value: &str) -> String {
-    // Conservative single-quote escaping for sh: wrap in `'…'`,
-    // escape embedded `'` as `'\''`. Good enough for the binary
-    // path and a SHA — both are safe ASCII payloads in practice
-    // but we never want a path-with-spaces to break the rebase.
-    let escaped = value.replace('\'', "'\\''");
-    format!("'{escaped}'")
-}
-
-fn resolve_repo_toplevel(repo_dir: Option<&Path>) -> Result<PathBuf, CliError> {
-    let raw = run_git_capture(repo_dir, &["rev-parse", "--show-toplevel"])?;
-    Ok(PathBuf::from(raw))
-}
-
-fn spawn_rebase(
-    repo_dir: &Path,
-    base: &str,
-    sequence_editor: Option<&str>,
-) -> Result<(), CliError> {
-    let mut cmd = Command::new("git");
-    cmd.arg("-C").arg(repo_dir).args(["rebase", "-i", base]);
-    if let Some(editor) = sequence_editor {
-        cmd.env("GIT_SEQUENCE_EDITOR", editor);
-    }
-    let status = cmd
-        .status()
-        .map_err(|e| CliError::Generic(format!("failed to spawn `git rebase -i`: {e}")))?;
-    if !status.success() {
-        // The rebase itself printed whatever it had to print on
-        // stderr — surface a short generic error and let the
-        // user follow up with `git status` / `git rebase --abort`.
-        return Err(CliError::Generic(format!(
-            "`git rebase -i {base}` exited {status}"
-        )));
-    }
-    Ok(())
-}
-
-fn run_git_capture(repo_dir: Option<&Path>, args: &[&str]) -> Result<String, CliError> {
-    let mut cmd = Command::new("git");
-    if let Some(dir) = repo_dir {
-        cmd.arg("-C").arg(dir);
-    }
-    cmd.args(args);
-    let output = cmd
-        .output()
-        .map_err(|e| CliError::Generic(format!("failed to spawn `git {}`: {e}", args.join(" "))))?;
-    if !output.status.success() {
-        let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string();
-        return Err(CliError::Generic(if stderr.is_empty() {
-            format!("`git {}` failed", args.join(" "))
-        } else {
-            stderr
-        }));
-    }
-    let stdout = String::from_utf8(output.stdout).map_err(|e| {
-        CliError::Generic(format!("`git {}` output is not UTF-8: {e}", args.join(" ")))
-    })?;
-    Ok(stdout.trim_end().to_string())
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;

crates/mergify-stack/src/commands/fixup.rs

@@ -13,12 +13,12 @@
 //! collapse it into the trunk's parent. Python rejects this with
 //! `INVALID_STATE`; we mirror the check.
 
-use std::path::{Path, PathBuf};
-use std::process::Command;
+use std::path::Path;
 
 use mergify_core::CliError;
 
 use crate::change_id;
+use crate::git::{resolve_repo_toplevel, run_git_capture, shell_quote, spawn_rebase};
 use crate::local_commits::{self, LocalCommit};
 use crate::trunk;
 
@@ -95,7 +95,7 @@ pub fn run(opts: &Options<'_>) -> Result<Outcome, CliError> {
 
     let shas: Vec<String> = resolved.iter().map(|c| c.sha.clone()).collect();
     let editor = build_sequence_editor(opts.mergify_binary, &shas);
-    spawn_rebase(&repo_dir, &base, &editor)?;
+    spawn_rebase(&repo_dir, &base, Some(&editor))?;
     Ok(Outcome::Squashed { fixed_up: resolved })
 }
 
@@ -144,55 +144,6 @@ fn build_sequence_editor(binary: &Path, shas: &[String]) -> String {
     format!("{bin} _internal rebase-todo-rewrite --action fixup --shas {shas}")
 }
 
-fn shell_quote(value: &str) -> String {
-    let escaped = value.replace('\'', "'\\''");
-    format!("'{escaped}'")
-}
-
-fn resolve_repo_toplevel(repo_dir: Option<&Path>) -> Result<PathBuf, CliError> {
-    let raw = run_git_capture(repo_dir, &["rev-parse", "--show-toplevel"])?;
-    Ok(PathBuf::from(raw))
-}
-
-fn spawn_rebase(repo_dir: &Path, base: &str, sequence_editor: &str) -> Result<(), CliError> {
-    let status = Command::new("git")
-        .arg("-C")
-        .arg(repo_dir)
-        .args(["rebase", "-i", base])
-        .env("GIT_SEQUENCE_EDITOR", sequence_editor)
-        .status()
-        .map_err(|e| CliError::Generic(format!("failed to spawn `git rebase -i`: {e}")))?;
-    if !status.success() {
-        return Err(CliError::Generic(format!(
-            "`git rebase -i {base}` exited {status}"
-        )));
-    }
-    Ok(())
-}
-
-fn run_git_capture(repo_dir: Option<&Path>, args: &[&str]) -> Result<String, CliError> {
-    let mut cmd = Command::new("git");
-    if let Some(dir) = repo_dir {
-        cmd.arg("-C").arg(dir);
-    }
-    cmd.args(args);
-    let output = cmd
-        .output()
-        .map_err(|e| CliError::Generic(format!("failed to spawn `git {}`: {e}", args.join(" "))))?;
-    if !output.status.success() {
-        let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string();
-        return Err(CliError::Generic(if stderr.is_empty() {
-            format!("`git {}` failed", args.join(" "))
-        } else {
-            stderr
-        }));
-    }
-    let stdout = String::from_utf8(output.stdout).map_err(|e| {
-        CliError::Generic(format!("`git {}` output is not UTF-8: {e}", args.join(" ")))
-    })?;
-    Ok(stdout.trim_end().to_string())
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;