feat!: optional rename tracking for directories.

Depending on the source of the rename-information, items that are children
of renamed parents may be provided to enable rename tracking based on these
containers, instead of always resorting to tracking leaf nodes (i.e. blobs).

Author: Byron

gix-diff/src/rewrites/mod.rs

@@ -1,8 +1,12 @@
-use crate::Rewrites;
+use crate::{tree, Rewrites};
+use std::collections::HashMap;
+use std::ops::Range;
 
 /// Types related to the rename tracker for renames, rewrites and copies.
 pub mod tracker;
 
+type ItemId = (gix_hash::ObjectId, Range<usize>);
+
 /// A type to retain state related to an ongoing tracking operation to retain sets of interesting changes
 /// of which some are retained to at a later stage compute the ones that seem to be renames or copies.
 pub struct Tracker<T> {
@@ -12,6 +16,9 @@ pub struct Tracker<T> {
     path_backing: Vec<u8>,
     /// How to track copies and/or rewrites.
     rewrites: Rewrites,
+    /// A mapping of parents (and their IDs) to their children.
+    /// We use IDs + path-ranges as a cheap way to refer to children so they can be located in `items` quickly.
+    parent_child_map: HashMap<tree::visit::ChangeId, Vec<ItemId>>,
 }
 
 /// Determine in which set of files to search for copies.

gix-diff/src/rewrites/tracker.rs

@@ -9,6 +9,7 @@ use std::ops::Range;
 use bstr::BStr;
 use gix_object::tree::{EntryKind, EntryMode};
 
+use crate::tree::visit::Relation;
 use crate::{
     blob::{platform::prepare_diff::Operation, DiffLineStats, ResourceKind},
     rewrites::{CopySource, Outcome, Tracker},
@@ -28,11 +29,22 @@ pub enum ChangeKind {
 
 /// A trait providing all functionality to abstract over the concept of a change, as seen by the [`Tracker`].
 pub trait Change: Clone {
-    /// Return the hash of this change for identification.
+    /// Return the hash of the object behind this change for identification.
     ///
     /// Note that this is the id of the object as stored in `git`, i.e. it must have gone through workspace
-    /// conversions.
+    /// conversions. What matters is that the IDs are comparable.
     fn id(&self) -> &gix_hash::oid;
+    /// Return the relation that this change may have with other changes.
+    ///
+    /// It allows to associate a directory with its children that are added or removed at the same moment.
+    /// Note that this is ignored for modifications.
+    ///
+    /// If rename-tracking should always be on leaf-level, this should be set to `None` consistently.
+    /// Note that trees will never be looked up by their `id` as their children are assumed to be passed in
+    /// with the respective relationship.
+    ///
+    /// Also note that the tracker only sees what's given to it, it will not lookup trees or match paths itself.
+    fn relation(&self) -> Option<crate::tree::visit::Relation>;
     /// Return the kind of this change.
     fn kind(&self) -> ChangeKind;
     /// Return more information about the kind of entry affected by this change.
@@ -142,6 +154,7 @@ impl<T: Change> Tracker<T> {
             items: vec![],
             path_backing: vec![],
             rewrites,
+            parent_child_map: Default::default(),
         }
     }
 }
@@ -150,23 +163,40 @@ impl<T: Change> Tracker<T> {
 impl<T: Change> Tracker<T> {
     /// We may refuse the push if that information isn't needed for what we have to track.
     pub fn try_push_change(&mut self, change: T, location: &BStr) -> Option<T> {
-        if !change.entry_mode().is_blob_or_symlink() {
+        let change_kind = change.kind();
+        if let (None, ChangeKind::Modification { .. }) = (self.rewrites.copies, change_kind) {
             return Some(change);
-        }
-        let keep = match (self.rewrites.copies, change.kind()) {
-            (Some(_find_copies), _) => true,
-            (None, ChangeKind::Modification { .. }) => false,
-            (None, _) => true,
         };
 
-        if !keep {
-            return Some(change);
-        }
+        let relation = change
+            .relation()
+            .filter(|_| matches!(change_kind, ChangeKind::Addition | ChangeKind::Deletion));
+        let entry_kind = change.entry_mode().kind();
+        let parent_info = match (relation, entry_kind) {
+            (None | Some(Relation::ChildOfParent(_)), EntryKind::Commit | EntryKind::Tree) => return Some(change),
+            (Some(Relation::Parent(id)), EntryKind::Tree) => {
+                let previous = self.parent_child_map.insert(id, vec![]);
+                assert!(
+                    previous.is_none(),
+                    "change-producer must assure parents only occour once with the same id"
+                );
+                None
+            }
+            (Some(Relation::ChildOfParent(id)), EntryKind::Blob | EntryKind::BlobExecutable | EntryKind::Link) => {
+                Some(id)
+            }
+            _ => None,
+        };
 
         let start = self.path_backing.len();
         self.path_backing.extend_from_slice(location);
+        let path = start..self.path_backing.len();
+
+        if let Some(children) = parent_info.and_then(|id| self.parent_child_map.get_mut(&id)) {
+            children.push((change.id().to_owned(), path.clone()));
+        }
         self.items.push(Item {
-            path: start..self.path_backing.len(),
+            path,
             change,
             emitted: false,
         });

gix-diff/src/tree/visit.rs

@@ -110,6 +110,7 @@ mod change_impls {
     use gix_hash::oid;
     use gix_object::tree::EntryMode;
 
+    use crate::tree::visit::Relation;
     use crate::{rewrites::tracker::ChangeKind, tree::visit::Change};
 
     impl crate::rewrites::tracker::Change for crate::tree::visit::Change {
@@ -119,6 +120,13 @@ mod change_impls {
             }
         }
 
+        fn relation(&self) -> Option<Relation> {
+            match self {
+                Change::Addition { relation, .. } | Change::Deletion { relation, .. } => *relation,
+                Change::Modification { .. } => None,
+            }
+        }
+
         fn kind(&self) -> ChangeKind {
             match self {
                 Change::Addition { .. } => ChangeKind::Addition,

gix-diff/tests/rewrites/mod.rs

@@ -1,21 +1,27 @@
 use gix_diff::rewrites::tracker::ChangeKind;
+use gix_diff::tree::visit::{ChangeId, Relation};
 use gix_hash::{oid, ObjectId};
 use gix_object::tree::{EntryKind, EntryMode};
 
 mod tracker;
 
-#[derive(Debug, Clone, PartialEq, Eq)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub struct Change {
     id: ObjectId,
     kind: ChangeKind,
     mode: EntryMode,
+    relation: Option<Relation>,
 }
 
 impl gix_diff::rewrites::tracker::Change for Change {
     fn id(&self) -> &oid {
         &self.id
     }
 
+    fn relation(&self) -> Option<Relation> {
+        self.relation
+    }
+
     fn kind(&self) -> ChangeKind {
         self.kind
     }
@@ -37,20 +43,59 @@ impl Change {
             id: NULL_ID,
             kind: ChangeKind::Modification,
             mode: EntryKind::Blob.into(),
+            relation: None,
         }
     }
     fn deletion() -> Self {
         Change {
             id: NULL_ID,
             kind: ChangeKind::Deletion,
             mode: EntryKind::Blob.into(),
+            relation: None,
         }
     }
     fn addition() -> Self {
         Change {
             id: NULL_ID,
             kind: ChangeKind::Addition,
             mode: EntryKind::Blob.into(),
+            relation: None,
+        }
+    }
+
+    fn addition_in_tree(id: ChangeId) -> Self {
+        Change {
+            id: NULL_ID,
+            kind: ChangeKind::Addition,
+            mode: EntryKind::Blob.into(),
+            relation: Some(Relation::ChildOfParent(id)),
+        }
+    }
+
+    fn deletion_in_tree(id: ChangeId) -> Self {
+        Change {
+            id: NULL_ID,
+            kind: ChangeKind::Deletion,
+            mode: EntryKind::Blob.into(),
+            relation: Some(Relation::ChildOfParent(id)),
+        }
+    }
+
+    fn tree_addition(id: ChangeId) -> Self {
+        Change {
+            id: NULL_ID,
+            kind: ChangeKind::Addition,
+            mode: EntryKind::Tree.into(),
+            relation: Some(Relation::Parent(id)),
+        }
+    }
+
+    fn tree_deletion(id: ChangeId) -> Self {
+        Change {
+            id: NULL_ID,
+            kind: ChangeKind::Deletion,
+            mode: EntryKind::Tree.into(),
+            relation: Some(Relation::Parent(id)),
         }
     }
 }

gix-diff/tests/rewrites/tracker.rs

@@ -1,3 +1,8 @@
+use crate::{
+    hex_to_id,
+    rewrites::{Change, NULL_ID},
+};
+use gix_diff::tree::visit::Relation;
 use gix_diff::{
     blob::DiffLineStats,
     rewrites,
@@ -14,11 +19,6 @@ use gix_diff::{
 use gix_object::tree::EntryKind;
 use pretty_assertions::assert_eq;
 
-use crate::{
-    hex_to_id,
-    rewrites::{Change, NULL_ID},
-};
-
 #[test]
 fn rename_by_id() -> crate::Result {
     // Limits are only applied when doing rewrite-checks
@@ -522,6 +522,106 @@ fn rename_by_50_percent_similarity() -> crate::Result {
     Ok(())
 }
 
+#[test]
+fn directories_without_relation_are_ignored() -> crate::Result {
+    let mut track = util::new_tracker(Default::default());
+    let tree_without_relation = Change {
+        id: NULL_ID,
+        kind: ChangeKind::Deletion,
+        mode: EntryKind::Tree.into(),
+        relation: None,
+    };
+    assert_eq!(
+        track.try_push_change(tree_without_relation, "dir".into()),
+        Some(tree_without_relation),
+        "trees, or non-blobs, are ignored, particularly when they have no relation"
+    );
+    Ok(())
+}
+
+#[test]
+#[ignore = "cannot match directories if their content doesn't match"]
+fn simple_directory_rename_with_changes() -> crate::Result {
+    Ok(())
+}
+
+#[test]
+#[ignore = "one side has relationships, the other side does not, it still works on leaf-level"]
+fn directory_rename_with_ids_one_side_without_on_the_other() -> crate::Result {
+    Ok(())
+}
+
+#[test]
+#[ignore = "TBD"]
+fn simple_directory_rename_by_id() -> crate::Result {
+    let rewrites = Rewrites {
+        copies: None,
+        percentage: Some(0.5),
+        limit: 0,
+    };
+    let mut track = util::new_tracker(Default::default());
+    let tree_dst_id = 1;
+    assert!(track
+        .try_push_change(Change::tree_addition(tree_dst_id), "d-renamed".into())
+        .is_none());
+    let tree_src_id = 3;
+    assert!(track
+        .try_push_change(Change::tree_deletion(tree_src_id), "d".into())
+        .is_none());
+    assert!(
+        track
+            .try_push_change(
+                Change {
+                    id: NULL_ID, /* does not matter for trees */
+                    kind: ChangeKind::Deletion,
+                    mode: EntryKind::Tree.into(),
+                    relation: Some(Relation::ChildOfParent(tree_src_id)),
+                },
+                "d/subdir".into(),
+            )
+            .is_some(),
+        "trees that are children are simply ignored. It's easier to compare views of worktrees (`gix-dirwalk`) \
+        and trees/indices that way."
+    );
+    assert!(track
+        .try_push_change(
+            Change {
+                id: NULL_ID,
+                kind: ChangeKind::Addition,
+                mode: EntryKind::Tree.into(),
+                relation: Some(Relation::ChildOfParent(tree_dst_id)),
+            },
+            "d-renamed/subdir".into(),
+        )
+        .is_some());
+    let _odb = util::add_retained_blobs(
+        &mut track,
+        [
+            (Change::deletion_in_tree(tree_src_id), "d/a", "a"),
+            (Change::deletion_in_tree(tree_src_id), "d/b", "b"),
+            (Change::deletion_in_tree(tree_src_id), "d/c", "c"),
+            (Change::deletion_in_tree(tree_src_id), "d/subdir/d", "d"),
+            (Change::addition_in_tree(tree_dst_id), "d-renamed/a", "a"),
+            (Change::addition_in_tree(tree_dst_id), "d-renamed/b", "b"),
+            (Change::addition_in_tree(tree_dst_id), "d-renamed/c", "c"),
+            (Change::addition_in_tree(tree_dst_id), "d-renamed/subdir/d", "d"),
+            (Change::deletion(), "a", "first\nsecond\n"),
+            (Change::addition(), "b", "firt\nsecond\n"),
+        ],
+    );
+    let mut called = false;
+    let out = util::assert_emit(&mut track, |dst, src| {
+        called = true;
+        assert_eq!(src, None, "there is just a single deletion, no pair");
+        assert_eq!(dst.location, "a");
+        assert_eq!(dst.change.kind, ChangeKind::Deletion);
+        Action::Continue
+    });
+    assert_eq!(out, Default::default());
+    assert!(called);
+    Ok(())
+}
+
 #[test]
 fn remove_only() -> crate::Result {
     let mut track = util::new_tracker(Default::default());
@@ -554,7 +654,7 @@ fn add_only() -> crate::Result {
     let out = util::assert_emit(&mut track, |dst, src| {
         assert!(!called);
         called = true;
-        assert!(src.is_none(), "there is just a single deletion, no pair");
+        assert!(src.is_none(), "there is just a single addition, no pair");
         assert_eq!(dst.location, "a");
         assert_eq!(dst.change.kind, ChangeKind::Addition);
         Action::Continue