From e0879603475051c65cc2fe39f768fc71b53181b6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 14 Sep 2025 08:20:36 +0200 Subject: [PATCH 1/2] fix: remove special handling for empty blob hash to match Git behaviour This feature was recently introduced, but was never released. Co-authored-by: Byron <63622+Byron@users.noreply.github.com> --- gix/src/repository/object.rs | 30 +------------------- gix/tests/gix/repository/object.rs | 45 ++++++++++++++++-------------- 2 files changed, 25 insertions(+), 50 deletions(-) diff --git a/gix/src/repository/object.rs b/gix/src/repository/object.rs index 74bbe4242b5..e69671e5e20 100644 --- a/gix/src/repository/object.rs +++ b/gix/src/repository/object.rs @@ -50,14 +50,6 @@ impl crate::Repository { repo: self, }); } - if id == ObjectId::empty_blob(self.object_hash()) { - return Ok(Object { - id, - kind: gix_object::Kind::Blob, - data: Vec::new(), - repo: self, - }); - } let mut buf = self.free_buf(); let kind = self.objects.find(&id, &mut buf)?.kind; Ok(Object::from_data(id, kind, buf, self)) @@ -106,12 +98,6 @@ impl crate::Repository { size: 0, }); } - if id == ObjectId::empty_blob(self.object_hash()) { - return Ok(gix_odb::find::Header::Loose { - kind: gix_object::Kind::Blob, - size: 0, - }); - } self.objects.header(id) } @@ -126,7 +112,7 @@ impl crate::Repository { #[doc(alias = "exists", alias = "git2")] pub fn has_object(&self, id: impl AsRef) -> bool { let id = id.as_ref(); - if id.to_owned().is_empty_tree() || id.to_owned().is_empty_blob() { + if id.to_owned().is_empty_tree() { true } else { self.objects.exists(id) @@ -147,12 +133,6 @@ impl crate::Repository { size: 0, })); } - if id == ObjectId::empty_blob(self.object_hash()) { - return Ok(Some(gix_odb::find::Header::Loose { - kind: gix_object::Kind::Blob, - size: 0, - })); - } self.objects.try_header(&id).map_err(Into::into) } @@ -167,14 +147,6 @@ impl crate::Repository { repo: self, })); } - if id == ObjectId::empty_blob(self.object_hash()) { - return Ok(Some(Object { - id, - kind: gix_object::Kind::Blob, - data: Vec::new(), - repo: self, - })); - } let mut buf = self.free_buf(); match self.objects.try_find(&id, &mut buf)? { diff --git a/gix/tests/gix/repository/object.rs b/gix/tests/gix/repository/object.rs index 3ef570606a6..31380f85fc8 100644 --- a/gix/tests/gix/repository/object.rs +++ b/gix/tests/gix/repository/object.rs @@ -512,9 +512,11 @@ mod find { } #[test] - fn empty_blob_can_always_be_found() -> crate::Result { + fn empty_blob_can_be_found_if_it_exists() -> crate::Result { let repo = basic_repo()?; let empty_blob = gix::hash::ObjectId::empty_blob(repo.object_hash()); + + // The basic_repo fixture contains an empty blob, so these should work assert_eq!(repo.find_object(empty_blob)?.into_blob().data.len(), 0); assert!(repo.has_object(empty_blob)); assert_eq!( @@ -523,7 +525,7 @@ mod find { kind: gix_object::Kind::Blob, size: 0, }, - "empty blob is considered a loose object" + "empty blob is found when it exists in the repository" ); assert_eq!( repo.try_find_object(empty_blob)? @@ -539,10 +541,22 @@ mod find { kind: gix_object::Kind::Blob, size: 0, }), - "empty blob is considered a loose object" + "empty blob is found when it exists in the repository" ); Ok(()) } + + #[test] + fn empty_blob_method_creates_correct_object() -> crate::Result { + let repo = basic_repo()?; + let empty_blob = repo.empty_blob(); + + // The empty_blob method should create an object with the right ID and empty data + assert_eq!(empty_blob.id, gix::hash::ObjectId::empty_blob(repo.object_hash())); + assert_eq!(empty_blob.data.len(), 0); + + Ok(()) + } } #[test] @@ -551,33 +565,22 @@ fn empty_objects_are_always_present_but_not_in_plumbing() -> crate::Result { let empty_blob_id = gix::hash::ObjectId::empty_blob(repo.object_hash()); assert!( - repo.has_object(empty_blob_id), - "empty object is always present even if it's not" + !repo.has_object(empty_blob_id), + "empty blob is not present unless it actually exists" ); assert!(!repo.objects.contains(&empty_blob_id)); - let header = repo.find_header(empty_blob_id)?; - assert_eq!(header.kind(), gix_object::Kind::Blob); - assert_eq!(header.size(), 0); + // Empty blob should cause errors when it doesn't exist + assert!(repo.find_header(empty_blob_id).is_err()); assert_eq!(repo.objects.try_header(&empty_blob_id)?, None); - let header = repo.try_find_header(empty_blob_id)?.expect("should find header"); - assert_eq!(header.kind(), gix_object::Kind::Blob); - assert_eq!(header.size(), 0); - - let obj = repo.find_object(empty_blob_id)?; - assert_eq!(obj.kind, gix_object::Kind::Blob); - assert_eq!(obj.data.len(), 0); + assert_eq!(repo.try_find_header(empty_blob_id)?, None); + assert!(repo.find_object(empty_blob_id).is_err()); let mut buf = Vec::new(); assert_eq!(repo.objects.try_find(&empty_blob_id, &mut buf)?, None); - let obj = repo.try_find_object(empty_blob_id)?.expect("should find object"); - assert_eq!(obj.kind, gix_object::Kind::Blob); - assert_eq!(obj.data.len(), 0); - - let blob = obj.try_into_blob()?; - assert_eq!(blob.data.len(), 0); + assert!(repo.try_find_object(empty_blob_id)?.is_none()); Ok(()) } From b9c7a7e0766e61c47c8abc3a95b65b0a888294ba Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 14 Sep 2025 08:21:52 +0200 Subject: [PATCH 2/2] refactor --- gix/src/repository/object.rs | 2 +- gix/tests/gix/repository/object.rs | 36 ++++++++++++++++++------------ 2 files changed, 23 insertions(+), 15 deletions(-) diff --git a/gix/src/repository/object.rs b/gix/src/repository/object.rs index e69671e5e20..c9d11a27e2b 100644 --- a/gix/src/repository/object.rs +++ b/gix/src/repository/object.rs @@ -438,7 +438,7 @@ impl crate::Repository { /// This means that this object can be used in an uninitialized, empty repository which would report to have no objects at all. pub fn empty_blob(&self) -> Blob<'_> { Blob { - id: gix_hash::ObjectId::empty_blob(self.object_hash()), + id: self.object_hash().empty_blob(), data: Vec::new(), repo: self, } diff --git a/gix/tests/gix/repository/object.rs b/gix/tests/gix/repository/object.rs index 31380f85fc8..4b45ea001ff 100644 --- a/gix/tests/gix/repository/object.rs +++ b/gix/tests/gix/repository/object.rs @@ -432,6 +432,7 @@ mod find { use gix_pack::Find; use crate::basic_repo; + use crate::repository::object::empty_bare_in_memory_repo; #[test] fn find_and_try_find_with_and_without_object_cache() -> crate::Result { @@ -515,9 +516,12 @@ mod find { fn empty_blob_can_be_found_if_it_exists() -> crate::Result { let repo = basic_repo()?; let empty_blob = gix::hash::ObjectId::empty_blob(repo.object_hash()); - - // The basic_repo fixture contains an empty blob, so these should work - assert_eq!(repo.find_object(empty_blob)?.into_blob().data.len(), 0); + + assert_eq!( + repo.find_object(empty_blob)?.into_blob().data.len(), + 0, + "The basic_repo fixture contains an empty blob" + ); assert!(repo.has_object(empty_blob)); assert_eq!( repo.find_header(empty_blob)?, @@ -547,14 +551,17 @@ mod find { } #[test] - fn empty_blob_method_creates_correct_object() -> crate::Result { - let repo = basic_repo()?; + fn empty_blob() -> crate::Result { + let repo = empty_bare_in_memory_repo()?; let empty_blob = repo.empty_blob(); - - // The empty_blob method should create an object with the right ID and empty data - assert_eq!(empty_blob.id, gix::hash::ObjectId::empty_blob(repo.object_hash())); + + assert_eq!(empty_blob.id, repo.object_hash().empty_blob()); assert_eq!(empty_blob.data.len(), 0); - + + assert!(!repo.has_object(empty_blob.id), "it doesn't exist by default"); + repo.write_blob(&empty_blob.data)?; + assert!(repo.has_object(empty_blob.id), "it exists after it was written"); + Ok(()) } } @@ -562,7 +569,7 @@ mod find { #[test] fn empty_objects_are_always_present_but_not_in_plumbing() -> crate::Result { let repo = empty_bare_in_memory_repo()?; - let empty_blob_id = gix::hash::ObjectId::empty_blob(repo.object_hash()); + let empty_blob_id = repo.object_hash().empty_blob(); assert!( !repo.has_object(empty_blob_id), @@ -570,18 +577,19 @@ fn empty_objects_are_always_present_but_not_in_plumbing() -> crate::Result { ); assert!(!repo.objects.contains(&empty_blob_id)); - // Empty blob should cause errors when it doesn't exist - assert!(repo.find_header(empty_blob_id).is_err()); + assert!( + repo.find_header(empty_blob_id).is_err(), + "Empty blob doesn't exist automatically just like in Git" + ); assert_eq!(repo.objects.try_header(&empty_blob_id)?, None); assert_eq!(repo.try_find_header(empty_blob_id)?, None); assert!(repo.find_object(empty_blob_id).is_err()); + assert!(repo.try_find_object(empty_blob_id)?.is_none()); let mut buf = Vec::new(); assert_eq!(repo.objects.try_find(&empty_blob_id, &mut buf)?, None); - assert!(repo.try_find_object(empty_blob_id)?.is_none()); - Ok(()) }