catfile: Convert interface to accept Revisions

Instead of accepting untyped strings, this commit refactors the catfile
interface to accept Revisions instead.

19 files changed, 124 insertions, 118 deletions

diff --git a/internal/git/catfile/batch.go b/internal/git/catfile/batch.go
index fe52ed339..fb9245b5c 100644
--- a/internal/git/catfile/batch.go
+++ b/internal/git/catfile/batch.go
@@ -77,7 +77,7 @@ func newBatchProcess(ctx context.Context, locator storage.Locator, repo reposito
 	return b, nil
 }
 
-func (b *batchProcess) reader(revspec string, expectedType string) (*Object, error) {
+func (b *batchProcess) reader(revision git.Revision, expectedType string) (*Object, error) {
 	b.Lock()
 	defer b.Unlock()
 
@@ -93,7 +93,7 @@ func (b *batchProcess) reader(revspec string, expectedType string) (*Object, err
 		return nil, fmt.Errorf("cannot create new Object: batch contains %d unread bytes", b.n)
 	}
 
-	if _, err := fmt.Fprintln(b.w, revspec); err != nil {
+	if _, err := fmt.Fprintln(b.w, revision.String()); err != nil {
 		return nil, err
 	}
 
diff --git a/internal/git/catfile/batchcheck.go b/internal/git/catfile/batchcheck.go
index 8c2a8d8d3..46e6834ff 100644
--- a/internal/git/catfile/batchcheck.go
+++ b/internal/git/catfile/batchcheck.go
@@ -60,11 +60,11 @@ func newBatchCheck(ctx context.Context, locator storage.Locator, repo repository
 	return bc, nil
 }
 
-func (bc *batchCheck) info(spec string) (*ObjectInfo, error) {
+func (bc *batchCheck) info(revision git.Revision) (*ObjectInfo, error) {
 	bc.Lock()
 	defer bc.Unlock()
 
-	if _, err := fmt.Fprintln(bc.w, spec); err != nil {
+	if _, err := fmt.Fprintln(bc.w, revision.String()); err != nil {
 		return nil, err
 	}
 
diff --git a/internal/git/catfile/catfile.go b/internal/git/catfile/catfile.go
index f07026c61..748243731 100644
--- a/internal/git/catfile/catfile.go
+++ b/internal/git/catfile/catfile.go
@@ -6,6 +6,7 @@ import (
 
 	"github.com/opentracing/opentracing-go"
 	"github.com/prometheus/client_golang/prometheus"
+	"gitlab.com/gitlab-org/gitaly/internal/git"
 	"gitlab.com/gitlab-org/gitaly/internal/git/repository"
 	"gitlab.com/gitlab-org/gitaly/internal/metadata"
 	"gitlab.com/gitlab-org/gitaly/internal/storage"
@@ -63,11 +64,11 @@ func init() {
 // A Batch instance can only serve single request at a time. If you want to
 // use it across multiple goroutines you need to add your own locking.
 type Batch interface {
-	Info(ctx context.Context, revspec string) (*ObjectInfo, error)
-	Tree(ctx context.Context, revspec string) (*Object, error)
-	Commit(ctx context.Context, revspec string) (*Object, error)
-	Blob(ctx context.Context, revspec string) (*Object, error)
-	Tag(ctx context.Context, revspec string) (*Object, error)
+	Info(ctx context.Context, revision git.Revision) (*ObjectInfo, error)
+	Tree(ctx context.Context, revision git.Revision) (*Object, error)
+	Commit(ctx context.Context, revision git.Revision) (*Object, error)
+	Blob(ctx context.Context, revision git.Revision) (*Object, error)
+	Tag(ctx context.Context, revision git.Revision) (*Object, error)
 }
 
 type batch struct {
@@ -78,41 +79,41 @@ type batch struct {
 	closed bool
 }
 
-// Info returns an ObjectInfo if spec exists. If spec does not exist the
-// error is of type NotFoundError.
-func (c *batch) Info(ctx context.Context, revspec string) (*ObjectInfo, error) {
-	return c.batchCheck.info(revspec)
+// Info returns an ObjectInfo if spec exists. If the revision does not exist
+// the error is of type NotFoundError.
+func (c *batch) Info(ctx context.Context, revision git.Revision) (*ObjectInfo, error) {
+	return c.batchCheck.info(revision)
 }
 
-// Tree returns a raw tree object. It is an error if revspec does not
-// point to a tree. To prevent this firstuse Info to resolve the revspec
-// and check the object type. Caller must consume the Reader before
-// making another call on C.
-func (c *batch) Tree(ctx context.Context, revspec string) (*Object, error) {
-	return c.batchProcess.reader(revspec, "tree")
+// Tree returns a raw tree object. It is an error if the revision does not
+// point to a tree. To prevent this, use Info to resolve the revision and check
+// the object type. Caller must consume the Reader before making another call
+// on C.
+func (c *batch) Tree(ctx context.Context, revision git.Revision) (*Object, error) {
+	return c.batchProcess.reader(revision, "tree")
 }
 
-// Commit returns a raw commit object. It is an error if revspec does not
-// point to a commit. To prevent this first use Info to resolve the revspec
-// and check the object type. Caller must consume the Reader before
-// making another call on C.
-func (c *batch) Commit(ctx context.Context, revspec string) (*Object, error) {
-	return c.batchProcess.reader(revspec, "commit")
+// Commit returns a raw commit object. It is an error if the revision does not
+// point to a commit. To prevent this, use Info to resolve the revision and
+// check the object type. Caller must consume the Reader before making another
+// call on C.
+func (c *batch) Commit(ctx context.Context, revision git.Revision) (*Object, error) {
+	return c.batchProcess.reader(revision, "commit")
 }
 
 // Blob returns a reader for the requested blob. The entire blob must be
 // read before any new objects can be requested from this Batch instance.
 //
-// It is an error if revspec does not point to a blob. To prevent this
-// first use Info to resolve the revspec and check the object type.
-func (c *batch) Blob(ctx context.Context, revspec string) (*Object, error) {
-	return c.batchProcess.reader(revspec, "blob")
+// It is an error if the revision does not point to a blob. To prevent this,
+// use Info to resolve the revision and check the object type.
+func (c *batch) Blob(ctx context.Context, revision git.Revision) (*Object, error) {
+	return c.batchProcess.reader(revision, "blob")
 }
 
 // Tag returns a raw tag object. Caller must consume the Reader before
 // making another call on C.
-func (c *batch) Tag(ctx context.Context, revspec string) (*Object, error) {
-	return c.batchProcess.reader(revspec, "tag")
+func (c *batch) Tag(ctx context.Context, revision git.Revision) (*Object, error) {
+	return c.batchProcess.reader(revision, "tag")
 }
 
 // Close closes the writers for batchCheck and batch. This is only used for
@@ -230,47 +231,47 @@ type instrumentedBatch struct {
 	Batch
 }
 
-func (ib *instrumentedBatch) Info(ctx context.Context, revspec string) (*ObjectInfo, error) {
-	span, ctx := opentracing.StartSpanFromContext(ctx, "Batch.Info", opentracing.Tag{"revspec", revspec})
+func (ib *instrumentedBatch) Info(ctx context.Context, revision git.Revision) (*ObjectInfo, error) {
+	span, ctx := opentracing.StartSpanFromContext(ctx, "Batch.Info", opentracing.Tag{"revision", revision})
 	defer span.Finish()
 
 	catfileLookupCounter.WithLabelValues("info").Inc()
 
-	return ib.Batch.Info(ctx, revspec)
+	return ib.Batch.Info(ctx, revision)
 }
 
-func (ib *instrumentedBatch) Tree(ctx context.Context, revspec string) (*Object, error) {
-	span, ctx := opentracing.StartSpanFromContext(ctx, "Batch.Tree", opentracing.Tag{"revspec", revspec})
+func (ib *instrumentedBatch) Tree(ctx context.Context, revision git.Revision) (*Object, error) {
+	span, ctx := opentracing.StartSpanFromContext(ctx, "Batch.Tree", opentracing.Tag{"revision", revision})
 	defer span.Finish()
 
 	catfileLookupCounter.WithLabelValues("tree").Inc()
 
-	return ib.Batch.Tree(ctx, revspec)
+	return ib.Batch.Tree(ctx, revision)
 }
 
-func (ib *instrumentedBatch) Commit(ctx context.Context, revspec string) (*Object, error) {
-	span, ctx := opentracing.StartSpanFromContext(ctx, "Batch.Commit", opentracing.Tag{"revspec", revspec})
+func (ib *instrumentedBatch) Commit(ctx context.Context, revision git.Revision) (*Object, error) {
+	span, ctx := opentracing.StartSpanFromContext(ctx, "Batch.Commit", opentracing.Tag{"revision", revision})
 	defer span.Finish()
 
 	catfileLookupCounter.WithLabelValues("commit").Inc()
 
-	return ib.Batch.Commit(ctx, revspec)
+	return ib.Batch.Commit(ctx, revision)
 }
 
-func (ib *instrumentedBatch) Blob(ctx context.Context, revspec string) (*Object, error) {
-	span, ctx := opentracing.StartSpanFromContext(ctx, "Batch.Blob", opentracing.Tag{"revspec", revspec})
+func (ib *instrumentedBatch) Blob(ctx context.Context, revision git.Revision) (*Object, error) {
+	span, ctx := opentracing.StartSpanFromContext(ctx, "Batch.Blob", opentracing.Tag{"revision", revision})
 	defer span.Finish()
 
 	catfileLookupCounter.WithLabelValues("blob").Inc()
 
-	return ib.Batch.Blob(ctx, revspec)
+	return ib.Batch.Blob(ctx, revision)
 }
 
-func (ib *instrumentedBatch) Tag(ctx context.Context, revspec string) (*Object, error) {
-	span, ctx := opentracing.StartSpanFromContext(ctx, "Batch.Tag", opentracing.Tag{"revspec", revspec})
+func (ib *instrumentedBatch) Tag(ctx context.Context, revision git.Revision) (*Object, error) {
+	span, ctx := opentracing.StartSpanFromContext(ctx, "Batch.Tag", opentracing.Tag{"revision", revision})
 	defer span.Finish()
 
 	catfileLookupCounter.WithLabelValues("tag").Inc()
 
-	return ib.Batch.Tag(ctx, revspec)
+	return ib.Batch.Tag(ctx, revision)
 }
diff --git a/internal/git/catfile/catfile_test.go b/internal/git/catfile/catfile_test.go
index 9f0a68274..71e4c31e0 100644
--- a/internal/git/catfile/catfile_test.go
+++ b/internal/git/catfile/catfile_test.go
@@ -13,6 +13,7 @@ import (
 
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly/internal/command"
+	"gitlab.com/gitlab-org/gitaly/internal/git"
 	"gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
 	"gitlab.com/gitlab-org/gitaly/internal/helper/text"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
@@ -31,13 +32,13 @@ func TestInfo(t *testing.T) {
 	require.NoError(t, err)
 
 	testCases := []struct {
-		desc   string
-		spec   string
-		output *ObjectInfo
+		desc     string
+		revision string
+		output   *ObjectInfo
 	}{
 		{
-			desc: "gitignore",
-			spec: "60ecb67744cb56576c30214ff52294f8ce2def98:.gitignore",
+			desc:     "gitignore",
+			revision: "60ecb67744cb56576c30214ff52294f8ce2def98:.gitignore",
 			output: &ObjectInfo{
 				Oid:  "dfaa3f97ca337e20154a98ac9d0be76ddd1fcc82",
 				Type: "blob",
@@ -48,7 +49,7 @@ func TestInfo(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.desc, func(t *testing.T) {
-			oi, err := c.Info(ctx, tc.spec)
+			oi, err := c.Info(ctx, git.Revision(tc.revision))
 			require.NoError(t, err)
 
 			require.Equal(t, tc.output, oi)
@@ -71,14 +72,14 @@ func TestBlob(t *testing.T) {
 
 	testCases := []struct {
 		desc       string
-		spec       string
+		revision   string
 		objInfo    ObjectInfo
 		content    []byte
 		requireErr func(*testing.T, error)
 	}{
 		{
-			desc: "gitignore",
-			spec: "60ecb67744cb56576c30214ff52294f8ce2def98:.gitignore",
+			desc:     "gitignore",
+			revision: "60ecb67744cb56576c30214ff52294f8ce2def98:.gitignore",
 			objInfo: ObjectInfo{
 				Oid:  "dfaa3f97ca337e20154a98ac9d0be76ddd1fcc82",
 				Type: "blob",
@@ -87,16 +88,16 @@ func TestBlob(t *testing.T) {
 			content: gitignoreBytes,
 		},
 		{
-			desc: "not existing ref",
-			spec: "stub",
+			desc:     "not existing ref",
+			revision: "stub",
 			requireErr: func(t *testing.T, err error) {
 				require.True(t, IsNotFound(err), "the error must be from 'not found' family")
 				require.EqualError(t, err, "object not found")
 			},
 		},
 		{
-			desc: "wrong object type",
-			spec: "1e292f8fedd741b75372e19097c76d327140c312", // is commit SHA1
+			desc:     "wrong object type",
+			revision: "1e292f8fedd741b75372e19097c76d327140c312", // is commit SHA1
 			requireErr: func(t *testing.T, err error) {
 				require.True(t, IsNotFound(err), "the error must be from 'not found' family")
 				require.EqualError(t, err, "expected 1e292f8fedd741b75372e19097c76d327140c312 to be a blob, got commit")
@@ -106,7 +107,7 @@ func TestBlob(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.desc, func(t *testing.T) {
-			blobObj, err := c.Blob(ctx, tc.spec)
+			blobObj, err := c.Blob(ctx, git.Revision(tc.revision))
 
 			if tc.requireErr != nil {
 				tc.requireErr(t, err)
@@ -137,20 +138,20 @@ func TestCommit(t *testing.T) {
 	require.NoError(t, err)
 
 	testCases := []struct {
-		desc   string
-		spec   string
-		output string
+		desc     string
+		revision string
+		output   string
 	}{
 		{
-			desc:   "commit with non-oid spec",
-			spec:   "60ecb67744cb56576c30214ff52294f8ce2def98^",
-			output: string(commitBytes),
+			desc:     "commit with non-oid spec",
+			revision: "60ecb67744cb56576c30214ff52294f8ce2def98^",
+			output:   string(commitBytes),
 		},
 	}
 
 	for _, tc := range testCases {
 		t.Run(tc.desc, func(t *testing.T) {
-			commitReader, err := c.Commit(ctx, tc.spec)
+			commitReader, err := c.Commit(ctx, git.Revision(tc.revision))
 			require.NoError(t, err)
 
 			contents, err := ioutil.ReadAll(commitReader)
@@ -176,14 +177,14 @@ func TestTag(t *testing.T) {
 
 	testCases := []struct {
 		desc       string
-		spec       string
+		revision   string
 		objInfo    ObjectInfo
 		content    []byte
 		requireErr func(*testing.T, error)
 	}{
 		{
-			desc: "tag",
-			spec: "f4e6814c3e4e7a0de82a9e7cd20c626cc963a2f8",
+			desc:     "tag",
+			revision: "f4e6814c3e4e7a0de82a9e7cd20c626cc963a2f8",
 			objInfo: ObjectInfo{
 				Oid:  "f4e6814c3e4e7a0de82a9e7cd20c626cc963a2f8",
 				Type: "tag",
@@ -192,16 +193,16 @@ func TestTag(t *testing.T) {
 			content: tagBytes,
 		},
 		{
-			desc: "not existing ref",
-			spec: "stub",
+			desc:     "not existing ref",
+			revision: "stub",
 			requireErr: func(t *testing.T, err error) {
 				require.True(t, IsNotFound(err), "the error must be from 'not found' family")
 				require.EqualError(t, err, "object not found")
 			},
 		},
 		{
-			desc: "wrong object type",
-			spec: "1e292f8fedd741b75372e19097c76d327140c312", // is commit SHA1
+			desc:     "wrong object type",
+			revision: "1e292f8fedd741b75372e19097c76d327140c312", // is commit SHA1
 			requireErr: func(t *testing.T, err error) {
 				require.True(t, IsNotFound(err), "the error must be from 'not found' family")
 				require.EqualError(t, err, "expected 1e292f8fedd741b75372e19097c76d327140c312 to be a tag, got commit")
@@ -211,7 +212,7 @@ func TestTag(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.desc, func(t *testing.T) {
-			tagObj, err := c.Tag(ctx, tc.spec)
+			tagObj, err := c.Tag(ctx, git.Revision(tc.revision))
 
 			if tc.requireErr != nil {
 				tc.requireErr(t, err)
@@ -243,14 +244,14 @@ func TestTree(t *testing.T) {
 
 	testCases := []struct {
 		desc       string
-		spec       string
+		revision   string
 		objInfo    ObjectInfo
 		content    []byte
 		requireErr func(*testing.T, error)
 	}{
 		{
-			desc: "tree with non-oid spec",
-			spec: "60ecb67744cb56576c30214ff52294f8ce2def98^{tree}",
+			desc:     "tree with non-oid spec",
+			revision: "60ecb67744cb56576c30214ff52294f8ce2def98^{tree}",
 			objInfo: ObjectInfo{
 				Oid:  "7e2f26d033ee47cd0745649d1a28277c56197921",
 				Type: "tree",
@@ -259,16 +260,16 @@ func TestTree(t *testing.T) {
 			content: treeBytes,
 		},
 		{
-			desc: "not existing ref",
-			spec: "stud",
+			desc:     "not existing ref",
+			revision: "stud",
 			requireErr: func(t *testing.T, err error) {
 				require.True(t, IsNotFound(err), "the error must be from 'not found' family")
 				require.EqualError(t, err, "object not found")
 			},
 		},
 		{
-			desc: "wrong object type",
-			spec: "1e292f8fedd741b75372e19097c76d327140c312", // is commit SHA1
+			desc:     "wrong object type",
+			revision: "1e292f8fedd741b75372e19097c76d327140c312", // is commit SHA1
 			requireErr: func(t *testing.T, err error) {
 				require.True(t, IsNotFound(err), "the error must be from 'not found' family")
 				require.EqualError(t, err, "expected 1e292f8fedd741b75372e19097c76d327140c312 to be a tree, got commit")
@@ -278,7 +279,7 @@ func TestTree(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.desc, func(t *testing.T) {
-			treeObj, err := c.Tree(ctx, tc.spec)
+			treeObj, err := c.Tree(ctx, git.Revision(tc.revision))
 
 			if tc.requireErr != nil {
 				tc.requireErr(t, err)
@@ -305,7 +306,7 @@ func TestRepeatedCalls(t *testing.T) {
 	c, err := New(ctx, config.NewLocator(config.Config), testRepository)
 	require.NoError(t, err)
 
-	treeOid := "7e2f26d033ee47cd0745649d1a28277c56197921"
+	treeOid := git.Revision("7e2f26d033ee47cd0745649d1a28277c56197921")
 	treeBytes, err := ioutil.ReadFile("testdata/tree-7e2f26d033ee47cd0745649d1a28277c56197921")
 	require.NoError(t, err)
 
diff --git a/internal/git/log/commit.go b/internal/git/log/commit.go
index 2e3032e80..eed8a2031 100644
--- a/internal/git/log/commit.go
+++ b/internal/git/log/commit.go
@@ -43,7 +43,7 @@ func GetCommitWithTrailers(ctx context.Context, locator storage.Locator, repo *g
 
 // GetCommitCatfile looks up a commit by revision using an existing catfile.Batch instance.
 func GetCommitCatfile(ctx context.Context, c catfile.Batch, revision string) (*gitalypb.GitCommit, error) {
-	obj, err := c.Commit(ctx, revision+"^{commit}")
+	obj, err := c.Commit(ctx, git.Revision(revision+"^{commit}"))
 	if err != nil {
 		return nil, err
 	}
@@ -92,7 +92,7 @@ func GetCommitCatfileWithTrailers(ctx context.Context, repo *gitalypb.Repository
 
 // GetCommitMessage looks up a commit message and returns it in its entirety.
 func GetCommitMessage(ctx context.Context, c catfile.Batch, repo *gitalypb.Repository, revision string) ([]byte, error) {
-	obj, err := c.Commit(ctx, revision+"^{commit}")
+	obj, err := c.Commit(ctx, git.Revision(revision+"^{commit}"))
 	if err != nil {
 		return nil, err
 	}
diff --git a/internal/git/log/tag.go b/internal/git/log/tag.go
index 33b95662a..b8f208166 100644
--- a/internal/git/log/tag.go
+++ b/internal/git/log/tag.go
@@ -9,6 +9,7 @@ import (
 	"io/ioutil"
 	"strings"
 
+	"gitlab.com/gitlab-org/gitaly/internal/git"
 	"gitlab.com/gitlab-org/gitaly/internal/git/catfile"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
@@ -25,7 +26,7 @@ const (
 // note: we pass in the tagName because the tag name from refs/tags may be different
 // than the name found in the actual tag object. We want to use the tagName found in refs/tags
 func GetTagCatfile(ctx context.Context, c catfile.Batch, tagID, tagName string, trimLen, trimRightNewLine bool) (*gitalypb.Tag, error) {
-	tagObj, err := c.Tag(ctx, tagID)
+	tagObj, err := c.Tag(ctx, git.Revision(tagID))
 	if err != nil {
 		return nil, err
 	}
@@ -142,17 +143,16 @@ func buildAnnotatedTag(ctx context.Context, b catfile.Batch, tagID, name string,
 // This matches the original behavior in the ruby implementation.
 // we also protect against circular tag references. Even though this is not possible in git,
 // we still want to protect against an infinite looop
-
-func dereferenceTag(ctx context.Context, b catfile.Batch, Oid string) (*gitalypb.GitCommit, error) {
+func dereferenceTag(ctx context.Context, b catfile.Batch, oid string) (*gitalypb.GitCommit, error) {
 	for depth := 0; depth < MaxTagReferenceDepth; depth++ {
-		i, err := b.Info(ctx, Oid)
+		i, err := b.Info(ctx, git.Revision(oid))
 		if err != nil {
 			return nil, err
 		}
 
 		switch i.Type {
 		case "tag":
-			tagObj, err := b.Tag(ctx, Oid)
+			tagObj, err := b.Tag(ctx, git.Revision(oid))
 			if err != nil {
 				return nil, err
 			}
@@ -162,10 +162,10 @@ func dereferenceTag(ctx context.Context, b catfile.Batch, Oid string) (*gitalypb
 				return nil, err
 			}
 
-			Oid = header.oid
+			oid = header.oid
 			continue
 		case "commit":
-			return GetCommitCatfile(ctx, b, Oid)
+			return GetCommitCatfile(ctx, b, oid)
 		default: // This current tag points to a tree or a blob
 			return nil, nil
 		}
diff --git a/internal/gitaly/service/blob/get_blob.go b/internal/gitaly/service/blob/get_blob.go
index c3f36d4d2..81566b81f 100644
--- a/internal/gitaly/service/blob/get_blob.go
+++ b/internal/gitaly/service/blob/get_blob.go
@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"io"
 
+	"gitlab.com/gitlab-org/gitaly/internal/git"
 	"gitlab.com/gitlab-org/gitaly/internal/git/catfile"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
@@ -24,7 +25,7 @@ func (s *server) GetBlob(in *gitalypb.GetBlobRequest, stream gitalypb.BlobServic
 		return status.Errorf(codes.Internal, "GetBlob: %v", err)
 	}
 
-	objectInfo, err := c.Info(ctx, in.Oid)
+	objectInfo, err := c.Info(ctx, git.Revision(in.Oid))
 	if err != nil && !catfile.IsNotFound(err) {
 		return status.Errorf(codes.Internal, "GetBlob: %v", err)
 	}
@@ -45,7 +46,7 @@ func (s *server) GetBlob(in *gitalypb.GetBlobRequest, stream gitalypb.BlobServic
 		return helper.DecorateError(codes.Unavailable, stream.Send(firstMessage))
 	}
 
-	blobObj, err := c.Blob(ctx, objectInfo.Oid)
+	blobObj, err := c.Blob(ctx, git.Revision(objectInfo.Oid))
 	if err != nil {
 		return status.Errorf(codes.Internal, "GetBlob: %v", err)
 	}
diff --git a/internal/gitaly/service/blob/get_blobs.go b/internal/gitaly/service/blob/get_blobs.go
index f8eb5ad86..918c86bb3 100644
--- a/internal/gitaly/service/blob/get_blobs.go
+++ b/internal/gitaly/service/blob/get_blobs.go
@@ -61,7 +61,7 @@ func sendGetBlobsResponse(req *gitalypb.GetBlobsRequest, stream gitalypb.BlobSer
 			continue
 		}
 
-		objectInfo, err := c.Info(ctx, treeEntry.Oid)
+		objectInfo, err := c.Info(ctx, git.Revision(treeEntry.Oid))
 		if err != nil {
 			return status.Errorf(codes.Internal, "GetBlobs: %v", err)
 		}
@@ -110,7 +110,7 @@ func sendBlobTreeEntry(response *gitalypb.GetBlobsResponse, stream gitalypb.Blob
 		return nil
 	}
 
-	blobObj, err := c.Blob(ctx, response.Oid)
+	blobObj, err := c.Blob(ctx, git.Revision(response.Oid))
 	if err != nil {
 		return status.Errorf(codes.Internal, "GetBlobs: %v", err)
 	}
diff --git a/internal/gitaly/service/cleanup/notifier/notifier.go b/internal/gitaly/service/cleanup/notifier/notifier.go
index 7817c4d60..7e93f3479 100644
--- a/internal/gitaly/service/cleanup/notifier/notifier.go
+++ b/internal/gitaly/service/cleanup/notifier/notifier.go
@@ -3,6 +3,7 @@ package notifier
 import (
 	"context"
 
+	"gitlab.com/gitlab-org/gitaly/internal/git"
 	"gitlab.com/gitlab-org/gitaly/internal/git/catfile"
 	"gitlab.com/gitlab-org/gitaly/internal/helper/chunk"
 	"gitlab.com/gitlab-org/gitaly/internal/storage"
@@ -44,7 +45,7 @@ func (n *Notifier) lookupType(ctx context.Context, oid string, isInternalRef boo
 		return gitalypb.ObjectType_COMMIT
 	}
 
-	info, err := n.catfile.Info(ctx, oid)
+	info, err := n.catfile.Info(ctx, git.Revision(oid))
 	if err != nil {
 		return gitalypb.ObjectType_UNKNOWN
 	}
diff --git a/internal/gitaly/service/commit/commit_signatures.go b/internal/gitaly/service/commit/commit_signatures.go
index f78f03f53..12b9fcaee 100644
--- a/internal/gitaly/service/commit/commit_signatures.go
+++ b/internal/gitaly/service/commit/commit_signatures.go
@@ -35,7 +35,7 @@ func (s *server) getCommitSignatures(request *gitalypb.GetCommitSignaturesReques
 	}
 
 	for _, commitID := range request.CommitIds {
-		commitObj, err := c.Commit(ctx, commitID)
+		commitObj, err := c.Commit(ctx, git.Revision(commitID))
 		if err != nil {
 			if catfile.IsNotFound(err) {
 				continue
diff --git a/internal/gitaly/service/commit/tree_entries_helper.go b/internal/gitaly/service/commit/tree_entries_helper.go
index 108f3df56..4f48814a0 100644
--- a/internal/gitaly/service/commit/tree_entries_helper.go
+++ b/internal/gitaly/service/commit/tree_entries_helper.go
@@ -11,6 +11,7 @@ import (
 	"path/filepath"
 	"strings"
 
+	"gitlab.com/gitlab-org/gitaly/internal/git"
 	"gitlab.com/gitlab-org/gitaly/internal/git/catfile"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 )
@@ -115,7 +116,7 @@ func treeEntries(ctx context.Context, c catfile.Batch, revision, path string, ro
 	}
 
 	if len(rootOid) == 0 {
-		rootTreeInfo, err := c.Info(ctx, revision+"^{tree}")
+		rootTreeInfo, err := c.Info(ctx, git.Revision(revision+"^{tree}"))
 		if err != nil {
 			if catfile.IsNotFound(err) {
 				return nil, nil
@@ -127,7 +128,7 @@ func treeEntries(ctx context.Context, c catfile.Batch, revision, path string, ro
 		rootOid = rootTreeInfo.Oid
 	}
 
-	treeObj, err := c.Tree(ctx, fmt.Sprintf("%s:%s", revision, path))
+	treeObj, err := c.Tree(ctx, git.Revision(fmt.Sprintf("%s:%s", revision, path)))
 	if err != nil {
 		if catfile.IsNotFound(err) {
 			return nil, nil
diff --git a/internal/gitaly/service/commit/tree_entry.go b/internal/gitaly/service/commit/tree_entry.go
index 00663bfa3..3a2d736b0 100644
--- a/internal/gitaly/service/commit/tree_entry.go
+++ b/internal/gitaly/service/commit/tree_entry.go
@@ -40,7 +40,7 @@ func sendTreeEntry(stream gitalypb.CommitService_TreeEntryServer, c catfile.Batc
 	}
 
 	if treeEntry.Type == gitalypb.TreeEntry_TREE {
-		treeInfo, err := c.Info(ctx, treeEntry.Oid)
+		treeInfo, err := c.Info(ctx, git.Revision(treeEntry.Oid))
 		if err != nil {
 			return err
 		}
@@ -54,7 +54,7 @@ func sendTreeEntry(stream gitalypb.CommitService_TreeEntryServer, c catfile.Batc
 		return helper.DecorateError(codes.Unavailable, stream.Send(response))
 	}
 
-	objectInfo, err := c.Info(ctx, treeEntry.Oid)
+	objectInfo, err := c.Info(ctx, git.Revision(treeEntry.Oid))
 	if err != nil {
 		return status.Errorf(codes.Internal, "TreeEntry: %v", err)
 	}
@@ -91,7 +91,7 @@ func sendTreeEntry(stream gitalypb.CommitService_TreeEntryServer, c catfile.Batc
 		return helper.DecorateError(codes.Unavailable, stream.Send(response))
 	}
 
-	blobObj, err := c.Blob(ctx, objectInfo.Oid)
+	blobObj, err := c.Blob(ctx, git.Revision(objectInfo.Oid))
 	if err != nil {
 		return err
 	}
diff --git a/internal/gitaly/service/operations/tags.go b/internal/gitaly/service/operations/tags.go
index 18751cd97..a9ec0bac0 100644
--- a/internal/gitaly/service/operations/tags.go
+++ b/internal/gitaly/service/operations/tags.go
@@ -140,7 +140,7 @@ func (s *Server) userCreateTagGo(ctx context.Context, req *gitalypb.UserCreateTa
 
 	// We allow all ways to name a revision that cat-file
 	// supports, not just OID. Resolve it.
-	targetRevision := string(req.TargetRevision)
+	targetRevision := git.Revision(req.TargetRevision)
 	targetInfo, err := catFile.Info(ctx, targetRevision)
 	if err != nil {
 		return nil, status.Errorf(codes.FailedPrecondition, "revspec '%s' not found", targetRevision)
diff --git a/internal/gitaly/service/ref/list_new_blobs.go b/internal/gitaly/service/ref/list_new_blobs.go
index 0792e36fd..1fba363bc 100644
--- a/internal/gitaly/service/ref/list_new_blobs.go
+++ b/internal/gitaly/service/ref/list_new_blobs.go
@@ -53,7 +53,7 @@ func (s *server) listNewBlobs(in *gitalypb.ListNewBlobsRequest, stream gitalypb.
 			continue
 		}
 
-		info, err := batch.Info(ctx, parts[0])
+		info, err := batch.Info(ctx, git.Revision(parts[0]))
 		if err != nil {
 			return err
 		}
diff --git a/internal/gitaly/service/ref/refs_test.go b/internal/gitaly/service/ref/refs_test.go
index 874902408..b83947d0d 100644
--- a/internal/gitaly/service/ref/refs_test.go
+++ b/internal/gitaly/service/ref/refs_test.go
@@ -722,7 +722,7 @@ func TestFindAllTagNestedTags(t *testing.T) {
 			batch, err := catfile.New(ctx, locator, testRepoCopy)
 			require.NoError(t, err)
 
-			info, err := batch.Info(ctx, tc.originalOid)
+			info, err := batch.Info(ctx, git.Revision(tc.originalOid))
 			require.NoError(t, err)
 
 			expectedTags := make(map[string]*gitalypb.Tag)
@@ -1656,7 +1656,7 @@ func TestFindTagNestedTag(t *testing.T) {
 			batch, err := catfile.New(ctx, locator, testRepoCopy)
 			require.NoError(t, err)
 
-			info, err := batch.Info(ctx, tc.originalOid)
+			info, err := batch.Info(ctx, git.Revision(tc.originalOid))
 			require.NoError(t, err)
 
 			tagID := tc.originalOid
diff --git a/internal/gitaly/service/repository/apply_gitattributes.go b/internal/gitaly/service/repository/apply_gitattributes.go
index eef1e868a..d8926e312 100644
--- a/internal/gitaly/service/repository/apply_gitattributes.go
+++ b/internal/gitaly/service/repository/apply_gitattributes.go
@@ -21,7 +21,7 @@ func applyGitattributes(ctx context.Context, c catfile.Batch, repoPath string, r
 	infoPath := filepath.Join(repoPath, "info")
 	attributesPath := filepath.Join(infoPath, "attributes")
 
-	_, err := c.Info(ctx, string(revision))
+	_, err := c.Info(ctx, git.Revision(revision))
 	if err != nil {
 		if catfile.IsNotFound(err) {
 			return status.Errorf(codes.InvalidArgument, "Revision doesn't exist")
@@ -30,7 +30,7 @@ func applyGitattributes(ctx context.Context, c catfile.Batch, repoPath string, r
 		return err
 	}
 
-	blobInfo, err := c.Info(ctx, fmt.Sprintf("%s:.gitattributes", revision))
+	blobInfo, err := c.Info(ctx, git.Revision(fmt.Sprintf("%s:.gitattributes", revision)))
 	if err != nil && !catfile.IsNotFound(err) {
 		return err
 	}
@@ -55,7 +55,7 @@ func applyGitattributes(ctx context.Context, c catfile.Batch, repoPath string, r
 	}
 	defer os.Remove(tempFile.Name())
 
-	blobObj, err := c.Blob(ctx, blobInfo.Oid)
+	blobObj, err := c.Blob(ctx, git.Revision(blobInfo.Oid))
 	if err != nil {
 		return err
 	}
diff --git a/internal/gitaly/service/repository/gc.go b/internal/gitaly/service/repository/gc.go
index 2890381c3..c722f6661 100644
--- a/internal/gitaly/service/repository/gc.go
+++ b/internal/gitaly/service/repository/gc.go
@@ -161,7 +161,7 @@ func checkRef(ctx context.Context, batch catfile.Batch, refName string, info os.
 		return errors.New("checkRef: Ref file is empty")
 	}
 
-	_, err := batch.Info(ctx, refName)
+	_, err := batch.Info(ctx, git.Revision(refName))
 	return err
 }
 
@@ -172,7 +172,7 @@ func (s *server) fixRef(ctx context.Context, repo *gitalypb.Repository, batch ca
 	}
 
 	// If the sha is not in the the repository, we can't fix it
-	if _, err := batch.Info(ctx, sha); err != nil {
+	if _, err := batch.Info(ctx, git.Revision(sha)); err != nil {
 		return nil
 	}
 
diff --git a/internal/gitaly/service/repository/raw_changes.go b/internal/gitaly/service/repository/raw_changes.go
index df80e5323..928bc61e7 100644
--- a/internal/gitaly/service/repository/raw_changes.go
+++ b/internal/gitaly/service/repository/raw_changes.go
@@ -39,13 +39,13 @@ func (s *server) GetRawChanges(req *gitalypb.GetRawChangesRequest, stream gitaly
 
 func validateRawChangesRequest(ctx context.Context, req *gitalypb.GetRawChangesRequest, batch catfile.Batch) error {
 	if from := req.FromRevision; from != git.NullSHA {
-		if _, err := batch.Info(ctx, from); err != nil {
+		if _, err := batch.Info(ctx, git.Revision(from)); err != nil {
 			return fmt.Errorf("invalid 'from' revision: %q", from)
 		}
 	}
 
 	if to := req.ToRevision; to != git.NullSHA {
-		if _, err := batch.Info(ctx, to); err != nil {
+		if _, err := batch.Info(ctx, git.Revision(to)); err != nil {
 			return fmt.Errorf("invalid 'to' revision: %q", to)
 		}
 	}
@@ -153,7 +153,7 @@ func changeFromDiff(ctx context.Context, batch catfile.Batch, d *rawdiff.Diff) (
 	}
 
 	if blobMode != submoduleTreeEntryMode {
-		info, err := batch.Info(ctx, shortBlobID)
+		info, err := batch.Info(ctx, git.Revision(shortBlobID))
 		if err != nil {
 			return nil, fmt.Errorf("find %q: %v", shortBlobID, err)
 		}
diff --git a/internal/gitaly/service/repository/snapshot_test.go b/internal/gitaly/service/repository/snapshot_test.go
index 592ebe9d9..f2ffccec2 100644
--- a/internal/gitaly/service/repository/snapshot_test.go
+++ b/internal/gitaly/service/repository/snapshot_test.go
@@ -14,6 +14,7 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/internal/git"
 	"gitlab.com/gitlab-org/gitaly/internal/git/catfile"
 	"gitlab.com/gitlab-org/gitaly/internal/gitaly/archive"
 	"gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
@@ -139,7 +140,7 @@ func TestGetSnapshotWithDedupe(t *testing.T) {
 			// ensure commit cannot be found in current repository
 			c, err := catfile.New(ctx, locator, testRepo)
 			require.NoError(t, err)
-			_, err = c.Info(ctx, originalAlternatesCommit)
+			_, err = c.Info(ctx, git.Revision(originalAlternatesCommit))
 			require.True(t, catfile.IsNotFound(err))
 
 			// write alternates file to point to alt objects folder
@@ -156,7 +157,7 @@ func TestGetSnapshotWithDedupe(t *testing.T) {
 
 			c, err = catfile.New(ctx, locator, testRepo)
 			require.NoError(t, err)
-			_, err = c.Info(ctx, string(commitSha))
+			_, err = c.Info(ctx, git.Revision(commitSha))
 			require.NoError(t, err)
 
 			_, repoCopyPath, cleanupCopy := copyRepoUsingSnapshot(t, locator, testRepo)