Apply the git.SafeCmd for the Git log wrapper

Closes: https://gitlab.com/gitlab-org/gitaly/issues/1936
Closes: https://gitlab.com/gitlab-org/gitaly/issues/1935

5 files changed, 25 insertions, 37 deletions

diff --git a/internal/git/log/last_commit.go b/internal/git/log/last_commit.go
index 0d9c4ca8b..76b3c9915 100644
--- a/internal/git/log/last_commit.go
+++ b/internal/git/log/last_commit.go
@@ -4,8 +4,6 @@ import (
 	"context"
 	"io/ioutil"
 
-	grpc_logrus "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus"
-	log "github.com/sirupsen/logrus"
 	"gitlab.com/gitlab-org/gitaly/internal/command"
 	"gitlab.com/gitlab-org/gitaly/internal/git"
 	"gitlab.com/gitlab-org/gitaly/internal/git/catfile"
@@ -15,7 +13,11 @@ import (
 
 // LastCommitForPath returns the last commit which modified path.
 func LastCommitForPath(ctx context.Context, batch *catfile.Batch, repo *gitalypb.Repository, revision string, path string) (*gitalypb.GitCommit, error) {
-	cmd, err := git.Command(ctx, repo, "log", "--format=%H", "--max-count=1", revision, "--", path)
+	cmd, err := git.SafeCmd(ctx, repo, nil, git.SubCmd{
+		Name:        "log",
+		Flags:       []git.Option{git.Flag{"--format=%H"}, git.Flag{"--max-count=1"}},
+		Args:        []string{revision},
+		PostSepArgs: []string{path}})
 	if err != nil {
 		return nil, err
 	}
@@ -29,26 +31,11 @@ func LastCommitForPath(ctx context.Context, batch *catfile.Batch, repo *gitalypb
 }
 
 // GitLogCommand returns a Command that executes git log with the given the arguments
-func GitLogCommand(ctx context.Context, repo *gitalypb.Repository, revisions []string, paths []string, extraArgs ...string) (*command.Command, error) {
-	grpc_logrus.Extract(ctx).WithFields(log.Fields{
-		"Revisions": revisions,
-	}).Debug("GitLog")
-
-	formatFlag := "--pretty=%H"
-
-	args := []string{
-		"log",
-		formatFlag,
-	}
-	args = append(args, extraArgs...)
-	args = append(args, revisions...)
-	args = append(args, "--")
-	args = append(args, paths...)
-
-	cmd, err := git.Command(ctx, repo, args...)
-	if err != nil {
-		return nil, err
-	}
-
-	return cmd, nil
+func GitLogCommand(ctx context.Context, repo *gitalypb.Repository, revisions []string, paths []string, extraArgs ...git.Option) (*command.Command, error) {
+	return git.SafeCmd(ctx, repo, nil, git.SubCmd{
+		Name:        "log",
+		Flags:       append([]git.Option{git.Flag{"--pretty=%H"}}, extraArgs...),
+		Args:        revisions,
+		PostSepArgs: paths,
+	})
 }
diff --git a/internal/service/commit/between.go b/internal/service/commit/between.go
index b6cc18dec..2f6d666eb 100644
--- a/internal/service/commit/between.go
+++ b/internal/service/commit/between.go
@@ -30,7 +30,7 @@ func (s *server) CommitsBetween(in *gitalypb.CommitsBetweenRequest, stream gital
 	sender := &commitsBetweenSender{stream: stream}
 	revisionRange := fmt.Sprintf("%s..%s", in.GetFrom(), in.GetTo())
 
-	if err := sendCommits(stream.Context(), sender, in.GetRepository(), []string{revisionRange}, nil, "--reverse"); err != nil {
+	if err := sendCommits(stream.Context(), sender, in.GetRepository(), []string{revisionRange}, nil, git.Flag{"--reverse"}); err != nil {
 		return helper.ErrInternal(err)
 	}
 
diff --git a/internal/service/commit/commits_by_message.go b/internal/service/commit/commits_by_message.go
index 915a186dc..877cabca0 100644
--- a/internal/service/commit/commits_by_message.go
+++ b/internal/service/commit/commits_by_message.go
@@ -38,15 +38,15 @@ func commitsByMessage(in *gitalypb.CommitsByMessageRequest, stream gitalypb.Comm
 	ctx := stream.Context()
 	sender := &commitsByMessageSender{stream: stream}
 
-	gitLogExtraOptions := []string{
-		"--grep=" + in.GetQuery(),
-		"--regexp-ignore-case",
+	gitLogExtraOptions := []git.Option{
+		git.Flag{"--grep=" + in.GetQuery()},
+		git.Flag{"--regexp-ignore-case"},
 	}
 	if offset := in.GetOffset(); offset > 0 {
-		gitLogExtraOptions = append(gitLogExtraOptions, fmt.Sprintf("--skip=%d", offset))
+		gitLogExtraOptions = append(gitLogExtraOptions, git.Flag{fmt.Sprintf("--skip=%d", offset)})
 	}
 	if limit := in.GetLimit(); limit > 0 {
-		gitLogExtraOptions = append(gitLogExtraOptions, fmt.Sprintf("--max-count=%d", limit))
+		gitLogExtraOptions = append(gitLogExtraOptions, git.Flag{fmt.Sprintf("--max-count=%d", limit)})
 	}
 
 	revision := in.GetRevision()
diff --git a/internal/service/commit/commits_helper.go b/internal/service/commit/commits_helper.go
index a1ac32cc5..d18f44fec 100644
--- a/internal/service/commit/commits_helper.go
+++ b/internal/service/commit/commits_helper.go
@@ -4,12 +4,13 @@ import (
 	"context"
 
 	grpc_logrus "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus"
+	"gitlab.com/gitlab-org/gitaly/internal/git"
 	"gitlab.com/gitlab-org/gitaly/internal/git/log"
 	"gitlab.com/gitlab-org/gitaly/internal/helper/chunk"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 )
 
-func sendCommits(ctx context.Context, sender chunk.Sender, repo *gitalypb.Repository, revisionRange []string, paths []string, extraArgs ...string) error {
+func sendCommits(ctx context.Context, sender chunk.Sender, repo *gitalypb.Repository, revisionRange []string, paths []string, extraArgs ...git.Option) error {
 	cmd, err := log.GitLogCommand(ctx, repo, revisionRange, paths, extraArgs...)
 	if err != nil {
 		return err
diff --git a/internal/service/commit/find_all_commits.go b/internal/service/commit/find_all_commits.go
index 8f717d90e..0ce2a20b8 100644
--- a/internal/service/commit/find_all_commits.go
+++ b/internal/service/commit/find_all_commits.go
@@ -63,20 +63,20 @@ func validateFindAllCommitsRequest(in *gitalypb.FindAllCommitsRequest) error {
 func findAllCommits(in *gitalypb.FindAllCommitsRequest, stream gitalypb.CommitService_FindAllCommitsServer, revisions []string) error {
 	sender := &findAllCommitsSender{stream: stream}
 
-	var gitLogExtraOptions []string
+	var gitLogExtraOptions []git.Option
 	if maxCount := in.GetMaxCount(); maxCount > 0 {
-		gitLogExtraOptions = append(gitLogExtraOptions, fmt.Sprintf("--max-count=%d", maxCount))
+		gitLogExtraOptions = append(gitLogExtraOptions, git.Flag{fmt.Sprintf("--max-count=%d", maxCount)})
 	}
 	if skip := in.GetSkip(); skip > 0 {
-		gitLogExtraOptions = append(gitLogExtraOptions, fmt.Sprintf("--skip=%d", skip))
+		gitLogExtraOptions = append(gitLogExtraOptions, git.Flag{fmt.Sprintf("--skip=%d", skip)})
 	}
 	switch in.GetOrder() {
 	case gitalypb.FindAllCommitsRequest_NONE:
 		// Do nothing
 	case gitalypb.FindAllCommitsRequest_DATE:
-		gitLogExtraOptions = append(gitLogExtraOptions, "--date-order")
+		gitLogExtraOptions = append(gitLogExtraOptions, git.Flag{"--date-order"})
 	case gitalypb.FindAllCommitsRequest_TOPO:
-		gitLogExtraOptions = append(gitLogExtraOptions, "--topo-order")
+		gitLogExtraOptions = append(gitLogExtraOptions, git.Flag{"--topo-order"})
 	}
 
 	return sendCommits(stream.Context(), sender, in.GetRepository(), revisions, nil, gitLogExtraOptions...)