Skip to content

Commit

Permalink
merged_tree: remove TreeDiffEntry::source
Browse files Browse the repository at this point in the history
  • Loading branch information
martinvonz committed Aug 18, 2024
1 parent d1333e4 commit 0655231
Show file tree
Hide file tree
Showing 11 changed files with 41 additions and 72 deletions.
3 changes: 1 addition & 2 deletions cli/src/commands/fix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
8 changes: 1 addition & 7 deletions cli/src/merge_tools/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?;
Expand Down
2 changes: 1 addition & 1 deletion cli/src/merge_tools/diff_working_copies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ pub(crate) fn check_out_trees(
) -> Result<DiffWorkingCopies, DiffCheckoutError> {
let changed_files: Vec<_> = left_tree
.diff_stream(right_tree, matcher)
.map(|TreeDiffEntry { target, .. }| target)
.map(|TreeDiffEntry { path, .. }| path)
.collect()
.block_on();

Expand Down
8 changes: 4 additions & 4 deletions lib/src/copies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
12 changes: 6 additions & 6 deletions lib/src/default_index/revset_engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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);
Expand Down
30 changes: 9 additions & 21 deletions lib/src/local_working_copy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
Expand Down
33 changes: 11 additions & 22 deletions lib/src/merged_tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)>,
}
Expand Down Expand Up @@ -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 {
Expand All @@ -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),
})
}
Expand All @@ -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())),
});
}
Expand All @@ -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)),
});
}
Expand Down Expand Up @@ -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)),
},
}))
Expand All @@ -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)),
},
}))
Expand Down
3 changes: 1 addition & 2 deletions lib/src/rewrite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
2 changes: 1 addition & 1 deletion lib/tests/test_commit_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ fn diff_paths(from_tree: &MergedTree, to_tree: &MergedTree) -> Vec<RepoPathBuf>
.diff_stream(to_tree, &EverythingMatcher)
.map(|diff| {
let _ = diff.value.unwrap();
diff.target
diff.path
})
.collect()
.block_on()
Expand Down
6 changes: 3 additions & 3 deletions lib/tests/test_local_working_copy_sparse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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]
Expand Down
6 changes: 3 additions & 3 deletions lib/tests/test_merged_tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 0655231

Please sign in to comment.