Skip to content

Commit

Permalink
repo: do not discard old working-copy commit if it's pointed by local…
Browse files Browse the repository at this point in the history
… branch

It would be confusing if a branch moved backwards by checking out unrelated
commit.

jj-vcs#1854
  • Loading branch information
yuja committed Jul 12, 2023
1 parent 78e9a1b commit b63e3b0
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 2 deletions.
17 changes: 15 additions & 2 deletions lib/src/repo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -857,6 +857,13 @@ impl MutableRepo {
workspace_id: WorkspaceId,
commit: &Commit,
) -> Result<(), EditCommitError> {
fn local_branch_target_ids(view: &View) -> impl Iterator<Item = &CommitId> {
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());
Expand All @@ -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);
}
}
Expand Down
35 changes: 35 additions & 0 deletions lib/tests/test_mut_repo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit b63e3b0

Please sign in to comment.