feat: add blob::PlatformRef::id_by_pick() to more efficiently pick merge results.

This works by either selecting a possibly unchanged and not even loaded resource,
instead of always loading it to provide a buffer, in case the user doesn't
actually want a buffer.

Note that this also alters `buffer_by_pick()` to enforce handling of the 'buffer-too-large'
option.

Author: Byron

gix-merge/src/blob/platform/merge.rs

@@ -266,6 +266,8 @@ pub(super) mod inner {
 
     ///
     pub mod builtin_merge {
+        use crate::blob::platform::resource;
+        use crate::blob::platform::resource::Data;
         use crate::blob::{builtin_driver, BuiltinDriver, PlatformRef, Resolution};
 
         /// An identifier to tell us how a merge conflict was resolved by [builtin_merge](PlatformRef::builtin_merge).
@@ -307,19 +309,20 @@ pub(super) mod inner {
                 input: &mut imara_diff::intern::InternedInput<&'parent [u8]>,
                 labels: builtin_driver::text::Labels<'_>,
             ) -> (Pick, Resolution) {
-                let base = self.ancestor.data.as_slice();
-                let ours = self.current.data.as_slice();
-                let theirs = self.other.data.as_slice();
+                let base = self.ancestor.data.as_slice().unwrap_or_default();
+                let ours = self.current.data.as_slice().unwrap_or_default();
+                let theirs = self.other.data.as_slice().unwrap_or_default();
                 let driver = if driver != BuiltinDriver::Binary
-                    && (is_binary_buf(ours) || is_binary_buf(theirs) || is_binary_buf(base))
+                    && (is_binary_buf(self.ancestor.data)
+                        || is_binary_buf(self.other.data)
+                        || is_binary_buf(self.current.data))
                 {
                     BuiltinDriver::Binary
                 } else {
                     driver
                 };
                 match driver {
                     BuiltinDriver::Text => {
-                        let ((base, ours), theirs) = base.zip(ours).zip(theirs).expect("would use binary if missing");
                         let resolution =
                             builtin_driver::text(out, input, labels, ours, base, theirs, self.options.text);
                         (Pick::Buffer, resolution)
@@ -334,7 +337,6 @@ pub(super) mod inner {
                         (pick, resolution)
                     }
                     BuiltinDriver::Union => {
-                        let ((base, ours), theirs) = base.zip(ours).zip(theirs).expect("would use binary if missing");
                         let resolution = builtin_driver::text(
                             out,
                             input,
@@ -353,11 +355,15 @@ pub(super) mod inner {
             }
         }
 
-        fn is_binary_buf(buf: Option<&[u8]>) -> bool {
-            buf.map_or(true, |buf| {
-                let buf = &buf[..buf.len().min(8000)];
-                buf.contains(&0)
-            })
+        fn is_binary_buf(data: resource::Data<'_>) -> bool {
+            match data {
+                Data::Missing => false,
+                Data::Buffer(buf) => {
+                    let buf = &buf[..buf.len().min(8000)];
+                    buf.contains(&0)
+                }
+                Data::TooLarge { .. } => true,
+            }
         }
     }
 }
@@ -412,13 +418,45 @@ impl<'parent> PlatformRef<'parent> {
     }
 
     /// Using a `pick` obtained from [`merge()`](Self::merge), obtain the respective buffer suitable for reading or copying.
-    /// Return `None` if the buffer is too large, or if the `pick` corresponds to a buffer (that was written separately).
-    pub fn buffer_by_pick(&self, pick: inner::builtin_merge::Pick) -> Option<&'parent [u8]> {
+    /// Return `Ok(None)`  if the `pick` corresponds to a buffer (that was written separately).
+    /// Return `Err(())` if the buffer is *too large*, so it was never read.
+    #[allow(clippy::result_unit_err)]
+    pub fn buffer_by_pick(&self, pick: inner::builtin_merge::Pick) -> Result<Option<&'parent [u8]>, ()> {
         match pick {
-            inner::builtin_merge::Pick::Ancestor => self.ancestor.data.as_slice(),
-            inner::builtin_merge::Pick::Ours => self.current.data.as_slice(),
-            inner::builtin_merge::Pick::Theirs => self.other.data.as_slice(),
-            inner::builtin_merge::Pick::Buffer => None,
+            inner::builtin_merge::Pick::Ancestor => self.ancestor.data.as_slice().map(Some).ok_or(()),
+            inner::builtin_merge::Pick::Ours => self.current.data.as_slice().map(Some).ok_or(()),
+            inner::builtin_merge::Pick::Theirs => self.other.data.as_slice().map(Some).ok_or(()),
+            inner::builtin_merge::Pick::Buffer => Ok(None),
+        }
+    }
+
+    /// Use `pick` to return the object id of the merged result, assuming that `buf` was passed as `out` to [merge()](Self::merge).
+    /// In case of binary or large files, this will simply be the existing ID of the resource.
+    /// In case of resources available in the object DB for binary merges, the object ID will be returned.
+    /// If new content was produced due to a content merge, `buf` will be written out
+    /// to the object database using `write_blob`.
+    /// Beware that the returned ID could be `Ok(None)` if the underlying resource was loaded
+    /// from the worktree *and* was too large so it was never loaded from disk.
+    /// `Ok(None)` will also be returned if one of the resources was missing.
+    /// `write_blob()` is used to turn buffers.
+    pub fn id_by_pick<E>(
+        &self,
+        pick: inner::builtin_merge::Pick,
+        buf: &[u8],
+        mut write_blob: impl FnMut(&[u8]) -> Result<gix_hash::ObjectId, E>,
+    ) -> Result<Option<gix_hash::ObjectId>, E> {
+        let field = match pick {
+            inner::builtin_merge::Pick::Ancestor => &self.ancestor,
+            inner::builtin_merge::Pick::Ours => &self.current,
+            inner::builtin_merge::Pick::Theirs => &self.other,
+            inner::builtin_merge::Pick::Buffer => return write_blob(buf).map(Some),
+        };
+        use crate::blob::platform::resource::Data;
+        match field.data {
+            Data::TooLarge { .. } | Data::Missing if !field.id.is_null() => Ok(Some(field.id.to_owned())),
+            Data::TooLarge { .. } | Data::Missing => Ok(None),
+            Data::Buffer(buf) if field.id.is_null() => write_blob(buf).map(Some),
+            Data::Buffer(_) => Ok(Some(field.id.to_owned())),
         }
     }
 }

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

@@ -5,12 +5,14 @@ use gix_merge::blob::Platform;
 mod merge {
     use crate::blob::platform::new_platform;
     use crate::blob::util::ObjectDb;
+    use crate::hex_to_id;
     use bstr::{BStr, ByteSlice};
     use gix_merge::blob::builtin_driver::text::ConflictStyle;
     use gix_merge::blob::platform::builtin_merge::Pick;
     use gix_merge::blob::platform::DriverChoice;
     use gix_merge::blob::{builtin_driver, pipeline, platform, BuiltinDriver, Resolution, ResourceKind};
     use gix_object::tree::EntryKind;
+    use std::convert::Infallible;
     use std::process::Stdio;
 
     #[test]
@@ -66,8 +68,9 @@ mod merge {
     #[test]
     fn builtin_with_conflict() -> crate::Result {
         let mut platform = new_platform(None, pipeline::Mode::ToGit);
+        let non_existing_ancestor_id = hex_to_id("ffffffffffffffffffffffffffffffffffffffff");
         platform.set_resource(
-            gix_hash::Kind::Sha1.null(),
+            non_existing_ancestor_id,
             EntryKind::Blob,
             "b".into(),
             ResourceKind::CommonAncestorOrBase,
@@ -152,20 +155,56 @@ theirs
         );
         assert!(buf.is_empty(), "it tells us where to get the content from");
         assert_eq!(
-            platform_ref.buffer_by_pick(res.0).unwrap().as_bstr(),
+            platform_ref.buffer_by_pick(res.0).unwrap().unwrap().as_bstr(),
             "ours",
             "getting access to the content is simplified"
         );
+        assert_eq!(
+            platform_ref
+                .id_by_pick(res.0, &buf, |_buf| -> Result<_, Infallible> {
+                    panic!("no need to write buffer")
+                })
+                .unwrap()
+                .unwrap(),
+            hex_to_id("424860eef4edb9f5a2dacbbd6dc8c2d2e7645035"),
+            "there is no need to write a buffer here, it just returns one of our inputs"
+        );
 
-        for (expected, expected_pick, resolve) in [
-            ("ours", Pick::Ours, builtin_driver::binary::ResolveWith::Ours),
-            ("theirs", Pick::Theirs, builtin_driver::binary::ResolveWith::Theirs),
-            ("b\n", Pick::Ancestor, builtin_driver::binary::ResolveWith::Ancestor),
+        for (expected, expected_pick, resolve, expected_id) in [
+            (
+                "ours",
+                Pick::Ours,
+                builtin_driver::binary::ResolveWith::Ours,
+                hex_to_id("424860eef4edb9f5a2dacbbd6dc8c2d2e7645035"),
+            ),
+            (
+                "theirs",
+                Pick::Theirs,
+                builtin_driver::binary::ResolveWith::Theirs,
+                hex_to_id("228068dbe790983c15535164cd483eb77ade97e4"),
+            ),
+            (
+                "b\n",
+                Pick::Ancestor,
+                builtin_driver::binary::ResolveWith::Ancestor,
+                non_existing_ancestor_id,
+            ),
         ] {
             platform_ref.options.resolve_binary_with = Some(resolve);
             let res = platform_ref.merge(&mut buf, default_labels(), &Default::default())?;
             assert_eq!(res, (expected_pick, Resolution::Complete));
-            assert_eq!(platform_ref.buffer_by_pick(res.0).unwrap().as_bstr(), expected);
+            assert_eq!(platform_ref.buffer_by_pick(res.0).unwrap().unwrap().as_bstr(), expected);
+
+            assert_eq!(
+                platform_ref
+                    .id_by_pick(res.0, &buf, |_buf| -> Result<_, Infallible> {
+                        panic!("no need to write buffer")
+                    })
+                    .unwrap()
+                    .unwrap(),
+                expected_id,
+                "{expected}: each input has an id, so it's just returned as is without handling buffers"
+            );
         }
 
         Ok(())
@@ -234,6 +273,18 @@ theirs
             (Pick::Buffer, Resolution::Complete),
             "merge drivers deal with binary themselves"
         );
+        assert_eq!(
+            platform_ref.buffer_by_pick(res.0),
+            Ok(None),
+            "This indicates the buffer must be read"
+        );
+
+        let marker = hex_to_id("ffffffffffffffffffffffffffffffffffffffff");
+        assert_eq!(
+            platform_ref.id_by_pick(res.0, &buf, |_buf| Ok::<_, Infallible>(marker)),
+            Ok(Some(marker)),
+            "the id is created by hashing the merge buffer"
+        );
         let mut lines = cleaned_driver_lines(&buf)?;
         for tmp_file in lines.by_ref().take(3) {
             assert!(tmp_file.contains_str(&b".tmp"[..]), "{tmp_file}");

gix-merge/tests/merge/main.rs

@@ -1,6 +1,11 @@
+use gix_hash::ObjectId;
 extern crate core;
 
 #[cfg(feature = "blob")]
 mod blob;
 
 pub use gix_testtools::Result;
+
+fn hex_to_id(hex: &str) -> ObjectId {
+    ObjectId::from_hex(hex.as_bytes()).expect("40 bytes hex")
+}