diff --git a/lib/src/conflicts.rs b/lib/src/conflicts.rs index a4940a67e..0e280640e 100644 --- a/lib/src/conflicts.rs +++ b/lib/src/conflicts.rs @@ -87,6 +87,11 @@ impl Conflict { &self.adds } + /// Whether this conflict is resolved. Does not resolve trivial conflicts. + pub fn is_resolved(&self) -> bool { + self.removes.is_empty() + } + /// Returns the resolved value, if this conflict is resolved. Does not /// resolve trivial conflicts. pub fn as_resolved(&self) -> Option<&T> { diff --git a/lib/src/merged_tree.rs b/lib/src/merged_tree.rs index 3201138ab..4418f4e19 100644 --- a/lib/src/merged_tree.rs +++ b/lib/src/merged_tree.rs @@ -16,18 +16,20 @@ use std::cmp::max; use std::sync::Arc; +use std::{iter, vec}; use itertools::Itertools; use crate::backend; -use crate::backend::TreeValue; +use crate::backend::{ConflictId, TreeValue}; use crate::conflicts::Conflict; -use crate::repo_path::{RepoPath, RepoPathComponent}; +use crate::repo_path::{RepoPath, RepoPathComponent, RepoPathJoin}; use crate::store::Store; use crate::tree::{try_resolve_file_conflict, Tree, TreeMergeError}; use crate::tree_builder::TreeBuilder; /// Presents a view of a merged set of trees. +#[derive(Clone, Debug)] pub enum MergedTree { /// A single tree, possibly with path-level conflicts. Legacy(Tree), @@ -130,6 +132,22 @@ impl MergedTree { )) } + /// This tree's directory + pub fn dir(&self) -> &RepoPath { + match self { + MergedTree::Legacy(tree) => tree.dir(), + MergedTree::Merge(conflict) => conflict.adds()[0].dir(), + } + } + + /// The `Store` associated with this tree. + pub fn store(&self) -> &Arc { + match self { + MergedTree::Legacy(tree) => tree.store(), + MergedTree::Merge(conflict) => conflict.adds()[0].store(), + } + } + /// The value at the given basename. The value can be `Resolved` even if /// `self` is a `Conflict`, which happens if the value at the path can be /// trivially merged. Does not recurse, so if `basename` refers to a Tree, @@ -167,6 +185,22 @@ impl MergedTree { MergedTree::Merge(conflict) => merge_trees(conflict), } } + + /// An iterator over the conflicts in this tree, including subtrees. + /// Recurses into subtrees and yields conflicts in those, but only if + /// all sides are trees, so tree/file conflicts will be reported as a single + /// conflict, not one for each path in the tree. + // TODO: Restrict this by a matcher (or add a separate method for that). + pub fn conflicts(&self) -> impl Iterator>)> { + ConflictIterator::new(self.clone()) + } +} + +fn all_tree_conflict_names(conflict: &Conflict) -> impl Iterator { + itertools::chain(conflict.removes(), conflict.adds()) + .map(|tree| tree.data().names()) + .kmerge() + .dedup() } fn merge_trees(conflict: &Conflict) -> Result, TreeMergeError> { @@ -174,11 +208,6 @@ fn merge_trees(conflict: &Conflict) -> Result, TreeMergeErr return Ok(Conflict::resolved(tree.clone())); } - let base_names = itertools::chain(conflict.removes(), conflict.adds()) - .map(|tree| tree.data().names()) - .kmerge() - .dedup(); - let base_tree = &conflict.adds()[0]; let store = base_tree.store(); let dir = base_tree.dir(); @@ -187,7 +216,7 @@ fn merge_trees(conflict: &Conflict) -> Result, TreeMergeErr // any conflicts. let mut new_tree = backend::Tree::default(); let mut conflicts = vec![]; - for basename in base_names { + for basename in all_tree_conflict_names(conflict) { let path_conflict = conflict.map(|tree| tree.value(basename).cloned()); let path_conflict = merge_tree_values(store, dir, path_conflict)?; if let Some(value) = path_conflict.as_resolved() { @@ -257,3 +286,137 @@ fn merge_tree_values( } } } + +struct ConflictEntriesNonRecursiveIterator<'a> { + merged_tree: &'a MergedTree, + basename_iter: Box + 'a>, +} + +impl<'a> ConflictEntriesNonRecursiveIterator<'a> { + fn new(merged_tree: &'a MergedTree) -> Self { + let basename_iter: Box + 'a> = match merged_tree + { + MergedTree::Legacy(tree) => Box::new( + tree.entries_non_recursive() + .filter(|entry| matches!(entry.value(), &TreeValue::Conflict(_))) + .map(|entry| entry.name()), + ), + MergedTree::Merge(conflict) => { + if conflict.is_resolved() { + Box::new(iter::empty()) + } else { + Box::new(all_tree_conflict_names(conflict)) + } + } + }; + ConflictEntriesNonRecursiveIterator { + merged_tree, + basename_iter, + } + } +} + +impl<'a> Iterator for ConflictEntriesNonRecursiveIterator<'a> { + type Item = (&'a RepoPathComponent, Conflict>); + + fn next(&mut self) -> Option { + for basename in self.basename_iter.by_ref() { + match self.merged_tree.value(basename) { + MergedTreeValue::Resolved(_) => {} + MergedTreeValue::Conflict(conflict) => { + return Some((basename, conflict)); + } + } + } + None + } +} + +/// The state for the non-recursive iteration over the conflicted entries in a +/// single directory. +struct ConflictsDirItem { + entry_iterator: ConflictEntriesNonRecursiveIterator<'static>, + // On drop, tree must outlive entry_iterator + tree: Box, +} + +impl ConflictsDirItem { + fn new(tree: MergedTree) -> Self { + // Put the tree in a box so it doesn't move if `ConflictsDirItem` moves. + let tree = Box::new(tree); + let entry_iterator = ConflictEntriesNonRecursiveIterator::new(&tree); + let entry_iterator: ConflictEntriesNonRecursiveIterator<'static> = + unsafe { std::mem::transmute(entry_iterator) }; + Self { + entry_iterator, + tree, + } + } +} + +enum ConflictIterator { + Legacy { + store: Arc, + conflicts_iter: vec::IntoIter<(RepoPath, ConflictId)>, + }, + Merge { + stack: Vec, + }, +} + +impl ConflictIterator { + fn new(tree: MergedTree) -> Self { + match tree { + MergedTree::Legacy(tree) => ConflictIterator::Legacy { + store: tree.store().clone(), + conflicts_iter: tree.conflicts().into_iter(), + }, + MergedTree::Merge(_) => ConflictIterator::Merge { + stack: vec![ConflictsDirItem::new(tree)], + }, + } + } +} + +impl Iterator for ConflictIterator { + type Item = (RepoPath, Conflict>); + + fn next(&mut self) -> Option { + match self { + ConflictIterator::Legacy { + store, + conflicts_iter, + } => { + if let Some((path, conflict_id)) = conflicts_iter.next() { + // TODO: propagate errors + let conflict = store.read_conflict(&path, &conflict_id).unwrap(); + Some((path, conflict)) + } else { + None + } + } + ConflictIterator::Merge { stack } => { + while let Some(top) = stack.last_mut() { + if let Some((basename, conflict)) = top.entry_iterator.next() { + let path = top.tree.dir().join(basename); + // TODO: propagate errors + if let Some(tree_conflict) = + conflict.to_tree_conflict(top.tree.store(), &path).unwrap() + { + // If all sides are trees or missing, descend into the merged tree + stack.push(ConflictsDirItem::new(MergedTree::Merge(tree_conflict))); + } else { + // Otherwise this is a conflict between files, trees, etc. If they could + // be automatically resolved, they should have been when the top-level + // tree conflict was written, so we assume that they can't be. + return Some((path, conflict)); + } + } else { + stack.pop(); + } + } + None + } + } + } +} diff --git a/lib/tests/test_merged_tree.rs b/lib/tests/test_merged_tree.rs index ff46981d6..fe3dfbca4 100644 --- a/lib/tests/test_merged_tree.rs +++ b/lib/tests/test_merged_tree.rs @@ -18,6 +18,7 @@ use jj_lib::conflicts::Conflict; use jj_lib::merged_tree::{MergedTree, MergedTreeValue}; use jj_lib::repo::Repo; use jj_lib::repo_path::{RepoPath, RepoPathComponent, RepoPathJoin}; +use jj_lib::tree::merge_trees; use testutils::{write_file, write_normal_file, TestRepo}; fn file_value(file_id: &FileId) -> TreeValue { @@ -293,3 +294,195 @@ fn test_resolve_with_conflict() { Conflict::new(vec![expected_base1], vec![expected_side1, expected_side2]) ) } + +#[test] +fn test_conflict_iterator() { + let test_repo = TestRepo::init(true); + let repo = &test_repo.repo; + + let unchanged_path = RepoPath::from_internal_string("dir/subdir/unchanged"); + let trivial_path = RepoPath::from_internal_string("dir/subdir/trivial"); + let trivial_hunk_path = RepoPath::from_internal_string("dir/non_trivial"); + let file_conflict_path = RepoPath::from_internal_string("dir/subdir/file_conflict"); + let modify_delete_path = RepoPath::from_internal_string("dir/subdir/modify_delete"); + let same_add_path = RepoPath::from_internal_string("dir/subdir/same_add"); + let different_add_path = RepoPath::from_internal_string("dir/subdir/different_add"); + let dir_file_path = RepoPath::from_internal_string("dir/subdir/dir_file"); + let added_dir_path = RepoPath::from_internal_string("dir/new_dir"); + let modify_delete_dir_path = RepoPath::from_internal_string("dir/modify_delete_dir"); + let base1 = testutils::create_tree( + repo, + &[ + (&unchanged_path, "unchanged"), + (&trivial_path, "base"), + (&trivial_hunk_path, "line1\nline2\nline3\n"), + (&file_conflict_path, "base"), + (&modify_delete_path, "base"), + // no same_add_path + // no different_add_path + (&dir_file_path, "base"), + // no added_dir_path + ( + &modify_delete_dir_path.join(&RepoPathComponent::from("base")), + "base", + ), + ], + ); + let side1 = testutils::create_tree( + repo, + &[ + (&unchanged_path, "unchanged"), + (&trivial_path, "base"), + (&file_conflict_path, "side1"), + (&trivial_hunk_path, "line1 side1\nline2\nline3\n"), + (&modify_delete_path, "modified"), + (&same_add_path, "same"), + (&different_add_path, "side1"), + (&dir_file_path, "side1"), + ( + &added_dir_path.join(&RepoPathComponent::from("side1")), + "side1", + ), + ( + &modify_delete_dir_path.join(&RepoPathComponent::from("side1")), + "side1", + ), + ], + ); + let side2 = testutils::create_tree( + repo, + &[ + (&unchanged_path, "unchanged"), + (&trivial_path, "side2"), + (&file_conflict_path, "side2"), + (&trivial_hunk_path, "line1\nline2\nline3 side2\n"), + // no modify_delete_path + (&same_add_path, "same"), + (&different_add_path, "side2"), + (&dir_file_path.join(&RepoPathComponent::from("dir")), "new"), + ( + &added_dir_path.join(&RepoPathComponent::from("side2")), + "side2", + ), + // no modify_delete_dir_path + ], + ); + + let tree = MergedTree::new(Conflict::new( + vec![base1.clone()], + vec![side1.clone(), side2.clone()], + )); + let conflicts = tree.conflicts().collect_vec(); + let conflict_at = |path: &RepoPath| { + Conflict::new( + vec![base1.path_value(path)], + vec![side1.path_value(path), side2.path_value(path)], + ) + }; + // We initially also get a conflict in trivial_hunk_path because we had + // forgotten to resolve conflicts + assert_eq!( + conflicts, + vec![ + (trivial_hunk_path.clone(), conflict_at(&trivial_hunk_path)), + (different_add_path.clone(), conflict_at(&different_add_path)), + (dir_file_path.clone(), conflict_at(&dir_file_path)), + (file_conflict_path.clone(), conflict_at(&file_conflict_path)), + (modify_delete_path.clone(), conflict_at(&modify_delete_path)), + ] + ); + + // After we resolve conflicts, there are only non-trivial conflicts left + let tree = MergedTree::Merge(tree.resolve().unwrap()); + let conflicts = tree.conflicts().collect_vec(); + assert_eq!( + conflicts, + vec![ + (different_add_path.clone(), conflict_at(&different_add_path)), + (dir_file_path.clone(), conflict_at(&dir_file_path)), + (file_conflict_path.clone(), conflict_at(&file_conflict_path)), + (modify_delete_path.clone(), conflict_at(&modify_delete_path)), + ] + ); + + let merged_legacy_tree = merge_trees(&side1, &base1, &side2).unwrap(); + let legacy_conflicts = MergedTree::legacy(merged_legacy_tree) + .conflicts() + .collect_vec(); + assert_eq!(legacy_conflicts, conflicts); +} +#[test] +fn test_conflict_iterator_higher_arity() { + let test_repo = TestRepo::init(true); + let repo = &test_repo.repo; + + let two_sided_path = RepoPath::from_internal_string("dir/2-sided"); + let three_sided_path = RepoPath::from_internal_string("dir/3-sided"); + let base1 = testutils::create_tree( + repo, + &[(&two_sided_path, "base1"), (&three_sided_path, "base1")], + ); + let base2 = testutils::create_tree( + repo, + &[(&two_sided_path, "base2"), (&three_sided_path, "base2")], + ); + let side1 = testutils::create_tree( + repo, + &[(&two_sided_path, "side1"), (&three_sided_path, "side1")], + ); + let side2 = testutils::create_tree( + repo, + &[(&two_sided_path, "base1"), (&three_sided_path, "side2")], + ); + let side3 = testutils::create_tree( + repo, + &[(&two_sided_path, "side3"), (&three_sided_path, "side3")], + ); + + let tree = MergedTree::new(Conflict::new( + vec![base1.clone(), base2.clone()], + vec![side1.clone(), side2.clone(), side3.clone()], + )); + let conflicts = tree.conflicts().collect_vec(); + let conflict_at = |path: &RepoPath| { + Conflict::new( + vec![base1.path_value(path), base2.path_value(path)], + vec![ + side1.path_value(path), + side2.path_value(path), + side3.path_value(path), + ], + ) + }; + // Both paths have the full, unsimplified conflict (3-sided) + assert_eq!( + conflicts, + vec![ + (two_sided_path.clone(), conflict_at(&two_sided_path)), + (three_sided_path.clone(), conflict_at(&three_sided_path)) + ] + ); + // Iterating over conflicts in a legacy tree yields the simplified conflict at + // each path + let merged_legacy_tree = merge_trees(&side1, &base1, &side2).unwrap(); + let merged_legacy_tree = merge_trees(&merged_legacy_tree, &base2, &side3).unwrap(); + let legacy_conflicts = MergedTree::legacy(merged_legacy_tree) + .conflicts() + .collect_vec(); + assert_eq!( + legacy_conflicts, + vec![ + ( + two_sided_path.clone(), + Conflict::new( + vec![base2.path_value(&two_sided_path)], + vec![ + side1.path_value(&two_sided_path), + side3.path_value(&two_sided_path), + ], + ) + ), + (three_sided_path.clone(), conflict_at(&three_sided_path)) + ] + ); +}