just playing with tangled
0
fork

Configure Feed

Select the types of activity you want to include in your feed.

copies: implement copy support in `MergedTree::diff_stream()` as adapter

The support for copy tracing is already simply added to the stream
just before yielding the item, so we can easily implement it as a
stream adapter. That ensures that we use the same logic for the
iterator- and stream-based versions. More importantly, it enables
further cleanups and a simpler interface.

authored by

Martin von Zweigbergk and committed by
Martin von Zweigbergk
e670837f fd9a236b

+68 -107
+38
lib/src/copies.rs
··· 15 15 //! Code for working with copies and renames. 16 16 17 17 use std::collections::HashMap; 18 + use std::pin::Pin; 19 + use std::task::{Context, Poll}; 18 20 19 21 use futures::executor::block_on_stream; 20 22 use futures::stream::BoxStream; 23 + use futures::Stream; 21 24 22 25 use crate::backend::{BackendResult, CopyRecord}; 26 + use crate::merged_tree::{MergedTree, TreeDiffEntry, TreeDiffStream}; 23 27 use crate::repo_path::{RepoPath, RepoPathBuf}; 24 28 25 29 /// A collection of CopyRecords. ··· 64 68 self.records.iter() 65 69 } 66 70 } 71 + 72 + /// Wraps a `TreeDiffStream`, adding support for copies and renames. 73 + pub struct CopiesTreeDiffStream<'a> { 74 + inner: TreeDiffStream<'a>, 75 + source_tree: MergedTree, 76 + copy_records: &'a CopyRecords, 77 + } 78 + 79 + impl<'a> CopiesTreeDiffStream<'a> { 80 + /// Create a new diff stream with copy information. 81 + pub fn new( 82 + inner: TreeDiffStream<'a>, 83 + source_tree: MergedTree, 84 + copy_records: &'a CopyRecords, 85 + ) -> Self { 86 + Self { 87 + inner, 88 + source_tree, 89 + copy_records, 90 + } 91 + } 92 + } 93 + 94 + impl Stream for CopiesTreeDiffStream<'_> { 95 + type Item = TreeDiffEntry; 96 + 97 + fn poll_next(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Option<Self::Item>> { 98 + self.inner.as_mut().poll_next(cx).map(|option| { 99 + option.map(|diff_entry| { 100 + diff_entry.adjust_for_copy_tracking(&self.source_tree, self.copy_records) 101 + }) 102 + }) 103 + } 104 + }
+11 -27
lib/src/merged_tree.rs
··· 31 31 32 32 use crate::backend; 33 33 use crate::backend::{BackendResult, CopyRecord, MergedTreeId, TreeId, TreeValue}; 34 - use crate::copies::CopyRecords; 34 + use crate::copies::{CopiesTreeDiffStream, CopyRecords}; 35 35 use crate::matchers::{EverythingMatcher, Matcher}; 36 36 use crate::merge::{Merge, MergeBuilder, MergedTreeVal, MergedTreeValue}; 37 37 use crate::repo_path::{RepoPath, RepoPathBuf, RepoPathComponent}; ··· 268 268 copy_records: &'matcher CopyRecords, 269 269 ) -> TreeDiffStream<'matcher> { 270 270 let concurrency = self.store().concurrency(); 271 - if concurrency <= 1 { 271 + let stream: TreeDiffStream<'matcher> = if concurrency <= 1 { 272 272 Box::pin(futures::stream::iter(TreeDiffIterator::new( 273 273 &self.trees, 274 274 &other.trees, 275 275 matcher, 276 - copy_records, 277 276 ))) 278 277 } else { 279 278 Box::pin(TreeDiffStreamImpl::new( 280 279 self.clone(), 281 280 other.clone(), 282 281 matcher, 283 - copy_records, 284 282 concurrency, 285 283 )) 286 - } 284 + }; 285 + Box::pin(CopiesTreeDiffStream::new( 286 + stream, 287 + self.clone(), 288 + copy_records, 289 + )) 287 290 } 288 291 289 292 /// Merges this tree with `other`, using `base` as base. Any conflicts will ··· 317 320 } 318 321 319 322 impl TreeDiffEntry { 320 - fn adjust_for_copy_tracking( 323 + pub(crate) fn adjust_for_copy_tracking( 321 324 self, 322 325 source_tree: &MergedTree, 323 326 copy_records: &CopyRecords, ··· 668 671 pub struct TreeDiffIterator<'matcher> { 669 672 store: Arc<Store>, 670 673 stack: Vec<TreeDiffItem>, 671 - source: MergedTree, 672 674 matcher: &'matcher dyn Matcher, 673 - copy_records: &'matcher CopyRecords, 674 675 } 675 676 676 677 struct TreeDiffDirItem { ··· 688 689 impl<'matcher> TreeDiffIterator<'matcher> { 689 690 /// Creates a iterator over the differences between two trees. Generally 690 691 /// prefer `MergedTree::diff()` of calling this directly. 691 - pub fn new( 692 - trees1: &Merge<Tree>, 693 - trees2: &Merge<Tree>, 694 - matcher: &'matcher dyn Matcher, 695 - copy_records: &'matcher CopyRecords, 696 - ) -> Self { 692 + pub fn new(trees1: &Merge<Tree>, trees2: &Merge<Tree>, matcher: &'matcher dyn Matcher) -> Self { 697 693 assert!(Arc::ptr_eq(trees1.first().store(), trees2.first().store())); 698 694 let root_dir = RepoPath::root(); 699 695 let mut stack = Vec::new(); ··· 705 701 Self { 706 702 store: trees1.first().store().clone(), 707 703 stack, 708 - source: MergedTree::new(trees1.clone()), 709 704 matcher, 710 - copy_records, 711 705 } 712 706 } 713 707 ··· 847 841 848 842 fn next(&mut self) -> Option<Self::Item> { 849 843 self.next_impl() 850 - .map(|diff_entry| diff_entry.adjust_for_copy_tracking(&self.source, self.copy_records)) 851 844 } 852 845 } 853 846 854 847 /// Stream of differences between two trees. 855 848 pub struct TreeDiffStreamImpl<'matcher> { 856 849 matcher: &'matcher dyn Matcher, 857 - copy_records: &'matcher CopyRecords, 858 - source_tree: MergedTree, 859 850 /// Pairs of tree values that may or may not be ready to emit, sorted in the 860 851 /// order we want to emit them. If either side is a tree, there will be 861 852 /// a corresponding entry in `pending_trees`. ··· 930 921 tree1: MergedTree, 931 922 tree2: MergedTree, 932 923 matcher: &'matcher dyn Matcher, 933 - copy_records: &'matcher CopyRecords, 934 924 max_concurrent_reads: usize, 935 925 ) -> Self { 936 926 let mut stream = Self { 937 927 matcher, 938 - copy_records, 939 - source_tree: tree1.clone(), 940 928 items: BTreeMap::new(), 941 929 pending_trees: VecDeque::new(), 942 930 max_concurrent_reads, ··· 1122 1110 type Item = TreeDiffEntry; 1123 1111 1124 1112 fn poll_next(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Option<Self::Item>> { 1125 - self.as_mut().poll_next_impl(cx).map(|option| { 1126 - option.map(|diff_entry| { 1127 - diff_entry.adjust_for_copy_tracking(&self.source_tree, self.copy_records) 1128 - }) 1129 - }) 1113 + self.as_mut().poll_next_impl(cx) 1130 1114 } 1131 1115 } 1132 1116
+19 -80
lib/tests/test_merged_tree.rs
··· 39 39 (diff.target, diff.value.unwrap()) 40 40 } 41 41 42 - fn diff_stream_equals_iter( 43 - tree1: &MergedTree, 44 - tree2: &MergedTree, 45 - matcher: &dyn Matcher, 46 - copy_records: &CopyRecords, 47 - ) { 48 - let iter_diff: Vec<_> = 49 - TreeDiffIterator::new(tree1.as_merge(), tree2.as_merge(), matcher, copy_records) 42 + fn diff_stream_equals_iter(tree1: &MergedTree, tree2: &MergedTree, matcher: &dyn Matcher) { 43 + let iter_diff: Vec<_> = TreeDiffIterator::new(tree1.as_merge(), tree2.as_merge(), matcher) 44 + .map(|diff| (diff.source, diff.target, diff.value.unwrap())) 45 + .collect(); 46 + let max_concurrent_reads = 10; 47 + let stream_diff: Vec<_> = 48 + TreeDiffStreamImpl::new(tree1.clone(), tree2.clone(), matcher, max_concurrent_reads) 50 49 .map(|diff| (diff.source, diff.target, diff.value.unwrap())) 51 - .collect(); 52 - let max_concurrent_reads = 10; 53 - let stream_diff: Vec<_> = TreeDiffStreamImpl::new( 54 - tree1.clone(), 55 - tree2.clone(), 56 - matcher, 57 - copy_records, 58 - max_concurrent_reads, 59 - ) 60 - .map(|diff| (diff.source, diff.target, diff.value.unwrap())) 61 - .collect() 62 - .block_on(); 50 + .collect() 51 + .block_on(); 63 52 assert_eq!(stream_diff, iter_diff); 64 53 } 65 54 ··· 808 797 ), 809 798 ) 810 799 ); 811 - diff_stream_equals_iter( 812 - &before_merged, 813 - &after_merged, 814 - &EverythingMatcher, 815 - &CopyRecords::default(), 816 - ); 800 + diff_stream_equals_iter(&before_merged, &after_merged, &EverythingMatcher); 817 801 } 818 802 819 803 fn create_copy_records(paths: &[(&RepoPath, &RepoPath)]) -> CopyRecords { ··· 918 902 ), 919 903 ) 920 904 ); 921 - diff_stream_equals_iter( 922 - &before_merged, 923 - &after_merged, 924 - &EverythingMatcher, 925 - &copy_records, 926 - ); 905 + diff_stream_equals_iter(&before_merged, &after_merged, &EverythingMatcher); 927 906 } 928 907 929 908 /// Diff two conflicted trees ··· 1015 994 }) 1016 995 .collect_vec(); 1017 996 assert_eq!(actual_diff, expected_diff); 1018 - diff_stream_equals_iter( 1019 - &left_merged, 1020 - &right_merged, 1021 - &EverythingMatcher, 1022 - &CopyRecords::default(), 1023 - ); 997 + diff_stream_equals_iter(&left_merged, &right_merged, &EverythingMatcher); 1024 998 // Test the reverse diff 1025 999 let actual_diff: Vec<_> = right_merged 1026 1000 .diff_stream(&left_merged, &EverythingMatcher, &Default::default()) ··· 1040 1014 }) 1041 1015 .collect_vec(); 1042 1016 assert_eq!(actual_diff, expected_diff); 1043 - diff_stream_equals_iter( 1044 - &right_merged, 1045 - &left_merged, 1046 - &EverythingMatcher, 1047 - &CopyRecords::default(), 1048 - ); 1017 + diff_stream_equals_iter(&right_merged, &left_merged, &EverythingMatcher); 1049 1018 } 1050 1019 1051 1020 #[test] ··· 1185 1154 (path6.to_owned(), (Merge::absent(), right_value(path6))), 1186 1155 ]; 1187 1156 assert_eq!(actual_diff, expected_diff); 1188 - diff_stream_equals_iter( 1189 - &left_merged, 1190 - &right_merged, 1191 - &EverythingMatcher, 1192 - &CopyRecords::default(), 1193 - ); 1157 + diff_stream_equals_iter(&left_merged, &right_merged, &EverythingMatcher); 1194 1158 } 1195 1159 1196 1160 // Test the reverse diff ··· 1235 1199 ), 1236 1200 ]; 1237 1201 assert_eq!(actual_diff, expected_diff); 1238 - diff_stream_equals_iter( 1239 - &right_merged, 1240 - &left_merged, 1241 - &EverythingMatcher, 1242 - &CopyRecords::default(), 1243 - ); 1202 + diff_stream_equals_iter(&right_merged, &left_merged, &EverythingMatcher); 1244 1203 } 1245 1204 1246 1205 // Diff while filtering by `path1` (file1 -> directory1) as a file ··· 1256 1215 (path1.to_owned(), (left_value(path1), Merge::absent())), 1257 1216 ]; 1258 1217 assert_eq!(actual_diff, expected_diff); 1259 - diff_stream_equals_iter( 1260 - &left_merged, 1261 - &right_merged, 1262 - &matcher, 1263 - &CopyRecords::default(), 1264 - ); 1218 + diff_stream_equals_iter(&left_merged, &right_merged, &matcher); 1265 1219 } 1266 1220 1267 1221 // Diff while filtering by `path1/file` (file1 -> directory1) as a file ··· 1280 1234 ), 1281 1235 ]; 1282 1236 assert_eq!(actual_diff, expected_diff); 1283 - diff_stream_equals_iter( 1284 - &left_merged, 1285 - &right_merged, 1286 - &matcher, 1287 - &CopyRecords::default(), 1288 - ); 1237 + diff_stream_equals_iter(&left_merged, &right_merged, &matcher); 1289 1238 } 1290 1239 1291 1240 // Diff while filtering by `path1` (file1 -> directory1) as a prefix ··· 1304 1253 ), 1305 1254 ]; 1306 1255 assert_eq!(actual_diff, expected_diff); 1307 - diff_stream_equals_iter( 1308 - &left_merged, 1309 - &right_merged, 1310 - &matcher, 1311 - &CopyRecords::default(), 1312 - ); 1256 + diff_stream_equals_iter(&left_merged, &right_merged, &matcher); 1313 1257 } 1314 1258 1315 1259 // Diff while filtering by `path6` (directory1 -> file1+(directory1-absent)) as ··· 1325 1269 .block_on(); 1326 1270 let expected_diff = vec![(path6.to_owned(), (Merge::absent(), right_value(path6)))]; 1327 1271 assert_eq!(actual_diff, expected_diff); 1328 - diff_stream_equals_iter( 1329 - &left_merged, 1330 - &right_merged, 1331 - &matcher, 1332 - &CopyRecords::default(), 1333 - ); 1272 + diff_stream_equals_iter(&left_merged, &right_merged, &matcher); 1334 1273 } 1335 1274 } 1336 1275