mirrors/jj
1
0
Fork 1
mirror of https://github.com/martinvonz/jj.git synced 2025-01-20 03:20:08 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 6

repo: merge rewrite state into single parent_mapping with enum

Browse source 
This simplifies the code and reduces the risk of inconsistencies in
the data.

Thanks to Yuya for the suggestion.
This commit is contained in:
Martin von Zweigbergk 2024-03-30 06:16:32 -07:00 committed by Martin von Zweigbergk
parent a6615bf36d
commit bbe906b426
2 changed files with 44 additions and 50 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

83
lib/src/repo.rs
View file

@ -727,21 +727,27 @@ impl RepoLoader {
} }
} }
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub(crate) enum RewriteType {
Rewritten,
Divergent,
Abandoned,
}
pub struct MutableRepo { pub struct MutableRepo {
base_repo: Arc<ReadonlyRepo>, base_repo: Arc<ReadonlyRepo>,
index: Box<dyn MutableIndex>, index: Box<dyn MutableIndex>,
view: DirtyCell<View>, view: DirtyCell<View>,
// TODO: make these fields private again // TODO: make these fields private again
// The commit identified by the key has been replaced by all the ones in the value, typically // The commit identified by the key has been replaced by all the ones in the value.
// because the key commit was abandoned (the value commits are then the abandoned commit's // * Branches pointing to the old commit should be updated to the new commit, resulting in a
// parents). A child of the key commit should be rebased onto all the value commits. A branch // conflict if there multiple new commits.
// pointing to the key commit should become a conflict pointing to all the value commits. // * Children of the old commit should be rebased onto the new commits. However, if the type is
pub(crate) parent_mapping: HashMap<CommitId, Vec<CommitId>>, // `Divergent`, they should be left in place.
/// Commits with a key in `parent_mapping` that have been divergently // * Working copies pointing to the old commit should be updated to the first of the new
/// rewritten into all the commits indicated by the value. // commits. However, if the type is `Abandoned`, a new working-copy commit should be created
pub(crate) divergent: HashSet<CommitId>, // on top of all of the new commits instead.
// Commits that were abandoned. Their children should be rebased onto their parents. pub(crate) parent_mapping: HashMap<CommitId, (RewriteType, Vec<CommitId>)>,
pub(crate) abandoned: HashSet<CommitId>,
} }
impl MutableRepo { impl MutableRepo {
@ -757,8 +763,6 @@ impl MutableRepo {
index: mut_index, index: mut_index,
view: DirtyCell::with_clean(mut_view), view: DirtyCell::with_clean(mut_view),
parent_mapping: Default::default(), parent_mapping: Default::default(),
divergent: Default::default(),
abandoned: Default::default(),
} }
} }
@ -775,9 +779,7 @@ impl MutableRepo {
} }
pub fn has_changes(&self) -> bool { 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<dyn MutableIndex>, View) { pub(crate) fn consume(self) -> (Box<dyn MutableIndex>, View) {
@ -825,9 +827,8 @@ impl MutableRepo {
/// docstring for `record_rewritten_commit` for details. /// docstring for `record_rewritten_commit` for details.
pub fn set_rewritten_commit(&mut self, old_id: CommitId, new_id: CommitId) { pub fn set_rewritten_commit(&mut self, old_id: CommitId, new_id: CommitId) {
assert_ne!(old_id, *self.store().root_commit_id()); assert_ne!(old_id, *self.store().root_commit_id());
self.divergent.remove(&old_id); self.parent_mapping
self.abandoned.remove(&old_id); .insert(old_id, (RewriteType::Rewritten, vec![new_id]));
self.parent_mapping.insert(old_id, vec![new_id]);
} }
/// Record a commit as being rewritten into multiple other commits in this /// Record a commit as being rewritten into multiple other commits in this
@ -843,10 +844,10 @@ impl MutableRepo {
new_ids: impl IntoIterator<Item = CommitId>, new_ids: impl IntoIterator<Item = CommitId>,
) { ) {
assert_ne!(old_id, *self.store().root_commit_id()); assert_ne!(old_id, *self.store().root_commit_id());
self.abandoned.remove(&old_id); self.parent_mapping.insert(
self.parent_mapping old_id.clone(),
.insert(old_id.clone(), new_ids.into_iter().collect()); (RewriteType::Divergent, new_ids.into_iter().collect()),
self.divergent.insert(old_id); );
} }
/// Record a commit as having been abandoned in this transaction. /// Record a commit as having been abandoned in this transaction.
@ -876,21 +877,15 @@ impl MutableRepo {
old_id: CommitId, old_id: CommitId,
new_parent_ids: impl IntoIterator<Item = CommitId>, new_parent_ids: impl IntoIterator<Item = CommitId>,
) { ) {
self.divergent.remove(&old_id);
assert_ne!(old_id, *self.store().root_commit_id()); assert_ne!(old_id, *self.store().root_commit_id());
self.abandoned.insert(old_id.clone()); self.parent_mapping.insert(
self.parent_mapping old_id,
.insert(old_id, new_parent_ids.into_iter().collect()); (RewriteType::Abandoned, new_parent_ids.into_iter().collect()),
} );
fn clear_descendant_rebaser_plans(&mut self) {
self.parent_mapping.clear();
self.divergent.clear();
self.abandoned.clear();
} }
pub fn has_rewrites(&self) -> bool { 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 /// Calculates new parents for a commit that's currently based on the given
@ -900,8 +895,7 @@ impl MutableRepo {
/// Panics if `parent_mapping` contains cycles /// Panics if `parent_mapping` contains cycles
pub fn new_parents(&self, old_ids: &[CommitId]) -> Vec<CommitId> { pub fn new_parents(&self, old_ids: &[CommitId]) -> Vec<CommitId> {
fn single_substitution_round( fn single_substitution_round(
parent_mapping: &HashMap<CommitId, Vec<CommitId>>, parent_mapping: &HashMap<CommitId, (RewriteType, Vec<CommitId>)>,
divergent: &HashSet<CommitId>,
ids: Vec<CommitId>, ids: Vec<CommitId>,
) -> (Vec<CommitId>, bool) { ) -> (Vec<CommitId>, bool) {
let mut made_replacements = false; let mut made_replacements = false;
@ -910,13 +904,11 @@ impl MutableRepo {
// being singletons. If CommitId-s were Copy. no allocations would be needed in // 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<u8>-s. // that case, but it probably doesn't matter much while they are Vec<u8>-s.
for id in ids.into_iter() { for id in ids.into_iter() {
if divergent.contains(&id) {
new_ids.push(id);
continue;
}
match parent_mapping.get(&id) { match parent_mapping.get(&id) {
None => new_ids.push(id), None | Some((RewriteType::Divergent, _)) => {
Some(replacements) => { new_ids.push(id);
}
Some((_, replacements)) => {
assert!( assert!(
// Each commit must have a parent, so a parent can // Each commit must have a parent, so a parent can
// not just be mapped to nothing. This assertion // not just be mapped to nothing. This assertion
@ -939,8 +931,7 @@ impl MutableRepo {
let mut allowed_iterations = 0..self.parent_mapping.len(); let mut allowed_iterations = 0..self.parent_mapping.len();
loop { loop {
let made_replacements; let made_replacements;
(new_ids, made_replacements) = (new_ids, made_replacements) = single_substitution_round(&self.parent_mapping, new_ids);
single_substitution_round(&self.parent_mapping, &self.divergent, new_ids);
if !made_replacements { if !made_replacements {
break; break;
} }
@ -958,7 +949,7 @@ impl MutableRepo {
} }
/// After the rebaser returned by this function is dropped, /// 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>( fn rebase_descendants_return_rebaser<'settings, 'repo>(
&'repo mut self, &'repo mut self,
settings: &'settings UserSettings, settings: &'settings UserSettings,
@ -985,7 +976,7 @@ impl MutableRepo {
let result = self let result = self
.rebase_descendants_return_rebaser(settings, options)? .rebase_descendants_return_rebaser(settings, options)?
.map_or(0, |rebaser| rebaser.into_map().len()); .map_or(0, |rebaser| rebaser.into_map().len());
self.clear_descendant_rebaser_plans(); self.parent_mapping.clear();
Ok(result) Ok(result)
} }
@ -1013,7 +1004,7 @@ impl MutableRepo {
// abandoned // abandoned
.rebase_descendants_return_rebaser(settings, options)? .rebase_descendants_return_rebaser(settings, options)?
.map_or(HashMap::new(), |rebaser| rebaser.into_map())); .map_or(HashMap::new(), |rebaser| rebaser.into_map()));
self.clear_descendant_rebaser_plans(); self.parent_mapping.clear();
result result
} }

11
lib/src/rewrite.rs
View file

@ -30,7 +30,7 @@ use crate::matchers::{Matcher, Visit};
use crate::merged_tree::{MergedTree, MergedTreeBuilder}; use crate::merged_tree::{MergedTree, MergedTreeBuilder};
use crate::object_id::ObjectId; use crate::object_id::ObjectId;
use crate::op_store::RefTarget; use crate::op_store::RefTarget;
use crate::repo::{MutableRepo, Repo}; use crate::repo::{MutableRepo, Repo, RewriteType};
use crate::repo_path::RepoPath; use crate::repo_path::RepoPath;
use crate::revset::{RevsetExpression, RevsetIteratorExt}; use crate::revset::{RevsetExpression, RevsetIteratorExt};
use crate::settings::UserSettings; use crate::settings::UserSettings;
@ -313,7 +313,7 @@ impl<'settings, 'repo> DescendantRebaser<'settings, 'repo> {
visited.insert(commit.id().clone()); visited.insert(commit.id().clone());
let mut dependents = vec![]; let mut dependents = vec![];
for parent in commit.parents() { 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 { for target in targets {
if to_visit_set.contains(target) && !visited.contains(target) { if to_visit_set.contains(target) && !visited.contains(target) {
dependents.push(store.get_commit(target).unwrap()); dependents.push(store.get_commit(target).unwrap());
@ -363,7 +363,10 @@ impl<'settings, 'repo> DescendantRebaser<'settings, 'repo> {
new_commit_ids: Vec<CommitId>, new_commit_ids: Vec<CommitId>,
) -> Result<(), BackendError> { ) -> Result<(), BackendError> {
// We arbitrarily pick a new working-copy commit among the candidates. // 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)?; 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 // Build a map from commit to branches pointing to it, so we don't need to scan
@ -472,7 +475,7 @@ impl<'settings, 'repo> DescendantRebaser<'settings, 'repo> {
} }
fn update_all_references(&mut self) -> Result<(), BackendError> { 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 // Call `new_parents()` here since `parent_mapping` only contains direct
// mappings, not transitive ones. // mappings, not transitive ones.
// TODO: keep parent_mapping updated with transitive mappings so we don't need // TODO: keep parent_mapping updated with transitive mappings so we don't need