From 2741afccfcb44c0d9a5704ee94672ee80bc37565 Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Tue, 25 Apr 2023 12:56:58 +0200 Subject: [PATCH 1/4] localrepo: Return OID when conflicts found in MergeTree The `MergeTree` function executes `git-merge-tree(1)`, parses the output and returns the information to the user. Currently, it exclusively returns either the OID of the tree created (success path) or the parsed error (failure path). But `git-merge-tree(1)` provides the OID of the tree even during the failure path. This is necessary information which is needed to find the contents of the conflicted files. Let's also parse the OID during the failure path and return it to the client. This fix exposes/removes the hard-coding of SHA1 OIDs in the `TestParseResult` tests. --- internal/git/localrepo/merge.go | 27 ++++++++++++++++----------- internal/git/localrepo/merge_test.go | 19 +++++++++++-------- 2 files changed, 27 insertions(+), 19 deletions(-) diff --git a/internal/git/localrepo/merge.go b/internal/git/localrepo/merge.go index 60a2608cbc..19e4c80e50 100644 --- a/internal/git/localrepo/merge.go +++ b/internal/git/localrepo/merge.go @@ -49,8 +49,13 @@ func (repo *Repo) MergeTree( flags = append(flags, git.Flag{Name: "--allow-unrelated-histories"}) } + objectHash, err := repo.ObjectHash(ctx) + if err != nil { + return "", fmt.Errorf("getting object hash %w", err) + } + var stdout, stderr bytes.Buffer - err := repo.ExecAndWait( + err = repo.ExecAndWait( ctx, git.Command{ Name: "merge-tree", @@ -75,12 +80,7 @@ func (repo *Repo) MergeTree( return "", fmt.Errorf("merge-tree: %w", err) } - return "", parseMergeTreeError(stdout.String()) - } - - objectHash, err := repo.ObjectHash(ctx) - if err != nil { - return "", fmt.Errorf("getting object hash %w", err) + return parseMergeTreeError(objectHash, stdout.String()) } oid, err := objectHash.FromHex(text.ChompBytes(stdout.Bytes())) @@ -94,26 +94,31 @@ func (repo *Repo) MergeTree( // parseMergeTreeError parses the output from git-merge-tree(1)'s stdout into // a MergeTreeResult struct. The format for the output can be found at // https://git-scm.com/docs/git-merge-tree#OUTPUT. -func parseMergeTreeError(output string) error { +func parseMergeTreeError(objectHash git.ObjectHash, output string) (git.ObjectID, error) { var mergeTreeError MergeTreeError lines := strings.SplitN(output, "\n\n", 2) // When the output is of unexpected length if len(lines) < 2 { - return errors.New("error parsing merge tree result") + return "", errors.New("error parsing merge tree result") } mergeTreeError.InfoMessage = strings.TrimSuffix(lines[1], "\n") oidAndConflicts := strings.Split(lines[0], "\n") if len(oidAndConflicts) < 2 { - return &mergeTreeError + return "", &mergeTreeError + } + + oid, err := objectHash.FromHex(text.ChompBytes([]byte(oidAndConflicts[0]))) + if err != nil { + return "", fmt.Errorf("hex to oid: %w", err) } mergeTreeError.ConflictingFiles = oidAndConflicts[1:] - return &mergeTreeError + return oid, &mergeTreeError } // MergeTreeError encapsulates any conflicting files and messages that occur diff --git a/internal/git/localrepo/merge_test.go b/internal/git/localrepo/merge_test.go index 11112d72d0..abfe30dfb1 100644 --- a/internal/git/localrepo/merge_test.go +++ b/internal/git/localrepo/merge_test.go @@ -2,6 +2,7 @@ package localrepo import ( "context" + "fmt" "testing" "github.com/stretchr/testify/require" @@ -277,18 +278,18 @@ func TestParseResult(t *testing.T) { testCases := []struct { desc string output string - oid string + oid git.ObjectID expectedErr *MergeTreeError }{ { desc: "single file conflict", - output: `4f18fcb9bc62a3a6a4f81236fd57eeb95bfb06b4 + output: fmt.Sprintf(`%s file Auto-merging file CONFLICT (content): Merge conflict in file -`, - oid: "4f18fcb9bc62a3a6a4f81236fd57eeb95bfb06b4", +`, gittest.DefaultObjectHash.EmptyTreeOID), + oid: git.ObjectID(gittest.DefaultObjectHash.EmptyTreeOID), expectedErr: &MergeTreeError{ ConflictingFiles: []string{ "file", @@ -298,14 +299,14 @@ CONFLICT (content): Merge conflict in file }, { desc: "multiple files conflict", - output: `4f18fcb9bc62a3a6a4f81236fd57eeb95bfb06b4 + output: fmt.Sprintf(`%s file1 file2 Auto-merging file CONFLICT (content): Merge conflict in file1 -`, - oid: "4f18fcb9bc62a3a6a4f81236fd57eeb95bfb06b4", +`, gittest.DefaultObjectHash.EmptyTreeOID), + oid: git.ObjectID(gittest.DefaultObjectHash.EmptyTreeOID), expectedErr: &MergeTreeError{ ConflictingFiles: []string{ "file1", @@ -318,11 +319,13 @@ CONFLICT (content): Merge conflict in file1 for _, tc := range testCases { t.Run(tc.desc, func(t *testing.T) { - err := parseMergeTreeError(tc.output) + oid, err := parseMergeTreeError(gittest.DefaultObjectHash, tc.output) if tc.expectedErr != nil { require.Equal(t, tc.expectedErr, err) return } + + require.Equal(t, tc.oid, oid) require.NoError(t, err) }) } -- GitLab From fbb2278dbd81bae6a74b0ad5dcb6e6902b2ce990 Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Tue, 25 Apr 2023 14:12:48 +0200 Subject: [PATCH 2/4] localrepo: Extend MergeTreeError with ConflictingFileInfo The MergeTreeError only holds the conflicting file name which is the output of `git-merge-tree(1)` when using the `--name-only` option. When using `git-merge-tree(1)` for `ListConflictFiles`, we'll need additional information such as the OID, mode and stage of the conflicting files. By adding ConflictingFileInfo, we make the struct extendable for additional fields. --- internal/git/localrepo/merge.go | 17 +++++++++++++---- internal/git/localrepo/merge_test.go | 16 +++++++--------- internal/gitaly/service/operations/merge.go | 6 +++--- 3 files changed, 23 insertions(+), 16 deletions(-) diff --git a/internal/git/localrepo/merge.go b/internal/git/localrepo/merge.go index 19e4c80e50..a9845fa0c3 100644 --- a/internal/git/localrepo/merge.go +++ b/internal/git/localrepo/merge.go @@ -116,16 +116,25 @@ func parseMergeTreeError(objectHash git.ObjectHash, output string) (git.ObjectID return "", fmt.Errorf("hex to oid: %w", err) } - mergeTreeError.ConflictingFiles = oidAndConflicts[1:] + mergeTreeError.ConflictingFileInfo = make([]ConflictingFileInfo, len(oidAndConflicts[1:])) + + for i, fileName := range oidAndConflicts[1:] { + mergeTreeError.ConflictingFileInfo[i].FileName = fileName + } return oid, &mergeTreeError } -// MergeTreeError encapsulates any conflicting files and messages that occur +// ConflictingFileInfo holds the conflicting file info output from git-merge-tree(1). +type ConflictingFileInfo struct { + FileName string +} + +// MergeTreeError encapsulates any conflicting file info and messages that occur // when a merge-tree(1) command fails. type MergeTreeError struct { - ConflictingFiles []string - InfoMessage string + ConflictingFileInfo []ConflictingFileInfo + InfoMessage string } // Error returns the error string for a conflict error. diff --git a/internal/git/localrepo/merge_test.go b/internal/git/localrepo/merge_test.go index abfe30dfb1..72d8292b84 100644 --- a/internal/git/localrepo/merge_test.go +++ b/internal/git/localrepo/merge_test.go @@ -128,8 +128,8 @@ func TestMergeTree(t *testing.T) { { desc: "with conflicts", expectedErr: &MergeTreeError{ - ConflictingFiles: []string{"file2"}, - InfoMessage: "Auto-merging file2\nCONFLICT (add/add): Merge conflict in file2", + ConflictingFileInfo: []ConflictingFileInfo{{FileName: "file2"}}, + InfoMessage: "Auto-merging file2\nCONFLICT (add/add): Merge conflict in file2", }, setupFunc: func(t *testing.T, ctx context.Context, repoPath string) (git.ObjectID, git.ObjectID, []gittest.TreeEntry) { tree1 := gittest.WriteTree(t, cfg, repoPath, []gittest.TreeEntry{ @@ -291,10 +291,8 @@ CONFLICT (content): Merge conflict in file `, gittest.DefaultObjectHash.EmptyTreeOID), oid: git.ObjectID(gittest.DefaultObjectHash.EmptyTreeOID), expectedErr: &MergeTreeError{ - ConflictingFiles: []string{ - "file", - }, - InfoMessage: "Auto-merging file\nCONFLICT (content): Merge conflict in file", + ConflictingFileInfo: []ConflictingFileInfo{{FileName: "file"}}, + InfoMessage: "Auto-merging file\nCONFLICT (content): Merge conflict in file", }, }, { @@ -308,9 +306,9 @@ CONFLICT (content): Merge conflict in file1 `, gittest.DefaultObjectHash.EmptyTreeOID), oid: git.ObjectID(gittest.DefaultObjectHash.EmptyTreeOID), expectedErr: &MergeTreeError{ - ConflictingFiles: []string{ - "file1", - "file2", + ConflictingFileInfo: []ConflictingFileInfo{ + {FileName: "file1"}, + {FileName: "file2"}, }, InfoMessage: "Auto-merging file\nCONFLICT (content): Merge conflict in file1", }, diff --git a/internal/gitaly/service/operations/merge.go b/internal/gitaly/service/operations/merge.go index 0438826ea3..88fd7a5ad2 100644 --- a/internal/gitaly/service/operations/merge.go +++ b/internal/gitaly/service/operations/merge.go @@ -156,9 +156,9 @@ func (s *Server) UserMergeBranch(stream gitalypb.OperationService_UserMergeBranc if mergeErr != nil { var conflictErr *localrepo.MergeTreeError if errors.As(mergeErr, &conflictErr) { - conflictingFiles := make([][]byte, 0, len(conflictErr.ConflictingFiles)) - for _, conflictingFile := range conflictErr.ConflictingFiles { - conflictingFiles = append(conflictingFiles, []byte(conflictingFile)) + conflictingFiles := make([][]byte, 0, len(conflictErr.ConflictingFileInfo)) + for _, conflictingFileInfo := range conflictErr.ConflictingFileInfo { + conflictingFiles = append(conflictingFiles, []byte(conflictingFileInfo.FileName)) } return structerr.NewFailedPrecondition("merging commits: %w", mergeErr). -- GitLab From d7447883f1f2ccc8551972aba9f5dd28b00fc54e Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Tue, 25 Apr 2023 15:46:01 +0200 Subject: [PATCH 3/4] localrepo: Make `--name-only` optional for git-merge-tree(1) The MergeTree command currently adds the `--name-only` flag by default. In this commit, we make this is an optional configuration under `WithConflictingFileNameOnly`. This allows us to get the entire conflicting file information from `git-merge-tree(1)` and parse it. We then add this information to the newly created `ConflictingFileInfo`. --- internal/git/localrepo/merge.go | 73 +++++++++++- internal/git/localrepo/merge_test.go | 123 +++++++++++++++++--- internal/gitaly/service/operations/merge.go | 2 +- 3 files changed, 175 insertions(+), 23 deletions(-) diff --git a/internal/git/localrepo/merge.go b/internal/git/localrepo/merge.go index a9845fa0c3..79f3bd60bb 100644 --- a/internal/git/localrepo/merge.go +++ b/internal/git/localrepo/merge.go @@ -5,6 +5,7 @@ import ( "context" "errors" "fmt" + "strconv" "strings" "gitlab.com/gitlab-org/gitaly/v15/internal/command" @@ -12,8 +13,23 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/helper/text" ) +// MergeStage denotes the stage indicated by git-merge-tree(1) in the conflicting +// files information section. The man page for git-merge(1) holds more information +// regarding the values of the stages and what they indicate. +type MergeStage uint + +const ( + // MergeStageAncestor denotes a conflicting file version from the common ancestor. + MergeStageAncestor = MergeStage(1) + // MergeStageOurs denotes a conflicting file version from our commit. + MergeStageOurs = MergeStage(2) + // MergeStageTheirs denotes a conflicting file version from their commit. + MergeStageTheirs = MergeStage(3) +) + type mergeTreeConfig struct { - allowUnrelatedHistories bool + allowUnrelatedHistories bool + conflictingFileNamesOnly bool } // MergeTreeOption is a function that sets a config in mergeTreeConfig. @@ -27,6 +43,14 @@ func WithAllowUnrelatedHistories() MergeTreeOption { } } +// WithConflictingFileNamesOnly lets MergeTree only parse the conflicting filenames and +// not the additional information. +func WithConflictingFileNamesOnly() MergeTreeOption { + return func(options *mergeTreeConfig) { + options.conflictingFileNamesOnly = true + } +} + // MergeTree calls git-merge-tree(1) with arguments, and parses the results from // stdout. func (repo *Repo) MergeTree( @@ -42,13 +66,16 @@ func (repo *Repo) MergeTree( 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"}) } + if config.conflictingFileNamesOnly { + flags = append(flags, git.Flag{Name: "--name-only"}) + } + objectHash, err := repo.ObjectHash(ctx) if err != nil { return "", fmt.Errorf("getting object hash %w", err) @@ -80,7 +107,7 @@ func (repo *Repo) MergeTree( return "", fmt.Errorf("merge-tree: %w", err) } - return parseMergeTreeError(objectHash, stdout.String()) + return parseMergeTreeError(objectHash, config, stdout.String()) } oid, err := objectHash.FromHex(text.ChompBytes(stdout.Bytes())) @@ -94,7 +121,7 @@ func (repo *Repo) MergeTree( // parseMergeTreeError parses the output from git-merge-tree(1)'s stdout into // a MergeTreeResult struct. The format for the output can be found at // https://git-scm.com/docs/git-merge-tree#OUTPUT. -func parseMergeTreeError(objectHash git.ObjectHash, output string) (git.ObjectID, error) { +func parseMergeTreeError(objectHash git.ObjectHash, cfg mergeTreeConfig, output string) (git.ObjectID, error) { var mergeTreeError MergeTreeError lines := strings.SplitN(output, "\n\n", 2) @@ -118,8 +145,40 @@ func parseMergeTreeError(objectHash git.ObjectHash, output string) (git.ObjectID mergeTreeError.ConflictingFileInfo = make([]ConflictingFileInfo, len(oidAndConflicts[1:])) - for i, fileName := range oidAndConflicts[1:] { - mergeTreeError.ConflictingFileInfo[i].FileName = fileName + // From git-merge-tree(1), the information is of the format ` ` + // unless the `--name-only` option is used, in which case only the filename is output. + // Note: that there is \t before the filename (https://gitlab.com/gitlab-org/git/blob/v2.40.0/builtin/merge-tree.c#L481) + for i, infoLine := range oidAndConflicts[1:] { + if cfg.conflictingFileNamesOnly { + mergeTreeError.ConflictingFileInfo[i].FileName = infoLine + } else { + infoAndFilename := strings.Split(infoLine, "\t") + if len(infoAndFilename) != 2 { + return "", fmt.Errorf("parsing conflicting file info: %s", infoLine) + } + + info := strings.Fields(infoAndFilename[0]) + if len(info) != 3 { + return "", fmt.Errorf("parsing conflicting file info: %s", infoLine) + } + + mergeTreeError.ConflictingFileInfo[i].OID, err = objectHash.FromHex(info[1]) + if err != nil { + return "", fmt.Errorf("hex to oid: %w", err) + } + + stage, err := strconv.Atoi(info[2]) + if err != nil { + return "", fmt.Errorf("converting stage to int: %w", err) + } + + if stage < 1 || stage > 3 { + return "", fmt.Errorf("invalid value for stage: %d", stage) + } + + mergeTreeError.ConflictingFileInfo[i].Stage = MergeStage(stage) + mergeTreeError.ConflictingFileInfo[i].FileName = infoAndFilename[1] + } } return oid, &mergeTreeError @@ -128,6 +187,8 @@ func parseMergeTreeError(objectHash git.ObjectHash, output string) (git.ObjectID // ConflictingFileInfo holds the conflicting file info output from git-merge-tree(1). type ConflictingFileInfo struct { FileName string + OID git.ObjectID + Stage MergeStage } // MergeTreeError encapsulates any conflicting file info and messages that occur diff --git a/internal/git/localrepo/merge_test.go b/internal/git/localrepo/merge_test.go index 72d8292b84..39426f229e 100644 --- a/internal/git/localrepo/merge_test.go +++ b/internal/git/localrepo/merge_test.go @@ -2,7 +2,9 @@ package localrepo import ( "context" + "errors" "fmt" + "strconv" "testing" "github.com/stretchr/testify/require" @@ -128,8 +130,14 @@ func TestMergeTree(t *testing.T) { { desc: "with conflicts", expectedErr: &MergeTreeError{ - ConflictingFileInfo: []ConflictingFileInfo{{FileName: "file2"}}, - InfoMessage: "Auto-merging file2\nCONFLICT (add/add): Merge conflict in file2", + ConflictingFileInfo: []ConflictingFileInfo{ + { + FileName: "file2", + OID: "", + Stage: 0, + }, + }, + InfoMessage: "Auto-merging file2\nCONFLICT (add/add): Merge conflict in file2", }, setupFunc: func(t *testing.T, ctx context.Context, repoPath string) (git.ObjectID, git.ObjectID, []gittest.TreeEntry) { tree1 := gittest.WriteTree(t, cfg, repoPath, []gittest.TreeEntry{ @@ -197,7 +205,7 @@ func TestMergeTree(t *testing.T) { ours, theirs, treeEntries := tc.setupFunc(t, ctx, repoPath) - mergeTreeResult, err := repo.MergeTree(ctx, string(ours), string(theirs)) + mergeTreeResult, err := repo.MergeTree(ctx, string(ours), string(theirs), WithConflictingFileNamesOnly()) require.Equal(t, tc.expectedErr, err) if tc.expectedErr == nil { @@ -279,45 +287,128 @@ func TestParseResult(t *testing.T) { desc string output string oid git.ObjectID - expectedErr *MergeTreeError + expectedErr error }{ { desc: "single file conflict", output: fmt.Sprintf(`%s -file +100644 %s 2%sfile +100644 %s 3%sfile Auto-merging file CONFLICT (content): Merge conflict in file -`, gittest.DefaultObjectHash.EmptyTreeOID), - oid: git.ObjectID(gittest.DefaultObjectHash.EmptyTreeOID), +`, gittest.DefaultObjectHash.EmptyTreeOID, gittest.DefaultObjectHash.EmptyTreeOID, "\t", gittest.DefaultObjectHash.EmptyTreeOID, "\t"), + oid: gittest.DefaultObjectHash.EmptyTreeOID, expectedErr: &MergeTreeError{ - ConflictingFileInfo: []ConflictingFileInfo{{FileName: "file"}}, - InfoMessage: "Auto-merging file\nCONFLICT (content): Merge conflict in file", + ConflictingFileInfo: []ConflictingFileInfo{ + { + FileName: "file", + OID: gittest.DefaultObjectHash.EmptyTreeOID, + Stage: MergeStageOurs, + }, + { + FileName: "file", + OID: gittest.DefaultObjectHash.EmptyTreeOID, + Stage: MergeStageTheirs, + }, + }, + InfoMessage: "Auto-merging file\nCONFLICT (content): Merge conflict in file", }, }, { desc: "multiple files conflict", output: fmt.Sprintf(`%s -file1 -file2 +100644 %s 2%sfile1 +100644 %s 3%sfile2 Auto-merging file CONFLICT (content): Merge conflict in file1 -`, gittest.DefaultObjectHash.EmptyTreeOID), - oid: git.ObjectID(gittest.DefaultObjectHash.EmptyTreeOID), +`, gittest.DefaultObjectHash.EmptyTreeOID, gittest.DefaultObjectHash.EmptyTreeOID, "\t", gittest.DefaultObjectHash.EmptyTreeOID, "\t"), + oid: gittest.DefaultObjectHash.EmptyTreeOID, expectedErr: &MergeTreeError{ ConflictingFileInfo: []ConflictingFileInfo{ - {FileName: "file1"}, - {FileName: "file2"}, + { + FileName: "file1", + OID: gittest.DefaultObjectHash.EmptyTreeOID, + Stage: MergeStageOurs, + }, + { + FileName: "file2", + OID: gittest.DefaultObjectHash.EmptyTreeOID, + Stage: MergeStageTheirs, + }, }, InfoMessage: "Auto-merging file\nCONFLICT (content): Merge conflict in file1", }, }, + { + desc: "no tab in conflicting file info", + output: fmt.Sprintf(`%s +100644 %s 2 file1 + +Auto-merging file +CONFLICT (content): Merge conflict in file1 +`, gittest.DefaultObjectHash.EmptyTreeOID, gittest.DefaultObjectHash.EmptyTreeOID), + oid: gittest.DefaultObjectHash.EmptyTreeOID, + expectedErr: fmt.Errorf("parsing conflicting file info: 100644 %s 2 file1", gittest.DefaultObjectHash.EmptyTreeOID), + }, + { + desc: "incorrect number of fields in conflicting file info", + output: fmt.Sprintf(`%s +%s 2%sfile1 + +Auto-merging file +CONFLICT (content): Merge conflict in file1 +`, gittest.DefaultObjectHash.EmptyTreeOID, gittest.DefaultObjectHash.EmptyTreeOID, "\t"), + oid: gittest.DefaultObjectHash.EmptyTreeOID, + expectedErr: fmt.Errorf("parsing conflicting file info: %s 2\tfile1", gittest.DefaultObjectHash.EmptyTreeOID), + }, + { + desc: "invalid OID in conflicting file info", + output: fmt.Sprintf(`%s +100644 23 2%sfile1 + +Auto-merging file +CONFLICT (content): Merge conflict in file1 +`, gittest.DefaultObjectHash.EmptyTreeOID, "\t"), + oid: gittest.DefaultObjectHash.EmptyTreeOID, + expectedErr: fmt.Errorf("hex to oid: %w", git.InvalidObjectIDLengthError{ + OID: "23", + CorrectLength: gittest.DefaultObjectHash.EncodedLen(), + Length: 2, + }), + }, + { + desc: "invalid stage type in conflicting file info", + output: fmt.Sprintf(`%s +100644 %s foo%sfile1 + +Auto-merging file +CONFLICT (content): Merge conflict in file1 +`, gittest.DefaultObjectHash.EmptyTreeOID, gittest.DefaultObjectHash.EmptyTreeOID, "\t"), + oid: gittest.DefaultObjectHash.EmptyTreeOID, + expectedErr: fmt.Errorf("converting stage to int: %w", &strconv.NumError{ + Func: "Atoi", + Num: "foo", + Err: errors.New("invalid syntax"), + }), + }, + { + desc: "invalid stage value in conflicting file info", + output: fmt.Sprintf(`%s +100644 %s 5%sfile1 + +Auto-merging file +CONFLICT (content): Merge conflict in file1 +`, gittest.DefaultObjectHash.EmptyTreeOID, gittest.DefaultObjectHash.EmptyTreeOID, "\t"), + oid: gittest.DefaultObjectHash.EmptyTreeOID, + expectedErr: fmt.Errorf("invalid value for stage: 5"), + }, } for _, tc := range testCases { t.Run(tc.desc, func(t *testing.T) { - oid, err := parseMergeTreeError(gittest.DefaultObjectHash, tc.output) + oid, err := parseMergeTreeError(gittest.DefaultObjectHash, mergeTreeConfig{}, tc.output) if tc.expectedErr != nil { require.Equal(t, tc.expectedErr, err) return diff --git a/internal/gitaly/service/operations/merge.go b/internal/gitaly/service/operations/merge.go index 88fd7a5ad2..a35392b9af 100644 --- a/internal/gitaly/service/operations/merge.go +++ b/internal/gitaly/service/operations/merge.go @@ -62,7 +62,7 @@ func (s *Server) merge( ours string, theirs string, ) (string, error) { - treeOID, err := quarantineRepo.MergeTree(ctx, ours, theirs, localrepo.WithAllowUnrelatedHistories()) + treeOID, err := quarantineRepo.MergeTree(ctx, ours, theirs, localrepo.WithAllowUnrelatedHistories(), localrepo.WithConflictingFileNamesOnly()) if err != nil { return "", err } -- GitLab From cef60a0e8b62e24529931fecc144b7fbbcab8a9d Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Tue, 25 Apr 2023 13:17:09 +0200 Subject: [PATCH 4/4] conflicts: Extract out the Git2Go implementation The Git2Go implementation of finding conflict files is currently in-lined into `ListConflictFiles`. This makes it hard to add a feature flag to switch to the upcoming git-merge-tree(1) implementation of the same. To make things easier we extract out this code into a new function `conflictFilesWithGit2Go`. We will introduce a similar function for the git-merge-tree(1) implementation in the upcoming commits. Then we can use a feature flag to switch between the two implementations and eventually remove the Git2Go implementation and inline the newer git-merge-tree(1) implementation. --- .../gitaly/service/conflicts/list_conflict_files.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/internal/gitaly/service/conflicts/list_conflict_files.go b/internal/gitaly/service/conflicts/list_conflict_files.go index f7cf317ac5..9d00077575 100644 --- a/internal/gitaly/service/conflicts/list_conflict_files.go +++ b/internal/gitaly/service/conflicts/list_conflict_files.go @@ -2,12 +2,14 @@ package conflicts import ( "bytes" + "context" "errors" "fmt" "io" "unicode/utf8" "gitlab.com/gitlab-org/gitaly/v15/internal/git" + "gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/v15/internal/git2go" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" @@ -39,6 +41,17 @@ func (s *server) ListConflictFiles(request *gitalypb.ListConflictFilesRequest, s return err } + return s.conflictFilesWithGit2Go(ctx, request, stream, ours, theirs, repo, repoPath) +} + +func (s *server) conflictFilesWithGit2Go( + ctx context.Context, + request *gitalypb.ListConflictFilesRequest, + stream gitalypb.ConflictsService_ListConflictFilesServer, + ours, theirs git.ObjectID, + repo *localrepo.Repo, + repoPath string, +) error { conflicts, err := s.git2goExecutor.Conflicts(ctx, repo, git2go.ConflictsCommand{ Repository: repoPath, Ours: ours.String(), -- GitLab