Support for in truncated history in git-describe (#298)

git-repository/src/commit.rs

@@ -21,7 +21,7 @@ pub mod describe {
     use crate::ext::ObjectIdExt;
     use crate::Repository;
     use git_hash::ObjectId;
-    use git_odb::FindExt;
+    use git_odb::Find;
     use std::borrow::Cow;
 
     /// The result of [try_resolve()][Platform::try_resolve()].
@@ -45,9 +45,7 @@ pub mod describe {
     #[allow(missing_docs)]
     pub enum Error {
         #[error(transparent)]
-        Describe(
-            #[from] git_revision::describe::Error<git_odb::find::existing_iter::Error<git_odb::store::find::Error>>,
-        ),
+        Describe(#[from] git_revision::describe::Error<git_odb::store::find::Error>),
         #[error("Could not produce an unambiguous shortened id for formatting.")]
         ShortId(#[from] crate::id::shorten::Error),
         #[error(transparent)]
@@ -160,7 +158,13 @@ pub mod describe {
             // TODO: dirty suffix with respective dirty-detection
             let outcome = git_revision::describe(
                 &self.id,
-                |id, buf| self.repo.objects.find_commit_iter(id, buf),
+                |id, buf| {
+                    Ok(self
+                        .repo
+                        .objects
+                        .try_find(id, buf)?
+                        .and_then(|d| d.try_into_commit_iter()))
+                },
                 git_revision::describe::Options {
                     name_by_oid: self.select.names(self.repo)?,
                     fallback_to_oid: self.id_as_fallback,

git-revision/src/describe.rs

@@ -135,7 +135,7 @@ where
     #[error("Commit {} could not be found during graph traversal", .oid.to_hex())]
     Find {
         #[source]
-        err: E,
+        err: Option<E>,
         oid: git_hash::ObjectId,
     },
     #[error("A commit could not be decoded during traversal")]
@@ -175,7 +175,7 @@ pub(crate) mod function {
         }: Options<'name>,
     ) -> Result<Option<Outcome<'name>>, Error<E>>
     where
-        Find: for<'b> FnMut(&oid, &'b mut Vec<u8>) -> Result<CommitRefIter<'b>, E>,
+        Find: for<'b> FnMut(&oid, &'b mut Vec<u8>) -> Result<Option<CommitRefIter<'b>>, E>,
         E: std::error::Error + Send + Sync + 'static,
     {
         if let Some(name) = name_by_oid.get(commit) {
@@ -316,22 +316,30 @@ pub(crate) mod function {
         first_parent: bool,
     ) -> Result<(), Error<E>>
     where
-        Find: for<'b> FnMut(&oid, &'b mut Vec<u8>) -> Result<CommitRefIter<'b>, E>,
+        Find: for<'b> FnMut(&oid, &'b mut Vec<u8>) -> Result<Option<CommitRefIter<'b>>, E>,
         E: std::error::Error + Send + Sync + 'static,
     {
-        let commit_iter = find(commit, buf).map_err(|err| Error::Find {
-            err,
-            oid: commit.to_owned(),
-        })?;
+        let commit_iter = find(commit, buf)
+            .map_err(|err| Error::Find {
+                err: Some(err),
+                oid: commit.to_owned(),
+            })?
+            .ok_or_else(|| Error::Find {
+                err: None,
+                oid: commit.to_owned(),
+            })?;
         for token in commit_iter {
             match token {
                 Ok(git_object::commit::ref_iter::Token::Tree { .. }) => continue,
                 Ok(git_object::commit::ref_iter::Token::Parent { id: parent_id }) => match seen.entry(parent_id) {
                     hash_map::Entry::Vacant(entry) => {
-                        let parent = find(&parent_id, parent_buf).map_err(|err| Error::Find {
-                            err,
+                        let parent = match find(&parent_id, parent_buf).map_err(|err| Error::Find {
+                            err: Some(err),
                             oid: commit.to_owned(),
-                        })?;
+                        })? {
+                            Some(p) => p,
+                            None => continue, // skip missing objects, they don't exist.
+                        };
 
                         let parent_commit_date = parent
                             .committer()
@@ -370,7 +378,7 @@ pub(crate) mod function {
         first_parent: bool,
     ) -> Result<u32, Error<E>>
     where
-        Find: for<'b> FnMut(&oid, &'b mut Vec<u8>) -> Result<CommitRefIter<'b>, E>,
+        Find: for<'b> FnMut(&oid, &'b mut Vec<u8>) -> Result<Option<CommitRefIter<'b>>, E>,
         E: std::error::Error + Send + Sync + 'static,
     {
         let mut commits_seen = 0;

git-revision/tests/describe/mod.rs

@@ -1,53 +1,56 @@
 use std::borrow::Cow;
 
 use git_object::bstr::ByteSlice;
-use git_repository::{odb::FindExt, Repository};
+use git_repository::{
+    odb::{Find, FindExt},
+    Repository,
+};
 use git_revision::describe;
 use git_testtools::hex_to_id;
 
 mod format;
 
 #[test]
-fn option_none_if_no_tag_found() {
+fn option_none_if_no_tag_found() -> crate::Result {
     let repo = repo();
-    let commit = repo.head_commit().unwrap();
+    let commit = repo.head_commit()?;
     let res = git_revision::describe(
         &commit.id,
-        |id, buf| repo.objects.find_commit_iter(id, buf),
+        |id, buf| repo.objects.find_commit_iter(id, buf).map(Some),
         Default::default(),
-    )
-    .unwrap();
+    )?;
     assert!(res.is_none(), "cannot find anything if there's no candidate");
+    Ok(())
 }
 
 #[test]
-fn fallback_if_configured_in_options_but_no_candidate() {
+fn fallback_if_configured_in_options_but_no_candidate() -> crate::Result {
     let repo = repo();
-    let commit = repo.head_commit().unwrap();
+    let commit = repo.head_commit()?;
     let res = git_revision::describe(
         &commit.id,
-        |id, buf| repo.objects.find_commit_iter(id, buf),
+        |id, buf| repo.objects.find_commit_iter(id, buf).map(Some),
         describe::Options {
             fallback_to_oid: true,
             ..Default::default()
         },
-    )
-    .unwrap()
+    )?
     .expect("fallback activated");
     assert!(res.name.is_none(), "no name can be found");
     assert_eq!(res.depth, 0, "just a default, not relevant as there is no name");
     assert_eq!(res.into_format(7).to_string(), "01ec18a");
+    Ok(())
 }
 
 #[test]
-fn not_enough_candidates() {
+fn not_enough_candidates() -> crate::Result {
     let repo = repo();
-    let commit = repo.head_commit().unwrap();
+    let commit = repo.head_commit()?;
 
     let name = Cow::Borrowed(b"at-c5".as_bstr());
     let res = git_revision::describe(
         &commit.id,
-        |id, buf| repo.objects.find_commit_iter(id, buf),
+        |id, buf| repo.objects.find_commit_iter(id, buf).map(Some),
         describe::Options {
             name_by_oid: vec![
                 (hex_to_id("efd9a841189668f1bab5b8ebade9cd0a1b139a37"), name.clone()),
@@ -61,8 +64,7 @@ fn not_enough_candidates() {
             max_candidates: 1,
             ..Default::default()
         },
-    )
-    .unwrap()
+    )?
     .expect("candidate found");
 
     assert_eq!(res.name, Some(name), "it finds the youngest/most-recent name");
@@ -71,6 +73,8 @@ fn not_enough_candidates() {
         res.depth, 3,
         "it calculates the final number of commits even though it aborted early"
     );
+
+    Ok(())
 }
 
 #[test]
@@ -96,7 +100,7 @@ fn typical_usecases() {
     let name = Cow::Borrowed(b"at-c5".as_bstr());
     let res = git_revision::describe(
         &commit.id,
-        |id, buf| repo.objects.find_commit_iter(id, buf),
+        |id, buf| repo.objects.find_commit_iter(id, buf).map(Some),
         describe::Options {
             name_by_oid: vec![
                 (hex_to_id("efd9a841189668f1bab5b8ebade9cd0a1b139a37"), name.clone()),
@@ -123,7 +127,7 @@ fn typical_usecases() {
 
     let res = git_revision::describe(
         &commit.id,
-        |id, buf| repo.objects.find_commit_iter(id, buf),
+        |id, buf| repo.objects.find_commit_iter(id, buf).map(Some),
         describe::Options {
             name_by_oid: res.name_by_oid,
             first_parent: true,
@@ -136,6 +140,25 @@ fn typical_usecases() {
     assert_eq!(res.name, Some(name),);
     assert_eq!(res.id, commit.id);
     assert_eq!(res.depth, 1);
+
+    let shallow_repo = git_repository::open(repo.work_dir().expect("non-bare").join("shallow-clone")).unwrap();
+
+    let res = git_revision::describe(
+        &commit.id,
+        |id, buf| {
+            shallow_repo
+                .objects
+                .try_find(id, buf)
+                .map(|r| r.and_then(|d| d.try_into_commit_iter()))
+        },
+        describe::Options {
+            name_by_oid: res.name_by_oid,
+            first_parent: true,
+            ..Default::default()
+        },
+    )
+    .unwrap();
+    assert!(res.is_none(), "no candidate found on truncated history");
 }
 
 fn repo() -> Repository {

git-revision/tests/fixtures/make_repo_with_branches.sh

@@ -21,3 +21,5 @@ git checkout -q main
 git commit -q --allow-empty -m c5
 git tag at-c5
 git merge branch1 -m m1b1
+
+git clone --depth 1 file://$PWD shallow-clone

git-revision/tests/revision.rs

@@ -1 +1,3 @@
 mod describe;
+
+pub type Result<T = ()> = std::result::Result<T, Box<dyn std::error::Error + 'static>>;