diff --git a/lib/src/repo.rs b/lib/src/repo.rs index cd5178f9f2..918b874858 100644 --- a/lib/src/repo.rs +++ b/lib/src/repo.rs @@ -728,21 +728,27 @@ impl RepoLoader { } } +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum RewriteType { + Rewritten, + Divergent, + Abandoned, +} + pub struct MutableRepo { base_repo: Arc, index: Box, view: DirtyCell, // TODO: make these fields private again - // The commit identified by the key has been replaced by all the ones in the value, typically - // because the key commit was abandoned (the value commits are then the abandoned commit's - // parents). A child of the key commit should be rebased onto all the value commits. A branch - // pointing to the key commit should become a conflict pointing to all the value commits. - pub(crate) parent_mapping: HashMap>, - /// Commits with a key in `parent_mapping` that have been divergently - /// rewritten into all the commits indicated by the value. - pub(crate) divergent: HashSet, - // Commits that were abandoned. Their children should be rebased onto their parents. - pub(crate) abandoned: HashSet, + // The commit identified by the key has been replaced by all the ones in the value. + // * Branches pointing to the old commit should be updated to the new commit, resulting in a + // conflict if there multiple new commits. + // * Children of the old commit should be rebased onto the new commits. However, if the type is + // `Divergent`, they should be left in place. + // * Working copies pointing to the old commit should be updated to the first of the new + // commits. However, if the type is `Abandoned`, a new working-copy commit should be created + // on top of all of the new commits instead. + pub(crate) parent_mapping: HashMap)>, } impl MutableRepo { @@ -758,8 +764,6 @@ impl MutableRepo { index: mut_index, view: DirtyCell::with_clean(mut_view), parent_mapping: Default::default(), - divergent: Default::default(), - abandoned: Default::default(), } } @@ -776,9 +780,7 @@ impl MutableRepo { } pub fn has_changes(&self) -> bool { - !(self.abandoned.is_empty() - && self.parent_mapping.is_empty() - && self.view() == &self.base_repo.view) + !(self.parent_mapping.is_empty() && self.view() == &self.base_repo.view) } pub(crate) fn consume(self) -> (Box, View) { @@ -826,9 +828,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.divergent.remove(&old_id); - self.abandoned.remove(&old_id); - self.parent_mapping.insert(old_id, vec![new_id]); + self.parent_mapping + .insert(old_id, (RewriteType::Rewritten, vec![new_id])); } /// Record a commit as being rewritten into multiple other commits in this @@ -844,10 +845,10 @@ impl MutableRepo { new_ids: impl IntoIterator, ) { assert_ne!(old_id, *self.store().root_commit_id()); - self.abandoned.remove(&old_id); - self.parent_mapping - .insert(old_id.clone(), new_ids.into_iter().collect()); - self.divergent.insert(old_id); + self.parent_mapping.insert( + old_id.clone(), + (RewriteType::Divergent, new_ids.into_iter().collect()), + ); } /// Record a commit as having been abandoned in this transaction. @@ -877,21 +878,15 @@ impl MutableRepo { old_id: CommitId, new_parent_ids: impl IntoIterator, ) { - 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, new_parent_ids.into_iter().collect()); - } - - fn clear_descendant_rebaser_plans(&mut self) { - self.parent_mapping.clear(); - self.divergent.clear(); - self.abandoned.clear(); + self.parent_mapping.insert( + old_id, + (RewriteType::Abandoned, new_parent_ids.into_iter().collect()), + ); } pub fn has_rewrites(&self) -> bool { - !(self.parent_mapping.is_empty() && self.abandoned.is_empty()) + !self.parent_mapping.is_empty() } /// Calculates new parents for a commit that's currently based on the given @@ -901,8 +896,7 @@ impl MutableRepo { /// Panics if `parent_mapping` contains cycles pub fn new_parents(&self, old_ids: &[CommitId]) -> Vec { fn single_substitution_round( - parent_mapping: &HashMap>, - divergent: &HashSet, + parent_mapping: &HashMap)>, ids: Vec, ) -> (Vec, bool) { let mut made_replacements = false; @@ -911,13 +905,11 @@ impl MutableRepo { // being singletons. If CommitId-s were Copy. no allocations would be needed in // that case, but it probably doesn't matter much while they are Vec-s. for id in ids.into_iter() { - if divergent.contains(&id) { - new_ids.push(id); - continue; - } match parent_mapping.get(&id) { - None => new_ids.push(id), - Some(replacements) => { + None | Some((RewriteType::Divergent, _)) => { + new_ids.push(id); + } + Some((_, replacements)) => { assert!( // Each commit must have a parent, so a parent can // not just be mapped to nothing. This assertion @@ -940,8 +932,7 @@ impl MutableRepo { let mut allowed_iterations = 0..self.parent_mapping.len(); loop { let made_replacements; - (new_ids, made_replacements) = - single_substitution_round(&self.parent_mapping, &self.divergent, new_ids); + (new_ids, made_replacements) = single_substitution_round(&self.parent_mapping, new_ids); if !made_replacements { break; } @@ -959,7 +950,7 @@ impl MutableRepo { } /// After the rebaser returned by this function is dropped, - /// self.clear_descendant_rebaser_plans() needs to be called. + /// self.parent_mapping needs to be cleared. fn rebase_descendants_return_rebaser<'settings, 'repo>( &'repo mut self, settings: &'settings UserSettings, @@ -986,7 +977,7 @@ impl MutableRepo { let result = self .rebase_descendants_return_rebaser(settings, options)? .map_or(0, |rebaser| rebaser.into_map().len()); - self.clear_descendant_rebaser_plans(); + self.parent_mapping.clear(); Ok(result) } @@ -1014,7 +1005,7 @@ impl MutableRepo { // abandoned .rebase_descendants_return_rebaser(settings, options)? .map_or(HashMap::new(), |rebaser| rebaser.into_map())); - self.clear_descendant_rebaser_plans(); + self.parent_mapping.clear(); result } diff --git a/lib/src/rewrite.rs b/lib/src/rewrite.rs index fb41264a87..382d11ebc9 100644 --- a/lib/src/rewrite.rs +++ b/lib/src/rewrite.rs @@ -30,7 +30,7 @@ use crate::matchers::{Matcher, Visit}; use crate::merged_tree::{MergedTree, MergedTreeBuilder}; use crate::object_id::ObjectId; use crate::op_store::RefTarget; -use crate::repo::{MutableRepo, Repo}; +use crate::repo::{MutableRepo, Repo, RewriteType}; use crate::repo_path::RepoPath; use crate::revset::{RevsetExpression, RevsetIteratorExt}; use crate::settings::UserSettings; @@ -317,7 +317,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) = mut_repo.parent_mapping.get(parent.id()) { + if let Some((_, targets)) = mut_repo.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()); @@ -367,7 +367,10 @@ impl<'settings, 'repo> DescendantRebaser<'settings, 'repo> { new_commit_ids: Vec, ) -> Result<(), BackendError> { // We arbitrarily pick a new working-copy commit among the candidates. - let abandoned_old_commit = self.mut_repo.abandoned.contains(&old_commit_id); + let abandoned_old_commit = matches!( + self.mut_repo.parent_mapping.get(&old_commit_id), + Some((RewriteType::Abandoned, _)) + ); self.update_wc_commits(&old_commit_id, &new_commit_ids[0], abandoned_old_commit)?; // Build a map from commit to branches pointing to it, so we don't need to scan @@ -476,7 +479,7 @@ impl<'settings, 'repo> DescendantRebaser<'settings, 'repo> { } fn update_all_references(&mut self) -> Result<(), BackendError> { - for (old_parent_id, new_parent_ids) in self.mut_repo.parent_mapping.clone() { + for (old_parent_id, (_, new_parent_ids)) in self.mut_repo.parent_mapping.clone() { // Call `new_parents()` here since `parent_mapping` only contains direct // mappings, not transitive ones. // TODO: keep parent_mapping updated with transitive mappings so we don't need