From 39838a76bff37dc9124d9357248e07f8d65ab043 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Wed, 16 Mar 2022 15:58:37 -0700 Subject: [PATCH] view: move merging of views up to repo level (#111) I want to make it so when we apply a repo-level change that removes a head, we rebase descendants of that head onto the closes visible ancestor, or onto its successor if the head has been rewritten (see #111 for details). The view itself doesn't have enough information for that; we need repo-level information (to figure out relationships between commits). The view doesn't have enough information to do the. --- lib/src/repo.rs | 107 ++++++++++++++++++++++++++++++++++++++++++++++-- lib/src/view.rs | 101 --------------------------------------------- 2 files changed, 104 insertions(+), 104 deletions(-) diff --git a/lib/src/repo.rs b/lib/src/repo.rs index 27d4d0879..f76703fd1 100644 --- a/lib/src/repo.rs +++ b/lib/src/repo.rs @@ -693,12 +693,113 @@ impl MutableRepo { self.index.merge_in(other_repo.index()); self.enforce_view_invariants(); - self.view - .get_mut() - .merge(self.index.as_index_ref(), &base_repo.view, &other_repo.view); + self.merge_view(&base_repo.view, &other_repo.view); self.view_dirty = true; } + fn merge_view(&mut self, base: &View, other: &View) { + // Merge checkouts. If there's a conflict, we keep the self side. + for (workspace_id, base_checkout) in base.checkouts() { + let self_checkout = self.view().get_checkout(workspace_id); + let other_checkout = other.get_checkout(workspace_id); + if other_checkout == Some(base_checkout) || other_checkout == self_checkout { + // The other side didn't change or both sides changed in the + // same way. + } else if let Some(other_checkout) = other_checkout { + if self_checkout == Some(base_checkout) { + self.view_mut() + .set_checkout(workspace_id.clone(), other_checkout.clone()); + } + } else { + // The other side removed the workspace. We want to remove it even if the self + // side changed the checkout. + self.view_mut().remove_checkout(workspace_id); + } + } + for (workspace_id, other_checkout) in other.checkouts() { + if self.view().get_checkout(workspace_id).is_none() + && base.get_checkout(workspace_id).is_none() + { + // The other side added the workspace. + self.view_mut() + .set_checkout(workspace_id.clone(), other_checkout.clone()); + } + } + + for removed_head in base.public_heads().difference(other.public_heads()) { + self.view_mut().remove_public_head(removed_head); + } + for added_head in other.public_heads().difference(base.public_heads()) { + self.view_mut().add_public_head(added_head); + } + + for removed_head in base.heads().difference(other.heads()) { + self.view_mut().remove_head(removed_head); + } + for added_head in other.heads().difference(base.heads()) { + self.view_mut().add_head(added_head); + } + // TODO: Should it be considered a conflict if a commit-head is removed on one + // side while a child or successor is created on another side? Maybe a + // warning? + + let mut maybe_changed_ref_names = HashSet::new(); + + let base_branches: HashSet<_> = base.branches().keys().cloned().collect(); + let other_branches: HashSet<_> = other.branches().keys().cloned().collect(); + for branch_name in base_branches.union(&other_branches) { + let base_branch = base.branches().get(branch_name); + let other_branch = other.branches().get(branch_name); + if other_branch == base_branch { + // Unchanged on other side + continue; + } + + maybe_changed_ref_names.insert(RefName::LocalBranch(branch_name.clone())); + if let Some(branch) = base_branch { + for remote in branch.remote_targets.keys() { + maybe_changed_ref_names.insert(RefName::RemoteBranch { + branch: branch_name.clone(), + remote: remote.clone(), + }); + } + } + if let Some(branch) = other_branch { + for remote in branch.remote_targets.keys() { + maybe_changed_ref_names.insert(RefName::RemoteBranch { + branch: branch_name.clone(), + remote: remote.clone(), + }); + } + } + } + + for tag_name in base.tags().keys() { + maybe_changed_ref_names.insert(RefName::Tag(tag_name.clone())); + } + for tag_name in other.tags().keys() { + maybe_changed_ref_names.insert(RefName::Tag(tag_name.clone())); + } + + for git_ref_name in base.git_refs().keys() { + maybe_changed_ref_names.insert(RefName::GitRef(git_ref_name.clone())); + } + for git_ref_name in other.git_refs().keys() { + maybe_changed_ref_names.insert(RefName::GitRef(git_ref_name.clone())); + } + + for ref_name in maybe_changed_ref_names { + let base_target = base.get_ref(&ref_name); + let other_target = other.get_ref(&ref_name); + self.view.get_mut().merge_single_ref( + self.index.as_index_ref(), + &ref_name, + base_target.as_ref(), + other_target.as_ref(), + ); + } + } + pub fn merge_single_ref( &mut self, ref_name: &RefName, diff --git a/lib/src/view.rs b/lib/src/view.rs index b1801a637..6f3a67f5f 100644 --- a/lib/src/view.rs +++ b/lib/src/view.rs @@ -236,107 +236,6 @@ impl View { &mut self.data } - pub fn merge(&mut self, index: IndexRef, base: &View, other: &View) { - // Merge checkouts. If there's a conflict, we keep the self side. - for (workspace_id, base_checkout) in base.checkouts() { - let self_checkout = self.get_checkout(workspace_id); - let other_checkout = other.get_checkout(workspace_id); - if other_checkout == Some(base_checkout) || other_checkout == self_checkout { - // The other side didn't change or both sides changed in the - // same way. - } else if let Some(other_checkout) = other_checkout { - if self_checkout == Some(base_checkout) { - self.set_checkout(workspace_id.clone(), other_checkout.clone()); - } - } else { - // The other side removed the workspace. We want to remove it even if the self - // side changed the checkout. - self.remove_checkout(workspace_id); - } - } - for (workspace_id, other_checkout) in other.checkouts() { - if self.get_checkout(workspace_id).is_none() - && base.get_checkout(workspace_id).is_none() - { - // The other side added the workspace. - self.set_checkout(workspace_id.clone(), other_checkout.clone()); - } - } - - for removed_head in base.public_heads().difference(other.public_heads()) { - self.remove_public_head(removed_head); - } - for added_head in other.public_heads().difference(base.public_heads()) { - self.add_public_head(added_head); - } - - for removed_head in base.heads().difference(other.heads()) { - self.remove_head(removed_head); - } - for added_head in other.heads().difference(base.heads()) { - self.add_head(added_head); - } - // TODO: Should it be considered a conflict if a commit-head is removed on one - // side while a child or successor is created on another side? Maybe a - // warning? - - let mut maybe_changed_ref_names = HashSet::new(); - - let base_branches: HashSet<_> = base.branches().keys().cloned().collect(); - let other_branches: HashSet<_> = other.branches().keys().cloned().collect(); - for branch_name in base_branches.union(&other_branches) { - let base_branch = base.branches().get(branch_name); - let other_branch = other.branches().get(branch_name); - if other_branch == base_branch { - // Unchanged on other side - continue; - } - - maybe_changed_ref_names.insert(RefName::LocalBranch(branch_name.clone())); - if let Some(branch) = base_branch { - for remote in branch.remote_targets.keys() { - maybe_changed_ref_names.insert(RefName::RemoteBranch { - branch: branch_name.clone(), - remote: remote.clone(), - }); - } - } - if let Some(branch) = other_branch { - for remote in branch.remote_targets.keys() { - maybe_changed_ref_names.insert(RefName::RemoteBranch { - branch: branch_name.clone(), - remote: remote.clone(), - }); - } - } - } - - for tag_name in base.tags().keys() { - maybe_changed_ref_names.insert(RefName::Tag(tag_name.clone())); - } - for tag_name in other.tags().keys() { - maybe_changed_ref_names.insert(RefName::Tag(tag_name.clone())); - } - - for git_ref_name in base.git_refs().keys() { - maybe_changed_ref_names.insert(RefName::GitRef(git_ref_name.clone())); - } - for git_ref_name in other.git_refs().keys() { - maybe_changed_ref_names.insert(RefName::GitRef(git_ref_name.clone())); - } - - for ref_name in maybe_changed_ref_names { - let base_target = base.get_ref(&ref_name); - let other_target = other.get_ref(&ref_name); - self.merge_single_ref( - index, - &ref_name, - base_target.as_ref(), - other_target.as_ref(), - ); - } - } - pub fn merge_single_ref( &mut self, index: IndexRef,