Merge branch 'pks-git-localrepo-split-object-types' into 'master'

localrepo: Split object functions by type

See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/6304

Merged-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Approved-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>

11 files changed, 766 insertions, 726 deletions

diff --git a/internal/git/localrepo/blob.go b/internal/git/localrepo/blob.go
new file mode 100644
index 000000000..bd52a202b
--- /dev/null
+++ b/internal/git/localrepo/blob.go
@@ -0,0 +1,52 @@
+package localrepo
+
+import (
+	"bytes"
+	"context"
+	"fmt"
+	"io"
+
+	"gitlab.com/gitlab-org/gitaly/v16/internal/git"
+	"gitlab.com/gitlab-org/gitaly/v16/internal/helper/text"
+)
+
+// WriteBlob writes a blob to the repository's object database and
+// returns its object ID. Path is used by git to decide which filters to
+// run on the content.
+func (repo *Repo) WriteBlob(ctx context.Context, path string, content io.Reader) (git.ObjectID, error) {
+	stdout := &bytes.Buffer{}
+	stderr := &bytes.Buffer{}
+
+	cmd, err := repo.Exec(ctx,
+		git.Command{
+			Name: "hash-object",
+			Flags: []git.Option{
+				git.ValueFlag{Name: "--path", Value: path},
+				git.Flag{Name: "--stdin"},
+				git.Flag{Name: "-w"},
+			},
+		},
+		git.WithStdin(content),
+		git.WithStdout(stdout),
+		git.WithStderr(stderr),
+	)
+	if err != nil {
+		return "", err
+	}
+
+	if err := cmd.Wait(); err != nil {
+		return "", errorWithStderr(err, stderr.Bytes())
+	}
+
+	objectHash, err := repo.ObjectHash(ctx)
+	if err != nil {
+		return "", fmt.Errorf("detecting object hash: %w", err)
+	}
+
+	oid, err := objectHash.FromHex(text.ChompBytes(stdout.Bytes()))
+	if err != nil {
+		return "", err
+	}
+
+	return oid, nil
+}
diff --git a/internal/git/localrepo/blob_test.go b/internal/git/localrepo/blob_test.go
new file mode 100644
index 000000000..d57cf472b
--- /dev/null
+++ b/internal/git/localrepo/blob_test.go
@@ -0,0 +1,82 @@
+package localrepo
+
+import (
+	"io"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/v16/internal/helper/perm"
+	"gitlab.com/gitlab-org/gitaly/v16/internal/testhelper"
+)
+
+type ReaderFunc func([]byte) (int, error)
+
+func (fn ReaderFunc) Read(b []byte) (int, error) { return fn(b) }
+
+func TestRepo_WriteBlob(t *testing.T) {
+	t.Parallel()
+
+	ctx := testhelper.Context(t)
+	_, repo, repoPath := setupRepo(t)
+
+	for _, tc := range []struct {
+		desc       string
+		attributes string
+		input      io.Reader
+		sha        string
+		error      error
+		content    string
+	}{
+		{
+			desc:  "error reading",
+			input: ReaderFunc(func([]byte) (int, error) { return 0, assert.AnError }),
+			error: assert.AnError,
+		},
+		{
+			desc:    "successful empty blob",
+			input:   strings.NewReader(""),
+			content: "",
+		},
+		{
+			desc:    "successful blob",
+			input:   strings.NewReader("some content"),
+			content: "some content",
+		},
+		{
+			desc:    "LF line endings left unmodified",
+			input:   strings.NewReader("\n"),
+			content: "\n",
+		},
+		{
+			desc:    "CRLF converted to LF due to global git config",
+			input:   strings.NewReader("\r\n"),
+			content: "\n",
+		},
+		{
+			desc:       "line endings preserved in binary files",
+			input:      strings.NewReader("\r\n"),
+			attributes: "file-path binary",
+			content:    "\r\n",
+		},
+	} {
+		t.Run(tc.desc, func(t *testing.T) {
+			attributesPath := filepath.Join(repoPath, "info", "attributes")
+			require.NoError(t, os.MkdirAll(filepath.Dir(attributesPath), perm.SharedDir))
+			require.NoError(t, os.WriteFile(attributesPath, []byte(tc.attributes), perm.PublicFile))
+
+			sha, err := repo.WriteBlob(ctx, "file-path", tc.input)
+			require.Equal(t, tc.error, err)
+			if tc.error != nil {
+				return
+			}
+
+			content, err := repo.ReadObject(ctx, sha)
+			require.NoError(t, err)
+			assert.Equal(t, tc.content, string(content))
+		})
+	}
+}
diff --git a/internal/git/localrepo/commit.go b/internal/git/localrepo/commit.go
index cc4abe005..5f941af0c 100644
--- a/internal/git/localrepo/commit.go
+++ b/internal/git/localrepo/commit.go
@@ -10,26 +10,75 @@ import (
 	"strings"
 	"time"
 
+	"gitlab.com/gitlab-org/gitaly/v16/internal/command"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/featureflag"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git"
+	"gitlab.com/gitlab-org/gitaly/v16/internal/git/catfile"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/helper/perm"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/helper/text"
+	"gitlab.com/gitlab-org/gitaly/v16/proto/go/gitalypb"
 )
 
