version: Get rid of `CatfileSupportsNulTerminatedOutput()`

Since we've updated the minimum version of Git to v2.42.0, we no longer
need to check if git-cat-file(1) supports the '-Z' option. So let's get
rid of the helper function and code around it.

11 files changed, 17 insertions, 164 deletions

diff --git a/internal/git/catfile/cache.go b/internal/git/catfile/cache.go
index 494493785..346dc0fbf 100644
--- a/internal/git/catfile/cache.go
+++ b/internal/git/catfile/cache.go
@@ -180,20 +180,9 @@ func (c *ProcessCache) ObjectReader(ctx context.Context, repo git.RepositoryExec
 	var err error
 	var cancel func()
 
-	version, err := repo.GitVersion(ctx)
-	if err != nil {
-		return nil, nil, fmt.Errorf("git version: %w", err)
-	}
-
-	if version.CatfileSupportsNulTerminatedOutput() {
-		cached, cancel, err = c.getOrCreateProcess(ctx, repo, &c.objectReaders, func(ctx context.Context) (cacheable, error) {
-			return newObjectReader(ctx, repo, c.catfileLookupCounter)
-		}, "catfile.ObjectReader")
-	} else {
-		cached, cancel, err = c.getOrCreateProcess(ctx, repo, &c.objectContentReaders, func(ctx context.Context) (cacheable, error) {
-			return newObjectContentReader(ctx, repo, c.catfileLookupCounter)
-		}, "catfile.ObjectContentReader")
-	}
+	cached, cancel, err = c.getOrCreateProcess(ctx, repo, &c.objectReaders, func(ctx context.Context) (cacheable, error) {
+		return newObjectReader(ctx, repo, c.catfileLookupCounter)
+	}, "catfile.ObjectReader")
 	if err != nil {
 		return nil, nil, err
 	}
@@ -212,20 +201,9 @@ func (c *ProcessCache) ObjectInfoReader(ctx context.Context, repo git.Repository
 	var err error
 	var cancel func()
 
-	version, err := repo.GitVersion(ctx)
-	if err != nil {
-		return nil, nil, fmt.Errorf("git version: %w", err)
-	}
-
-	if version.CatfileSupportsNulTerminatedOutput() {
-		cached, cancel, err = c.getOrCreateProcess(ctx, repo, &c.objectReaders, func(ctx context.Context) (cacheable, error) {
-			return newObjectReader(ctx, repo, c.catfileLookupCounter)
-		}, "catfile.ObjectReader")
-	} else {
-		cached, cancel, err = c.getOrCreateProcess(ctx, repo, &c.objectInfoReaders, func(ctx context.Context) (cacheable, error) {
-			return newObjectInfoReader(ctx, repo, c.catfileLookupCounter)
-		}, "catfile.ObjectInfoReader")
-	}
+	cached, cancel, err = c.getOrCreateProcess(ctx, repo, &c.objectReaders, func(ctx context.Context) (cacheable, error) {
+		return newObjectReader(ctx, repo, c.catfileLookupCounter)
+	}, "catfile.ObjectReader")
 	if err != nil {
 		return nil, nil, err
 	}
diff --git a/internal/git/catfile/cache_test.go b/internal/git/catfile/cache_test.go
index d4e371797..b871264b7 100644
--- a/internal/git/catfile/cache_test.go
+++ b/internal/git/catfile/cache_test.go
@@ -215,9 +215,6 @@ func TestCache_ObjectReader(t *testing.T) {
 
 	repoExecutor := newRepoExecutor(t, cfg, repo)
 
-	version, err := repoExecutor.GitVersion(ctx)
-	require.NoError(t, err)
-
 	cache := newCache(time.Hour, 10, helper.NewManualTicker())
 	defer cache.Stop()
 
@@ -249,13 +246,7 @@ func TestCache_ObjectReader(t *testing.T) {
 		// cache and wait for the cache to collect it.
 		cancel()
 
-		var allKeys []key
-		if version.CatfileSupportsNulTerminatedOutput() {
-			allKeys = keys(t, &cache.objectReaders)
-		} else {
-			allKeys = keys(t, &cache.objectContentReaders)
-		}
-
+		allKeys := keys(t, &cache.objectReaders)
 		require.Equal(t, []key{{
 			sessionID:   "1",
 			repoStorage: repo.GetStorageName(),
@@ -326,9 +317,6 @@ func TestCache_ObjectInfoReader(t *testing.T) {
 
 	repoExecutor := newRepoExecutor(t, cfg, repo)
 
-	version, err := repoExecutor.GitVersion(ctx)
-	require.NoError(t, err)
-
 	cache := newCache(time.Hour, 10, helper.NewManualTicker())
 	defer cache.Stop()
 
@@ -359,13 +347,7 @@ func TestCache_ObjectInfoReader(t *testing.T) {
 		// Cancel the process such it will be considered for return to the cache.
 		cancel()
 
-		var allKeys []key
-		if version.CatfileSupportsNulTerminatedOutput() {
-			allKeys = keys(t, &cache.objectReaders)
-		} else {
-			allKeys = keys(t, &cache.objectInfoReaders)
-		}
-
+		allKeys := keys(t, &cache.objectReaders)
 		require.Equal(t, []key{{
 			sessionID:   "1",
 			repoStorage: repo.GetStorageName(),
diff --git a/internal/git/catfile/object_content_reader.go b/internal/git/catfile/object_content_reader.go
index e0796fa84..f2e3a3ee2 100644
--- a/internal/git/catfile/object_content_reader.go
+++ b/internal/git/catfile/object_content_reader.go
@@ -45,19 +45,11 @@ func newObjectContentReader(
 	repo git.RepositoryExecutor,
 	counter *prometheus.CounterVec,
 ) (*objectContentReader, error) {
-	gitVersion, err := repo.GitVersion(ctx)
-	if err != nil {
-		return nil, fmt.Errorf("detecting Git version: %w", err)
-	}
-
 	flags := []git.Option{
 		git.Flag{Name: "--batch"},
 		git.Flag{Name: "--buffer"},
 		git.Flag{Name: "-z"},
-	}
-
-	if gitVersion.CatfileSupportsNulTerminatedOutput() {
-		flags = append(flags, git.Flag{Name: "-Z"})
+		git.Flag{Name: "-Z"},
 	}
 
 	if featureflag.MailmapOptions.IsEnabled(ctx) {
@@ -87,7 +79,7 @@ func newObjectContentReader(
 		queue: requestQueue{
 			objectHash:      objectHash,
 			isObjectQueue:   true,
-			isNulTerminated: gitVersion.CatfileSupportsNulTerminatedOutput(),
+			isNulTerminated: true,
 			stdout:          bufio.NewReader(batchCmd),
 			stdin:           bufio.NewWriter(batchCmd),
 		},
diff --git a/internal/git/catfile/object_content_reader_test.go b/internal/git/catfile/object_content_reader_test.go
index 26529b564..7d3472598 100644
--- a/internal/git/catfile/object_content_reader_test.go
+++ b/internal/git/catfile/object_content_reader_test.go
@@ -15,7 +15,6 @@ import (
 	"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/structerr"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/testhelper/testcfg"
 )
@@ -353,10 +352,7 @@ func TestObjectContentReader_queue(t *testing.T) {
 		defer cleanup()
 
 		err = queue.RequestObject(ctx, treeWithNewlines.Revision()+":path\nwith\nnewline")
-		if !catfileSupportsNul(t, ctx, cfg) {
-			require.Equal(t, structerr.NewInvalidArgument("Git too old to support requests with newlines"), err)
-			return
-		}
+
 		require.NoError(t, err)
 		require.NoError(t, queue.Flush(ctx))
 
@@ -403,10 +399,6 @@ func TestObjectContentReader_queue(t *testing.T) {
 		defer cleanup()
 
 		err = queue.RequestObject(ctx, "does\nnot\nexist")
-		if !catfileSupportsNul(t, ctx, cfg) {
-			require.Equal(t, structerr.NewInvalidArgument("Git too old to support requests with newlines"), err)
-			return
-		}
 		require.NoError(t, err)
 		require.NoError(t, queue.Flush(ctx))
 
diff --git a/internal/git/catfile/object_info_reader.go b/internal/git/catfile/object_info_reader.go
index 37f81b0ac..39905e9e0 100644
--- a/internal/git/catfile/object_info_reader.go
+++ b/internal/git/catfile/object_info_reader.go
@@ -147,19 +147,11 @@ func newObjectInfoReader(
 	repo git.RepositoryExecutor,
 	counter *prometheus.CounterVec,
 ) (*objectInfoReader, error) {
-	gitVersion, err := repo.GitVersion(ctx)
-	if err != nil {
-		return nil, fmt.Errorf("detecting Git version: %w", err)
-	}
-
 	flags := []git.Option{
 		git.Flag{Name: "--batch-check"},
 		git.Flag{Name: "--buffer"},
 		git.Flag{Name: "-z"},
-	}
-
-	if gitVersion.CatfileSupportsNulTerminatedOutput() {
-		flags = append(flags, git.Flag{Name: "-Z"})
+		git.Flag{Name: "-Z"},
 	}
 
 	if featureflag.MailmapOptions.IsEnabled(ctx) {
@@ -189,7 +181,7 @@ func newObjectInfoReader(
 		counter:    counter,
 		queue: requestQueue{
 			objectHash:      objectHash,
-			isNulTerminated: gitVersion.CatfileSupportsNulTerminatedOutput(),
+			isNulTerminated: true,
 			stdout:          bufio.NewReader(batchCmd),
 			stdin:           bufio.NewWriter(batchCmd),
 		},
diff --git a/internal/git/catfile/object_info_reader_test.go b/internal/git/catfile/object_info_reader_test.go
index 21c73ea86..373f5c789 100644
--- a/internal/git/catfile/object_info_reader_test.go
+++ b/internal/git/catfile/object_info_reader_test.go
@@ -16,7 +16,6 @@ import (
 	"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/structerr"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/testhelper/testcfg"
 )
@@ -395,10 +394,6 @@ func TestObjectInfoReader_queue(t *testing.T) {
 		defer cleanup()
 
 		err = queue.RequestObject(ctx, treeWithNewlines.Revision()+":path\nwith\nnewline")
-		if !catfileSupportsNul(t, ctx, cfg) {
-			require.Equal(t, structerr.NewInvalidArgument("Git too old to support requests with newlines"), err)
-			return
-		}
 		require.NoError(t, err)
 		require.NoError(t, queue.Flush(ctx))
 
@@ -439,10 +434,6 @@ func TestObjectInfoReader_queue(t *testing.T) {
 		defer cleanup()
 
 		err = queue.RequestInfo(ctx, "does\nnot\nexist")
-		if !catfileSupportsNul(t, ctx, cfg) {
-			require.Equal(t, structerr.NewInvalidArgument("Git too old to support requests with newlines"), err)
-			return
-		}
 		require.NoError(t, err)
 		require.NoError(t, queue.Flush(ctx))
 
diff --git a/internal/git/catfile/object_reader_test.go b/internal/git/catfile/object_reader_test.go
index c18757865..a6198cee0 100644
--- a/internal/git/catfile/object_reader_test.go
+++ b/internal/git/catfile/object_reader_test.go
@@ -29,12 +29,6 @@ func TestObjectReader_reader(t *testing.T) {
 	})
 	cmdExecutor := newRepoExecutor(t, cfg, repoProto)
 
-	version, err := cmdExecutor.GitVersion(ctx)
-	require.NoError(t, err)
-	if !version.CatfileSupportsNulTerminatedOutput() {
-		t.Skip()
-	}
-
 	commitID := gittest.WriteCommit(t, cfg, repoPath,
 		gittest.WithBranch("main"),
 		gittest.WithMessage("commit message"),
@@ -177,12 +171,6 @@ func testObjectReaderObject(t *testing.T, ctx context.Context) {
 	})
 	cmdExecutor := newRepoExecutor(t, cfg, repoProto)
 
-	version, err := cmdExecutor.GitVersion(ctx)
-	require.NoError(t, err)
-	if !version.CatfileSupportsNulTerminatedOutput() {
-		t.Skip()
-	}
-
 	commitID := gittest.WriteCommit(t, cfg, repoPath,
 		gittest.WithBranch("main"),
 		gittest.WithMessage("commit message"),
@@ -291,12 +279,6 @@ func TestObjectReader_queue(t *testing.T) {
 	})
 	cmdExecutor := newRepoExecutor(t, cfg, repoProto)
 
-	version, err := cmdExecutor.GitVersion(ctx)
-	require.NoError(t, err)
-	if !version.CatfileSupportsNulTerminatedOutput() {
-		t.Skip()
-	}
-
 	foobarBlob := gittest.WriteBlob(t, cfg, repoPath, []byte("foobar"))
 	barfooBlob := gittest.WriteBlob(t, cfg, repoPath, []byte("barfoo"))
 
diff --git a/internal/git/catfile/request_queue_test.go b/internal/git/catfile/request_queue_test.go
index f0f917200..9542afa99 100644
--- a/internal/git/catfile/request_queue_test.go
+++ b/internal/git/catfile/request_queue_test.go
@@ -26,7 +26,7 @@ func TestRequestQueue_ReadObject(t *testing.T) {
 	cfg := testcfg.Build(t)
 
 	oid := git.ObjectID(strings.Repeat("1", gittest.DefaultObjectHash.EncodedLen()))
-	lineEnding := detectEncodedLineEnding(t, ctx, cfg)
+	lineEnding := "\\x00"
 
 	t.Run("ReadInfo on ReadObject queue", func(t *testing.T) {
 		_, queue := newInterceptedObjectQueue(t, ctx, cfg, `#!/usr/bin/env bash
@@ -251,7 +251,7 @@ func TestRequestQueue_RequestObject(t *testing.T) {
 	cfg := testcfg.Build(t)
 
 	oid := git.ObjectID(strings.Repeat("1", gittest.DefaultObjectHash.EncodedLen()))
-	lineEnding := detectEncodedLineEnding(t, ctx, cfg)
+	lineEnding := "\\x00"
 
 	requireRevision := func(t *testing.T, queue *requestQueue, rev git.Revision) {
 		object, err := queue.ReadObject(ctx)
@@ -349,7 +349,7 @@ func TestRequestQueue_RequestInfo(t *testing.T) {
 	oid := git.ObjectID(strings.Repeat("1", gittest.DefaultObjectHash.EncodedLen()))
 	expectedInfo := &ObjectInfo{oid, "blob", 955, gittest.DefaultObjectHash.Format}
 
-	lineEnding := detectEncodedLineEnding(t, ctx, cfg)
+	lineEnding := "\\x00"
 
 	requireRevision := func(t *testing.T, queue *requestQueue) {
 		info, err := queue.ReadInfo(ctx)
@@ -442,7 +442,7 @@ func TestRequestQueue_CommandStats(t *testing.T) {
 	cfg := testcfg.Build(t)
 
 	oid := git.ObjectID(strings.Repeat("1", gittest.DefaultObjectHash.EncodedLen()))
-	lineEnding := detectEncodedLineEnding(t, ctx, cfg)
+	lineEnding := "\\x00"
 
 	_, queue := newInterceptedObjectQueue(t, ctx, cfg, fmt.Sprintf(`#!/usr/bin/env bash
 			IFS= read -r -d '' revision
@@ -517,10 +517,3 @@ func newInterceptedInfoQueue(t *testing.T, ctx context.Context, cfg config.Cfg,
 
 	return reader, queue
 }
-
-func detectEncodedLineEnding(t *testing.T, ctx context.Context, cfg config.Cfg) string {
-	if catfileSupportsNul(t, ctx, cfg) {
-		return "\\x00"
-	}
-	return "\\n"
-}
diff --git a/internal/git/catfile/testhelper_test.go b/internal/git/catfile/testhelper_test.go
index 3e2a7016d..372ddb49f 100644
--- a/internal/git/catfile/testhelper_test.go
+++ b/internal/git/catfile/testhelper_test.go
@@ -109,10 +109,3 @@ func (o *staticObject) Read(p []byte) (int, error) {
 func (o *staticObject) WriteTo(w io.Writer) (int64, error) {
 	return io.Copy(w, o.reader)
 }
-
-func catfileSupportsNul(t *testing.T, ctx context.Context, cfg config.Cfg) bool {
-	t.Helper()
-	gitVersion, err := gittest.NewCommandFactory(t, cfg).GitVersion(ctx)
-	require.NoError(t, err)
-	return gitVersion.CatfileSupportsNulTerminatedOutput()
-}
diff --git a/internal/git/version.go b/internal/git/version.go
index fac10aeb9..4a552b62c 100644
--- a/internal/git/version.go
+++ b/internal/git/version.go
@@ -75,21 +75,6 @@ func (v Version) IsSupported() bool {
 	return !v.LessThan(minimumVersion)
 }
 
-// CatfileSupportsNulTerminatedOutput detects whether git-cat-file(1) knows the `-Z` switch, which causes it to
-// NUL-terminate both stdin and stdout. This new switch has been introduced upstream via a9ea4c23dc (Merge branch
-// 'ps/cat-file-null-output', 2023-06-22).
-//
-// The patches will be part of Git v2.42.0 and have been backported to Git v2.41.0.gl1.
-func (v Version) CatfileSupportsNulTerminatedOutput() bool {
-	if v.major == 2 && v.minor == 41 && v.gl > 0 {
-		return true
-	}
-
-	return !v.LessThan(Version{
-		major: 2, minor: 42,
-	})
-}
-
 // LessThan determines whether the version is older than another version.
 func (v Version) LessThan(other Version) bool {
 	switch {
diff --git a/internal/git/version_test.go b/internal/git/version_test.go
index 5e1d30971..c9a0db8fb 100644
--- a/internal/git/version_test.go
+++ b/internal/git/version_test.go
@@ -122,30 +122,3 @@ func TestVersion_IsSupported(t *testing.T) {
 		})
 	}
 }
-
-func TestVersion_CatfileSupportsNulTerminatedOutput(t *testing.T) {
-	t.Parallel()
-
-	for _, tc := range []struct {
-		version string
-		expect  bool
-	}{
-		{"1.0.0", false},
-		{"2.40.2", false},
-		{"2.41.0", false},
-		{"2.41.1", false},
-		{"2.41.0.gl1", true},
-		{"2.41.0.gl2", true},
-		{"2.41.1.gl1", true},
-		{"2.41.1.gl2", true},
-		{"2.42.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.CatfileSupportsNulTerminatedOutput())
-		})
-	}
-}