change!: adjust hash verification return types for the common interface

This mostly just affects return types – using
`git_hash::verify::Error` instead of bespoke duplicated versions
thereof, and occasionally returning an `ObjectId` instead of `()`
for convenience.

Author: emilazy

gitoxide-core/src/pack/explode.rs

@@ -78,11 +78,11 @@ enum Error {
     },
     #[error("Object didn't verify after right after writing it")]
     Verify(#[from] objs::data::verify::Error),
-    #[error("{kind} object {expected} wasn't re-encoded without change - new hash is {actual}")]
+    #[error("{kind} object wasn't re-encoded without change")]
     ObjectEncodeMismatch {
+        #[source]
+        source: gix::hash::verify::Error,
         kind: object::Kind,
-        actual: ObjectId,
-        expected: ObjectId,
     },
     #[error("The recently written file for loose object {id} could not be found")]
     WrittenFileMissing { id: ObjectId },
@@ -201,17 +201,16 @@ pub fn pack_or_pack_index(
                         kind: object_kind,
                         id: index_entry.oid,
                     })?;
-                    if written_id != index_entry.oid {
+                    if let Err(err) = written_id.verify(&index_entry.oid) {
                         if let object::Kind::Tree = object_kind {
                             progress.info(format!(
                                 "The tree in pack named {} was written as {} due to modes 100664 and 100640 rewritten as 100644.",
                                 index_entry.oid, written_id
                             ));
                         } else {
                             return Err(Error::ObjectEncodeMismatch {
+                                source: err,
                                 kind: object_kind,
-                                actual: index_entry.oid,
-                                expected: written_id,
                             });
                         }
                     }

gix-commitgraph/src/file/verify.rs

@@ -28,11 +28,8 @@ pub enum Error<E: std::error::Error + 'static> {
     Filename(String),
     #[error("commit {id} has invalid generation {generation}")]
     Generation { generation: u32, id: gix_hash::ObjectId },
-    #[error("checksum mismatch: expected {expected}, got {actual}")]
-    Mismatch {
-        actual: gix_hash::ObjectId,
-        expected: gix_hash::ObjectId,
-    },
+    #[error(transparent)]
+    Checksum(#[from] checksum::Error),
     #[error("{0}")]
     Processor(#[source] E),
     #[error("commit {id} has invalid root tree ID {root_tree_id}")]
@@ -42,6 +39,17 @@ pub enum Error<E: std::error::Error + 'static> {
     },
 }
 
+///
+pub mod checksum {
+    /// The error used in [`super::File::verify_checksum()`].
+    #[derive(thiserror::Error, Debug)]
+    #[allow(missing_docs)]
+    pub enum Error {
+        #[error(transparent)]
+        Verify(#[from] gix_hash::verify::Error),
+    }
+}
+
 /// The positive result of [`File::traverse()`] providing some statistical information.
 #[derive(Clone, Debug, Eq, PartialEq)]
 #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
@@ -73,8 +81,7 @@ impl File {
         E: std::error::Error + 'static,
         Processor: FnMut(&file::Commit<'a>) -> Result<(), E>,
     {
-        self.verify_checksum()
-            .map_err(|(actual, expected)| Error::Mismatch { actual, expected })?;
+        self.verify_checksum()?;
         verify_split_chain_filename_hash(&self.path, self.checksum()).map_err(Error::Filename)?;
 
         let null_id = self.object_hash().null_ref();
@@ -138,23 +145,18 @@ impl File {
     /// Assure the [`checksum`][File::checksum()] matches the actual checksum over all content of this file, excluding the trailing
     /// checksum itself.
     ///
-    /// Return the actual checksum on success or `(actual checksum, expected checksum)` if there is a mismatch.
-    pub fn verify_checksum(&self) -> Result<gix_hash::ObjectId, (gix_hash::ObjectId, gix_hash::ObjectId)> {
-        // Even though we could use gix_hash::bytes_of_file(…), this would require using our own
-        // Error type to support io::Error and Mismatch. As we only gain progress, there probably isn't much value
+    /// Return the actual checksum on success or [`checksum::Error`] if there is a mismatch.
+    pub fn verify_checksum(&self) -> Result<gix_hash::ObjectId, checksum::Error> {
+        // Even though we could use gix_hash::bytes_of_file(…), this would require extending our
+        // Error type to support io::Error. As we only gain progress, there probably isn't much value
         // as these files are usually small enough to process them in less than a second, even for the large ones.
         // But it's possible, once a progress instance is passed.
         let data_len_without_trailer = self.data.len() - self.hash_len;
         let mut hasher = gix_hash::hasher(self.object_hash());
         hasher.update(&self.data[..data_len_without_trailer]);
         let actual = hasher.finalize();
-
-        let expected = self.checksum();
-        if actual == expected {
-            Ok(actual)
-        } else {
-            Err((actual, expected.into()))
-        }
+        actual.verify(self.checksum())?;
+        Ok(actual)
     }
 }
 

gix-commitgraph/src/verify.rs

@@ -164,9 +164,7 @@ impl Graph {
                         file::verify::Error::RootTreeId { id, root_tree_id } => {
                             file::verify::Error::RootTreeId { id, root_tree_id }
                         }
-                        file::verify::Error::Mismatch { actual, expected } => {
-                            file::verify::Error::Mismatch { actual, expected }
-                        }
+                        file::verify::Error::Checksum(err) => file::verify::Error::Checksum(err),
                         file::verify::Error::Generation { generation, id } => {
                             file::verify::Error::Generation { generation, id }
                         }

gix-index/src/decode/mod.rs

@@ -22,11 +22,8 @@ mod error {
         Extension(#[from] extension::decode::Error),
         #[error("Index trailer should have been {expected} bytes long, but was {actual}")]
         UnexpectedTrailerLength { expected: usize, actual: usize },
-        #[error("Shared index checksum was {actual_checksum} but should have been {expected_checksum}")]
-        ChecksumMismatch {
-            actual_checksum: gix_hash::ObjectId,
-            expected_checksum: gix_hash::ObjectId,
-        },
+        #[error("Shared index checksum mismatch")]
+        Verify(#[from] gix_hash::verify::Error),
     }
 }
 pub use error::Error;
@@ -217,12 +214,7 @@ impl State {
         let checksum = gix_hash::ObjectId::from_bytes_or_panic(data);
         let checksum = (!checksum.is_null()).then_some(checksum);
         if let Some((expected_checksum, actual_checksum)) = expected_checksum.zip(checksum) {
-            if actual_checksum != expected_checksum {
-                return Err(Error::ChecksumMismatch {
-                    actual_checksum,
-                    expected_checksum,
-                });
-            }
+            actual_checksum.verify(&expected_checksum)?;
         }
         let EntriesOutcome {
             entries,

gix-index/src/extension/end_of_index_entry/decode.rs

@@ -29,7 +29,10 @@ pub fn decode(data: &[u8], object_hash: gix_hash::Kind) -> Option<usize> {
 
     let (offset, checksum) = ext_data.split_at(4);
     let offset = from_be_u32(offset) as usize;
-    if offset < header::SIZE || offset > start_of_eoie || checksum.len() != gix_hash::Kind::Sha1.len_in_bytes() {
+    let Ok(checksum) = gix_hash::oid::try_from_bytes(checksum) else {
+        return None;
+    };
+    if offset < header::SIZE || offset > start_of_eoie || checksum.kind() != gix_hash::Kind::Sha1 {
         return None;
     }
 
@@ -41,7 +44,7 @@ pub fn decode(data: &[u8], object_hash: gix_hash::Kind) -> Option<usize> {
         last_chunk = Some(chunk);
     }
 
-    if hasher.finalize().as_slice() != checksum {
+    if hasher.finalize().verify(checksum).is_err() {
         return None;
     }
     // The last-to-this chunk ends where ours starts

gix-index/src/file/init.rs

@@ -75,20 +75,15 @@ impl File {
                     let _span = gix_features::trace::detail!("gix::open_index::hash_index", path = ?path);
                     let meta = file.metadata()?;
                     let num_bytes_to_hash = meta.len() - object_hash.len_in_bytes() as u64;
-                    let actual_hash = gix_hash::bytes(
+                    gix_hash::bytes(
                         &mut file,
                         num_bytes_to_hash,
                         object_hash,
                         &mut gix_features::progress::Discard,
                         &Default::default(),
-                    )?;
-
-                    if actual_hash != expected {
-                        return Err(Error::Decode(decode::Error::ChecksumMismatch {
-                            actual_checksum: actual_hash,
-                            expected_checksum: expected,
-                        }));
-                    }
+                    )?
+                    .verify(&expected)
+                    .map_err(decode::Error::from)?;
                 }
             }
 

gix-index/src/file/verify.rs

@@ -9,11 +9,8 @@ mod error {
     pub enum Error {
         #[error("Could not read index file to generate hash")]
         Io(#[from] std::io::Error),
-        #[error("Index checksum should have been {expected}, but was {actual}")]
-        ChecksumMismatch {
-            actual: gix_hash::ObjectId,
-            expected: gix_hash::ObjectId,
-        },
+        #[error("Index checksum mismatch")]
+        Verify(#[from] gix_hash::verify::Error),
     }
 }
 pub use error::Error;
@@ -25,19 +22,15 @@ impl File {
         if let Some(checksum) = self.checksum {
             let num_bytes_to_hash = self.path.metadata()?.len() - checksum.as_bytes().len() as u64;
             let should_interrupt = AtomicBool::new(false);
-            let actual = gix_hash::bytes_of_file(
+            gix_hash::bytes_of_file(
                 &self.path,
                 num_bytes_to_hash,
                 checksum.kind(),
                 &mut gix_features::progress::Discard,
                 &should_interrupt,
-            )?;
-            (actual == checksum).then_some(()).ok_or(Error::ChecksumMismatch {
-                actual,
-                expected: checksum,
-            })
-        } else {
-            Ok(())
+            )?
+            .verify(&checksum)?;
         }
+        Ok(())
     }
 }

gix-index/tests/index/file/read.rs

@@ -328,7 +328,7 @@ fn v2_split_index_recursion_is_handled_gracefully() {
     let err = try_file("v2_split_index_recursive").expect_err("recursion fails gracefully");
     assert!(matches!(
         err,
-        gix_index::file::init::Error::Decode(gix_index::decode::Error::ChecksumMismatch { .. })
+        gix_index::file::init::Error::Decode(gix_index::decode::Error::Verify(_))
     ));
 }
 

gix-object/src/data.rs

@@ -51,31 +51,22 @@ impl<'a> Data<'a> {
 
 /// Types supporting object hash verification
 pub mod verify {
-
     /// Returned by [`crate::Data::verify_checksum()`]
     #[derive(Debug, thiserror::Error)]
     #[allow(missing_docs)]
     pub enum Error {
-        #[error("Object expected to have id {desired}, but actual id was {actual}")]
-        ChecksumMismatch {
-            desired: gix_hash::ObjectId,
-            actual: gix_hash::ObjectId,
-        },
+        #[error(transparent)]
+        Verify(#[from] gix_hash::verify::Error),
     }
 
     impl crate::Data<'_> {
-        /// Compute the checksum of `self` and compare it with the `desired` hash.
+        /// Compute the checksum of `self` and compare it with the `expected` hash.
         /// If the hashes do not match, an [`Error`] is returned, containing the actual
         /// hash of `self`.
-        pub fn verify_checksum(&self, desired: &gix_hash::oid) -> Result<(), Error> {
-            let actual_id = crate::compute_hash(desired.kind(), self.kind, self.data);
-            if desired != actual_id {
-                return Err(Error::ChecksumMismatch {
-                    desired: desired.into(),
-                    actual: actual_id,
-                });
-            }
-            Ok(())
+        pub fn verify_checksum(&self, expected: &gix_hash::oid) -> Result<gix_hash::ObjectId, Error> {
+            let actual = crate::compute_hash(expected.kind(), self.kind, self.data);
+            actual.verify(expected)?;
+            Ok(actual)
         }
     }
 }

gix-odb/src/store_impls/loose/verify.rs

@@ -19,11 +19,11 @@ pub mod integrity {
             kind: gix_object::Kind,
             id: gix_hash::ObjectId,
         },
-        #[error("{kind} object {expected} wasn't re-encoded without change - new hash is {actual}")]
-        ObjectHashMismatch {
+        #[error("{kind} object wasn't re-encoded without change")]
+        ObjectEncodeMismatch {
+            #[source]
+            source: gix_hash::verify::Error,
             kind: gix_object::Kind,
-            actual: gix_hash::ObjectId,
-            expected: gix_hash::ObjectId,
         },
         #[error("Objects were deleted during iteration - try again")]
         Retry,
@@ -77,14 +77,13 @@ impl Store {
                 .try_find(&id, &mut buf)
                 .map_err(|_| integrity::Error::Retry)?
                 .ok_or(integrity::Error::Retry)?;
-            let actual_id = sink.write_buf(object.kind, object.data).expect("sink never fails");
-            if actual_id != id {
-                return Err(integrity::Error::ObjectHashMismatch {
+            sink.write_buf(object.kind, object.data)
+                .expect("sink never fails")
+                .verify(&id)
+                .map_err(|err| integrity::Error::ObjectEncodeMismatch {
+                    source: err,
                     kind: object.kind,
-                    actual: actual_id,
-                    expected: id,
-                });
-            }
+                })?;
             object.decode().map_err(|err| integrity::Error::ObjectDecode {
                 source: err,
                 kind: object.kind,