Finish the entire merge implementation and cover everything with tests.

Author: Byron

Cargo.lock

@@ -43,6 +43,7 @@ document-features = { version = "0.2.0", optional = true }
 [dev-dependencies]
 gix-testtools = { path = "../tests/tools" }
 gix-odb = { path = "../gix-odb" }
+gix-utils = { version = "^0.1.12", path = "../gix-utils" }
 pretty_assertions = "1.4.0"
 
 [package.metadata.docs.rs]

gix-merge/Cargo.toml

@@ -18,16 +18,12 @@ pub enum Error {
 }
 
 /// A way to configure [`commit()`](crate::commit()).
-#[derive(Default, Debug, Copy, Clone)]
+#[derive(Default, Debug, Clone)]
 pub struct Options {
     /// If `true`, merging unrelated commits is allowed, with the merge-base being assumed as empty tree.
     pub allow_missing_merge_base: bool,
     /// Options to define how trees should be merged.
     pub tree_merge: crate::tree::Options,
-    /// Options to define how to merge blobs.
-    ///
-    /// Note that these are temporarily overwritten if multiple merge-bases are merged into one.
-    pub blob_merge: crate::blob::platform::merge::Options,
 }
 
 pub(super) mod function {
@@ -51,16 +47,20 @@ pub(super) mod function {
     ///
     /// Note that `objects` *should* have an object cache to greatly accelerate tree-retrieval.
     #[allow(clippy::too_many_arguments)]
-    pub fn commit<'objects>(
+    pub fn commit<'objects, E>(
         our_commit: gix_hash::ObjectId,
         their_commit: gix_hash::ObjectId,
         mut labels: crate::blob::builtin_driver::text::Labels<'_>,
         graph: &mut gix_revwalk::Graph<'_, '_, gix_revwalk::graph::Commit<gix_revision::merge_base::Flags>>,
         diff_resource_cache: &mut gix_diff::blob::Platform,
         blob_merge: &mut crate::blob::Platform,
         objects: &'objects impl gix_object::FindObjectOrHeader,
+        write_blob_to_odb: impl FnMut(&[u8]) -> Result<gix_hash::ObjectId, E>,
         options: Options,
-    ) -> Result<crate::tree::Outcome<'objects>, Error> {
+    ) -> Result<crate::tree::Outcome<'objects>, Error>
+    where
+        E: Into<Box<dyn std::error::Error + Send + Sync + 'static>>,
+    {
         let merge_bases_commit_ids = gix_revision::merge_base(our_commit, &[their_commit], graph)?;
         let (merge_base_commit_id, ancestor_name) = match merge_bases_commit_ids {
             Some(base_commit) if base_commit.len() == 1 => (base_commit[0], None),
@@ -97,6 +97,7 @@ pub(super) mod function {
             &their_tree_id,
             labels,
             objects,
+            write_blob_to_odb,
             &mut state,
             diff_resource_cache,
             blob_merge,

gix-merge/src/commit.rs

@@ -31,8 +31,8 @@ function baseline () (
   their_commit_id="$(git rev-parse "$their_committish")"
 
   local merge_info="${output_name}.merge-info"
-  git merge-tree -z --write-tree "$our_commit_id" "$their_commit_id" > "$merge_info" || :
-  echo "$dir" "$our_commit_id" "$their_commit_id" "$merge_info" >> ../baseline.cases
+  git merge-tree -z --write-tree "$our_committish" "$their_committish" > "$merge_info" || :
+  echo "$dir" "$our_commit_id" "$our_committish" "$their_commit_id" "$their_committish" "$merge_info" >> ../baseline.cases
 )
 
 git init simple
@@ -88,3 +88,4 @@ git init simple
 )
 
 baseline simple without-conflict side1 side3
+baseline simple various-conflicts side1 side2

gix-merge/src/tree.rs

@@ -9,7 +9,9 @@ fn run_baseline() -> crate::Result {
         root,
         odb,
         our_commit_id,
+        our_side_name,
         their_commit_id,
+        their_side_name,
         merge_info,
         case_name,
     } in baseline::Expectations::new(&root, &cases)
@@ -25,50 +27,123 @@ fn run_baseline() -> crate::Result {
                     percentage: Some(0.5),
                     limit: 0,
                 }),
+                blob_merge: Default::default(),
+                blob_merge_command_ctx: Default::default(),
             },
-            blob_merge: Default::default(),
         };
         let mut actual = gix_merge::commit(
             our_commit_id,
             their_commit_id,
             gix_merge::blob::builtin_driver::text::Labels {
                 ancestor: None,
-                current: Some("ours".into()),
-                other: Some("theirs".into()),
+                current: Some(our_side_name.as_str().into()),
+                other: Some(their_side_name.as_str().into()),
             },
             &mut graph,
             &mut diff_resource_cache,
             &mut blob_merge,
             &odb,
+            |content| odb.write_buf(gix_object::Kind::Blob, content),
             options,
         )?;
 
