From bb6b551a4c7afb0df62dc8a373d37ce9dbb144c5 Mon Sep 17 00:00:00 2001
From: Yuya Nishihara <yuya@tcha.org>
Date: Fri, 20 Dec 2024 12:15:01 +0900
Subject: [PATCH] commit_builder: add is_empty(), is_discardable(), and
 .abandon()

It's convenient to test if rewriter.reparent()-ed commit is empty than
implementing the same check based on rewriter.new_parents(). CommitRewriter
is an intermediate state, and it doesn't know whether the commit will be rebased
or reparented.
---
 lib/src/commit.rs         | 24 ++++++++++++++++------
 lib/src/commit_builder.rs | 43 +++++++++++++++++++++++++++++++++++++++
 lib/src/rewrite.rs        |  2 ++
 3 files changed, 63 insertions(+), 6 deletions(-)

diff --git a/lib/src/commit.rs b/lib/src/commit.rs
index 9d5f210b3c..f616e8e4c0 100644
--- a/lib/src/commit.rs
+++ b/lib/src/commit.rs
@@ -126,12 +126,7 @@ impl Commit {
     /// Returns whether commit's content is empty. Commit description is not
     /// taken into consideration.
     pub fn is_empty(&self, repo: &dyn Repo) -> BackendResult<bool> {
-        let parents: Vec<_> = self.parents().try_collect()?;
-        if let [parent] = &parents[..] {
-            return Ok(parent.tree_id() == self.tree_id());
-        }
-        let parent_tree = merge_commit_trees(repo, &parents)?;
-        Ok(*self.tree_id() == parent_tree.id())
+        is_backend_commit_empty(repo, &self.store, &self.data)
     }
 
     pub fn has_conflict(&self) -> BackendResult<bool> {
@@ -183,6 +178,23 @@ impl Commit {
     }
 }
 
+pub(crate) fn is_backend_commit_empty(
+    repo: &dyn Repo,
+    store: &Arc<Store>,
+    commit: &backend::Commit,
+) -> BackendResult<bool> {
+    if let [parent_id] = &*commit.parents {
+        return Ok(commit.root_tree == *store.get_commit(parent_id)?.tree_id());
+    }
+    let parents: Vec<_> = commit
+        .parents
+        .iter()
+        .map(|id| store.get_commit(id))
+        .try_collect()?;
+    let parent_tree = merge_commit_trees(repo, &parents)?;
+    Ok(commit.root_tree == parent_tree.id())
+}
+
 pub trait CommitIteratorExt<'c, I> {
     fn ids(self) -> impl Iterator<Item = &'c CommitId>;
 }
diff --git a/lib/src/commit_builder.rs b/lib/src/commit_builder.rs
index 1884f394a5..fb0723bf72 100644
--- a/lib/src/commit_builder.rs
+++ b/lib/src/commit_builder.rs
@@ -24,6 +24,7 @@ use crate::backend::ChangeId;
 use crate::backend::CommitId;
 use crate::backend::MergedTreeId;
 use crate::backend::Signature;
+use crate::commit::is_backend_commit_empty;
 use crate::commit::Commit;
 use crate::repo::MutableRepo;
 use crate::repo::Repo;
@@ -73,6 +74,11 @@ impl CommitBuilder<'_> {
         self
     }
 
+    /// [`Commit::is_empty()`] for the new commit.
+    pub fn is_empty(&self) -> BackendResult<bool> {
+        self.inner.is_empty(self.mut_repo)
+    }
+
     pub fn change_id(&self) -> &ChangeId {
         self.inner.change_id()
     }
@@ -114,6 +120,11 @@ impl CommitBuilder<'_> {
         self
     }
 
+    /// [`Commit::is_discardable()`] for the new commit.
+    pub fn is_discardable(&self) -> BackendResult<bool> {
+        self.inner.is_discardable(self.mut_repo)
+    }
+
     pub fn sign_settings(&self) -> &SignSettings {
         self.inner.sign_settings()
     }
@@ -131,6 +142,12 @@ impl CommitBuilder<'_> {
     pub fn write(self) -> BackendResult<Commit> {
         self.inner.write(self.mut_repo)
     }
+
+    /// Records the old commit as abandoned instead of writing new commit. This
+    /// is noop for the builder created by [`MutableRepo::new_commit()`].
+    pub fn abandon(self) {
+        self.inner.abandon(self.mut_repo);
+    }
 }
 
 /// Like `CommitBuilder`, but doesn't mutably borrow `MutableRepo`.
@@ -254,6 +271,11 @@ impl DetachedCommitBuilder {
         self
     }
 
+    /// [`Commit::is_empty()`] for the new commit.
+    pub fn is_empty(&self, repo: &dyn Repo) -> BackendResult<bool> {
+        is_backend_commit_empty(repo, &self.store, &self.commit)
+    }
+
     pub fn change_id(&self) -> &ChangeId {
         &self.commit.change_id
     }
@@ -295,6 +317,11 @@ impl DetachedCommitBuilder {
         self
     }
 
+    /// [`Commit::is_discardable()`] for the new commit.
+    pub fn is_discardable(&self, repo: &dyn Repo) -> BackendResult<bool> {
+        Ok(self.description().is_empty() && self.is_empty(repo)?)
+    }
+
     pub fn sign_settings(&self) -> &SignSettings {
         &self.sign_settings
     }
@@ -328,6 +355,22 @@ impl DetachedCommitBuilder {
     pub fn write_hidden(&self) -> BackendResult<Commit> {
         write_to_store(&self.store, self.commit.clone(), &self.sign_settings)
     }
+
+    /// Records the old commit as abandoned in the `mut_repo`.
+    ///
+    /// This is noop if there's no old commit that would be rewritten to the new
+    /// commit by `write()`.
+    pub fn abandon(self, mut_repo: &mut MutableRepo) {
+        let commit = self.commit;
+        if let Some(rewrite_source) = &self.rewrite_source {
+            if rewrite_source.change_id() == &commit.change_id {
+                mut_repo.record_abandoned_commit_with_parents(
+                    rewrite_source.id().clone(),
+                    commit.parents,
+                );
+            }
+        }
+    }
 }
 
 fn write_to_store(
diff --git a/lib/src/rewrite.rs b/lib/src/rewrite.rs
index bf26f787b8..47b8091f9c 100644
--- a/lib/src/rewrite.rs
+++ b/lib/src/rewrite.rs
@@ -210,6 +210,8 @@ impl<'repo> CommitRewriter<'repo> {
     }
 
     /// Records the old commit as abandoned with the new parents.
+    ///
+    /// This is equivalent to `reparent(settings).abandon()`, but is cheaper.
     pub fn abandon(self) {
         let old_commit_id = self.old_commit.id().clone();
         let new_parents = self.new_parents;