git2go: Add optional Committer to MergeCommand

Squash commits in GitLab use different committer and author.
Committer is the user that merges, and author is the user
that created the MR.
As we want to add squashing to this command, we have to make
sure that we support Committer* arguments as well.

Committer variables are optional, author data is
used when they are empty.
That way this change is backward compatible.
However, if at least one of Committer* variables are set,
all of them have to be.

3 files changed, 73 insertions, 4 deletions

diff --git a/cmd/gitaly-git2go/merge.go b/cmd/gitaly-git2go/merge.go
index 541cecf84..f97125061 100644
--- a/cmd/gitaly-git2go/merge.go
+++ b/cmd/gitaly-git2go/merge.go
@@ -92,8 +92,13 @@ func merge(request git2go.MergeCommand) (string, error) {
 		return "", fmt.Errorf("could not write tree: %w", err)
 	}
 
-	committer := git.Signature(git2go.NewSignature(request.AuthorName, request.AuthorMail, request.AuthorDate))
-	commit, err := repo.CreateCommitFromIds("", &committer, &committer, request.Message, tree, ours.Id(), theirs.Id())
+	author := git.Signature(git2go.NewSignature(request.AuthorName, request.AuthorMail, request.AuthorDate))
+	committer := author
+	if request.CommitterMail != "" {
+		committer = git.Signature(git2go.NewSignature(request.CommitterName, request.CommitterMail, request.CommitterDate))
+	}
+
+	commit, err := repo.CreateCommitFromIds("", &author, &committer, request.Message, tree, ours.Id(), theirs.Id())
 	if err != nil {
 		return "", fmt.Errorf("could not create merge commit: %w", err)
 	}
diff --git a/cmd/gitaly-git2go/merge_test.go b/cmd/gitaly-git2go/merge_test.go
index c63650fc6..b9fff3f6e 100644
--- a/cmd/gitaly-git2go/merge_test.go
+++ b/cmd/gitaly-git2go/merge_test.go
@@ -65,6 +65,22 @@ func TestMerge_missingArguments(t *testing.T) {
 			request:     git2go.MergeCommand{Repository: repoPath, AuthorName: "Foo", AuthorMail: "foo@example.com", Message: "Foo", Ours: "HEAD"},
 			expectedErr: "merge: invalid parameters: missing theirs",
 		},
+		// Committer* arguments are required only when at least one of them is non-empty
+		{
+			desc:        "missing committer mail",
+			request:     git2go.MergeCommand{Repository: repoPath, AuthorName: "Foo", AuthorMail: "foo@example.com", CommitterName: "Bar", Message: "Foo", Theirs: "HEAD", Ours: "HEAD"},
+			expectedErr: "merge: invalid parameters: missing committer mail",
+		},
+		{
+			desc:        "missing committer name",
+			request:     git2go.MergeCommand{Repository: repoPath, AuthorName: "Foo", AuthorMail: "foo@example.com", CommitterMail: "bar@example.com", Message: "Foo", Theirs: "HEAD", Ours: "HEAD"},
+			expectedErr: "merge: invalid parameters: missing committer name",
+		},
+		{
+			desc:        "missing committer date",
+			request:     git2go.MergeCommand{Repository: repoPath, AuthorName: "Foo", AuthorMail: "foo@example.com", CommitterName: "Bar", CommitterMail: "bar@example.com", Message: "Foo", Theirs: "HEAD", Ours: "HEAD"},
+			expectedErr: "merge: invalid parameters: missing committer date",
+		},
 	}
 
 	for _, tc := range testcases {
@@ -101,6 +117,7 @@ func TestMerge_trees(t *testing.T) {
 		ours             map[string]string
 		theirs           map[string]string
 		expected         map[string]string
+		withCommitter    bool
 		expectedResponse git2go.MergeResult
 		expectedErr      error
 	}{
@@ -123,6 +140,25 @@ func TestMerge_trees(t *testing.T) {
 			},
 		},
 		{
+			desc: "trivial merge with different committer succeeds",
+			base: map[string]string{
+				"file": "a",
+			},
+			ours: map[string]string{
+				"file": "a",
+			},
+			theirs: map[string]string{
+				"file": "a",
+			},
+			expected: map[string]string{
+				"file": "a",
+			},
+			withCommitter: true,
+			expectedResponse: git2go.MergeResult{
+				CommitID: "cba8c5ddf5a5a24f2f606e4b62d348feb1214b70",
+			},
+		},
+		{
 			desc: "non-trivial merge succeeds",
 			base: map[string]string{
 				"file": "a\nb\nc\nd\ne\nf\n",
@@ -193,9 +229,10 @@ func TestMerge_trees(t *testing.T) {
 		theirs := cmdtesthelper.BuildCommit(t, repoPath, []*git.Oid{base}, tc.theirs)
 
 		authorDate := time.Date(2020, 7, 30, 7, 45, 50, 0, time.FixedZone("UTC+2", +2*60*60))
+		committerDate := time.Date(2021, 7, 30, 7, 45, 50, 0, time.FixedZone("UTC+2", +2*60*60))
 
 		t.Run(tc.desc, func(t *testing.T) {
-			response, err := executor.Merge(ctx, repoProto, git2go.MergeCommand{
+			mergeCommand := git2go.MergeCommand{
 				Repository: repoPath,
 				AuthorName: "John Doe",
 				AuthorMail: "john.doe@example.com",
@@ -203,7 +240,13 @@ func TestMerge_trees(t *testing.T) {
 				Message:    "Merge message",
 				Ours:       ours.String(),
 				Theirs:     theirs.String(),
-			})
+			}
+			if tc.withCommitter {
+				mergeCommand.CommitterName = "Jane Doe"
+				mergeCommand.CommitterMail = "jane.doe@example.com"
+				mergeCommand.CommitterDate = committerDate
+			}
+			response, err := executor.Merge(ctx, repoProto, mergeCommand)
 
 			if tc.expectedErr != nil {
 				require.Equal(t, tc.expectedErr, err)
diff --git a/internal/git2go/merge.go b/internal/git2go/merge.go
index b03b3e499..17e9a4fe2 100644
--- a/internal/git2go/merge.go
+++ b/internal/git2go/merge.go
@@ -25,6 +25,15 @@ type MergeCommand struct {
 	AuthorMail string
 	// AuthorDate is the author date of merge commit.
 	AuthorDate time.Time
+	// CommitterName. Can be empty if all Committer* vars are empty.
+	// In that case AuthorName is used instead.
+	CommitterName string
+	// CommitterMail. Can be empty if all Committer* vars are empty.
+	// In that case AuthorMail is used instead.
+	CommitterMail string
+	// CommitterDate. Can be empty if all Committer* vars are empty.
+	// In that case AuthorDate is used instead.
+	CommitterDate time.Time
 	// Message is the message to be used for the merge commit.
 	Message string
 	// Ours is the commit into which theirs is to be merged.
@@ -77,5 +86,17 @@ func (m MergeCommand) verify() error {
 	if m.Theirs == "" {
 		return errors.New("missing theirs")
 	}
+	// If at least one Committer* var is set, require all of them to be set.
+	if m.CommitterMail != "" || !m.CommitterDate.IsZero() || m.CommitterName != "" {
+		if m.CommitterMail == "" {
+			return errors.New("missing committer mail")
+		}
+		if m.CommitterName == "" {
+			return errors.New("missing committer name")
+		}
+		if m.CommitterDate.IsZero() {
+			return errors.New("missing committer date")
+		}
+	}
 	return nil
 }