-// ErrMissingTree indicates a missing tree when attemping to write a commit
-var ErrMissingTree = errors.New("missing tree")
+var (
+	// ErrMissingTree indicates a missing tree when attemping to write a commit
+	ErrMissingTree = errors.New("missing tree")
+	// ErrMissingCommitterName indicates an attempt to write a commit without a
+	// comitter name
+	ErrMissingCommitterName = errors.New("missing committer name")
+	// ErrMissingAuthorName indicates an attempt to write a commit without a
+	// comitter name
+	ErrMissingAuthorName = errors.New("missing author name")
+	// ErrDisallowedCharacters indicates the name and/or email contains disallowed
+	// characters
+	ErrDisallowedCharacters = errors.New("disallowed characters")
+	// ErrObjectNotFound is returned in case an object could not be found.
+	ErrObjectNotFound = errors.New("object not found")
+)
+
+type readCommitConfig struct {
+	withTrailers bool
+}
+
+// ReadCommitOpt is an option for ReadCommit.
+type ReadCommitOpt func(*readCommitConfig)
+
+// WithTrailers will cause ReadCommit to parse commit trailers.
+func WithTrailers() ReadCommitOpt {
+	return func(cfg *readCommitConfig) {
+		cfg.withTrailers = true
+	}
+}
+
+// ReadCommit reads the commit specified by the given revision. If no such
+// revision exists, it will return an ErrObjectNotFound error.
+func (repo *Repo) ReadCommit(ctx context.Context, revision git.Revision, opts ...ReadCommitOpt) (*gitalypb.GitCommit, error) {
+	var cfg readCommitConfig
+	for _, opt := range opts {
+		opt(&cfg)
+	}
 
-// ErrMissingCommitterName indicates an attempt to write a commit without a
-// comitter name
-var ErrMissingCommitterName = errors.New("missing committer name")
+	objectReader, cancel, err := repo.catfileCache.ObjectReader(ctx, repo)
+	if err != nil {
+		return nil, err
+	}
+	defer cancel()
 
-// ErrMissingAuthorName indicates an attempt to write a commit without a
-// comitter name
-var ErrMissingAuthorName = errors.New("missing author name")
+	var commit *gitalypb.GitCommit
+	if cfg.withTrailers {
+		commit, err = catfile.GetCommitWithTrailers(ctx, repo.gitCmdFactory, repo, objectReader, revision)
+	} else {
+		commit, err = catfile.GetCommit(ctx, objectReader, revision)
+	}
 
-// ErrDisallowedCharacters indicates the name and/or email contains disallowed
-// characters
-var ErrDisallowedCharacters = errors.New("disallowed characters")
+	if err != nil {
+		if errors.As(err, &catfile.NotFoundError{}) {
+			return nil, ErrObjectNotFound
+		}
+		return nil, err
+	}
+
+	return commit, nil
+}
 
 // WriteCommitConfig contains fields for writing a commit
 type WriteCommitConfig struct {
@@ -173,3 +222,38 @@ func (repo *Repo) WriteCommit(ctx context.Context, cfg WriteCommitConfig) (git.O
 
 	return oid, nil
 }
+
+// InvalidCommitError is returned when the revision does not point to a valid commit object.
+type InvalidCommitError git.Revision
+
+func (err InvalidCommitError) Error() string {
+	return fmt.Sprintf("invalid commit: %q", string(err))
+}
+
+// IsAncestor returns whether the parent is an ancestor of the child. InvalidCommitError is returned
+// if either revision does not point to a commit in the repository.
+func (repo *Repo) IsAncestor(ctx context.Context, parent, child git.Revision) (bool, error) {
+	const notValidCommitName = "fatal: Not a valid commit name"
+
+	stderr := &bytes.Buffer{}
+	if err := repo.ExecAndWait(ctx,
+		git.Command{
+			Name:  "merge-base",
+			Flags: []git.Option{git.Flag{Name: "--is-ancestor"}},
+			Args:  []string{parent.String(), child.String()},
+		},
+		git.WithStderr(stderr),
+	); err != nil {
+		status, ok := command.ExitStatus(err)
+		if ok && status == 1 {
+			return false, nil
+		} else if ok && strings.HasPrefix(stderr.String(), notValidCommitName) {
+			commitOID := strings.TrimSpace(strings.TrimPrefix(stderr.String(), notValidCommitName))
+			return false, InvalidCommitError(commitOID)
+		}
+
+		return false, fmt.Errorf("determine ancestry: %w, stderr: %q", err, stderr)
+	}
+
+	return true, nil
+}
diff --git a/internal/git/localrepo/commit_test.go b/internal/git/localrepo/commit_test.go
index 311b91e3e..9d8ae8fc7 100644
--- a/internal/git/localrepo/commit_test.go
+++ b/internal/git/localrepo/commit_test.go
@@ -3,6 +3,7 @@ package localrepo
 import (
 	"bytes"
 	"context"
+	"fmt"
 	"strings"
 	"testing"
 	"time"
@@ -17,8 +18,157 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v16/internal/signature"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/testhelper/testcfg"
+	"gitlab.com/gitlab-org/gitaly/v16/proto/go/gitalypb"
 )
 
+func TestRepo_ReadCommit(t *testing.T) {
+	t.Parallel()
+
+	ctx := testhelper.Context(t)
+
+	cfg, repo, repoPath := setupRepo(t)
+
+	firstParentID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithMessage("first parent"))
+	secondParentID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithMessage("second parent"))
+
+	treeID := gittest.WriteTree(t, cfg, repoPath, []gittest.TreeEntry{
+		{Path: "file", Mode: "100644", Content: "content"},
+	})
+	commitWithoutTrailers := gittest.WriteCommit(t, cfg, repoPath,
+		gittest.WithParents(firstParentID, secondParentID),
+		gittest.WithTree(treeID),
+		gittest.WithMessage("subject\n\nbody\n"),
+		gittest.WithBranch("main"),
+	)
+	commitWithTrailers := gittest.WriteCommit(t, cfg, repoPath,
+		gittest.WithParents(commitWithoutTrailers),
+		gittest.WithTree(treeID),
+		gittest.WithMessage("with trailers\n\ntrailers\n\nSigned-off-by: John Doe <john.doe@example.com>"),
+	)
+
+	// We can't use git-commit-tree(1) directly, but have to manually write signed commits.
+	signedCommit := text.ChompBytes(gittest.ExecOpts(t, cfg, gittest.ExecConfig{
+		Stdin: strings.NewReader(fmt.Sprintf(
+			`tree %s
+parent %s
+author %[3]s
+committer %[3]s
+gpgsig -----BEGIN PGP SIGNATURE-----
+some faked pgp-signature
+ -----END PGP SIGNATURE-----
+
+signed commit subject
+
+signed commit body
+`, treeID, firstParentID, gittest.DefaultCommitterSignature)),
+	}, "-C", repoPath, "hash-object", "-t", "commit", "-w", "--stdin"))
+
+	for _, tc := range []struct {
+		desc           string
+		revision       git.Revision
+		opts           []ReadCommitOpt
+		expectedCommit *gitalypb.GitCommit
+		expectedErr    error
+	}{
+		{
+			desc:        "invalid commit",
+			revision:    gittest.DefaultObjectHash.ZeroOID.Revision(),
+			expectedErr: ErrObjectNotFound,
+		},
+		{
+			desc:        "invalid commit with trailers",
+			revision:    gittest.DefaultObjectHash.ZeroOID.Revision(),
+			expectedErr: ErrObjectNotFound,
+			opts:        []ReadCommitOpt{WithTrailers()},
+		},
+		{
+			desc:     "valid commit",
+			revision: "refs/heads/main",
+			expectedCommit: &gitalypb.GitCommit{
+				Id:     commitWithoutTrailers.String(),
+				TreeId: treeID.String(),
+				ParentIds: []string{
+					firstParentID.String(),
+					secondParentID.String(),
+				},
+				Subject:   []byte("subject"),
+				Body:      []byte("subject\n\nbody\n"),
+				BodySize:  14,
+				Author:    gittest.DefaultCommitAuthor,
+				Committer: gittest.DefaultCommitAuthor,
+			},
+		},
+		{
+			desc:     "trailers do not get parsed without WithTrailers()",
+			revision: commitWithTrailers.Revision(),
+			expectedCommit: &gitalypb.GitCommit{
+				Id:     commitWithTrailers.String(),
+				TreeId: treeID.String(),
+				ParentIds: []string{
+					commitWithoutTrailers.String(),
+				},
+				Subject:   []byte("with trailers"),
+				Body:      []byte("with trailers\n\ntrailers\n\nSigned-off-by: John Doe <john.doe@example.com>"),
+				BodySize:  71,
+				Author:    gittest.DefaultCommitAuthor,
+				Committer: gittest.DefaultCommitAuthor,
+			},
+		},
+		{
+			desc:     "trailers get parsed with WithTrailers()",
+			revision: commitWithTrailers.Revision(),
+			opts:     []ReadCommitOpt{WithTrailers()},
+			expectedCommit: &gitalypb.GitCommit{
+				Id:     commitWithTrailers.String(),
+				TreeId: treeID.String(),
+				ParentIds: []string{
+					commitWithoutTrailers.String(),
+				},
+				Subject:   []byte("with trailers"),
+				Body:      []byte("with trailers\n\ntrailers\n\nSigned-off-by: John Doe <john.doe@example.com>"),
+				BodySize:  71,
+				Author:    gittest.DefaultCommitAuthor,
+				Committer: gittest.DefaultCommitAuthor,
+				Trailers: []*gitalypb.CommitTrailer{
+					{
+						Key:   []byte("Signed-off-by"),
+						Value: []byte("John Doe <john.doe@example.com>"),
+					},
+				},
+			},
+		},
+		{
+			desc:     "with PGP signature",
+			revision: git.Revision(signedCommit),
+			opts:     []ReadCommitOpt{},
+			expectedCommit: &gitalypb.GitCommit{
+				Id:     signedCommit,
+				TreeId: treeID.String(),
+				ParentIds: []string{
+					firstParentID.String(),
+				},
+				Subject:       []byte("signed commit subject"),
+				Body:          []byte("signed commit subject\n\nsigned commit body\n"),
+				BodySize:      42,
+				Author:        gittest.DefaultCommitAuthor,
+				Committer:     gittest.DefaultCommitAuthor,
+				SignatureType: gitalypb.SignatureType_PGP,
+			},
+		},
+		{
+			desc:        "not a commit",
+			revision:    "refs/heads/main^{tree}",
+			expectedErr: ErrObjectNotFound,
+		},
+	} {
+		t.Run(tc.desc, func(t *testing.T) {
+			commit, err := repo.ReadCommit(ctx, tc.revision, tc.opts...)
+			require.Equal(t, tc.expectedErr, err)
+			require.Equal(t, tc.expectedCommit, commit)
+		})
+	}
+}
+
 //go:generate rm -rf testdata/gpg-keys testdata/signing_gpg_key testdata/signing_gpg_key.pub
 //go:generate mkdir -p testdata/gpg-keys
 //go:generate chmod 0700 testdata/gpg-keys
