git: Drop `CmdStream` parameter from `SafeCmdWithoutRepo`

Before we had variable command options, we refactored some of our
functions to accept a `CmdStream` parameter. This is confusing nowaday
as there is now multiple ways to pass standard streams to commands, once
via the parameter and once via `WithStdout()` and the likes.

Let's drop the parameter in favor of using options to avoid the
confusion and to unify interfaces.

7 files changed, 16 insertions, 9 deletions

diff --git a/internal/git/objectpool/clone.go b/internal/git/objectpool/clone.go
index af9b00688..f9330610d 100644
--- a/internal/git/objectpool/clone.go
+++ b/internal/git/objectpool/clone.go
@@ -17,7 +17,7 @@ func (o *ObjectPool) clone(ctx context.Context, repo *gitalypb.Repository) error
 		return err
 	}
 
-	cmd, err := git.SafeCmdWithoutRepo(ctx, git.CmdStream{}, nil,
+	cmd, err := git.SafeCmdWithoutRepo(ctx, nil,
 		git.SubCmd{
 			Name: "clone",
 			Flags: []git.Option{
diff --git a/internal/git/objectpool/pool.go b/internal/git/objectpool/pool.go
index bf73f2929..df9e7c285 100644
--- a/internal/git/objectpool/pool.go
+++ b/internal/git/objectpool/pool.go
@@ -124,7 +124,7 @@ func (o *ObjectPool) Init(ctx context.Context) (err error) {
 		return nil
 	}
 
-	cmd, err := git.SafeCmdWithoutRepo(ctx, git.CmdStream{}, nil,
+	cmd, err := git.SafeCmdWithoutRepo(ctx, nil,
 		git.SubCmd{
 			Name: "init",
 			Flags: []git.Option{
diff --git a/internal/git/safecmd.go b/internal/git/safecmd.go
index 24638d4e7..541ce54d1 100644
--- a/internal/git/safecmd.go
+++ b/internal/git/safecmd.go
@@ -366,7 +366,7 @@ func SafeStdinCmd(ctx context.Context, repo repository.GitRepo, globals []Option
 
 // SafeCmdWithoutRepo works like Command but without a git repository. It
 // validates the arguments in the command before executing.
-func SafeCmdWithoutRepo(ctx context.Context, stream CmdStream, globals []Option, sc SubCmd, opts ...CmdOpt) (*command.Command, error) {
+func SafeCmdWithoutRepo(ctx context.Context, globals []Option, sc SubCmd, opts ...CmdOpt) (*command.Command, error) {
 	cc := &cmdCfg{}
 
 	if err := handleOpts(ctx, sc, cc, opts); err != nil {
@@ -378,7 +378,11 @@ func SafeCmdWithoutRepo(ctx context.Context, stream CmdStream, globals []Option,
 		return nil, err
 	}
 
-	return NewCommandFactory().unsafeBareCmd(ctx, stream, cc.env, args...)
+	return NewCommandFactory().unsafeBareCmd(ctx, CmdStream{
+		In:  cc.stdin,
+		Out: cc.stdout,
+		Err: cc.stderr,
+	}, cc.env, args...)
 }
 
 func combineArgs(globals []Option, sc Cmd, cc *cmdCfg) (_ []string, err error) {
diff --git a/internal/git/safecmd_test.go b/internal/git/safecmd_test.go
index 8838056fc..94f7c6920 100644
--- a/internal/git/safecmd_test.go
+++ b/internal/git/safecmd_test.go
@@ -230,7 +230,7 @@ func TestSafeCmdValid(t *testing.T) {
 			// ignore first indeterministic arg (executable path)
 			require.Equal(t, tt.expectArgs, cmd.Args()[1:])
 
-			cmd, err = SafeCmdWithoutRepo(ctx, CmdStream{}, tt.globals, tt.subCmd, opts...)
+			cmd, err = SafeCmdWithoutRepo(ctx, tt.globals, tt.subCmd, opts...)
 			require.NoError(t, err)
 			require.Equal(t, tt.expectArgs, cmd.Args()[1:])
 
diff --git a/internal/gitaly/service/remote/remotes.go b/internal/gitaly/service/remote/remotes.go
index 074969d08..7afdec9d2 100644
--- a/internal/gitaly/service/remote/remotes.go
+++ b/internal/gitaly/service/remote/remotes.go
@@ -75,7 +75,7 @@ func (s *server) FindRemoteRepository(ctx context.Context, req *gitalypb.FindRem
 		return nil, status.Error(codes.InvalidArgument, "FindRemoteRepository: empty remote can't be checked.")
 	}
 
-	cmd, err := git.SafeCmdWithoutRepo(ctx, git.CmdStream{}, nil,
+	cmd, err := git.SafeCmdWithoutRepo(ctx, nil,
 		git.SubCmd{
 			Name: "ls-remote",
 			Args: []string{
diff --git a/internal/gitaly/service/repository/create.go b/internal/gitaly/service/repository/create.go
index b9fc42d3a..861d6aafe 100644
--- a/internal/gitaly/service/repository/create.go
+++ b/internal/gitaly/service/repository/create.go
@@ -21,7 +21,7 @@ func (s *server) CreateRepository(ctx context.Context, req *gitalypb.CreateRepos
 	}
 
 	stderr := &bytes.Buffer{}
-	cmd, err := git.SafeCmdWithoutRepo(ctx, git.CmdStream{Err: stderr}, nil,
+	cmd, err := git.SafeCmdWithoutRepo(ctx, nil,
 		git.SubCmd{
 			Name: "init",
 			Flags: []git.Option{
@@ -30,6 +30,7 @@ func (s *server) CreateRepository(ctx context.Context, req *gitalypb.CreateRepos
 			},
 			Args: []string{diskPath},
 		},
+		git.WithStderr(stderr),
 	)
 	if err != nil {
 		return nil, helper.ErrInternalf("create git init: %w", err)
diff --git a/internal/gitaly/service/repository/create_from_bundle.go b/internal/gitaly/service/repository/create_from_bundle.go
index a92486fdc..f6224535b 100644
--- a/internal/gitaly/service/repository/create_from_bundle.go
+++ b/internal/gitaly/service/repository/create_from_bundle.go
@@ -71,7 +71,7 @@ func (s *server) CreateRepositoryFromBundle(stream gitalypb.RepositoryService_Cr
 	}
 
 	stderr := bytes.Buffer{}
-	cmd, err := git.SafeCmdWithoutRepo(ctx, git.CmdStream{Err: &stderr}, nil,
+	cmd, err := git.SafeCmdWithoutRepo(ctx, nil,
 		git.SubCmd{
 			Name: "clone",
 			Flags: []git.Option{
@@ -80,6 +80,7 @@ func (s *server) CreateRepositoryFromBundle(stream gitalypb.RepositoryService_Cr
 			},
 			PostSepArgs: []string{bundlePath, repoPath},
 		},
+		git.WithStderr(&stderr),
 		git.WithRefTxHook(ctx, repo, s.cfg),
 	)
 	if err != nil {
@@ -93,13 +94,14 @@ func (s *server) CreateRepositoryFromBundle(stream gitalypb.RepositoryService_Cr
 
 	// We do a fetch to get all refs including keep-around refs
 	stderr.Reset()
-	cmd, err = git.SafeCmdWithoutRepo(ctx, git.CmdStream{Err: &stderr},
+	cmd, err = git.SafeCmdWithoutRepo(ctx,
 		[]git.Option{git.ValueFlag{Name: "-C", Value: repoPath}},
 		git.SubCmd{
 			Name:  "fetch",
 			Flags: []git.Option{git.Flag{Name: "--quiet"}},
 			Args:  []string{bundlePath, "refs/*:refs/*"},
 		},
+		git.WithStderr(&stderr),
 		git.WithRefTxHook(ctx, repo, s.cfg),
 	)
 	if err != nil {