Use gix-merge to merge trees when cherry-picking.

This change affects the entire edit mode, and every rebase.

Author: Byron

Cargo.lock

@@ -8,4 +8,6 @@ publish = false
 [dependencies]
 gitbutler-commit.workspace = true
 git2.workspace = true
+gitbutler-oxidize.workspace = true
+gix.workspace = true
 anyhow.workspace = true

crates/gitbutler-cherry-pick/Cargo.toml

@@ -6,9 +6,10 @@
 
 use std::ops::Deref;
 
-use anyhow::Context;
+use anyhow::{Context, Result};
 use git2::MergeOptions;
 use gitbutler_commit::commit_ext::CommitExt;
+use gitbutler_oxidize::git2_to_gix_object_id;
 
 #[derive(Default)]
 pub enum ConflictedTreeKey {
@@ -40,30 +41,55 @@ impl Deref for ConflictedTreeKey {
 }
 
 pub trait RepositoryExt {
+    /// Cherry-pick, but understands GitButler conflicted states.
+    ///
+    /// This method *should* always be used in favour of native functions.
     fn cherry_pick_gitbutler(
         &self,
         head: &git2::Commit,
         to_rebase: &git2::Commit,
         merge_options: Option<&MergeOptions>,
-    ) -> Result<git2::Index, anyhow::Error>;
-    fn find_real_tree(
-        &self,
-        commit: &git2::Commit,
+    ) -> Result<git2::Index>;
+
+    /// Find the real tree of a commit, which is the tree of the commit if it's not in a conflicted state
+    /// or the tree according to `side` if it is conflicted.
+    ///
+    /// Unless you want to find a particular side, you likely want to pass Default::default()
+    /// as the [`side`](ConflictedTreeKey) which will give the automatically resolved resolution
+    fn find_real_tree(&self, commit: &git2::Commit, side: ConflictedTreeKey) -> Result<git2::Tree>;
+}
+
+pub trait GixRepositoryExt {
+    /// Cherry-pick, but understands GitButler conflicted states.
+    /// Note that it will automatically resolve conflicts in *our* favor, so any tree produced
+    /// here can be used.
+    ///
+    /// This method *should* always be used in favour of native functions.
+    fn cherry_pick_gitbutler<'repo>(
+        &'repo self,
+        head: &git2::Commit,
+        to_rebase: &git2::Commit,
+    ) -> Result<gix::merge::tree::Outcome<'repo>>;
+
+    /// Find the real tree of a commit, which is the tree of the commit if it's not in a conflicted state
+    /// or the tree according to `side` if it is conflicted.
+    ///
+    /// Unless you want to find a particular side, you likely want to pass Default::default()
+    /// as the [`side`](ConflictedTreeKey) which will give the automatically resolved resolution
+    fn find_real_tree<'repo>(
+        &'repo self,
+        commit_id: &gix::oid,
         side: ConflictedTreeKey,
-    ) -> Result<git2::Tree, anyhow::Error>;
+    ) -> Result<gix::Id<'repo>>;
 }
 
 impl RepositoryExt for git2::Repository {
-    /// cherry-pick, but understands GitButler conflicted states
-    ///
-    /// cherry_pick_gitbutler should always be used in favour of libgit2 or gitoxide
-    /// cherry pick functions
     fn cherry_pick_gitbutler(
         &self,
         head: &git2::Commit,
         to_rebase: &git2::Commit,
         merge_options: Option<&MergeOptions>,
-    ) -> Result<git2::Index, anyhow::Error> {
+    ) -> Result<git2::Index> {
         // we need to do a manual 3-way patch merge
         // find the base, which is the parent of to_rebase
         let base = if to_rebase.is_conflicted() {
@@ -77,22 +103,13 @@ impl RepositoryExt for git2::Repository {
         // Get the auto-resolution
         let ours = self.find_real_tree(head, Default::default())?;
         // Get the original theirs
-        let thiers = self.find_real_tree(to_rebase, ConflictedTreeKey::Theirs)?;
+        let theirs = self.find_real_tree(to_rebase, ConflictedTreeKey::Theirs)?;
 
-        self.merge_trees(&base, &ours, &thiers, merge_options)
+        self.merge_trees(&base, &ours, &theirs, merge_options)
             .context("failed to merge trees for cherry pick")
     }
 
-    /// Find the real tree of a commit, which is the tree of the commit if it's not in a conflicted state
-    /// or the parent parent tree if it is in a conflicted state
-    ///
-    /// Unless you want to find a particular side, you likly want to pass Default::default()
-    /// as the ConfclitedTreeKey which will give the automatically resolved resolution
-    fn find_real_tree(
-        &self,
-        commit: &git2::Commit,
-        side: ConflictedTreeKey,
-    ) -> Result<git2::Tree, anyhow::Error> {
+    fn find_real_tree(&self, commit: &git2::Commit, side: ConflictedTreeKey) -> Result<git2::Tree> {
         let tree = commit.tree()?;
         if commit.is_conflicted() {
             let conflicted_side = tree
@@ -105,3 +122,64 @@ impl RepositoryExt for git2::Repository {
         }
     }
 }
+
+impl GixRepositoryExt for gix::Repository {
+    fn cherry_pick_gitbutler<'repo>(
+        &'repo self,
+        head: &git2::Commit,
+        to_rebase: &git2::Commit,
+    ) -> Result<gix::merge::tree::Outcome<'repo>> {
+        // we need to do a manual 3-way patch merge
+        // find the base, which is the parent of to_rebase
+        let base = if to_rebase.is_conflicted() {
+            // Use to_rebase's recorded base
+            self.find_real_tree(
+                &git2_to_gix_object_id(to_rebase.id()),
+                ConflictedTreeKey::Base,
+            )?
+        } else {
+            let base_commit = to_rebase.parent(0)?;
+            // Use the parent's auto-resolution
+            self.find_real_tree(&git2_to_gix_object_id(base_commit.id()), Default::default())?
+        };
+        // Get the auto-resolution
+        let ours = self.find_real_tree(&git2_to_gix_object_id(head.id()), Default::default())?;
+        // Get the original theirs
+        let theirs = self.find_real_tree(
+            &git2_to_gix_object_id(to_rebase.id()),
+            ConflictedTreeKey::Theirs,
+        )?;
+
+        self.merge_trees(
+            base,
+            ours,
+            theirs,
+            gix::merge::blob::builtin_driver::text::Labels {
+                ancestor: Some("base".into()),
+                current: Some("ours".into()),
+                other: Some("theirs".into()),
+            },
+            self.tree_merge_options()?
+                .with_tree_favor(Some(gix::merge::tree::TreeFavor::Ours))
+                .with_file_favor(Some(gix::merge::tree::FileFavor::Ours)),
+        )
+        .context("failed to merge trees for cherry pick")
+    }
+
+    fn find_real_tree<'repo>(
+        &'repo self,
+        commit_id: &gix::oid,
+        side: ConflictedTreeKey,
+    ) -> Result<gix::Id<'repo>> {
+        let commit = self.find_commit(commit_id)?;
+        Ok(if commit.is_conflicted() {
+            let tree = commit.tree()?;
+            let conflicted_side = tree
+                .find_entry(&*side)
+                .context("Failed to get conflicted side of commit")?;
+            conflicted_side.id()
+        } else {
+            commit.tree_id()?
+        })
+    }
+}

