Merge branch 'find-commit-catfile' into 'master'

Use 'git cat-file' to retrieve commits

See merge request gitlab-org/gitaly!771

28 files changed, 378 insertions, 341 deletions

diff --git a/changelogs/unreleased/find-commit-catfile.yml b/changelogs/unreleased/find-commit-catfile.yml
new file mode 100644
index 000000000..f00e5afe0
--- /dev/null
+++ b/changelogs/unreleased/find-commit-catfile.yml
@@ -0,0 +1,5 @@
+---
+title: Use 'git cat-file' to retrieve commits
+merge_request: 771
+author:
+type: performance
diff --git a/internal/git/helper.go b/internal/git/helper.go
index 64d242c0d..ba4e476de 100644
--- a/internal/git/helper.go
+++ b/internal/git/helper.go
@@ -8,10 +8,7 @@ import (
 	"strings"
 	"time"
 
-	"github.com/golang/protobuf/ptypes/timestamp"
-	pb "gitlab.com/gitlab-org/gitaly-proto/go"
 	"gitlab.com/gitlab-org/gitaly/internal/command"
-	"gitlab.com/gitlab-org/gitaly/internal/helper"
 )
 
 // FallbackTimeValue is the value returned by `SafeTimeParse` in case it
@@ -39,54 +36,6 @@ func ValidateRevision(revision []byte) error {
 	return nil
 }
 
