Merge branch 'ps-precondition-fail-on-merge' into 'master'

operations: Make UserMergeBranch return PreconditionFailed on conflicts

Closes #3195

See merge request gitlab-org/gitaly!3700

2 files changed, 48 insertions, 1 deletions

diff --git a/internal/gitaly/service/operations/merge.go b/internal/gitaly/service/operations/merge.go
index 846c578e5..79e1598e7 100644
--- a/internal/gitaly/service/operations/merge.go
+++ b/internal/gitaly/service/operations/merge.go
@@ -78,7 +78,12 @@ func (s *Server) UserMergeBranch(stream gitalypb.OperationService_UserMergeBranc
 		if errors.Is(err, git2go.ErrInvalidArgument) {
 			return helper.ErrInvalidArgument(err)
 		}
-		return err
+
+		if strings.Contains(err.Error(), "could not auto-merge due to conflicts") {
+			return helper.ErrFailedPreconditionf("Failed to merge for source_sha %s into target_sha %s",
+				firstRequest.CommitId, revision.String())
+		}
+		return helper.ErrInternal(err)
 	}
 
 	mergeOID, err := git.NewObjectIDFromHex(merge.CommitID)
diff --git a/internal/gitaly/service/operations/merge_test.go b/internal/gitaly/service/operations/merge_test.go
index b91846b69..eb7a08cce 100644
--- a/internal/gitaly/service/operations/merge_test.go
+++ b/internal/gitaly/service/operations/merge_test.go
@@ -15,6 +15,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git/gittest"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git/localrepo"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/config"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/helper/text"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper/testassert"
@@ -405,6 +406,47 @@ func TestFailedMergeDueToHooks(t *testing.T) {
 	}
 }
 
+func TestUserMergeBranch_conflict(t *testing.T) {
+	t.Parallel()
+	ctx, cancel := testhelper.Context()
+	defer cancel()
+
+	ctx, cfg, repoProto, repoPath, client := setupOperationsService(t, ctx)
+
+	const mergeIntoBranch = "mergeIntoBranch"
+	const mergeFromBranch = "mergeFromBranch"
+	const conflictingFile = "file"
+
+	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.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",
+	}))
+
+	mergeBidi, err := client.UserMergeBranch(ctx)
+	require.NoError(t, err)
+
+	require.NoError(t, mergeBidi.Send(&gitalypb.UserMergeBranchRequest{
+		Repository: repoProto,
+		User:       gittest.TestUser,
+		Branch:     []byte(mergeIntoBranch),
+		CommitId:   divergedFrom.String(),
+		Message:    []byte("msg"),
+	}), "send first request")
+
+	firstResponse, err := mergeBidi.Recv()
+	testassert.GrpcEqualErr(t, helper.ErrFailedPreconditionf("Failed to merge for source_sha %s into target_sha %s", divergedFrom, divergedInto), err)
+	require.Nil(t, firstResponse)
+}
+
 func TestSuccessfulUserFFBranchRequest(t *testing.T) {
 	t.Parallel()
 	ctx, cancel := testhelper.Context()