Rewrite instead of abandoning empty commits.

cli/src/commands/rebase.rs

@@ -148,6 +148,7 @@ pub(crate) struct RebaseArgs {
     /// If true, when rebasing would produce an empty commit, the commit is
     /// skipped.
     /// Will never skip merge commits with multiple non-empty parents.
+    /// Will never skip the working commit.
     #[arg(long, conflicts_with = "revision")]
     skip_empty: bool,
 

lib/src/rewrite.rs

@@ -158,12 +158,16 @@ 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.
+        // If the user runs `jj checkout foo`, then `jj rebase -s foo -d bar`, and we
+        // drop the checked out empty commit, then the user will unknowingly
+        // have done the equivalent of `jj edit foo` instead of `jj checkout
+        // foo`. Thus, we never allow dropping the working commit. See #2766 and
+        // #2760 for discussions.
+        if should_abandon && !mut_repo.view().is_wc_commit_id(old_commit.id()) {
+            // 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());
         }
     }

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,87 @@ fn test_empty_commit_option(empty_behavior: EmptyBehaviour) {
         }
     );
 }
+
+#[test]
+fn test_rebase_abandoning_empty() {
+    let settings = testutils::user_settings();
+    let test_repo = TestRepo::init();
+    let repo = &test_repo.repo;
+
+    // Rebase B onto B2, where B2 and B have the same tree, abandoning all empty
+    // commits.
+    //
+    // We expect B and D to be skipped because they're empty, but not E because it's
+    // the working commit.
+    //
+    // D (empty)  E (WC, empty)       E' (WC, empty)
+    // |         /                    |
+    // C---------                     C'
+    // |                           => |
+    // B B2                           B2
+    // |/                             |
+    // A                              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_d = create_random_commit(tx.mut_repo(), &settings)
+        .set_parents(vec![commit_c.id().clone()])
+        .set_tree_id(commit_c.tree_id().clone())
+        .write()
+        .unwrap();
+    let commit_e = create_random_commit(tx.mut_repo(), &settings)
+        .set_parents(vec![commit_c.id().clone()])
+        .set_tree_id(commit_c.tree_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 workspace = WorkspaceId::new("ws".to_string());
+    tx.mut_repo()
+        .set_wc_commit(workspace.clone(), commit_e.id().clone())
+        .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(), 3);
+    let new_commit_c =
+        assert_rebased_onto(tx.mut_repo(), &rebase_map, &commit_c, &[&commit_b2.id()]);
+    assert_abandoned_with_parent(tx.mut_repo(), &rebase_map, &commit_d, new_commit_c.id());
+    let new_commit_e =
+        assert_rebased_onto(tx.mut_repo(), &rebase_map, &commit_e, &[&new_commit_c.id()]);
+
+    assert_eq!(
+        *tx.mut_repo().view().heads(),
+        hashset! {new_commit_e.id().clone()}
+    );
+
+    assert_eq!(
+        tx.mut_repo().view().get_wc_commit_id(&workspace),
+        Some(new_commit_e.id())
+    );
+}