git: Allow git-rev-list(1) to mix revs and pseudo-revs

The git-rev-list(1) command is used to compute a list of objects based
on a set of revisions passed in by the user. Next to passing normal
revisions, it also allows pseudo-revisions like "--not" or "--all" which
have some special logic and may influence normal revisions which follow
after the given pseudo-revision. But because they start with a leading
dash and we refuse positional arguments with dashes, we haven't been
able to use their full potential yet.

With the new per-command validation of positional arguments, this
shortcoming can now easily be fixed: while we keep normal validation of
positional arguments in place, we do override it for "--all" and "--not"
such that they're allowed in positional arguments when git-rev-list(1)
executes. They're harmless, and being able to mix them with normal
revisions will allow better interfaces for us in the future.

2 files changed, 58 insertions, 0 deletions

diff --git a/internal/git/command_description.go b/internal/git/command_description.go
index 158420a7e..72d4b837c 100644
--- a/internal/git/command_description.go
+++ b/internal/git/command_description.go
@@ -155,6 +155,24 @@ var commandDescriptions = map[string]commandDescription{
 	},
 	"rev-list": {
 		flags: scNoRefUpdates,
+		validatePositionalArgs: func(args []string) error {
+			for _, arg := range args {
+				// git-rev-list(1) supports pseudo-revision arguments which can be
+				// intermingled with normal positional arguments. Given that these
+				// pseudo-revisions have leading dashes, normal validation would
+				// refuse them as positional arguments. We thus override validation
+				// for two of these which we are using in our codebase. There are
+				// more, but we can add them at a later point if they're ever
+				// required.
+				if arg == "--all" || arg == "--not" {
+					continue
+				}
+				if err := validatePositionalArg(arg); err != nil {
+					return fmt.Errorf("rev-list: %w", err)
+				}
+			}
+			return nil
+		},
 	},
 	"rev-parse": {
 		flags: scNoRefUpdates | scNoEndOfOptions,
diff --git a/internal/git/command_description_test.go b/internal/git/command_description_test.go
index eae4820e4..a0915b9d5 100644
--- a/internal/git/command_description_test.go
+++ b/internal/git/command_description_test.go
@@ -1,6 +1,7 @@
 package git
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -15,3 +16,42 @@ func TestCommandDescriptions_mayGeneratePackfiles(t *testing.T) {
 	require.True(t, ok)
 	require.False(t, applyDescription.mayGeneratePackfiles())
 }
+
+func TestCommandDescriptions_revListPositionalArgs(t *testing.T) {
+	revlist, ok := commandDescriptions["rev-list"]
+	require.True(t, ok)
+	require.NotNil(t, revlist.validatePositionalArgs)
+
+	for _, tc := range []struct {
+		desc        string
+		args        []string
+		expectedErr error
+	}{
+		{
+			desc: "normal reference",
+			args: []string{
+				"master",
+			},
+		},
+		{
+			desc: "reference with leading dash",
+			args: []string{
+				"-master",
+			},
+			expectedErr: fmt.Errorf("rev-list: %w",
+				fmt.Errorf("positional arg \"-master\" cannot start with dash '-': %w", ErrInvalidArg),
+			),
+		},
+		{
+			desc: "revisions and pseudo-revisions",
+			args: []string{
+				"master --not --all",
+			},
+		},
+	} {
+		t.Run(tc.desc, func(t *testing.T) {
+			err := revlist.validatePositionalArgs(tc.args)
+			require.Equal(t, tc.expectedErr, err)
+		})
+	}
+}