diff --git a/lib/src/rewrite.rs b/lib/src/rewrite.rs index dba86dea291..472dafa2d46 100644 --- a/lib/src/rewrite.rs +++ b/lib/src/rewrite.rs @@ -158,7 +158,11 @@ pub fn rebase_commit_with_options( } EmptyBehaviour::AbandonAllEmpty => *parent.tree_id() == new_tree_id, }; - if should_abandon { + // 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. diff --git a/lib/tests/test_rewrite.rs b/lib/tests/test_rewrite.rs index 255d7a48b07..ff8df025f56 100644 --- a/lib/tests/test_rewrite.rs +++ b/lib/tests/test_rewrite.rs @@ -1621,7 +1621,9 @@ fn test_rebase_commit_via_rebase_api() { // Rebase B onto B2 // - // C + // D (empty) E (WC, empty) + // | / + // C--------- // | // B B2 // |/ @@ -1636,12 +1638,26 @@ fn test_rebase_commit_via_rebase_api() { .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(); + tx.mut_repo() + .set_wc_commit(WorkspaceId::new("ws".to_string()), commit_e.id().clone()) + .unwrap(); + let rebase_options = RebaseOptions { empty: EmptyBehaviour::AbandonAllEmpty, }; @@ -1657,12 +1673,15 @@ fn test_rebase_commit_via_rebase_api() { .mut_repo() .rebase_descendants_with_options_return_map(&settings, rebase_options) .unwrap(); - assert_eq!(rebase_map.len(), 1); + 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_c.id().clone()} + hashset! {new_commit_e.id().clone()} ); }