merged_tree: split function that merges trees without resolving confl…

…icts

This allows us to resolve conflicts of matching files only. I originally
considered adding a matcher argument to merge() functions, but the resulting
API looked misleading. If merge() took a matcher, callers might expect unmatched
trees and files were omitted, not left unresolved. It's also slower than diffing
unresolved trees because merge(.., matcher) would have to write partially
resolved trees to the store.

Since "ancestor_tree" isn't resolved by itself, this patch has subtle behavior
change. For example, "jj diff -r9eaef582" in the "git" repository is no longer
empty. I think the new behavior is also technically correct, but I'm not pretty
sure.

lib/src/default_index/revset_engine.rs

@@ -1142,7 +1142,8 @@ fn has_diff_from_parent(
             return Ok(false);
         }
     }
-    let from_tree = rewrite::merge_commit_trees_without_repo(store, &index, &parents)?;
+    let from_tree =
+        rewrite::merge_commit_trees_no_resolve_without_repo(store, &index, &parents)?.resolve()?;
     let to_tree = commit.tree()?;
     let mut tree_diff = from_tree.diff_stream(&to_tree, matcher);
     async {
@@ -1163,7 +1164,8 @@ fn matches_diff_from_parent(
     files_matcher: &dyn Matcher,
 ) -> BackendResult<bool> {
     let parents: Vec<_> = commit.parents().try_collect()?;
-    let from_tree = rewrite::merge_commit_trees_without_repo(store, &index, &parents)?;
+    let from_tree =
+        rewrite::merge_commit_trees_no_resolve_without_repo(store, &index, &parents)?.resolve()?;
     let to_tree = commit.tree()?;
     let tree_diff = from_tree.diff_stream(&to_tree, files_matcher);
     // Conflicts are compared in materialized form. Alternatively, conflict

lib/src/merged_tree.rs

@@ -301,17 +301,23 @@ impl MergedTree {
         }
     }
 
-    /// Merges this tree with `other`, using `base` as base.
+    /// Merges this tree with `other`, using `base` as base. Any conflicts will
+    /// be resolved recursively if possible.
     pub fn merge(&self, base: &MergedTree, other: &MergedTree) -> BackendResult<MergedTree> {
+        self.merge_no_resolve(base, other).resolve()
+    }
+
+    /// Merges this tree with `other`, using `base` as base, without attempting
+    /// to resolve file conflicts.
+    pub fn merge_no_resolve(&self, base: &MergedTree, other: &MergedTree) -> MergedTree {
         let nested = Merge::from_vec(vec![
             self.trees.clone(),
             base.trees.clone(),
             other.trees.clone(),
         ]);
-        let flattened = MergedTree {
+        MergedTree {
             trees: nested.flatten().simplify(),
-        };
-        flattened.resolve()
+        }
     }
 }
 

lib/src/rewrite.rs

@@ -33,13 +33,19 @@ use crate::repo_path::RepoPath;
 use crate::settings::UserSettings;
 use crate::store::Store;
 
+/// Merges `commits` and tries to resolve any conflicts recursively.
 #[instrument(skip(repo))]
 pub fn merge_commit_trees(repo: &dyn Repo, commits: &[Commit]) -> BackendResult<MergedTree> {
-    merge_commit_trees_without_repo(repo.store(), repo.index(), commits)
+    if let [commit] = commits {
+        commit.tree()
+    } else {
+        merge_commit_trees_no_resolve_without_repo(repo.store(), repo.index(), commits)?.resolve()
+    }
 }
 
+/// Merges `commits` without attempting to resolve file conflicts.
 #[instrument(skip(index))]
-pub fn merge_commit_trees_without_repo(
+pub fn merge_commit_trees_no_resolve_without_repo(
     store: &Arc<Store>,
     index: &dyn Index,
     commits: &[Commit],
@@ -58,9 +64,10 @@ pub fn merge_commit_trees_without_repo(
                 .iter()
                 .map(|id| store.get_commit(id))
                 .try_collect()?;
-            let ancestor_tree = merge_commit_trees_without_repo(store, index, &ancestors)?;
+            let ancestor_tree =
+                merge_commit_trees_no_resolve_without_repo(store, index, &ancestors)?;
             let other_tree = other_commit.tree()?;
-            new_tree = new_tree.merge(&ancestor_tree, &other_tree)?;
+            new_tree = new_tree.merge_no_resolve(&ancestor_tree, &other_tree);
         }
         Ok(new_tree)
     }

lib/tests/test_revset.rs

@@ -3315,6 +3315,80 @@ fn test_evaluate_expression_diff_contains() {
     );
 }
 
+#[test]
+fn test_evaluate_expression_file_merged_parents() {
+    let settings = testutils::user_settings();
+    let test_workspace = TestWorkspace::init(&settings);
+    let repo = &test_workspace.repo;
+
+    let mut tx = repo.start_transaction(&settings);
+    let mut_repo = tx.mut_repo();
+
+    // file2 can be merged automatically, file1 can't.
+    let file_path1 = RepoPath::from_internal_string("file1");
+    let file_path2 = RepoPath::from_internal_string("file2");
+    let tree1 = create_tree(repo, &[(file_path1, "1\n"), (file_path2, "1\n")]);
+    let tree2 = create_tree(repo, &[(file_path1, "1\n2\n"), (file_path2, "2\n1\n")]);
+    let tree3 = create_tree(repo, &[(file_path1, "1\n3\n"), (file_path2, "1\n3\n")]);
+    let tree4 = create_tree(repo, &[(file_path1, "1\n4\n"), (file_path2, "2\n1\n3\n")]);
+
+    let mut create_commit = |parent_ids, tree_id| {
+        mut_repo
+            .new_commit(&settings, parent_ids, tree_id)
+            .write()
+            .unwrap()
+    };
+    let commit1 = create_commit(vec![repo.store().root_commit_id().clone()], tree1.id());
+    let commit2 = create_commit(vec![commit1.id().clone()], tree2.id());
+    let commit3 = create_commit(vec![commit1.id().clone()], tree3.id());
+    let commit4 = create_commit(vec![commit2.id().clone(), commit3.id().clone()], tree4.id());
+
+    let query = |revset_str: &str| {
+        resolve_commit_ids_in_workspace(
+            mut_repo,
+            revset_str,
+            &test_workspace.workspace,
+            Some(test_workspace.workspace.workspace_root()),
+        )
+    };
+
+    assert_eq!(
+        query("file('file1')"),
+        vec![
+            commit4.id().clone(),
+            commit3.id().clone(),
+            commit2.id().clone(),
+            commit1.id().clone(),
+        ]
+    );
+    assert_eq!(
+        query("file('file2')"),
+        vec![
+            commit3.id().clone(),
+            commit2.id().clone(),
+            commit1.id().clone(),
+        ]
+    );
+
+    assert_eq!(
+        query("diff_contains(regex:'[1234]', 'file1')"),
+        vec![
+            commit4.id().clone(),
+            commit3.id().clone(),
+            commit2.id().clone(),
+            commit1.id().clone(),
+        ]
+    );
+    assert_eq!(
+        query("diff_contains(regex:'[1234]', 'file2')"),
+        vec![
+            commit3.id().clone(),
+            commit2.id().clone(),
+            commit1.id().clone(),
+        ]
+    );
+}
+
 #[test]
 fn test_evaluate_expression_conflict() {
     let settings = testutils::user_settings();