Merge branch 'jc/fix-merge-tree-merge' into 'master'

localrepo: Allow unrelated histories in merge tree

See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5385

Merged-by: karthik nayak <knayak@gitlab.com>
Approved-by: karthik nayak <knayak@gitlab.com>
Reviewed-by: karthik nayak <knayak@gitlab.com>
Co-authored-by: John Cai <jcai@gitlab.com>

4 files changed, 103 insertions, 29 deletions

diff --git a/internal/git/localrepo/merge.go b/internal/git/localrepo/merge.go
index f3dc32a92..60a2608cb 100644
--- a/internal/git/localrepo/merge.go
+++ b/internal/git/localrepo/merge.go
@@ -12,22 +12,50 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v15/internal/helper/text"
 )
 
+type mergeTreeConfig struct {
+	allowUnrelatedHistories bool
+}
+
+// MergeTreeOption is a function that sets a config in mergeTreeConfig.
+type MergeTreeOption func(*mergeTreeConfig)
+
+// WithAllowUnrelatedHistories lets MergeTree accept two commits that do not
+// share a common ancestor.
+func WithAllowUnrelatedHistories() MergeTreeOption {
+	return func(options *mergeTreeConfig) {
+		options.allowUnrelatedHistories = true
+	}
+}
+
 // MergeTree calls git-merge-tree(1) with arguments, and parses the results from
 // stdout.
 func (repo *Repo) MergeTree(
 	ctx context.Context,
 	ours, theirs string,
+	mergeTreeOptions ...MergeTreeOption,
 ) (git.ObjectID, error) {
+	var config mergeTreeConfig
+
+	for _, option := range mergeTreeOptions {
+		option(&config)
+	}
+
+	flags := []git.Option{
+		git.Flag{Name: "--write-tree"},
+		git.Flag{Name: "--name-only"},
+	}
+
+	if config.allowUnrelatedHistories {
+		flags = append(flags, git.Flag{Name: "--allow-unrelated-histories"})
+	}
+
 	var stdout, stderr bytes.Buffer
 	err := repo.ExecAndWait(
 		ctx,
 		git.Command{
-			Name: "merge-tree",
-			Flags: []git.Option{
-				git.Flag{Name: "--write-tree"},
-				git.Flag{Name: "--name-only"},
-			},
-			Args: []string{ours, theirs},
+			Name:  "merge-tree",
+			Flags: flags,
+			Args:  []string{ours, theirs},
 		},
 		git.WithStderr(&stderr),
 		git.WithStdout(&stdout),
diff --git a/internal/git/localrepo/merge_test.go b/internal/git/localrepo/merge_test.go
index f4755635b..11112d72d 100644
--- a/internal/git/localrepo/merge_test.go
+++ b/internal/git/localrepo/merge_test.go
@@ -210,6 +210,67 @@ func TestMergeTree(t *testing.T) {
 			}
 		})
 	}
+
+	t.Run("allow unrelated histories", func(t *testing.T) {
+		ctx := testhelper.Context(t)
+
+		repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
+			SkipCreationViaService: true,
+		})
+		repo := NewTestRepo(t, cfg, repoProto)
+
+		tree1 := gittest.WriteTree(t, cfg, repoPath, []gittest.TreeEntry{
+			{
+				Mode:    "100644",
+				Path:    "file1",
+				Content: "foo",
+			},
+		})
+		tree2 := gittest.WriteTree(t, cfg, repoPath, []gittest.TreeEntry{
+			{
+				Mode:    "100644",
+				Path:    "file2",
+				Content: "baz",
+			},
+		})
+		ours := gittest.WriteCommit(t, cfg, repoPath,
+			gittest.WithTree(tree1),
+			gittest.WithAuthorName("Woody"),
+			gittest.WithCommitterName("Woody"),
+		)
+		theirs := gittest.WriteCommit(t, cfg, repoPath,
+			gittest.WithTree(tree2),
+			gittest.WithAuthorName("Buzz"),
+			gittest.WithCommitterName("Buzz"),
+		)
+
+		mergeTreeResult, err := repo.MergeTree(
+			ctx,
+			string(ours),
+			string(theirs),
+			WithAllowUnrelatedHistories(),
+		)
+		require.NoError(t, err)
+
+		gittest.RequireTree(
+			t,
+			cfg,
+			repoPath,
+			string(mergeTreeResult),
+			[]gittest.TreeEntry{
+				{
+					Mode:    "100644",
+					Path:    "file1",
+					Content: "foo",
+				},
+				{
+					Mode:    "100644",
+					Path:    "file2",
+					Content: "baz",
+				},
+			},
+		)
+	})
 }
 
 func TestParseResult(t *testing.T) {
diff --git a/internal/gitaly/service/operations/merge.go b/internal/gitaly/service/operations/merge.go
index fe7b6e198..1bcf9f452 100644
--- a/internal/gitaly/service/operations/merge.go
+++ b/internal/gitaly/service/operations/merge.go
@@ -63,7 +63,7 @@ func (s *Server) merge(
 	ours string,
 	theirs string,
 ) (string, error) {
-	treeOID, err := quarantineRepo.MergeTree(ctx, ours, theirs)
+	treeOID, err := quarantineRepo.MergeTree(ctx, ours, theirs, localrepo.WithAllowUnrelatedHistories())
 	if err != nil {
 		return "", err
 	}
diff --git a/internal/gitaly/service/operations/merge_test.go b/internal/gitaly/service/operations/merge_test.go
index 6764a3593..2d44f246d 100644
--- a/internal/gitaly/service/operations/merge_test.go
+++ b/internal/gitaly/service/operations/merge_test.go
@@ -913,38 +913,23 @@ 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(baseBranch), gittest.WithTreeEntries(gittest.TreeEntry{
+	baseCommit := gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch(mergeIntoBranch), 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.WithParents(baseCommit),
-		gittest.WithTreeEntries(gittest.TreeEntry{
-			Mode: "100644", Path: conflictingFile, Content: "data-1",
-		}),
-	)
+	divergedInto := gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch(mergeIntoBranch), gittest.WithTreeEntries(gittest.TreeEntry{
+		Mode: "100644", Path: conflictingFile, Content: "data-1",
+	}))
 
-	divergedFrom := gittest.WriteCommit(
-		t,
-		cfg,
-		repoPath,
-		gittest.WithBranch(mergeFromBranch),
-		gittest.WithParents(baseCommit),
-		gittest.WithTreeEntries(gittest.TreeEntry{
-			Mode: "100644", Path: conflictingFile, Content: "data-2",
-		}),
-	)
+	divergedFrom := gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch(mergeFromBranch), gittest.WithTreeEntries(gittest.TreeEntry{
+		Mode: "100644", Path: conflictingFile, Content: "data-2",
+	}))
 
 	mergeBidi, err := client.UserMergeBranch(ctx)
 	require.NoError(t, err)