gitaly-git2go: Drop "apply" subcommand

The "apply" subcommand has never really been used in production due to a
bug in libgit2 that causes certain patches to misapply. As we have never
been able to get the fix upstreamed, we were barred from using libgit2
here and instead still continue to use a worktree and git-apply(1). As
we have essentially deprecated libgit2, this is also not going to change
either.

We still had a dependency on the "apply" subcommand in our tests for
RawDiff and RawPatch. These have been refactored now though, removing
the last usage of the command. So let's drop the implementation of the
"apply" subcommand altogether.

2 files changed, 0 insertions, 354 deletions

diff --git a/internal/git2go/apply.go b/internal/git2go/apply.go
deleted file mode 100644
index b61351eba..000000000
--- a/internal/git2go/apply.go
+++ /dev/null
@@ -1,132 +0,0 @@
-package git2go
-
-import (
-	"context"
-	"encoding/gob"
-	"fmt"
-	"io"
-
-	"gitlab.com/gitlab-org/gitaly/v16/internal/git"
-	"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage"
-)
-
-// 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, repo storage.Repository, params ApplyParams) (git.ObjectID, error) {
-	reader, writer := io.Pipe()
-	defer writer.Close()
-
-	go func() {
-		// CloseWithError is documented to always return nil.
-		_ = 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
-		}())
-	}()
-
-	execEnv := b.gitCmdFactory.GetExecutionEnvironment(ctx)
-
-	args := []string{"-git-binary-path", execEnv.BinaryPath}
-	if b.signingKey != "" {
-		args = append(args, "-signing-key", b.signingKey)
-	}
-
-	var result Result
-	output, err := b.run(ctx, repo, reader, "apply", args...)
-	if err != nil {
-		return "", fmt.Errorf("run: %w", err)
-	}
-
-	if err := gob.NewDecoder(output).Decode(&result); err != nil {
-		return "", fmt.Errorf("decode: %w", err)
-	}
-
-	if result.Err != nil {
-		return "", result.Err
-	}
-
-	commitID, err := git.ObjectHashSHA1.FromHex(result.CommitID)
-	if err != nil {
-		return "", fmt.Errorf("could not parse commit ID: %w", err)
-	}
-
-	return commitID, nil
-}
diff --git a/internal/git2go/apply_test.go b/internal/git2go/apply_test.go
deleted file mode 100644
index 338761201..000000000
--- a/internal/git2go/apply_test.go
+++ /dev/null
@@ -1,222 +0,0 @@
-//go:build !gitaly_test_sha256
-
-package git2go
-
-import (
-	"errors"
-	"strings"
-	"testing"
-
-	"github.com/stretchr/testify/require"
-	"gitlab.com/gitlab-org/gitaly/v16/internal/git"
-	"gitlab.com/gitlab-org/gitaly/v16/internal/git/gittest"
-	"gitlab.com/gitlab-org/gitaly/v16/internal/git/localrepo"
-	"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/config"
-	"gitlab.com/gitlab-org/gitaly/v16/internal/testhelper"
-	"gitlab.com/gitlab-org/gitaly/v16/internal/testhelper/testcfg"
-)
-
-func TestExecutor_Apply(t *testing.T) {
-	ctx := testhelper.Context(t)
-	cfg := testcfg.Build(t)
-	testcfg.BuildGitalyGit2Go(t, cfg)
-
-	repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
-		SkipCreationViaService: true,
-	})
-
-	repo := localrepo.NewTestRepo(t, cfg, repoProto)
-	executor := NewExecutor(cfg, gittest.NewCommandFactory(t, cfg), config.NewLocator(cfg))
-
-	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 := defaultCommitAuthorSignature()
-	committer := defaultCommitAuthorSignature()
-
-	parentCommitSHA, err := executor.Commit(ctx, repo, CommitCommand{
-		Repository: repoPath,
-		Author:     author,
-		Committer:  committer,
-		Message:    "base commit",
-		Actions:    []Action{CreateFile{Path: "file", OID: oidBase.String()}},
-	})
-	require.NoError(t, err)
-
-	noCommonAncestor, err := executor.Commit(ctx, repo, CommitCommand{
-		Repository: repoPath,
-		Author:     author,
-		Committer:  committer,
-		Message:    "commit with ab",
-		Actions:    []Action{CreateFile{Path: "file", OID: oidA.String()}},
-	})
-	require.NoError(t, err)
-
-	updateToA, err := executor.Commit(ctx, repo, CommitCommand{
-		Repository: repoPath,
-		Author:     author,
-		Committer:  committer,
-		Message:    "commit with a",
-		Parent:     parentCommitSHA.String(),
-		Actions:    []Action{UpdateFile{Path: "file", OID: oidA.String()}},
-	})
-	require.NoError(t, err)
-
-	updateToB, err := executor.Commit(ctx, repo, CommitCommand{
-		Repository: repoPath,
-		Author:     author,
-		Committer:  committer,
-		Message:    "commit to b",
-		Parent:     parentCommitSHA.String(),
-		Actions:    []Action{UpdateFile{Path: "file", OID: oidB.String()}},
-	})
-	require.NoError(t, err)
-
-	updateFromAToB, err := executor.Commit(ctx, repo, CommitCommand{
-		Repository: repoPath,
-		Author:     author,
-		Committer:  committer,
-		Message:    "commit a -> b",
-		Parent:     updateToA.String(),
-		Actions:    []Action{UpdateFile{Path: "file", OID: oidB.String()}},
-	})
-	require.NoError(t, err)
-
-	otherFile, err := executor.Commit(ctx, repo, CommitCommand{
-		Repository: repoPath,
-		Author:     author,
-		Committer:  committer,
-		Message:    "commit with other-file",
-		Actions:    []Action{CreateFile{Path: "other-file", OID: oidA.String()}},
-	})
-	require.NoError(t, err)
-
-	diffBetween := func(tb testing.TB, fromCommit, toCommit git.ObjectID) []byte {
-		tb.Helper()
-		return gittest.Exec(tb, cfg, "-C", repoPath, "format-patch", "--stdout", fromCommit.String()+".."+toCommit.String())
-	}
-
-	for _, tc := range []struct {
-		desc         string
-		patches      []Patch
-		parentCommit git.ObjectID
-		tree         []gittest.TreeEntry
-		error        error
-	}{
-		{
-			desc: "patch applies cleanly",
-			patches: []Patch{
-				{
-					Author:  author,
-					Message: "test commit message",
-					Diff:    diffBetween(t, parentCommitSHA, updateToA),
-				},
-			},
-			parentCommit: parentCommitSHA,
-			tree: []gittest.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: []gittest.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: []gittest.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: []gittest.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, repo, ApplyParams{
-				Repository:   repoPath,
-				Committer:    committer,
-				ParentCommit: parentCommitSHA.String(),
-				Patches:      NewSlicePatchIterator(tc.patches),
-			})
-			if tc.error != nil {
-				require.True(t, errors.Is(err, tc.error), err)
-				return
-			}
-
-			commit, err := repo.ReadCommit(ctx, commitID.Revision())
-			require.NoError(t, err)
-
-			require.Equal(t, []string{string(tc.parentCommit)}, commit.ParentIds)
-			require.Equal(t, gittest.DefaultCommitAuthor, commit.Author)
-			require.Equal(t, gittest.DefaultCommitAuthor, commit.Committer)
-			require.Equal(t, []byte(tc.patches[len(tc.patches)-1].Message), commit.Body)
-
-			gittest.RequireTree(t, cfg, repoPath, commitID.String(), tc.tree)
-		})
-	}
-}