fix: when binary merges are performed, adjust the returned resolution to indicate auto-resolution.

Previously it wasn't possible to detect auto-resolution, even though it can be assumed if a
resolution mode was provided.

Author: Byron

gix-merge/src/blob/builtin_driver/binary.rs

@@ -25,8 +25,10 @@ pub(super) mod function {
     use crate::blob::Resolution;
 
     /// As this algorithm doesn't look at the actual data, it returns a choice solely based on logic.
+    /// This also means that the caller has to assure this only gets called if the input *doesn't* match.
     ///
-    /// It always results in a conflict with `current` being picked unless `on_conflict` is not `None`.
+    /// It always results in a conflict with `current` being picked unless `on_conflict` is not `None`,
+    /// which is when we always return [`Resolution::CompleteWithAutoResolvedConflict`].
     pub fn merge(on_conflict: Option<ResolveWith>) -> (Pick, Resolution) {
         match on_conflict {
             None => (Pick::Ours, Resolution::Conflict),
@@ -36,7 +38,7 @@ pub(super) mod function {
                     ResolveWith::Theirs => Pick::Theirs,
                     ResolveWith::Ancestor => Pick::Ancestor,
                 },
-                Resolution::Complete,
+                Resolution::CompleteWithAutoResolvedConflict,
             ),
         }
     }

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

@@ -329,13 +329,21 @@ pub(super) mod inner {
                         (Pick::Buffer, resolution)
                     }
                     BuiltinDriver::Binary => {
-                        let (pick, resolution) = builtin_driver::binary(self.options.resolve_binary_with);
-                        let pick = match pick {
-                            builtin_driver::binary::Pick::Ours => Pick::Ours,
-                            builtin_driver::binary::Pick::Theirs => Pick::Theirs,
-                            builtin_driver::binary::Pick::Ancestor => Pick::Ancestor,
-                        };
-                        (pick, resolution)
+                        // easier to reason about the 'split' compared to merging both conditions
+                        #[allow(clippy::if_same_then_else)]
+                        if !(self.current.id.is_null() || self.other.id.is_null()) && self.current.id == self.other.id {
+                            (Pick::Ours, Resolution::Complete)
+                        } else if (self.current.id.is_null() || self.other.id.is_null()) && ours == theirs {
+                            (Pick::Ours, Resolution::Complete)
+                        } else {
+                            let (pick, resolution) = builtin_driver::binary(self.options.resolve_binary_with);
+                            let pick = match pick {
+                                builtin_driver::binary::Pick::Ours => Pick::Ours,
+                                builtin_driver::binary::Pick::Theirs => Pick::Theirs,
+                                builtin_driver::binary::Pick::Ancestor => Pick::Ancestor,
+                            };
+                            (pick, resolution)
+                        }
                     }
                     BuiltinDriver::Union => {
                         let resolution = builtin_driver::text(

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

@@ -10,16 +10,16 @@ fn binary() {
     );
     assert_eq!(
         builtin_driver::binary(Some(ResolveWith::Ancestor)),
-        (Pick::Ancestor, Resolution::Complete),
+        (Pick::Ancestor, Resolution::CompleteWithAutoResolvedConflict),
         "Otherwise we can pick anything and it will mark it as complete"
     );
     assert_eq!(
         builtin_driver::binary(Some(ResolveWith::Ours)),
-        (Pick::Ours, Resolution::Complete)
+        (Pick::Ours, Resolution::CompleteWithAutoResolvedConflict)
     );
     assert_eq!(
         builtin_driver::binary(Some(ResolveWith::Theirs)),
-        (Pick::Theirs, Resolution::Complete)
+        (Pick::Theirs, Resolution::CompleteWithAutoResolvedConflict)
     );
 }
 

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

@@ -59,12 +59,61 @@ mod merge {
         let res = platform_ref.merge(&mut buf, default_labels(), &Default::default())?;
         assert_eq!(
             res,
-            (Pick::Theirs, Resolution::Complete),
+            (Pick::Theirs, Resolution::CompleteWithAutoResolvedConflict),
             "the auto-binary driver respects its own options"
         );
         Ok(())
     }
 
+    #[test]
+    fn same_binaries_do_not_count_as_conflicted() -> crate::Result {
+        let mut platform = new_platform(None, pipeline::Mode::ToGit);
+        platform.set_resource(
+            gix_hash::Kind::Sha1.null(),
+            EntryKind::Blob,
+            "a".into(),
+            ResourceKind::CommonAncestorOrBase,
+            &gix_object::find::Never,
+        )?;
+
+        let mut db = ObjectDb::default();
+        for (content, kind) in [
+            ("any\0", ResourceKind::CurrentOrOurs),
+            ("any\0", ResourceKind::OtherOrTheirs),
+        ] {
+            let id = db.insert(content);
+            platform.set_resource(
+                id,
+                EntryKind::Blob,
+                "path matters only for attribute lookup".into(),
+                kind,
+                &db,
+            )?;
+        }
+        let platform_ref = platform.prepare_merge(&db, Default::default())?;
+        assert_eq!(
+            platform_ref.driver,
+            DriverChoice::BuiltIn(BuiltinDriver::Text),
+            "it starts out at the default text driver"
+        );
+
+        let mut buf = Vec::new();
+        let res = platform_ref.merge(&mut buf, default_labels(), &Default::default())?;
+        assert_eq!(
+            res,
+            (Pick::Ours, Resolution::Complete),
+            "as both are the same, it just picks ours, declaring it non-conflicting"
+        );
+
+        let mut input = imara_diff::intern::InternedInput::new(&[][..], &[]);
+        assert_eq!(
+            platform_ref.builtin_merge(BuiltinDriver::Binary, &mut buf, &mut input, default_labels()),
+            res,
+            "no way to work around it - calling the built-in binary merge is the same"
+        );
+        Ok(())
+    }
+
     #[test]
     fn builtin_with_conflict() -> crate::Result {
         let mut platform = new_platform(None, pipeline::Mode::ToGit);
@@ -192,7 +241,7 @@ theirs
         ] {
             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!(res, (expected_pick, Resolution::CompleteWithAutoResolvedConflict));
             assert_eq!(platform_ref.buffer_by_pick(res.0).unwrap().unwrap().as_bstr(), expected);
 
             assert_eq!(