Add end-of-options to supported commands

5 files changed, 27 insertions, 21 deletions

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 08d05b215..9243413ef 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -124,19 +124,11 @@ test:go1.14-git-2.24-ruby-2.6:
   image: registry.gitlab.com/gitlab-org/gitlab-build-images:ruby-2.6-golang-1.14-git-2.24
   <<: *test_definition
 
-test:go1.14-git-2.22-ruby-2.6:
-  image: registry.gitlab.com/gitlab-org/gitlab-build-images:ruby-2.6-golang-1.14-git-2.26
-  <<: *test_definition
-
 test:go1.13-git-2.26-ruby-2.6:
   image: registry.gitlab.com/gitlab-org/gitlab-build-images:ruby-2.6-golang-1.13-git-2.26
   <<: *test_definition
 
 test:go1.13-git-2.24-ruby-2.6:
-  image: registry.gitlab.com/gitlab-org/gitlab-build-images:ruby-2.6-golang-1.13-git-2.22
-  <<: *test_definition
-
-test:go1.13-git-2.22-ruby-2.6:
   image: registry.gitlab.com/gitlab-org/gitlab-build-images:ruby-2.6-golang-1.13-git-2.24
   <<: *test_definition
 
diff --git a/changelogs/unreleased/po-end-of-options.yml b/changelogs/unreleased/po-end-of-options.yml
new file mode 100644
index 000000000..1b2d85e0a
--- /dev/null
+++ b/changelogs/unreleased/po-end-of-options.yml
@@ -0,0 +1,5 @@
+---
+title: Add end-of-options to supported commands
+merge_request: 2192
+author:
+type: added
diff --git a/internal/git/safecmd.go b/internal/git/safecmd.go
index 6906c9b55..d60fb076b 100644
--- a/internal/git/safecmd.go
+++ b/internal/git/safecmd.go
@@ -59,6 +59,14 @@ var subCmdNameRegex = regexp.MustCompile(`^[[:alnum:]]+(-[[:alnum:]]+)*$`)
 // IsCmd allows SubCmd to satisfy the Cmd interface
 func (sc SubCmd) IsCmd() {}
 
+func (sc SubCmd) supportsEndOfOptions() bool {
+	switch sc.Name {
+	case "linguist", "for-each-ref", "archive", "upload-archive", "grep", "clone", "config", "rev-parse", "remote", "blame", "ls-tree":
+		return false
+	}
+	return true
+}
+
 // ValidateArgs checks all arguments in the sub command and validates them
 func (sc SubCmd) ValidateArgs() ([]string, error) {
 	var safeArgs []string
@@ -83,6 +91,10 @@ func (sc SubCmd) ValidateArgs() ([]string, error) {
 		safeArgs = append(safeArgs, a)
 	}
 
+	if sc.supportsEndOfOptions() {
+		safeArgs = append(safeArgs, "--end-of-options")
+	}
+
 	if len(sc.PostSepArgs) > 0 {
 		safeArgs = append(safeArgs, "--")
 	}
diff --git a/internal/git/safecmd_test.go b/internal/git/safecmd_test.go
index 257a5646d..07f742bcc 100644
--- a/internal/git/safecmd_test.go
+++ b/internal/git/safecmd_test.go
@@ -125,6 +125,8 @@ func TestSafeCmdValid(t *testing.T) {
 	reenableGitCmd := disableGitCmd()
 	defer reenableGitCmd()
 
+	endOfOptions := "--end-of-options"
+
 	for _, tt := range []struct {
 		globals    []git.Option
 		subCmd     git.SubCmd
@@ -132,14 +134,14 @@ func TestSafeCmdValid(t *testing.T) {
 	}{
 		{
 			subCmd:     git.SubCmd{Name: "meow"},
-			expectArgs: []string{"meow"},
+			expectArgs: []string{"meow", endOfOptions},
 		},
 		{
 			globals: []git.Option{
 				git.Flag{"--aaaa-bbbb"},
 			},
 			subCmd:     git.SubCmd{Name: "cccc"},
-			expectArgs: []string{"--aaaa-bbbb", "cccc"},
+			expectArgs: []string{"--aaaa-bbbb", "cccc", endOfOptions},
 		},
 		{
 			subCmd: git.SubCmd{
@@ -147,7 +149,7 @@ func TestSafeCmdValid(t *testing.T) {
 				Args:        []string{""},
 				PostSepArgs: []string{"-woof", ""},
 			},
-			expectArgs: []string{"meow", "", "--", "-woof", ""},
+			expectArgs: []string{"meow", "", endOfOptions, "--", "-woof", ""},
 		},
 		{
 			globals: []git.Option{
@@ -164,7 +166,7 @@ func TestSafeCmdValid(t *testing.T) {
 				Args:        []string{"1", "2"},
 				PostSepArgs: []string{"3", "4", "5"},
 			},
-			expectArgs: []string{"-a", "-b", "c", "d", "-e", "-f", "g", "-h=i", "1", "2", "--", "3", "4", "5"},
+			expectArgs: []string{"-a", "-b", "c", "d", "-e", "-f", "g", "-h=i", "1", "2", endOfOptions, "--", "3", "4", "5"},
 		},
 		{
 			subCmd: git.SubCmd{
@@ -175,7 +177,7 @@ func TestSafeCmdValid(t *testing.T) {
 					git.Flag{"--adjective"},
 				},
 			},
-			expectArgs: []string{"noun", "verb", "-", "--adjective"},
+			expectArgs: []string{"noun", "verb", "-", "--adjective", endOfOptions},
 		},
 		{
 			globals: []git.Option{
@@ -190,7 +192,7 @@ func TestSafeCmdValid(t *testing.T) {
 					git.ValueFlag{"--why", "looking-for-first-contribution"},
 				},
 			},
-			expectArgs: []string{"--contributing", "--author", "a-gopher", "accept", "--is-important", "--why", "looking-for-first-contribution", "mr"},
+			expectArgs: []string{"--contributing", "--author", "a-gopher", "accept", "--is-important", "--why", "looking-for-first-contribution", "mr", endOfOptions},
 		},
 	} {
 		cmd, err := git.SafeCmd(ctx, testRepo, tt.globals, tt.subCmd)
diff --git a/internal/service/smarthttp/upload_pack_test.go b/internal/service/smarthttp/upload_pack_test.go
index 26d078846..780039a0e 100644
--- a/internal/service/smarthttp/upload_pack_test.go
+++ b/internal/service/smarthttp/upload_pack_test.go
@@ -164,13 +164,8 @@ func TestUploadPackRequestWithGitConfigOptions(t *testing.T) {
 	response, err = makePostUploadPackRequest(ctx, t, serverSocketPath, rpcRequest, requestBodyCopy)
 	testhelper.RequireGrpcError(t, err, codes.Unavailable)
 
-	// Remove the if clause if support is dropped for Git versions before 2.22
-	if git.NoMissingWantErrMessage() {
-		assert.Equal(t, "", response.String(), "Ref is hidden so no response should be received")
-	} else {
-		expected := fmt.Sprintf("0049ERR upload-pack: not our ref %v", want)
-		assert.Equal(t, expected, response.String(), "Ref is hidden, expected error message did not appear")
-	}
+	expected := fmt.Sprintf("0049ERR upload-pack: not our ref %v", want)
+	assert.Equal(t, expected, response.String(), "Ref is hidden, expected error message did not appear")
 }
 
 func TestUploadPackRequestWithGitProtocol(t *testing.T) {