Merge branch 'bvl-validate-committer-on-apply-patch' into 'master'

Fix incorrect committer when committing patches

Closes #1379

See merge request gitlab-org/gitaly!947

5 files changed, 26 insertions, 12 deletions

diff --git a/changelogs/unreleased/bvl-validate-committer-on-apply-patch.yml b/changelogs/unreleased/bvl-validate-committer-on-apply-patch.yml
new file mode 100644
index 000000000..108085cd1
--- /dev/null
+++ b/changelogs/unreleased/bvl-validate-committer-on-apply-patch.yml
@@ -0,0 +1,5 @@
+---
+title: Fix incorrect committer when committing patches
+merge_request: 947
+author:
+type: fixed
diff --git a/internal/service/operations/apply_patch_test.go b/internal/service/operations/apply_patch_test.go
index d5604047c..dfa08ed69 100644
--- a/internal/service/operations/apply_patch_test.go
+++ b/internal/service/operations/apply_patch_test.go
@@ -11,6 +11,7 @@ import (
 
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly-proto/go/gitalypb"
+	"gitlab.com/gitlab-org/gitaly/internal/git/log"
 	"gitlab.com/gitlab-org/gitaly/internal/service/operations"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/streamio"
@@ -111,19 +112,29 @@ func TestSuccessfulUserApplyPatch(t *testing.T) {
 				testRepoPath,
 				"log",
 				testCase.branchName,
-				"--format=%s",
+				"--format=%H",
 				maxCount,
 				"--reverse",
 			}
 
 			output := testhelper.MustRunCommand(t, nil, "git", gitArgs...)
-			commitMessages := strings.Split(string(output), "\n")
+			shas := strings.Split(string(output), "\n")
 			// Throw away the last element, as that's going to be
 			// an empty string.
-			if len(commitMessages) > 0 {
-				commitMessages = commitMessages[:len(commitMessages)-1]
+			if len(shas) > 0 {
+				shas = shas[:len(shas)-1]
 			}
-			require.Equal(t, commitMessages, testCase.commitMessages)
+
+			for index, sha := range shas {
+				commit, err := log.GetCommit(ctx, testRepo, sha)
+				require.NoError(t, err)
+
+				require.NotNil(t, commit)
+				require.Equal(t, string(commit.Subject), testCase.commitMessages[index])
+				require.Equal(t, string(commit.Author.Email), "patchuser@gitlab.org")
+				require.Equal(t, string(commit.Committer.Email), string(user.Email))
+			}
+
 		})
 	}
 }
diff --git a/internal/service/operations/testdata/0001-A-commit-from-a-patch.patch b/internal/service/operations/testdata/0001-A-commit-from-a-patch.patch
index 88a714c63..cc38682a0 100644
--- a/internal/service/operations/testdata/0001-A-commit-from-a-patch.patch
+++ b/internal/service/operations/testdata/0001-A-commit-from-a-patch.patch
@@ -1,5 +1,5 @@
 From 3fee0042e610fb3563e4379e316704cb1210f3de Mon Sep 17 00:00:00 2001
-From: Bob Van Landuyt <bob@vanlanduyt.co>
+From: Patch User <patchuser@gitlab.org>
 Date: Thu, 18 Oct 2018 13:40:35 +0200
 Subject: [PATCH] A commit from a patch
 
@@ -15,6 +15,5 @@ index 3742e48..e40a3b9 100644
  Sample repo for testing gitlab features
 +
 +This was applied in a patch!
--- 
+--
 2.19.1
-
diff --git a/internal/service/operations/testdata/0001-This-does-not-apply-to-the-feature-branch.patch b/internal/service/operations/testdata/0001-This-does-not-apply-to-the-feature-branch.patch
index 1abcb0e1e..905002ae8 100644
--- a/internal/service/operations/testdata/0001-This-does-not-apply-to-the-feature-branch.patch
+++ b/internal/service/operations/testdata/0001-This-does-not-apply-to-the-feature-branch.patch
@@ -1,5 +1,5 @@
 From 00c68c2b4f954370ce82a1162bc29c13f524897e Mon Sep 17 00:00:00 2001
-From: Bob Van Landuyt <bob@vanlanduyt.co>
+From: Patch User <patchuser@gitlab.org>
 Date: Mon, 22 Oct 2018 11:05:48 +0200
 Subject: [PATCH] This does not apply to the `feature` branch
 
@@ -19,6 +19,5 @@ index 0000000..fef26e4
 +    puts 'foo'
 +  end
 +end
--- 
+--
 2.19.1
-
diff --git a/ruby/lib/gitlab/git/repository.rb b/ruby/lib/gitlab/git/repository.rb
index 2faca7ad1..22723943e 100644
--- a/ruby/lib/gitlab/git/repository.rb
+++ b/ruby/lib/gitlab/git/repository.rb
@@ -500,7 +500,7 @@ module Gitlab
         env = git_env.merge(extra_env)
 
         with_worktree(worktree_path, start_point, env: env) do
-          result, status = run_git(%w[am --quiet --3way], chdir: worktree_path) do |stdin|
+          result, status = run_git(%w[am --quiet --3way], chdir: worktree_path, env: env) do |stdin|
             loop { stdin.write(patches.next) }
           end