gitaly-git2go: Return structured conflict error on merge conflict

We have recently started efforts to return structured error types from
Gitaly RPCs to tell upstream callers why an error was raised, which is
going to replace the current approach where error information is passed
as plain string in a normal RPC response. The first candidate we're
converting is UserMergeBranch.

While it is easy in UserMergeBranch to return error details in case
access checks fail, we also want to return details in case the merge
failed due to conflicts. The most important info we want to return in
this case is which files are conflicting as well as the revisions which
do. While we already have info about the revisions, Gitaly currently
cannot tell which files were conflicting: this information is only known
to gitaly-git2go, and we do not pass up this information in any way.

Now that we have gob serialization in place in gitaly-git2go's merge
subcommand, this is an easy fix though given that we can easily return
"real" errors. This commit thus introduces a new `ConflictingFilesError`
which has a slice of conflicting files as field and returns it in case
gob serialization is in use.

Note that we do not yet use this rich information for now, but this
change only lays the groundwork. As such, no change in behaviour is
expected from this commit.

2 files changed, 55 insertions, 6 deletions

diff --git a/cmd/gitaly-git2go/merge.go b/cmd/gitaly-git2go/merge.go
index cd7fb4673..6ad8e2bb3 100644
--- a/cmd/gitaly-git2go/merge.go
+++ b/cmd/gitaly-git2go/merge.go
@@ -45,7 +45,7 @@ func (cmd *mergeSubcommand) Run(_ context.Context, r io.Reader, w io.Writer) err
 		request.AuthorDate = time.Now()
 	}
 
-	commitID, err := merge(request)
+	commitID, err := merge(request, useGob)
 	if useGob {
 		return gob.NewEncoder(w).Encode(git2go.Result{
 			CommitID: commitID,
@@ -68,7 +68,7 @@ func (cmd *mergeSubcommand) Run(_ context.Context, r io.Reader, w io.Writer) err
 	return nil
 }
 
-func merge(request git2go.MergeCommand) (string, error) {
+func merge(request git2go.MergeCommand, useStructuredErrors bool) (string, error) {
 	repo, err := git2goutil.OpenRepository(request.Repository)
 	if err != nil {
 		return "", fmt.Errorf("could not open repository: %w", err)
@@ -99,10 +99,21 @@ func merge(request git2go.MergeCommand) (string, error) {
 
 	if index.HasConflicts() {
 		if !request.AllowConflicts {
+			if useStructuredErrors {
+				conflictingFiles, err := getConflictingFiles(index)
+				if err != nil {
+					return "", fmt.Errorf("getting conflicting files: %w", err)
+				}
+
+				return "", git2go.ConflictingFilesError{
+					ConflictingFiles: conflictingFiles,
+				}
+			}
+
 			return "", errors.New("could not auto-merge due to conflicts")
 		}
 
-		if err := resolveConflicts(repo, index); err != nil {
+		if err := resolveConflicts(repo, index, useStructuredErrors); err != nil {
 			return "", fmt.Errorf("could not resolve conflicts: %w", err)
 		}
 	}
@@ -121,7 +132,7 @@ func merge(request git2go.MergeCommand) (string, error) {
 	return commit.String(), nil
 }
 
-func resolveConflicts(repo *git.Repository, index *git.Index) error {
+func resolveConflicts(repo *git.Repository, index *git.Index, useStructuredErrors bool) error {
 	// We need to get all conflicts up front as resolving conflicts as we
 	// iterate breaks the iterator.
 	indexConflicts, err := getConflicts(index)
@@ -182,12 +193,46 @@ func resolveConflicts(repo *git.Repository, index *git.Index) error {
 	}
 
 	if index.HasConflicts() {
+		if useStructuredErrors {
+			conflictingFiles, err := getConflictingFiles(index)
+			if err != nil {
+				return fmt.Errorf("getting conflicting files: %w", err)
+			}
+
+			return git2go.ConflictingFilesError{
+				ConflictingFiles: conflictingFiles,
+			}
+		}
+
 		return errors.New("index still has conflicts")
 	}
 
 	return nil
 }
 
+func getConflictingFiles(index *git.Index) ([]string, error) {
+	conflicts, err := getConflicts(index)
+	if err != nil {
+		return nil, fmt.Errorf("getting conflicts: %w", err)
+	}
+
+	conflictingFiles := make([]string, 0, len(conflicts))
+	for _, conflict := range conflicts {
+		switch {
+		case conflict.Our != nil:
+			conflictingFiles = append(conflictingFiles, conflict.Our.Path)
+		case conflict.Ancestor != nil:
+			conflictingFiles = append(conflictingFiles, conflict.Ancestor.Path)
+		case conflict.Their != nil:
+			conflictingFiles = append(conflictingFiles, conflict.Their.Path)
+		default:
+			return nil, errors.New("invalid conflict")
+		}
+	}
+
+	return conflictingFiles, nil
+}
+
 func isConflictMergeable(conflict git.IndexConflict) bool {
 	conflictIndexEntriesCount := 0
 
diff --git a/cmd/gitaly-git2go/merge_test.go b/cmd/gitaly-git2go/merge_test.go
index ef1c2100b..22c95e9a0 100644
--- a/cmd/gitaly-git2go/merge_test.go
+++ b/cmd/gitaly-git2go/merge_test.go
@@ -190,7 +190,9 @@ func testMergeTrees(t *testing.T, ctx context.Context) {
 			theirs: map[string]string{
 				"1": "qux",
 			},
-			expectedErr: fmt.Errorf("merge: %w", git2go.SerializableError(errors.New("could not auto-merge due to conflicts"))),
+			expectedErr: fmt.Errorf("merge: %w", git2go.ConflictingFilesError{
+				ConflictingFiles: []string{"1"},
+			}),
 			//nolint:revive
 			expectedErrWithoutGob: errors.New("merge: could not auto-merge due to conflicts\n"),
 		},
@@ -330,7 +332,9 @@ func testMergeRecursive(t *testing.T, ctx context.Context) {
 		Theirs:     theirs[len(theirs)-1].String(),
 	})
 	if featureflag.Git2GoMergeGob.IsEnabled(ctx) {
-		require.Equal(t, fmt.Errorf("merge: %w", git2go.SerializableError(errors.New("could not auto-merge due to conflicts"))), err)
+		require.Equal(t, fmt.Errorf("merge: %w", git2go.ConflictingFilesError{
+			ConflictingFiles: []string{"theirs"},
+		}), err)
 	} else {
 		//nolint:revive
 		require.Equal(t, errors.New("merge: could not auto-merge due to conflicts\n"), err)