catfile: Replace the previous implementation of ParseCommit

In the previous commit, we introduced a state-machine based
`parseCommit()`. Let's replace the existing `ParseCommit()` function
with the one we introduced.

This involves changing the interface to now return `catfile.Commit`
instead of the previous `gitalypb.GitCommit`.

21 files changed, 37 insertions, 317 deletions

diff --git a/internal/git/catfile/commit.go b/internal/git/catfile/commit.go
index 0af44fee8..94277bb10 100644
--- a/internal/git/catfile/commit.go
+++ b/internal/git/catfile/commit.go
@@ -14,7 +14,7 @@ import (
 )
 
 // GetCommit looks up a commit by revision using an existing Batch instance.
-func GetCommit(ctx context.Context, objectReader ObjectContentReader, revision git.Revision) (*gitalypb.GitCommit, error) {
+func GetCommit(ctx context.Context, objectReader ObjectContentReader, revision git.Revision) (*Commit, error) {
 	object, err := objectReader.Object(ctx, revision+"^{commit}")
 	if err != nil {
 		return nil, err
@@ -63,7 +63,7 @@ func GetCommitWithTrailers(
 		}
 	}
 
-	return commit, nil
+	return commit.GitCommit, nil
 }
 
 // GetCommitMessage looks up a commit message and returns it in its entirety.
diff --git a/internal/git/catfile/commit_test.go b/internal/git/catfile/commit_test.go
index 55d6012d9..d4a51457e 100644
--- a/internal/git/catfile/commit_test.go
+++ b/internal/git/catfile/commit_test.go
@@ -55,8 +55,11 @@ func TestGetCommit(t *testing.T) {
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
 			commit, err := GetCommit(ctx, objectReader, git.Revision(tc.revision))
-			require.Equal(t, tc.expectedErr, err)
-			testhelper.ProtoEqual(t, tc.expectedCommit, commit)
+			if tc.expectedErr != nil || err != nil {
+				require.Equal(t, tc.expectedErr, err)
+			} else {
+				testhelper.ProtoEqual(t, tc.expectedCommit, commit.GitCommit)
+			}
 		})
 	}
 }
diff --git a/internal/git/catfile/parse_commit.go b/internal/git/catfile/parse_commit.go
index c79c0071f..1b22d9d4a 100644
--- a/internal/git/catfile/parse_commit.go
+++ b/internal/git/catfile/parse_commit.go
@@ -84,7 +84,7 @@ type Commit struct {
 	SignatureData SignatureData
 }
 
-// parseCommit implements a state machine to parse the various sections
+// ParseCommit implements a state machine to parse the various sections
 // of a commit. To understand the state machine, see the definition
 // for parseState above.
 //
