ok/jj
1
0
Fork 0
forked from mirrors/jj
Code Pull requests Activity

rewrite: don't rewrite the "removed" side of a branch conflict

Browse source 
Let's say we have a simple history like this:

```
B C D
 \|/
  A
```

Branch `main` initially points to commit B. Two concurrent operations
then move the branch to commits C and D. When the two concurrent
operations get merged, the branch will be recorded as pointing to
"C+D-B". If a subsequent operation now abandons commit B, we would
update the "removed" side of the branch conflict. That seems a little
dishonest. I think the reason I did it that way was in order to not
keep B visible back when having it present in the "removed" side would
keep it visible (before 33bf6ce1d5).

I noticed this issue while working on #241 because
`test_import_refs_reimport()` started failing. That test case is
pretty much exactly the case above.
This commit is contained in:
Martin von Zweigbergk 2022-03-26 22:33:08 -07:00 committed by Martin von Zweigbergk
parent aff2293e1d
commit e3254fa5c4
2 changed files with 10 additions and 30 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

29
lib/src/rewrite.rs
View file

@ -305,33 +305,18 @@ impl<'settings, 'repo> DescendantRebaser<'settings, 'repo> {
let local_target = self.mut_repo.get_local_branch(branch_name).unwrap();
for old_add in local_target.adds() {
if old_add == old_commit_id {
branch_updates.push((branch_name.clone(), true));
}
}
for old_add in local_target.removes() {
if old_add == old_commit_id {
// Arguments reversed for removes
branch_updates.push((branch_name.clone(), false));
branch_updates.push(branch_name.clone());
}
}
}
let (old_target, new_target) =
DescendantRebaser::ref_target_update(old_commit_id.clone(), new_commit_ids);
for (branch_name, is_add) in branch_updates {
if is_add {
self.mut_repo.merge_single_ref(
&RefName::LocalBranch(branch_name),
Some(&old_target),
Some(&new_target),
);
} else {
// Arguments reversed for removes
self.mut_repo.merge_single_ref(
&RefName::LocalBranch(branch_name),
Some(&new_target),
Some(&old_target),
);
}
for branch_name in branch_updates {
self.mut_repo.merge_single_ref(
&RefName::LocalBranch(branch_name),
Some(&old_target),
Some(&new_target),
);
}
}

11
lib/tests/test_rewrite.rs
View file

@ -1122,9 +1122,9 @@ fn test_rebase_descendants_rewrite_updates_branch_conflict() {
let test_repo = testutils::init_repo(&settings, false);
let repo = &test_repo.repo;
// Branch "main" is a conflict removing commit A and adding commit B and C.
// Branch "main" is a conflict removing commit A and adding commits B and C.
// A gets rewritten as A2 and A3. B gets rewritten as B2 and B2. The branch
// should become a conflict removing A2, A3, and B, and adding A, B2, B3, C.
// should become a conflict removing A and B, and adding B2, B3, C.
let mut tx = repo.start_transaction("test");
let mut graph_builder = CommitGraphBuilder::new(&settings, tx.mut_repo());
let commit_a = graph_builder.initial_commit();
@ -1156,14 +1156,9 @@ fn test_rebase_descendants_rewrite_updates_branch_conflict() {
assert_eq!(
tx.mut_repo().get_local_branch("main"),
Some(RefTarget::Conflict {
removes: vec![
commit_a2.id().clone(),
commit_a3.id().clone(),
commit_b.id().clone(),
],
removes: vec![commit_a.id().clone(), commit_b.id().clone()],
adds: vec![
commit_c.id().clone(),
commit_a.id().clone(),
commit_b2.id().clone(),
commit_b3.id().clone(),
]