Merge branch 'ps-git-cmd-factory-remote' into 'master'

Replacement of the git.NewCommandWithoutRepo in remote package

See merge request gitlab-org/gitaly!3070

7 files changed, 23 insertions, 16 deletions

diff --git a/internal/gitaly/service/register.go b/internal/gitaly/service/register.go
index 5cfc3f8d4..318892d13 100644
--- a/internal/gitaly/service/register.go
+++ b/internal/gitaly/service/register.go
@@ -90,7 +90,7 @@ func RegisterAll(grpcServer *grpc.Server, cfg config.Cfg, rubyServer *rubyserver
 	))
 	gitalypb.RegisterWikiServiceServer(grpcServer, wiki.NewServer(rubyServer, locator))
 	gitalypb.RegisterConflictsServiceServer(grpcServer, conflicts.NewServer(rubyServer, cfg, locator))
-	gitalypb.RegisterRemoteServiceServer(grpcServer, remote.NewServer(cfg, rubyServer, locator))
+	gitalypb.RegisterRemoteServiceServer(grpcServer, remote.NewServer(cfg, rubyServer, locator, gitCmdFactory))
 	gitalypb.RegisterServerServiceServer(grpcServer, server.NewServer(cfg.Storages))
 	gitalypb.RegisterObjectPoolServiceServer(grpcServer, objectpool.NewServer(cfg, locator, gitCmdFactory))
 	gitalypb.RegisterHookServiceServer(grpcServer, hook.NewServer(cfg, hookManager))
diff --git a/internal/gitaly/service/remote/fetch_internal_remote.go b/internal/gitaly/service/remote/fetch_internal_remote.go
index 321be53b0..84bfc20e8 100644
--- a/internal/gitaly/service/remote/fetch_internal_remote.go
+++ b/internal/gitaly/service/remote/fetch_internal_remote.go
@@ -33,7 +33,7 @@ func (s *server) FetchInternalRemote(ctx context.Context, req *gitalypb.FetchInt
 	}
 
 	stderr := &bytes.Buffer{}
