feat!: Don't fail on big files during blob-merge, but turn them into binary merges.

Binary merges are mere choices of which side to pick, which works well for big files
as well. Git doesn't define this well during its own merges, so there is some room here.

Author: Byron

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

@@ -18,8 +18,6 @@ pub struct Options {
 #[derive(Debug, thiserror::Error)]
 #[allow(missing_docs)]
 pub enum Error {
-    #[error("At least one resource was too large to be processed")]
-    ResourceTooLarge,
     #[error(transparent)]
     PrepareExternalDriver(#[from] inner::prepare_external_driver::Error),
     #[error("Failed to launch external merge driver: {cmd}")]
@@ -299,25 +297,29 @@ pub(super) mod inner {
             /// Returns `None` if one of the buffers is too large, making a merge impossible.
             /// Note that if the *pick* wasn't [`Pick::Buffer`], then `out` will not have been cleared,
             /// and one has to take the data from the respective resource.
+            ///
+            /// If there is no buffer loaded as the resource is too big, we will automatically perform a binary merge
+            /// which effectively chooses our side by default.
             pub fn builtin_merge(
                 &self,
                 driver: BuiltinDriver,
                 out: &mut Vec<u8>,
                 input: &mut imara_diff::intern::InternedInput<&'parent [u8]>,
                 labels: builtin_driver::text::Labels<'_>,
-            ) -> Option<(Pick, Resolution)> {
-                let base = self.ancestor.data.as_slice()?;
-                let ours = self.current.data.as_slice()?;
-                let theirs = self.other.data.as_slice()?;
+            ) -> (Pick, Resolution) {
+                let base = self.ancestor.data.as_slice();
+                let ours = self.current.data.as_slice();
+                let theirs = self.other.data.as_slice();
                 let driver = if driver != BuiltinDriver::Binary
                     && (is_binary_buf(ours) || is_binary_buf(theirs) || is_binary_buf(base))
                 {
                     BuiltinDriver::Binary
                 } else {
                     driver
                 };
-                Some(match 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)
@@ -332,6 +334,7 @@ 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,
@@ -346,13 +349,15 @@ pub(super) mod inner {
                         );
                         (Pick::Buffer, resolution)
                     }
-                })
+                }
             }
         }
 
-        fn is_binary_buf(buf: &[u8]) -> bool {
-            let buf = &buf[..buf.len().min(8000)];
-            buf.contains(&0)
+        fn is_binary_buf(buf: Option<&[u8]>) -> bool {
+            buf.map_or(true, |buf| {
+                let buf = &buf[..buf.len().min(8000)];
+                buf.contains(&0)
+            })
         }
     }
 }
@@ -400,9 +405,7 @@ impl<'parent> PlatformRef<'parent> {
             Err(builtin) => {
                 let mut input = imara_diff::intern::InternedInput::new(&[][..], &[]);
                 out.clear();
-                let (pick, resolution) = self
-                    .builtin_merge(builtin, out, &mut input, labels)
-                    .ok_or(Error::ResourceTooLarge)?;
+                let (pick, resolution) = self.builtin_merge(builtin, out, &mut input, labels);
                 Ok((pick, resolution))
             }
         }

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

@@ -295,11 +295,7 @@ theirs
 
         let mut input = imara_diff::intern::InternedInput::new(&[][..], &[]);
         let res = platform_ref.builtin_merge(BuiltinDriver::Text, &mut buf, &mut input, Default::default());
-        assert_eq!(
-            res,
-            Some((Pick::Buffer, Resolution::Complete)),
-            "both versions are deleted"
-        );
+        assert_eq!(res, (Pick::Buffer, Resolution::Complete), "both versions are deleted");
         assert!(buf.is_empty(), "the result is the same on direct invocation");
 
         let print_all = "for arg in $@ %O %A %B %L %P %S %X %Y %F; do echo $arg; done";
@@ -358,24 +354,27 @@ theirs
         assert_eq!(platform_ref.other.data, platform::resource::Data::TooLarge { size: 12 });
 
         let mut out = Vec::new();
-        let err = platform_ref
-            .merge(&mut out, Default::default(), &Default::default())
-            .unwrap_err();
-        assert!(matches!(err, platform::merge::Error::ResourceTooLarge));
+        let res = platform_ref.merge(&mut out, Default::default(), &Default::default())?;
+        assert_eq!(
+            res,
+            (Pick::Ours, Resolution::Conflict),
+            "this is the default for binary merges, which are used in this case"
+        );
 
         let mut input = imara_diff::intern::InternedInput::new(&[][..], &[]);
         assert_eq!(
             platform_ref.builtin_merge(BuiltinDriver::Text, &mut out, &mut input, Default::default(),),
-            None
+            res,
+            "we can't enforce it, it will just default to using binary"
         );
 
         let err = platform_ref
             .prepare_external_driver("bogus".into(), Default::default(), Default::default())
             .unwrap_err();
-        assert!(matches!(
-            err,
-            platform::prepare_external_driver::Error::ResourceTooLarge { .. }
-        ));
+        assert!(
+            matches!(err, platform::prepare_external_driver::Error::ResourceTooLarge { .. }),
+            "however, for external drivers, resources can still be too much to handle, until we learn how to stream them"
+        );
         Ok(())
     }