merged_tree: provide separate version of diff_stream() with copy info

I plan to provide a richer version of `TreeDiffEntry` with copy info
(and to make `TreeDiffEntry` itself "poorer"). Most callers want to
know about copies/renames, but at least working copy implementations
probably don't. This patch adds separate `diff_stream()` and
`diff_stream_with_copies()` so we can provide the simpler interface
for callers that don't need copy info.
This commit is contained in:
Martin von Zweigbergk 2024-08-17 14:28:13 -07:00 committed by Martin von Zweigbergk
parent bce8550db1
commit 70598498b0
12 changed files with 50 additions and 38 deletions

View file

@ -172,8 +172,8 @@ pub(crate) fn cmd_fix(
// Also fix any new paths that were changed in this commit.
let tree = commit.tree()?;
let parent_tree = commit.parent_tree(tx.repo())?;
let copy_records = Default::default();
let mut diff_stream = parent_tree.diff_stream(&tree, &matcher, &copy_records);
// TODO: handle copy tracking
let mut diff_stream = parent_tree.diff_stream(&tree, &matcher);
async {
while let Some(TreeDiffEntry {
source: _, // TODO handle copy tracking

View file

@ -1298,7 +1298,7 @@ impl TreeDiff {
fn diff_stream(&self) -> TreeDiffStream<'_> {
self.from_tree
.diff_stream(&self.to_tree, &*self.matcher, &self.copy_records)
.diff_stream_with_copies(&self.to_tree, &*self.matcher, &self.copy_records)
}
fn into_formatted<F, E>(self, show: F) -> TreeDiffFormatted<F>

View file

@ -290,7 +290,8 @@ impl<'a> DiffRenderer<'a> {
)?;
}
DiffFormat::Stat => {
let tree_diff = from_tree.diff_stream(to_tree, matcher, copy_records);
let tree_diff =
from_tree.diff_stream_with_copies(to_tree, matcher, copy_records);
show_diff_stat(formatter, store, tree_diff, path_converter, width)?;
}
DiffFormat::Types => {
@ -304,7 +305,8 @@ impl<'a> DiffRenderer<'a> {
)?;
}
DiffFormat::NameOnly => {
let tree_diff = from_tree.diff_stream(to_tree, matcher, copy_records);
let tree_diff =
from_tree.diff_stream_with_copies(to_tree, matcher, copy_records);
show_names(formatter, tree_diff, path_converter)?;
}
DiffFormat::Git { context } => {
@ -319,13 +321,15 @@ impl<'a> DiffRenderer<'a> {
)?;
}
DiffFormat::ColorWords { context } => {
let tree_diff = from_tree.diff_stream(to_tree, matcher, copy_records);
let tree_diff =
from_tree.diff_stream_with_copies(to_tree, matcher, copy_records);
show_color_words_diff(formatter, store, tree_diff, path_converter, *context)?;
}
DiffFormat::Tool(tool) => {
match tool.diff_invocation_mode {
DiffToolMode::FileByFile => {
let tree_diff = from_tree.diff_stream(to_tree, matcher, copy_records);
let tree_diff =
from_tree.diff_stream_with_copies(to_tree, matcher, copy_records);
show_file_by_file_diff(
ui,
formatter,
@ -1160,7 +1164,7 @@ pub fn show_git_diff(
copy_records: &CopyRecords,
num_context_lines: usize,
) -> Result<(), DiffRenderError> {
let tree_diff = from_tree.diff_stream(to_tree, matcher, copy_records);
let tree_diff = from_tree.diff_stream_with_copies(to_tree, matcher, copy_records);
let mut diff_stream = materialized_diff_stream(store, tree_diff);
let copied_sources = collect_copied_sources(copy_records, matcher);
@ -1271,7 +1275,7 @@ pub fn show_diff_summary(
matcher: &dyn Matcher,
copy_records: &CopyRecords,
) -> Result<(), DiffRenderError> {
let mut tree_diff = from_tree.diff_stream(to_tree, matcher, copy_records);
let mut tree_diff = from_tree.diff_stream_with_copies(to_tree, matcher, copy_records);
let copied_sources = collect_copied_sources(copy_records, matcher);
async {
@ -1444,7 +1448,7 @@ pub fn show_types(
matcher: &dyn Matcher,
copy_records: &CopyRecords,
) -> Result<(), DiffRenderError> {
let mut tree_diff = from_tree.diff_stream(to_tree, matcher, copy_records);
let mut tree_diff = from_tree.diff_stream_with_copies(to_tree, matcher, copy_records);
let copied_sources = collect_copied_sources(copy_records, matcher);
async {

View file

@ -494,8 +494,9 @@ pub fn edit_diff_builtin(
matcher: &dyn Matcher,
) -> Result<MergedTreeId, BuiltinToolError> {
let store = left_tree.store().clone();
// TODO: handle copy tracking
let changed_files: Vec<_> = left_tree
.diff_stream(right_tree, matcher, &Default::default())
.diff_stream(right_tree, matcher)
.map(
|TreeDiffEntry {
source: _, // TODO handle copy tracking

View file

@ -131,7 +131,7 @@ pub(crate) fn check_out_trees(
output_is: Option<DiffSide>,
) -> Result<DiffWorkingCopies, DiffCheckoutError> {
let changed_files: Vec<_> = left_tree
.diff_stream(right_tree, matcher, &Default::default())
.diff_stream(right_tree, matcher)
.map(|TreeDiffEntry { target, .. }| target)
.collect()
.block_on();

View file

@ -1147,8 +1147,8 @@ fn has_diff_from_parent(
// Conflict resolution is expensive, try that only for matched files.
let from_tree = rewrite::merge_commit_trees_no_resolve_without_repo(store, &index, &parents)?;
let to_tree = commit.tree()?;
let copy_records = Default::default();
let mut tree_diff = from_tree.diff_stream(&to_tree, matcher, &copy_records);
// TODO: handle copy tracking
let mut tree_diff = from_tree.diff_stream(&to_tree, matcher);
async {
while let Some(entry) = tree_diff.next().await {
let (from_value, to_value) = entry.value?;
@ -1170,12 +1170,12 @@ fn matches_diff_from_parent(
text_pattern: &StringPattern,
files_matcher: &dyn Matcher,
) -> BackendResult<bool> {
let copy_records = Default::default(); // TODO handle copy tracking
let parents: Vec<_> = commit.parents().try_collect()?;
// Conflict resolution is expensive, try that only for matched files.
let from_tree = rewrite::merge_commit_trees_no_resolve_without_repo(store, &index, &parents)?;
let to_tree = commit.tree()?;
let mut tree_diff = from_tree.diff_stream(&to_tree, files_matcher, &copy_records);
// TODO: handle copy tracking
let mut tree_diff = from_tree.diff_stream(&to_tree, files_matcher);
async {
while let Some(entry) = tree_diff.next().await {
let (left_value, right_value) = entry.value?;

View file

@ -1340,7 +1340,6 @@ impl TreeState {
new_tree: &MergedTree,
matcher: &dyn Matcher,
) -> Result<CheckoutStats, CheckoutError> {
let copy_records = Default::default();
// TODO: maybe it's better not include the skipped counts in the "intended"
// counts
let mut stats = CheckoutStats {
@ -1353,7 +1352,7 @@ impl TreeState {
let mut deleted_files = HashSet::new();
let mut diff_stream = Box::pin(
old_tree
.diff_stream(new_tree, matcher, &copy_records)
.diff_stream(new_tree, matcher)
.map(
|TreeDiffEntry {
source: _, // TODO handle copy tracking
@ -1452,10 +1451,9 @@ impl TreeState {
})?;
let matcher = self.sparse_matcher();
let copy_records = Default::default();
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(), &copy_records);
let mut diff_stream = old_tree.diff_stream(new_tree, matcher.as_ref());
while let Some(TreeDiffEntry {
source: _, // TODO handle copy tracking
target: path,

View file

@ -265,10 +265,9 @@ impl MergedTree {
&self,
other: &MergedTree,
matcher: &'matcher dyn Matcher,
copy_records: &'matcher CopyRecords,
) -> TreeDiffStream<'matcher> {
let concurrency = self.store().concurrency();
let stream: TreeDiffStream<'matcher> = if concurrency <= 1 {
if concurrency <= 1 {
Box::pin(futures::stream::iter(TreeDiffIterator::new(
&self.trees,
&other.trees,
@ -281,7 +280,17 @@ impl MergedTree {
matcher,
concurrency,
))
};
}
}
/// Like `diff_stream()` but takes the given copy records into account.
pub fn diff_stream_with_copies<'a>(
&self,
other: &MergedTree,
matcher: &'a dyn Matcher,
copy_records: &'a CopyRecords,
) -> TreeDiffStream<'a> {
let stream = self.diff_stream(other, matcher);
Box::pin(CopiesTreeDiffStream::new(
stream,
self.clone(),

View file

@ -89,9 +89,9 @@ pub fn restore_tree(
// TODO: We should be able to not traverse deeper in the diff if the matcher
// matches an entire subtree.
let mut tree_builder = MergedTreeBuilder::new(destination.id().clone());
let copy_records = Default::default();
async {
let mut diff_stream = source.diff_stream(destination, matcher, &copy_records);
// 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,

View file

@ -26,7 +26,7 @@ use testutils::{assert_rebased_onto, create_tree, CommitGraphBuilder, TestRepo,
fn diff_paths(from_tree: &MergedTree, to_tree: &MergedTree) -> Vec<RepoPathBuf> {
from_tree
.diff_stream(to_tree, &EverythingMatcher, &Default::default())
.diff_stream(to_tree, &EverythingMatcher)
.map(|diff| {
let _ = diff.value.unwrap();
diff.target

View file

@ -197,7 +197,7 @@ fn test_sparse_commit() {
// tree.
let modified_tree = test_workspace.snapshot().unwrap();
let diff: Vec<_> = tree
.diff_stream(&modified_tree, &EverythingMatcher, &Default::default())
.diff_stream(&modified_tree, &EverythingMatcher)
.collect()
.block_on();
assert_eq!(diff.len(), 1);
@ -219,7 +219,7 @@ fn test_sparse_commit() {
// updated in the tree.
let modified_tree = test_workspace.snapshot().unwrap();
let diff: Vec<_> = tree
.diff_stream(&modified_tree, &EverythingMatcher, &Default::default())
.diff_stream(&modified_tree, &EverythingMatcher)
.collect()
.block_on();
assert_eq!(diff.len(), 2);

View file

@ -762,7 +762,7 @@ fn test_diff_resolved() {
let after_merged = MergedTree::new(Merge::resolved(after.clone()));
let diff: Vec<_> = before_merged
.diff_stream(&after_merged, &EverythingMatcher, &Default::default())
.diff_stream(&after_merged, &EverythingMatcher)
.map(diff_entry_tuple)
.collect()
.block_on();
@ -853,7 +853,7 @@ fn test_diff_copy_tracing() {
create_copy_records(&[(removed_path, added_path), (modified_path, copied_path)]);
let diff: Vec<_> = before_merged
.diff_stream(&after_merged, &EverythingMatcher, &copy_records)
.diff_stream_with_copies(&after_merged, &EverythingMatcher, &copy_records)
.map(|diff| (diff.source, diff.target, diff.value.unwrap()))
.collect()
.block_on();
@ -977,7 +977,7 @@ fn test_diff_conflicted() {
// Test the forwards diff
let actual_diff: Vec<_> = left_merged
.diff_stream(&right_merged, &EverythingMatcher, &Default::default())
.diff_stream(&right_merged, &EverythingMatcher)
.map(diff_entry_tuple)
.collect()
.block_on();
@ -997,7 +997,7 @@ fn test_diff_conflicted() {
diff_stream_equals_iter(&left_merged, &right_merged, &EverythingMatcher);
// Test the reverse diff
let actual_diff: Vec<_> = right_merged
.diff_stream(&left_merged, &EverythingMatcher, &Default::default())
.diff_stream(&left_merged, &EverythingMatcher)
.map(diff_entry_tuple)
.collect()
.block_on();
@ -1115,7 +1115,7 @@ fn test_diff_dir_file() {
// Test the forwards diff
{
let actual_diff: Vec<_> = left_merged
.diff_stream(&right_merged, &EverythingMatcher, &Default::default())
.diff_stream(&right_merged, &EverythingMatcher)
.map(diff_entry_tuple)
.collect()
.block_on();
@ -1160,7 +1160,7 @@ fn test_diff_dir_file() {
// Test the reverse diff
{
let actual_diff: Vec<_> = right_merged
.diff_stream(&left_merged, &EverythingMatcher, &Default::default())
.diff_stream(&left_merged, &EverythingMatcher)
.map(diff_entry_tuple)
.collect()
.block_on();
@ -1206,7 +1206,7 @@ fn test_diff_dir_file() {
{
let matcher = FilesMatcher::new([&path1]);
let actual_diff: Vec<_> = left_merged
.diff_stream(&right_merged, &matcher, &Default::default())
.diff_stream(&right_merged, &matcher)
.map(diff_entry_tuple)
.collect()
.block_on();
@ -1222,7 +1222,7 @@ fn test_diff_dir_file() {
{
let matcher = FilesMatcher::new([path1.join(file)]);
let actual_diff: Vec<_> = left_merged
.diff_stream(&right_merged, &matcher, &Default::default())
.diff_stream(&right_merged, &matcher)
.map(diff_entry_tuple)
.collect()
.block_on();
@ -1241,7 +1241,7 @@ fn test_diff_dir_file() {
{
let matcher = PrefixMatcher::new([&path1]);
let actual_diff: Vec<_> = left_merged
.diff_stream(&right_merged, &matcher, &Default::default())
.diff_stream(&right_merged, &matcher)
.map(diff_entry_tuple)
.collect()
.block_on();
@ -1263,7 +1263,7 @@ fn test_diff_dir_file() {
{
let matcher = FilesMatcher::new([&path6]);
let actual_diff: Vec<_> = left_merged
.diff_stream(&right_merged, &matcher, &Default::default())
.diff_stream(&right_merged, &matcher)
.map(diff_entry_tuple)
.collect()
.block_on();