feat!: Provide more source information when emitting rewrites.

Byron · Byron · commit df81577f19f1 · 2024-02-28T10:32:23.000+01:00
Previously, the source was entirely missing, now it's also made available.
Further, all the cloning of these resources is now left to the user,
which should safe time.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/gix-diff/src/rewrites/tracker.rs b/gix-diff/src/rewrites/tracker.rs
@@ -83,7 +83,7 @@ pub mod visit {
 
     /// The source of a rewrite, rename or copy.
     #[derive(Debug, Clone, PartialEq, PartialOrd)]
-    pub struct Source<'a> {
+    pub struct Source<'a, T> {
         /// The kind of entry.
         pub entry_mode: EntryMode,
         /// The hash of the state of the source as seen in the object database.
@@ -92,6 +92,8 @@ pub mod visit {
         pub kind: SourceKind,
         /// The repository-relative location of this entry.
         pub location: &'a BStr,
+        /// The change that was registered as source.
+        pub change: &'a T,
         /// If this is a rewrite, indicate how many lines would need to change to turn this source into the destination.
         pub diff: Option<DiffLineStats>,
     }
@@ -193,7 +195,7 @@ impl<T: Change> Tracker<T> {
     /// will panic if `change` is not a modification, and it's valid to not call `push` at all.
     pub fn emit<PushSourceTreeFn, E>(
         &mut self,
-        mut cb: impl FnMut(visit::Destination<'_, T>, Option<visit::Source<'_>>) -> crate::tree::visit::Action,
+        mut cb: impl FnMut(visit::Destination<'_, T>, Option<visit::Source<'_, T>>) -> crate::tree::visit::Action,
         diff_cache: &mut crate::blob::Platform,
         objects: &impl gix_object::FindObjectOrHeader,
         mut push_source_tree: PushSourceTreeFn,
@@ -283,7 +285,7 @@ impl<T: Change> Tracker<T> {
     fn match_pairs_of_kind(
         &mut self,
         kind: visit::SourceKind,
-        cb: &mut impl FnMut(visit::Destination<'_, T>, Option<visit::Source<'_>>) -> crate::tree::visit::Action,
+        cb: &mut impl FnMut(visit::Destination<'_, T>, Option<visit::Source<'_, T>>) -> crate::tree::visit::Action,
         percentage: Option<f32>,
         out: &mut Outcome,
         diff_cache: &mut crate::blob::Platform,
@@ -326,7 +328,7 @@ impl<T: Change> Tracker<T> {
 
     fn match_pairs(
         &mut self,
-        cb: &mut impl FnMut(visit::Destination<'_, T>, Option<visit::Source<'_>>) -> crate::tree::visit::Action,
+        cb: &mut impl FnMut(visit::Destination<'_, T>, Option<visit::Source<'_, T>>) -> crate::tree::visit::Action,
         percentage: Option<f32>,
         kind: visit::SourceKind,
         stats: &mut Outcome,
@@ -360,6 +362,7 @@ impl<T: Change> Tracker<T> {
                         id,
                         kind,
                         location,
+                        change: &src.change,
                         diff,
                     },
                     src_idx,
@@ -371,11 +374,15 @@ impl<T: Change> Tracker<T> {
             let location = dest.location(&self.path_backing);
             let change = dest.change.clone();
             let dest = visit::Destination { change, location };
+            let src_idx = src.as_ref().map(|t| t.1);
+            let res = cb(dest, src.map(|t| t.0));
+
             self.items[dest_idx].emitted = true;
-            if let Some(src_idx) = src.as_ref().map(|t| t.1) {
+            if let Some(src_idx) = src_idx {
                 self.items[src_idx].emitted = true;
             }
-            if cb(dest, src.map(|t| t.0)) == crate::tree::visit::Action::Cancel {
+
+            if res == crate::tree::visit::Action::Cancel {
                 return Ok(crate::tree::visit::Action::Cancel);
             }
         }
diff --git a/gix-diff/tests/Cargo.toml b/gix-diff/tests/Cargo.toml
@@ -25,3 +25,4 @@ gix-filter = { path = "../../gix-filter" }
 gix-traverse = { path = "../../gix-traverse" }
 gix-testtools = { path = "../../tests/tools" }
 shell-words = "1"
+pretty_assertions = "1.4.0"
diff --git a/gix-diff/tests/fixtures/generated-archives/make_diff_repo.tar.xz b/gix-diff/tests/fixtures/generated-archives/make_diff_repo.tar.xz
diff --git a/gix-diff/tests/rewrites/mod.rs b/gix-diff/tests/rewrites/mod.rs
@@ -4,7 +4,7 @@ use gix_object::tree::{EntryKind, EntryMode};
 
 mod tracker;
 
-#[derive(Clone)]
+#[derive(Debug, Clone, PartialEq, Eq)]
 pub struct Change {
     id: ObjectId,
     kind: ChangeKind,
diff --git a/gix-diff/tests/rewrites/tracker.rs b/gix-diff/tests/rewrites/tracker.rs
@@ -12,6 +12,7 @@ use gix_diff::{
     Rewrites,
 };
 use gix_object::tree::EntryKind;
+use pretty_assertions::assert_eq;
 
 use crate::{
     hex_to_id,
@@ -51,6 +52,7 @@ fn rename_by_id() -> crate::Result {
                     id: NULL_ID,
                     kind: SourceKind::Rename,
                     location: "b".into(),
+                    change: &Change::deletion(),
                     diff: None,
                 }
             );
@@ -146,11 +148,16 @@ fn copy_by_id() -> crate::Result {
         let out = util::assert_emit_with_objects(
             &mut track,
             |dst, src| {
+                let id = hex_to_id("2e65efe2a145dda7ee51d1741299f848e5bf752e");
                 let source_a = Source {
                     entry_mode: EntryKind::Blob.into(),
-                    id: hex_to_id("2e65efe2a145dda7ee51d1741299f848e5bf752e"),
+                    id: id,
                     kind: SourceKind::Copy,
                     location: "a".into(),
+                    change: &Change {
+                        id,
+                        ..Change::modification()
+                    },
                     diff: None,
                 };
                 match calls {
@@ -219,6 +226,10 @@ fn copy_by_id_search_in_all_sources() -> crate::Result {
                     id: content_id,
                     kind: SourceKind::Copy,
                     location: "a-src".into(),
+                    change: &Change {
+                        id: content_id,
+                        ..Change::modification()
+                    },
                     diff: None,
                 };
                 match calls {
@@ -289,11 +300,16 @@ fn copy_by_50_percent_similarity() -> crate::Result {
     let out = util::assert_emit_with_objects(
         &mut track,
         |dst, src| {
+            let id = hex_to_id("78981922613b2afb6025042ff6bd878ac1994e85");
             let source_a = Source {
                 entry_mode: EntryKind::Blob.into(),
-                id: hex_to_id("78981922613b2afb6025042ff6bd878ac1994e85"),
+                id: id,
                 kind: SourceKind::Copy,
                 location: "a".into(),
+                change: &Change {
+                    id,
+                    ..Change::modification()
+                },
                 diff: Some(DiffLineStats {
                     removals: 0,
                     insertions: 1,
@@ -459,13 +475,18 @@ fn rename_by_50_percent_similarity() -> crate::Result {
         |dst, src| {
             match calls {
                 0 => {
+                    let id = hex_to_id("66a52ee7a1d803dc57859c3e95ac9dcdc87c0164");
                     assert_eq!(
                         src.unwrap(),
                         Source {
                             entry_mode: EntryKind::Blob.into(),
-                            id: hex_to_id("66a52ee7a1d803dc57859c3e95ac9dcdc87c0164"),
+                            id: id,
                             kind: SourceKind::Rename,
                             location: "a".into(),
+                            change: &Change {
+                                id,
+                                ..Change::deletion()
+                            },
                             diff: Some(DiffLineStats {
                                 removals: 1,
                                 insertions: 1,
@@ -532,7 +553,7 @@ fn add_only() -> crate::Result {
     let out = util::assert_emit(&mut track, |dst, src| {
         assert!(!called);
         called = true;
-        assert_eq!(src, None, "there is just a single deletion, no pair");
+        assert!(src.is_none(), "there is just a single deletion, no pair");
         assert_eq!(dst.location, "a");
         assert_eq!(dst.change.kind, ChangeKind::Addition);
         Action::Continue
@@ -569,22 +590,22 @@ mod util {
 
     pub fn assert_emit(
         tracker: &mut rewrites::Tracker<Change>,
-        cb: impl FnMut(Destination<'_, Change>, Option<Source<'_>>) -> Action,
+        cb: impl FnMut(Destination<'_, Change>, Option<Source<'_, Change>>) -> Action,
     ) -> rewrites::Outcome {
         assert_emit_with_objects(tracker, cb, gix_object::find::Never)
     }
 
     pub fn assert_emit_with_objects(
         tracker: &mut rewrites::Tracker<Change>,
-        cb: impl FnMut(Destination<'_, Change>, Option<Source<'_>>) -> Action,
+        cb: impl FnMut(Destination<'_, Change>, Option<Source<'_, Change>>) -> Action,
         objects: impl gix_object::FindObjectOrHeader,
     ) -> rewrites::Outcome {
         assert_emit_with_objects_and_sources(tracker, cb, objects, None)
     }
 
     pub fn assert_emit_with_objects_and_sources<'a>(
         tracker: &mut rewrites::Tracker<Change>,
-        cb: impl FnMut(Destination<'_, Change>, Option<Source<'_>>) -> Action,
+        cb: impl FnMut(Destination<'_, Change>, Option<Source<'_, Change>>) -> Action,
         objects: impl gix_object::FindObjectOrHeader,
         sources: impl IntoIterator<Item = (Change, &'a str)>,
     ) -> rewrites::Outcome {
