From a4aed2391f10dd48157f1fd44000db3ceaf9d513 Mon Sep 17 00:00:00 2001
From: Matt Stark <msta@google.com>
Date: Tue, 2 Jan 2024 11:20:52 +1100
Subject: [PATCH] Rewrite instead of abandoning empty commits.

Fixes #2760


Given the tree:
```
A-B-C
 \
  B2
```
And the command `jj rebase -s B -d B2`

We were previously marking B as abandoned, despite the comment stating that we were marking it as being succeeded by B2. This resulted in a call to `rewrite(rewrites={}, abandoned={B})` instead of `rewrite(rewrites={B=>B2}, abandoned={})`, which then made the new parent of `C` into `A` instead of `B2`
---
 lib/src/rewrite.rs        |  5 ++--
 lib/tests/test_rewrite.rs | 56 ++++++++++++++++++++++++++++++++++++++-
 2 files changed, 57 insertions(+), 4 deletions(-)

diff --git a/lib/src/rewrite.rs b/lib/src/rewrite.rs
index 0a786ac9e5..dba86dea29 100644
--- a/lib/src/rewrite.rs
+++ b/lib/src/rewrite.rs
@@ -159,11 +159,10 @@ pub fn rebase_commit_with_options(
             EmptyBehaviour::AbandonAllEmpty => *parent.tree_id() == new_tree_id,
         };
         if should_abandon {
-            mut_repo.record_abandoned_commit(old_commit.id().clone());
-            // Record old_commit as being succeeded by the parent for the purposes of
-            // the rebase.
+            // Record old_commit as being succeeded by the parent.
             // This ensures that when we stack commits, the second commit knows to
             // rebase on top of the parent commit, rather than the abandoned commit.
+            mut_repo.record_rewritten_commit(old_commit.id().clone(), parent.id().clone());
             return Ok(parent.clone());
         }
     }
diff --git a/lib/tests/test_rewrite.rs b/lib/tests/test_rewrite.rs
index 67882a1ba2..255d7a48b0 100644
--- a/lib/tests/test_rewrite.rs
+++ b/lib/tests/test_rewrite.rs
@@ -19,7 +19,7 @@ use jj_lib::merged_tree::MergedTree;
 use jj_lib::op_store::{RefTarget, RemoteRef, RemoteRefState, WorkspaceId};
 use jj_lib::repo::Repo;
 use jj_lib::repo_path::RepoPath;
-use jj_lib::rewrite::{restore_tree, EmptyBehaviour, RebaseOptions};
+use jj_lib::rewrite::{rebase_commit_with_options, restore_tree, EmptyBehaviour, RebaseOptions};
 use maplit::{hashmap, hashset};
 use test_case::test_case;
 use testutils::{
@@ -1612,3 +1612,57 @@ fn test_empty_commit_option(empty_behavior: EmptyBehaviour) {
         }
     );
 }
+
+#[test]
+fn test_rebase_commit_via_rebase_api() {
+    let settings = testutils::user_settings();
+    let test_repo = TestRepo::init();
+    let repo = &test_repo.repo;
+
+    // Rebase B onto B2
+    //
+    // C
+    // |
+    // B B2
+    // |/
+    // A
+    let mut tx = repo.start_transaction(&settings);
+    let commit_a = write_random_commit(tx.mut_repo(), &settings);
+    let commit_b = create_random_commit(tx.mut_repo(), &settings)
+        .set_parents(vec![commit_a.id().clone()])
+        .write()
+        .unwrap();
+    let commit_c = create_random_commit(tx.mut_repo(), &settings)
+        .set_parents(vec![commit_b.id().clone()])
+        .write()
+        .unwrap();
+    let commit_b2 = create_random_commit(tx.mut_repo(), &settings)
+        .set_parents(vec![commit_a.id().clone()])
+        .set_tree_id(commit_b.tree_id().clone())
+        .write()
+        .unwrap();
+
+    let rebase_options = RebaseOptions {
+        empty: EmptyBehaviour::AbandonAllEmpty,
+    };
+    rebase_commit_with_options(
+        &settings,
+        tx.mut_repo(),
+        &commit_b,
+        &[commit_b2.clone()],
+        &rebase_options,
+    )
+    .unwrap();
+    let rebase_map = tx
+        .mut_repo()
+        .rebase_descendants_with_options_return_map(&settings, rebase_options)
+        .unwrap();
+    assert_eq!(rebase_map.len(), 1);
+    let new_commit_c =
+        assert_rebased_onto(tx.mut_repo(), &rebase_map, &commit_c, &[&commit_b2.id()]);
+
+    assert_eq!(
+        *tx.mut_repo().view().heads(),
+        hashset! {new_commit_c.id().clone()}
+    );
+}