git: Always assume we can handle object type filters

Now that the minimum required Git version is v2.33.0 and newer, we can
assume that git-rev-list(1) can always handle object type filters.

Drop `SupportsObjectTypeFilter()` and adjust callsites accordingly.
Given that we now always set up those filters unconditionally, this
commit also gets rid of any additional pipeline steps which verify that
the current object's type is indeed the expected type.

Note that in some cases, we can get rid of the `CatfileInfo()` step
completely because we already know the type of the object, which we
could thus directly pass to the `CatfileObject()` pipeline step. This
refactoring is deferred to a later point in time though such that
pending refactorings for the catfile cache can land first.

6 files changed, 8 insertions, 90 deletions

diff --git a/internal/git/gitpipe/revision_test.go b/internal/git/gitpipe/revision_test.go
index f483905e7..3db71ec60 100644
--- a/internal/git/gitpipe/revision_test.go
+++ b/internal/git/gitpipe/revision_test.go
@@ -19,21 +19,8 @@ func TestRevlist(t *testing.T) {
 	repoProto, _ := gittest.CloneRepo(t, cfg, cfg.Storages[0])
 	repo := localrepo.NewTestRepo(t, cfg, repoProto)
 
-	needsObjectTypeFilters := func(t *testing.T) {
-		ctx, cancel := testhelper.Context()
-		defer cancel()
-
-		gitVersion, err := git.CurrentVersion(ctx, git.NewExecCommandFactory(cfg))
-		require.NoError(t, err)
-
-		if !gitVersion.SupportsObjectTypeFilter() {
-			t.Skip("Git does not support object type filters")
-		}
-	}
-
 	for _, tc := range []struct {
 		desc            string
-		precondition    func(t *testing.T)
 		revisions       []string
 		options         []RevlistOption
 		expectedResults []RevisionResult
@@ -346,8 +333,7 @@ func TestRevlist(t *testing.T) {
 			},
 		},
 		{
-			desc:         "tree with blob object type filter",
-			precondition: needsObjectTypeFilters,
+			desc: "tree with blob object type filter",
 			revisions: []string{
 				"79d5f98270ad677c86a7e1ab2baa922958565135",
 			},
@@ -368,8 +354,7 @@ func TestRevlist(t *testing.T) {
 			},
 		},
 		{
-			desc:         "tree with tag object type filter",
-			precondition: needsObjectTypeFilters,
+			desc: "tree with tag object type filter",
 			revisions: []string{
 				"--all",
 			},
@@ -384,8 +369,7 @@ func TestRevlist(t *testing.T) {
 			},
 		},
 		{
-			desc:         "tree with commit object type filter",
-			precondition: needsObjectTypeFilters,
+			desc: "tree with commit object type filter",
 			revisions: []string{
 				"79d5f98270ad677c86a7e1ab2baa922958565135",
 			},
@@ -398,8 +382,7 @@ func TestRevlist(t *testing.T) {
 			},
 		},
 		{
-			desc:         "tree with commit object type filter",
-			precondition: needsObjectTypeFilters,
+			desc: "tree with commit object type filter",
 			revisions: []string{
 				"^refs/heads/master~",
 				"refs/heads/master",
@@ -414,8 +397,7 @@ func TestRevlist(t *testing.T) {
 			},
 		},
 		{
-			desc:         "tree with object type and blob size filter",
-			precondition: needsObjectTypeFilters,
+			desc: "tree with object type and blob size filter",
 			revisions: []string{
 				"79d5f98270ad677c86a7e1ab2baa922958565135",
 			},
@@ -447,10 +429,6 @@ func TestRevlist(t *testing.T) {
 		},
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
-			if tc.precondition != nil {
-				tc.precondition(t)
-			}
-
 			ctx, cancel := testhelper.Context()
 			defer cancel()
 
diff --git a/internal/git/version.go b/internal/git/version.go
index 7447aaeff..4a20a5054 100644
--- a/internal/git/version.go
+++ b/internal/git/version.go
@@ -98,12 +98,6 @@ func (v Version) IsSupported() bool {
 	return !v.LessThan(minimumVersion)
 }
 
-// SupportsObjectTypeFilter checks if a version corresponds to a Git version which supports object
-// type filters.
-func (v Version) SupportsObjectTypeFilter() bool {
-	return !v.LessThan(Version{major: 2, minor: 32, patch: 0})
-}
-
 // FlushesUpdaterefStatus determines whether the given Git version properly flushes status messages
 // in git-update-ref(1).
 func (v Version) FlushesUpdaterefStatus() bool {
diff --git a/internal/git/version_test.go b/internal/git/version_test.go
index 0cd3d620d..23b8d048b 100644
--- a/internal/git/version_test.go
+++ b/internal/git/version_test.go
@@ -242,25 +242,6 @@ func TestVersion_IsSupported(t *testing.T) {
 	}
 }
 
-func TestVersion_SupportsObjectTypeFilter(t *testing.T) {
-	for _, tc := range []struct {
-		version string
-		expect  bool
-	}{
-		{"2.32.0.gl3", false},
-		{"2.33.0.gl3", true},
-		{"2.33.1.gl3", true},
-		{"2.34.0", true},
-		{"3.0.0", true},
-	} {
-		t.Run(tc.version, func(t *testing.T) {
-			version, err := parseVersion(tc.version)
-			require.NoError(t, err)
-			require.Equal(t, tc.expect, version.FlushesUpdaterefStatus())
-		})
-	}
-}
-
 func TestVersion_FlushesUpdaterefStatus(t *testing.T) {
 	for _, tc := range []struct {
 		version string
diff --git a/internal/gitaly/service/blob/blobs.go b/internal/gitaly/service/blob/blobs.go
index 651bbdf29..13bebc344 100644
--- a/internal/gitaly/service/blob/blobs.go
+++ b/internal/gitaly/service/blob/blobs.go
@@ -7,7 +7,6 @@ import (
 	"io"
 	"strings"
 
-	"gitlab.com/gitlab-org/gitaly/v14/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git/catfile"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git/gitpipe"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/helper"
@@ -55,24 +54,13 @@ func (s *server) ListBlobs(req *gitalypb.ListBlobsRequest, stream gitalypb.BlobS
 		},
 	})
 
-	gitVersion, err := git.CurrentVersion(ctx, s.gitCmdFactory)
-	if err != nil {
-		return helper.ErrInternalf("cannot determine Git version: %v", err)
-	}
-
 	revlistOptions := []gitpipe.RevlistOption{
 		gitpipe.WithObjects(),
-	}
-
-	if gitVersion.SupportsObjectTypeFilter() {
-		revlistOptions = append(revlistOptions, gitpipe.WithObjectTypeFilter(gitpipe.ObjectTypeBlob))
+		gitpipe.WithObjectTypeFilter(gitpipe.ObjectTypeBlob),
 	}
 
 	revlistIter := gitpipe.Revlist(ctx, repo, req.GetRevisions(), revlistOptions...)
 	catfileInfoIter := gitpipe.CatfileInfo(ctx, catfileProcess, revlistIter)
-	catfileInfoIter = gitpipe.CatfileInfoFilter(ctx, catfileInfoIter, func(r gitpipe.CatfileInfoResult) bool {
-		return r.ObjectInfo.Type == "blob"
-	})
 
 	if err := processBlobs(ctx, catfileProcess, catfileInfoIter, req.GetLimit(), req.GetBytesLimit(),
 		func(oid string, size int64, contents []byte, path []byte) error {
diff --git a/internal/gitaly/service/blob/lfs_pointers.go b/internal/gitaly/service/blob/lfs_pointers.go
index d878b5ab2..4c2fca119 100644
--- a/internal/gitaly/service/blob/lfs_pointers.go
+++ b/internal/gitaly/service/blob/lfs_pointers.go
@@ -56,24 +56,14 @@ func (s *server) ListLFSPointers(in *gitalypb.ListLFSPointersRequest, stream git
 		return helper.ErrInternal(fmt.Errorf("creating catfile process: %w", err))
 	}
 
-	gitVersion, err := git.CurrentVersion(ctx, s.gitCmdFactory)
-	if err != nil {
-		return helper.ErrInternalf("cannot determine Git version: %v", err)
-	}
-
 	revlistOptions := []gitpipe.RevlistOption{
 		gitpipe.WithObjects(),
 		gitpipe.WithBlobLimit(lfsPointerMaxSize),
-	}
-	if gitVersion.SupportsObjectTypeFilter() {
-		revlistOptions = append(revlistOptions, gitpipe.WithObjectTypeFilter(gitpipe.ObjectTypeBlob))
+		gitpipe.WithObjectTypeFilter(gitpipe.ObjectTypeBlob),
 	}
 
 	revlistIter := gitpipe.Revlist(ctx, repo, in.GetRevisions(), revlistOptions...)
 	catfileInfoIter := gitpipe.CatfileInfo(ctx, catfileProcess, revlistIter)
-	catfileInfoIter = gitpipe.CatfileInfoFilter(ctx, catfileInfoIter, func(r gitpipe.CatfileInfoResult) bool {
-		return r.ObjectInfo.Type == "blob" && r.ObjectInfo.Size <= lfsPointerMaxSize
-	})
 	catfileObjectIter := gitpipe.CatfileObject(ctx, catfileProcess, catfileInfoIter)
 
 	if err := sendLFSPointers(chunker, catfileObjectIter, int(in.Limit)); err != nil {
diff --git a/internal/gitaly/service/ref/tag_signatures.go b/internal/gitaly/service/ref/tag_signatures.go
index 57f656f4f..6b6659e10 100644
--- a/internal/gitaly/service/ref/tag_signatures.go
+++ b/internal/gitaly/service/ref/tag_signatures.go
@@ -6,7 +6,6 @@ import (
 	"io"
 	"strings"
 
-	"gitlab.com/gitlab-org/gitaly/v14/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git/catfile"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git/gitpipe"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/helper"
@@ -53,25 +52,13 @@ func (s *server) GetTagSignatures(req *gitalypb.GetTagSignaturesRequest, stream
 		},
 	})
 
-	gitVersion, err := git.CurrentVersion(ctx, s.gitCmdFactory)
-	if err != nil {
-		return helper.ErrInternalf("cannot determine Git version: %v", err)
-	}
-
 	revlistOptions := []gitpipe.RevlistOption{
 		gitpipe.WithObjects(),
-	}
-
-	if gitVersion.SupportsObjectTypeFilter() {
-		revlistOptions = append(revlistOptions, gitpipe.WithObjectTypeFilter(gitpipe.ObjectTypeTag))
+		gitpipe.WithObjectTypeFilter(gitpipe.ObjectTypeTag),
 	}
 
 	revlistIter := gitpipe.Revlist(ctx, repo, req.GetTagRevisions(), revlistOptions...)
 	catfileInfoIter := gitpipe.CatfileInfo(ctx, catfileProcess, revlistIter)
-	catfileInfoIter = gitpipe.CatfileInfoFilter(ctx, catfileInfoIter, func(r gitpipe.CatfileInfoResult) bool {
-		return r.ObjectInfo.Type == "tag"
-	})
-
 	catfileObjectIter := gitpipe.CatfileObject(ctx, catfileProcess, catfileInfoIter)
 
 	for catfileObjectIter.Next() {