-	cmd, err := git.NewCommand(ctx, req.Repository, nil,
+	cmd, err := s.gitCmdFactory.New(ctx, req.Repository, nil,
 		git.SubCmd{
 			Name:  "fetch",
 			Flags: []git.Option{git.Flag{Name: "--prune"}},
diff --git a/internal/gitaly/service/remote/find_remote_root_ref.go b/internal/gitaly/service/remote/find_remote_root_ref.go
index c87c8f7a1..abcd68f4b 100644
--- a/internal/gitaly/service/remote/find_remote_root_ref.go
+++ b/internal/gitaly/service/remote/find_remote_root_ref.go
@@ -14,8 +14,8 @@ import (
 
 const headPrefix = "HEAD branch: "
 
-func findRemoteRootRef(ctx context.Context, repo *gitalypb.Repository, remote string) (string, error) {
-	cmd, err := git.NewCommand(ctx, repo, nil,
+func (s *server) findRemoteRootRef(ctx context.Context, repo *gitalypb.Repository, remote string) (string, error) {
+	cmd, err := s.gitCmdFactory.New(ctx, repo, nil,
 		git.SubSubCmd{
 			Name:   "remote",
 			Action: "show",
@@ -54,7 +54,7 @@ func (s *server) FindRemoteRootRef(ctx context.Context, in *gitalypb.FindRemoteR
 		return nil, status.Error(codes.InvalidArgument, "empty remote can't be queried")
 	}
 
-	ref, err := findRemoteRootRef(ctx, in.GetRepository(), remote)
+	ref, err := s.findRemoteRootRef(ctx, in.GetRepository(), remote)
 	if err != nil {
 		if _, ok := status.FromError(err); ok {
 			return nil, err
diff --git a/internal/gitaly/service/remote/remotes.go b/internal/gitaly/service/remote/remotes.go
index 7cb229c55..b127f55d2 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.NewCommandWithoutRepo(ctx, nil,
+	cmd, err := s.gitCmdFactory.NewWithoutRepo(ctx, nil,
 		git.SubCmd{
 			Name: "ls-remote",
 			Args: []string{
@@ -118,7 +118,7 @@ func (s *server) ListRemotes(req *gitalypb.ListRemotesRequest, stream gitalypb.R
 	repo := req.GetRepository()
 
 	ctx := stream.Context()
-	cmd, err := git.NewCommand(ctx, repo, nil, git.SubCmd{Name: "remote", Flags: []git.Option{git.Flag{Name: "-v"}}},
+	cmd, err := s.gitCmdFactory.New(ctx, repo, nil, git.SubCmd{Name: "remote", Flags: []git.Option{git.Flag{Name: "-v"}}},
 		git.WithRefTxHook(ctx, repo, config.Config),
 	)
 	if err != nil {
diff --git a/internal/gitaly/service/remote/server.go b/internal/gitaly/service/remote/server.go
index ef48bbf9a..05ddd7122 100644
--- a/internal/gitaly/service/remote/server.go
+++ b/internal/gitaly/service/remote/server.go
@@ -2,6 +2,7 @@ package remote
 
 import (
 	"gitlab.com/gitlab-org/gitaly/client"
+	"gitlab.com/gitlab-org/gitaly/internal/git"
 	"gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
 	"gitlab.com/gitlab-org/gitaly/internal/gitaly/rubyserver"
 	"gitlab.com/gitlab-org/gitaly/internal/storage"
@@ -9,19 +10,21 @@ import (
 )
 
 type server struct {
-	cfg     config.Cfg
-	ruby    *rubyserver.Server
-	locator storage.Locator
+	cfg           config.Cfg
+	ruby          *rubyserver.Server
+	locator       storage.Locator
+	gitCmdFactory git.CommandFactory
 
 	conns *client.Pool
 }
 
 // NewServer creates a new instance of a grpc RemoteServiceServer
-func NewServer(cfg config.Cfg, rs *rubyserver.Server, locator storage.Locator) gitalypb.RemoteServiceServer {
+func NewServer(cfg config.Cfg, rs *rubyserver.Server, locator storage.Locator, gitCmdFactory git.CommandFactory) gitalypb.RemoteServiceServer {
 	return &server{
-		cfg:     cfg,
-		ruby:    rs,
-		locator: locator,
+		cfg:           cfg,
+		ruby:          rs,
+		locator:       locator,
+		gitCmdFactory: gitCmdFactory,
 		conns: client.NewPoolWithOptions(
 			client.WithDialer(client.HealthCheckDialer(client.DialContext)),
 			client.WithDialOptions(client.FailOnNonTempDialError()...),
diff --git a/internal/gitaly/service/remote/testhelper_test.go b/internal/gitaly/service/remote/testhelper_test.go
index c90669e73..e6480b920 100644
--- a/internal/gitaly/service/remote/testhelper_test.go
+++ b/internal/gitaly/service/remote/testhelper_test.go
@@ -5,6 +5,7 @@ import (
 	"testing"
 
 	log "github.com/sirupsen/logrus"
+	"gitlab.com/gitlab-org/gitaly/internal/git"
 	"gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
 	"gitlab.com/gitlab-org/gitaly/internal/gitaly/rubyserver"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
@@ -38,7 +39,10 @@ func testMain(m *testing.M) int {
 func RunRemoteServiceServer(t *testing.T, opts ...testhelper.TestServerOpt) (string, func()) {
 	srv := testhelper.NewServer(t, nil, nil, opts...)
 
-	gitalypb.RegisterRemoteServiceServer(srv.GrpcServer(), NewServer(config.Config, RubyServer, config.NewLocator(config.Config)))
+	cfg := config.Config
+	locator := config.NewLocator(cfg)
+	gitCmdFactory := git.NewExecCommandFactory(cfg)
+	gitalypb.RegisterRemoteServiceServer(srv.GrpcServer(), NewServer(cfg, RubyServer, locator, gitCmdFactory))
 	reflection.Register(srv.GrpcServer())
 
 	srv.Start(t)
diff --git a/internal/praefect/replicator_test.go b/internal/praefect/replicator_test.go
index 4cc9e41dd..6db17aa5d 100644
--- a/internal/praefect/replicator_test.go
+++ b/internal/praefect/replicator_test.go
@@ -1036,7 +1036,7 @@ func newReplicationService(tb testing.TB) (*grpc.Server, string) {
 	gitCmdFactory := git.NewExecCommandFactory(gitaly_config.Config)
 	gitalypb.RegisterRepositoryServiceServer(svr, repository.NewServer(gitaly_config.Config, RubyServer, locator, gitCmdFactory))
 	gitalypb.RegisterObjectPoolServiceServer(svr, objectpoolservice.NewServer(gitaly_config.Config, locator, gitCmdFactory))
-	gitalypb.RegisterRemoteServiceServer(svr, remote.NewServer(gitaly_config.Config, RubyServer, locator))
+	gitalypb.RegisterRemoteServiceServer(svr, remote.NewServer(gitaly_config.Config, RubyServer, locator, gitCmdFactory))
 	gitalypb.RegisterSSHServiceServer(svr, ssh.NewServer(gitaly_config.Config, locator))
 	gitalypb.RegisterRefServiceServer(svr, ref.NewServer(gitaly_config.Config, locator))
 	healthpb.RegisterHealthServer(svr, health.NewServer())