From 6e3ceb4d1cc07785f2f677e2f25f1693c27c300a Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Sat, 23 Mar 2024 06:55:11 -0700 Subject: [PATCH] repo: store separate `divergent` field, pass into `DescendantRebaser` With this patch, `MutableRepo` has the same tracking of rewritten commits as `DescendantRebaser`, so we can simply pass that state into `DescendantRebaser` when we create it. The next step is to remove the state from `DescendantRebaser`. --- lib/src/repo.rs | 26 +++++++++++++++++--------- lib/src/rewrite.rs | 29 +++++++++-------------------- 2 files changed, 26 insertions(+), 29 deletions(-) diff --git a/lib/src/repo.rs b/lib/src/repo.rs index 56a354ea8..ada0e9487 100644 --- a/lib/src/repo.rs +++ b/lib/src/repo.rs @@ -732,7 +732,10 @@ pub struct MutableRepo { base_repo: Arc, index: Box, view: DirtyCell, - rewritten_commits: HashMap>, + parent_mapping: HashMap>, + /// Commits with a key in `parent_mapping` that have been divergently + /// rewritten into all the commits indicated by the value. + divergent: HashSet, abandoned_commits: HashSet, } @@ -748,7 +751,8 @@ impl MutableRepo { base_repo, index: mut_index, view: DirtyCell::with_clean(mut_view), - rewritten_commits: Default::default(), + parent_mapping: Default::default(), + divergent: Default::default(), abandoned_commits: Default::default(), } } @@ -767,7 +771,7 @@ impl MutableRepo { pub fn has_changes(&self) -> bool { !(self.abandoned_commits.is_empty() - && self.rewritten_commits.is_empty() + && self.parent_mapping.is_empty() && self.view() == &self.base_repo.view) } @@ -816,7 +820,8 @@ impl MutableRepo { /// docstring for `record_rewritten_commit` for details. pub fn set_rewritten_commit(&mut self, old_id: CommitId, new_id: CommitId) { assert_ne!(old_id, *self.store().root_commit_id()); - self.rewritten_commits.insert(old_id, vec![new_id]); + self.divergent.remove(&old_id); + self.parent_mapping.insert(old_id, vec![new_id]); } /// Record a commit as being rewritten into multiple other commits in this @@ -832,8 +837,9 @@ impl MutableRepo { new_ids: impl IntoIterator, ) { assert_ne!(old_id, *self.store().root_commit_id()); - self.rewritten_commits - .insert(old_id, new_ids.into_iter().collect()); + self.parent_mapping + .insert(old_id.clone(), new_ids.into_iter().collect()); + self.divergent.insert(old_id); } /// Record a commit as having been abandoned in this transaction. @@ -851,12 +857,13 @@ impl MutableRepo { } fn clear_descendant_rebaser_plans(&mut self) { - self.rewritten_commits.clear(); + self.parent_mapping.clear(); + self.divergent.clear(); self.abandoned_commits.clear(); } pub fn has_rewrites(&self) -> bool { - !(self.rewritten_commits.is_empty() && self.abandoned_commits.is_empty()) + !(self.parent_mapping.is_empty() && self.abandoned_commits.is_empty()) } /// After the rebaser returned by this function is dropped, @@ -873,7 +880,8 @@ impl MutableRepo { let mut rebaser = DescendantRebaser::new( settings, self, - self.rewritten_commits.clone(), + self.parent_mapping.clone(), + self.divergent.clone(), self.abandoned_commits.clone(), ); *rebaser.mut_options() = options; diff --git a/lib/src/rewrite.rs b/lib/src/rewrite.rs index f3973ac9a..ab3e86df0 100644 --- a/lib/src/rewrite.rs +++ b/lib/src/rewrite.rs @@ -313,17 +313,18 @@ impl<'settings, 'repo> DescendantRebaser<'settings, 'repo> { pub fn new( settings: &'settings UserSettings, mut_repo: &'repo mut MutableRepo, - rewritten: HashMap>, + parent_mapping: HashMap>, + divergent: HashSet, abandoned: HashSet, ) -> DescendantRebaser<'settings, 'repo> { let store = mut_repo.store(); let root_commit_id = store.root_commit_id(); assert!(!abandoned.contains(root_commit_id)); - assert!(!rewritten.contains_key(root_commit_id)); - let old_commits_expression = RevsetExpression::commits(rewritten.keys().cloned().collect()) - .union(&RevsetExpression::commits( - abandoned.iter().cloned().collect(), - )); + assert!(!parent_mapping.contains_key(root_commit_id)); + let old_commits_expression = + RevsetExpression::commits(parent_mapping.keys().cloned().collect()).union( + &RevsetExpression::commits(abandoned.iter().cloned().collect()), + ); let heads_to_add_expression = old_commits_expression .parents() .minus(&old_commits_expression); @@ -349,7 +350,7 @@ impl<'settings, 'repo> DescendantRebaser<'settings, 'repo> { visited.insert(commit.id().clone()); let mut dependents = vec![]; for parent in commit.parents() { - if let Some(targets) = rewritten.get(parent.id()) { + if let Some(targets) = parent_mapping.get(parent.id()) { for target in targets { if to_visit_set.contains(target) && !visited.contains(target) { dependents.push(store.get_commit(target).unwrap()); @@ -364,19 +365,7 @@ impl<'settings, 'repo> DescendantRebaser<'settings, 'repo> { }, ); - let new_commits = rewritten.values().flatten().cloned().collect(); - - let mut parent_mapping = HashMap::new(); - let mut divergent = HashSet::new(); - for (old_commit, new_commits) in rewritten { - if new_commits.len() == 1 { - parent_mapping.insert(old_commit, vec![new_commits.into_iter().next().unwrap()]); - } else { - let new_commits = mut_repo.index().heads(&mut new_commits.iter()); - parent_mapping.insert(old_commit.clone(), new_commits); - divergent.insert(old_commit); - } - } + let new_commits = parent_mapping.values().flatten().cloned().collect(); // Build a map from commit to branches pointing to it, so we don't need to scan // all branches each time we rebase a commit.