-        match merge_info {
-            Ok(expected_tree_id) => {
-                let actual_id = actual.tree.write(|tree| odb.write(tree))?;
-                assert_eq!(actual_id, expected_tree_id, "{case_name}: merged tree mismatch");
-            }
-            Err(_conflicts) => {
-                todo!("compare conflicts")
-            }
+        let actual_id = actual.tree.write(|tree| odb.write(tree))?;
+        assert_eq!(actual_id, merge_info.merged_tree, "{case_name}: merged tree mismatch");
+        if let Some(conflicts) = merge_info.conflicts {
+            dbg!(&conflicts, &merge_info.information);
+            todo!("compare merge conflict information")
         }
     }
 
     Ok(())
 }
 
+// TODO: make sure everything is read eventually, even if only to improve debug messages in case of failure.
+#[allow(dead_code)]
 mod baseline {
+    use gix_object::tree::EntryMode;
     use gix_worktree::stack::state::attributes;
     use std::path::{Path, PathBuf};
 
-    pub struct Conflict;
+    /// An entry in the conflict
+    #[derive(Debug)]
+    pub struct Entry {
+        /// The relative path in the repository
+        pub location: String,
+        /// The content id.
+        pub id: gix_hash::ObjectId,
+        /// The kind of entry.
+        pub mode: EntryMode,
+    }
+
+    /// Keep track of all the sides of a conflict. Some might not be set to indicate removal, including the ancestor.
+    #[derive(Default, Debug)]
+    pub struct Conflict {
+        pub ancestor: Option<Entry>,
+        pub ours: Option<Entry>,
+        pub theirs: Option<Entry>,
+    }
+
+    #[derive(Debug)]
+    pub enum ConflictKind {
+        /// The conflict was resolved by automatically merging the content.
+        AutoMerging,
+        /// The content could not be resolved so it's conflicting.
+        ConflictContents,
+        /// Directory in theirs in the way of our file
+        ConflictDirectoryBlocksFile,
+        /// Modified in ours but deleted in theirs
+        ConflictModifyDelete,
+    }
+
+    /// More loosely structured information about the `Conflict`.
+    #[derive(Debug)]
+    pub struct ConflictInfo {
+        /// All the paths involved in the informational message
+        pub paths: Vec<String>,
+        /// The type of the conflict, further described in `message`.
+        pub kind: ConflictKind,
+        /// An arbitrary message formed from paths and kind
+        pub message: String,
+    }
+
+    impl Conflict {
+        fn any_location(&self) -> Option<&str> {
+            self.ancestor
+                .as_ref()
+                .or(self.ours.as_ref())
+                .or(self.theirs.as_ref())
+                .map(|a| a.location.as_str())
+        }
+        fn storage_for(&mut self, side: Side, location: &str) -> Option<&mut Option<Entry>> {
+            let current_location = self.any_location();
+            let location_is_same = current_location.is_none() || current_location == Some(location);
+            let side = match side {
+                Side::Ancestor => &mut self.ancestor,
+                Side::Ours => &mut self.ours,
+                Side::Theirs => &mut self.theirs,
+            };
+            (!side.is_some() && location_is_same).then_some(side)
+        }
+    }
+
+    pub struct MergeInfo {
+        /// The hash of the merged tree - it may contain intermediate files if the merge didn't succeed entirely.
+        pub merged_tree: gix_hash::ObjectId,
+        /// If there were conflicts, this is the conflicting paths.
+        pub conflicts: Option<Vec<Conflict>>,
+        /// Structured details which to some extent can be compared to our own conflict information.
+        pub information: Vec<ConflictInfo>,
+    }
 
     pub struct Expectation {
         pub root: PathBuf,
         pub odb: gix_odb::memory::Proxy<gix_odb::Handle>,
         pub our_commit_id: gix_hash::ObjectId,
+        pub our_side_name: String,
         pub their_commit_id: gix_hash::ObjectId,
-        pub merge_info: Result<gix_hash::ObjectId, Conflict>,
+        pub their_side_name: String,
+        pub merge_info: MergeInfo,
         pub case_name: String,
     }
 
@@ -92,8 +167,21 @@ mod baseline {
         fn next(&mut self) -> Option<Self::Item> {
             let line = self.lines.next()?;
             let mut tokens = line.split(' ');
-            let (Some(subdir), Some(our_commit_id), Some(their_commit_id), Some(merge_info_filename)) =
-                (tokens.next(), tokens.next(), tokens.next(), tokens.next())
+            let (
+                Some(subdir),
+                Some(our_commit_id),
+                Some(our_side_name),
+                Some(their_commit_id),
+                Some(their_side_name),
+                Some(merge_info_filename),
+            ) = (
+                tokens.next(),
+                tokens.next(),
+                tokens.next(),
+                tokens.next(),
+                tokens.next(),
+                tokens.next(),
+            )
             else {
                 unreachable!("invalid line: {line:?}")
             };
@@ -109,7 +197,9 @@ mod baseline {
                 root: subdir_path,
                 odb: objects,
                 our_commit_id,
+                our_side_name: our_side_name.to_owned(),
                 their_commit_id,
+                their_side_name: their_side_name.to_owned(),
                 merge_info,
                 case_name: format!(
                     "{subdir}-{}",
@@ -122,11 +212,93 @@ mod baseline {
         }
     }
 
-    fn parse_merge_info(content: String) -> Result<gix_hash::ObjectId, Conflict> {
-        let mut lines = content.split('\0').filter(|t| !t.is_empty());
+    fn parse_merge_info(content: String) -> MergeInfo {
+        let mut lines = content.split('\0').filter(|t| !t.is_empty()).peekable();
         let tree_id = gix_hash::ObjectId::from_hex(lines.next().unwrap().as_bytes()).unwrap();
-        assert_eq!(lines.next(), None, "TODO: implement multi-line answer");
-        Ok(tree_id)
+        let mut out = MergeInfo {
+            merged_tree: tree_id,
+            conflicts: None,
+            information: Vec::new(),
+        };
+
+        let mut conflicts = Vec::new();
+        let mut conflict = Conflict::default();
+        while let Some(line) = lines.peek() {
+            let (entry, side) = match parse_conflict_file_info(line) {
+                Some(t) => t,
+                None => break,
+            };
+            lines.next();
+            let field = match conflict.storage_for(side, &entry.location) {
+                None => {
+                    conflicts.push(conflict);
+                    conflict = Conflict::default();
+                    conflict
+                        .storage_for(side, &entry.location)
+                        .expect("always available for new side")
+                }
+                Some(field) => field,
+            };
+            *field = Some(entry);
+        }
+
+        while lines.peek().is_some() {
+            out.information
+                .push(parse_info(&mut lines).expect("if there are lines, it should be valid info"));
+        }
+        assert_eq!(lines.next(), None, "TODO: conflict messages");
+        out.conflicts = (!conflicts.is_empty()).then_some(conflicts);
+        out
+    }
+
+    #[derive(Copy, Clone)]
+    enum Side {
+        Ancestor,
+        Ours,
+        Theirs,
+    }
+
+    fn parse_conflict_file_info(line: &str) -> Option<(Entry, Side)> {
+        let (info, path) = line.split_at(line.find('\t')?);
+        let mut tokens = info.split(' ');
+        let (oct_mode, hex_id, stage) = (
+            tokens.next().expect("mode"),
+            tokens.next().expect("id"),
+            tokens.next().expect("stage"),
+        );
+        assert_eq!(
+            tokens.next(),
+            None,
+            "info line not understood, expected three fields only"
+        );
+        Some((
+            Entry {
+                location: path.to_owned(),
+                id: gix_hash::ObjectId::from_hex(hex_id.as_bytes()).unwrap(),
+                mode: EntryMode(gix_utils::btoi::to_signed_with_radix::<usize>(oct_mode.as_bytes(), 8).unwrap() as u16),
+            },
+            match stage {
+                "1" => Side::Ancestor,
+                "2" => Side::Ours,
+                "3" => Side::Theirs,
+                invalid => panic!("{invalid} is an unexpected side"),
+            },
+        ))
+    }
+
+    fn parse_info<'a>(mut lines: impl Iterator<Item = &'a str>) -> Option<ConflictInfo> {
+        let num_paths: usize = lines.next()?.parse().ok()?;
+        let paths: Vec<_> = lines.by_ref().take(num_paths).map(ToOwned::to_owned).collect();
+        let kind = match lines.next()? {
+            "Auto-merging" => ConflictKind::AutoMerging,
+            "CONFLICT (contents)" => ConflictKind::ConflictContents,
+            "CONFLICT (file/directory)" => ConflictKind::ConflictDirectoryBlocksFile,
+            "CONFLICT (modify/delete)" => ConflictKind::ConflictModifyDelete,
+            conflict_type => panic!("Unkonwn conflict type: {conflict_type}"),
+        };
+        let message = lines.next()?.to_owned();
+        dbg!(&kind, &message);
+        Some(ConflictInfo { paths, kind, message })
     }
 
     pub fn new_platform(