split.rs: stop using DescendantRebaser::new

This requires creating a new public API as a substitute. I took the opportunity
to also add some comments to the
`MutRepo::record_rewritten_commit`/`record_abandoned_commit` functions.

I imade the simplest possible addition to the API; it is not a very elegant
one. Eventually, the entire `record_rewritten_commit` API should probably be
refactored again.

I also added some comments explaining what these functions do.

cli/src/commands/split.rs

@@ -15,8 +15,7 @@ use std::io::Write;
 
 use jj_lib::backend::ObjectId;
 use jj_lib::repo::Repo;
-use jj_lib::rewrite::{merge_commit_trees, DescendantRebaser};
-use maplit::{hashmap, hashset};
+use jj_lib::rewrite::merge_commit_trees;
 use tracing::instrument;
 
 use crate::cli_util::{CommandError, CommandHelper, RevisionArg};
@@ -135,14 +134,14 @@ don't make any changes, then the operation will be aborted.
         .generate_new_change_id()
         .set_description(second_description)
         .write()?;
-    let mut rebaser = DescendantRebaser::new(
-        command.settings(),
-        tx.mut_repo(),
-        hashmap! { commit.id().clone() => hashset!{second_commit.id().clone()} },
-        hashset! {},
-    );
-    rebaser.rebase_all()?;
-    let num_rebased = rebaser.rebased().len();
+
+    // Currently, `rebase_descendents` would treat `commit` as being rewritten to
+    // *both* `first_commit` and `second_commit`, as if it was becoming divergent.
+    // However, we want only the `second_commit` to inherit `commit`'s branches and
+    // descendants.
+    tx.mut_repo()
+        .set_rewritten_commit(commit.id().clone(), [second_commit.id().clone()]);
+    let num_rebased = tx.mut_repo().rebase_descendants(command.settings())?;
     if num_rebased > 0 {
         writeln!(ui.stderr(), "Rebased {num_rebased} descendant commits")?;
     }

lib/src/repo.rs

@@ -810,12 +810,43 @@ impl MutableRepo {
         Ok(commit)
     }
 
-    /// Record a commit as having been rewritten in this transaction. This
-    /// record is used by `rebase_descendants()`.
+    /// Record a commit as having been rewritten to one or more ids in this
+    /// transaction.
     ///
-    /// Rewritten commits don't have to be recorded here. This is just a
-    /// convenient place to record it. It won't matter after the transaction
-    /// has been committed.
+    /// This record is used by `rebase_descendants` to know which commits have
+    /// children that need to be rebased, and where to rebase them to. See the
+    /// docstring for `record_rewritten_commit` for details.
+    //  TODO(ilyagr): Make this API saner, e.g. make `self.rewritten_commits` public
+    // and make empty values correspond to abandoned commits.
+    pub fn set_rewritten_commit(
+        &mut self,
+        old_id: CommitId,
+        new_ids: impl IntoIterator<Item = CommitId>,
+    ) {
+        assert_ne!(old_id, *self.store().root_commit_id());
+        self.rewritten_commits
+            .insert(old_id, new_ids.into_iter().collect());
+    }
+
+    /// Record a commit as having been rewritten in this transaction. If it was
+    /// already rewritten, mark it as divergent (unlike `set_rewritten_commit`)
+    ///
+    /// This record is used by `rebase_descendants` to know which commits have
+    /// children that need to be rebased, and where to rebase the children (as
+    /// well as branches) to.
+    ///
+    /// The `rebase_descendants` logic treats these records as follows:
+    ///
+    /// - If a commit is recorded as rewritten to a single commit, its
+    ///   descendants would be rebased to become descendants of `new_id`. Any
+    ///   branches at `old_id` are also moved to `new_id`.
+    /// - If a commit is recorded as rewritten to more than one commit, it is
+    ///   assumed to have become divergent. Its descendants are *not* rebased.
+    ///   However, the *branches* that were at `old_id` are moved to each of the
+    ///   new ids, and thus become conflicted.
+    ///
+    /// In neither case would `rebase_descendants` modify the `old_id` commit
+    /// itself.
     pub fn record_rewritten_commit(&mut self, old_id: CommitId, new_id: CommitId) {
         assert_ne!(old_id, *self.store().root_commit_id());
         self.rewritten_commits
@@ -828,12 +859,15 @@ impl MutableRepo {
         self.rewritten_commits.clear();
     }
 
-    /// Record a commit as having been abandoned in this transaction. This
-    /// record is used by `rebase_descendants()`.
+    /// Record a commit as having been abandoned in this transaction.
     ///
-    /// Abandoned commits don't have to be recorded here. This is just a
-    /// convenient place to record it. It won't matter after the transaction
-    /// has been committed.
+    /// This record is used by `rebase_descendants` to know which commits have
+    /// children that need to be rebased, and where to rebase the children (as
+    /// well as branches) to.
+    ///
+    /// The `rebase_descendants` logic will rebase the descendants of `old_id`
+    /// to become the descendants of parent(s) of `old_id`. Any branches at
+    /// `old_id` would be moved to the parent(s) of `old_id` as well.
     pub fn record_abandoned_commit(&mut self, old_id: CommitId) {
         assert_ne!(old_id, *self.store().root_commit_id());
         self.abandoned_commits.insert(old_id);
@@ -877,6 +911,9 @@ impl MutableRepo {
         Ok(Some(rebaser))
     }
 
+    // TODO(ilyagr): Either document that this also moves branches (rename the
+    // function and the related functions?) or change things so that this only
+    // rebases descendants.
     pub fn rebase_descendants_with_options(
         &mut self,
         settings: &UserSettings,

lib/tests/test_rewrite.rs

@@ -1119,6 +1119,7 @@ fn test_rebase_descendants_update_branches_after_divergent_rewrite() {
     // B main         |/B2 main?
     // |         =>   |/
     // A              A
+    // TODO(ilyagr): Check what happens if B had a descendant with a branch on it.
     let mut tx = repo.start_transaction(&settings);
     let mut graph_builder = CommitGraphBuilder::new(&settings, tx.mut_repo());
     let commit_a = graph_builder.initial_commit();