operations: Introduce MergeTreeMerge featureflag

This new featureflag is for using an implementation of
UserMergeBranch that uses `git merge-tree` instead of
the previous implementation based on git2go.

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

3 files changed, 112 insertions, 23 deletions

diff --git a/internal/gitaly/service/operations/merge.go b/internal/gitaly/service/operations/merge.go
index d1a1f6b8b..876a6bcf4 100644
--- a/internal/gitaly/service/operations/merge.go
+++ b/internal/gitaly/service/operations/merge.go
@@ -15,6 +15,7 @@ import (
 	"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/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb"
 )
 
@@ -46,6 +47,20 @@ func validateMergeBranchRequest(request *gitalypb.UserMergeBranchRequest) error
 	return nil
 }
 
+func (s *Server) merge(
+	ctx context.Context,
+	repoPath string,
+	quarantineRepo *localrepo.Repo,
+	authorName string,
+	authorMail string,
+	authorDate time.Time,
+	message string,
+	ours string,
+	theirs string,
+) (string, error) {
+	return "", helper.ErrInternalf("UserMergeBranch using Git is not yet implemented")
+}
+
 func (s *Server) mergeWithGit2Go(
 	ctx context.Context,
 	repoPath string,
@@ -66,7 +81,6 @@ func (s *Server) mergeWithGit2Go(
 		Ours:       ours,
 		Theirs:     theirs,
 	})
