'consecutive' negotiation algorithm (Part 1)

Cargo.toml

@@ -253,7 +253,6 @@ members = [
     "cargo-smart-release",
     "tests/tools",
 
-    "gix-revision/tests",
     "gix-diff/tests",
     "gix-pack/tests",
     "gix-index/tests",

crate-status.md

@@ -469,6 +469,7 @@ Make it the best-performing implementation and the most convenient one.
 
 ### gix-revision
 * [x] `describe()` (similar to `git name-rev`)
+* [x] primitives to help with graph traversal, along with commit-graph acceleration.
 * parse specifications 
     * [x] parsing and navigation
     * [x] revision ranges
@@ -638,7 +639,7 @@ See its [README.md](https://github.com/Byron/gitoxide/blob/main/gix-lock/README.
     * **Id**
         * [x] short hashes with detection of ambiguity.
     * **Commit**
-        * [x] `describe()` like functionality
+        * [x] `git describe` like functionality, with optional commit-graph acceleration
         * [x] create new commit from tree
     * **Objects**
         * [x] lookup

gitoxide-core/Cargo.toml

@@ -33,15 +33,14 @@ async-client = ["gix/async-network-client-async-std", "gix-transport-configurati
 
 #! ### Other
 ## Data structures implement `serde::Serialize` and `serde::Deserialize`.
-serde = ["gix-commitgraph/serde", "gix/serde", "serde_json", "dep:serde", "bytesize/serde"]
+serde = ["gix/serde", "serde_json", "dep:serde", "bytesize/serde"]
 
 
 [dependencies]
 # deselect everything else (like "performance") as this should be controllable by the parent application.
 gix = { version = "^0.44.1", path = "../gix", default-features = false }
 gix-pack-for-configuration-only = { package = "gix-pack", version = "^0.35.0", path = "../gix-pack", default-features = false, features = ["pack-cache-lru-dynamic", "pack-cache-lru-static"] }
 gix-transport-configuration-only = { package = "gix-transport", version = "^0.31.0", path = "../gix-transport", default-features = false }
-gix-commitgraph = { version = "^0.14.0", path = "../gix-commitgraph" }
 serde = { version = "1.0.114", optional = true, default-features = false, features = ["derive"] }
 anyhow = "1.0.42"
 thiserror = "1.0.34"

gitoxide-core/src/commitgraph/verify.rs

@@ -1,7 +1,7 @@
 use std::{io, path::Path};
 
 use anyhow::{Context as AnyhowContext, Result};
-use gix_commitgraph::{graph::verify::Outcome, Graph};
+use gix::commitgraph::Graph;
 
 use crate::OutputFormat;
 
@@ -31,15 +31,15 @@ pub fn graph_or_file<W1, W2>(
         mut out,
         output_statistics,
     }: Context<W1, W2>,
-) -> Result<gix_commitgraph::graph::verify::Outcome>
+) -> Result<gix::commitgraph::verify::Outcome>
 where
     W1: io::Write,
     W2: io::Write,
 {
     let g = Graph::at(path).with_context(|| "Could not open commit graph")?;
 
     #[allow(clippy::unnecessary_wraps, unknown_lints)]
-    fn noop_processor(_commit: &gix_commitgraph::file::Commit<'_>) -> std::result::Result<(), std::fmt::Error> {
+    fn noop_processor(_commit: &gix::commitgraph::file::Commit<'_>) -> std::result::Result<(), std::fmt::Error> {
         Ok(())
     }
     let stats = g
@@ -57,7 +57,7 @@ where
     Ok(stats)
 }
 
-fn print_human_output(out: &mut impl io::Write, stats: &Outcome) -> io::Result<()> {
+fn print_human_output(out: &mut impl io::Write, stats: &gix::commitgraph::verify::Outcome) -> io::Result<()> {
     writeln!(out, "number of commits with the given number of parents")?;
     let mut parent_counts: Vec<_> = stats.parent_counts.iter().map(|(a, b)| (*a, *b)).collect();
     parent_counts.sort_by_key(|e| e.0);

gix-commitgraph/src/graph/access.rs → gix-commitgraph/src/access.rs

@@ -1,15 +1,12 @@
-use crate::{
-    file::{self, Commit, File},
-    graph::{self, Graph},
-};
+use crate::{file, file::Commit, File, Graph, Position};
 
 /// Access
 impl Graph {
     /// Returns the commit at the given position `pos`.
     ///
     /// # Panics
     /// If `pos` is greater or equal to [`num_commits()`][Graph::num_commits()].
-    pub fn commit_at(&self, pos: graph::Position) -> Commit<'_> {
+    pub fn commit_at(&self, pos: Position) -> Commit<'_> {
         let r = self.lookup_by_pos(pos);
         r.file.commit_at(r.pos)
     }
@@ -24,7 +21,7 @@ impl Graph {
     ///
     /// # Panics
     /// If `pos` is greater or equal to [`num_commits()`][Graph::num_commits()].
-    pub fn id_at(&self, pos: graph::Position) -> &gix_hash::oid {
+    pub fn id_at(&self, pos: Position) -> &gix_hash::oid {
         let r = self.lookup_by_pos(pos);
         r.file.id_at(r.pos)
     }
@@ -40,7 +37,7 @@ impl Graph {
     }
 
     /// Translate the given `id` to its position in the file.
-    pub fn lookup(&self, id: impl AsRef<gix_hash::oid>) -> Option<graph::Position> {
+    pub fn lookup(&self, id: impl AsRef<gix_hash::oid>) -> Option<Position> {
         Some(self.lookup_by_id(id.as_ref())?.graph_pos)
     }
 
@@ -59,15 +56,15 @@ impl Graph {
                 return Some(LookupByIdResult {
                     file,
                     file_pos: lex_pos,
-                    graph_pos: graph::Position(current_file_start + lex_pos.0),
+                    graph_pos: Position(current_file_start + lex_pos.0),
                 });
             }
             current_file_start += file.num_commits();
         }
         None
     }
 
-    fn lookup_by_pos(&self, pos: graph::Position) -> LookupByPositionResult<'_> {
+    fn lookup_by_pos(&self, pos: Position) -> LookupByPositionResult<'_> {
         let mut remaining = pos.0;
         for (file_index, file) in self.files.iter().enumerate() {
             match remaining.checked_sub(file.num_commits()) {
@@ -88,7 +85,7 @@ impl Graph {
 #[derive(Clone)]
 struct LookupByIdResult<'a> {
     pub file: &'a File,
-    pub graph_pos: graph::Position,
+    pub graph_pos: Position,
     pub file_pos: file::Position,
 }
 

gix-commitgraph/src/file/access.rs

@@ -4,7 +4,10 @@ use std::{
     path::Path,
 };
 
-use crate::file::{self, commit::Commit, File, COMMIT_DATA_ENTRY_SIZE_SANS_HASH};
+use crate::{
+    file::{self, commit::Commit, COMMIT_DATA_ENTRY_SIZE_SANS_HASH},
+    File,
+};
 
 /// Access
 impl File {

gix-commitgraph/src/file/commit.rs

@@ -6,8 +6,8 @@ use std::{
 };
 
 use crate::{
-    file::{self, File, EXTENDED_EDGES_MASK, LAST_EXTENDED_EDGE_MASK, NO_PARENT},
-    graph,
+    file::{self, EXTENDED_EDGES_MASK, LAST_EXTENDED_EDGE_MASK, NO_PARENT},
+    File, Position,
 };
 
 /// The error used in the [`file::commit`][self] module.
@@ -25,6 +25,7 @@ pub enum Error {
 }
 
 /// A commit as stored in a [`File`].
+#[derive(Copy, Clone)]
 pub struct Commit<'a> {
     file: &'a File,
     pos: file::Position,
@@ -72,11 +73,11 @@ impl<'a> Commit<'a> {
     }
 
     /// Returns an iterator over the parent positions for lookup in the owning [Graph][crate::Graph].
-    pub fn iter_parents(&'a self) -> impl Iterator<Item = Result<graph::Position, Error>> + 'a {
+    pub fn iter_parents(self) -> Parents<'a> {
         // I didn't find a combinator approach that a) was as strict as ParentIterator, b) supported
         // fuse-after-first-error behavior, and b) was significantly shorter or more understandable
         // than ParentIterator. So here we are.
-        ParentIterator {
+        Parents {
             commit_data: self,
             state: ParentIteratorState::First,
         }
@@ -88,7 +89,7 @@ impl<'a> Commit<'a> {
     }
 
     /// Returns the first parent of this commit.
-    pub fn parent1(&self) -> Result<Option<graph::Position>, Error> {
+    pub fn parent1(&self) -> Result<Option<Position>, Error> {
         self.iter_parents().next().transpose()
     }
 
@@ -127,13 +128,13 @@ impl<'a> PartialEq for Commit<'a> {
 }
 
 /// An iterator over parents of a [`Commit`].
-pub struct ParentIterator<'a> {
-    commit_data: &'a Commit<'a>,
+pub struct Parents<'a> {
+    commit_data: Commit<'a>,
     state: ParentIteratorState<'a>,
 }
 
-impl<'a> Iterator for ParentIterator<'a> {
-    type Item = Result<graph::Position, Error>;
+impl<'a> Iterator for Parents<'a> {
+    type Item = Result<Position, Error>;
 
     fn next(&mut self) -> Option<Self::Item> {
         let state = std::mem::replace(&mut self.state, ParentIteratorState::Exhausted);
@@ -221,7 +222,7 @@ enum ParentIteratorState<'a> {
 #[derive(Clone, Copy, Debug)]
 enum ParentEdge {
     None,
-    GraphPosition(graph::Position),
+    GraphPosition(Position),
     ExtraEdgeIndex(u32),
 }
 
@@ -233,22 +234,22 @@ impl ParentEdge {
         if raw & EXTENDED_EDGES_MASK != 0 {
             ParentEdge::ExtraEdgeIndex(raw & !EXTENDED_EDGES_MASK)
         } else {
-            ParentEdge::GraphPosition(graph::Position(raw))
+            ParentEdge::GraphPosition(Position(raw))
         }
     }
 }
 
 enum ExtraEdge {
-    Internal(graph::Position),
-    Last(graph::Position),
+    Internal(Position),
+    Last(Position),
 }
 
 impl ExtraEdge {
     pub fn from_raw(raw: u32) -> Self {
         if raw & LAST_EXTENDED_EDGE_MASK != 0 {
-            Self::Last(graph::Position(raw & !LAST_EXTENDED_EDGE_MASK))
+            Self::Last(Position(raw & !LAST_EXTENDED_EDGE_MASK))
         } else {
-            Self::Internal(graph::Position(raw))
+            Self::Internal(Position(raw))
         }
     }
 }

gix-commitgraph/src/file/init.rs

@@ -6,9 +6,12 @@ use std::{
 use bstr::ByteSlice;
 use memmap2::Mmap;
 
-use crate::file::{
-    ChunkId, File, BASE_GRAPHS_LIST_CHUNK_ID, COMMIT_DATA_CHUNK_ID, COMMIT_DATA_ENTRY_SIZE_SANS_HASH,
-    EXTENDED_EDGES_LIST_CHUNK_ID, FAN_LEN, HEADER_LEN, OID_FAN_CHUNK_ID, OID_LOOKUP_CHUNK_ID, SIGNATURE,
+use crate::{
+    file::{
+        ChunkId, BASE_GRAPHS_LIST_CHUNK_ID, COMMIT_DATA_CHUNK_ID, COMMIT_DATA_ENTRY_SIZE_SANS_HASH,
+        EXTENDED_EDGES_LIST_CHUNK_ID, FAN_LEN, HEADER_LEN, OID_FAN_CHUNK_ID, OID_LOOKUP_CHUNK_ID, SIGNATURE,
+    },
+    File,
 };
 
 /// The error used in [`File::at()`].

gix-commitgraph/src/file/mod.rs

@@ -1,12 +1,6 @@
 //! Operations on a single commit-graph file.
 
-use std::{
-    fmt::{Display, Formatter},
-    ops::Range,
-    path::PathBuf,
-};
-
-use memmap2::Mmap;
+use std::fmt::{Display, Formatter};
 
 pub use self::{commit::Commit, init::Error};
 
@@ -16,7 +10,7 @@ mod init;
 pub mod verify;
 
 const COMMIT_DATA_ENTRY_SIZE_SANS_HASH: usize = 16;
-const FAN_LEN: usize = 256;
+pub(crate) const FAN_LEN: usize = 256;
 const HEADER_LEN: usize = 8;
 
 const SIGNATURE: &[u8] = b"CGPH";
@@ -34,31 +28,14 @@ const NO_PARENT: u32 = 0x7000_0000;
 const EXTENDED_EDGES_MASK: u32 = 0x8000_0000;
 const LAST_EXTENDED_EDGE_MASK: u32 = 0x8000_0000;
 
-/// A single commit-graph file.
-///
-/// All operations on a `File` are local to that graph file. Since a commit graph can span multiple
-/// files, all interesting graph operations belong on [`Graph`][crate::Graph].
-pub struct File {
-    base_graph_count: u8,
-    base_graphs_list_offset: Option<usize>,
-    commit_data_offset: usize,
-    data: Mmap,
-    extra_edges_list_range: Option<Range<usize>>,
-    fan: [u32; FAN_LEN],
-    oid_lookup_offset: usize,
-    path: PathBuf,
-    hash_len: usize,
-    object_hash: gix_hash::Kind,
-}
-
 /// The position of a given commit within a graph file, starting at 0.
 ///
-/// Commits within a graph file are sorted in lexicographical order by OID; a commit's lexigraphical position
+/// Commits within a graph file are sorted in lexicographical order by OID; a commit's lexicographical position
 /// is its position in this ordering. If a commit graph spans multiple files, each file's commits
-/// start at lexigraphical position 0, so it is unique across a single file but is not unique across
-/// the whole commit graph. Each commit also has a graph position ([`graph::Position`][crate::graph::Position]),
-/// which is unique /// across the whole commit graph. In order to avoid accidentally mixing lexigraphical positions with graph
-/// positions, distinct types are used for each.
+/// start at lexicographical position 0, so it is unique across a single file but is not unique across
+/// the whole commit graph. Each commit also has a graph position ([`Position`][crate::Position]),
+/// which is unique across the whole commit graph.
+/// In order to avoid accidentally mixing lexicographical positions with graph positions, distinct types are used for each.
 #[derive(Clone, Copy, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)]
 pub struct Position(pub u32);
 

gix-commitgraph/src/file/verify.rs

@@ -5,10 +5,7 @@ use std::{
     path::Path,
 };
 
-use crate::{
-    file::{self, File},
-    GENERATION_NUMBER_INFINITY, GENERATION_NUMBER_MAX,
-};
+use crate::{file, File, GENERATION_NUMBER_INFINITY, GENERATION_NUMBER_MAX};
 
 /// The error used in [`File::traverse()`].
 #[derive(thiserror::Error, Debug)]