[F] Rebase commit committer should be kept

src/git.rs

@@ -86,6 +86,12 @@ pub struct GitMirror {
     dry_run: bool,
 }
 
+struct CommitterIdentity {
+    name: String,
+    email: String,
+    date: String,
+}
+
 #[derive(Clone, Debug)]
 pub struct Redactor {
     secrets: Vec<String>,
@@ -582,6 +588,7 @@ impl GitMirror {
             return Ok(format!("dry-run-rebased-{}", short_sha(tip)));
         }
 
+        let commits = self.replay_commits(base, tip)?;
         let worktree = tempfile::TempDir::new().context("failed to create temporary worktree")?;
         let worktree_path = worktree.path().to_path_buf();
         self.run([
@@ -589,12 +596,13 @@ impl GitMirror {
             "add",
             "--detach",
             worktree_path.to_str().unwrap(),
-            tip,
+            onto,
         ])?;
 
-        let rebase_result = self.worktree_git(&worktree_path, ["rebase", "--onto", onto, base]);
+        let replay_result = self.replay_commits_preserving_committer(&worktree_path, &commits);
-        if let Err(error) = rebase_result {
+        if let Err(error) = replay_result {
-            let _ = self.worktree_git(&worktree_path, ["rebase", "--abort"]);
+            let _ = self.worktree_git(&worktree_path, ["cherry-pick", "--abort"]);
+            let _ = self.worktree_git(&worktree_path, ["reset", "--hard"]);
             let _ = self.run([
                 "worktree",
                 "remove",
@@ -613,6 +621,66 @@ impl GitMirror {
         Ok(rebased.trim().to_string())
     }
 
+    fn replay_commits(&self, base: &str, tip: &str) -> Result<Vec<String>> {
+        let range = format!("{base}..{tip}");
+        Ok(self
+            .output([
+                "rev-list",
+                "--reverse",
+                "--topo-order",
+                "--no-merges",
+                &range,
+            ])?
+            .lines()
+            .map(str::trim)
+            .filter(|line| !line.is_empty())
+            .map(ToOwned::to_owned)
+            .collect())
+    }
+
+    fn replay_commits_preserving_committer(
+        &self,
+        worktree: &Path,
+        commits: &[String],
+    ) -> Result<()> {
+        for commit in commits {
+            let committer = self.committer_identity(commit)?;
+            self.worktree_git(worktree, ["cherry-pick", "--no-commit", commit])?;
+            self.worktree_git_with_committer(
+                worktree,
+                ["commit", "--no-gpg-sign", "-C", commit],
+                &committer,
+            )?;
+        }
+        Ok(())
+    }
+
+    fn committer_identity(&self, commit: &str) -> Result<CommitterIdentity> {
+        let output = self.output(["show", "-s", "--format=%cn%x00%ce%x00%cI", commit])?;
+        let output = output.trim_end_matches('\n');
+        let mut parts = output.split('\0');
+        let name = parts
+            .next()
+            .filter(|value| !value.is_empty())
+            .ok_or_else(|| anyhow::anyhow!("commit {commit} has no committer name"))?;
+        let email = parts
+            .next()
+            .filter(|value| !value.is_empty())
+            .ok_or_else(|| anyhow::anyhow!("commit {commit} has no committer email"))?;
+        let date = parts
+            .next()
+            .filter(|value| !value.is_empty())
+            .ok_or_else(|| anyhow::anyhow!("commit {commit} has no committer date"))?;
+        if parts.next().is_some() {
+            bail!("commit {commit} has unexpected committer metadata");
+        }
+        Ok(CommitterIdentity {
+            name: name.to_string(),
+            email: email.to_string(),
+            date: date.to_string(),
+        })
+    }
+
     pub fn is_ancestor(&self, ancestor: &str, descendant: &str) -> Result<bool> {
         let status = self
             .command()
@@ -709,6 +777,35 @@ impl GitMirror {
             .into())
         }
     }
+
+    fn worktree_git_with_committer<const N: usize>(
+        &self,
+        worktree: &Path,
+        args: [&str; N],
+        committer: &CommitterIdentity,
+    ) -> Result<()> {
+        let output = Command::new("git")
+            .arg("-C")
+            .arg(worktree)
+            .args(args)
+            .env("GIT_COMMITTER_NAME", &committer.name)
+            .env("GIT_COMMITTER_EMAIL", &committer.email)
+            .env("GIT_COMMITTER_DATE", &committer.date)
+            .output()
+            .with_context(|| "failed to run git")?;
+        if output.status.success() {
+            Ok(())
+        } else {
+            Err(GitCommandError::new(
+                "git",
+                self.redactor
+                    .redact(&String::from_utf8_lossy(&output.stdout)),
+                self.redactor
+                    .redact(&String::from_utf8_lossy(&output.stderr)),
+            )
+            .into())
+        }
+    }
 }
 
 fn short_sha(sha: &str) -> &str {

tests/unit/git.rs

@@ -178,7 +178,15 @@ fn auto_rebase_branch_conflict_replays_later_tip_and_marks_force_targets() {
     let a_tip = fixture.commit_file("a", "a.txt", "a\n", 1_700_000_100);
     fixture.push_head(&fixture.remote_a, "main");
     fixture.reset_hard(&base);
-    let b_tip = fixture.commit_file("b", "b.txt", "b\n", 1_700_000_200);
+    let b_tip = fixture.commit_file_with_committer(
+        "b",
+        "b.txt",
+        "b\n",
+        1_700_000_200,
+        "Original Committer",
+        "original-committer@example.test",
+        1_700_000_250,
+    );
     fixture.push_head(&fixture.remote_b, "main");
 
     let mirror = fixture.mirror();
@@ -205,6 +213,10 @@ fn auto_rebase_branch_conflict_replays_later_tip_and_marks_force_targets() {
         .find(|update| update.target_remote == "b")
         .unwrap();
     assert!(b_update.force);
+    assert_eq!(
+        fixture.mirror_committer(&decision.sha),
+        fixture.mirror_committer(&b_tip)
+    );
 
     mirror
         .push_branch_updates(&fixture.remotes(), &decision.updates)
@@ -495,6 +507,35 @@ impl GitFixture {
         self.head()
     }
 
+    fn commit_file_with_committer(
+        &self,
+        message: &str,
+        file_name: &str,
+        contents: &str,
+        author_timestamp: i64,
+        committer_name: &str,
+        committer_email: &str,
+        committer_timestamp: i64,
+    ) -> String {
+        let path = self.work.join(file_name);
+        fs::write(path, contents).unwrap();
+        git(Some(&self.work), ["add", file_name]);
+
+        let author_date = format!("@{author_timestamp} +0000");
+        let committer_date = format!("@{committer_timestamp} +0000");
+        let output = Command::new("git")
+            .current_dir(&self.work)
+            .env("GIT_AUTHOR_DATE", &author_date)
+            .env("GIT_COMMITTER_NAME", committer_name)
+            .env("GIT_COMMITTER_EMAIL", committer_email)
+            .env("GIT_COMMITTER_DATE", &committer_date)
+            .args(["commit", "-m", message])
+            .output()
+            .unwrap();
+        assert_success(&output, "git commit");
+        self.head()
+    }
+
     fn head(&self) -> String {
         git_output(Some(&self.work), ["rev-parse", "HEAD"])
     }
@@ -559,6 +600,20 @@ impl GitFixture {
         .status
         .success()
     }
+
+    fn mirror_committer(&self, reference: &str) -> String {
+        git_output(
+            None,
+            [
+                "--git-dir",
+                self.mirror_path.to_str().unwrap(),
+                "show",
+                "-s",
+                "--format=%cn <%ce> %cI",
+                reference,
+            ],
+        )
+    }
 }
 
 fn git<const N: usize>(current_dir: Option<&Path>, args: [&str; N]) {