From 1d72d52eb7743c84b268e0fe9aceaed5c4f56b88 Mon Sep 17 00:00:00 2001 From: Piotr Stankowski Date: Mon, 10 Jan 2022 15:30:03 +0100 Subject: 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. --- cmd/gitaly-git2go/merge.go | 9 ++++++-- cmd/gitaly-git2go/merge_test.go | 47 +++++++++++++++++++++++++++++++++++++++-- internal/git2go/merge.go | 21 ++++++++++++++++++ 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 }{ @@ -122,6 +139,25 @@ func TestMerge_trees(t *testing.T) { CommitID: "7d5ae8fb6d2b301c53560bd728004d77778998df", }, }, + { + 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{ @@ -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 } -- cgit v1.2.3