@@ -93,7 +93,7 @@ type Commit struct {
 // that we throw errors only wherever git does.
 //
 // [1]: https://gitlab.com/gitlab-org/git/-/blob/master/commit.c
-func (p *parser) parseCommit(object git.Object) (*Commit, error) {
+func (p *parser) ParseCommit(object git.Object) (*Commit, error) {
 	commit := &gitalypb.GitCommit{Id: object.ObjectID().String()}
 	var payload []byte
 	currentSignatureIndex := 0
diff --git a/internal/git/catfile/parse_commit_test.go b/internal/git/catfile/parse_commit_test.go
index e79c45ef3..2e80dc38f 100644
--- a/internal/git/catfile/parse_commit_test.go
+++ b/internal/git/catfile/parse_commit_test.go
@@ -501,7 +501,7 @@ func TestParseCommits(t *testing.T) {
 
 			setup := tc.setup(t)
 
-			commit, err := newParser().parseCommit(newStaticObject(setup.content, "commit", setup.oid))
+			commit, err := NewParser().ParseCommit(newStaticObject(setup.content, "commit", setup.oid))
 			require.Equal(t, setup.expectedErr, err)
 			testhelper.ProtoEqual(t, setup.expectedCommit.GitCommit, commit.GitCommit)
 			require.Equal(t, setup.expectedCommit.SignatureData.Payload, commit.SignatureData.Payload)
diff --git a/internal/git/catfile/parser.go b/internal/git/catfile/parser.go
index 5df14954d..b64fe16eb 100644
--- a/internal/git/catfile/parser.go
+++ b/internal/git/catfile/parser.go
@@ -11,14 +11,13 @@ import (
 	"time"
 
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git"
-	"gitlab.com/gitlab-org/gitaly/v16/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/v16/proto/go/gitalypb"
 	"google.golang.org/protobuf/types/known/timestamppb"
 )
 
 // Parser parses Git objects into their gitalypb representations.
 type Parser interface {
-	ParseCommit(object git.Object) (*gitalypb.GitCommit, error)
+	ParseCommit(object git.Object) (*Commit, error)
 	ParseTag(object git.Object) (*gitalypb.Tag, error)
 }
 
@@ -37,95 +36,6 @@ func newParser() *parser {
 	}
 }
 
-// ParseCommit parses the commit data from the Reader.
-func (p *parser) ParseCommit(object git.Object) (*gitalypb.GitCommit, error) {
-	commit := &gitalypb.GitCommit{Id: object.ObjectID().String()}
-
-	var lastLine bool
-	p.bufferedReader.Reset(object)
-
-	bytesRemaining := object.ObjectSize()
-	for !lastLine {
-		line, err := p.bufferedReader.ReadString('\n')
-		if errors.Is(err, io.EOF) {
-			lastLine = true
-		} else if err != nil {
-			return nil, fmt.Errorf("parse raw commit: header: %w", err)
-		}
-		bytesRemaining -= int64(len(line))
-
-		if len(line) == 0 || line[0] == ' ' {
-			continue
-		}
-		// A blank line indicates the start of the commit body
-		if line == "\n" {
-			break
-		}
-
-		// There might not be a final line break if there was an EOF
-		if line[len(line)-1] == '\n' {
-			line = line[:len(line)-1]
-		}
-
-		key, value, ok := strings.Cut(line, " ")
-		if !ok {
-			continue
-		}
-
-		switch key {
-		case "parent":
-			commit.ParentIds = append(commit.ParentIds, value)
-		case "author":
-			commit.Author = parseCommitAuthor(value)
-		case "committer":
-			commit.Committer = parseCommitAuthor(value)
-		case "gpgsig", "gpgsig-sha256":
-			commit.SignatureType = detectSignatureType(value)
-		case "tree":
-			commit.TreeId = value
-		case "encoding":
-			commit.Encoding = value
-		}
-	}
-
-	if !lastLine {
-		body := make([]byte, bytesRemaining)
-		if _, err := io.ReadFull(p.bufferedReader, body); err != nil {
-			return nil, fmt.Errorf("reading commit message: %w", err)
-		}
-
-		// After we have copied the body, we must make sure that there really is no
-		// additional data. For once, this is to detect bugs in our implementation where we
-		// would accidentally have truncated the commit message. On the other hand, we also
-		// need to do this such that we observe the EOF, which we must observe in order to
-		// unblock reading the next object.
-		//
-		// This all feels a bit complicated, where it would be much easier to just read into
-		// a preallocated `bytes.Buffer`. But this complexity is indeed required to optimize
-		// allocations. So if you want to change this, please make sure to execute the
-		// `BenchmarkListAllCommits` benchmark.
-		if n, err := io.Copy(io.Discard, p.bufferedReader); err != nil {
-			return nil, fmt.Errorf("reading commit message: %w", err)
-		} else if n != 0 {
-			return nil, fmt.Errorf(
-				"commit message exceeds expected length %v by %v bytes",
-				object.ObjectSize(), n,
-			)
-		}
-
-		if len(body) > 0 {
-			commit.Subject = subjectFromBody(body)
-			commit.BodySize = int64(len(body))
-			commit.Body = body
-			if max := helper.MaxCommitOrTagMessageSize; len(body) > max {
-				commit.Body = commit.Body[:max]
-			}
-		}
-	}
-
-	return commit, nil
-}
-
 const maxUnixCommitDate = 1 << 53
 
 // fallbackTimeValue is the value returned in case there is a parse error. It's the maximum
diff --git a/internal/git/catfile/parser_test.go b/internal/git/catfile/parser_test.go
index 6d2e6d53c..b54251965 100644
--- a/internal/git/catfile/parser_test.go
+++ b/internal/git/catfile/parser_test.go
@@ -1,215 +1,16 @@
 package catfile
 
 import (
-	"bytes"
 	"fmt"
-	"strings"
 	"testing"
-	"time"
 
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git/gittest"
-	"gitlab.com/gitlab-org/gitaly/v16/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/v16/proto/go/gitalypb"
 	"google.golang.org/protobuf/types/known/timestamppb"
 )
 
-func TestParser_ParseCommit(t *testing.T) {
-	t.Parallel()
-
-	info := &ObjectInfo{
-		Oid:  gittest.DefaultObjectHash.EmptyTreeOID,
-		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.
-	for _, tc := range []struct {
-		desc string
-		in   string
-		out  *gitalypb.GitCommit
-	}{
-		{
-			desc: "empty commit object",
-			in:   "",
-			out:  &gitalypb.GitCommit{Id: info.Oid.String()},
-		},
-		{
-			desc: "no email",
-			in:   "author Jane Doe",
-			out: &gitalypb.GitCommit{
-				Id:     info.Oid.String(),
-				Author: &gitalypb.CommitAuthor{Name: []byte("Jane Doe")},
-			},
-		},
-		{
-			desc: "unmatched <",
-			in:   "author Jane Doe <janedoe@example.com",
-			out: &gitalypb.GitCommit{
-				Id:     info.Oid.String(),
-				Author: &gitalypb.CommitAuthor{Name: []byte("Jane Doe")},
-			},
-		},
-		{
-			desc: "unmatched >",
-			in:   "author Jane Doe janedoe@example.com>",
-			out: &gitalypb.GitCommit{
-				Id:     info.Oid.String(),
-				Author: &gitalypb.CommitAuthor{Name: []byte("Jane Doe janedoe@example.com>")},
-			},
-		},
-		{
-			desc: "missing date",
-			in:   "author Jane Doe <janedoe@example.com> ",
-			out: &gitalypb.GitCommit{
-				Id:     info.Oid.String(),
-				Author: &gitalypb.CommitAuthor{Name: []byte("Jane Doe"), Email: []byte("janedoe@example.com")},
-			},
-		},
-		{
-			desc: "date too high",
-			in:   "author Jane Doe <janedoe@example.com> 9007199254740993 +0200",
-			out: &gitalypb.GitCommit{
-				Id: info.Oid.String(),
-				Author: &gitalypb.CommitAuthor{
-					Name:     []byte("Jane Doe"),
-					Email:    []byte("janedoe@example.com"),
-					Date:     &timestamppb.Timestamp{Seconds: 9223371974719179007},
-					Timezone: []byte("+0200"),
-				},
-			},
-		},
-		{
-			desc: "date negative",
-			in:   "author Jane Doe <janedoe@example.com> -1 +0200",
-			out: &gitalypb.GitCommit{
-				Id: info.Oid.String(),
-				Author: &gitalypb.CommitAuthor{
-					Name:     []byte("Jane Doe"),
-					Email:    []byte("janedoe@example.com"),
-					Date:     &timestamppb.Timestamp{Seconds: 9223371974719179007},
-					Timezone: []byte("+0200"),
-				},
-			},
-		},
-		{
-			desc: "ssh signature",
-			in: `gpgsig -----BEGIN SSH SIGNATURE-----
-U1NIU0lHAAAAAQAAADMAAAALc3NoLWVkMjU1MTkAAAAgtc+Qk8jhMwVZk/jFEFCM16LNQb
-30q5kK30bbetfjyTMAAAADZ2l0AAAAAAAAAAZzaGE1MTIAAABTAAAAC3NzaC1lZDI1NTE5
-AAAAQLSyv010gOFwIs9QTtDvlfIEWiAw2iQL/T9usGcxHXn/W5l0cOFCd7O+WaMDg0t0nW
-fF3T79iV8paT4/OfX8Ygg=
------END SSH SIGNATURE-----`,
-			out: &gitalypb.GitCommit{
-				Id:            info.Oid.String(),
-				SignatureType: gitalypb.SignatureType_SSH,
-			},
-		},
-		{
-			desc: "SHA256 signature",
-			in: fmt.Sprintf(`gpgsig-sha256 -----BEGIN PGP SIGNATURE-----
-Version: ObjectivePGP
-Comment: https://www.objectivepgp.com
-Charset: UTF-8
-%c
-wsFcBAABCgAGBQJecon1AAoJEDYMjTn1G2THmSsP/At/jskLdF0i7p0nKf4JLjeeqRJ4k2IUg87U
-ZwV6mbLo5XFm8Sq7CJBAGAhlOZE4BAwKALuawmgs5XMEZwK2z6AIgosGTVpmxDTTI11bXt4XIOdz
-qF7c/gUrJOZzjFXOqDsd5UuPRupwznC5eKlLbfImR+NYxKryo8JGdF5t52ph4kChcQsKlSkXuYNI
-+9UgbaMclEjb0OLm+mcP9QxW+Cs9JS2Jb4Jh6XONWW1nDN3ZTDDskguIqqF47UxIgSImrmpMcEj9
-YSNU0oMoHM4+1DoXp1t99EGPoAMvO+a5g8gd1jouCIrI6KOX+GeG/TFFM0mQwg/d/N9LR049m8ed
-vgqg/lMiWUxQGL2IPpYPcgiUEqfn7ete+NMzQV5zstxF/q7Yj2BhM2L7FPHxKaoy/w5Q/DcAO4wN
-5gxVmIvbCDk5JOx8I+boIS8ZxSvIlJ5IWaPrcjg5Mc40it+WHvMqxVnCzH0c6KcXaJ2SibVb59HR
-pdRhEXXw/hRN65l/xwyM8sklQalAGu755gNJZ4k9ApBVUssZyiu+te2+bDirAcmK8/x1jvMQY6bn
-DFxBE7bMHDp24IFPaVID84Ryt3vSSBEkrUGm7OkyDESTpHCr4sfD5o3LCUCIibTqv/CAhe59mhbB
-2AXL7X+EzylKy6C1N5KUUiMTW94AuF6f8FqBoxnf
-=U6zM
------END PGP SIGNATURE-----`, ' '),
-			out: &gitalypb.GitCommit{
-				Id:            info.Oid.String(),
-				SignatureType: gitalypb.SignatureType_PGP,
-			},
-		},
-		{
-			desc: "huge",
-			in:   "author " + strings.Repeat("A", 100000),
-			out: &gitalypb.GitCommit{
-				Id: info.Oid.String(),
-				Author: &gitalypb.CommitAuthor{
-					Name: bytes.Repeat([]byte("A"), 100000),
-				},
-			},
-		},
-		{
-			desc: "has encoding",
-			in:   "encoding Windows-1251",
-			out: &gitalypb.GitCommit{
-				Id:       info.Oid.String(),
-				Encoding: "Windows-1251",
-			},
-		},
-	} {
-		t.Run(tc.desc, func(t *testing.T) {
-			info.Size = int64(len(tc.in))
-			out, err := NewParser().ParseCommit(newStaticObject(tc.in, "commit", info.Oid))
-			require.NoError(t, err, "parse error")
-			require.Equal(t, tc.out, out)
-		})
-	}
-}
-
-func TestParseCommitAuthor(t *testing.T) {
-	t.Parallel()
-
-	for _, tc := range []struct {
-		desc     string
-		author   string
-		expected *gitalypb.CommitAuthor
-	}{
-		{
-			desc:     "empty author",
-			author:   "",
-			expected: &gitalypb.CommitAuthor{},
-		},
-		{
-			desc:   "normal author",
-			author: "Au Thor <au.thor@example.com> 1625121079 +0000",
-			expected: &gitalypb.CommitAuthor{
-				Name:     []byte("Au Thor"),
-				Email:    []byte("au.thor@example.com"),
-				Date:     timestamppb.New(time.Unix(1625121079, 0)),
-				Timezone: []byte("+0000"),
-			},
-		},
-		{
-			desc:   "author with missing mail",
-			author: "Au Thor <> 1625121079 +0000",
-			expected: &gitalypb.CommitAuthor{
-				Name:     []byte("Au Thor"),
-				Date:     timestamppb.New(time.Unix(1625121079, 0)),
-				Timezone: []byte("+0000"),
-			},
-		},
-		{
-			desc:   "author with missing date",
-			author: "Au Thor <au.thor@example.com>",
-			expected: &gitalypb.CommitAuthor{
-				Name:  []byte("Au Thor"),
-				Email: []byte("au.thor@example.com"),
-			},
-		},
-	} {
-		t.Run(tc.desc, func(t *testing.T) {
-			testhelper.ProtoEqual(t, tc.expected, parseCommitAuthor(tc.author))
-		})
-	}
-}
-
 func TestParser_ParseTag(t *testing.T) {
 	t.Parallel()
 
diff --git a/internal/git/catfile/tag.go b/internal/git/catfile/tag.go
index 0dfbe8755..afce2f26e 100644
--- a/internal/git/catfile/tag.go
+++ b/internal/git/catfile/tag.go
@@ -61,15 +61,17 @@ func buildAnnotatedTag(ctx context.Context, objectReader ObjectContentReader, ob
 
 	switch tagged.objectType {
 	case "commit":
-		tag.TargetCommit, err = GetCommit(ctx, objectReader, git.Revision(tagged.objectID))
+		commit, err := GetCommit(ctx, objectReader, git.Revision(tagged.objectID))
 		if err != nil {
 			return nil, fmt.Errorf("buildAnnotatedTag error when getting target commit: %w", err)
 		}
+		tag.TargetCommit = commit.GitCommit
 
 	case "tag":
-		tag.TargetCommit, err = dereferenceTag(ctx, objectReader, git.Revision(tagged.objectID))
-		if err != nil {
+		if commit, err := dereferenceTag(ctx, objectReader, git.Revision(tagged.objectID)); err != nil {
 			return nil, fmt.Errorf("buildAnnotatedTag error when dereferencing tag: %w", err)
+		} else if commit != nil {
+			tag.TargetCommit = commit.GitCommit
 		}
 	}
 
@@ -79,7 +81,7 @@ func buildAnnotatedTag(ctx context.Context, objectReader ObjectContentReader, ob
 // dereferenceTag recursively dereferences annotated tags until it finds a non-tag object. If it is
 // a commit, then it will parse and return this commit. Otherwise, if the tagged object is not a
 // commit, it will simply discard the object and not return an error.
-func dereferenceTag(ctx context.Context, objectReader ObjectContentReader, oid git.Revision) (*gitalypb.GitCommit, error) {
+func dereferenceTag(ctx context.Context, objectReader ObjectContentReader, oid git.Revision) (*Commit, error) {
 	object, err := objectReader.Object(ctx, oid+"^{}")
 	if err != nil {
 		return nil, fmt.Errorf("peeling tag: %w", err)
diff --git a/internal/git/localrepo/commit.go b/internal/git/localrepo/commit.go
index df5effc2a..2f77069ef 100644
--- a/internal/git/localrepo/commit.go
+++ b/internal/git/localrepo/commit.go
@@ -68,7 +68,10 @@ func (repo *Repo) ReadCommit(ctx context.Context, revision git.Revision, opts ..
 	if cfg.withTrailers {
 		commit, err = catfile.GetCommitWithTrailers(ctx, repo.gitCmdFactory, repo, objectReader, revision)
 	} else {
-		commit, err = catfile.GetCommit(ctx, objectReader, revision)
+		var c *catfile.Commit
+		if c, err = catfile.GetCommit(ctx, objectReader, revision); err == nil {
+			commit = c.GitCommit
+		}
 	}
 
 	if err != nil {
diff --git a/internal/git/log/last_commit.go b/internal/git/log/last_commit.go
index d92251b32..eb5333668 100644
--- a/internal/git/log/last_commit.go
+++ b/internal/git/log/last_commit.go
@@ -21,7 +21,7 @@ func LastCommitForPath(
 	revision git.Revision,
 	path string,
 	options *gitalypb.GlobalOptions,
-) (*gitalypb.GitCommit, error) {
+) (*catfile.Commit, error) {
 	var stdout strings.Builder
 	cmd, err := gitCmdFactory.New(ctx, repo, git.Command{
 		Name:        "log",
diff --git a/internal/git/log/parser.go b/internal/git/log/parser.go
index 1c9e9d566..dce154770 100644
--- a/internal/git/log/parser.go
+++ b/internal/git/log/parser.go
@@ -55,7 +55,7 @@ func (parser *Parser) Parse(ctx context.Context) bool {
 		return false
 	}
 
-	parser.currentCommit = commit
+	parser.currentCommit = commit.GitCommit
 	return true
 }
 
diff --git a/internal/gitaly/service/commit/find_commits.go b/internal/gitaly/service/commit/find_commits.go
index e2d717b25..15425330c 100644
--- a/internal/gitaly/service/commit/find_commits.go
+++ b/internal/gitaly/service/commit/find_commits.go
@@ -195,7 +195,7 @@ func (g *GetCommits) Commit(ctx context.Context, trailers, shortStat, refs bool)
 		}
 	}
 
-	return commit, nil
+	return commit.GitCommit, nil
 }
 
 func streamCommits(getCommits *GetCommits, stream gitalypb.CommitService_FindCommitsServer, trailers, shortStat bool, refs bool) error {
diff --git a/internal/gitaly/service/commit/last_commit_for_path.go b/internal/gitaly/service/commit/last_commit_for_path.go
index e0f69a97c..555a34e77 100644
--- a/internal/gitaly/service/commit/last_commit_for_path.go
+++ b/internal/gitaly/service/commit/last_commit_for_path.go
@@ -57,7 +57,7 @@ func (s *server) lastCommitForPath(ctx context.Context, in *gitalypb.LastCommitF
 		return &gitalypb.LastCommitForPathResponse{}, nil
 	}
 
-	return &gitalypb.LastCommitForPathResponse{Commit: commit}, err
+	return &gitalypb.LastCommitForPathResponse{Commit: commit.GitCommit}, err
 }
 
 func validateLastCommitForPathRequest(locator storage.Locator, in *gitalypb.LastCommitForPathRequest) error {
diff --git a/internal/gitaly/service/commit/list_all_commits.go b/internal/gitaly/service/commit/list_all_commits.go
index 0ee326c23..62633d6e5 100644
--- a/internal/gitaly/service/commit/list_all_commits.go
+++ b/internal/gitaly/service/commit/list_all_commits.go
@@ -79,7 +79,7 @@ func (s *server) ListAllCommits(
 			return structerr.NewInternal("parsing commit: %w", err)
 		}
 
-		if err := chunker.Send(commit); err != nil {
+		if err := chunker.Send(commit.GitCommit); err != nil {
 			return structerr.NewInternal("sending commit: %w", err)
 		}
 	}
diff --git a/internal/gitaly/service/commit/list_commits.go b/internal/gitaly/service/commit/list_commits.go
index 1a16158a5..09f627873 100644
--- a/internal/gitaly/service/commit/list_commits.go
+++ b/internal/gitaly/service/commit/list_commits.go
@@ -139,7 +139,7 @@ func (s *server) ListCommits(
 			return structerr.NewInternal("parsing commit: %w", err)
 		}
 
-		if err := chunker.Send(commit); err != nil {
+		if err := chunker.Send(commit.GitCommit); err != nil {
 			return structerr.NewInternal("sending commit: %w", err)
 		}
 	}
diff --git a/internal/gitaly/service/commit/list_commits_by_oid.go b/internal/gitaly/service/commit/list_commits_by_oid.go
index 3175c4307..f97c2aca5 100644
--- a/internal/gitaly/service/commit/list_commits_by_oid.go
+++ b/internal/gitaly/service/commit/list_commits_by_oid.go
@@ -50,7 +50,7 @@ func (s *server) ListCommitsByOid(in *gitalypb.ListCommitsByOidRequest, stream g
 			return err
 		}
 
-		if err := sender.Send(commit); err != nil {
+		if err := sender.Send(commit.GitCommit); err != nil {
 			return err
 		}
 	}
diff --git a/internal/gitaly/service/commit/list_commits_by_ref_name.go b/internal/gitaly/service/commit/list_commits_by_ref_name.go
index e77c9f916..6c6d25b07 100644
--- a/internal/gitaly/service/commit/list_commits_by_ref_name.go
+++ b/internal/gitaly/service/commit/list_commits_by_ref_name.go
@@ -37,7 +37,7 @@ func (s *server) ListCommitsByRefName(in *gitalypb.ListCommitsByRefNameRequest,
 		}
 
 		commitByRef := &gitalypb.ListCommitsByRefNameResponse_CommitForRef{
-			Commit: commit, RefName: refName,
+			Commit: commit.GitCommit, RefName: refName,
 		}
 
 		if err := sender.Send(commitByRef); err != nil {
diff --git a/internal/gitaly/service/commit/list_last_commits_for_tree.go b/internal/gitaly/service/commit/list_last_commits_for_tree.go
index fa7e7d0f7..85a5d2a69 100644
--- a/internal/gitaly/service/commit/list_last_commits_for_tree.go
+++ b/internal/gitaly/service/commit/list_last_commits_for_tree.go
@@ -74,7 +74,7 @@ func (s *server) listLastCommitsForTree(in *gitalypb.ListLastCommitsForTreeReque
 
 		commitForTree := &gitalypb.ListLastCommitsForTreeResponse_CommitForTree{
 			PathBytes: []byte(entry.Path),
-			Commit:    commit,
+			Commit:    commit.GitCommit,
 		}
 
 		batch = append(batch, commitForTree)
diff --git a/internal/gitaly/service/operations/tags.go b/internal/gitaly/service/operations/tags.go
index 83b2d7b1f..bc78ca810 100644
--- a/internal/gitaly/service/operations/tags.go
+++ b/internal/gitaly/service/operations/tags.go
@@ -331,7 +331,7 @@ func (s *Server) createTag(
 		if err != nil {
 			return nil, "", structerr.NewInternal("getting commit: %w", err)
 		}
-		tagObject.TargetCommit = peeledTargetCommit
+		tagObject.TargetCommit = peeledTargetCommit.GitCommit
 	}
 
 	return tagObject, refObjectID, nil
diff --git a/internal/gitaly/service/ref/find_all_tags.go b/internal/gitaly/service/ref/find_all_tags.go
index 2091b0011..e58917c15 100644
--- a/internal/gitaly/service/ref/find_all_tags.go
+++ b/internal/gitaly/service/ref/find_all_tags.go
@@ -100,10 +100,11 @@ func (s *server) findAllTags(ctx context.Context, repo *localrepo.Repo, sortFiel
 			// which refers to a commit object. Otherwise, we discard the object's
 			// contents.
 			if peeledTag.ObjectType() == "commit" {
-				result.TargetCommit, err = parser.ParseCommit(peeledTag)
+				commit, err := parser.ParseCommit(peeledTag)
 				if err != nil {
 					return fmt.Errorf("parsing tagged commit: %w", err)
 				}
+				result.TargetCommit = commit.GitCommit
 			} else {
 				if _, err := io.Copy(io.Discard, peeledTag); err != nil {
 					return fmt.Errorf("discarding tagged object contents: %w", err)
@@ -117,7 +118,7 @@ func (s *server) findAllTags(ctx context.Context, repo *localrepo.Repo, sortFiel
 
 			result = &gitalypb.Tag{
 				Id:           tag.ObjectID().String(),
-				TargetCommit: commit,
+				TargetCommit: commit.GitCommit,
 			}
 		default:
 			if _, err := io.Copy(io.Discard, tag); err != nil {
diff --git a/internal/gitaly/service/ref/find_tag.go b/internal/gitaly/service/ref/find_tag.go
index 69f3d93f8..1c700bd65 100644
--- a/internal/gitaly/service/ref/find_tag.go
+++ b/internal/gitaly/service/ref/find_tag.go
@@ -57,7 +57,7 @@ func parseTagLine(ctx context.Context, objectReader catfile.ObjectContentReader,
 		if err != nil {
 			return nil, fmt.Errorf("getting commit catfile: %w", err)
 		}
-		tag.TargetCommit = commit
+		tag.TargetCommit = commit.GitCommit
 		return tag, nil
 	default:
 		return tag, nil
diff --git a/internal/gitaly/service/ref/util.go b/internal/gitaly/service/ref/util.go
index 4f90c3b0d..144d83308 100644
--- a/internal/gitaly/service/ref/util.go
+++ b/internal/gitaly/service/ref/util.go
@@ -35,7 +35,7 @@ func buildAllBranchesBranch(ctx context.Context, objectReader catfile.ObjectCont
 
 	return &gitalypb.FindAllBranchesResponse_Branch{
 		Name:   elements[0],
-		Target: target,
+		Target: target.GitCommit,
 	}, nil
 }
 
@@ -47,7 +47,7 @@ func buildBranch(ctx context.Context, objectReader catfile.ObjectContentReader,
 
 	return &gitalypb.Branch{
 		Name:         elements[0],
-		TargetCommit: target,
+		TargetCommit: target.GitCommit,
 	}, nil
 }