From 622b606bfbd368fe30ca6cf3a8463411c8ded3e9 Mon Sep 17 00:00:00 2001 From: Scott Taylor Date: Wed, 3 Jul 2024 18:58:38 -0500 Subject: [PATCH] repo: abandon working copy only if no other workspaces reference it Currently, if two workspaces are editing the same discardable commit and one of them switches to editing a different commit, it is abandoned even though the other workspace is still editing it. This commit treats workspaces as referencing their working-copy commits so that they won't be abandoned. --- lib/src/repo.rs | 22 +++++++++++++++------- lib/tests/test_mut_repo.rs | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 7 deletions(-) diff --git a/lib/src/repo.rs b/lib/src/repo.rs index 466657a0c2..a012420850 100644 --- a/lib/src/repo.rs +++ b/lib/src/repo.rs @@ -1325,10 +1325,17 @@ impl MutableRepo { &mut self, workspace_id: &WorkspaceId, ) -> Result<(), EditCommitError> { - fn local_branch_target_ids(view: &View) -> impl Iterator { - view.local_branches() - .flat_map(|(_, target)| target.added_ids()) - } + let is_commit_referenced = |view: &View, commit_id: &CommitId| -> bool { + view.wc_commit_ids() + .iter() + .filter(|&(ws_id, _)| ws_id != workspace_id) + .map(|(_, wc_id)| wc_id) + .chain( + view.local_branches() + .flat_map(|(_, target)| target.added_ids()), + ) + .any(|id| id == commit_id) + }; let maybe_wc_commit_id = self .view @@ -1341,11 +1348,12 @@ impl MutableRepo { if wc_commit.is_discardable(self)? && self .view - .with_ref(|v| local_branch_target_ids(v).all(|id| id != wc_commit.id())) + .with_ref(|v| !is_commit_referenced(v, wc_commit.id())) && self.view().heads().contains(wc_commit.id()) { - // Abandon the working-copy commit we're leaving if it's empty, not pointed by - // local branch, and a head commit. + // Abandon the working-copy commit we're leaving if it's + // discardable, not pointed by local branch or other working + // copies, and a head commit. self.record_abandoned_commit(wc_commit_id); } } diff --git a/lib/tests/test_mut_repo.rs b/lib/tests/test_mut_repo.rs index ff19e181e3..b5f4c33ea8 100644 --- a/lib/tests/test_mut_repo.rs +++ b/lib/tests/test_mut_repo.rs @@ -216,6 +216,38 @@ fn test_edit_previous_empty_with_local_branch() { assert!(mut_repo.view().heads().contains(old_wc_commit.id())); } +#[test] +fn test_edit_previous_empty_with_other_workspace() { + // Test that MutableRepo::edit() does not abandon the previous commit if it + // is pointed by another workspace + let settings = testutils::user_settings(); + let test_repo = TestRepo::init(); + let repo = &test_repo.repo; + + let mut tx = repo.start_transaction(&settings); + let mut_repo = tx.mut_repo(); + let old_wc_commit = mut_repo + .new_commit( + &settings, + vec![repo.store().root_commit_id().clone()], + repo.store().empty_merged_tree_id(), + ) + .write() + .unwrap(); + let ws_id = WorkspaceId::default(); + mut_repo.edit(ws_id.clone(), &old_wc_commit).unwrap(); + let other_ws_id = WorkspaceId::new("other".to_string()); + mut_repo.edit(other_ws_id.clone(), &old_wc_commit).unwrap(); + let repo = tx.commit("test"); + + let mut tx = repo.start_transaction(&settings); + let mut_repo = tx.mut_repo(); + let new_wc_commit = write_random_commit(mut_repo, &settings); + mut_repo.edit(ws_id, &new_wc_commit).unwrap(); + mut_repo.rebase_descendants(&settings).unwrap(); + assert!(mut_repo.view().heads().contains(old_wc_commit.id())); +} + #[test] fn test_edit_previous_empty_non_head() { // Test that MutableRepo::edit() does not abandon the previous commit if it