gitaly-git2go apply subcommand

Implements apply subcommand for gitaly-git2go that allows for
applying patches from diffs. Similar to 'git am', three-way merge
is attempted as a fallback if the patch does not apply cleanly.

5 files changed, 343 insertions, 5 deletions

diff --git a/internal/git2go/apply.go b/internal/git2go/apply.go
new file mode 100644
index 000000000..447e92530
--- /dev/null
+++ b/internal/git2go/apply.go
@@ -0,0 +1,112 @@
+package git2go
+
+import (
+	"context"
+	"encoding/gob"
+	"fmt"
+	"io"
+)
+
+// ErrMergeConflict is returned when there is a merge conflict.
+var ErrMergeConflict = wrapError{Message: "merge conflict"}
+
+// Patch represents a single patch.
+type Patch struct {
+	// Author is the author of the patch.
+	Author Signature
+	// Message is used as the commit message when applying the patch.
+	Message string
+	// Diff contains the diff of the patch.
+	Diff []byte
+}
+
+// ApplyParams are the parameters for Apply.
+type ApplyParams struct {
+	// Repository is the path to the repository.
+	Repository string
+	// Committer is the committer applying the patch.
+	Committer Signature
+	// ParentCommit is the OID of the commit to apply the patches against.
+	ParentCommit string
+	// Patches iterates over all the patches to be applied.
+	Patches PatchIterator
+}
+
+// PatchIterator iterates over a stream of patches.
+type PatchIterator interface {
+	// Next returns whether there is a next patch.
+	Next() bool
+	// Value returns the patch being currently iterated upon.
+	Value() Patch
+	// Err returns the iteration error. Err should
+	// be always checked after Next returns false.
+	Err() error
+}
+
+type slicePatchIterator struct {
+	value   Patch
+	patches []Patch
+}
+
+// NewSlicePatchIterator returns a PatchIterator that iterates over the slice
+// of patches.
+func NewSlicePatchIterator(patches []Patch) PatchIterator {
+	return &slicePatchIterator{patches: patches}
+}
+
+func (iter *slicePatchIterator) Next() bool {
+	if len(iter.patches) == 0 {
+		return false
+	}
+
+	iter.value = iter.patches[0]
+	iter.patches = iter.patches[1:]
+	return true
+}
+
+func (iter *slicePatchIterator) Value() Patch { return iter.value }
+
+func (iter *slicePatchIterator) Err() error { return nil }
+
+// Apply applies the provided patches and returns the OID of the commit with the patches
+// applied.
+func (b Executor) Apply(ctx context.Context, params ApplyParams) (string, error) {
+	reader, writer := io.Pipe()
+	defer writer.Close()
+
+	go func() {
+		writer.CloseWithError(func() error {
+			patches := params.Patches
+			params.Patches = nil
+
+			encoder := gob.NewEncoder(writer)
+			if err := encoder.Encode(params); err != nil {
+				return fmt.Errorf("encode header: %w", err)
+			}
+
+			for patches.Next() {
+				if err := encoder.Encode(patches.Value()); err != nil {
+					return fmt.Errorf("encode patch: %w", err)
+				}
+			}
+
+			if err := patches.Err(); err != nil {
+				return fmt.Errorf("patch iterator: %w", err)
+			}
+
+			return nil
+		}())
+	}()
+
+	var result Result
+	output, err := run(ctx, b.binaryPath, reader, "apply", "-git-binary-path", b.gitBinaryPath)
+	if err != nil {
+		return "", fmt.Errorf("run: %w", err)
+	}
+
+	if err := gob.NewDecoder(output).Decode(&result); err != nil {
+		return "", fmt.Errorf("decode: %w", err)
+	}
+
+	return result.CommitID, result.Error
+}
diff --git a/internal/git2go/apply_test.go b/internal/git2go/apply_test.go
new file mode 100644
index 000000000..1744432b6
--- /dev/null
+++ b/internal/git2go/apply_test.go
@@ -0,0 +1,217 @@
+package git2go
+
+import (
+	"errors"
+	"path/filepath"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/internal/command"
+	"gitlab.com/gitlab-org/gitaly/internal/git"
+	"gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
+	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
+)
+
+func TestExecutor_Apply(t *testing.T) {
+	pbRepo, repoPath, clean := testhelper.InitBareRepo(t)
+	defer clean()
+
+	repo := git.NewRepository(pbRepo)
+	executor := New(filepath.Join(config.Config.BinDir, "gitaly-git2go"), command.GitPath())
+
+	ctx, cancel := testhelper.Context()
+	defer cancel()
+
+	oidBase, err := repo.WriteBlob(ctx, "file", strings.NewReader("base"))
+	require.NoError(t, err)
+
+	oidA, err := repo.WriteBlob(ctx, "file", strings.NewReader("a"))
+	require.NoError(t, err)
+
+	oidB, err := repo.WriteBlob(ctx, "file", strings.NewReader("b"))
+	require.NoError(t, err)
+
+	author := NewSignature("Test Author", "test.author@example.com", time.Now())
+	committer := NewSignature("Test Committer", "test.committer@example.com", time.Now())
+
+	parentCommitSHA, err := executor.Commit(ctx, CommitParams{
+		Repository: repoPath,
+		Author:     author,
+		Committer:  committer,
+		Message:    "base commit",
+		Actions:    []Action{CreateFile{Path: "file", OID: oidBase}},
+	})
+	require.NoError(t, err)
+
+	noCommonAncestor, err := executor.Commit(ctx, CommitParams{
+		Repository: repoPath,
+		Author:     author,
+		Committer:  committer,
+		Message:    "commit with ab",
+		Actions:    []Action{CreateFile{Path: "file", OID: oidA}},
+	})
+	require.NoError(t, err)
+
+	updateToA, err := executor.Commit(ctx, CommitParams{
+		Repository: repoPath,
+		Author:     author,
+		Committer:  committer,
+		Message:    "commit with a",
+		Parent:     parentCommitSHA,
+		Actions:    []Action{UpdateFile{Path: "file", OID: oidA}},
+	})
+	require.NoError(t, err)
+
+	updateToB, err := executor.Commit(ctx, CommitParams{
+		Repository: repoPath,
+		Author:     author,
+		Committer:  committer,
+		Message:    "commit to b",
+		Parent:     parentCommitSHA,
+		Actions:    []Action{UpdateFile{Path: "file", OID: oidB}},
+	})
+	require.NoError(t, err)
+
+	updateFromAToB, err := executor.Commit(ctx, CommitParams{
+		Repository: repoPath,
+		Author:     author,
+		Committer:  committer,
+		Message:    "commit a -> b",
+		Parent:     updateToA,
+		Actions:    []Action{UpdateFile{Path: "file", OID: oidB}},
+	})
+	require.NoError(t, err)
+
+	otherFile, err := executor.Commit(ctx, CommitParams{
+		Repository: repoPath,
+		Author:     author,
+		Committer:  committer,
+		Message:    "commit with other-file",
+		Actions:    []Action{CreateFile{Path: "other-file", OID: oidA}},
+	})
+	require.NoError(t, err)
+
+	diffBetween := func(t testing.TB, fromCommit, toCommit string) []byte {
+		t.Helper()
+		return testhelper.MustRunCommand(t, nil,
+			"git", "-C", repoPath, "format-patch", "--stdout", fromCommit+".."+toCommit)
+	}
+
+	for _, tc := range []struct {
+		desc         string
+		patches      []Patch
+		parentCommit string
+		tree         []testhelper.TreeEntry
+		error        error
+	}{
+		{
+			desc: "patch applies cleanly",
+			patches: []Patch{
+				{
+					Author:  author,
+					Message: "test commit message",
+					Diff:    diffBetween(t, parentCommitSHA, updateToA),
+				},
+			},
+			parentCommit: parentCommitSHA,
+			tree: []testhelper.TreeEntry{
+				{Path: "file", Mode: "100644", Content: "a"},
+			},
+		},
+		{
+			desc: "multiple patches apply cleanly",
+			patches: []Patch{
+				{
+					Author:  author,
+					Message: "commit with a",
+					Diff:    diffBetween(t, parentCommitSHA, updateToA),
+				},
+				{
+					Author:  author,
+					Message: "commit with a -> b",
+					Diff:    diffBetween(t, updateToA, updateFromAToB),
+				},
+			},
+			parentCommit: updateToA,
+			tree: []testhelper.TreeEntry{
+				{Path: "file", Mode: "100644", Content: "b"},
+			},
+		},
+		{
+			desc: "three way merged",
+			patches: []Patch{
+				{
+					Author:  author,
+					Message: "three-way merged files",
+					Diff:    diffBetween(t, parentCommitSHA, otherFile),
+				},
+			},
+			parentCommit: parentCommitSHA,
+			tree: []testhelper.TreeEntry{
+				{Path: "file", Mode: "100644", Content: "base"},
+				{Path: "other-file", Mode: "100644", Content: "a"},
+			},
+		},
+		{
+			// This test asserts incorrect behavior due to a bug in libgit2's
+			// git_apply_to_tree function. The correct behavior would be to
+			// return an error about merge conflict but we currently concatenate
+			// the blobs of the two trees together. This test will fail once the
+			// issue is fixed.
+			//
+			// See: https://gitlab.com/gitlab-org/gitaly/-/issues/3325
+			desc: "no common ancestor",
+			patches: []Patch{
+				{
+					Author:  author,
+					Message: "three-way merged file",
+					Diff:    diffBetween(t, parentCommitSHA, noCommonAncestor),
+				},
+			},
+			parentCommit: parentCommitSHA,
+			// error: ErrMergeConflict, <- correct output
+			tree: []testhelper.TreeEntry{
+				{Path: "file", Mode: "100644", Content: "abase"},
+			},
+		},
+		{
+			desc: "merge conflict",
+			patches: []Patch{
+				{
+					Author:  author,
+					Message: "applies cleanly",
+					Diff:    diffBetween(t, parentCommitSHA, updateToA),
+				},
+				{
+					Author:  author,
+					Message: "conflicts",
+					Diff:    diffBetween(t, parentCommitSHA, updateToB),
+				},
+			},
+			error: ErrMergeConflict,
+		},
+	} {
+		t.Run(tc.desc, func(t *testing.T) {
+			commitID, err := executor.Apply(ctx, ApplyParams{
+				Repository:   repoPath,
+				Committer:    committer,
+				ParentCommit: parentCommitSHA,
+				Patches:      NewSlicePatchIterator(tc.patches),
+			})
+			if tc.error != nil {
+				require.True(t, errors.Is(err, tc.error), err)
+				return
+			}
+
+			require.Equal(t, commit{
+				Parent:    tc.parentCommit,
+				Author:    author,
+				Committer: committer,
+				Message:   tc.patches[len(tc.patches)-1].Message,
+			}, getCommit(t, ctx, repo, commitID))
+			testhelper.RequireTree(t, repoPath, commitID, tc.tree)
+		})
+	}
+}
diff --git a/internal/git2go/commit.go b/internal/git2go/commit.go
index ee24862d9..2a9478609 100644
--- a/internal/git2go/commit.go
+++ b/internal/git2go/commit.go
@@ -39,6 +39,8 @@ type CommitParams struct {
 	Repository string
 	// Author is the author of the commit.
 	Author Signature
+	// Committer is the committer of the commit.
+	Committer Signature
 	// Message is message of the commit.
 	Message string
 	// Parent is the OID of the commit to use as the parent of this commit.
diff --git a/internal/git2go/commit_test.go b/internal/git2go/commit_test.go
index 218c5b51d..0fbe24bd7 100644
--- a/internal/git2go/commit_test.go
+++ b/internal/git2go/commit_test.go
@@ -13,6 +13,7 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/internal/command"
 	"gitlab.com/gitlab-org/gitaly/internal/git"
 	"gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
@@ -63,7 +64,7 @@ func TestExecutor_Commit(t *testing.T) {
 	updatedFile, err := repo.WriteBlob(ctx, "file", bytes.NewBufferString("updated"))
 	require.NoError(t, err)
 
-	executor := New(filepath.Join(config.Config.BinDir, "gitaly-git2go"))
+	executor := New(filepath.Join(config.Config.BinDir, "gitaly-git2go"), command.GitPath())
 
 	for _, tc := range []struct {
 		desc  string
@@ -461,12 +462,14 @@ func TestExecutor_Commit(t *testing.T) {
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
 			author := NewSignature("Author Name", "author.email@example.com", time.Now())
+			committer := NewSignature("Committer Name", "committer.email@example.com", time.Now())
 			var parentCommit string
 			for i, step := range tc.steps {
 				message := fmt.Sprintf("commit %d", i+1)
 				commitID, err := executor.Commit(ctx, CommitParams{
 					Repository: repoPath,
 					Author:     author,
+					Committer:  committer,
 					Message:    message,
 					Parent:     parentCommit,
 					Actions:    step.actions,
@@ -482,7 +485,7 @@ func TestExecutor_Commit(t *testing.T) {
 				require.Equal(t, commit{
 					Parent:    parentCommit,
 					Author:    author,
-					Committer: author,
+					Committer: committer,
 					Message:   message,
 				}, getCommit(t, ctx, repo, commitID))
 
diff --git a/internal/git2go/executor.go b/internal/git2go/executor.go
index 0007ab7e5..65058d2d2 100644
--- a/internal/git2go/executor.go
+++ b/internal/git2go/executor.go
@@ -2,10 +2,14 @@ package git2go
 
 // Executor executes gitaly-git2go.
 type Executor struct {
-	binaryPath string
+	binaryPath    string
+	gitBinaryPath string
 }
 
 // New returns a new gitaly-git2go executor using the provided binary.
-func New(binaryPath string) Executor {
-	return Executor{binaryPath: binaryPath}
+func New(binaryPath, gitBinaryPath string) Executor {
+	return Executor{
+		binaryPath:    binaryPath,
+		gitBinaryPath: gitBinaryPath,
+	}
 }