adapt to changes in gix-revision

Author: Byron

gix-negotiate/src/consecutive.rs

@@ -19,7 +19,7 @@ impl Default for Algorithm {
 
 impl Algorithm {
     /// Add `id` to our priority queue and *add* `flags` to it.
-    fn add_to_queue(&mut self, id: ObjectId, mark: Flags, graph: &mut crate::Graph<'_>) -> Result<(), Error> {
+    fn add_to_queue(&mut self, id: ObjectId, mark: Flags, graph: &mut crate::Graph<'_, '_>) -> Result<(), Error> {
         let mut is_common = false;
         let mut has_mark = false;
         if let Some(commit) = graph
@@ -43,7 +43,7 @@ impl Algorithm {
         id: ObjectId,
         mode: Mark,
         ancestors: Ancestors,
-        graph: &mut crate::Graph<'_>,
+        graph: &mut crate::Graph<'_, '_>,
     ) -> Result<(), Error> {
         let mut is_common = false;
         if let Some(commit) = graph
@@ -89,7 +89,7 @@ impl Algorithm {
 }
 
 impl Negotiator for Algorithm {
-    fn known_common(&mut self, id: ObjectId, graph: &mut crate::Graph<'_>) -> Result<(), Error> {
+    fn known_common(&mut self, id: ObjectId, graph: &mut crate::Graph<'_, '_>) -> Result<(), Error> {
         if graph
             .get(&id)
             .map_or(true, |commit| !commit.data.flags.contains(Flags::SEEN))
@@ -100,11 +100,11 @@ impl Negotiator for Algorithm {
         Ok(())
     }
 
-    fn add_tip(&mut self, id: ObjectId, graph: &mut crate::Graph<'_>) -> Result<(), Error> {
+    fn add_tip(&mut self, id: ObjectId, graph: &mut crate::Graph<'_, '_>) -> Result<(), Error> {
         self.add_to_queue(id, Flags::SEEN, graph)
     }
 
-    fn next_have(&mut self, graph: &mut crate::Graph<'_>) -> Option<Result<ObjectId, Error>> {
+    fn next_have(&mut self, graph: &mut crate::Graph<'_, '_>) -> Option<Result<ObjectId, Error>> {
         loop {
             let id = self.revs.pop_value().filter(|_| self.non_common_revs != 0)?;
             let commit = graph.get_mut(&id).expect("it was added to the graph by now");
@@ -145,7 +145,7 @@ impl Negotiator for Algorithm {
         }
     }
 
-    fn in_common_with_remote(&mut self, id: ObjectId, graph: &mut crate::Graph<'_>) -> Result<bool, Error> {
+    fn in_common_with_remote(&mut self, id: ObjectId, graph: &mut crate::Graph<'_, '_>) -> Result<bool, Error> {
         let known_to_be_common = graph
             .get(&id)
             .map_or(false, |commit| commit.data.flags.contains(Flags::COMMON));

gix-negotiate/src/lib.rs

@@ -58,7 +58,7 @@ pub struct Metadata {
 }
 
 /// The graph our callers use to store traversal information, for (re-)use in the negotiation implementation.
-pub type Graph<'find> = gix_revwalk::Graph<'find, gix_revwalk::graph::Commit<Metadata>>;
+pub type Graph<'find, 'cache> = gix_revwalk::Graph<'find, 'cache, gix_revwalk::graph::Commit<Metadata>>;
 
 /// A map associating an object id with its commit-metadata.
 pub type IdMap = gix_revwalk::graph::IdMap<gix_revwalk::graph::Commit<Metadata>>;
@@ -125,22 +125,22 @@ pub trait Negotiator {
     /// Mark `id` as common between the remote and us.
     ///
     /// These ids are typically the local tips of remote tracking branches.
-    fn known_common(&mut self, id: gix_hash::ObjectId, graph: &mut Graph<'_>) -> Result<(), Error>;
+    fn known_common(&mut self, id: gix_hash::ObjectId, graph: &mut Graph<'_, '_>) -> Result<(), Error>;
 
     /// Add `id` as starting point of a traversal across commits that aren't necessarily common between the remote and us.
     ///
     /// These tips are usually the commits of local references whose tips should lead to objects that we have in common with the remote.
-    fn add_tip(&mut self, id: gix_hash::ObjectId, graph: &mut Graph<'_>) -> Result<(), Error>;
+    fn add_tip(&mut self, id: gix_hash::ObjectId, graph: &mut Graph<'_, '_>) -> Result<(), Error>;
 
     /// Produce the next id of an object that we want the server to know we have. It's an object we don't know we have in common or not.
     ///
     /// Returns `None` if we have exhausted all options, which might mean we have traversed the entire commit graph.
-    fn next_have(&mut self, graph: &mut Graph<'_>) -> Option<Result<gix_hash::ObjectId, Error>>;
+    fn next_have(&mut self, graph: &mut Graph<'_, '_>) -> Option<Result<gix_hash::ObjectId, Error>>;
 
     /// Mark `id` as being common with the remote (as informed by the remote itself) and return `true` if we knew it was common already.
     ///
     /// We can assume to have already seen `id` as we were the one to inform the remote in a prior `have`.
-    fn in_common_with_remote(&mut self, id: gix_hash::ObjectId, graph: &mut Graph<'_>) -> Result<bool, Error>;
+    fn in_common_with_remote(&mut self, id: gix_hash::ObjectId, graph: &mut Graph<'_, '_>) -> Result<bool, Error>;
 }
 
 /// An error that happened during any of the methods on a [`Negotiator`].

gix-negotiate/src/noop.rs

@@ -5,19 +5,19 @@ use crate::{Error, Negotiator};
 pub(crate) struct Noop;
 
 impl Negotiator for Noop {
-    fn known_common(&mut self, _id: ObjectId, _graph: &mut crate::Graph<'_>) -> Result<(), Error> {
+    fn known_common(&mut self, _id: ObjectId, _graph: &mut crate::Graph<'_, '_>) -> Result<(), Error> {
         Ok(())
     }
 
-    fn add_tip(&mut self, _id: ObjectId, _graph: &mut crate::Graph<'_>) -> Result<(), Error> {
+    fn add_tip(&mut self, _id: ObjectId, _graph: &mut crate::Graph<'_, '_>) -> Result<(), Error> {
         Ok(())
     }
 
-    fn next_have(&mut self, _graph: &mut crate::Graph<'_>) -> Option<Result<ObjectId, Error>> {
+    fn next_have(&mut self, _graph: &mut crate::Graph<'_, '_>) -> Option<Result<ObjectId, Error>> {
         None
     }
 
-    fn in_common_with_remote(&mut self, _id: ObjectId, _graph: &mut crate::Graph<'_>) -> Result<bool, Error> {
+    fn in_common_with_remote(&mut self, _id: ObjectId, _graph: &mut crate::Graph<'_, '_>) -> Result<bool, Error> {
         Ok(false)
     }
 }

gix-negotiate/src/skipping.rs

@@ -19,7 +19,7 @@ impl Default for Algorithm {
 
 impl Algorithm {
     /// Add `id` to our priority queue and *add* `flags` to it.
-    fn add_to_queue(&mut self, id: ObjectId, mark: Flags, graph: &mut crate::Graph<'_>) -> Result<(), Error> {
+    fn add_to_queue(&mut self, id: ObjectId, mark: Flags, graph: &mut crate::Graph<'_, '_>) -> Result<(), Error> {
         let commit = graph.try_lookup_or_insert_commit(id, |entry| {
             entry.flags |= mark | Flags::SEEN;
         })?;
@@ -32,7 +32,7 @@ impl Algorithm {
         Ok(())
     }
 
-    fn mark_common(&mut self, id: ObjectId, graph: &mut crate::Graph<'_>) -> Result<(), Error> {
+    fn mark_common(&mut self, id: ObjectId, graph: &mut crate::Graph<'_, '_>) -> Result<(), Error> {
         let mut is_common = false;
         if let Some(commit) = graph
             .try_lookup_or_insert_commit(id, |entry| {
@@ -76,7 +76,7 @@ impl Algorithm {
         &mut self,
         entry: Metadata,
         parent_id: ObjectId,
-        graph: &mut crate::Graph<'_>,
+        graph: &mut crate::Graph<'_, '_>,
     ) -> Result<bool, Error> {
         let mut was_seen = false;
         if let Some(parent) = graph
@@ -113,7 +113,7 @@ impl Algorithm {
 }
 
 impl Negotiator for Algorithm {
-    fn known_common(&mut self, id: ObjectId, graph: &mut crate::Graph<'_>) -> Result<(), Error> {
+    fn known_common(&mut self, id: ObjectId, graph: &mut crate::Graph<'_, '_>) -> Result<(), Error> {
         if graph
             .get(&id)
             .map_or(false, |commit| commit.data.flags.contains(Flags::SEEN))
@@ -123,7 +123,7 @@ impl Negotiator for Algorithm {
         self.add_to_queue(id, Flags::ADVERTISED, graph)
     }
 
-    fn add_tip(&mut self, id: ObjectId, graph: &mut crate::Graph<'_>) -> Result<(), Error> {
+    fn add_tip(&mut self, id: ObjectId, graph: &mut crate::Graph<'_, '_>) -> Result<(), Error> {
         if graph
             .get(&id)
             .map_or(false, |commit| commit.data.flags.contains(Flags::SEEN))
@@ -133,7 +133,7 @@ impl Negotiator for Algorithm {
         self.add_to_queue(id, Flags::default(), graph)
     }
 
-    fn next_have(&mut self, graph: &mut crate::Graph<'_>) -> Option<Result<ObjectId, Error>> {
+    fn next_have(&mut self, graph: &mut crate::Graph<'_, '_>) -> Option<Result<ObjectId, Error>> {
         loop {
             let id = self.revs.pop_value().filter(|_| self.non_common_revs != 0)?;
             let commit = graph.get_mut(&id).expect("it was added to the graph by now");
@@ -166,7 +166,7 @@ impl Negotiator for Algorithm {
         }
     }
 
-    fn in_common_with_remote(&mut self, id: ObjectId, graph: &mut crate::Graph<'_>) -> Result<bool, Error> {
+    fn in_common_with_remote(&mut self, id: ObjectId, graph: &mut crate::Graph<'_, '_>) -> Result<bool, Error> {
         let mut was_seen = false;
         let known_to_be_common = graph.get(&id).map_or(false, |commit| {
             was_seen = commit.data.flags.contains(Flags::SEEN);

gix-negotiate/tests/baseline/mod.rs

@@ -57,7 +57,7 @@ fn run() -> crate::Result {
                 let cache = use_cache
                     .then(|| gix_commitgraph::at(store.store_ref().path().join("info")).ok())
                     .flatten();
-                let mut graph = gix_revwalk::Graph::new(&store, cache);
+                let mut graph = gix_revwalk::Graph::new(&store, cache.as_ref());
                 let mut negotiator = algo.into_negotiator();
                 if debug {
                     eprintln!("ALGO {algo_name} CASE {case}");

gix-revision/src/describe.rs

@@ -160,7 +160,7 @@ pub(crate) mod function {
     /// candidate by setting `fallback_to_oid` to true.
     pub fn describe<'name>(
         commit: &oid,
-        graph: &mut Graph<'_, Flags>,
+        graph: &mut Graph<'_, '_, Flags>,
         Options {
             name_by_oid,
             mut max_candidates,
@@ -304,7 +304,7 @@ pub(crate) mod function {
     }
 
     fn parents_by_date_onto_queue_and_track_names(
-        graph: &mut Graph<'_, Flags>,
+        graph: &mut Graph<'_, '_, Flags>,
         queue: &mut PriorityQueue<CommitTime, gix_hash::ObjectId>,
         commit: gix_hash::ObjectId,
         commit_flags: Flags,
@@ -326,7 +326,7 @@ pub(crate) mod function {
 
     fn finish_depth_computation(
         mut queue: PriorityQueue<CommitTime, gix_hash::ObjectId>,
-        graph: &mut Graph<'_, Flags>,
+        graph: &mut Graph<'_, '_, Flags>,
         best_candidate: &mut Candidate<'_>,
         first_parent: bool,
     ) -> Result<u32, Error> {

gix-revision/src/merge_base.rs

@@ -42,7 +42,7 @@ pub(crate) mod function {
     pub fn merge_base(
         first: ObjectId,
         others: &[ObjectId],
-        graph: &mut Graph<'_, Flags>,
+        graph: &mut Graph<'_, '_, Flags>,
     ) -> Result<Option<Vec<ObjectId>>, Error> {
         let _span = gix_trace::coarse!("gix_revision::merge_base()", ?first, ?others,);
         if others.is_empty() || others.contains(&first) {
@@ -60,7 +60,7 @@ pub(crate) mod function {
     /// That way, we return only the topologically most recent commits in `commits`.
     fn remove_redundant(
         commits: &[(ObjectId, GenThenTime)],
-        graph: &mut Graph<'_, Flags>,
+        graph: &mut Graph<'_, '_, Flags>,
     ) -> Result<Vec<ObjectId>, Error> {
         if commits.is_empty() {
             return Ok(Vec::new());
@@ -151,7 +151,7 @@ pub(crate) mod function {
     fn paint_down_to_common(
         first: ObjectId,
         others: &[ObjectId],
-        graph: &mut Graph<'_, Flags>,
+        graph: &mut Graph<'_, '_, Flags>,
     ) -> Result<Vec<(ObjectId, GenThenTime)>, Error> {
         let mut queue = PriorityQueue::<GenThenTime, ObjectId>::new();
         graph.insert_data(first, |commit| -> Result<_, Error> {
@@ -213,10 +213,10 @@ pub(crate) mod function {
         time: gix_date::SecondsSinceUnixEpoch,
     }
 
-    impl TryFrom<gix_revwalk::graph::LazyCommit<'_>> for GenThenTime {
+    impl TryFrom<gix_revwalk::graph::LazyCommit<'_, '_>> for GenThenTime {
         type Error = gix_object::decode::Error;
 
-        fn try_from(commit: LazyCommit<'_>) -> Result<Self, Self::Error> {
+        fn try_from(commit: LazyCommit<'_, '_>) -> Result<Self, Self::Error> {
             Ok(GenThenTime {
                 generation: commit
                     .generation()

gix-revision/tests/describe/mod.rs

@@ -22,7 +22,7 @@ fn run_test(
         let cache = use_commitgraph
             .then(|| gix_commitgraph::Graph::from_info_dir(&store.store_ref().path().join("info")).ok())
             .flatten();
-        let mut graph = gix_revision::Graph::new(&store, cache);
+        let mut graph = gix_revision::Graph::new(&store, cache.as_ref());
         run_assertions(
             gix_revision::describe(&commit_id, &mut graph, options(commit_id)),
             commit_id,

gix-revision/tests/merge_base/mod.rs

@@ -13,10 +13,10 @@ mod baseline {
         for baseline_path in expectation_paths(&root)? {
             count += 1;
             for use_commitgraph in [false, true] {
+                let cache = use_commitgraph
+                    .then(|| gix_commitgraph::Graph::from_info_dir(&odb.store_ref().path().join("info")).unwrap());
                 for expected in parse_expectations(&baseline_path)? {
-                    let cache = use_commitgraph
-                        .then(|| gix_commitgraph::Graph::from_info_dir(&odb.store_ref().path().join("info")).unwrap());
-                    let mut graph = gix_revision::Graph::new(&odb, cache);
+                    let mut graph = gix_revision::Graph::new(&odb, cache.as_ref());
 
                     let actual = merge_base(expected.first, &expected.others, &mut graph)?;
                     assert_eq!(

gix-revwalk/src/graph/mod.rs

@@ -207,7 +207,7 @@ impl<'find, 'cache, T> Graph<'find, 'cache, T> {
     pub fn new(objects: impl gix_object::Find + 'find, cache: Option<&'cache gix_commitgraph::Graph>) -> Self {
         Graph {
             find: Box::new(objects),
-            cache: cache.into(),
+            cache,
             map: gix_hashtable::HashMap::default(),
             buf: Vec::new(),
             parent_buf: Vec::new(),

gix/src/commit.rs

@@ -73,6 +73,8 @@ pub mod describe {
     #[derive(Debug, thiserror::Error)]
     #[allow(missing_docs)]
     pub enum Error {
+        #[error(transparent)]
+        OpenCache(#[from] crate::repository::commit_graph_if_enabled::Error),
         #[error(transparent)]
         Describe(#[from] gix_revision::describe::Error),
         #[error("Could not produce an unambiguous shortened id for formatting.")]
@@ -218,11 +220,11 @@ pub mod describe {
         ///
         /// It is greatly recommended to [assure an object cache is set](crate::Repository::object_cache_size_if_unset())
         /// to save ~40% of time.
-        pub fn try_resolve(&self) -> Result<Option<Resolution<'repo>>, Error> {
-            let mut graph = gix_revwalk::Graph::new(
-                &self.repo.objects,
-                gix_commitgraph::Graph::from_info_dir(self.repo.objects.store_ref().path().join("info").as_ref()).ok(),
-            );
+        pub fn try_resolve_with_cache(
+            &self,
+            cache: Option<&'_ gix_commitgraph::Graph>,
+        ) -> Result<Option<Resolution<'repo>>, Error> {
+            let mut graph = self.repo.revision_graph(cache);
             let outcome = gix_revision::describe(
                 &self.id,
                 &mut graph,
@@ -240,6 +242,16 @@ pub mod describe {
             }))
         }
 
+        /// Like [`Self::try_resolve_with_cache()`], but obtains the commitgraph-cache internally for a single use.
+        ///
+        /// # Performance
+        ///
+        /// Prefer to use the [`Self::try_resolve_with_cache()`] method when processing more than one commit at a time.
+        pub fn try_resolve(&self) -> Result<Option<Resolution<'repo>>, Error> {
+            let cache = self.repo.commit_graph_if_enabled()?;
+            self.try_resolve_with_cache(cache.as_ref())
+        }
+
         /// Like [`try_format()`](Self::try_format()), but turns `id_as_fallback()` on to always produce a format.
         pub fn format(&mut self) -> Result<gix_revision::describe::Format<'static>, Error> {
             self.id_as_fallback = true;

gix/src/remote/connection/fetch/negotiate.rs

@@ -65,7 +65,7 @@ pub(crate) enum Action {
 pub(crate) fn mark_complete_and_common_ref(
     repo: &crate::Repository,
     negotiator: &mut dyn gix_negotiate::Negotiator,
-    graph: &mut gix_negotiate::Graph<'_>,
+    graph: &mut gix_negotiate::Graph<'_, '_>,
     ref_map: &fetch::RefMap,
     shallow: &fetch::Shallow,
     mapping_is_ignored: impl Fn(&fetch::Mapping) -> bool,
@@ -258,7 +258,7 @@ pub(crate) fn add_wants(
 /// Remove all commits that are more recent than the cut-off, which is the commit time of the oldest common commit we have with the server.
 fn mark_recent_complete_commits(
     queue: &mut Queue,
-    graph: &mut gix_negotiate::Graph<'_>,
+    graph: &mut gix_negotiate::Graph<'_, '_>,
     cutoff: SecondsSinceUnixEpoch,
 ) -> Result<(), Error> {
     let _span = gix_trace::detail!("mark_recent_complete", queue_len = queue.len());
@@ -286,7 +286,7 @@ fn mark_recent_complete_commits(
 
 fn mark_all_refs_in_repo(
     repo: &crate::Repository,
-    graph: &mut gix_negotiate::Graph<'_>,
+    graph: &mut gix_negotiate::Graph<'_, '_>,
     queue: &mut Queue,
     mark: Flags,
 ) -> Result<(), Error> {
@@ -310,7 +310,7 @@ fn mark_all_refs_in_repo(
 
 fn mark_alternate_complete(
     repo: &crate::Repository,
-    graph: &mut gix_negotiate::Graph<'_>,
+    graph: &mut gix_negotiate::Graph<'_, '_>,
     queue: &mut Queue,
 ) -> Result<(), Error> {
     let alternates = repo.objects.store_ref().alternate_db_paths()?;
@@ -332,7 +332,7 @@ fn mark_alternate_complete(
 /// Returns the amount of haves actually sent.
 pub(crate) fn one_round(
     negotiator: &mut dyn gix_negotiate::Negotiator,
-    graph: &mut gix_negotiate::Graph<'_>,
+    graph: &mut gix_negotiate::Graph<'_, '_>,
     haves_to_send: usize,
     arguments: &mut gix_protocol::fetch::Arguments,
     previous_response: Option<&gix_protocol::fetch::Response>,

gix/src/remote/connection/fetch/receive_pack.rs

@@ -155,7 +155,8 @@ where
             r.objects.unset_object_cache();
             r
         };
-        let mut graph = graph_repo.revision_graph();
+        let cache = graph_repo.commit_graph_if_enabled().ok().flatten();
+        let mut graph = graph_repo.revision_graph(cache.as_ref());
         let action = negotiate::mark_complete_and_common_ref(
             &graph_repo,
             negotiator.deref_mut(),

gix/src/repository/graph.rs

@@ -2,8 +2,8 @@ impl crate::Repository {
     /// Create a graph data-structure capable of accelerating graph traversals and storing state of type `T` with each commit
     /// it encountered.
     ///
-    /// Note that the commitgraph will be used if it is present and readable, but it won't be an error if it is corrupted. In that case,
-    /// it will just not be used.
+    /// Note that the `cache` will be used if present, and it's best obtained with
+    /// [`commit_graph_if_enabled()`](crate::Repository::commit_graph_if_enabled()).
     ///
     /// Note that a commitgraph is only allowed to be used if `core.commitGraph` is true (the default), and that configuration errors are
     /// ignored as well.
@@ -12,23 +12,34 @@ impl crate::Repository {
     ///
     /// Note that the [Graph][gix_revwalk::Graph] can be sensitive to various object database settings that may affect the performance
     /// of the commit walk.
-    pub fn revision_graph<T>(&self) -> gix_revwalk::Graph<'_, T> {
-        gix_revwalk::Graph::new(
-            &self.objects,
-            self.config
-                .may_use_commit_graph()
-                .unwrap_or(true)
-                .then(|| gix_commitgraph::at(self.objects.store_ref().path().join("info")).ok())
-                .flatten(),
-        )
+    pub fn revision_graph<'cache, T>(
+        &self,
+        cache: Option<&'cache gix_commitgraph::Graph>,
+    ) -> gix_revwalk::Graph<'_, 'cache, T> {
+        gix_revwalk::Graph::new(&self.objects, cache)
     }
 
     /// Return a cache for commits and their graph structure, as managed by `git commit-graph`, for accelerating commit walks on
     /// a low level.
     ///
     /// Note that [`revision_graph()`][crate::Repository::revision_graph()] should be preferred for general purpose walks that don't
-    /// rely on the actual commit cache to be present, while leveraging it if possible.
+    /// rely on the actual commit cache to be present, while leveraging the commit-graph if possible.
     pub fn commit_graph(&self) -> Result<gix_commitgraph::Graph, gix_commitgraph::init::Error> {
         gix_commitgraph::at(self.objects.store_ref().path().join("info"))
     }
+
+    /// Return a newly opened commit-graph if it is available *and* enabled in the Git configuration.
+    pub fn commit_graph_if_enabled(
+        &self,
+    ) -> Result<Option<gix_commitgraph::Graph>, super::commit_graph_if_enabled::Error> {
+        Ok(self
+            .config
+            .may_use_commit_graph()?
+            .then(|| gix_commitgraph::at(self.objects.store_ref().path().join("info")))
+            .transpose()
+            .or_else(|err| match err {
+                gix_commitgraph::init::Error::Io { err, .. } if err.kind() == std::io::ErrorKind::NotFound => Ok(None),
+                _ => Err(err),
+            })?)
+    }
 }

gix/src/repository/mod.rs

@@ -72,6 +72,19 @@ mod submodule;
 mod thread_safe;
 mod worktree;
 
+///
+pub mod commit_graph_if_enabled {
+    /// The error returned by [Repository::commit_graph_if_enabled()](crate::Repository::commit_graph_if_enabled()).
+    #[derive(Debug, thiserror::Error)]
+    #[allow(missing_docs)]
+    pub enum Error {
+        #[error(transparent)]
+        ConfigBoolean(#[from] crate::config::boolean::Error),
+        #[error(transparent)]
+        OpenCommitGraph(#[from] gix_commitgraph::init::Error),
+    }
+}
+
 ///
 #[cfg(feature = "index")]
 pub mod index_from_tree {