Remove more log parsing

3 files changed, 24 insertions, 59 deletions

diff --git a/internal/git/log/commit.go b/internal/git/log/commit.go
index 1f1772c0b..4746dbcf0 100644
--- a/internal/git/log/commit.go
+++ b/internal/git/log/commit.go
@@ -25,6 +25,10 @@ func GetCommit(ctx context.Context, repo *pb.Repository, revision string) (*pb.G
 		return nil, err
 	}
 
+	return getCommitCatfile(c, revision)
+}
+
+func getCommitCatfile(c *catfile.Batch, revision string) (*pb.GitCommit, error) {
 	info, err := c.Info(revision)
 	if err != nil {
 		if catfile.IsNotFound(err) {
@@ -113,6 +117,8 @@ func parseRawCommit(raw []byte, info *catfile.ObjectInfo) (*pb.GitCommit, error)
 	return commit, nil
 }
 
+const maxUnixCommitDate = 1 << 53
+
 func parseCommitAuthor(line string) (*pb.CommitAuthor, error) {
 	author := &pb.CommitAuthor{}
 
@@ -132,9 +138,10 @@ func parseCommitAuthor(line string) (*pb.CommitAuthor, error) {
 	author.Email = []byte(splitEmail[0])
 
 	sec, err := strconv.ParseInt(strings.Fields(splitEmail[1])[0], 10, 64)
-	if err != nil {
+	if err != nil || sec > maxUnixCommitDate {
 		sec = git.FallbackTimeValue.Unix()
 	}
+
 	author.Date = &timestamp.Timestamp{Seconds: sec}
 
 	return author, nil
diff --git a/internal/git/log/lastcommit.go b/internal/git/log/lastcommit.go
index 00477ecc7..582650f57 100644
--- a/internal/git/log/lastcommit.go
+++ b/internal/git/log/lastcommit.go
@@ -14,19 +14,6 @@ import (
 	log "github.com/sirupsen/logrus"
 )
 
-var commitLogFormatFields = []string{
-	"%H",  // commit hash
-	"%an", // author name
-	"%ae", // author email
-	"%aI", // author date, strict ISO 8601 format
-	"%cn", // committer name
-	"%ce", // committer email
-	"%cI", // committer date, strict ISO 8601 format
-	"%P",  // parent hashes
-}
-
-const fieldDelimiterGitFormatString = "%x1f"
-
 // LastCommitForPath returns the last commit which modified path.
 func LastCommitForPath(ctx context.Context, repo *pb.Repository, revision string, path string) (*pb.GitCommit, error) {
 	cmd, err := git.Command(ctx, repo, "log", "--format=%H", "--max-count=1", revision, "--", path)
@@ -48,11 +35,10 @@ func GitLogCommand(ctx context.Context, repo *pb.Repository, revisions []string,
 		"Revisions": revisions,
 	}).Debug("GitLog")
 
-	formatFlag := "--pretty=format:" + strings.Join(commitLogFormatFields, fieldDelimiterGitFormatString)
+	formatFlag := "--pretty=%H"
 
 	args := []string{
 		"log",
-		"-z", // use 0x00 as the entry terminator (instead of \n)
 		formatFlag,
 	}
 	args = append(args, extraArgs...)
diff --git a/internal/git/log/log.go b/internal/git/log/log.go
index 62efabfda..b3196f68a 100644
--- a/internal/git/log/log.go
+++ b/internal/git/log/log.go
@@ -2,37 +2,32 @@ package log
 
 import (
 	"bufio"
-	"bytes"
 	"context"
 	"fmt"
 	"io"
-	"strings"
 
 	pb "gitlab.com/gitlab-org/gitaly-proto/go"
-	"gitlab.com/gitlab-org/gitaly/internal/git"
+	"gitlab.com/gitlab-org/gitaly/internal/git/catfile"
 )
 
 // Parser holds necessary state for parsing a git log stream
 type Parser struct {
-	reader        *bufio.Reader
+	scanner       *bufio.Scanner
 	currentCommit *pb.GitCommit
-	finished      bool
 	err           error
-	*commitMessageHelper
+	c             *catfile.Batch
 }
 
-const fieldDelimiter = "\x1f"
-
 // NewLogParser returns a new Parser
 func NewLogParser(ctx context.Context, repo *pb.Repository, src io.Reader) (*Parser, error) {
-	cmh, err := newCommitMessageHelper(ctx, repo)
+	c, err := catfile.New(ctx, repo)
 	if err != nil {
 		return nil, err
 	}
 
 	parser := &Parser{
-		reader:              bufio.NewReader(src),
-		commitMessageHelper: cmh,
+		scanner: bufio.NewScanner(src),
+		c:       c,
 	}
 
 	return parser, nil
@@ -42,47 +37,20 @@ func NewLogParser(ctx context.Context, repo *pb.Repository, src io.Reader) (*Par
 // parsing all logs or when it encounters an error, in which case use Parser.Err()
 // to get the error.
 func (parser *Parser) Parse() bool {
-	if parser.finished {
+	if !parser.scanner.Scan() || parser.err != nil {
 		return false
 	}
 
-	line, err := parser.reader.ReadBytes('\x00')
-	if err != nil && err != io.EOF {
-		parser.err = err
-	} else if err == io.EOF {
-		parser.finished = true
-	}
+	commitId := parser.scanner.Text()
 
-	if len(line) == 0 {
-		return false
-	}
-
-	if line[len(line)-1] == '\x00' {
-		line = line[:len(line)-1] // strip off the null byte
-	}
-
-	elements := bytes.Split(line, []byte(fieldDelimiter))
-	if len(elements) != len(commitLogFormatFields) {
-		parser.err = fmt.Errorf("error parsing ref: %q", line)
-		return false
-	}
-
-	subject, body, err := parser.commitMessage(string(elements[0]))
+	commit, err := getCommitCatfile(parser.c, commitId)
 	if err != nil {
 		parser.err = err
 		return false
 	}
 
-	var parentIds []string
-	if parentFieldIndex := len(commitLogFormatFields) - 1; len(elements[parentFieldIndex]) > 0 {
-		parentIds = strings.Split(string(elements[parentFieldIndex]), " ")
-	}
-
-	commit, err := git.NewCommit(elements[0], []byte(subject), []byte(body),
-		elements[1], elements[2], elements[3], elements[4], elements[5],
-		elements[6], parentIds...)
-	if err != nil {
-		parser.err = err
+	if commit == nil {
+		parser.err = fmt.Errorf("could not retrieve commit %q", commitId)
 		return false
 	}
 
@@ -99,5 +67,9 @@ func (parser *Parser) Commit() *pb.GitCommit {
 // Err returns the error encountered (if any) when parsing the diff stream. It should be called only when Parser.Parse()
 // returns false.
 func (parser *Parser) Err() error {
+	if parser.err == nil {
+		parser.err = parser.scanner.Err()
+	}
+
 	return parser.err
 }