@@ -324,3 +474,75 @@ func testWriteCommitValidation(t *testing.T, ctx context.Context) {
 		})
 	}
 }
+
+func TestRepo_IsAncestor(t *testing.T) {
+	t.Parallel()
+
+	ctx := testhelper.Context(t)
+
+	cfg, repo, repoPath := setupRepo(t)
+
+	parentCommitID := gittest.WriteCommit(t, cfg, repoPath)
+	childCommitID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithParents(parentCommitID))
+
+	for _, tc := range []struct {
+		desc         string
+		parent       git.Revision
+		child        git.Revision
+		isAncestor   bool
+		errorMatcher func(testing.TB, error)
+	}{
+		{
+			desc:       "parent is ancestor",
+			parent:     parentCommitID.Revision(),
+			child:      childCommitID.Revision(),
+			isAncestor: true,
+		},
+		{
+			desc:       "parent is not ancestor",
+			parent:     childCommitID.Revision(),
+			child:      parentCommitID.Revision(),
+			isAncestor: false,
+		},
+		{
+			desc:   "parent is not valid commit",
+			parent: gittest.DefaultObjectHash.ZeroOID.Revision(),
+			child:  childCommitID.Revision(),
+			errorMatcher: func(tb testing.TB, err error) {
+				require.Equal(tb, InvalidCommitError(gittest.DefaultObjectHash.ZeroOID), err)
+			},
+		},
+		{
+			desc:   "child is not valid commit",
+			parent: childCommitID.Revision(),
+			child:  gittest.DefaultObjectHash.ZeroOID.Revision(),
+			errorMatcher: func(tb testing.TB, err error) {
+				require.Equal(tb, InvalidCommitError(gittest.DefaultObjectHash.ZeroOID), err)
+			},
+		},
+		{
+			desc:   "child points to a tree",
+			parent: childCommitID.Revision(),
+			child:  childCommitID.Revision() + "^{tree}",
+			errorMatcher: func(tb testing.TB, actualErr error) {
+				treeOID, err := repo.ResolveRevision(ctx, childCommitID.Revision()+"^{tree}")
+				require.NoError(tb, err)
+				require.EqualError(tb, actualErr, fmt.Sprintf(
+					`determine ancestry: exit status 128, stderr: "error: object %s is a tree, not a commit\nfatal: Not a valid commit name %s^{tree}\n"`,
+					treeOID, childCommitID,
+				))
+			},
+		},
+	} {
+		t.Run(tc.desc, func(t *testing.T) {
+			isAncestor, err := repo.IsAncestor(ctx, tc.parent, tc.child)
+			if tc.errorMatcher != nil {
+				tc.errorMatcher(t, err)
+				return
+			}
+
+			require.NoError(t, err)
+			require.Equal(t, tc.isAncestor, isAncestor)
+		})
+	}
+}
diff --git a/internal/git/localrepo/objects.go b/internal/git/localrepo/objects.go
index 428462354..4239f5218 100644
--- a/internal/git/localrepo/objects.go
+++ b/internal/git/localrepo/objects.go
@@ -7,176 +7,36 @@ import (
 	"fmt"
 	"io"
 	"regexp"
-	"strings"
-	"time"
 
-	"gitlab.com/gitlab-org/gitaly/v16/internal/command"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git/catfile"
-	"gitlab.com/gitlab-org/gitaly/v16/internal/helper/text"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/structerr"
-	"gitlab.com/gitlab-org/gitaly/v16/proto/go/gitalypb"
 )
 
-// ErrObjectNotFound is returned in case an object could not be found.
-var ErrObjectNotFound = errors.New("object not found")
+// ObjectType is an Enum for the type of object of
+// the ls-tree entry, which can be can be tree, blob or commit
+type ObjectType int
 