-// SafeTimeParse parses a git date string with the RFC3339 format. If the date
-// is invalid (possibly because the date is larger than golang's largest value)
-// it returns the maximum date possible.
-func SafeTimeParse(timeStr string) time.Time {
-	t, err := time.Parse(time.RFC3339, timeStr)
-	if err != nil {
-		return FallbackTimeValue
-	}
-
-	return t
-}
-
-// NewCommit creates a commit based on the given elements
-func NewCommit(id, subject, body, authorName, authorEmail, authorDate,
-	committerName, committerEmail, committerDate []byte, parentIds ...string) (*pb.GitCommit, error) {
-	authorDateTime := SafeTimeParse(string(authorDate))
-	committerDateTime := SafeTimeParse(string(committerDate))
-
-	author := pb.CommitAuthor{
-		Name:  authorName,
-		Email: authorEmail,
-		Date:  &timestamp.Timestamp{Seconds: authorDateTime.Unix()},
-	}
-	committer := pb.CommitAuthor{
-		Name:  committerName,
-		Email: committerEmail,
-		Date:  &timestamp.Timestamp{Seconds: committerDateTime.Unix()},
-	}
-
-	commit := &pb.GitCommit{
-		Id:        string(id),
-		Subject:   subject,
-		Author:    &author,
-		Committer: &committer,
-		ParentIds: parentIds,
-		BodySize:  int64(len(body)),
-	}
-
-	bodyLengthToSend := len(body)
-	if threshold := helper.MaxCommitOrTagMessageSize; bodyLengthToSend > threshold {
-		bodyLengthToSend = threshold
-	}
-
-	commit.Body = body[:bodyLengthToSend]
-
-	return commit, nil
-}
-
 // Version returns the used git version.
 func Version() (string, error) {
 	ctx, cancel := context.WithCancel(context.Background())
diff --git a/internal/git/log/commit.go b/internal/git/log/commit.go
index eea40cfc2..ddb093834 100644
--- a/internal/git/log/commit.go
+++ b/internal/git/log/commit.go
@@ -1,77 +1,138 @@
 package log
 
 import (
+	"bufio"
+	"bytes"
 	"context"
+	"io/ioutil"
+	"strconv"
 	"strings"
 
-	"gitlab.com/gitlab-org/gitaly/internal/command"
-	"gitlab.com/gitlab-org/gitaly/internal/git"
-
 	pb "gitlab.com/gitlab-org/gitaly-proto/go"
+	"gitlab.com/gitlab-org/gitaly/internal/git"
+	"gitlab.com/gitlab-org/gitaly/internal/git/catfile"
+	"gitlab.com/gitlab-org/gitaly/internal/helper"
 
-	"github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus"
-	log "github.com/sirupsen/logrus"
+	"github.com/golang/protobuf/ptypes/timestamp"
 )
 
-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
+// GetCommit tries to resolve revision to a Git commit. Returns nil if
+// no object is found at revision.
+func GetCommit(ctx context.Context, repo *pb.Repository, revision string) (*pb.GitCommit, error) {
+	c, err := catfile.New(ctx, repo)
+	if err != nil {
+		return nil, err
+	}
+
+	return GetCommitCatfile(c, revision)
 }
 
-const fieldDelimiterGitFormatString = "%x1f"
+// GetCommitCatfile looks up a commit by revision using an existing *catfile.Batch instance.
+func GetCommitCatfile(c *catfile.Batch, revision string) (*pb.GitCommit, error) {
+	info, err := c.Info(revision + "^{commit}")
+	if err != nil {
+		if catfile.IsNotFound(err) {
+			return nil, nil
+		}
 
-// GetCommit returns a single GitCommit
-func GetCommit(ctx context.Context, repo *pb.Repository, revision string, path string) (*pb.GitCommit, error) {
-	paths := []string{}
-	if len(path) > 0 {
-		paths = append(paths, path)
+		return nil, err
 	}
 
-	cmd, err := GitLogCommand(ctx, repo, []string{revision}, paths, "--max-count=1")
+	r, err := c.Commit(info.Oid)
 	if err != nil {
 		return nil, err
 	}
 
-	logParser, err := NewLogParser(ctx, repo, cmd)
+	raw, err := ioutil.ReadAll(r)
 	if err != nil {
 		return nil, err
 	}
 
-	if ok := logParser.Parse(); !ok {
-		return nil, logParser.Err()
+	return parseRawCommit(raw, info)
+}
+
+func parseRawCommit(raw []byte, info *catfile.ObjectInfo) (*pb.GitCommit, error) {
+	split := bytes.SplitN(raw, []byte("\n\n"), 2)
+
+	header := split[0]
+	var body []byte
+	if len(split) == 2 {
+		body = split[1]
+	}
+
+	commit := &pb.GitCommit{
+		Id:       info.Oid,
+		Body:     body,
+		Subject:  subjectFromBody(body),
+		BodySize: int64(len(body)),
+	}
+	if max := helper.MaxCommitOrTagMessageSize; len(commit.Body) > max {
+		commit.Body = commit.Body[:max]
+	}
+
+	scanner := bufio.NewScanner(bytes.NewReader(header))
+	for scanner.Scan() {
+		line := scanner.Text()
+		if len(line) == 0 || line[0] == ' ' {
+			continue
+		}
+
+		headerSplit := strings.SplitN(line, " ", 2)
+		if len(headerSplit) != 2 {
+			continue
+		}
+
+		switch headerSplit[0] {
+		case "parent":
+			commit.ParentIds = append(commit.ParentIds, headerSplit[1])
+		case "author":
+			commit.Author = parseCommitAuthor(headerSplit[1])
+		case "committer":
+			commit.Committer = parseCommitAuthor(headerSplit[1])
+		}
+	}
+	if err := scanner.Err(); err != nil {
+		return nil, err
 	}
 
-	return logParser.Commit(), nil
+	return commit, nil
 }
 
-// GitLogCommand returns a Command that executes git log with the given the arguments
-func GitLogCommand(ctx context.Context, repo *pb.Repository, revisions []string, paths []string, extraArgs ...string) (*command.Command, error) {
-	grpc_logrus.Extract(ctx).WithFields(log.Fields{
-		"Revisions": revisions,
-	}).Debug("GitLog")
+const maxUnixCommitDate = 1 << 53
 
-	formatFlag := "--pretty=format:" + strings.Join(commitLogFormatFields, fieldDelimiterGitFormatString)
+func parseCommitAuthor(line string) *pb.CommitAuthor {
+	author := &pb.CommitAuthor{}
 
-	args := []string{
-		"log",
-		"-z", // use 0x00 as the entry terminator (instead of \n)
-		formatFlag,
+	splitName := strings.SplitN(line, "<", 2)
+	author.Name = []byte(strings.TrimSuffix(splitName[0], " "))
+
+	if len(splitName) < 2 {
+		return author
 	}
-	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
+	line = splitName[1]
+	splitEmail := strings.SplitN(line, ">", 2)
+	if len(splitEmail) < 2 {
+		return author
 	}
 
-	return cmd, nil
+	author.Email = []byte(splitEmail[0])
+
+	secSplit := strings.Fields(splitEmail[1])
+	if len(secSplit) < 1 {
+		return author
+	}
+
+	sec, err := strconv.ParseInt(secSplit[0], 10, 64)
+	if err != nil || sec > maxUnixCommitDate || sec < 0 {
+		sec = git.FallbackTimeValue.Unix()
+	}
+
+	author.Date = &timestamp.Timestamp{Seconds: sec}
+
+	return author
+}
+
+func subjectFromBody(body []byte) []byte {
+	return bytes.TrimRight(bytes.SplitN(body, []byte("\n"), 2)[0], "\r\n")
 }
diff --git a/internal/git/log/commit_test.go b/internal/git/log/commit_test.go
new file mode 100644
index 000000000..5d0b93a3b
--- /dev/null
+++ b/internal/git/log/commit_test.go
@@ -0,0 +1,101 @@
+package log
+
+import (
+	"testing"
+
+	"github.com/golang/protobuf/ptypes/timestamp"
+	"github.com/stretchr/testify/require"
+	pb "gitlab.com/gitlab-org/gitaly-proto/go"
+	"gitlab.com/gitlab-org/gitaly/internal/git/catfile"
+)
+
+func TestParseRawCommit(t *testing.T) {
+	info := &catfile.ObjectInfo{
+		Oid:  "a984dfa4dee018c6d5f5f57ffec0d0e22763df16",
+		Type: "commit",
+	}
+
+	// Valid-but-interesting commits should be test at the FindCommit level.
+	// Invalid objects (that Git would complain about during fsck) can be
+	// tested here.
+	//
+	// Once a repository contains a pathological object it can be hard to get
+	// rid of it. Because of this I think it's nicer to ignore such objects
+	// than to throw hard errors.
+	testCases := []struct {
+		desc string
+		in   []byte
+		out  *pb.GitCommit
+	}{
+		{
+			desc: "empty commit object",
+			in:   []byte{},
+			out:  &pb.GitCommit{Id: info.Oid},
+		},
+		{
+			desc: "no email",
+			in:   []byte("author Jane Doe"),
+			out: &pb.GitCommit{
+				Id:     info.Oid,
+				Author: &pb.CommitAuthor{Name: []byte("Jane Doe")},
+			},
+		},
+		{
+			desc: "unmatched <",
+			in:   []byte("author Jane Doe <janedoe@example.com"),
+			out: &pb.GitCommit{
+				Id:     info.Oid,
+				Author: &pb.CommitAuthor{Name: []byte("Jane Doe")},
+			},
+		},
+		{
+			desc: "unmatched >",
+			in:   []byte("author Jane Doe janedoe@example.com>"),
+			out: &pb.GitCommit{
+				Id:     info.Oid,
+				Author: &pb.CommitAuthor{Name: []byte("Jane Doe janedoe@example.com>")},
+			},
+		},
+		{
+			desc: "missing date",
+			in:   []byte("author Jane Doe <janedoe@example.com> "),
+			out: &pb.GitCommit{
+				Id:     info.Oid,
+				Author: &pb.CommitAuthor{Name: []byte("Jane Doe"), Email: []byte("janedoe@example.com")},
+			},
+		},
+		{
+			desc: "date too high",
+			in:   []byte("author Jane Doe <janedoe@example.com> 9007199254740993 +0200"),
+			out: &pb.GitCommit{
+				Id: info.Oid,
+				Author: &pb.CommitAuthor{
+					Name:  []byte("Jane Doe"),
+					Email: []byte("janedoe@example.com"),
+					Date:  &timestamp.Timestamp{Seconds: 9223371974719179007},
+				},
+			},
+		},
+		{
+			desc: "date negative",
+			in:   []byte("author Jane Doe <janedoe@example.com> -1 +0200"),
+			out: &pb.GitCommit{
+				Id: info.Oid,
+				Author: &pb.CommitAuthor{
+					Name:  []byte("Jane Doe"),
+					Email: []byte("janedoe@example.com"),
+					Date:  &timestamp.Timestamp{Seconds: 9223371974719179007},
+				},
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			info.Size = int64(len(tc.in))
+			out, err := parseRawCommit(tc.in, info)
+			require.NoError(t, err, "parse error")
+			require.Equal(t, *tc.out, *out)
+		})
+	}
+}
diff --git a/internal/git/log/commitmessage.go b/internal/git/log/commitmessage.go
deleted file mode 100644
index ec620d57a..000000000
--- a/internal/git/log/commitmessage.go
+++ /dev/null
@@ -1,124 +0,0 @@
-package log
-
-import (
-	"context"
-	"fmt"
-	"io/ioutil"
-	"strings"
-
-	pb "gitlab.com/gitlab-org/gitaly-proto/go"
-	"gitlab.com/gitlab-org/gitaly/internal/git/catfile"
-)
-
-type commitMessageResponse struct {
-	subject string
-	body    string
-	err     error
-}
-
-type commitMessageRequest struct {
-	commitID string
-	response chan commitMessageResponse
-}
-
-type commitMessageHelper struct {
-	ctx        context.Context
-	requests   chan commitMessageRequest
-	catFileErr chan error
-}
-
-func newCommitMessageHelper(ctx context.Context, repo *pb.Repository) (*commitMessageHelper, error) {
-	cmh := &commitMessageHelper{
-		ctx:        ctx,
-		requests:   make(chan commitMessageRequest),
-		catFileErr: make(chan error),
-	}
-
-	c, err := catfile.New(ctx, repo)
-	if err != nil {
-		return nil, err
-	}
-
-	go func() {
-		select {
-		case cmh.catFileErr <- cmh.handleCatFile(c):
-		case <-ctx.Done():
-			// This case is here to ensure this goroutine won't leak. We can't assume
-			// someone is listening on the cmh.catFileErr channel.
-		}
-
-		close(cmh.catFileErr)
-	}()
-
-	return cmh, nil
-}
-
-// commitMessage returns the raw binary subject and body for the given commitID.
-func (cmh *commitMessageHelper) commitMessage(commitID string) (string, string, error) {
-	response := make(chan commitMessageResponse)
-
-	select {
-	case cmh.requests <- commitMessageRequest{commitID: commitID, response: response}:
-		result := <-response
-		return result.subject, result.body, result.err
-	case err := <-cmh.catFileErr:
-		return "", "", fmt.Errorf("git cat-file is not running: %v", err)
-	}
-}
-
-// handleCatFile gets the raw commit message for a sequence of commit
-// ID's from a git-cat-file process.
-func (cmh *commitMessageHelper) handleCatFile(c *catfile.Batch) error {
-	for {
-		select {
-		case messageRequest := <-cmh.requests:
-			subject, body, err := getCommitMessage(c, messageRequest.commitID)
-
-			// Always return a response, because a client is blocked waiting for it.
-			messageRequest.response <- commitMessageResponse{
-				subject: subject,
-				body:    body,
-				err:     err,
-			}
-
-			if err != nil {
-				// Shut down the current goroutine.
-				return err
-			}
-		case <-cmh.ctx.Done():
-			// We need this case because we cannot count on the client to close the
-			// requests channel.
-			return cmh.ctx.Err()
-		}
-	}
-}
-
-// getCommitMessage returns subject, body, error by querying git cat-file via stdin and stdout.
-func getCommitMessage(c *catfile.Batch, commitID string) (string, string, error) {
-	objInfo, err := c.Info(commitID)
-	if err != nil {
-		return "", "", err
-	}
-
-	if objInfo.Oid == "" || objInfo.Type != "commit" {
-		return "", "", fmt.Errorf("invalid ObjectInfo for %q: %v", commitID, objInfo)
-	}
-
-	commitReader, err := c.Commit(objInfo.Oid)
-	if err != nil {
-		return "", "", err
-	}
-
-	rawCommit, err := ioutil.ReadAll(commitReader)
-	if err != nil {
-		return "", "", err
-	}
-
-	var body string
-	if split := strings.SplitN(string(rawCommit), "\n\n", 2); len(split) == 2 {
-		body = split[1]
-	}
-	subject := strings.TrimRight(strings.SplitN(body, "\n", 2)[0], "\r\n")
-
-	return subject, body, nil
-}
diff --git a/internal/git/log/last_commit.go b/internal/git/log/last_commit.go
new file mode 100644
index 000000000..950b2eefb
--- /dev/null
+++ b/internal/git/log/last_commit.go
@@ -0,0 +1,55 @@
+package log
+
+import (
+	"context"
+	"io/ioutil"
+	"strings"
+
+	"gitlab.com/gitlab-org/gitaly/internal/command"
+	"gitlab.com/gitlab-org/gitaly/internal/git"
+
+	pb "gitlab.com/gitlab-org/gitaly-proto/go"
+
+	"github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus"
+	log "github.com/sirupsen/logrus"
+)
+
+// 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)
+	if err != nil {
+		return nil, err
+	}
+
+	commitID, err := ioutil.ReadAll(cmd)
+	if err != nil {
+		return nil, err
+	}
+
+	return GetCommit(ctx, repo, strings.TrimSpace(string(commitID)))
+}
+
+// GitLogCommand returns a Command that executes git log with the given the arguments
+func GitLogCommand(ctx context.Context, repo *pb.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
+}
diff --git a/internal/git/log/log.go b/internal/git/log/log.go
index 62efabfda..da8c8f4cc 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
 }
diff --git a/internal/service/commit/find_commit.go b/internal/service/commit/find_commit.go
index 3eeb68063..2a967d2ba 100644
--- a/internal/service/commit/find_commit.go
+++ b/internal/service/commit/find_commit.go
@@ -19,6 +19,6 @@ func (s *server) FindCommit(ctx context.Context, in *pb.FindCommitRequest) (*pb.
 
 	repo := in.GetRepository()
 
-	commit, err := log.GetCommit(ctx, repo, string(revision), "")
+	commit, err := log.GetCommit(ctx, repo, string(revision))
 	return &pb.FindCommitResponse{Commit: commit}, err
 }
diff --git a/internal/service/commit/find_commit_test.go b/internal/service/commit/find_commit_test.go
index 003f53f5f..511b9c69c 100644
--- a/internal/service/commit/find_commit_test.go
+++ b/internal/service/commit/find_commit_test.go
@@ -39,7 +39,7 @@ func TestSuccessfulFindCommitRequest(t *testing.T) {
 		Message:  bigMessage,
 		ParentID: "60ecb67744cb56576c30214ff52294f8ce2def98",
 	})
-	bigCommit, err := log.GetCommit(ctx, testRepo, bigCommitID, "")
+	bigCommit, err := log.GetCommit(ctx, testRepo, bigCommitID)
 	require.NoError(t, err)
 
 	testCases := []struct {
@@ -247,6 +247,7 @@ func TestSuccessfulFindCommitRequest(t *testing.T) {
 			allCommits = append(allCommits, response.Commit)
 		})
 	}
+	require.Equal(t, len(testCases), len(allCommits), "length of allCommits")
 
 	gogitCtx := metadata.NewOutgoingContext(
 		ctx,
diff --git a/internal/service/commit/last_commit_for_path.go b/internal/service/commit/last_commit_for_path.go
index df4c4b133..ce049bf86 100644
--- a/internal/service/commit/last_commit_for_path.go
+++ b/internal/service/commit/last_commit_for_path.go
@@ -22,7 +22,7 @@ func (s *server) LastCommitForPath(ctx context.Context, in *pb.LastCommitForPath
 		path = "."
 	}
 
-	commit, err := log.GetCommit(ctx, in.GetRepository(), string(in.GetRevision()), path)
+	commit, err := log.LastCommitForPath(ctx, in.GetRepository(), string(in.GetRevision()), path)
 	if err != nil {
 		return nil, err
 	}
diff --git a/internal/service/conflicts/resolve_conflicts_test.go b/internal/service/conflicts/resolve_conflicts_test.go
index 30c3796c2..b871a48eb 100644
--- a/internal/service/conflicts/resolve_conflicts_test.go
+++ b/internal/service/conflicts/resolve_conflicts_test.go
@@ -98,7 +98,7 @@ func TestSuccessfulResolveConflictsRequest(t *testing.T) {
 	require.NoError(t, err)
 	require.Empty(t, r.GetResolutionError())
 
-	headCommit, err := log.GetCommit(ctxOuter, testRepo, sourceBranch, "")
+	headCommit, err := log.GetCommit(ctxOuter, testRepo, sourceBranch)
 	require.NoError(t, err)
 	require.Contains(t, headCommit.ParentIds, "1450cd639e0bc6721eb02800169e464f212cde06")
 	require.Contains(t, headCommit.ParentIds, "824be604a34828eb682305f0d963056cfac87b2d")
diff --git a/internal/service/operations/branches_test.go b/internal/service/operations/branches_test.go
index b6b67f921..80ffcb245 100644
--- a/internal/service/operations/branches_test.go
+++ b/internal/service/operations/branches_test.go
@@ -31,7 +31,7 @@ func TestSuccessfulUserCreateBranchRequest(t *testing.T) {
 	defer conn.Close()
 
 	startPoint := "c7fbe50c7c7419d9701eebe64b1fdacc3df5b9dd"
-	startPointCommit, err := log.GetCommit(ctx, testRepo, startPoint, "")
+	startPointCommit, err := log.GetCommit(ctx, testRepo, startPoint)
 	require.NoError(t, err)
 	user := &pb.User{
 		Name:  []byte("Alejandro Rodríguez"),
diff --git a/internal/service/operations/cherry_pick_test.go b/internal/service/operations/cherry_pick_test.go
index 6c64b4378..49ab13cc4 100644
--- a/internal/service/operations/cherry_pick_test.go
+++ b/internal/service/operations/cherry_pick_test.go
@@ -36,7 +36,7 @@ func TestSuccessfulUserCherryPickRequest(t *testing.T) {
 	destinationBranch := "cherry-picking-dst"
 	testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "branch", destinationBranch, "master")
 
-	masterHeadCommit, err := log.GetCommit(ctxOuter, testRepo, "master", "")
+	masterHeadCommit, err := log.GetCommit(ctxOuter, testRepo, "master")
 	require.NoError(t, err)
 
 	user := &pb.User{
@@ -45,7 +45,7 @@ func TestSuccessfulUserCherryPickRequest(t *testing.T) {
 		GlId:  "user-123",
 	}
 
-	cherryPickedCommit, err := log.GetCommit(ctxOuter, testRepo, "8a0f2ee90d940bfb0ba1e14e8214b0649056e4ab", "")
+	cherryPickedCommit, err := log.GetCommit(ctxOuter, testRepo, "8a0f2ee90d940bfb0ba1e14e8214b0649056e4ab")
 	require.NoError(t, err)
 
 	testRepoCopy, _, cleanup := testhelper.NewTestRepo(t)
@@ -114,7 +114,7 @@ func TestSuccessfulUserCherryPickRequest(t *testing.T) {
 			response, err := client.UserCherryPick(ctx, testCase.request)
 			require.NoError(t, err)
 
-			headCommit, err := log.GetCommit(ctx, testRepo, string(testCase.request.BranchName), "")
+			headCommit, err := log.GetCommit(ctx, testRepo, string(testCase.request.BranchName))
 			require.NoError(t, err)
 
 			expectedBranchUpdate := testCase.branchUpdate
@@ -149,7 +149,7 @@ func TestSuccessfulGitHooksForUserCherryPickRequest(t *testing.T) {
 		GlId:  "user-123",
 	}
 
-	cherryPickedCommit, err := log.GetCommit(ctxOuter, testRepo, "8a0f2ee90d940bfb0ba1e14e8214b0649056e4ab", "")
+	cherryPickedCommit, err := log.GetCommit(ctxOuter, testRepo, "8a0f2ee90d940bfb0ba1e14e8214b0649056e4ab")
 	require.NoError(t, err)
 
 	request := &pb.UserCherryPickRequest{
@@ -193,7 +193,7 @@ func TestFailedUserCherryPickRequestDueToValidations(t *testing.T) {
 	testRepo, _, cleanup := testhelper.NewTestRepo(t)
 	defer cleanup()
 
-	cherryPickedCommit, err := log.GetCommit(ctxOuter, testRepo, "8a0f2ee90d940bfb0ba1e14e8214b0649056e4ab", "")
+	cherryPickedCommit, err := log.GetCommit(ctxOuter, testRepo, "8a0f2ee90d940bfb0ba1e14e8214b0649056e4ab")
 	require.NoError(t, err)
 
 	destinationBranch := "cherry-picking-dst"
@@ -288,7 +288,7 @@ func TestFailedUserCherryPickRequestDueToPreReceiveError(t *testing.T) {
 		GlId:  "user-123",
 	}
 
-	cherryPickedCommit, err := log.GetCommit(ctxOuter, testRepo, "8a0f2ee90d940bfb0ba1e14e8214b0649056e4ab", "")
+	cherryPickedCommit, err := log.GetCommit(ctxOuter, testRepo, "8a0f2ee90d940bfb0ba1e14e8214b0649056e4ab")
 	require.NoError(t, err)
 
 	request := &pb.UserCherryPickRequest{
@@ -340,7 +340,7 @@ func TestFailedUserCherryPickRequestDueToCreateTreeError(t *testing.T) {
 	}
 
 	// This commit already exists in master
-	cherryPickedCommit, err := log.GetCommit(ctxOuter, testRepo, "4a24d82dbca5c11c61556f3b35ca472b7463187e", "")
+	cherryPickedCommit, err := log.GetCommit(ctxOuter, testRepo, "4a24d82dbca5c11c61556f3b35ca472b7463187e")
 	require.NoError(t, err)
 
 	request := &pb.UserCherryPickRequest{
@@ -383,7 +383,7 @@ func TestFailedUserCherryPickRequestDueToCommitError(t *testing.T) {
 		GlId:  "user-123",
 	}
 
-	cherryPickedCommit, err := log.GetCommit(ctxOuter, testRepo, sourceBranch, "")
+	cherryPickedCommit, err := log.GetCommit(ctxOuter, testRepo, sourceBranch)
 	require.NoError(t, err)
 
 	request := &pb.UserCherryPickRequest{
diff --git a/internal/service/operations/commit_files_test.go b/internal/service/operations/commit_files_test.go
index ccf2ac194..2928a8126 100644
--- a/internal/service/operations/commit_files_test.go
+++ b/internal/service/operations/commit_files_test.go
@@ -99,7 +99,7 @@ func TestSuccessfulUserCommitFilesRequest(t *testing.T) {
 			require.Equal(t, tc.repoCreated, r.GetBranchUpdate().GetRepoCreated())
 			require.Equal(t, tc.branchCreated, r.GetBranchUpdate().GetBranchCreated())
 
-			headCommit, err := log.GetCommit(ctxOuter, tc.repo, tc.branchName, "")
+			headCommit, err := log.GetCommit(ctxOuter, tc.repo, tc.branchName)
 			require.NoError(t, err)
 			require.Equal(t, authorName, headCommit.Author.Name)
 			require.Equal(t, user.Name, headCommit.Committer.Name)
diff --git a/internal/service/operations/merge_test.go b/internal/service/operations/merge_test.go
index fb75f3bb9..0224a5b1f 100644
--- a/internal/service/operations/merge_test.go
+++ b/internal/service/operations/merge_test.go
@@ -73,7 +73,7 @@ func TestSuccessfulMerge(t *testing.T) {
 	firstResponse, err := mergeBidi.Recv()
 	require.NoError(t, err, "receive first response")
 
-	_, err = gitlog.GetCommit(ctx, testRepo, firstResponse.CommitId, "")
+	_, err = gitlog.GetCommit(ctx, testRepo, firstResponse.CommitId)
 	require.NoError(t, err, "look up git commit before merge is applied")
 
 	require.NoError(t, mergeBidi.Send(&pb.UserMergeBranchRequest{Apply: true}), "apply merge")
@@ -87,7 +87,7 @@ func TestSuccessfulMerge(t *testing.T) {
 	})
 	require.NoError(t, err, "consume EOF")
 
-	commit, err := gitlog.GetCommit(ctx, testRepo, mergeBranchName, "")
+	commit, err := gitlog.GetCommit(ctx, testRepo, mergeBranchName)
 	require.NoError(t, err, "look up git commit after call has finished")
 
 	require.Equal(t, pb.OperationBranchUpdate{CommitId: commit.Id}, *(secondResponse.BranchUpdate))
@@ -169,7 +169,7 @@ func TestAbortedMerge(t *testing.T) {
 			require.Equal(t, "", secondResponse.GetBranchUpdate().GetCommitId(), "merge should not have been applied")
 			require.Error(t, err)
 
-			commit, err := gitlog.GetCommit(ctx, testRepo, mergeBranchName, "")
+			commit, err := gitlog.GetCommit(ctx, testRepo, mergeBranchName)
 			require.NoError(t, err, "look up git commit after call has finished")
 
 			require.Equal(t, mergeBranchHeadBefore, commit.Id, "branch should not change when the merge is aborted")
@@ -219,7 +219,7 @@ func TestFailedMergeConcurrentUpdate(t *testing.T) {
 	require.NoError(t, err, "receive second response")
 	require.Equal(t, *secondResponse, pb.UserMergeBranchResponse{}, "response should be empty")
 
-	commit, err := gitlog.GetCommit(ctx, testRepo, mergeBranchName, "")
+	commit, err := gitlog.GetCommit(ctx, testRepo, mergeBranchName)
 	require.NoError(t, err, "get commit after RPC finished")
 	require.Equal(t, commit.Id, concurrentCommitID, "RPC should not have trampled concurrent update")
 }
diff --git a/internal/service/operations/revert_test.go b/internal/service/operations/revert_test.go
index e8e32158d..fbe0f1e3c 100644
--- a/internal/service/operations/revert_test.go
+++ b/internal/service/operations/revert_test.go
@@ -33,7 +33,7 @@ func TestSuccessfulUserRevertRequest(t *testing.T) {
 	destinationBranch := "revert-dst"
 	testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "branch", destinationBranch, "master")
 
-	masterHeadCommit, err := log.GetCommit(ctxOuter, testRepo, "master", "")
+	masterHeadCommit, err := log.GetCommit(ctxOuter, testRepo, "master")
 	require.NoError(t, err)
 
 	user := &pb.User{
@@ -42,7 +42,7 @@ func TestSuccessfulUserRevertRequest(t *testing.T) {
 		GlId:  "user-123",
 	}
 
-	revertedCommit, err := log.GetCommit(ctxOuter, testRepo, "d59c60028b053793cecfb4022de34602e1a9218e", "")
+	revertedCommit, err := log.GetCommit(ctxOuter, testRepo, "d59c60028b053793cecfb4022de34602e1a9218e")
 	require.NoError(t, err)
 
 	testRepoCopy, _, cleanup := testhelper.NewTestRepo(t)
@@ -111,7 +111,7 @@ func TestSuccessfulUserRevertRequest(t *testing.T) {
 			response, err := client.UserRevert(ctx, testCase.request)
 			require.NoError(t, err)
 
-			headCommit, err := log.GetCommit(ctx, testRepo, string(testCase.request.BranchName), "")
+			headCommit, err := log.GetCommit(ctx, testRepo, string(testCase.request.BranchName))
 			require.NoError(t, err)
 
 			expectedBranchUpdate := testCase.branchUpdate
@@ -146,7 +146,7 @@ func TestSuccessfulGitHooksForUserRevertRequest(t *testing.T) {
 		GlId:  "user-123",
 	}
 
-	revertedCommit, err := log.GetCommit(ctxOuter, testRepo, "d59c60028b053793cecfb4022de34602e1a9218e", "")
+	revertedCommit, err := log.GetCommit(ctxOuter, testRepo, "d59c60028b053793cecfb4022de34602e1a9218e")
 	require.NoError(t, err)
 
 	request := &pb.UserRevertRequest{
@@ -190,7 +190,7 @@ func TestFailedUserRevertRequestDueToValidations(t *testing.T) {
 	testRepo, _, cleanup := testhelper.NewTestRepo(t)
 	defer cleanup()
 
-	revertedCommit, err := log.GetCommit(ctxOuter, testRepo, "d59c60028b053793cecfb4022de34602e1a9218e", "")
+	revertedCommit, err := log.GetCommit(ctxOuter, testRepo, "d59c60028b053793cecfb4022de34602e1a9218e")
 	require.NoError(t, err)
 
 	destinationBranch := "revert-dst"
@@ -285,7 +285,7 @@ func TestFailedUserRevertRequestDueToPreReceiveError(t *testing.T) {
 		GlId:  "user-123",
 	}
 
-	revertedCommit, err := log.GetCommit(ctxOuter, testRepo, "d59c60028b053793cecfb4022de34602e1a9218e", "")
+	revertedCommit, err := log.GetCommit(ctxOuter, testRepo, "d59c60028b053793cecfb4022de34602e1a9218e")
 	require.NoError(t, err)
 
 	request := &pb.UserRevertRequest{
@@ -337,7 +337,7 @@ func TestFailedUserRevertRequestDueToCreateTreeError(t *testing.T) {
 	}
 
 	// This revert patch of the following commit cannot be applied to the destinationBranch above
-	revertedCommit, err := log.GetCommit(ctxOuter, testRepo, "372ab6950519549b14d220271ee2322caa44d4eb", "")
+	revertedCommit, err := log.GetCommit(ctxOuter, testRepo, "372ab6950519549b14d220271ee2322caa44d4eb")
 	require.NoError(t, err)
 
 	request := &pb.UserRevertRequest{
@@ -380,7 +380,7 @@ func TestFailedUserRevertRequestDueToCommitError(t *testing.T) {
 		GlId:  "user-123",
 	}
 
-	revertedCommit, err := log.GetCommit(ctxOuter, testRepo, sourceBranch, "")
+	revertedCommit, err := log.GetCommit(ctxOuter, testRepo, sourceBranch)
 	require.NoError(t, err)
 
 	request := &pb.UserRevertRequest{
diff --git a/internal/service/operations/squash_test.go b/internal/service/operations/squash_test.go
index 586632b92..53781fe3e 100644
--- a/internal/service/operations/squash_test.go
+++ b/internal/service/operations/squash_test.go
@@ -54,7 +54,7 @@ func TestSuccessfulUserSquashRequest(t *testing.T) {
 	require.NoError(t, err)
 	require.Empty(t, response.GetGitError())
 
-	commit, err := log.GetCommit(ctx, testRepo, response.SquashSha, "")
+	commit, err := log.GetCommit(ctx, testRepo, response.SquashSha)
 	require.NoError(t, err)
 	require.Equal(t, commit.ParentIds, []string{startSha})
 	require.Equal(t, string(commit.Author.Email), "johndoe@gitlab.com")
diff --git a/internal/service/operations/tags_test.go b/internal/service/operations/tags_test.go
index c695a8762..c716abf85 100644
--- a/internal/service/operations/tags_test.go
+++ b/internal/service/operations/tags_test.go
@@ -116,7 +116,7 @@ func TestSuccessfulUserCreateTagRequest(t *testing.T) {
 	defer cleanupFn()
 
 	targetRevision := "c7fbe50c7c7419d9701eebe64b1fdacc3df5b9dd"
-	targetRevisionCommit, err := log.GetCommit(ctx, testRepo, targetRevision, "")
+	targetRevisionCommit, err := log.GetCommit(ctx, testRepo, targetRevision)
 	require.NoError(t, err)
 
 	user := &pb.User{
diff --git a/internal/service/ref/branches_test.go b/internal/service/ref/branches_test.go
index d1860c53f..54f386b36 100644
--- a/internal/service/ref/branches_test.go
+++ b/internal/service/ref/branches_test.go
@@ -27,11 +27,11 @@ func TestSuccessfulCreateBranchRequest(t *testing.T) {
 	testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepo(t)
 	defer cleanupFn()
 
-	headCommit, err := log.GetCommit(ctx, testRepo, "HEAD", "")
+	headCommit, err := log.GetCommit(ctx, testRepo, "HEAD")
 	require.NoError(t, err)
 
 	startPoint := "c7fbe50c7c7419d9701eebe64b1fdacc3df5b9dd"
-	startPointCommit, err := log.GetCommit(ctx, testRepo, startPoint, "")
+	startPointCommit, err := log.GetCommit(ctx, testRepo, startPoint)
 	require.NoError(t, err)
 
 	testCases := []struct {
@@ -238,7 +238,7 @@ func TestSuccessfulFindBranchRequest(t *testing.T) {
 	defer cleanupFn()
 
 	branchNameInput := "master"
-	branchTarget, err := log.GetCommit(ctx, testRepo, branchNameInput, "")
+	branchTarget, err := log.GetCommit(ctx, testRepo, branchNameInput)
 	require.NoError(t, err)
 
 	branch := &pb.Branch{
diff --git a/internal/service/ref/refs.go b/internal/service/ref/refs.go
index 756055f02..e8a62e528 100644
--- a/internal/service/ref/refs.go
+++ b/internal/service/ref/refs.go
@@ -15,6 +15,7 @@ import (
 
 	pb "gitlab.com/gitlab-org/gitaly-proto/go"
 	"gitlab.com/gitlab-org/gitaly/internal/git"
+	"gitlab.com/gitlab-org/gitaly/internal/git/catfile"
 	"gitlab.com/gitlab-org/gitaly/internal/helper/lines"
 	"gitlab.com/gitlab-org/gitaly/internal/rubyserver"
 	"golang.org/x/net/context"
@@ -222,7 +223,13 @@ func parseSortKey(sortKey pb.FindLocalBranchesRequest_SortBy) string {
 
 // FindLocalBranches creates a stream of branches for all local branches in the given repository
 func (s *server) FindLocalBranches(in *pb.FindLocalBranchesRequest, stream pb.RefService_FindLocalBranchesServer) error {
-	writer := newFindLocalBranchesWriter(stream)
+	ctx := stream.Context()
+	c, err := catfile.New(ctx, in.Repository)
+	if err != nil {
+		return err
+	}
+
+	writer := newFindLocalBranchesWriter(stream, c)
 	opts := &findRefsOpts{
 		cmdArgs: []string{
 			// %00 inserts the null character into the output (see for-each-ref docs)
@@ -231,7 +238,7 @@ func (s *server) FindLocalBranches(in *pb.FindLocalBranchesRequest, stream pb.Re
 		},
 	}
 
-	return findRefs(stream.Context(), writer, in.Repository, []string{"refs/heads"}, opts)
+	return findRefs(ctx, writer, in.Repository, []string{"refs/heads"}, opts)
 }
 
 func (s *server) FindAllBranches(in *pb.FindAllBranchesRequest, stream pb.RefService_FindAllBranchesServer) error {
@@ -262,12 +269,18 @@ func (s *server) FindAllBranches(in *pb.FindAllBranchesRequest, stream pb.RefSer
 		}
 	}
 
+	ctx := stream.Context()
+	c, err := catfile.New(ctx, in.Repository)
+	if err != nil {
+		return err
+	}
+
 	opts := &findRefsOpts{
 		cmdArgs: args,
 	}
-	writer := newFindAllBranchesWriter(stream)
+	writer := newFindAllBranchesWriter(stream, c)
 
-	return findRefs(stream.Context(), writer, in.Repository, patterns, opts)
+	return findRefs(ctx, writer, in.Repository, patterns, opts)
 }
 
 // ListBranchNamesContainingCommit returns a maximum of in.GetLimit() Branch names
diff --git a/internal/service/ref/refs_test.go b/internal/service/ref/refs_test.go
index 134e43b7c..076606ff2 100644
--- a/internal/service/ref/refs_test.go
+++ b/internal/service/ref/refs_test.go
@@ -407,7 +407,7 @@ func TestSuccessfulFindAllTagsRequest(t *testing.T) {
 		Message:  "An empty commit with REALLY BIG message\n\n" + strings.Repeat("a", helper.MaxCommitOrTagMessageSize+1),
 		ParentID: "60ecb67744cb56576c30214ff52294f8ce2def98",
 	})
-	bigCommit, err := log.GetCommit(ctx, testRepoCopy, bigCommitID, "")
+	bigCommit, err := log.GetCommit(ctx, testRepoCopy, bigCommitID)
 	require.NoError(t, err)
 
 	annotatedTagID := testhelper.CreateTag(t, testRepoCopyPath, "v1.2.0", blobID, &testhelper.CreateTagOpts{Message: "Blob tag"})
@@ -748,16 +748,19 @@ func TestSuccessfulFindAllBranchesRequest(t *testing.T) {
 	remoteBranch := &pb.FindAllBranchesResponse_Branch{
 		Name: []byte("refs/remotes/origin/fake-remote-branch"),
 		Target: &pb.GitCommit{
-			Id:      "913c66a37b4a45b9769037c55c2d238bd0942d2e",
-			Subject: []byte("Files, encoding and much more"),
+			Id:        "913c66a37b4a45b9769037c55c2d238bd0942d2e",
+			Subject:   []byte("Files, encoding and much more"),
+			Body:      []byte("Files, encoding and much more\n\nSigned-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>\n"),
+			BodySize:  98,
+			ParentIds: []string{"cfe32cf61b73a0d5e9f13e774abde7ff789b1660"},
 			Author: &pb.CommitAuthor{
 				Name:  []byte("Dmitriy Zaporozhets"),
-				Email: []byte("<dmitriy.zaporozhets@gmail.com>"),
+				Email: []byte("dmitriy.zaporozhets@gmail.com"),
 				Date:  &timestamp.Timestamp{Seconds: 1393488896},
 			},
 			Committer: &pb.CommitAuthor{
 				Name:  []byte("Dmitriy Zaporozhets"),
-				Email: []byte("<dmitriy.zaporozhets@gmail.com>"),
+				Email: []byte("dmitriy.zaporozhets@gmail.com"),
 				Date:  &timestamp.Timestamp{Seconds: 1393488896},
 			},
 		},
@@ -830,7 +833,7 @@ func TestSuccessfulFindAllBranchesRequestWithMergedBranches(t *testing.T) {
 		expectedBranches = append(expectedBranches, branch)
 	}
 
-	masterCommit, err := log.GetCommit(ctx, testRepo, "master", "")
+	masterCommit, err := log.GetCommit(ctx, testRepo, "master")
 	require.NoError(t, err)
 	expectedBranches = append(expectedBranches, &pb.FindAllBranchesResponse_Branch{
 		Name:   []byte("refs/heads/master"),
diff --git a/internal/service/ref/testhelper_test.go b/internal/service/ref/testhelper_test.go
index cfa4ac76d..2e1e6b1c3 100644
--- a/internal/service/ref/testhelper_test.go
+++ b/internal/service/ref/testhelper_test.go
@@ -22,44 +22,53 @@ import (
 var (
 	localBranches = map[string]*pb.GitCommit{
 		"refs/heads/100%branch": {
-			Id:      "1b12f15a11fc6e62177bef08f47bc7b5ce50b141",
-			Subject: []byte("Merge branch 'add-directory-with-space' into 'master'\r \r Add a directory containing a space in its name\r \r needed for verifying the fix of `https://gitlab.com/gitlab-com/support-forum/issues/952` \r \r See merge request !11"),
+			Id:        "1b12f15a11fc6e62177bef08f47bc7b5ce50b141",
+			Body:      []byte("Merge branch 'add-directory-with-space' into 'master'\r\n\r\nAdd a directory containing a space in its name\r\n\r\nneeded for verifying the fix of `https://gitlab.com/gitlab-com/support-forum/issues/952` \r\n\r\nSee merge request !11"),
+			BodySize:  221,
+			ParentIds: []string{"6907208d755b60ebeacb2e9dfea74c92c3449a1f", "38008cb17ce1466d8fec2dfa6f6ab8dcfe5cf49e"},
+			Subject:   []byte("Merge branch 'add-directory-with-space' into 'master'"),
 			Author: &pb.CommitAuthor{
 				Name:  []byte("Stan Hu"),
-				Email: []byte("<stanhu@gmail.com>"),
+				Email: []byte("stanhu@gmail.com"),
 				Date:  &timestamp.Timestamp{Seconds: 1471558878},
 			},
 			Committer: &pb.CommitAuthor{
 				Name:  []byte("Stan Hu"),
-				Email: []byte("<stanhu@gmail.com>"),
+				Email: []byte("stanhu@gmail.com"),
 				Date:  &timestamp.Timestamp{Seconds: 1471558878},
 			},
 		},
 		"refs/heads/improve/awesome": {
-			Id:      "5937ac0a7beb003549fc5fd26fc247adbce4a52e",
-			Subject: []byte("Add submodule from gitlab.com"),
+			Id:        "5937ac0a7beb003549fc5fd26fc247adbce4a52e",
+			Subject:   []byte("Add submodule from gitlab.com"),
+			Body:      []byte("Add submodule from gitlab.com\n\nSigned-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>\n"),
+			BodySize:  98,
+			ParentIds: []string{"570e7b2abdd848b95f2f578043fc23bd6f6fd24d"},
 			Author: &pb.CommitAuthor{
 				Name:  []byte("Dmitriy Zaporozhets"),
-				Email: []byte("<dmitriy.zaporozhets@gmail.com>"),
+				Email: []byte("dmitriy.zaporozhets@gmail.com"),
 				Date:  &timestamp.Timestamp{Seconds: 1393491698},
 			},
 			Committer: &pb.CommitAuthor{
 				Name:  []byte("Dmitriy Zaporozhets"),
-				Email: []byte("<dmitriy.zaporozhets@gmail.com>"),
+				Email: []byte("dmitriy.zaporozhets@gmail.com"),
 				Date:  &timestamp.Timestamp{Seconds: 1393491698},
 			},
 		},
 		"refs/heads/'test'": {
-			Id:      "e56497bb5f03a90a51293fc6d516788730953899",
-			Subject: []byte("Merge branch 'tree_helper_spec' into 'master'"),
+			Id:        "e56497bb5f03a90a51293fc6d516788730953899",
+			Subject:   []byte("Merge branch 'tree_helper_spec' into 'master'"),
+			Body:      []byte("Merge branch 'tree_helper_spec' into 'master'\n\nAdd directory structure for tree_helper spec\n\nThis directory structure is needed for a testing the method flatten_tree(tree) in the TreeHelper module\n\nSee [merge request #275](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/275#note_732774)\n\nSee merge request !2\n"),
+			BodySize:  317,
+			ParentIds: []string{"5937ac0a7beb003549fc5fd26fc247adbce4a52e", "4cd80ccab63c82b4bad16faa5193fbd2aa06df40"},
 			Author: &pb.CommitAuthor{
 				Name:  []byte("Sytse Sijbrandij"),
-				Email: []byte("<sytse@gitlab.com>"),
+				Email: []byte("sytse@gitlab.com"),
 				Date:  &timestamp.Timestamp{Seconds: 1420925009},
 			},
 			Committer: &pb.CommitAuthor{
 				Name:  []byte("Sytse Sijbrandij"),
-				Email: []byte("<sytse@gitlab.com>"),
+				Email: []byte("sytse@gitlab.com"),
 				Date:  &timestamp.Timestamp{Seconds: 1420925009},
 			},
 		},
@@ -140,7 +149,7 @@ func assertContainsBranch(t *testing.T, branches []*pb.FindAllBranchesResponse_B
 
 	for _, b := range branches {
 		if bytes.Equal(branch.Name, b.Name) {
-			require.Equal(t, branch.Target, b.Target, "mismatched targets")
+			require.Equal(t, b.Target, branch.Target, "mismatched targets")
 			return // Found the branch and it maches. Success!
 		}
 		branchNames = append(branchNames, b.Name)
diff --git a/internal/service/ref/util.go b/internal/service/ref/util.go
index 0db913136..562d5b27c 100644
--- a/internal/service/ref/util.go
+++ b/internal/service/ref/util.go
@@ -4,33 +4,25 @@ import (
 	"bytes"
 
 	pb "gitlab.com/gitlab-org/gitaly-proto/go"
-	"gitlab.com/gitlab-org/gitaly/internal/git"
+	"gitlab.com/gitlab-org/gitaly/internal/git/catfile"
+	"gitlab.com/gitlab-org/gitaly/internal/git/log"
 	"gitlab.com/gitlab-org/gitaly/internal/helper/lines"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
 )
 
-var localBranchFormatFields = []string{
-	"%(refname)", "%(objectname)", "%(contents:subject)", "%(authorname)",
-	"%(authoremail)", "%(authordate:iso-strict)", "%(committername)",
-	"%(committeremail)", "%(committerdate:iso-strict)",
-}
+var localBranchFormatFields = []string{"%(refname)", "%(objectname)"}
 
 func parseRef(ref []byte) ([][]byte, error) {
 	elements := bytes.Split(ref, []byte("\x00"))
-	if len(elements) != 9 {
+	if len(elements) != len(localBranchFormatFields) {
 		return nil, status.Errorf(codes.Internal, "error parsing ref %q", ref)
 	}
 	return elements, nil
 }
 
-func buildCommitFromBranchInfo(elements [][]byte) (*pb.GitCommit, error) {
-	return git.NewCommit(elements[0], elements[1], nil, elements[2],
-		elements[3], elements[4], elements[5], elements[6], elements[7])
-}
-
-func buildLocalBranch(elements [][]byte) (*pb.FindLocalBranchResponse, error) {
-	target, err := buildCommitFromBranchInfo(elements[1:])
+func buildLocalBranch(c *catfile.Batch, elements [][]byte) (*pb.FindLocalBranchResponse, error) {
+	target, err := log.GetCommitCatfile(c, string(elements[1]))
 	if err != nil {
 		return nil, err
 	}
@@ -54,8 +46,8 @@ func buildLocalBranch(elements [][]byte) (*pb.FindLocalBranchResponse, error) {
 	}, nil
 }
 
-func buildBranch(elements [][]byte) (*pb.FindAllBranchesResponse_Branch, error) {
-	target, err := buildCommitFromBranchInfo(elements[1:])
+func buildBranch(c *catfile.Batch, elements [][]byte) (*pb.FindAllBranchesResponse_Branch, error) {
+	target, err := log.GetCommitCatfile(c, string(elements[1]))
 	if err != nil {
 		return nil, err
 	}
@@ -78,7 +70,7 @@ func newFindAllTagNamesWriter(stream pb.Ref_FindAllTagNamesServer) lines.Sender
 	}
 }
 
-func newFindLocalBranchesWriter(stream pb.Ref_FindLocalBranchesServer) lines.Sender {
+func newFindLocalBranchesWriter(stream pb.Ref_FindLocalBranchesServer, c *catfile.Batch) lines.Sender {
 	return func(refs [][]byte) error {
 		var branches []*pb.FindLocalBranchResponse
 
@@ -87,7 +79,7 @@ func newFindLocalBranchesWriter(stream pb.Ref_FindLocalBranchesServer) lines.Sen
 			if err != nil {
 				return err
 			}
-			branch, err := buildLocalBranch(elements)
+			branch, err := buildLocalBranch(c, elements)
 			if err != nil {
 				return err
 			}
@@ -97,7 +89,7 @@ func newFindLocalBranchesWriter(stream pb.Ref_FindLocalBranchesServer) lines.Sen
 	}
 }
 
-func newFindAllBranchesWriter(stream pb.RefService_FindAllBranchesServer) lines.Sender {
+func newFindAllBranchesWriter(stream pb.RefService_FindAllBranchesServer, c *catfile.Batch) lines.Sender {
 	return func(refs [][]byte) error {
 		var branches []*pb.FindAllBranchesResponse_Branch
 
@@ -106,7 +98,7 @@ func newFindAllBranchesWriter(stream pb.RefService_FindAllBranchesServer) lines.
 			if err != nil {
 				return err
 			}
-			branch, err := buildBranch(elements)
+			branch, err := buildBranch(c, elements)
 			if err != nil {
 				return err
 			}
diff --git a/internal/service/repository/fetch_test.go b/internal/service/repository/fetch_test.go
index 9fcd6f59d..8ff989c1d 100644
--- a/internal/service/repository/fetch_test.go
+++ b/internal/service/repository/fetch_test.go
@@ -51,7 +51,7 @@ func TestFetchSourceBranchSourceRepositorySuccess(t *testing.T) {
 	require.NoError(t, err)
 	require.True(t, resp.Result, "response.Result should be true")
 
-	fetchedCommit, err := gitLog.GetCommit(ctx, targetRepo, targetRef, "")
+	fetchedCommit, err := gitLog.GetCommit(ctx, targetRepo, targetRef)
 	require.NoError(t, err)
 	require.Equal(t, newCommitID, fetchedCommit.GetId())
 }
@@ -86,7 +86,7 @@ func TestFetchSourceBranchSameRepositorySuccess(t *testing.T) {
 	require.NoError(t, err)
 	require.True(t, resp.Result, "response.Result should be true")
 
-	fetchedCommit, err := gitLog.GetCommit(ctx, repo, targetRef, "")
+	fetchedCommit, err := gitLog.GetCommit(ctx, repo, targetRef)
 	require.NoError(t, err)
 	require.Equal(t, newCommitID, fetchedCommit.GetId())
 }
diff --git a/internal/service/wiki/delete_page_test.go b/internal/service/wiki/delete_page_test.go
index 5dc4cd402..b232907e7 100644
--- a/internal/service/wiki/delete_page_test.go
+++ b/internal/service/wiki/delete_page_test.go
@@ -73,7 +73,7 @@ func TestSuccessfulWikiDeletePageRequest(t *testing.T) {
 			require.NoError(t, err)
 
 			headID := testhelper.MustRunCommand(t, nil, "git", "-C", wikiRepoPath, "show", "--format=format:%H", "--no-patch", "HEAD")
-			commit, err := gitlog.GetCommit(ctx, wikiRepo, string(headID), "")
+			commit, err := gitlog.GetCommit(ctx, wikiRepo, string(headID))
 			require.NoError(t, err, "look up git commit after deleting a wiki page")
 
 			require.Equal(t, authorName, commit.Author.Name, "author name mismatched")
diff --git a/internal/service/wiki/testhelper_test.go b/internal/service/wiki/testhelper_test.go
index 65d27c966..49acff46f 100644
--- a/internal/service/wiki/testhelper_test.go
+++ b/internal/service/wiki/testhelper_test.go
@@ -197,7 +197,7 @@ func createTestWikiPage(t *testing.T, client pb.WikiServiceClient, wikiRepo *pb.
 
 	writeWikiPage(t, client, wikiRepo, opts)
 	head1ID := testhelper.MustRunCommand(t, nil, "git", "-C", wikiRepoPath, "show", "--format=format:%H", "--no-patch", "HEAD")
-	pageCommit, err := gitlog.GetCommit(ctx, wikiRepo, string(head1ID), "")
+	pageCommit, err := gitlog.GetCommit(ctx, wikiRepo, string(head1ID))
 	require.NoError(t, err, "look up git commit after writing a wiki page")
 
 	return pageCommit
diff --git a/internal/service/wiki/update_page_test.go b/internal/service/wiki/update_page_test.go
index 66d076ee7..9e8b2303c 100644
--- a/internal/service/wiki/update_page_test.go
+++ b/internal/service/wiki/update_page_test.go
@@ -99,7 +99,7 @@ func TestSuccessfulWikiUpdatePageRequest(t *testing.T) {
 			require.NoError(t, err)
 
 			headID := testhelper.MustRunCommand(t, nil, "git", "-C", wikiRepoPath, "show", "--format=format:%H", "--no-patch", "HEAD")
-			commit, err := gitlog.GetCommit(ctx, wikiRepo, string(headID), "")
+			commit, err := gitlog.GetCommit(ctx, wikiRepo, string(headID))
 			require.NoError(t, err, "look up git commit before merge is applied")
 
 			require.Equal(t, authorName, commit.Author.Name, "author name mismatched")
diff --git a/internal/service/wiki/write_page_test.go b/internal/service/wiki/write_page_test.go
index acf500be5..729a71c46 100644
--- a/internal/service/wiki/write_page_test.go
+++ b/internal/service/wiki/write_page_test.go
@@ -102,7 +102,7 @@ func TestSuccessfulWikiWritePageRequest(t *testing.T) {
 			require.Empty(t, resp.DuplicateError, "DuplicateError must be empty")
 
 			headID := testhelper.MustRunCommand(t, nil, "git", "-C", wikiRepoPath, "show", "--format=format:%H", "--no-patch", "HEAD")
-			commit, err := gitlog.GetCommit(ctx, wikiRepo, string(headID), "")
+			commit, err := gitlog.GetCommit(ctx, wikiRepo, string(headID))
 			require.NoError(t, err, "look up git commit after writing a wiki page")
 
 			require.Equal(t, authorName, commit.Author.Name, "author name mismatched")