delegate-based tree diff traversal for maximum flexibility and performance

Author: Byron

Cargo.lock

@@ -105,3 +105,7 @@ members = [
 
     "tests/tools",
 ]
+
+[patch.crates-io]
+# Until https://github.com/tailhook/quick-error/pull/56 is merged
+quick-error = {path = "/Users/byron/dev/github.com/tailhook/quick-error"}

Cargo.toml

@@ -18,6 +18,7 @@ git-hash = { version = "^0.2.0", path = "../git-hash" }
 git-object = { version = "0.7", path = "../git-object" }
 git-odb = { version = "0.12", path = "../git-odb" }
 petgraph = { version = "0.5.1", default-features = false }
+quick-error = "2.0.0"
 
 [dev-dependencies]
 test-tools = { path = "../tests/tools" }

git-diff/Cargo.toml

@@ -1,99 +1,129 @@
 // TODO: add deny(missing_docs)
 #![forbid(unsafe_code, rust_2018_idioms)]
-#![allow(dead_code, unused_variables)]
 
 ///
 pub mod tree {
     use git_hash::{oid, ObjectId};
-    use git_object::{
-        bstr::{BStr, BString},
-        immutable, tree,
-    };
+    use git_object::immutable;
+    use quick_error::quick_error;
 
     const EMPTY_TREE: immutable::Tree<'static> = immutable::Tree::empty();
 
-    type PathNodeId = usize;
-
-    /// A tree of paths to make storage and allocation of paths more efficient.
-    #[derive(Default)]
-    pub struct PathTree {
-        graph: petgraph::graph::DiGraph<BString, ()>,
-    }
-
-    impl PathTree {
-        /// Find the path with the given `id` and place all of its elements in `out_elements`.
-        /// Returns the amount of elements the path contains, like `&["a", "b", "c"]` would be `Some(3)`.
-        ///
-        /// The output vector will be cleared beforehand.
-        pub fn elements(&self, id: PathNodeId, out_elements: &mut Vec<&BStr>) -> Option<usize> {
-            todo!("fetch elements")
+    quick_error! {
+        #[derive(Debug)]
+        pub enum Error {
+            NotFound(oid: ObjectId) {
+                display("The object {} referenced by the tree was not found in the database", oid)
+            }
+            Cancelled {
+                display("The delegate cancelled the operation")
+            }
         }
     }
 
-    pub enum Change {
-        Addition {
-            mode: tree::Mode,
-            oid: ObjectId,
-            path: PathNodeId,
-        },
-        Copy,
-        Deletion,
-        Modification,
-        Renaming,
-        Type,
+    #[derive(Default, Clone)]
+    pub struct State {
+        buf1: Vec<u8>,
+        buf2: Vec<u8>,
     }
 
-    pub struct Changes<'a>(Option<&'a immutable::Tree<'a>>);
+    pub struct VisitChanges<'a>(Option<&'a immutable::Tree<'a>>);
 
-    impl<'a, T> From<T> for Changes<'a>
+    impl<'a, T> From<T> for VisitChanges<'a>
     where
         T: Into<Option<&'a immutable::Tree<'a>>>,
     {
         fn from(v: T) -> Self {
-            Changes(v.into())
+            VisitChanges(v.into())
         }
     }
 
-    // Possible things to detect (from git diff --help)
-    // o   A: addition of a file
-    // o   C: copy of a file into a new one
-    // o   D: deletion of a file
-    // o   M: modification of the contents or mode of a file
-    // o   R: renaming of a file
-    // o   T: change in the type of the file
-    // o   U: file is unmerged (you must complete the merge before it can be committed)
-    // o   X: "unknown" change type (most probably a bug, please report it)
-    impl<'a> Changes<'a> {
+    impl<'a> VisitChanges<'a> {
         /// Returns the changes that need to be applied to `self` to get `other`.
         pub fn to_obtain<LocateFn>(
             &self,
             _other: &git_object::immutable::Tree<'_>,
-            buf: (&mut Vec<u8>, &mut Vec<u8>),
-            locate: LocateFn,
-            out_paths: &mut PathTree,
-            out_changes: &mut Vec<Change>,
-        ) where
-            LocateFn: for<'b> FnMut(&oid, &'b mut Vec<u8>) -> Option<immutable::Tree<'b>>,
+            _state: &mut State,
+            _locate: LocateFn,
+            _delegate: &mut impl Delegate,
+        ) -> Result<(), Error>
+        where
+            LocateFn: for<'b> FnMut(&oid, &'b mut Vec<u8>) -> Option<immutable::Object<'b>>,
         {
-            out_paths.graph.clear();
-            out_changes.clear();
-
             let _this = *self.0.as_ref().unwrap_or(&&EMPTY_TREE);
             todo!("changes tree to tree")
         }
     }
 
-    #[cfg(test)]
-    mod tests {
-        use super::*;
-
-        #[test]
-        fn size_of_change() {
-            assert_eq!(
-                std::mem::size_of::<Change>(),
-                32,
-                "this type shouldn't grow without us knowing"
-            )
+    pub mod delegate {
+        use git_hash::ObjectId;
+        use git_object::{bstr::BStr, tree};
+
+        pub enum Change {
+            Addition { mode: tree::Mode, oid: ObjectId },
+            Copy,
+            Deletion,
+            Modification,
+            Renaming,
+            Type,
+        }
+
+        #[derive(Clone, Copy, PartialOrd, PartialEq, Ord, Eq, Hash)]
+        pub enum PathComponentMode {
+            Replace,
+            Push,
+        }
+
+        #[derive(Clone, Copy, PartialOrd, PartialEq, Ord, Eq, Hash)]
+        pub struct PathComponent<'a> {
+            pub component: &'a BStr,
+            /// An ID referring uniquely to the path built thus far. Used to keep track of source paths
+            /// in case of [renames][Change::Rename] and [copies][Change::Copy].
+            pub id: usize,
+        }
+
+        #[derive(Clone, Copy, PartialOrd, PartialEq, Ord, Eq, Hash)]
+        pub enum Action {
+            Continue,
+            Cancel,
+        }
+
+        pub trait Delegate {
+            fn update_path_component(&mut self, component: PathComponent<'_>, mode: PathComponentMode);
+            fn pop_path_component(&mut self);
+            fn record(change: Change) -> Action;
+        }
+
+        #[derive(Clone, Default)]
+        pub struct Recorder;
+
+        impl Delegate for Recorder {
+            fn update_path_component(&mut self, _component: PathComponent<'_>, _mode: PathComponentMode) {
+                todo!()
+            }
+
+            fn pop_path_component(&mut self) {
+                todo!()
+            }
+
+            fn record(_change: Change) -> Action {
+                todo!()
+            }
+        }
+
+        #[cfg(test)]
+        mod tests {
+            use super::*;
+
+            #[test]
+            fn size_of_change() {
+                assert_eq!(
+                    std::mem::size_of::<Change>(),
+                    22,
+                    "this type shouldn't grow without us knowing"
+                )
+            }
         }
     }
+    pub use delegate::Delegate;
 }

git-diff/src/lib.rs

@@ -41,14 +41,12 @@ mod with_tree {
                 .and_then(|tree| tree.into_tree())
         };
 
-        let (mut buf3, mut buf4) = (Vec::new(), Vec::new());
-        git_diff::tree::Changes::from(previous_tree.as_ref()).to_obtain(
+        git_diff::tree::VisitChanges::from(previous_tree.as_ref()).to_obtain(
             &main_tree,
-            (&mut buf3, &mut buf4),
+            &mut git_diff::tree::State::default(),
             |_oid, _buf| todo!("Actual lookup in db"),
-            &mut git_diff::tree::PathTree::default(),
-            &mut Vec::new(),
-        );
+            &mut git_diff::tree::delegate::Recorder::default(),
+        )?;
         Ok(())
     }
     #[test]