From 6d3e2017f8c057de2a326105c0beca7420641fb6 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Thu, 25 Nov 2021 23:12:00 -0800 Subject: [PATCH] view: merge checkouts for all workspaces, not just the default one (#13) --- lib/src/repo.rs | 4 +++ lib/src/view.rs | 43 ++++++++++++++++++++------ lib/tests/test_view.rs | 70 ++++++++++++++++++++++++++++++++++++------ 3 files changed, 98 insertions(+), 19 deletions(-) diff --git a/lib/src/repo.rs b/lib/src/repo.rs index 5b17aff59..35ae07a78 100644 --- a/lib/src/repo.rs +++ b/lib/src/repo.rs @@ -551,6 +551,10 @@ impl MutableRepo { self.view_mut().set_checkout(workspace_id, commit_id); } + pub fn remove_checkout(&mut self, workspace_id: &WorkspaceId) { + self.view_mut().remove_checkout(workspace_id); + } + pub fn check_out( &mut self, workspace_id: WorkspaceId, diff --git a/lib/src/view.rs b/lib/src/view.rs index 2663e3f26..50dfd15b2 100644 --- a/lib/src/view.rs +++ b/lib/src/view.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::collections::{BTreeMap, HashSet}; +use std::collections::{BTreeMap, HashMap, HashSet}; use crate::backend::CommitId; use crate::index::IndexRef; @@ -45,6 +45,10 @@ impl View { self.get_checkout(&WorkspaceId::default()).unwrap() } + pub fn checkouts(&self) -> &HashMap { + &self.data.checkouts + } + pub fn get_checkout(&self, workspace_id: &WorkspaceId) -> Option<&CommitId> { self.data.checkouts.get(workspace_id) } @@ -77,6 +81,10 @@ impl View { self.data.checkouts.insert(workspace_id, commit_id); } + pub fn remove_checkout(&mut self, workspace_id: &WorkspaceId) { + self.data.checkouts.remove(workspace_id); + } + pub fn add_head(&mut self, head_id: &CommitId) { self.data.head_ids.insert(head_id.clone()); } @@ -234,15 +242,30 @@ impl View { } pub fn merge(&mut self, index: IndexRef, base: &View, other: &View) { - // TODO: Merge checkout for all workspaces or pass in a particular workspace ID - // to this function - if other.checkout() == base.checkout() || other.checkout() == self.checkout() { - // Keep the self side - } else if self.checkout() == base.checkout() { - self.set_checkout(WorkspaceId::default(), other.checkout().clone()); - } else { - // TODO: Return an error here. Or should we just pick one of the - // sides and emit a warning? + // 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()) { diff --git a/lib/tests/test_view.rs b/lib/tests/test_view.rs index eae3c875a..ba07b0f0a 100644 --- a/lib/tests/test_view.rs +++ b/lib/tests/test_view.rs @@ -155,27 +155,79 @@ fn test_merge_views_checkout() { let test_workspace = testutils::init_repo(&settings, false); let repo = &test_workspace.repo; + // Workspace 1 gets updated in both transactions. + // Workspace 2 gets updated only in tx1. + // Workspace 3 gets updated only in tx2. + // Workspace 4 gets deleted in tx1 and modified in tx2. + // Workspace 5 gets deleted in tx2 and modified in tx1. + // Workspace 6 gets added in tx1. + // Workspace 7 gets added in tx2. + let mut initial_tx = repo.start_transaction("test"); + let commit1 = + testutils::create_random_commit(&settings, repo).write_to_repo(initial_tx.mut_repo()); + let commit2 = + testutils::create_random_commit(&settings, repo).write_to_repo(initial_tx.mut_repo()); + let commit3 = + testutils::create_random_commit(&settings, repo).write_to_repo(initial_tx.mut_repo()); + let ws1_id = WorkspaceId::new("ws1".to_string()); + let ws2_id = WorkspaceId::new("ws2".to_string()); + let ws3_id = WorkspaceId::new("ws3".to_string()); + let ws4_id = WorkspaceId::new("ws4".to_string()); + let ws5_id = WorkspaceId::new("ws5".to_string()); + let ws6_id = WorkspaceId::new("ws6".to_string()); + let ws7_id = WorkspaceId::new("ws7".to_string()); + initial_tx + .mut_repo() + .set_checkout(ws1_id.clone(), commit1.id().clone()); + initial_tx + .mut_repo() + .set_checkout(ws2_id.clone(), commit1.id().clone()); + initial_tx + .mut_repo() + .set_checkout(ws3_id.clone(), commit1.id().clone()); + initial_tx + .mut_repo() + .set_checkout(ws4_id.clone(), commit1.id().clone()); + initial_tx + .mut_repo() + .set_checkout(ws5_id.clone(), commit1.id().clone()); + let repo = initial_tx.commit(); + let mut tx1 = repo.start_transaction("test"); - let checkout_tx1 = testutils::create_random_commit(&settings, repo) - .set_open(false) - .write_to_repo(tx1.mut_repo()); tx1.mut_repo() - .set_checkout(WorkspaceId::default(), checkout_tx1.id().clone()); + .set_checkout(ws1_id.clone(), commit2.id().clone()); + tx1.mut_repo() + .set_checkout(ws2_id.clone(), commit2.id().clone()); + tx1.mut_repo().remove_checkout(&ws4_id); + tx1.mut_repo() + .set_checkout(ws5_id.clone(), commit2.id().clone()); + tx1.mut_repo() + .set_checkout(ws6_id.clone(), commit2.id().clone()); tx1.commit(); let mut tx2 = repo.start_transaction("test"); - let checkout_tx2 = testutils::create_random_commit(&settings, repo) - .set_open(false) - .write_to_repo(tx2.mut_repo()); tx2.mut_repo() - .set_checkout(WorkspaceId::default(), checkout_tx2.id().clone()); + .set_checkout(ws1_id.clone(), commit3.id().clone()); + tx2.mut_repo() + .set_checkout(ws3_id.clone(), commit3.id().clone()); + tx2.mut_repo() + .set_checkout(ws4_id.clone(), commit3.id().clone()); + tx2.mut_repo().remove_checkout(&ws5_id); + tx2.mut_repo() + .set_checkout(ws7_id.clone(), commit3.id().clone()); tx2.commit(); let repo = repo.reload(); // We currently arbitrarily pick the first transaction's checkout (first by // transaction end time). - assert_eq!(repo.view().checkout(), checkout_tx1.id()); + assert_eq!(repo.view().get_checkout(&ws1_id), Some(commit2.id())); + assert_eq!(repo.view().get_checkout(&ws2_id), Some(commit2.id())); + assert_eq!(repo.view().get_checkout(&ws3_id), Some(commit3.id())); + assert_eq!(repo.view().get_checkout(&ws4_id), None); + assert_eq!(repo.view().get_checkout(&ws5_id), None); + assert_eq!(repo.view().get_checkout(&ws6_id), Some(commit2.id())); + assert_eq!(repo.view().get_checkout(&ws7_id), Some(commit3.id())); } #[test]