Port UserApplyPatch to Go

This commit adds a Go port of UserApplyPatch.

UserApplyPatch is an RPC that parses a stream of patches in unix
mailbox format, applies them sequentially on top of the base commit
and sets the target branch to point to the resulting commit.

The original Ruby implementation basically just streamed the patches
to `git am`. The Go port uses the same approach although returns more
descriptive errors when the command fails.

The RPC does not support setting a base branch. The Go implementation
follows the same default branch discovery for determining the base
commit as the Ruby implementation. Notably, neither support creating
the first branch in the repository.

This was initially implemented using libgit2. During the implementation,
a bug was discovered in the patching routine. As the fix wasn't merged
upstream in a long time, we've implemented this using a worktree for now
to unblock the efforts to remove the Ruby sidecar.

4 files changed, 192 insertions, 30 deletions

diff --git a/internal/gitaly/service/operations/apply_patch.go b/internal/gitaly/service/operations/apply_patch.go
index 92b1db943..4f1551594 100644
--- a/internal/gitaly/service/operations/apply_patch.go
+++ b/internal/gitaly/service/operations/apply_patch.go
@@ -1,14 +1,28 @@
 package operations
 
 import (
+	"bytes"
+	"context"
+	"errors"
 	"fmt"
+	"path/filepath"
+	"time"
 
+	"github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/rubyserver"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/service/ref"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/helper"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/helper/text"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
+	"gitlab.com/gitlab-org/gitaly/v14/streamio"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
 )
 
