Skip to content

Commit

Permalink
Ensure you never drop the working commit with --skip-empty
Browse files Browse the repository at this point in the history
See jj-vcs#2766 for discussions
  • Loading branch information
matts1 committed Jan 3, 2024
1 parent b7f36e1 commit a1a001c
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 4 deletions.
6 changes: 5 additions & 1 deletion lib/src/rewrite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
25 changes: 22 additions & 3 deletions lib/tests/test_rewrite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1621,7 +1621,9 @@ fn test_rebase_commit_via_rebase_api() {

// Rebase B onto B2
//
// C
// D (empty) E (WC, empty)
// | /
// C---------
// |
// B B2
// |/
Expand All @@ -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,
};
Expand All @@ -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()}
);
}

0 comments on commit a1a001c

Please sign in to comment.