crates/gitbutler-cherry-pick/src/lib.rs

@@ -7,5 +7,6 @@ publish = false
 
 [dependencies]
 git2.workspace = true
+gix.workspace = true
 bstr.workspace = true
 uuid.workspace = true

crates/gitbutler-commit/Cargo.toml

@@ -32,6 +32,29 @@ impl CommitExt for git2::Commit<'_> {
     }
 }
 
+impl CommitExt for gix::Commit<'_> {
+    fn message_bstr(&self) -> &BStr {
+        self.message_raw()
+            .expect("valid commit that can be parsed: TODO - allow it to return errors?")
+    }
+
+    fn change_id(&self) -> Option<String> {
+        self.gitbutler_headers().map(|headers| headers.change_id)
+    }
+
+    fn is_signed(&self) -> bool {
+        self.decode().map_or(false, |decoded| {
+            decoded.extra_headers().pgp_signature().is_some()
+        })
+    }
+
+    fn is_conflicted(&self) -> bool {
+        self.gitbutler_headers()
+            .and_then(|headers| headers.conflicted.map(|conflicted| conflicted > 0))
+            .unwrap_or(false)
+    }
+}
+
 fn contains<'a, I>(iter: I, item: &git2::Commit<'a>) -> bool
 where
     I: IntoIterator<Item = git2::Commit<'a>>,

crates/gitbutler-commit/src/commit_ext.rs

@@ -1,4 +1,4 @@
-use bstr::{BStr, BString};
+use bstr::{BStr, BString, ByteSlice};
 use uuid::Uuid;
 
 /// Header used to determine which version of the headers is in use. This should never be changed
@@ -113,6 +113,41 @@ impl HasCommitHeaders for git2::Commit<'_> {
     }
 }
 
+impl HasCommitHeaders for gix::Commit<'_> {
+    fn gitbutler_headers(&self) -> Option<CommitHeadersV2> {
+        let decoded = self.decode().ok()?;
+        if let Some(header) = decoded.extra_headers().find(HEADERS_VERSION_HEADER) {
+            let version_number = header.to_owned();
+
+            // Parse v2 headers
+            if version_number == V2_HEADERS_VERSION {
+                let change_id = decoded.extra_headers().find(V2_CHANGE_ID_HEADER)?;
+                // We can safely assume that the change id should be UTF8
+                let change_id = change_id.to_str().ok()?.to_string();
+
+                let conflicted = decoded
+                    .extra_headers()
+                    .find(V2_CONFLICTED_HEADER)
+                    .and_then(|value| value.to_str().ok()?.parse::<u64>().ok());
+
+                Some(CommitHeadersV2 {
+                    change_id,
+                    conflicted,
+                })
+            } else {
+                // Must be for a version we don't recognise
+                None
+            }
+        } else {
+            // Parse v1 headers
+            let change_id = decoded.extra_headers().find(V1_CHANGE_ID_HEADER)?;
+            let change_id = change_id.to_str().ok()?.to_string();
+            let headers = CommitHeadersV1 { change_id };
+            Some(headers.into())
+        }
+    }
+}
+
 /// Lifecycle
 impl CommitHeadersV2 {
     /// Used to create a CommitHeadersV2. This does not allow a change_id to be