+var errNoDefaultBranch = errors.New("no default branch")
+
 func (s *Server) UserApplyPatch(stream gitalypb.OperationService_UserApplyPatchServer) error {
 	firstRequest, err := stream.Recv()
 	if err != nil {
@@ -25,6 +39,22 @@ func (s *Server) UserApplyPatch(stream gitalypb.OperationService_UserApplyPatchS
 	}
 
 	requestCtx := stream.Context()
+
+	if featureflag.GoUserApplyPatch.IsEnabled(requestCtx) {
+		if err := s.userApplyPatch(requestCtx, header, stream); err != nil {
+			if errors.Is(err, errNoDefaultBranch) {
+				// This is here to match the behavior of the original Ruby implementation which failed with the error
+				// when attempting to apply a patch to a repository that had no branches. Once the Ruby port has been
+				// removed, we could return a more descriptive error or support creating the first branch in the repository.
+				return status.Error(codes.Unknown, "TypeError: no implicit conversion of nil into String")
+			}
+
+			return helper.ErrInternal(err)
+		}
+
+		return nil
+	}
+
 	rubyClient, err := s.ruby.OperationServiceClient(requestCtx)
 	if err != nil {
 		return err
@@ -63,6 +93,145 @@ func (s *Server) UserApplyPatch(stream gitalypb.OperationService_UserApplyPatchS
 	return stream.SendAndClose(response)
 }
 
+func (s *Server) userApplyPatch(ctx context.Context, header *gitalypb.UserApplyPatchRequest_Header, stream gitalypb.OperationService_UserApplyPatchServer) error {
+	path, err := s.locator.GetRepoPath(header.Repository)
+	if err != nil {
+		return err
+	}
+
+	branchCreated := false
+	targetBranch := git.NewReferenceNameFromBranchName(string(header.TargetBranch))
+
+	repo := s.localrepo(header.Repository)
+	parentCommitID, err := repo.ResolveRevision(ctx, targetBranch.Revision()+"^{commit}")
+	if err != nil {
+		if !errors.Is(err, git.ErrReferenceNotFound) {
+			return fmt.Errorf("resolve target branch: %w", err)
+		}
+
+		defaultBranch, err := ref.DefaultBranchName(ctx, repo)
+		if err != nil {
+			return fmt.Errorf("default branch name: %w", err)
+		} else if defaultBranch == nil {
+			return errNoDefaultBranch
+		}
+
+		branchCreated = true
+		parentCommitID, err = repo.ResolveRevision(ctx, git.Revision(defaultBranch)+"^{commit}")
+		if err != nil {
+			return fmt.Errorf("resolve default branch commit: %w", err)
+		}
+	}
+
+	// This should be changed to consider timezones after the Ruby implementation is removed.
+	// https://gitlab.com/gitlab-org/gitaly/-/issues/3711
+	committerTime := time.Now()
+	if header.Timestamp != nil {
+		committerTime = header.Timestamp.AsTime()
+	}
+
+	worktreePath := newWorktreePath(path, "am-")
+	if err := s.addWorktree(ctx, header.Repository, worktreePath, parentCommitID.String()); err != nil {
+		return fmt.Errorf("add worktree: %w", err)
+	}
+
+	defer func() {
+		ctx, cancel := context.WithTimeout(helper.SuppressCancellation(ctx), 30*time.Second)
+		defer cancel()
+
+		worktreeName := filepath.Base(worktreePath)
+		if err := s.removeWorktree(ctx, header.Repository, worktreeName); err != nil {
+			ctxlogrus.Extract(ctx).WithField("worktree_name", worktreeName).WithError(err).Error("failed to remove worktree")
+		}
+	}()
+
+	var stdout, stderr bytes.Buffer
+	cmd, err := s.gitCmdFactory.NewWithDir(ctx, worktreePath,
+		git.SubCmd{
+			Name: "am",
+			Flags: []git.Option{
+				git.Flag{Name: "--quiet"},
+				git.Flag{Name: "--3way"},
+			},
+		},
+		git.WithEnv(
+			"GIT_COMMITTER_NAME="+string(header.GetUser().Name),
+			"GIT_COMMITTER_EMAIL="+string(header.GetUser().Email),
+			fmt.Sprintf("GIT_COMMITTER_DATE=%d %s", committerTime.Unix(), committerTime.Format("-0700")),
+		),
+		git.WithStdin(streamio.NewReader(func() ([]byte, error) {
+			req, err := stream.Recv()
+			return req.GetPatches(), err
+		})),
+		git.WithStdout(&stdout),
+		git.WithStderr(&stderr),
+		git.WithRefTxHook(ctx, header.Repository, s.cfg),
+	)
+
+	if err != nil {
+		return fmt.Errorf("create git am: %w", err)
+	}
+
+	if err := cmd.Wait(); err != nil {
+		// The Ruby implementation doesn't include stderr in errors, which makes
+		// it difficult to determine the cause of an error. This special cases the
+		// user facing patching error which is returned usually to maintain test
+		// compatibility but returns the error and stderr otherwise. Once the Ruby
+		// implementation is removed, this should probably be dropped.
+		if bytes.HasPrefix(stdout.Bytes(), []byte("Patch failed at")) {
+			return status.Error(codes.FailedPrecondition, stdout.String())
+		}
+
+		return fmt.Errorf("apply patch: %w, stderr: %q", err, &stderr)
+	}
+
+	var revParseStdout, revParseStderr bytes.Buffer
+	revParseCmd, err := s.gitCmdFactory.NewWithDir(ctx, worktreePath,
+		git.SubCmd{
+			Name: "rev-parse",
+			Flags: []git.Option{
+				git.Flag{Name: "--quiet"},
+				git.Flag{Name: "--verify"},
+			},
+			Args: []string{"HEAD^{commit}"},
+		},
+		git.WithStdout(&revParseStdout),
+		git.WithStderr(&revParseStderr),
+	)
+	if err != nil {
+		return fmt.Errorf("create git rev-parse: %w", gitError{ErrMsg: revParseStderr.String(), Err: err})
+	}
+
+	if err := revParseCmd.Wait(); err != nil {
+		return fmt.Errorf("get patched commit: %w", gitError{ErrMsg: revParseStderr.String(), Err: err})
+	}
+
+	patchedCommit, err := git.NewObjectIDFromHex(text.ChompBytes(revParseStdout.Bytes()))
+	if err != nil {
+		return fmt.Errorf("parse patched commit oid: %w", err)
+	}
+
+	currentCommit := parentCommitID
+	if branchCreated {
+		currentCommit = git.ZeroOID
+	}
+
+	if err := s.updateReferenceWithHooks(ctx, header.Repository, header.User, nil, targetBranch, patchedCommit, currentCommit); err != nil {
+		return fmt.Errorf("update reference: %w", err)
+	}
+
+	if err := stream.SendAndClose(&gitalypb.UserApplyPatchResponse{
+		BranchUpdate: &gitalypb.OperationBranchUpdate{
+			CommitId:      patchedCommit.String(),
+			BranchCreated: branchCreated,
+		},
+	}); err != nil {
+		return fmt.Errorf("send and close: %w", err)
+	}
+
+	return nil
+}
+
 func validateUserApplyPatchHeader(header *gitalypb.UserApplyPatchRequest_Header) error {
 	if header.GetRepository() == nil {
 		return fmt.Errorf("missing Repository")
diff --git a/internal/gitaly/service/operations/apply_patch_test.go b/internal/gitaly/service/operations/apply_patch_test.go
index e0170cc9d..c4a4eec78 100644
--- a/internal/gitaly/service/operations/apply_patch_test.go
+++ b/internal/gitaly/service/operations/apply_patch_test.go
@@ -1,6 +1,7 @@
 package operations
 
 import (
+	"context"
 	"fmt"
 	"io"
 	"os"
@@ -9,8 +10,6 @@ import (
 	"testing/iotest"
 	"time"
 
-	"github.com/golang/protobuf/ptypes"
-	"github.com/golang/protobuf/ptypes/timestamp"
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git/catfile"
@@ -26,12 +25,10 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v14/streamio"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
+	"google.golang.org/protobuf/types/known/timestamppb"
 )
 
-func testUserApplyPatch(t *testing.T, cfg config.Cfg, rubySrv *rubyserver.Server) {
-	ctx, cancel := testhelper.Context()
-	defer cancel()
-
+func testUserApplyPatch(t *testing.T, ctx context.Context, cfg config.Cfg, rubySrv *rubyserver.Server) {
 	type actionFunc func(testing.TB, *localrepo.Repo) git2go.Action
 
 	createFile := func(filepath string, content string) actionFunc {
@@ -117,7 +114,7 @@ To restore the original branch and stop patching, run "git am --abort".
 					createFile("file", "base-content"),
 				},
 			},
-			error: status.Error(codes.Internal, "TypeError: no implicit conversion of nil into String"),
+			error: status.Error(codes.Unknown, "TypeError: no implicit conversion of nil into String"),
 		},
 		{
 			desc:          "creating a new branch from HEAD works",
@@ -294,7 +291,7 @@ To restore the original branch and stop patching, run "git am --abort".
 			}
 
 			if baseCommit != "" {
-				require.NoError(t, repo.UpdateRef(ctx, tc.baseReference, git.ObjectID(baseCommit), git.ZeroOID))
+				require.NoError(t, repo.UpdateRef(ctx, tc.baseReference, baseCommit, git.ZeroOID))
 			}
 
 			if tc.extraBranches != nil {
@@ -308,7 +305,7 @@ To restore the original branch and stop patching, run "git am --abort".
 
 				for _, extraBranch := range tc.extraBranches {
 					require.NoError(t, repo.UpdateRef(ctx,
-						git.NewReferenceNameFromBranchName(extraBranch), git.ObjectID(emptyCommit), git.ZeroOID),
+						git.NewReferenceNameFromBranchName(extraBranch), emptyCommit, git.ZeroOID),
 					)
 				}
 			}
@@ -403,9 +400,6 @@ To restore the original branch and stop patching, run "git am --abort".
 			actualCommit.ParentIds = nil // the parent changes with the patches, we just check it is set
 			actualCommit.TreeId = ""     // treeID is asserted via its contents below
 
-			authorTimestamp, err := ptypes.TimestampProto(authorTime)
-			require.NoError(t, err)
-
 			expectedBody := []byte("commit subject\n\ncommit message body\n")
 			expectedTimezone := []byte("+0000")
 			testassert.ProtoEqual(t,
@@ -416,7 +410,7 @@ To restore the original branch and stop patching, run "git am --abort".
 					Author: &gitalypb.CommitAuthor{
 						Name:     []byte("Test Author"),
 						Email:    []byte("author@example.com"),
-						Date:     authorTimestamp,
+						Date:     timestamppb.New(authorTime),
 						Timezone: expectedTimezone,
 					},
 					Committer: &gitalypb.CommitAuthor{
@@ -435,10 +429,7 @@ To restore the original branch and stop patching, run "git am --abort".
 	}
 }
 
-func testSuccessfulUserApplyPatch(t *testing.T, cfg config.Cfg, rubySrv *rubyserver.Server) {
-	ctx, cancel := testhelper.Context()
-	defer cancel()
-
+func testSuccessfulUserApplyPatch(t *testing.T, ctx context.Context, cfg config.Cfg, rubySrv *rubyserver.Server) {
 	ctx, cfg, repoProto, repoPath, client := setupOperationsServiceWithRuby(t, ctx, cfg, rubySrv)
 
 	repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -544,10 +535,7 @@ func testSuccessfulUserApplyPatch(t *testing.T, cfg config.Cfg, rubySrv *rubyser
 	}
 }
 
-func testUserApplyPatchStableID(t *testing.T, cfg config.Cfg, rubySrv *rubyserver.Server) {
-	ctx, cancel := testhelper.Context()
-	defer cancel()
-
+func testUserApplyPatchStableID(t *testing.T, ctx context.Context, cfg config.Cfg, rubySrv *rubyserver.Server) {
 	ctx, cfg, repoProto, _, client := setupOperationsServiceWithRuby(t, ctx, cfg, rubySrv)
 
 	repo := localrepo.NewTestRepo(t, cfg, repoProto)
@@ -561,7 +549,7 @@ func testUserApplyPatchStableID(t *testing.T, cfg config.Cfg, rubySrv *rubyserve
 				Repository:   repoProto,
 				User:         gittest.TestUser,
 				TargetBranch: []byte("branch"),
-				Timestamp:    &timestamp.Timestamp{Seconds: 1234512345},
+				Timestamp:    &timestamppb.Timestamp{Seconds: 1234512345},
 			},
 		},
 	}))
@@ -591,22 +579,19 @@ func testUserApplyPatchStableID(t *testing.T, cfg config.Cfg, rubySrv *rubyserve
 		Author: &gitalypb.CommitAuthor{
 			Name:     []byte("Patch User"),
 			Email:    []byte("patchuser@gitlab.org"),
-			Date:     &timestamp.Timestamp{Seconds: 1539862835},
+			Date:     &timestamppb.Timestamp{Seconds: 1539862835},
 			Timezone: []byte("+0200"),
 		},
 		Committer: &gitalypb.CommitAuthor{
 			Name:     gittest.TestUser.Name,
 			Email:    gittest.TestUser.Email,
-			Date:     &timestamp.Timestamp{Seconds: 1234512345},
+			Date:     &timestamppb.Timestamp{Seconds: 1234512345},
 			Timezone: []byte("+0000"),
 		},
 	}, patchedCommit)
 }
 
-func testFailedPatchApplyPatch(t *testing.T, cfg config.Cfg, rubySrv *rubyserver.Server) {
-	ctx, cancel := testhelper.Context()
-	defer cancel()
-
+func testFailedPatchApplyPatch(t *testing.T, ctx context.Context, cfg config.Cfg, rubySrv *rubyserver.Server) {
 	ctx, _, repo, _, client := setupOperationsServiceWithRuby(t, ctx, cfg, rubySrv)
 
 	testPatch := testhelper.MustReadFile(t, "testdata/0001-This-does-not-apply-to-the-feature-branch.patch")
diff --git a/internal/gitaly/service/operations/testhelper_test.go b/internal/gitaly/service/operations/testhelper_test.go
index a29418e32..0582b12d7 100644
--- a/internal/gitaly/service/operations/testhelper_test.go
+++ b/internal/gitaly/service/operations/testhelper_test.go
@@ -20,6 +20,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/service/repository"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/service/ssh"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/gitlab"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper/testcfg"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper/testserver"
@@ -59,7 +60,7 @@ func TestWithRubySidecar(t *testing.T) {
 	require.NoError(t, rubySrv.Start())
 	t.Cleanup(rubySrv.Stop)
 
-	fs := []func(t *testing.T, cfg config.Cfg, rubySrv *rubyserver.Server){
+	fs := []func(t *testing.T, ctx context.Context, cfg config.Cfg, rubySrv *rubyserver.Server){
 		testSuccessfulUserApplyPatch,
 		testUserApplyPatchStableID,
 		testFailedPatchApplyPatch,
@@ -67,7 +68,11 @@ func TestWithRubySidecar(t *testing.T) {
 	}
 	for _, f := range fs {
 		t.Run(runtime.FuncForPC(reflect.ValueOf(f).Pointer()).Name(), func(t *testing.T) {
-			f(t, cfg, rubySrv)
+			testhelper.NewFeatureSets(
+				[]featureflag.FeatureFlag{featureflag.GoUserApplyPatch},
+			).Run(t, func(t *testing.T, ctx context.Context) {
+				f(t, ctx, cfg, rubySrv)
+			})
 		})
 	}
 }
diff --git a/internal/metadata/featureflag/feature_flags.go b/internal/metadata/featureflag/feature_flags.go
index 1466b8cc8..9b2ff7dc2 100644
--- a/internal/metadata/featureflag/feature_flags.go
+++ b/internal/metadata/featureflag/feature_flags.go
@@ -30,6 +30,8 @@ var (
 	UserSquashWithoutWorktree = FeatureFlag{Name: "user_squash_without_worktree", OnByDefault: false}
 	// QuarantinedResolveCOnflicts enables use of a quarantine object directory for ResolveConflicts.
 	QuarantinedResolveConflicts = FeatureFlag{Name: "quarantined_resolve_conflicts", OnByDefault: false}
+	// GoUserApplyPatch enables the Go implementation of UserApplyPatch
+	GoUserApplyPatch = FeatureFlag{Name: "go_user_apply_patch", OnByDefault: false}
 )
 
 // All includes all feature flags.
@@ -44,4 +46,5 @@ var All = []FeatureFlag{
 	QuarantinedUserCreateTag,
 	UserSquashWithoutWorktree,
 	QuarantinedResolveConflicts,
+	GoUserApplyPatch,
 }