ok/jj
1
0
Fork 0
forked from mirrors/jj

merged_tree: do not propagate conflicting empty tree value to parent

Otherwise an empty subtree would be added to the parent tree.

If the stored tree contained an empty subtree, simplify() wouldn't work
against new "absent" subtree representation. I don't know if there's a
such code path, but I believe it's very rare to encounter the problem.

#2654
This commit is contained in:
Yuya Nishihara 2023-12-02 13:44:30 +09:00
parent 1db033504c
commit 4ffbf40c82
2 changed files with 21 additions and 5 deletions

View file

@ -537,12 +537,10 @@ fn merge_tree_values(
if let Some(trees) = values.to_tree_merge(store, path)? {
// If all sides are trees or missing, merge the trees recursively, treating
// missing trees as empty.
let empty_tree_id = store.empty_tree_id();
let merged_tree = merge_trees(&trees)?;
if merged_tree.as_resolved().map(|tree| tree.id()) == Some(store.empty_tree_id()) {
Ok(Merge::absent())
} else {
Ok(merged_tree.map(|tree| Some(TreeValue::Tree(tree.id().clone()))))
}
Ok(merged_tree
.map(|tree| (tree.id() != empty_tree_id).then(|| TreeValue::Tree(tree.id().clone()))))
} else {
// Try to resolve file conflicts by merging the file contents. Treats missing
// files as empty.

View file

@ -490,6 +490,24 @@ fn test_resolve_with_conflict() {
)
}
#[test]
fn test_resolve_with_conflict_containing_empty_subtree() {
let test_repo = TestRepo::init();
let repo = &test_repo.repo;
// Since "dir" in side2 is absent, the root tree should be empty as well.
// If it were added to the root tree, side2.id() would differ.
let conflict_path = RepoPath::from_internal_string("dir/file_conflict");
let base1 = create_single_tree(repo, &[(conflict_path, "base1")]);
let side1 = create_single_tree(repo, &[(conflict_path, "side1")]);
let side2 = create_single_tree(repo, &[]);
let original_tree = Merge::from_removes_adds(vec![base1], vec![side1, side2]);
let tree = MergedTree::new(original_tree.clone());
let resolved_tree = tree.resolve().unwrap();
assert_eq!(resolved_tree, original_tree);
}
#[test]
fn test_conflict_iterator() {
let test_repo = TestRepo::init();