feat!: revision::Walk yields revision::Info structs instead of Ids.

This enables re-use of information that was already obtained, like the parents of
a commit and possibly its commit-time.

Author: Byron

gix/examples/stats.rs

@@ -1,37 +1,39 @@
+use gix::prelude::ObjectIdExt;
 use gix::Reference;
+use std::io::Write;
 
 fn main() -> Result<(), Box<dyn std::error::Error>> {
     let mut repo = gix::discover(".")?;
     println!("Repo: {}", repo.work_dir().unwrap_or_else(|| repo.git_dir()).display());
-    let mut max_commit_size = 0;
-    let mut avg_commit_size = 0;
+    let mut max_parents = 0;
+    let mut avg_parents = 0;
     repo.object_cache_size(32 * 1024);
-    let commit_ids = repo
+    let mut most_recent_commit_id = None;
+    let num_commits = repo
         .head()?
         .into_fully_peeled_id()
-        .ok_or("There are no commits - nothing to do here.")??
+        .ok_or("Cannot provide meaningful stats on empty repos")??
         .ancestors()
         .all()?
-        .inspect(|id| {
-            if let Ok(Ok(object)) = id.as_ref().map(|id| id.object()) {
-                avg_commit_size += object.data.len();
-                if object.data.len() > max_commit_size {
-                    max_commit_size = object.data.len();
-                }
+        .map_while(Result::ok)
+        .inspect(|commit| {
+            if most_recent_commit_id.is_none() {
+                most_recent_commit_id = Some(commit.id);
             }
+            avg_parents += commit.parent_ids.len();
+            max_parents = max_parents.max(commit.parent_ids.len());
         })
-        .collect::<Result<Vec<_>, _>>()?;
-    println!("Num Commits: {}", commit_ids.len());
-    println!("Max commit Size: {max_commit_size}");
-    println!("Avg commit Size: {}", avg_commit_size / commit_ids.len());
-    assert!(!commit_ids.is_empty(), "checked that before");
+        .count();
+    println!("Num Commits: {num_commits}");
+    println!("Max parents: {max_parents}");
+    println!("Avg parents: {}", avg_parents / num_commits);
 
-    let last_commit_id = &commit_ids[0];
     println!("Most recent commit message");
 
-    let object = last_commit_id.object()?;
+    let object = most_recent_commit_id.expect("already checked").attach(&repo).object()?;
     let commit = object.into_commit();
     println!("{}", commit.message_raw()?);
+    std::io::stdout().flush()?;
 
     let tree = commit.tree()?;
 

gix/src/revision/spec/parse/delegate/navigate.rs

@@ -65,7 +65,7 @@ impl<'repo> delegate::Navigate for Delegate<'repo> {
                         .filter_map(Result::ok)
                         .next()
                     {
-                        Some(id) => replacements.push((*obj, id.detach())),
+                        Some(commit) => replacements.push((*obj, commit.id)),
                         None => errors.push((
                             *obj,
                             Error::AncestorOutOfRange {
@@ -193,8 +193,8 @@ impl<'repo> delegate::Navigate for Delegate<'repo> {
                             let mut matched = false;
                             let mut count = 0;
                             let commits = iter.map(|res| {
-                                res.map_err(Error::from).and_then(|commit_id| {
-                                    commit_id.object().map_err(Error::from).map(|obj| obj.into_commit())
+                                res.map_err(Error::from).and_then(|commit| {
+                                    commit.id().object().map_err(Error::from).map(|obj| obj.into_commit())
                                 })
                             });
                             for commit in commits {
@@ -250,8 +250,8 @@ impl<'repo> delegate::Navigate for Delegate<'repo> {
                                 let mut matched = false;
                                 let mut count = 0;
                                 let commits = iter.map(|res| {
-                                    res.map_err(Error::from).and_then(|commit_id| {
-                                        commit_id.object().map_err(Error::from).map(|obj| obj.into_commit())
+                                    res.map_err(Error::from).and_then(|commit| {
+                                        commit.id().object().map_err(Error::from).map(|obj| obj.into_commit())
                                     })
                                 });
                                 for commit in commits {

gix/src/revision/walk.rs

@@ -1,6 +1,7 @@
 use gix_hash::ObjectId;
 use gix_odb::FindExt;
 
+use crate::ext::ObjectIdExt;
 use crate::{revision, Repository};
 
 /// The error returned by [`Platform::all()`].
@@ -13,6 +14,73 @@ pub enum Error {
     ShallowCommits(#[from] crate::shallow::open::Error),
 }
 
+/// Information about a commit that we obtained naturally as part of the iteration.
+#[derive(Debug, Clone)]
+pub struct Info<'repo> {
+    /// The detached id of the commit.
+    pub id: gix_hash::ObjectId,
+    /// All parent ids we have encountered. Note that these will be at most one if [`Parents::First`] is enabled.
+    pub parent_ids: gix_traverse::commit::ParentIds,
+    /// The time at which the commit was created. It's only `Some(_)` if sorting is not [`Sorting::BreadthFirst`], as the walk
+    /// needs to require the commit-date.
+    pub commit_time: Option<u64>,
+
+    repo: &'repo Repository,
+}
+
+/// Access
+impl<'repo> Info<'repo> {
+    /// Provide an attached version of our [`id`][Info::id] field.
+    pub fn id(&self) -> crate::Id<'repo> {
+        self.id.attach(self.repo)
+    }
+
+    /// Read the whole object from the object database.
+    ///
+    /// Note that this is an expensive operation which shouldn't be performed unless one needs more than parent ids
+    /// and commit time.
+    pub fn object(&self) -> Result<crate::Commit<'repo>, crate::object::find::existing::Error> {
+        Ok(self.id().object()?.into_commit())
+    }
+
+    /// Provide an iterator yielding attached versions of our [`parent_ids`][Info::parent_ids] field.
+    pub fn parent_ids(&self) -> impl Iterator<Item = crate::Id<'repo>> + '_ {
+        self.parent_ids.iter().map(|id| id.attach(self.repo))
+    }
+
+    /// Returns the commit-time of this commit.
+    ///
+    /// ### Panics
+    ///
+    /// If the iteration wasn't ordered by date.
+    pub fn commit_time(&self) -> u64 {
+        self.commit_time.expect("traversal involving date caused it to be set")
+    }
+}
+
+/// Initialization and detachment
+impl<'repo> Info<'repo> {
+    /// Create a new instance that represents `info`, but is attached to `repo` as well.
+    pub fn new(info: gix_traverse::commit::Info, repo: &'repo Repository) -> Self {
+        Info {
+            id: info.id,
+            parent_ids: info.parent_ids,
+            commit_time: info.commit_time,
+            repo,
+        }
+    }
+    /// Consume this instance and remove the reference to the underlying repository.
+    ///
+    /// This is useful for sending instances across threads, for example.
+    pub fn detach(self) -> gix_traverse::commit::Info {
+        gix_traverse::commit::Info {
+            id: self.id,
+            parent_ids: self.parent_ids,
+            commit_time: self.commit_time,
+        }
+    }
+}
+
 /// A platform to traverse the revision graph by adding starting points as well as points which shouldn't be crossed,
 /// returned by [`Repository::rev_walk()`].
 ///
@@ -41,7 +109,7 @@ impl<'repo> Platform<'repo> {
 
 /// Create-time builder methods
 impl<'repo> Platform<'repo> {
-    /// Set the sort mode for commits to the given value. The default is to order by topology.
+    /// Set the sort mode for commits to the given value. The default is to order topologically breadth-first.
     pub fn sorting(mut self, sorting: gix_traverse::commit::Sorting) -> Self {
         self.sorting = sorting;
         self
@@ -117,9 +185,7 @@ impl<'repo> Platform<'repo> {
                 )
                 .sorting(sorting)?
                 .parents(parents)
-                .commit_graph(use_commit_graph.then(|| self.repo.commit_graph().ok()).flatten())
-                // TODO: use wrapped items instead
-                .map(|res| res.map(|item| item.id)),
+                .commit_graph(use_commit_graph.then(|| self.repo.commit_graph().ok()).flatten()),
             ),
         })
     }
@@ -135,20 +201,21 @@ impl<'repo> Platform<'repo> {
 }
 
 pub(crate) mod iter {
-    use crate::{ext::ObjectIdExt, Id};
-
     /// The iterator returned by [`crate::revision::walk::Platform::all()`].
     pub struct Walk<'repo> {
         pub(crate) repo: &'repo crate::Repository,
-        pub(crate) inner:
-            Box<dyn Iterator<Item = Result<gix_hash::ObjectId, gix_traverse::commit::ancestors::Error>> + 'repo>,
+        pub(crate) inner: Box<
+            dyn Iterator<Item = Result<gix_traverse::commit::Info, gix_traverse::commit::ancestors::Error>> + 'repo,
+        >,
     }
 
     impl<'repo> Iterator for Walk<'repo> {
-        type Item = Result<Id<'repo>, gix_traverse::commit::ancestors::Error>;
+        type Item = Result<super::Info<'repo>, gix_traverse::commit::ancestors::Error>;
 
         fn next(&mut self) -> Option<Self::Item> {
-            self.inner.next().map(|res| res.map(|id| id.attach(self.repo)))
+            self.inner
+                .next()
+                .map(|res| res.map(|info| super::Info::new(info, self.repo)))
         }
     }
 }

gix/tests/id/mod.rs

@@ -75,13 +75,18 @@ mod ancestors {
     fn all() -> crate::Result {
         let repo = crate::repo("make_repo_with_fork_and_dates.sh")?.to_thread_local();
         let head = repo.head()?.into_fully_peeled_id().expect("born")?;
-        let commits_graph_order = head.ancestors().all()?.collect::<Result<Vec<_>, _>>()?;
+        let commits_graph_order = head
+            .ancestors()
+            .all()?
+            .map(|c| c.map(|c| c.detach()))
+            .collect::<Result<Vec<_>, _>>()?;
         assert_eq!(commits_graph_order.len(), 4, "need a specific amount of commits");
 
         let commits_by_commit_date = head
             .ancestors()
             .sorting(commit::Sorting::ByCommitTimeNewestFirst)
             .all()?
+            .map(|c| c.map(|c| c.detach()))
             .collect::<Result<Vec<_>, _>>()?;
         assert_eq!(
             commits_by_commit_date.len(),
@@ -123,6 +128,7 @@ mod ancestors {
                         id != hex_to_id("9902e3c3e8f0c569b4ab295ddf473e6de763e1e7")
                             && id != hex_to_id("bcb05040a6925f2ff5e10d3ae1f9264f2e8c43ac")
                     })?
+                    .map(|c| c.map(|c| c.id))
                     .collect::<Result<Vec<_>, _>>()?;
                 assert_eq!(
                     commits_graph_order,

gix/tests/repository/object.rs

@@ -125,15 +125,18 @@ mod find {
             }
             for commit_id in repo.head()?.peeled()?.id().expect("born").ancestors().all()? {
                 let commit = commit_id?;
-                assert_eq!(commit.object()?.kind, gix_object::Kind::Commit);
+                assert_eq!(commit.id().object()?.kind, gix_object::Kind::Commit);
                 if round == 2 {
                     assert_eq!(
-                        commit.object()?.kind,
+                        commit.id().object()?.kind,
                         gix_object::Kind::Commit,
                         "repeated request triggers cache and doesn't fail"
                     );
                 }
-                assert_eq!(commit.try_object()?.expect("exists").kind, gix_object::Kind::Commit,);
+                assert_eq!(
+                    commit.id().try_object()?.expect("exists").kind,
+                    gix_object::Kind::Commit,
+                );
             }
         }
         Ok(())

gix/tests/repository/shallow.rs

@@ -9,7 +9,12 @@ fn no() -> crate::Result {
         let repo = named_subrepo_opts("make_shallow_repo.sh", name, crate::restricted())?;
         assert!(!repo.is_shallow());
         assert!(repo.shallow_commits()?.is_none());
-        let commits: Vec<_> = repo.head_id()?.ancestors().all()?.collect::<Result<_, _>>()?;
+        let commits: Vec<_> = repo
+            .head_id()?
+            .ancestors()
+            .all()?
+            .map(|c| c.map(|c| c.id))
+            .collect::<Result<_, _>>()?;
         let expected = if name == "base" {
             vec![
                 hex_to_id("30887839de28edf7ab66c860e5c58b4d445f6b12"),
@@ -49,7 +54,12 @@ mod traverse {
     fn boundary_is_detected_triggering_no_error() -> crate::Result {
         for name in ["shallow.git", "shallow"] {
             let repo = named_subrepo_opts("make_shallow_repo.sh", name, crate::restricted())?;
-            let commits: Vec<_> = repo.head_id()?.ancestors().all()?.collect::<Result<_, _>>()?;
+            let commits: Vec<_> = repo
+                .head_id()?
+                .ancestors()
+                .all()?
+                .map(|c| c.map(|c| c.id))
+                .collect::<Result<_, _>>()?;
             assert_eq!(commits, [hex_to_id("30887839de28edf7ab66c860e5c58b4d445f6b12")]);
         }
         Ok(())
@@ -78,6 +88,7 @@ mod traverse {
                 .ancestors()
                 .sorting(Sorting::ByCommitTimeNewestFirst)
                 .all()?
+                .map(|c| c.map(|c| c.id))
                 .collect::<Result<_, _>>()?;
             assert_eq!(
                 commits,