From 0655231958ef16660775f3e6c427fd3d121b4c11 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Sat, 17 Aug 2024 14:18:24 -0700 Subject: [PATCH] merged_tree: remove `TreeDiffEntry::source` --- cli/src/commands/fix.rs | 3 +- cli/src/merge_tools/builtin.rs | 8 +---- cli/src/merge_tools/diff_working_copies.rs | 2 +- lib/src/copies.rs | 8 ++--- lib/src/default_index/revset_engine.rs | 12 ++++---- lib/src/local_working_copy.rs | 30 ++++++------------- lib/src/merged_tree.rs | 33 +++++++-------------- lib/src/rewrite.rs | 3 +- lib/tests/test_commit_builder.rs | 2 +- lib/tests/test_local_working_copy_sparse.rs | 6 ++-- lib/tests/test_merged_tree.rs | 6 ++-- 11 files changed, 41 insertions(+), 72 deletions(-) diff --git a/cli/src/commands/fix.rs b/cli/src/commands/fix.rs index 7e223d6091..9bbd6bc63d 100644 --- a/cli/src/commands/fix.rs +++ b/cli/src/commands/fix.rs @@ -176,8 +176,7 @@ pub(crate) fn cmd_fix( let mut diff_stream = parent_tree.diff_stream(&tree, &matcher); async { while let Some(TreeDiffEntry { - source: _, // TODO handle copy tracking - target: repo_path, + path: repo_path, value: diff, }) = diff_stream.next().await { diff --git a/cli/src/merge_tools/builtin.rs b/cli/src/merge_tools/builtin.rs index 52cbe35ced..5aa8cc5e85 100644 --- a/cli/src/merge_tools/builtin.rs +++ b/cli/src/merge_tools/builtin.rs @@ -497,13 +497,7 @@ pub fn edit_diff_builtin( // TODO: handle copy tracking let changed_files: Vec<_> = left_tree .diff_stream(right_tree, matcher) - .map( - |TreeDiffEntry { - source: _, // TODO handle copy tracking - target: path, - value: diff, - }| diff.map(|_| path), - ) + .map(|TreeDiffEntry { path, value: diff }| diff.map(|_| path)) .try_collect() .block_on()?; let files = make_diff_files(&store, left_tree, right_tree, &changed_files)?; diff --git a/cli/src/merge_tools/diff_working_copies.rs b/cli/src/merge_tools/diff_working_copies.rs index 1e6d43c38a..61a2777a95 100644 --- a/cli/src/merge_tools/diff_working_copies.rs +++ b/cli/src/merge_tools/diff_working_copies.rs @@ -132,7 +132,7 @@ pub(crate) fn check_out_trees( ) -> Result { let changed_files: Vec<_> = left_tree .diff_stream(right_tree, matcher) - .map(|TreeDiffEntry { target, .. }| target) + .map(|TreeDiffEntry { path, .. }| path) .collect() .block_on(); diff --git a/lib/src/copies.rs b/lib/src/copies.rs index 7c5b6d7e49..604623d669 100644 --- a/lib/src/copies.rs +++ b/lib/src/copies.rs @@ -109,18 +109,18 @@ impl Stream for CopiesTreeDiffStream<'_> { self.inner.as_mut().poll_next(cx).map(|option| { option.map(|diff_entry| { let Some(CopyRecord { source, .. }) = - self.copy_records.for_target(&diff_entry.target) + self.copy_records.for_target(&diff_entry.path) else { return CopiesTreeDiffEntry { - source: diff_entry.source, - target: diff_entry.target, + source: diff_entry.path.clone(), + target: diff_entry.path, value: diff_entry.value, }; }; CopiesTreeDiffEntry { source: source.clone(), - target: diff_entry.target, + target: diff_entry.path, value: diff_entry.value.and_then(|(_, target_value)| { self.source_tree .path_value(source) diff --git a/lib/src/default_index/revset_engine.rs b/lib/src/default_index/revset_engine.rs index 6852290fdb..7be9766ed3 100644 --- a/lib/src/default_index/revset_engine.rs +++ b/lib/src/default_index/revset_engine.rs @@ -1152,7 +1152,7 @@ fn has_diff_from_parent( async { while let Some(entry) = tree_diff.next().await { let (from_value, to_value) = entry.value?; - let from_value = resolve_file_values(store, &entry.source, from_value)?; + let from_value = resolve_file_values(store, &entry.path, from_value)?; if from_value == to_value { continue; } @@ -1179,17 +1179,17 @@ fn matches_diff_from_parent( async { while let Some(entry) = tree_diff.next().await { let (left_value, right_value) = entry.value?; - let left_value = resolve_file_values(store, &entry.source, left_value)?; + let left_value = resolve_file_values(store, &entry.path, left_value)?; if left_value == right_value { continue; } // Conflicts are compared in materialized form. Alternatively, // conflict pairs can be compared one by one. #4062 - let left_future = materialize_tree_value(store, &entry.source, left_value); - let right_future = materialize_tree_value(store, &entry.target, right_value); + let left_future = materialize_tree_value(store, &entry.path, left_value); + let right_future = materialize_tree_value(store, &entry.path, right_value); let (left_value, right_value) = futures::try_join!(left_future, right_future)?; - let left_content = to_file_content(&entry.source, left_value)?; - let right_content = to_file_content(&entry.target, right_value)?; + let left_content = to_file_content(&entry.path, left_value)?; + let right_content = to_file_content(&entry.path, right_value)?; // Filter lines prior to comparison. This might produce inferior // hunks due to lack of contexts, but is way faster than full diff. let left_lines = match_lines(&left_content, text_pattern); diff --git a/lib/src/local_working_copy.rs b/lib/src/local_working_copy.rs index 45c3818f30..a097955a4d 100644 --- a/lib/src/local_working_copy.rs +++ b/lib/src/local_working_copy.rs @@ -1353,22 +1353,15 @@ impl TreeState { let mut diff_stream = Box::pin( old_tree .diff_stream(new_tree, matcher) - .map( - |TreeDiffEntry { - source: _, // TODO handle copy tracking - target: path, - value: diff, - }| async { - match diff { - Ok((before, after)) => { - let result = - materialize_tree_value(&self.store, &path, after).await; - (path, result.map(|value| (before.is_present(), value))) - } - Err(err) => (path, Err(err)), + .map(|TreeDiffEntry { path, value: diff }| async { + match diff { + Ok((before, after)) => { + let result = materialize_tree_value(&self.store, &path, after).await; + (path, result.map(|value| (before.is_present(), value))) } - }, - ) + Err(err) => (path, Err(err)), + } + }) .buffered(self.store.concurrency().max(1)), ); while let Some((path, data)) = diff_stream.next().await { @@ -1454,12 +1447,7 @@ impl TreeState { let mut changed_file_states = Vec::new(); let mut deleted_files = HashSet::new(); let mut diff_stream = old_tree.diff_stream(new_tree, matcher.as_ref()); - while let Some(TreeDiffEntry { - source: _, // TODO handle copy tracking - target: path, - value: diff, - }) = diff_stream.next().await - { + while let Some(TreeDiffEntry { path, value: diff }) = diff_stream.next().await { let (_before, after) = diff?; if after.is_absent() { deleted_files.insert(path); diff --git a/lib/src/merged_tree.rs b/lib/src/merged_tree.rs index 8e36752d83..efbf6f8280 100644 --- a/lib/src/merged_tree.rs +++ b/lib/src/merged_tree.rs @@ -320,10 +320,8 @@ impl MergedTree { /// A single entry in a tree diff. pub struct TreeDiffEntry { - /// The source path. - pub source: RepoPathBuf, - /// The target path. - pub target: RepoPathBuf, + /// The path. + pub path: RepoPathBuf, /// The resolved tree values if available. pub value: BackendResult<(MergedTreeValue, MergedTreeValue)>, } @@ -760,8 +758,7 @@ impl Iterator for TreeDiffIterator<'_> { TreeDiffItem::File(..) => { if let TreeDiffItem::File(path, before, after) = self.stack.pop().unwrap() { return Some(TreeDiffEntry { - source: path.clone(), - target: path, + path, value: Ok((before, after)), }); } else { @@ -780,15 +777,13 @@ impl Iterator for TreeDiffIterator<'_> { (Ok(before_tree), Ok(after_tree)) => (before_tree, after_tree), (Err(before_err), _) => { return Some(TreeDiffEntry { - source: path.clone(), - target: path, + path, value: Err(before_err), }) } (_, Err(after_err)) => { return Some(TreeDiffEntry { - source: path.clone(), - target: path, + path, value: Err(after_err), }) } @@ -803,8 +798,7 @@ impl Iterator for TreeDiffIterator<'_> { if !tree_before && tree_after { if before.is_present() { return Some(TreeDiffEntry { - source: path.clone(), - target: path, + path, value: Ok((before, Merge::absent())), }); } @@ -817,8 +811,7 @@ impl Iterator for TreeDiffIterator<'_> { } } else if !tree_before && !tree_after { return Some(TreeDiffEntry { - source: path.clone(), - target: path, + path, value: Ok((before, after)), }); } @@ -1050,13 +1043,11 @@ impl Stream for TreeDiffStreamImpl<'_> { let (key, result) = entry.remove_entry(); Poll::Ready(Some(match result { Err(err) => TreeDiffEntry { - source: key.path.clone(), - target: key.path, + path: key.path, value: Err(err), }, Ok((before, after)) => TreeDiffEntry { - source: key.path.clone(), - target: key.path, + path: key.path, value: Ok((before, after)), }, })) @@ -1066,13 +1057,11 @@ impl Stream for TreeDiffStreamImpl<'_> { let (key, result) = entry.remove_entry(); Poll::Ready(Some(match result { Err(err) => TreeDiffEntry { - source: key.path.clone(), - target: key.path, + path: key.path, value: Err(err), }, Ok((before, after)) => TreeDiffEntry { - source: key.path.clone(), - target: key.path, + path: key.path, value: Ok((before, after)), }, })) diff --git a/lib/src/rewrite.rs b/lib/src/rewrite.rs index 577e21677f..72d065089e 100644 --- a/lib/src/rewrite.rs +++ b/lib/src/rewrite.rs @@ -93,8 +93,7 @@ pub fn restore_tree( // TODO: handle copy tracking let mut diff_stream = source.diff_stream(destination, matcher); while let Some(TreeDiffEntry { - source: _, // TODO handle copy tracking - target: repo_path, + path: repo_path, value: diff, }) = diff_stream.next().await { diff --git a/lib/tests/test_commit_builder.rs b/lib/tests/test_commit_builder.rs index b3f345925a..99776da259 100644 --- a/lib/tests/test_commit_builder.rs +++ b/lib/tests/test_commit_builder.rs @@ -29,7 +29,7 @@ fn diff_paths(from_tree: &MergedTree, to_tree: &MergedTree) -> Vec .diff_stream(to_tree, &EverythingMatcher) .map(|diff| { let _ = diff.value.unwrap(); - diff.target + diff.path }) .collect() .block_on() diff --git a/lib/tests/test_local_working_copy_sparse.rs b/lib/tests/test_local_working_copy_sparse.rs index c690417a94..b2a614557a 100644 --- a/lib/tests/test_local_working_copy_sparse.rs +++ b/lib/tests/test_local_working_copy_sparse.rs @@ -201,7 +201,7 @@ fn test_sparse_commit() { .collect() .block_on(); assert_eq!(diff.len(), 1); - assert_eq!(diff[0].target.as_ref(), dir1_file1_path); + assert_eq!(diff[0].path.as_ref(), dir1_file1_path); // Set sparse patterns to also include dir2/ let mut locked_ws = test_workspace @@ -223,8 +223,8 @@ fn test_sparse_commit() { .collect() .block_on(); assert_eq!(diff.len(), 2); - assert_eq!(diff[0].target.as_ref(), dir1_file1_path); - assert_eq!(diff[1].target.as_ref(), dir2_file1_path); + assert_eq!(diff[0].path.as_ref(), dir1_file1_path); + assert_eq!(diff[1].path.as_ref(), dir2_file1_path); } #[test] diff --git a/lib/tests/test_merged_tree.rs b/lib/tests/test_merged_tree.rs index a56729dcb9..4ea39585cf 100644 --- a/lib/tests/test_merged_tree.rs +++ b/lib/tests/test_merged_tree.rs @@ -36,17 +36,17 @@ fn file_value(file_id: &FileId) -> TreeValue { } fn diff_entry_tuple(diff: TreeDiffEntry) -> (RepoPathBuf, (MergedTreeValue, MergedTreeValue)) { - (diff.target, diff.value.unwrap()) + (diff.path, diff.value.unwrap()) } fn diff_stream_equals_iter(tree1: &MergedTree, tree2: &MergedTree, matcher: &dyn Matcher) { let iter_diff: Vec<_> = TreeDiffIterator::new(tree1.as_merge(), tree2.as_merge(), matcher) - .map(|diff| (diff.source, diff.target, diff.value.unwrap())) + .map(|diff| (diff.path, diff.value.unwrap())) .collect(); let max_concurrent_reads = 10; let stream_diff: Vec<_> = TreeDiffStreamImpl::new(tree1.clone(), tree2.clone(), matcher, max_concurrent_reads) - .map(|diff| (diff.source, diff.target, diff.value.unwrap())) + .map(|diff| (diff.path, diff.value.unwrap())) .collect() .block_on(); assert_eq!(stream_diff, iter_diff);