-
 	if err != nil {
 		if errors.Is(err, git2go.ErrInvalidArgument) {
 			return "", helper.ErrInvalidArgument(err)
@@ -113,16 +127,30 @@ func (s *Server) UserMergeBranch(stream gitalypb.OperationService_UserMergeBranc
 		return helper.ErrInvalidArgument(err)
 	}
 
-	commitID, err := s.mergeWithGit2Go(ctx, repoPath, quarantineRepo,
-		string(firstRequest.User.Name),
-		string(firstRequest.User.Email),
-		authorDate,
-		string(firstRequest.Message),
-		revision.String(),
-		firstRequest.CommitId)
-	if err != nil {
+	var mergeCommitID string
+	var mergeErr error
+
+	if featureflag.MergeTreeMerge.IsEnabled(ctx) {
+		mergeCommitID, mergeErr = s.merge(ctx, repoPath, quarantineRepo,
+			string(firstRequest.User.Name),
+			string(firstRequest.User.Email),
+			authorDate,
+			string(firstRequest.Message),
+			revision.String(),
+			firstRequest.CommitId)
+	} else {
+		mergeCommitID, mergeErr = s.mergeWithGit2Go(ctx, repoPath, quarantineRepo,
+			string(firstRequest.User.Name),
+			string(firstRequest.User.Email),
+			authorDate,
+			string(firstRequest.Message),
+			revision.String(),
+			firstRequest.CommitId)
+	}
+
+	if mergeErr != nil {
 		var conflictErr git2go.ConflictingFilesError
-		if errors.As(err, &conflictErr) {
+		if errors.As(mergeErr, &conflictErr) {
 			conflictingFiles := make([][]byte, 0, len(conflictErr.ConflictingFiles))
 			for _, conflictingFile := range conflictErr.ConflictingFiles {
 				conflictingFiles = append(conflictingFiles, []byte(conflictingFile))
@@ -152,7 +180,7 @@ func (s *Server) UserMergeBranch(stream gitalypb.OperationService_UserMergeBranc
 		return err
 	}
 
-	mergeOID, err := git.ObjectHashSHA1.FromHex(commitID)
+	mergeOID, err := git.ObjectHashSHA1.FromHex(mergeCommitID)
 	if err != nil {
 		return helper.ErrInternalf("could not parse merge ID: %w", err)
 	}
diff --git a/internal/gitaly/service/operations/merge_test.go b/internal/gitaly/service/operations/merge_test.go
index 3965bb435..af041e720 100644
--- a/internal/gitaly/service/operations/merge_test.go
+++ b/internal/gitaly/service/operations/merge_test.go
@@ -24,6 +24,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitlab"
 	"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/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testcfg"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testserver"
@@ -42,8 +43,14 @@ var (
 
 func TestUserMergeBranch_successful(t *testing.T) {
 	t.Parallel()
+	testhelper.NewFeatureSets(featureflag.MergeTreeMerge).Run(
+		t,
+		testUserMergeBranchSuccessful,
+	)
+}
 
-	ctx := testhelper.Context(t)
+func testUserMergeBranchSuccessful(t *testing.T, ctx context.Context) {
+	t.Parallel()
 
 	ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
 
@@ -224,9 +231,13 @@ func TestUserMergeBranch_failure(t *testing.T) {
 
 func TestUserMergeBranch_quarantine(t *testing.T) {
 	t.Parallel()
+	testhelper.NewFeatureSets(featureflag.MergeTreeMerge).Run(
+		t,
+		testUserMergeBranchQuarantine,
+	)
+}
 
-	ctx := testhelper.Context(t)
-
+func testUserMergeBranchQuarantine(t *testing.T, ctx context.Context) {
 	ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
 	repo := localrepo.NewTestRepo(t, cfg, repoProto)
 
@@ -282,8 +293,14 @@ func TestUserMergeBranch_quarantine(t *testing.T) {
 
 func TestUserMergeBranch_stableMergeIDs(t *testing.T) {
 	t.Parallel()
+	testhelper.NewFeatureSets(featureflag.MergeTreeMerge).Run(
+		t,
+		testUserMergeBranchStableMergeIDs,
+	)
+}
 
-	ctx := testhelper.Context(t)
+func testUserMergeBranchStableMergeIDs(t *testing.T, ctx context.Context) {
+	t.Parallel()
 
 	ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
 
@@ -309,7 +326,7 @@ func TestUserMergeBranch_stableMergeIDs(t *testing.T) {
 	require.NoError(t, mergeBidi.Send(firstRequest), "send first request")
 	response, err := mergeBidi.Recv()
 	require.NoError(t, err, "receive first response")
-	require.Equal(t, response.CommitId, expectedMergeID)
+	require.Equal(t, expectedMergeID, response.CommitId)
 
 	require.NoError(t, mergeBidi.Send(&gitalypb.UserMergeBranchRequest{Apply: true}), "apply merge")
 	response, err = mergeBidi.Recv()
@@ -350,8 +367,14 @@ func TestUserMergeBranch_stableMergeIDs(t *testing.T) {
 
 func TestUserMergeBranch_abort(t *testing.T) {
 	t.Parallel()
+	testhelper.NewFeatureSets(featureflag.MergeTreeMerge).Run(
+		t,
+		testUserMergeBranchAbort,
+	)
+}
 
-	ctx := testhelper.Context(t)
+func testUserMergeBranchAbort(t *testing.T, ctx context.Context) {
+	t.Parallel()
 
 	ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
 
@@ -414,8 +437,14 @@ func TestUserMergeBranch_abort(t *testing.T) {
 
 func TestUserMergeBranch_concurrentUpdate(t *testing.T) {
 	t.Parallel()
+	testhelper.NewFeatureSets(featureflag.MergeTreeMerge).Run(
+		t,
+		testUserMergeBranchConcurrentUpdate,
+	)
+}
 
-	ctx := testhelper.Context(t)
+func testUserMergeBranchConcurrentUpdate(t *testing.T, ctx context.Context) {
+	t.Parallel()
 
 	ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
 
@@ -470,8 +499,14 @@ func TestUserMergeBranch_concurrentUpdate(t *testing.T) {
 
 func TestUserMergeBranch_ambiguousReference(t *testing.T) {
 	t.Parallel()
+	testhelper.NewFeatureSets(featureflag.MergeTreeMerge).Run(
+		t,
+		testUserMergeBranchAmbiguousReference,
+	)
+}
 
-	ctx := testhelper.Context(t)
+func testUserMergeBranchAmbiguousReference(t *testing.T, ctx context.Context) {
+	t.Parallel()
 
 	ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
 
@@ -532,8 +567,14 @@ func TestUserMergeBranch_ambiguousReference(t *testing.T) {
 
 func TestUserMergeBranch_failingHooks(t *testing.T) {
 	t.Parallel()
+	testhelper.NewFeatureSets(featureflag.MergeTreeMerge).Run(
+		t,
+		testUserMergeBranchFailingHooks,
+	)
+}
 
-	ctx := testhelper.Context(t)
+func testUserMergeBranchFailingHooks(t *testing.T, ctx context.Context) {
+	t.Parallel()
 
 	ctx, cfg, repo, repoPath, client := setupOperationsService(t, ctx)
 
@@ -623,8 +664,14 @@ func TestUserMergeBranch_failingHooks(t *testing.T) {
 
 func TestUserMergeBranch_conflict(t *testing.T) {
 	t.Parallel()
+	testhelper.NewFeatureSets(featureflag.MergeTreeMerge).Run(
+		t,
+		testUserMergeBranchConflict,
+	)
+}
 
-	ctx := testhelper.Context(t)
+func testUserMergeBranchConflict(t *testing.T, ctx context.Context) {
+	t.Parallel()
 
 	ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
 
@@ -679,9 +726,13 @@ func TestUserMergeBranch_conflict(t *testing.T) {
 
 func TestUserMergeBranch_allowed(t *testing.T) {
 	t.Parallel()
+	testhelper.NewFeatureSets(featureflag.MergeTreeMerge).Run(
+		t,
+		testUserMergeBranchAllowed,
+	)
+}
 
-	ctx := testhelper.Context(t)
-
+func testUserMergeBranchAllowed(t *testing.T, ctx context.Context) {
 	mergeBranchHeadAfter := "ff0ac4dfa30d6b26fd14aa83a75650355270bf76"
 
 	for _, tc := range []struct {
diff --git a/internal/metadata/featureflag/ff_merge_git_merge-tree.go b/internal/metadata/featureflag/ff_merge_git_merge-tree.go
new file mode 100644
index 000000000..38539e074
--- /dev/null
+++ b/internal/metadata/featureflag/ff_merge_git_merge-tree.go
@@ -0,0 +1,10 @@
+package featureflag
+
+// MergeTreeMerge enables implementation of UserMergeBranch using
+// `git merge-tree` instead of s.git2goExecutor.Merge()
+var MergeTreeMerge = NewFeatureFlag(
+	"merge_tree_merge",
+	"15.6.0",
+	"https://gitlab.com/gitlab-org/gitaly/-/issues/4600",
+	false,
+)