From c9e147c425082491e674bcd24a0679123ca85742 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Fri, 9 Aug 2024 01:36:57 +0900 Subject: [PATCH] merged_tree: allow to postpone resolution of intermediate trees This allows us to diff trees without fully resolving conflicts: let from_tree = merge_no_resolve(..); for (path, (from, to)) in from_tree.diff(to_tree, matcher) { let from = resolve_conflicts(from); if from == to { continue; // resolved file may be identical ... 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 | 6 ++++-- lib/src/merged_tree.rs | 14 ++++++++++---- lib/src/rewrite.rs | 15 +++++++++++---- 3 files changed, 25 insertions(+), 10 deletions(-) diff --git a/lib/src/default_index/revset_engine.rs b/lib/src/default_index/revset_engine.rs index 7b0a79db1..a9609944c 100644 --- a/lib/src/default_index/revset_engine.rs +++ b/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 { 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 diff --git a/lib/src/merged_tree.rs b/lib/src/merged_tree.rs index f83acf77a..e5c053bea 100644 --- a/lib/src/merged_tree.rs +++ b/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 { + 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() + } } } diff --git a/lib/src/rewrite.rs b/lib/src/rewrite.rs index 5e6f913a6..3a58defef 100644 --- a/lib/src/rewrite.rs +++ b/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 { - 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, 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) }