rewrite: make rebase_commit_with_options() mark abandoned commit

When `rebase_commit_with_options()` decides to abandons a commit, it
records the new parents in the `MutableRepo`, but it's currently the
caller's responsibility to remember to mark it as abandoned. Let's
move that logic into the function to reduce the risk of future bugs.
This commit is contained in:
Martin von Zweigbergk 2024-03-24 14:52:09 -07:00 committed by Martin von Zweigbergk
parent 3ddf9f4329
commit cfdb341c6b
2 changed files with 22 additions and 14 deletions

View file

@ -860,12 +860,26 @@ impl MutableRepo {
// TODO: Propagate errors from commit lookup or take a Commit as argument.
pub fn record_abandoned_commit(&mut self, old_id: CommitId) {
assert_ne!(old_id, *self.store().root_commit_id());
self.divergent.remove(&old_id);
self.abandoned.insert(old_id.clone());
// Descendants should be rebased onto the commit's parents
let old_commit = self.store().get_commit(&old_id).unwrap();
self.record_abandoned_commit_with_parents(old_id, old_commit.parent_ids().to_vec());
}
/// Record a commit as having been abandoned in this transaction.
///
/// A later `rebase_descendants()` will rebase children of `old_id` onto
/// `new_parent_ids`. A working copy pointing to `old_id` will point to a
/// new commit on top of `new_parent_ids`.
pub fn record_abandoned_commit_with_parents(
&mut self,
old_id: CommitId,
new_parent_ids: impl IntoIterator<Item = CommitId>,
) {
self.divergent.remove(&old_id);
assert_ne!(old_id, *self.store().root_commit_id());
self.abandoned.insert(old_id.clone());
self.parent_mapping
.insert(old_id, old_commit.parent_ids().to_vec());
.insert(old_id, new_parent_ids.into_iter().collect());
}
fn clear_descendant_rebaser_plans(&mut self) {

View file

@ -188,10 +188,10 @@ pub fn rebase_commit_with_options(
EmptyBehaviour::AbandonAllEmpty => *parent.tree_id() == new_tree_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.set_rewritten_commit(old_commit.id().clone(), parent.id().clone());
mut_repo.record_abandoned_commit_with_parents(
old_commit.id().clone(),
std::iter::once(parent.id().clone()),
);
return Ok(RebasedCommit::Abandoned {
parent: parent.clone(),
});
@ -566,13 +566,7 @@ impl<'settings, 'repo> DescendantRebaser<'settings, 'repo> {
)?;
let new_commit = match rebased_commit {
RebasedCommit::Rewritten(new_commit) => new_commit,
RebasedCommit::Abandoned { parent } => {
self.mut_repo
.parent_mapping
.insert(old_commit_id.clone(), vec![parent.id().clone()]);
self.mut_repo.abandoned.insert(old_commit.id().clone());
parent
}
RebasedCommit::Abandoned { parent } => parent,
};
self.rebased
.insert(old_commit_id.clone(), new_commit.id().clone());