operations: Implement merge using git-merge-tree(1)merge-using-merge-tree

git-merge-tree(1) is a recent Git command that was introduced to do
in-memory merges without a worktree. Implement merge() with
git-merge-tree(1)

Co-authored-by: John Cai <jcai@gitlab.com>

2 files changed, 139 insertions, 22 deletions

diff --git a/internal/gitaly/service/operations/merge.go b/internal/gitaly/service/operations/merge.go
index 6eef9c304..5047b16b3 100644
--- a/internal/gitaly/service/operations/merge.go
+++ b/internal/gitaly/service/operations/merge.go
@@ -1,6 +1,7 @@
 package operations
 
 import (
+	"bytes"
 	"context"
 	"errors"
 	"fmt"
@@ -9,12 +10,14 @@ import (
 
 	"github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus"
 	"github.com/sirupsen/logrus"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/command"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git/updateref"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git2go"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/hook"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/helper"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/helper/text"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb"
 )
@@ -47,6 +50,58 @@ func validateMergeBranchRequest(request *gitalypb.UserMergeBranchRequest) error
 	return nil
 }
 
+func createCommitFromTree(
+	ctx context.Context,
+	quarantineRepo *localrepo.Repo,
+	resultTree []byte,
+	authorName string,
+	authorMail string,
+	authorDate time.Time,
+	message string,
+	ours string,
+	theirs string,
+) (string, error) {
+	commitEnv := []string{
+		"GIT_COMMITTER_NAME=" + authorName,
+		"GIT_COMMITTER_EMAIL=" + authorMail,
+		"GIT_COMMITTER_DATE=" + authorDate.String(),
+		"GIT_AUTHOR_NAME=" + authorName,
+		"GIT_AUTHOR_EMAIL=" + authorMail,
+		"GIT_AUTHOR_DATE=" + authorDate.String(),
+	}
+
+	flags := []git.Option{
+		git.ValueFlag{
+			Name:  "-m",
+			Value: message,
+		},
+		git.ValueFlag{
+			Name:  "-p",
+			Value: ours,
+		},
+		git.ValueFlag{
+			Name:  "-p",
+			Value: theirs,
+		},
+	}
+
+	var stdout, stderr bytes.Buffer
+	if err := quarantineRepo.ExecAndWait(ctx, git.SubCmd{
+		Name:  "commit-tree",
+		Flags: flags,
+		Args: []string{
+			string(resultTree),
+		},
+	}, git.WithStdout(&stdout), git.WithStderr(&stderr), git.WithEnv(commitEnv...)); err != nil {
+		return "", fmt.Errorf("creating commit: %w", gitError{
+			Err:    err,
+			ErrMsg: stderr.String(),
+		})
+	}
+
+	return text.ChompBytes(stdout.Bytes()), nil
+}
+
 func (s *Server) merge(
 	ctx context.Context,
 	repoPath string,
@@ -58,7 +113,44 @@ func (s *Server) merge(
 	ours string,
 	theirs string,
 ) (string, error) {
-	return "", helper.ErrInternalf("UserMergeBranch using Git is not yet implemented")
+	var mergeTreeStderr bytes.Buffer
+	var mergeTreeStdout bytes.Buffer
+	cmdMergeTree, err := s.gitCmdFactory.New(ctx, quarantineRepo,
+		git.SubCmd{
+			Name: "merge-tree",
+			Flags: []git.Option{
+				git.Flag{Name: "--write-tree"},
+				git.Flag{Name: "--name-only"},
+			},
+			Args: []string{ours, theirs},
+		},
+		git.WithStderr(&mergeTreeStderr),
+		git.WithStdout(&mergeTreeStdout),
+	)
+	if err != nil {
+		return "", fmt.Errorf("merge-tree: %w", gitError{ErrMsg: mergeTreeStderr.String(), Err: err})
+	}
+
+	cmdErr := cmdMergeTree.Wait()
+	if exitCode, success := command.ExitStatus(cmdErr); success && exitCode > 1 {
+		return "", fmt.Errorf("merge-tree: %w", gitError{ErrMsg: mergeTreeStderr.String(), Err: err})
+	}
+
+	mergeTreeResult, err := git.NewMergeTreeResult(mergeTreeStdout.String())
+	if err != nil {
+		return "", err
+	}
+
+	var commit string
+	if commit, err = createCommitFromTree(
+		ctx, quarantineRepo, []byte(mergeTreeResult.TreeOid),
+		authorName, authorMail, authorDate,
+		message, ours, theirs,
+	); err != nil {
+		return "", fmt.Errorf("create commit from tree: %w", err)
+	}
+
+	return commit, nil
 }
 
 func (s *Server) mergeWithGit2Go(
@@ -82,6 +174,13 @@ func (s *Server) mergeWithGit2Go(
 		Theirs:     theirs,
 	})
 	if err != nil {
+		var conflictErr git2go.ConflictingFilesError
+		if errors.As(err, &conflictErr) {
+			return "", &git.ConflictingFilesError{
+				ConflictingFiles: conflictErr.ConflictingFiles,
+			}
+		}
+
 		if errors.Is(err, git2go.ErrInvalidArgument) {
 			return "", helper.ErrInvalidArgument(err)
 		}
@@ -130,7 +229,12 @@ func (s *Server) UserMergeBranch(stream gitalypb.OperationService_UserMergeBranc
 	var mergeCommitID string
 	var mergeErr error
 
-	if featureflag.MergeTreeMerge.IsEnabled(ctx) {
+	version, err := s.gitCmdFactory.GitVersion(ctx)
+	if err != nil {
+		return helper.ErrInternalf("getting git version: %w", err)
+	}
+
+	if featureflag.MergeTreeMerge.IsEnabled(ctx) && version.IsMergeTreeSupported() {
 		mergeCommitID, mergeErr = s.merge(ctx, repoPath, quarantineRepo,
 			string(firstRequest.User.Name),
 			string(firstRequest.User.Email),
@@ -149,7 +253,7 @@ func (s *Server) UserMergeBranch(stream gitalypb.OperationService_UserMergeBranc
 	}
 
 	if mergeErr != nil {
-		var conflictErr git2go.ConflictingFilesError
+		var conflictErr *git.ConflictingFilesError
 		if errors.As(mergeErr, &conflictErr) {
 			conflictingFiles := make([][]byte, 0, len(conflictErr.ConflictingFiles))
 			for _, conflictingFile := range conflictErr.ConflictingFiles {
@@ -157,7 +261,7 @@ func (s *Server) UserMergeBranch(stream gitalypb.OperationService_UserMergeBranc
 			}
 
 			detailedErr, err := helper.ErrWithDetails(
-				helper.ErrFailedPreconditionf("merging commits: %w", err),
+				helper.ErrFailedPreconditionf("merging commits: %w", mergeErr),
 				&gitalypb.UserMergeBranchError{
 					Error: &gitalypb.UserMergeBranchError_MergeConflict{
 						MergeConflict: &gitalypb.MergeConflictError{
diff --git a/internal/gitaly/service/operations/merge_test.go b/internal/gitaly/service/operations/merge_test.go
index 0e835f555..035c14287 100644
--- a/internal/gitaly/service/operations/merge_test.go
+++ b/internal/gitaly/service/operations/merge_test.go
@@ -43,7 +43,7 @@ var (
 
 func TestUserMergeBranch_successful(t *testing.T) {
 	t.Parallel()
-	testhelper.NewFeatureSets(featureflag.MergeTreeMerge).Run(
+	testhelper.NewFeatureSets(featureflag.MergeTreeMerge, featureflag.GitV238).Run(
 		t,
 		testUserMergeBranchSuccessful,
 	)
@@ -231,7 +231,7 @@ func TestUserMergeBranch_failure(t *testing.T) {
 
 func TestUserMergeBranch_quarantine(t *testing.T) {
 	t.Parallel()
-	testhelper.NewFeatureSets(featureflag.MergeTreeMerge).Run(
+	testhelper.NewFeatureSets(featureflag.MergeTreeMerge, featureflag.GitV238).Run(
 		t,
 		testUserMergeBranchQuarantine,
 	)
@@ -293,7 +293,7 @@ func testUserMergeBranchQuarantine(t *testing.T, ctx context.Context) {
 
 func TestUserMergeBranch_stableMergeIDs(t *testing.T) {
 	t.Parallel()
-	testhelper.NewFeatureSets(featureflag.MergeTreeMerge).Run(
+	testhelper.NewFeatureSets(featureflag.MergeTreeMerge, featureflag.GitV238).Run(
 		t,
 		testUserMergeBranchStableMergeIDs,
 	)
@@ -367,7 +367,7 @@ func testUserMergeBranchStableMergeIDs(t *testing.T, ctx context.Context) {
 
 func TestUserMergeBranch_abort(t *testing.T) {
 	t.Parallel()
-	testhelper.NewFeatureSets(featureflag.MergeTreeMerge).Run(
+	testhelper.NewFeatureSets(featureflag.MergeTreeMerge, featureflag.GitV238).Run(
 		t,
 		testUserMergeBranchAbort,
 	)
@@ -437,7 +437,7 @@ func testUserMergeBranchAbort(t *testing.T, ctx context.Context) {
 
 func TestUserMergeBranch_concurrentUpdate(t *testing.T) {
 	t.Parallel()
-	testhelper.NewFeatureSets(featureflag.MergeTreeMerge).Run(
+	testhelper.NewFeatureSets(featureflag.MergeTreeMerge, featureflag.GitV238).Run(
 		t,
 		testUserMergeBranchConcurrentUpdate,
 	)
@@ -499,7 +499,7 @@ func testUserMergeBranchConcurrentUpdate(t *testing.T, ctx context.Context) {
 
 func TestUserMergeBranch_ambiguousReference(t *testing.T) {
 	t.Parallel()
-	testhelper.NewFeatureSets(featureflag.MergeTreeMerge).Run(
+	testhelper.NewFeatureSets(featureflag.MergeTreeMerge, featureflag.GitV238).Run(
 		t,
 		testUserMergeBranchAmbiguousReference,
 	)
@@ -567,7 +567,7 @@ func testUserMergeBranchAmbiguousReference(t *testing.T, ctx context.Context) {
 
 func TestUserMergeBranch_failingHooks(t *testing.T) {
 	t.Parallel()
-	testhelper.NewFeatureSets(featureflag.MergeTreeMerge).Run(
+	testhelper.NewFeatureSets(featureflag.MergeTreeMerge, featureflag.GitV238).Run(
 		t,
 		testUserMergeBranchFailingHooks,
 	)
@@ -664,7 +664,7 @@ func testUserMergeBranchFailingHooks(t *testing.T, ctx context.Context) {
 
 func TestUserMergeBranch_conflict(t *testing.T) {
 	t.Parallel()
-	testhelper.NewFeatureSets(featureflag.MergeTreeMerge).Run(
+	testhelper.NewFeatureSets(featureflag.MergeTreeMerge, featureflag.GitV238).Run(
 		t,
 		testUserMergeBranchConflict,
 	)
@@ -675,23 +675,36 @@ func testUserMergeBranchConflict(t *testing.T, ctx context.Context) {
 
 	ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
 
+	const baseBranch = "base"
 	const mergeIntoBranch = "mergeIntoBranch"
 	const mergeFromBranch = "mergeFromBranch"
 	const conflictingFile = "file"
 
-	baseCommit := gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch(mergeIntoBranch), gittest.WithTreeEntries(gittest.TreeEntry{
+	baseCommit := gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch(baseBranch), gittest.WithTreeEntries(gittest.TreeEntry{
 		Mode: "100644", Path: conflictingFile, Content: "data",
 	}))
 
-	gittest.Exec(t, cfg, "-C", repoPath, "branch", mergeFromBranch, baseCommit.String())
-
-	divergedInto := gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch(mergeIntoBranch), gittest.WithTreeEntries(gittest.TreeEntry{
-		Mode: "100644", Path: conflictingFile, Content: "data-1",
-	}))
+	divergedInto := gittest.WriteCommit(
+		t,
+		cfg,
+		repoPath,
+		gittest.WithBranch(mergeIntoBranch),
+		gittest.WithParents(baseCommit),
+		gittest.WithTreeEntries(gittest.TreeEntry{
+			Mode: "100644", Path: conflictingFile, Content: "data-1",
+		}),
+	)
 
-	divergedFrom := gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch(mergeFromBranch), gittest.WithTreeEntries(gittest.TreeEntry{
-		Mode: "100644", Path: conflictingFile, Content: "data-2",
-	}))
+	divergedFrom := gittest.WriteCommit(
+		t,
+		cfg,
+		repoPath,
+		gittest.WithBranch(mergeFromBranch),
+		gittest.WithParents(baseCommit),
+		gittest.WithTreeEntries(gittest.TreeEntry{
+			Mode: "100644", Path: conflictingFile, Content: "data-2",
+		}),
+	)
 
 	mergeBidi, err := client.UserMergeBranch(ctx)
 	require.NoError(t, err)
@@ -726,7 +739,7 @@ func testUserMergeBranchConflict(t *testing.T, ctx context.Context) {
 
 func TestUserMergeBranch_allowed(t *testing.T) {
 	t.Parallel()
-	testhelper.NewFeatureSets(featureflag.MergeTreeMerge).Run(
+	testhelper.NewFeatureSets(featureflag.MergeTreeMerge, featureflag.GitV238).Run(
 		t,
 		testUserMergeBranchAllowed,
 	)