git: Drop `SubSubCmd` structure

In the `git` package we have both a `SubCmd` and a `SubSubCmd` structure
which both implement the `Cmd` interface. This abstraction is arguably
not worth it: the only thing that is different between both is that the
`SubSubCmd` also has an associated "action", like it is present in `git
remote set-url`. We thus duplicate quite a bunch of logic only just to
be able to handle this additional action.

Let's drop this abstraction and instead just add the `Action` field to
the `git.SubCmd` structure. If it's set we'll add it to the computed
arguments. If unset, we retain the current behaviour.

10 files changed, 23 insertions, 59 deletions

diff --git a/internal/git/command.go b/internal/git/command.go
index 89d60ad85..06360cdee 100644
--- a/internal/git/command.go
+++ b/internal/git/command.go
@@ -12,6 +12,8 @@ var (
 	// ErrHookPayloadRequired indicates a HookPayload is needed but
 	// absent from the command.
 	ErrHookPayloadRequired = errors.New("hook payload is required but not configured")
+
+	actionRegex = regexp.MustCompile(`^[[:alnum:]]+[-[:alnum:]]*$`)
 )
 
 // Cmd is an interface for safe git commands
@@ -24,6 +26,8 @@ type Cmd interface {
 type SubCmd struct {
 	// Name is the name of the Git command to run, e.g. "log", "cat-flie" or "worktree".
 	Name string
+	// Action is the action of the Git command, e.g. "set-url" in `git remote set-url`
+	Action string
 	// Flags is the number of optional flags to pass before positional arguments, e.g.
 	// `--oneline` or `--format=fuller`.
 	Flags []Option
@@ -53,52 +57,12 @@ func (sc SubCmd) CommandArgs() ([]string, error) {
 	}
 	safeArgs = append(safeArgs, sc.Name)
 
-	commandArgs, err := commandDescription.args(sc.Flags, sc.Args, sc.PostSepArgs)
-	if err != nil {
-		return nil, err
-	}
-	safeArgs = append(safeArgs, commandArgs...)
-
-	return safeArgs, nil
-}
-
-// SubSubCmd is a positional argument that appears in the list of options for
-// a subcommand.
-type SubSubCmd struct {
-	// Name is the name of the subcommand, e.g. "remote" in `git remote set-url`
-	Name string
-	// Action is the action of the subcommand, e.g. "set-url" in `git remote set-url`
-	Action string
-
-	// Flags are optional flags before the positional args
-	Flags []Option
-	// Args are positional arguments after all flags
-	Args []string
-	// PostSepArgs are positional args after the "--" separator
-	PostSepArgs []string
-}
-
-// Subcommand returns the name of the given git command which this SubSubCmd
-// executes. E.g. for `git remote add`, it would return "remote".
-func (sc SubSubCmd) Subcommand() string { return sc.Name }
-
-var actionRegex = regexp.MustCompile(`^[[:alnum:]]+[-[:alnum:]]*$`)
-
-// CommandArgs checks all arguments in the SubSubCommand and validates them,
-// returning the array of all arguments required to execute it.
-func (sc SubSubCmd) CommandArgs() ([]string, error) {
-	var safeArgs []string
-
-	commandDescription, ok := commandDescriptions[sc.Name]
-	if !ok {
-		return nil, fmt.Errorf("invalid sub command name %q: %w", sc.Name, ErrInvalidArg)
-	}
-	safeArgs = append(safeArgs, sc.Name)
-
-	if !actionRegex.MatchString(sc.Action) {
-		return nil, fmt.Errorf("invalid sub command action %q: %w", sc.Action, ErrInvalidArg)
+	if sc.Action != "" {
+		if !actionRegex.MatchString(sc.Action) {
+			return nil, fmt.Errorf("invalid action %q: %w", sc.Action, ErrInvalidArg)
+		}
+		safeArgs = append(safeArgs, sc.Action)
 	}
-	safeArgs = append(safeArgs, sc.Action)
 
 	commandArgs, err := commandDescription.args(sc.Flags, sc.Args, sc.PostSepArgs)
 	if err != nil {
diff --git a/internal/git/housekeeping/commit_graph.go b/internal/git/housekeeping/commit_graph.go
index 709bbbbe1..898fadda6 100644
--- a/internal/git/housekeeping/commit_graph.go
+++ b/internal/git/housekeeping/commit_graph.go
@@ -69,7 +69,7 @@ func WriteCommitGraph(ctx context.Context, repo *localrepo.Repo, cfg WriteCommit
 	}
 
 	var stderr bytes.Buffer
-	if err := repo.ExecAndWait(ctx, git.SubSubCmd{
+	if err := repo.ExecAndWait(ctx, git.SubCmd{
 		Name:   "commit-graph",
 		Action: "write",
 		Flags:  flags,
diff --git a/internal/git/housekeeping/worktrees.go b/internal/git/housekeeping/worktrees.go
index f3e413fe0..af1f12863 100644
--- a/internal/git/housekeeping/worktrees.go
+++ b/internal/git/housekeeping/worktrees.go
@@ -98,7 +98,7 @@ var errUnknownWorktree = errors.New("unknown worktree")
 
 func removeWorktree(ctx context.Context, repo *localrepo.Repo, name string) error {
 	var stderr bytes.Buffer
-	err := repo.ExecAndWait(ctx, git.SubSubCmd{
+	err := repo.ExecAndWait(ctx, git.SubCmd{
 		Name:   "worktree",
 		Action: "remove",
 		Flags:  []git.Option{git.Flag{Name: "--force"}},
@@ -161,7 +161,7 @@ func cleanDisconnectedWorktrees(ctx context.Context, repo *localrepo.Repo) error
 		return nil
 	}
 
-	return repo.ExecAndWait(ctx, git.SubSubCmd{
+	return repo.ExecAndWait(ctx, git.SubCmd{
 		Name:   "worktree",
 		Action: "prune",
 	}, git.WithRefTxHook(repo))
diff --git a/internal/git/objectpool/fetch.go b/internal/git/objectpool/fetch.go
index 2818d152c..54451c0c9 100644
--- a/internal/git/objectpool/fetch.go
+++ b/internal/git/objectpool/fetch.go
@@ -127,7 +127,7 @@ func (o *ObjectPool) pruneReferences(ctx context.Context, origin *localrepo.Repo
 	// Instead we ask for a dry-run, parse the output and queue up every reference into a
 	// git-update-ref(1) process. While ugly, it works around the performance issues.
 	prune, err := o.Repo.Exec(ctx,
-		git.SubSubCmd{
+		git.SubCmd{
 			Name:   "remote",
 			Action: "prune",
 			Args:   []string{"origin"},
diff --git a/internal/gitaly/service/operations/apply_patch.go b/internal/gitaly/service/operations/apply_patch.go
index bbe2e20f2..6f90027ba 100644
--- a/internal/gitaly/service/operations/apply_patch.go
+++ b/internal/gitaly/service/operations/apply_patch.go
@@ -212,7 +212,7 @@ func (s *Server) addWorktree(ctx context.Context, repo *localrepo.Repo, worktree
 	}
 
 	var stderr bytes.Buffer
-	if err := repo.ExecAndWait(ctx, git.SubSubCmd{
+	if err := repo.ExecAndWait(ctx, git.SubCmd{
 		Name:   "worktree",
 		Action: "add",
 		Flags:  flags,
@@ -226,7 +226,7 @@ func (s *Server) addWorktree(ctx context.Context, repo *localrepo.Repo, worktree
 
 func (s *Server) removeWorktree(ctx context.Context, repo *gitalypb.Repository, worktreeName string) error {
 	cmd, err := s.gitCmdFactory.New(ctx, repo,
-		git.SubSubCmd{
+		git.SubCmd{
 			Name:   "worktree",
 			Action: "remove",
 			Flags:  []git.Option{git.Flag{Name: "--force"}},
diff --git a/internal/gitaly/service/remote/find_remote_root_ref.go b/internal/gitaly/service/remote/find_remote_root_ref.go
index 1b47094a1..bf2086536 100644
--- a/internal/gitaly/service/remote/find_remote_root_ref.go
+++ b/internal/gitaly/service/remote/find_remote_root_ref.go
@@ -46,7 +46,7 @@ func (s *server) findRemoteRootRefCmd(ctx context.Context, request *gitalypb.Fin
 	}
 
 	return s.gitCmdFactory.New(ctx, request.Repository,
-		git.SubSubCmd{
+		git.SubCmd{
 			Name:   "remote",
 			Action: "show",
 			Args:   []string{"inmemory"},
diff --git a/internal/gitaly/service/repository/create_bundle.go b/internal/gitaly/service/repository/create_bundle.go
index 768ae199e..f4573c29a 100644
--- a/internal/gitaly/service/repository/create_bundle.go
+++ b/internal/gitaly/service/repository/create_bundle.go
@@ -22,7 +22,7 @@ func (s *server) CreateBundle(req *gitalypb.CreateBundleRequest, stream gitalypb
 		return helper.ErrInternalf("running Cleanup on repository: %w", err)
 	}
 
-	cmd, err := s.gitCmdFactory.New(ctx, repository, git.SubSubCmd{
+	cmd, err := s.gitCmdFactory.New(ctx, repository, git.SubCmd{
 		Name:   "bundle",
 		Action: "create",
 		Flags:  []git.Option{git.OutputToStdout, git.Flag{Name: "--all"}},
diff --git a/internal/gitaly/service/repository/create_bundle_from_ref_list.go b/internal/gitaly/service/repository/create_bundle_from_ref_list.go
index f01be7867..1e483b2c7 100644
--- a/internal/gitaly/service/repository/create_bundle_from_ref_list.go
+++ b/internal/gitaly/service/repository/create_bundle_from_ref_list.go
@@ -49,7 +49,7 @@ func (s *server) CreateBundleFromRefList(stream gitalypb.RepositoryService_Creat
 
 	repo := s.localrepo(repository)
 	cmd, err := repo.Exec(ctx,
-		git.SubSubCmd{
+		git.SubCmd{
 			Name:   "bundle",
 			Action: "create",
 			Flags: []git.Option{
diff --git a/internal/gitaly/service/repository/fetch_bundle.go b/internal/gitaly/service/repository/fetch_bundle.go
index 52d885e57..452f86d16 100644
--- a/internal/gitaly/service/repository/fetch_bundle.go
+++ b/internal/gitaly/service/repository/fetch_bundle.go
@@ -106,7 +106,7 @@ func (s *server) updateHeadFromBundle(ctx context.Context, repo *localrepo.Repo,
 // findBundleHead tries to extract HEAD and its target from a bundle. Returns
 // nil when HEAD is not found.
 func (s *server) findBundleHead(ctx context.Context, repo git.RepositoryExecutor, bundlePath string) (*git.Reference, error) {
-	cmd, err := repo.Exec(ctx, git.SubSubCmd{
+	cmd, err := repo.Exec(ctx, git.SubCmd{
 		Name:   "bundle",
 		Action: "list-heads",
 		Args:   []string{bundlePath, "HEAD"},
diff --git a/internal/gitaly/service/repository/midx.go b/internal/gitaly/service/repository/midx.go
index 7a460bc8b..c74a46d00 100644
--- a/internal/gitaly/service/repository/midx.go
+++ b/internal/gitaly/service/repository/midx.go
@@ -65,7 +65,7 @@ func (s *server) safeMidxCommand(ctx context.Context, repo repository.GitRepo, c
 
 func (s *server) midxWrite(ctx context.Context, repo repository.GitRepo) error {
 	cmd, err := s.gitCmdFactory.New(ctx, repo,
-		git.SubSubCmd{
+		git.SubCmd{
 			Name:   "multi-pack-index",
 			Action: "write",
 		},
@@ -100,7 +100,7 @@ func (s *server) midxVerify(ctx context.Context, repo repository.GitRepo) error
 	ctxlogger := ctxlogrus.Extract(ctx)
 
 	cmd, err := s.gitCmdFactory.New(ctx, repo,
-		git.SubSubCmd{
+		git.SubCmd{
 			Name:   "multi-pack-index",
 			Action: "verify",
 		},
@@ -136,7 +136,7 @@ func (s *server) midxRewrite(ctx context.Context, repo repository.GitRepo) error
 
 func (s *server) midxExpire(ctx context.Context, repo repository.GitRepo) error {
 	cmd, err := s.gitCmdFactory.New(ctx, repo,
-		git.SubSubCmd{
+		git.SubCmd{
 			Name:   "multi-pack-index",
 			Action: "expire",
 		},
@@ -177,7 +177,7 @@ func (s *server) midxRepack(ctx context.Context, repo repository.GitRepo) error
 	// will only be respected if git version is >=2.28.0.
 	// Bitmap index 'repack.writeBitmaps' is not yet supported.
 	cmd, err := s.gitCmdFactory.New(ctx, repo,
-		git.SubSubCmd{
+		git.SubCmd{
 			Name:   "multi-pack-index",
 			Action: "repack",
 			Flags: []git.Option{