merged_tree: remove canceling terms prior to resolving file-level con…

…flict

I think this is a variant of the problem fixed by 7fda80f "tree: simplify
conflict before resolving at hunk level." We need to simplify() the conflict
before and after extracting file ids because the source conflict values may
contain trees to be cancelled out, and the file values may differ only in exec
bits. Since the legacy tree passes a simplified conflict in to this function,
I made the merged tree do the same.

Fixes #2654

CHANGELOG.md

@@ -37,6 +37,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed bugs
 
+* Fixed another file conflict resolution issue where `jj status` would disagree
+  with the actual file content.
+  [#2654](https://github.com/martinvonz/jj/issues/2654)
+
 
 ## [0.11.0] - 2023-11-01
 

lib/src/merged_tree.rs

@@ -543,8 +543,12 @@ fn merge_tree_values(
             .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.
-        if let Some(resolved) = try_resolve_file_conflict(store, path, &values)? {
+        // files as empty. The values may contain trees canceling each other (notably
+        // padded absent trees), so we need to simplify them first.
+        let simplified = values.clone().simplify();
+        // No fast path for simplified.is_resolved(). If it could be resolved, it would
+        // have been caught by values.resolve_trivial() above.
+        if let Some(resolved) = try_resolve_file_conflict(store, path, &simplified)? {
             Ok(Merge::normal(resolved))
         } else {
             // Failed to merge the files, or the paths are not files

lib/src/tree.rs

@@ -396,6 +396,10 @@ fn merge_tree_value(
     })
 }
 
+/// Resolves file-level conflict by merging content hunks.
+///
+/// The input `conflict` is supposed to be simplified. It shouldn't contain
+/// non-file values that cancel each other.
 pub fn try_resolve_file_conflict(
     store: &Store,
     filename: &RepoPath,
@@ -430,7 +434,9 @@ pub fn try_resolve_file_conflict(
         }));
     }
 
-    // Simplify the conflict for two reasons:
+    // While the input conflict should be simplified by caller, it might contain
+    // terms which only differ in executable bits. Simplify the conflict further
+    // for two reasons:
     // 1. Avoid reading unchanged file contents
     // 2. The simplified conflict can sometimes be resolved when the unsimplfied one
     //    cannot

lib/tests/test_merged_tree.rs

@@ -1455,3 +1455,34 @@ fn test_merge_simplify_file_conflict() {
         MergeResult::Conflict(_)
     ));
 }
+
+/// Like `test_merge_simplify_file_conflict()`, but some of the conflicts are
+/// absent.
+#[test]
+fn test_merge_simplify_file_conflict_with_absent() {
+    let test_repo = TestRepo::init();
+    let repo = &test_repo.repo;
+
+    // conflict_path doesn't exist in parent and child2_left, and these two
+    // trees can't be canceled out at the root level. Still the file merge
+    // should succeed by eliminating absent entries.
+    let child2_path = RepoPath::from_internal_string("file_child2");
+    let conflict_path = RepoPath::from_internal_string("dir/file_conflict");
+    let child1 = create_single_tree(repo, &[(conflict_path, "1\n0\n")]);
+    let parent = create_single_tree(repo, &[]);
+    let child2_left = create_single_tree(repo, &[(child2_path, "")]);
+    let child2_base = create_single_tree(repo, &[(child2_path, ""), (conflict_path, "0\n")]);
+    let child2_right = create_single_tree(repo, &[(child2_path, ""), (conflict_path, "0\n2\n")]);
+    let child1_merged = MergedTree::resolved(child1);
+    let parent_merged = MergedTree::resolved(parent);
+    let child2_merged = MergedTree::new(Merge::from_removes_adds(
+        vec![child2_base],
+        vec![child2_left, child2_right],
+    ));
+
+    let expected = create_single_tree(repo, &[(child2_path, ""), (conflict_path, "1\n0\n2\n")]);
+    let expected_merged = MergedTree::resolved(expected);
+
+    let merged = child1_merged.merge(&parent_merged, &child2_merged).unwrap();
+    assert_eq!(merged, expected_merged);
+}