-// WriteBlob writes a blob to the repository's object database and
-// returns its object ID. Path is used by git to decide which filters to
-// run on the content.
-func (repo *Repo) WriteBlob(ctx context.Context, path string, content io.Reader) (git.ObjectID, error) {
-	stdout := &bytes.Buffer{}
-	stderr := &bytes.Buffer{}
-
-	cmd, err := repo.Exec(ctx,
-		git.Command{
-			Name: "hash-object",
-			Flags: []git.Option{
-				git.ValueFlag{Name: "--path", Value: path},
-				git.Flag{Name: "--stdin"},
-				git.Flag{Name: "-w"},
-			},
-		},
-		git.WithStdin(content),
-		git.WithStdout(stdout),
-		git.WithStderr(stderr),
-	)
-	if err != nil {
-		return "", err
-	}
-
-	if err := cmd.Wait(); err != nil {
-		return "", errorWithStderr(err, stderr.Bytes())
-	}
-
-	objectHash, err := repo.ObjectHash(ctx)
-	if err != nil {
-		return "", fmt.Errorf("detecting object hash: %w", err)
-	}
-
-	oid, err := objectHash.FromHex(text.ChompBytes(stdout.Bytes()))
-	if err != nil {
-		return "", err
-	}
-
-	return oid, nil
-}
-
-// FormatTagError is used by FormatTag() below
-type FormatTagError struct {
-	expectedLines int
-	actualLines   int
-}
-
-func (e FormatTagError) Error() string {
-	return fmt.Sprintf("should have %d tag header lines, got %d", e.expectedLines, e.actualLines)
-}
-
-// FormatTag is used by WriteTag (or for testing) to make the tag
-// signature to feed to git-mktag, i.e. the plain-text mktag
-// format. This does not create an object, just crafts input for "git
-// mktag" to consume.
-//
-// We are not being paranoid about exhaustive input validation here
-// because we're just about to run git's own "fsck" check on this.
-//
-// However, if someone injected parameters with extra newlines they
-// could cause subsequent values to be ignored via a crafted
-// message. This someone could also locally craft a tag locally and
-// "git push" it. But allowing e.g. someone to provide their own
-// timestamp here would at best be annoying, and at worst run up
-// against some other assumption (e.g. that some hook check isn't as
-// strict on locally generated data).
-func FormatTag(
-	objectID git.ObjectID,
-	objectType string,
-	tagName, tagBody []byte,
-	committer *gitalypb.User,
-	committerDate time.Time,
-) (string, error) {
-	if committerDate.IsZero() {
-		committerDate = time.Now()
-	}
-
-	tagHeaderFormat := "object %s\n" +
-		"type %s\n" +
-		"tag %s\n" +
-		"tagger %s <%s> %s\n"
-	tagBuf := fmt.Sprintf(tagHeaderFormat, objectID.String(), objectType, tagName, committer.GetName(), committer.GetEmail(), git.FormatSignatureTime(committerDate))
-
-	maxHeaderLines := 4
-	actualHeaderLines := strings.Count(tagBuf, "\n")
-	if actualHeaderLines != maxHeaderLines {
-		return "", FormatTagError{expectedLines: maxHeaderLines, actualLines: actualHeaderLines}
-	}
-
-	tagBuf += "\n"
-	tagBuf += string(tagBody)
-
-	return tagBuf, nil
-}
-
-// MktagError is used by WriteTag() below
-type MktagError struct {
-	tagName []byte
-	stderr  string
-}
-
-func (e MktagError) Error() string {
-	// TODO: Upper-case error message purely for transitory backwards compatibility
-	return fmt.Sprintf("Could not update refs/tags/%s. Please refresh and try again.", e.tagName)
-}
-
-// WriteTag writes a tag to the repository's object database with
-// git-mktag and returns its object ID.
-//
-// It's important that this be git-mktag and not git-hash-object due
-// to its fsck sanity checking semantics.
-func (repo *Repo) WriteTag(
-	ctx context.Context,
-	objectID git.ObjectID,
-	objectType string,
-	tagName, tagBody []byte,
-	committer *gitalypb.User,
-	committerDate time.Time,
-) (git.ObjectID, error) {
-	stdout := &bytes.Buffer{}
-	stderr := &bytes.Buffer{}
-
-	tagBuf, err := FormatTag(objectID, objectType, tagName, tagBody, committer, committerDate)
-	if err != nil {
-		return "", err
-	}
-
-	content := strings.NewReader(tagBuf)
-
-	cmd, err := repo.Exec(ctx,
-		git.Command{
-			Name: "mktag",
-		},
-		git.WithStdin(content),
-		git.WithStdout(stdout),
-		git.WithStderr(stderr),
-	)
-	if err != nil {
-		return "", err
-	}
-
-	if err := cmd.Wait(); err != nil {
-		return "", MktagError{tagName: tagName, stderr: stderr.String()}
-	}
-
-	objectHash, err := repo.ObjectHash(ctx)
-	if err != nil {
-		return "", fmt.Errorf("detecting object hash: %w", err)
-	}
+// Enum values for ObjectType
+const (
+	Unknown ObjectType = iota
+	Tree
+	Blob
+	Submodule
+)
 
-	tagID, err := objectHash.FromHex(text.ChompBytes(stdout.Bytes()))
-	if err != nil {
-		return "", fmt.Errorf("could not parse tag ID: %w", err)
+// ObjectTypeFromString translates a string representation of the object type into an ObjectType enum.
+func ObjectTypeFromString(s string) ObjectType {
+	switch s {
+	case "tree":
+		return Tree
+	case "blob":
+		return Blob
+	case "commit":
+		return Submodule
+	default:
+		return Unknown
 	}
-
-	return tagID, nil
 }
 
 // InvalidObjectError is returned when trying to get an object id that is invalid or does not exist.
@@ -228,86 +88,6 @@ func (repo *Repo) ReadObject(ctx context.Context, oid git.ObjectID) ([]byte, err
 	return data, nil
 }
 
-type readCommitConfig struct {
-	withTrailers bool
-}
-
-// ReadCommitOpt is an option for ReadCommit.
-type ReadCommitOpt func(*readCommitConfig)
-
-// WithTrailers will cause ReadCommit to parse commit trailers.
-func WithTrailers() ReadCommitOpt {
-	return func(cfg *readCommitConfig) {
-		cfg.withTrailers = true
-	}
-}
-
-// ReadCommit reads the commit specified by the given revision. If no such
-// revision exists, it will return an ErrObjectNotFound error.
-func (repo *Repo) ReadCommit(ctx context.Context, revision git.Revision, opts ...ReadCommitOpt) (*gitalypb.GitCommit, error) {
-	var cfg readCommitConfig
-	for _, opt := range opts {
-		opt(&cfg)
-	}
-
-	objectReader, cancel, err := repo.catfileCache.ObjectReader(ctx, repo)
-	if err != nil {
-		return nil, err
-	}
-	defer cancel()
-
-	var commit *gitalypb.GitCommit
-	if cfg.withTrailers {
-		commit, err = catfile.GetCommitWithTrailers(ctx, repo.gitCmdFactory, repo, objectReader, revision)
-	} else {
-		commit, err = catfile.GetCommit(ctx, objectReader, revision)
-	}
-
-	if err != nil {
-		if errors.As(err, &catfile.NotFoundError{}) {
-			return nil, ErrObjectNotFound
-		}
-		return nil, err
-	}
-
-	return commit, nil
-}
-
-// InvalidCommitError is returned when the revision does not point to a valid commit object.
-type InvalidCommitError git.Revision
-
-func (err InvalidCommitError) Error() string {
-	return fmt.Sprintf("invalid commit: %q", string(err))
-}
-
-// IsAncestor returns whether the parent is an ancestor of the child. InvalidCommitError is returned
-// if either revision does not point to a commit in the repository.
-func (repo *Repo) IsAncestor(ctx context.Context, parent, child git.Revision) (bool, error) {
-	const notValidCommitName = "fatal: Not a valid commit name"
-
-	stderr := &bytes.Buffer{}
-	if err := repo.ExecAndWait(ctx,
-		git.Command{
-			Name:  "merge-base",
-			Flags: []git.Option{git.Flag{Name: "--is-ancestor"}},
-			Args:  []string{parent.String(), child.String()},
-		},
-		git.WithStderr(stderr),
-	); err != nil {
-		status, ok := command.ExitStatus(err)
-		if ok && status == 1 {
-			return false, nil
-		} else if ok && strings.HasPrefix(stderr.String(), notValidCommitName) {
-			commitOID := strings.TrimSpace(strings.TrimPrefix(stderr.String(), notValidCommitName))
-			return false, InvalidCommitError(commitOID)
-		}
-
-		return false, fmt.Errorf("determine ancestry: %w, stderr: %q", err, stderr)
-	}
-
-	return true, nil
-}
-
 // BadObjectError is returned when attempting to walk a bad object.
 type BadObjectError struct {
 	// ObjectID is the object id of the object that was bad.
diff --git a/internal/git/localrepo/objects_test.go b/internal/git/localrepo/objects_test.go
index d9075e5d4..fad157f32 100644
--- a/internal/git/localrepo/objects_test.go
+++ b/internal/git/localrepo/objects_test.go
@@ -2,244 +2,19 @@ package localrepo
 
 import (
 	"bytes"
-	"fmt"
-	"io"
-	"os"
-	"path/filepath"
 	"strings"
 	"testing"
-	"time"
 
-	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git/catfile"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git/gittest"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/config"
-	"gitlab.com/gitlab-org/gitaly/v16/internal/helper/perm"
-	"gitlab.com/gitlab-org/gitaly/v16/internal/helper/text"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/testhelper/testcfg"
-	"gitlab.com/gitlab-org/gitaly/v16/proto/go/gitalypb"
 	"google.golang.org/grpc/metadata"
 )
 
-type ReaderFunc func([]byte) (int, error)
-
-func (fn ReaderFunc) Read(b []byte) (int, error) { return fn(b) }
-
-func TestRepo_WriteBlob(t *testing.T) {
-	t.Parallel()
-
-	ctx := testhelper.Context(t)
-	_, repo, repoPath := setupRepo(t)
-
-	for _, tc := range []struct {
-		desc       string
-		attributes string
-		input      io.Reader
-		sha        string
-		error      error
-		content    string
-	}{
-		{
-			desc:  "error reading",
-			input: ReaderFunc(func([]byte) (int, error) { return 0, assert.AnError }),
-			error: assert.AnError,
-		},
-		{
-			desc:    "successful empty blob",
-			input:   strings.NewReader(""),
-			content: "",
-		},
-		{
-			desc:    "successful blob",
-			input:   strings.NewReader("some content"),
-			content: "some content",
-		},
-		{
-			desc:    "LF line endings left unmodified",
-			input:   strings.NewReader("\n"),
-			content: "\n",
-		},
-		{
-			desc:    "CRLF converted to LF due to global git config",
-			input:   strings.NewReader("\r\n"),
-			content: "\n",
-		},
-		{
-			desc:       "line endings preserved in binary files",
-			input:      strings.NewReader("\r\n"),
-			attributes: "file-path binary",
-			content:    "\r\n",
-		},
-	} {
-		t.Run(tc.desc, func(t *testing.T) {
-			attributesPath := filepath.Join(repoPath, "info", "attributes")
-			require.NoError(t, os.MkdirAll(filepath.Dir(attributesPath), perm.SharedDir))
-			require.NoError(t, os.WriteFile(attributesPath, []byte(tc.attributes), perm.PublicFile))
-
-			sha, err := repo.WriteBlob(ctx, "file-path", tc.input)
-			require.Equal(t, tc.error, err)
-			if tc.error != nil {
-				return
-			}
-
-			content, err := repo.ReadObject(ctx, sha)
-			require.NoError(t, err)
-			assert.Equal(t, tc.content, string(content))
-		})
-	}
-}
-
-func TestFormatTag(t *testing.T) {
-	t.Parallel()
-
-	for _, tc := range []struct {
-		desc       string
-		objectID   git.ObjectID
-		objectType string
-		tagName    []byte
-		tagBody    []byte
-		author     *gitalypb.User
-		authorDate time.Time
-		err        error
-	}{
-		// Just trivial tests here, most of this is tested in
-		// internal/gitaly/service/operations/tags_test.go
-		{
-			desc:       "basic signature",
-			objectID:   gittest.DefaultObjectHash.ZeroOID,
-			objectType: "commit",
-			tagName:    []byte("my-tag"),
-			author: &gitalypb.User{
-				Name:  []byte("root"),
-				Email: []byte("root@localhost"),
-			},
-			tagBody: []byte(""),
-		},
-		{
-			desc:       "basic signature",
-			objectID:   gittest.DefaultObjectHash.ZeroOID,
-			objectType: "commit",
-			tagName:    []byte("my-tag\ninjection"),
-			tagBody:    []byte(""),
-			author: &gitalypb.User{
-				Name:  []byte("root"),
-				Email: []byte("root@localhost"),
-			},
-			err: FormatTagError{expectedLines: 4, actualLines: 5},
-		},
-		{
-			desc:       "signature with fixed time",
-			objectID:   gittest.DefaultObjectHash.ZeroOID,
-			objectType: "commit",
-			tagName:    []byte("my-tag"),
-			tagBody:    []byte(""),
-			author: &gitalypb.User{
-				Name:  []byte("root"),
-				Email: []byte("root@localhost"),
-			},
-			authorDate: time.Unix(12345, 0),
-		},
-	} {
-		t.Run(tc.desc, func(t *testing.T) {
-			signature, err := FormatTag(tc.objectID, tc.objectType, tc.tagName, tc.tagBody, tc.author, tc.authorDate)
-			if err != nil {
-				require.Equal(t, tc.err, err)
-				require.Equal(t, "", signature)
-			} else {
-				require.NoError(t, err)
-				require.Contains(t, signature, "object ")
-				require.Contains(t, signature, "tag ")
-				require.Contains(t, signature, "tagger ")
-			}
-		})
-	}
-}
-
-func TestRepo_WriteTag(t *testing.T) {
-	t.Parallel()
-
-	ctx := testhelper.Context(t)
-
-	cfg, repo, repoPath := setupRepo(t)
-
-	commitID := gittest.WriteCommit(t, cfg, repoPath)
-
-	for _, tc := range []struct {
-		desc        string
-		objectID    git.ObjectID
-		objectType  string
-		tagName     []byte
-		tagBody     []byte
-		author      *gitalypb.User
-		authorDate  time.Time
-		expectedTag string
-	}{
-		// Just trivial tests here, most of this is tested in
-		// internal/gitaly/service/operations/tags_test.go
-		{
-			desc:       "basic signature",
-			objectID:   commitID,
-			objectType: "commit",
-			tagName:    []byte("my-tag"),
-			tagBody:    []byte(""),
-			author: &gitalypb.User{
-				Name:  []byte("root"),
-				Email: []byte("root@localhost"),
-			},
-		},
-		{
-			desc:       "signature with time",
-			objectID:   commitID,
-			objectType: "commit",
-			tagName:    []byte("tag-with-timestamp"),
-			tagBody:    []byte(""),
-			author: &gitalypb.User{
-				Name:  []byte("root"),
-				Email: []byte("root@localhost"),
-			},
-			authorDate: time.Unix(12345, 0).UTC(),
-			expectedTag: fmt.Sprintf(`object %s
-type commit
-tag tag-with-timestamp
-tagger root <root@localhost> 12345 +0000
-`, commitID),
-		},
-		{
-			desc:       "signature with time and timezone",
-			objectID:   commitID,
-			objectType: "commit",
-			tagName:    []byte("tag-with-timezone"),
-			tagBody:    []byte(""),
-			author: &gitalypb.User{
-				Name:  []byte("root"),
-				Email: []byte("root@localhost"),
-			},
-			authorDate: time.Unix(12345, 0).In(time.FixedZone("myzone", -60*60)),
-			expectedTag: fmt.Sprintf(`object %s
-type commit
-tag tag-with-timezone
-tagger root <root@localhost> 12345 -0100
-`, commitID),
-		},
-	} {
-		t.Run(tc.desc, func(t *testing.T) {
-			tagObjID, err := repo.WriteTag(ctx, tc.objectID, tc.objectType, tc.tagName, tc.tagBody, tc.author, tc.authorDate)
-			require.NoError(t, err)
-
-			repoTagObjID := gittest.Exec(t, cfg, "-C", repoPath, "rev-parse", tagObjID.String())
-			require.Equal(t, text.ChompBytes(repoTagObjID), tagObjID.String())
-
-			if tc.expectedTag != "" {
-				tag := gittest.Exec(t, cfg, "-C", repoPath, "cat-file", "-p", tagObjID.String())
-				require.Equal(t, tc.expectedTag, text.ChompBytes(tag))
-			}
-		})
-	}
-}
-
 func TestRepo_ReadObject(t *testing.T) {
 	t.Parallel()
 
@@ -346,226 +121,6 @@ func TestRepo_ReadObject_catfileCount(t *testing.T) {
 	gitCmdFactory.RequireCommandCount(t, "cat-file", 1)
 }
 
-func TestRepo_ReadCommit(t *testing.T) {
-	t.Parallel()
-
-	ctx := testhelper.Context(t)
-
-	cfg, repo, repoPath := setupRepo(t)
-
-	firstParentID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithMessage("first parent"))
-	secondParentID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithMessage("second parent"))
-
-	treeID := gittest.WriteTree(t, cfg, repoPath, []gittest.TreeEntry{
-		{Path: "file", Mode: "100644", Content: "content"},
-	})
-	commitWithoutTrailers := gittest.WriteCommit(t, cfg, repoPath,
-		gittest.WithParents(firstParentID, secondParentID),
-		gittest.WithTree(treeID),
-		gittest.WithMessage("subject\n\nbody\n"),
-		gittest.WithBranch("main"),
-	)
-	commitWithTrailers := gittest.WriteCommit(t, cfg, repoPath,
-		gittest.WithParents(commitWithoutTrailers),
-		gittest.WithTree(treeID),
-		gittest.WithMessage("with trailers\n\ntrailers\n\nSigned-off-by: John Doe <john.doe@example.com>"),
-	)
-
-	// We can't use git-commit-tree(1) directly, but have to manually write signed commits.
-	signedCommit := text.ChompBytes(gittest.ExecOpts(t, cfg, gittest.ExecConfig{
-		Stdin: strings.NewReader(fmt.Sprintf(
-			`tree %s
-parent %s
-author %[3]s
-committer %[3]s
-gpgsig -----BEGIN PGP SIGNATURE-----
-some faked pgp-signature
- -----END PGP SIGNATURE-----
-
-signed commit subject
-
-signed commit body
-`, treeID, firstParentID, gittest.DefaultCommitterSignature)),
-	}, "-C", repoPath, "hash-object", "-t", "commit", "-w", "--stdin"))
-
-	for _, tc := range []struct {
-		desc           string
-		revision       git.Revision
-		opts           []ReadCommitOpt
-		expectedCommit *gitalypb.GitCommit
-		expectedErr    error
-	}{
-		{
-			desc:        "invalid commit",
-			revision:    gittest.DefaultObjectHash.ZeroOID.Revision(),
-			expectedErr: ErrObjectNotFound,
-		},
-		{
-			desc:        "invalid commit with trailers",
-			revision:    gittest.DefaultObjectHash.ZeroOID.Revision(),
-			expectedErr: ErrObjectNotFound,
-			opts:        []ReadCommitOpt{WithTrailers()},
-		},
-		{
-			desc:     "valid commit",
-			revision: "refs/heads/main",
-			expectedCommit: &gitalypb.GitCommit{
-				Id:     commitWithoutTrailers.String(),
-				TreeId: treeID.String(),
-				ParentIds: []string{
-					firstParentID.String(),
-					secondParentID.String(),
-				},
-				Subject:   []byte("subject"),
-				Body:      []byte("subject\n\nbody\n"),
-				BodySize:  14,
-				Author:    gittest.DefaultCommitAuthor,
-				Committer: gittest.DefaultCommitAuthor,
-			},
-		},
-		{
-			desc:     "trailers do not get parsed without WithTrailers()",
-			revision: commitWithTrailers.Revision(),
-			expectedCommit: &gitalypb.GitCommit{
-				Id:     commitWithTrailers.String(),
-				TreeId: treeID.String(),
-				ParentIds: []string{
-					commitWithoutTrailers.String(),
-				},
-				Subject:   []byte("with trailers"),
-				Body:      []byte("with trailers\n\ntrailers\n\nSigned-off-by: John Doe <john.doe@example.com>"),
-				BodySize:  71,
-				Author:    gittest.DefaultCommitAuthor,
-				Committer: gittest.DefaultCommitAuthor,
-			},
-		},
-		{
-			desc:     "trailers get parsed with WithTrailers()",
-			revision: commitWithTrailers.Revision(),
-			opts:     []ReadCommitOpt{WithTrailers()},
-			expectedCommit: &gitalypb.GitCommit{
-				Id:     commitWithTrailers.String(),
-				TreeId: treeID.String(),
-				ParentIds: []string{
-					commitWithoutTrailers.String(),
-				},
-				Subject:   []byte("with trailers"),
-				Body:      []byte("with trailers\n\ntrailers\n\nSigned-off-by: John Doe <john.doe@example.com>"),
-				BodySize:  71,
-				Author:    gittest.DefaultCommitAuthor,
-				Committer: gittest.DefaultCommitAuthor,
-				Trailers: []*gitalypb.CommitTrailer{
-					{
-						Key:   []byte("Signed-off-by"),
-						Value: []byte("John Doe <john.doe@example.com>"),
-					},
-				},
-			},
-		},
-		{
-			desc:     "with PGP signature",
-			revision: git.Revision(signedCommit),
-			opts:     []ReadCommitOpt{},
-			expectedCommit: &gitalypb.GitCommit{
-				Id:     signedCommit,
-				TreeId: treeID.String(),
-				ParentIds: []string{
-					firstParentID.String(),
-				},
-				Subject:       []byte("signed commit subject"),
-				Body:          []byte("signed commit subject\n\nsigned commit body\n"),
-				BodySize:      42,
-				Author:        gittest.DefaultCommitAuthor,
-				Committer:     gittest.DefaultCommitAuthor,
-				SignatureType: gitalypb.SignatureType_PGP,
-			},
-		},
-		{
-			desc:        "not a commit",
-			revision:    "refs/heads/main^{tree}",
-			expectedErr: ErrObjectNotFound,
-		},
-	} {
-		t.Run(tc.desc, func(t *testing.T) {
-			commit, err := repo.ReadCommit(ctx, tc.revision, tc.opts...)
-			require.Equal(t, tc.expectedErr, err)
-			require.Equal(t, tc.expectedCommit, commit)
-		})
-	}
-}
-
-func TestRepo_IsAncestor(t *testing.T) {
-	t.Parallel()
-
-	ctx := testhelper.Context(t)
-
-	cfg, repo, repoPath := setupRepo(t)
-
-	parentCommitID := gittest.WriteCommit(t, cfg, repoPath)
-	childCommitID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithParents(parentCommitID))
-
-	for _, tc := range []struct {
-		desc         string
-		parent       git.Revision
-		child        git.Revision
-		isAncestor   bool
-		errorMatcher func(testing.TB, error)
-	}{
-		{
-			desc:       "parent is ancestor",
-			parent:     parentCommitID.Revision(),
-			child:      childCommitID.Revision(),
-			isAncestor: true,
-		},
-		{
-			desc:       "parent is not ancestor",
-			parent:     childCommitID.Revision(),
-			child:      parentCommitID.Revision(),
-			isAncestor: false,
-		},
-		{
-			desc:   "parent is not valid commit",
-			parent: gittest.DefaultObjectHash.ZeroOID.Revision(),
-			child:  childCommitID.Revision(),
-			errorMatcher: func(tb testing.TB, err error) {
-				require.Equal(tb, InvalidCommitError(gittest.DefaultObjectHash.ZeroOID), err)
-			},
-		},
-		{
-			desc:   "child is not valid commit",
-			parent: childCommitID.Revision(),
-			child:  gittest.DefaultObjectHash.ZeroOID.Revision(),
-			errorMatcher: func(tb testing.TB, err error) {
-				require.Equal(tb, InvalidCommitError(gittest.DefaultObjectHash.ZeroOID), err)
-			},
-		},
-		{
-			desc:   "child points to a tree",
-			parent: childCommitID.Revision(),
-			child:  childCommitID.Revision() + "^{tree}",
-			errorMatcher: func(tb testing.TB, actualErr error) {
-				treeOID, err := repo.ResolveRevision(ctx, childCommitID.Revision()+"^{tree}")
-				require.NoError(tb, err)
-				require.EqualError(tb, actualErr, fmt.Sprintf(
-					`determine ancestry: exit status 128, stderr: "error: object %s is a tree, not a commit\nfatal: Not a valid commit name %s^{tree}\n"`,
-					treeOID, childCommitID,
-				))
-			},
-		},
-	} {
-		t.Run(tc.desc, func(t *testing.T) {
-			isAncestor, err := repo.IsAncestor(ctx, tc.parent, tc.child)
-			if tc.errorMatcher != nil {
-				tc.errorMatcher(t, err)
-				return
-			}
-
-			require.NoError(t, err)
-			require.Equal(t, tc.isAncestor, isAncestor)
-		})
-	}
-}
-
 func TestWalkUnreachableObjects(t *testing.T) {
 	t.Parallel()
 
diff --git a/internal/git/localrepo/parser.go b/internal/git/localrepo/parser.go
index f5f2e9932..1b8c7469c 100644
--- a/internal/git/localrepo/parser.go
+++ b/internal/git/localrepo/parser.go
@@ -66,7 +66,7 @@ func (p *Parser) NextEntry() (*TreeEntry, error) {
 		Mode: string(treeEntryMode),
 		OID:  objectID,
 		Path: string(treeEntryPath),
-		Type: ToEnum(string(treeEntryType)),
+		Type: ObjectTypeFromString(string(treeEntryType)),
 	}, nil
 }
 
diff --git a/internal/git/localrepo/tag.go b/internal/git/localrepo/tag.go
new file mode 100644
index 000000000..483fdbeb6
--- /dev/null
+++ b/internal/git/localrepo/tag.go
@@ -0,0 +1,130 @@
+package localrepo
+
+import (
+	"bytes"
+	"context"
+	"fmt"
+	"strings"
+	"time"
+
+	"gitlab.com/gitlab-org/gitaly/v16/internal/git"
+	"gitlab.com/gitlab-org/gitaly/v16/internal/helper/text"
+	"gitlab.com/gitlab-org/gitaly/v16/proto/go/gitalypb"
+)
+
+// FormatTagError is used by FormatTag() below
+type FormatTagError struct {
+	expectedLines int
+	actualLines   int
+}
+
+func (e FormatTagError) Error() string {
+	return fmt.Sprintf("should have %d tag header lines, got %d", e.expectedLines, e.actualLines)
+}
+
+// FormatTag is used by WriteTag (or for testing) to make the tag
+// signature to feed to git-mktag, i.e. the plain-text mktag
+// format. This does not create an object, just crafts input for "git
+// mktag" to consume.
+//
+// We are not being paranoid about exhaustive input validation here
+// because we're just about to run git's own "fsck" check on this.
+//
+// However, if someone injected parameters with extra newlines they
+// could cause subsequent values to be ignored via a crafted
+// message. This someone could also locally craft a tag locally and
+// "git push" it. But allowing e.g. someone to provide their own
+// timestamp here would at best be annoying, and at worst run up
+// against some other assumption (e.g. that some hook check isn't as
+// strict on locally generated data).
+func FormatTag(
+	objectID git.ObjectID,
+	objectType string,
+	tagName, tagBody []byte,
+	committer *gitalypb.User,
+	committerDate time.Time,
+) (string, error) {
+	if committerDate.IsZero() {
+		committerDate = time.Now()
+	}
+
+	tagHeaderFormat := "object %s\n" +
+		"type %s\n" +
+		"tag %s\n" +
+		"tagger %s <%s> %s\n"
+	tagBuf := fmt.Sprintf(tagHeaderFormat, objectID.String(), objectType, tagName, committer.GetName(), committer.GetEmail(), git.FormatSignatureTime(committerDate))
+
+	maxHeaderLines := 4
+	actualHeaderLines := strings.Count(tagBuf, "\n")
+	if actualHeaderLines != maxHeaderLines {
+		return "", FormatTagError{expectedLines: maxHeaderLines, actualLines: actualHeaderLines}
+	}
+
+	tagBuf += "\n"
+	tagBuf += string(tagBody)
+
+	return tagBuf, nil
+}
+
+// MktagError is used by WriteTag() below
+type MktagError struct {
+	tagName []byte
+	stderr  string
+}
+
+func (e MktagError) Error() string {
+	// TODO: Upper-case error message purely for transitory backwards compatibility
+	return fmt.Sprintf("Could not update refs/tags/%s. Please refresh and try again.", e.tagName)
+}
+
+// WriteTag writes a tag to the repository's object database with
+// git-mktag and returns its object ID.
+//
+// It's important that this be git-mktag and not git-hash-object due
+// to its fsck sanity checking semantics.
+func (repo *Repo) WriteTag(
+	ctx context.Context,
+	objectID git.ObjectID,
+	objectType string,
+	tagName, tagBody []byte,
+	committer *gitalypb.User,
+	committerDate time.Time,
+) (git.ObjectID, error) {
+	stdout := &bytes.Buffer{}
+	stderr := &bytes.Buffer{}
+
+	tagBuf, err := FormatTag(objectID, objectType, tagName, tagBody, committer, committerDate)
+	if err != nil {
+		return "", err
+	}
+
+	content := strings.NewReader(tagBuf)
+
+	cmd, err := repo.Exec(ctx,
+		git.Command{
+			Name: "mktag",
+		},
+		git.WithStdin(content),
+		git.WithStdout(stdout),
+		git.WithStderr(stderr),
+	)
+	if err != nil {
+		return "", err
+	}
+
+	if err := cmd.Wait(); err != nil {
+		return "", MktagError{tagName: tagName, stderr: stderr.String()}
+	}
+
+	objectHash, err := repo.ObjectHash(ctx)
+	if err != nil {
+		return "", fmt.Errorf("detecting object hash: %w", err)
+	}
+
+	tagID, err := objectHash.FromHex(text.ChompBytes(stdout.Bytes()))
+	if err != nil {
+		return "", fmt.Errorf("could not parse tag ID: %w", err)
+	}
+
+	return tagID, nil
+}
diff --git a/internal/git/localrepo/tag_test.go b/internal/git/localrepo/tag_test.go
new file mode 100644
index 000000000..224aff23b
--- /dev/null
+++ b/internal/git/localrepo/tag_test.go
@@ -0,0 +1,162 @@
+package localrepo
+
+import (
+	"fmt"
+	"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/helper/text"
+	"gitlab.com/gitlab-org/gitaly/v16/internal/testhelper"
+	"gitlab.com/gitlab-org/gitaly/v16/proto/go/gitalypb"
+)
+
+func TestFormatTag(t *testing.T) {
+	t.Parallel()
+
+	for _, tc := range []struct {
+		desc       string
+		objectID   git.ObjectID
+		objectType string
+		tagName    []byte
+		tagBody    []byte
+		author     *gitalypb.User
+		authorDate time.Time
+		err        error
+	}{
+		// Just trivial tests here, most of this is tested in
+		// internal/gitaly/service/operations/tags_test.go
+		{
+			desc:       "basic signature",
+			objectID:   gittest.DefaultObjectHash.ZeroOID,
+			objectType: "commit",
+			tagName:    []byte("my-tag"),
+			author: &gitalypb.User{
+				Name:  []byte("root"),
+				Email: []byte("root@localhost"),
+			},
+			tagBody: []byte(""),
+		},
+		{
+			desc:       "basic signature",
+			objectID:   gittest.DefaultObjectHash.ZeroOID,
+			objectType: "commit",
+			tagName:    []byte("my-tag\ninjection"),
+			tagBody:    []byte(""),
+			author: &gitalypb.User{
+				Name:  []byte("root"),
+				Email: []byte("root@localhost"),
+			},
+			err: FormatTagError{expectedLines: 4, actualLines: 5},
+		},
+		{
+			desc:       "signature with fixed time",
+			objectID:   gittest.DefaultObjectHash.ZeroOID,
+			objectType: "commit",
+			tagName:    []byte("my-tag"),
+			tagBody:    []byte(""),
+			author: &gitalypb.User{
+				Name:  []byte("root"),
+				Email: []byte("root@localhost"),
+			},
+			authorDate: time.Unix(12345, 0),
+		},
+	} {
+		t.Run(tc.desc, func(t *testing.T) {
+			signature, err := FormatTag(tc.objectID, tc.objectType, tc.tagName, tc.tagBody, tc.author, tc.authorDate)
+			if err != nil {
+				require.Equal(t, tc.err, err)
+				require.Equal(t, "", signature)
+			} else {
+				require.NoError(t, err)
+				require.Contains(t, signature, "object ")
+				require.Contains(t, signature, "tag ")
+				require.Contains(t, signature, "tagger ")
+			}
+		})
+	}
+}
+
+func TestRepo_WriteTag(t *testing.T) {
+	t.Parallel()
+
+	ctx := testhelper.Context(t)
+
+	cfg, repo, repoPath := setupRepo(t)
+
+	commitID := gittest.WriteCommit(t, cfg, repoPath)
+
+	for _, tc := range []struct {
+		desc        string
+		objectID    git.ObjectID
+		objectType  string
+		tagName     []byte
+		tagBody     []byte
+		author      *gitalypb.User
+		authorDate  time.Time
+		expectedTag string
+	}{
+		// Just trivial tests here, most of this is tested in
+		// internal/gitaly/service/operations/tags_test.go
+		{
+			desc:       "basic signature",
+			objectID:   commitID,
+			objectType: "commit",
+			tagName:    []byte("my-tag"),
+			tagBody:    []byte(""),
+			author: &gitalypb.User{
+				Name:  []byte("root"),
+				Email: []byte("root@localhost"),
+			},
+		},
+		{
+			desc:       "signature with time",
+			objectID:   commitID,
+			objectType: "commit",
+			tagName:    []byte("tag-with-timestamp"),
+			tagBody:    []byte(""),
+			author: &gitalypb.User{
+				Name:  []byte("root"),
+				Email: []byte("root@localhost"),
+			},
+			authorDate: time.Unix(12345, 0).UTC(),
+			expectedTag: fmt.Sprintf(`object %s
+type commit
+tag tag-with-timestamp
+tagger root <root@localhost> 12345 +0000
+`, commitID),
+		},
+		{
+			desc:       "signature with time and timezone",
+			objectID:   commitID,
+			objectType: "commit",
+			tagName:    []byte("tag-with-timezone"),
+			tagBody:    []byte(""),
+			author: &gitalypb.User{
+				Name:  []byte("root"),
+				Email: []byte("root@localhost"),
+			},
+			authorDate: time.Unix(12345, 0).In(time.FixedZone("myzone", -60*60)),
+			expectedTag: fmt.Sprintf(`object %s
+type commit
+tag tag-with-timezone
+tagger root <root@localhost> 12345 -0100
+`, commitID),
+		},
+	} {
+		t.Run(tc.desc, func(t *testing.T) {
+			tagObjID, err := repo.WriteTag(ctx, tc.objectID, tc.objectType, tc.tagName, tc.tagBody, tc.author, tc.authorDate)
+			require.NoError(t, err)
+
+			repoTagObjID := gittest.Exec(t, cfg, "-C", repoPath, "rev-parse", tagObjID.String())
+			require.Equal(t, text.ChompBytes(repoTagObjID), tagObjID.String())
+
+			if tc.expectedTag != "" {
+				tag := gittest.Exec(t, cfg, "-C", repoPath, "cat-file", "-p", tagObjID.String())
+				require.Equal(t, tc.expectedTag, text.ChompBytes(tag))
+			}
+		})
+	}
+}
diff --git a/internal/git/localrepo/tree.go b/internal/git/localrepo/tree.go
index 12c62a4a2..b436e4c56 100644
--- a/internal/git/localrepo/tree.go
+++ b/internal/git/localrepo/tree.go
@@ -15,21 +15,9 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v16/internal/structerr"
 )
 
