Skip to content

Commit

Permalink
rewrite: allow working-copy to be abandoned
Browse files Browse the repository at this point in the history
This removes the special handling of the working-copy commit. By
recording when an empty/emptied commit was abanoned, we rebase
descendants correctly and create a new empty working-copy commit on
top.
martinvonz committed Feb 26, 2024
1 parent b695fa1 commit 1cbf2b4
Showing 2 changed files with 46 additions and 36 deletions.
40 changes: 27 additions & 13 deletions lib/src/rewrite.rs
Original file line number Diff line number Diff line change
@@ -105,13 +105,22 @@ pub fn rebase_commit(
old_commit: &Commit,
new_parents: &[Commit],
) -> Result<Commit, TreeMergeError> {
rebase_commit_with_options(
let rebased_commit = rebase_commit_with_options(
settings,
mut_repo,
old_commit,
new_parents,
&Default::default(),
)
)?;
match rebased_commit {
RebasedCommit::Rewritten(new_commit) => Ok(new_commit),
RebasedCommit::Abandoned { parent: _ } => panic!("Commit was unexpectedly abandoned"),
}
}

pub enum RebasedCommit {
Rewritten(Commit),
Abandoned { parent: Commit },
}

pub fn rebase_commit_with_options(
@@ -120,7 +129,7 @@ pub fn rebase_commit_with_options(
old_commit: &Commit,
new_parents: &[Commit],
options: &RebaseOptions,
) -> Result<Commit, TreeMergeError> {
) -> Result<RebasedCommit, TreeMergeError> {
let old_parents = old_commit.parents();
let old_parent_trees = old_parents
.iter()
@@ -159,28 +168,26 @@ pub fn rebase_commit_with_options(
}
EmptyBehaviour::AbandonAllEmpty => *parent.tree_id() == new_tree_id,
};
// 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()) {
if should_abandon {
// 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());
return Ok(RebasedCommit::Abandoned {
parent: parent.clone(),
});
}
}
let new_parent_ids = new_parents
.iter()
.map(|commit| commit.id().clone())
.collect();
Ok(mut_repo
let new_commit = mut_repo
.rewrite_commit(settings, old_commit)
.set_parents(new_parent_ids)
.set_tree_id(new_tree_id)
.write()?)
.write()?;
Ok(RebasedCommit::Rewritten(new_commit))
}

pub fn rebase_to_dest_parent(
@@ -569,13 +576,20 @@ impl<'settings, 'repo> DescendantRebaser<'settings, 'repo> {
.iter()
.map(|new_parent_id| self.mut_repo.store().get_commit(new_parent_id))
.try_collect()?;
let new_commit = rebase_commit_with_options(
let rebased_commit: RebasedCommit = rebase_commit_with_options(
self.settings,
self.mut_repo,
&old_commit,
&new_parents,
&self.options,
)?;
let new_commit = match rebased_commit {
RebasedCommit::Rewritten(new_commit) => new_commit,
RebasedCommit::Abandoned { parent } => {
self.abandoned.insert(old_commit.id().clone());
parent
}
};
let previous_rebased_value = self
.rebased
.insert(old_commit_id.clone(), new_commit.id().clone());
42 changes: 19 additions & 23 deletions lib/tests/test_rewrite.rs
Original file line number Diff line number Diff line change
@@ -1663,24 +1663,17 @@ fn test_rebase_abandoning_empty() {
// Rebase B onto B2, where B2 and B have the same tree, abandoning all empty
// commits.
//
// We expect B, D, and G to be skipped because they're empty, but not E because
// it's the working commit. F also remains as it's not empty.
//
// F G (empty) F'
// |/ |
// E (WC, empty) D (empty) E' (WC, empty)
// | / |
// We expect B, D, E, and G to be skipped because they're empty. F remains
// as it's not empty.
// F G (empty)
// |/
// E (WC, empty) D (empty) F' E' (WC, empty)
// | / |/
// C------------- C'
// | => |
// B B2 B2
// |/ |
// A A
//
// TODO(#2869): There is a minor bug here. We'd like the result to be
// equivalent to rebasing and then `jj abandon`-ing all the empty commits.
// In case of the working copy, this would mean that F' would be a direct
// child of C', and the working copy would be a new commit that's also a
// direct child of C'.

let mut tx = repo.start_transaction(&settings);
let commit_a = write_random_commit(tx.mut_repo(), &settings);
@@ -1742,19 +1735,22 @@ fn test_rebase_abandoning_empty() {
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_abandoned_with_parent(tx.mut_repo(), &rebase_map, &commit_e, new_commit_c.id());
let new_commit_f =
assert_rebased_onto(tx.mut_repo(), &rebase_map, &commit_f, &[new_commit_e.id()]);
assert_abandoned_with_parent(tx.mut_repo(), &rebase_map, &commit_g, new_commit_e.id());
assert_rebased_onto(tx.mut_repo(), &rebase_map, &commit_f, &[new_commit_c.id()]);
assert_abandoned_with_parent(tx.mut_repo(), &rebase_map, &commit_g, new_commit_c.id());

assert_eq!(
*tx.mut_repo().view().heads(),
hashset! {new_commit_f.id().clone()}
);
let new_wc_commit_id = tx
.mut_repo()
.view()
.get_wc_commit_id(&workspace)
.unwrap()
.clone();
let new_wc_commit = tx.mut_repo().store().get_commit(&new_wc_commit_id).unwrap();
assert_eq!(new_wc_commit.parent_ids(), &[new_commit_c.id().clone()]);

assert_eq!(
tx.mut_repo().view().get_wc_commit_id(&workspace),
Some(new_commit_e.id())
*tx.mut_repo().view().heads(),
hashset! {new_commit_f.id().clone(), new_wc_commit_id.clone()}
);
}

0 comments on commit 1cbf2b4

Please sign in to comment.