From 9f06bf8be52ec8458088a7fe6ea041e966c26b03 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Wed, 12 Jul 2023 14:35:40 +0900 Subject: [PATCH] repo: do not discard old working-copy commit if it's pointed by local branch It would be confusing if a branch moved backwards by checking out unrelated commit. #1854 --- lib/src/repo.rs | 17 +++++++++++++++-- lib/tests/test_mut_repo.rs | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/lib/src/repo.rs b/lib/src/repo.rs index 2d17176f7c..dc24e2558c 100644 --- a/lib/src/repo.rs +++ b/lib/src/repo.rs @@ -857,6 +857,13 @@ impl MutableRepo { workspace_id: WorkspaceId, commit: &Commit, ) -> Result<(), EditCommitError> { + fn local_branch_target_ids(view: &View) -> impl Iterator { + view.branches() + .values() + .filter_map(|branch_target| branch_target.local_target.as_ref()) + .flat_map(|target| target.adds()) + } + let maybe_wc_commit_id = self .view .with_ref(|v| v.get_wc_commit_id(&workspace_id).cloned()); @@ -865,8 +872,14 @@ impl MutableRepo { .store() .get_commit(&wc_commit_id) .map_err(EditCommitError::WorkingCopyCommitNotFound)?; - if wc_commit.is_discardable() && self.view().heads().contains(wc_commit.id()) { - // Abandon the working-copy commit we're leaving if it's empty and a head commit + if wc_commit.is_discardable() + && self + .view + .with_ref(|v| local_branch_target_ids(v).all(|id| id != 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. self.record_abandoned_commit(wc_commit_id); } } diff --git a/lib/tests/test_mut_repo.rs b/lib/tests/test_mut_repo.rs index 80174f6291..446f96a340 100644 --- a/lib/tests/test_mut_repo.rs +++ b/lib/tests/test_mut_repo.rs @@ -151,6 +151,41 @@ fn test_checkout_previous_empty_with_description(use_git: bool) { assert!(mut_repo.view().heads().contains(old_wc_commit.id())); } +#[test_case(false ; "local backend")] +#[test_case(true ; "git backend")] +fn test_checkout_previous_empty_with_local_branch(use_git: bool) { + // Test that MutableRepo::check_out() does not abandon the previous commit if it + // is pointed by local branch. + let settings = testutils::user_settings(); + let test_repo = TestRepo::init(use_git); + let repo = &test_repo.repo; + + let mut tx = repo.start_transaction(&settings, "test"); + 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_tree_id().clone(), + ) + .write() + .unwrap(); + mut_repo.set_local_branch( + "b".to_owned(), + RefTarget::Normal(old_wc_commit.id().clone()), + ); + let ws_id = WorkspaceId::default(); + mut_repo.edit(ws_id.clone(), &old_wc_commit).unwrap(); + let repo = tx.commit(); + + let mut tx = repo.start_transaction(&settings, "test"); + 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_case(false ; "local backend")] #[test_case(true ; "git backend")] fn test_checkout_previous_empty_non_head(use_git: bool) {