Skip to content

Commit

Permalink
lib repo.rs & rewrite.rs: Move clearing of rewritten/abandoned co…
Browse files Browse the repository at this point in the history
…mmits

This commit is a little out of place in this sequence, but
it seems to make more sense for MutRepo to own these maps.

@yuja [pointed out] that any tests written using `create_descendant_rebaser` now
need to do this cleanup, but there are no longer any such tests after the
previous commits and a follow-up commit removes `create_descendant_rebaser`
entirely.

[pointed out]: #2737 (comment)
  • Loading branch information
ilyagr committed Jan 2, 2024
1 parent 4461d61 commit 7cef879
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 11 deletions.
21 changes: 12 additions & 9 deletions lib/src/repo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -859,10 +859,6 @@ impl MutableRepo {
.insert(new_id);
}

pub fn clear_rewritten_commits(&mut self) {
self.rewritten_commits.clear();
}

/// Record a commit as having been abandoned in this transaction.
///
/// This record is used by `rebase_descendants` to know which commits have
Expand All @@ -877,7 +873,8 @@ impl MutableRepo {
self.abandoned_commits.insert(old_id);
}

pub fn clear_abandoned_commits(&mut self) {
fn clear_descendant_rebaser_plans(&mut self) {
self.rewritten_commits.clear();
self.abandoned_commits.clear();
}

Expand All @@ -900,6 +897,8 @@ impl MutableRepo {
)
}

/// After the rebaser returned by this function is dropped,
/// self.clear_descendant_rebaser_plans() needs to be called.
fn rebase_descendants_return_rebaser<'settings, 'repo>(
&'repo mut self,
settings: &'settings UserSettings,
Expand All @@ -923,9 +922,11 @@ impl MutableRepo {
settings: &UserSettings,
options: RebaseOptions,
) -> Result<usize, TreeMergeError> {
Ok(self
let result = self
.rebase_descendants_return_rebaser(settings, options)?
.map_or(0, |rebaser| rebaser.rebased().len()))
.map_or(0, |rebaser| rebaser.rebased().len());
self.clear_descendant_rebaser_plans();
Ok(result)
}

pub fn rebase_descendants(&mut self, settings: &UserSettings) -> Result<usize, TreeMergeError> {
Expand All @@ -936,12 +937,14 @@ impl MutableRepo {
&mut self,
settings: &UserSettings,
) -> Result<HashMap<CommitId, CommitId>, TreeMergeError> {
Ok(self
let result = Ok(self
// We do not set RebaseOptions here, since this function does not currently return
// enough information to describe the results of a rebase if some commits got
// abandoned
.rebase_descendants_return_rebaser(settings, Default::default())?
.map_or(HashMap::new(), |rebaser| rebaser.rebased().clone()))
.map_or(HashMap::new(), |rebaser| rebaser.rebased().clone()));
self.clear_descendant_rebaser_plans();
result
}

pub fn set_wc_commit(
Expand Down
2 changes: 0 additions & 2 deletions lib/src/rewrite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -600,8 +600,6 @@ impl<'settings, 'repo> DescendantRebaser<'settings, 'repo> {
self.heads_to_remove.clear();
self.heads_to_add.clear();
self.mut_repo.set_view(view);
self.mut_repo.clear_rewritten_commits();
self.mut_repo.clear_abandoned_commits();
Ok(None)
}

Expand Down

0 comments on commit 7cef879

Please sign in to comment.