fix: when no worktree-root is specified, fail instead of using empty blobs.

Author: Byron

gix-diff/src/blob/pipeline.rs

@@ -157,6 +157,8 @@ pub mod convert_to_diffable {
         ConvertToGit(#[from] gix_filter::pipeline::convert::to_git::Error),
         #[error("Memory allocation failed")]
         OutOfMemory(#[from] TryReserveError),
+        #[error("An ObjectId wasn't provided, but a worktree root wasn't provided either for reading from disk")]
+        UnsetWorktreeRoot,
     }
 }
 
@@ -208,6 +210,7 @@ impl Pipeline {
     ///
     /// If `id` [is null](gix_hash::ObjectId::is_null()) or the file in question doesn't exist in the worktree in case
     /// [a root](WorktreeRoots) is present, then `out` will be left cleared and [Outcome::data] will be `None`.
+    /// Note that it is an error to not provide a workdir root *and* a null `id`.
     ///
     /// Note that `mode` is trusted, and we will not re-validate that the entry in the worktree actually is of that mode.
     ///
@@ -383,9 +386,10 @@ impl Pipeline {
                 Ok(Outcome { driver_index, data })
             }
             None => {
-                let data = if id.is_null() {
-                    None
-                } else {
+                if id.is_null() {
+                    return Err(convert_to_diffable::Error::UnsetWorktreeRoot);
+                }
+                let data = {
                     let header = objects
                         .try_header(id)
                         .map_err(gix_object::find::existing_object::Error::Find)?

gix-diff/src/blob/platform.rs

@@ -369,6 +369,12 @@ impl Platform {
     /// depending on whether or not a [worktree root](pipeline::WorktreeRoots) is set for the resource of `kind`,
     /// with resources with worktree roots using the `rela_path` as unique identifier.
     ///
+    /// ### Warning: Unbounded Memory Caching
+    ///
+    /// As the instance is meant to be used for repeated diffs during rename-checking, it is caching diffable objects
+    /// in memory.
+    /// Be sure to either not reuse the instance, or call [clear_resource_cache*()](Self::clear_resource_cache()) regularly.
+    ///
     /// ### Important
     ///
     /// If an error occurs, the previous resource of `kind` will be cleared, preventing further diffs

gix-diff/tests/diff/blob/pipeline.rs

@@ -283,19 +283,23 @@ pub(crate) mod convert_to_diffable {
         drop(tmp);
 
         buf.push(1);
-        let out = filter.convert_to_diffable(
-            &null,
-            EntryKind::Blob,
-            a_name.into(),
-            ResourceKind::NewOrDestination,
-            &mut |_, _| {},
-            &gix_object::find::Never,
-            pipeline::Mode::default(),
-            &mut buf,
-        )?;
+        let err = filter
+            .convert_to_diffable(
+                &null,
+                EntryKind::Blob,
+                a_name.into(),
+                ResourceKind::NewOrDestination,
+                &mut |_, _| {},
+                &gix_object::find::Never,
+                pipeline::Mode::default(),
+                &mut buf,
+            )
+            .unwrap_err();
 
-        assert!(out.driver_index.is_none(), "there was no driver");
-        assert_eq!(out.data, None);
+        assert_eq!(
+            err.to_string(),
+            "An ObjectId wasn't provided, but a worktree root wasn't provided either for reading from disk"
+        );
         assert_eq!(buf.len(), 0, "it's always cleared before any potential use");
 
         Ok(())
@@ -692,6 +696,7 @@ pub(crate) mod convert_to_diffable {
             assert_eq!(buf.len(), 0, "always cleared");
 
             buf.push(1);
+            filter.roots.new_root = filter.roots.old_root.clone();
             let out = filter.convert_to_diffable(
                 &null,
                 EntryKind::Link,
@@ -707,6 +712,7 @@ pub(crate) mod convert_to_diffable {
             assert_eq!(out.driver_index, Some(4), "despite missing, we get driver information");
             assert_eq!(out.data, None);
             assert_eq!(buf.len(), 0, "always cleared");
+            filter.roots.new_root.take();
 
             buf.push(1);
             let id = db.insert("link-target");

gix-diff/tests/diff/blob/platform.rs

@@ -293,7 +293,7 @@ fn diff_skipped_due_to_external_command_and_enabled_option() -> crate::Result {
 
 #[test]
 fn source_and_destination_do_not_exist() -> crate::Result {
-    let mut platform = new_platform(None, pipeline::Mode::default());
+    let mut platform = new_platform_multiroot(None, pipeline::Mode::default(), PopulateRoot::OldAndNew);
     platform.set_resource(
         gix_hash::Kind::Sha1.null(),
         EntryKind::Blob,
@@ -305,7 +305,7 @@ fn source_and_destination_do_not_exist() -> crate::Result {
     platform.set_resource(
         gix_hash::Kind::Sha1.null(),
         EntryKind::BlobExecutable,
-        "a".into(),
+        "also-missing".into(),
         ResourceKind::NewOrDestination,
         &gix_object::find::Never,
     )?;
@@ -339,7 +339,7 @@ fn source_and_destination_do_not_exist() -> crate::Result {
                 .expect("resources set")
         ),
         format!(
-            r#"{}"test" "missing" "/dev/null" "." "." "/dev/null" "." "." "a""#,
+            r#"{}"test" "missing" "/dev/null" "." "." "/dev/null" "." "." "also-missing""#,
             (!cfg!(windows))
                 .then_some(r#"GIT_DIFF_PATH_COUNTER="1" GIT_DIFF_PATH_TOTAL="1" GIT_DIR="." "#)
                 .unwrap_or_default()
@@ -371,6 +371,19 @@ fn invalid_resource_types() {
 fn new_platform(
     drivers: impl IntoIterator<Item = gix_diff::blob::Driver>,
     mode: gix_diff::blob::pipeline::Mode,
+) -> Platform {
+    new_platform_multiroot(drivers, mode, PopulateRoot::Old)
+}
+
+enum PopulateRoot {
+    OldAndNew,
+    Old,
+}
+
+fn new_platform_multiroot(
+    drivers: impl IntoIterator<Item = gix_diff::blob::Driver>,
+    mode: gix_diff::blob::pipeline::Mode,
+    populate_root: PopulateRoot,
 ) -> Platform {
     let root = gix_testtools::scripted_fixture_read_only_standalone("make_blob_repo.sh").expect("valid fixture");
     let attributes = gix_worktree::Stack::new(
@@ -388,7 +401,10 @@ fn new_platform(
     let filter = gix_diff::blob::Pipeline::new(
         pipeline::WorktreeRoots {
             old_root: Some(root.clone()),
-            new_root: None,
+            new_root: match populate_root {
+                PopulateRoot::OldAndNew => Some(root.clone()),
+                PopulateRoot::Old => None,
+            },
         },
         gix_filter::Pipeline::default(),
         drivers.into_iter().collect(),