-// ObjectType is an Enum for the type of object of
-// the ls-tree entry, which can be can be tree, blob or commit
-type ObjectType int
-
 // Entries holds every ls-tree Entry
 type Entries []TreeEntry
 
-// Enum values for ObjectType
-const (
-	Unknown ObjectType = iota
-	Tree
-	Blob
-	Submodule
-)
-
 func (e Entries) Len() int {
 	return len(e)
 }
@@ -70,21 +58,6 @@ func (b TreeEntriesByPath) Less(i, j int) bool {
 	return iPath < jPath
 }
 
-// ToEnum translates a string representation of the object type into an
-// ObjectType enum.
-func ToEnum(s string) ObjectType {
-	switch s {
-	case "tree":
-		return Tree
-	case "blob":
-		return Blob
-	case "commit":
-		return Submodule
-	default:
-		return Unknown
-	}
-}
-
 // TreeEntry represents an entry of a git tree object.
 type TreeEntry struct {
 	// OID is the object ID the tree entry refers to.
diff --git a/internal/git/localrepo/tree_test.go b/internal/git/localrepo/tree_test.go
index e96923dff..64dc77418 100644
--- a/internal/git/localrepo/tree_test.go
+++ b/internal/git/localrepo/tree_test.go
@@ -794,7 +794,7 @@ func requireTreeEquals(
 			)
 			require.NoError(t, err)
 
-			require.Equal(t, expect.Entries[i].Type, ToEnum(objectInfo.Type))
+			require.Equal(t, expect.Entries[i].Type, ObjectTypeFromString(objectInfo.Type))
 		}
 
 		if entry.Type == Tree {