git: Deprecate `Version.FlushesUpdaterefStatus`remove-git-v2.35.0

This was used to identify if the git version is greater than v2.36.0.
Now that the minimum version is set to v2.37.0, we no longer need to
verify the git version to add this configuration. Let's remove it and
it's associated usages with tests.

7 files changed, 20 insertions, 104 deletions

diff --git a/internal/git/gittest/command_factory.go b/internal/git/gittest/command_factory.go
index 863de697b..0285a1f9c 100644
--- a/internal/git/gittest/command_factory.go
+++ b/internal/git/gittest/command_factory.go
@@ -1,7 +1,6 @@
 package gittest
 
 import (
-	"context"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -17,11 +16,3 @@ func NewCommandFactory(tb testing.TB, cfg config.Cfg, opts ...git.ExecCommandFac
 	tb.Cleanup(cleanup)
 	return factory
 }
-
-// GitSupportsStatusFlushing returns whether or not the current version of Git
-// supports status flushing.
-func GitSupportsStatusFlushing(t *testing.T, ctx context.Context, cfg config.Cfg) bool {
-	version, err := NewCommandFactory(t, cfg).GitVersion(ctx)
-	require.NoError(t, err)
-	return version.FlushesUpdaterefStatus()
-}
diff --git a/internal/git/updateref/updateref.go b/internal/git/updateref/updateref.go
index d99b49750..234a9c1fb 100644
--- a/internal/git/updateref/updateref.go
+++ b/internal/git/updateref/updateref.go
@@ -30,10 +30,6 @@ type Updater struct {
 	stdout     *bufio.Reader
 	stderr     *bytes.Buffer
 	objectHash git.ObjectHash
-
-	// withStatusFlushing determines whether the Git version used supports proper flushing of
-	// status messages.
-	withStatusFlushing bool
 }
 
 // UpdaterOpt is a type representing options for the Updater.
@@ -82,23 +78,17 @@ func New(ctx context.Context, repo git.RepositoryExecutor, opts ...UpdaterOpt) (
 		return nil, err
 	}
 
-	gitVersion, err := repo.GitVersion(ctx)
-	if err != nil {
-		return nil, fmt.Errorf("determining git version: %w", err)
-	}
-
 	objectHash, err := repo.ObjectHash(ctx)
 	if err != nil {
 		return nil, fmt.Errorf("detecting object hash: %w", err)
 	}
 
 	updater := &Updater{
-		repo:               repo,
-		cmd:                cmd,
-		stderr:             &stderr,
-		stdout:             bufio.NewReader(cmd),
-		objectHash:         objectHash,
-		withStatusFlushing: gitVersion.FlushesUpdaterefStatus(),
+		repo:       repo,
+		cmd:        cmd,
+		stderr:     &stderr,
+		stdout:     bufio.NewReader(cmd),
+		objectHash: objectHash,
 	}
 
 	// By writing an explicit "start" to the command, we enable
@@ -185,22 +175,19 @@ func (u *Updater) setState(state string) error {
 	// "<command>: ok" lines printed to its stdout. Ideally, we should thus verify here whether
 	// the command was successfully executed by checking for exactly this line, otherwise we
 	// cannot be sure whether the command has correctly been processed by Git or if an error was
-	// raised. Unfortunately, Git only knows to flush these reports either starting with v2.34.0
-	// or with our backported version v2.33.0.gl3.
-	if u.withStatusFlushing {
-		line, err := u.stdout.ReadString('\n')
-		if err != nil {
-			// We need to explicitly cancel the command here and wait for it to
-			// terminate such that we can retrieve the command's stderr in a race-free
-			// manner.
-			_ = u.Cancel()
-
-			return fmt.Errorf("state update to %q failed: %w, stderr: %q", state, err, u.stderr)
-		}
+	// raised.
+	line, err := u.stdout.ReadString('\n')
+	if err != nil {
+		// We need to explicitly cancel the command here and wait for it to
+		// terminate such that we can retrieve the command's stderr in a race-free
+		// manner.
+		_ = u.Cancel()
 
-		if line != fmt.Sprintf("%s: ok\n", state) {
-			return fmt.Errorf("state update to %q not successful: expected ok, got %q", state, line)
-		}
+		return fmt.Errorf("state update to %q failed: %w, stderr: %q", state, err, u.stderr)
+	}
+
+	if line != fmt.Sprintf("%s: ok\n", state) {
+		return fmt.Errorf("state update to %q not successful: expected ok, got %q", state, line)
 	}
 
 	return nil
diff --git a/internal/git/updateref/updateref_test.go b/internal/git/updateref/updateref_test.go
index 50c2d7eb1..e39a55665 100644
--- a/internal/git/updateref/updateref_test.go
+++ b/internal/git/updateref/updateref_test.go
@@ -131,10 +131,6 @@ func TestUpdater_concurrentLocking(t *testing.T) {
 
 	cfg := testcfg.Build(t)
 
-	if !gittest.GitSupportsStatusFlushing(t, ctx, cfg) {
-		t.Skip("git does not support flushing yet, which is known to be flaky")
-	}
-
 	repoProto, repoPath := gittest.CreateRepository(ctx, t, cfg, gittest.CreateRepositoryConfig{
 		SkipCreationViaService: true,
 	})
@@ -228,10 +224,6 @@ func TestUpdater_cancel(t *testing.T) {
 
 	cfg, repo, repoPath, updater := setupUpdater(t, ctx)
 
-	if !gittest.GitSupportsStatusFlushing(t, ctx, cfg) {
-		t.Skip("git does not support flushing yet, which is known to be flaky")
-	}
-
 	gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("main"))
 
 	// Queue the branch for deletion and lock it.
@@ -286,22 +278,15 @@ func TestUpdater_capturesStderr(t *testing.T) {
 
 	ctx := testhelper.Context(t)
 
-	cfg, _, _, updater := setupUpdater(t, ctx)
+	_, _, _, updater := setupUpdater(t, ctx)
 
 	newValue := git.ObjectID(strings.Repeat("1", gittest.DefaultObjectHash.EncodedLen()))
 	oldValue := gittest.DefaultObjectHash.ZeroOID
 
 	require.NoError(t, updater.Update("refs/heads/main", newValue, oldValue))
 
-	var expectedErr string
-	if gittest.GitSupportsStatusFlushing(t, ctx, cfg) {
-		expectedErr = fmt.Sprintf("state update to \"commit\" failed: EOF, stderr: \"fatal: commit: cannot update ref '%[1]s': "+
-			"trying to write ref '%[1]s' with nonexistent object %[2]s\\n\"", "refs/heads/main", newValue)
-	} else {
-		expectedErr = fmt.Sprintf("git update-ref: exit status 128, stderr: "+
-			"\"fatal: commit: cannot update ref '%[1]s': "+
-			"trying to write ref '%[1]s' with nonexistent object %[2]s\\n\"", "refs/heads/main", newValue)
-	}
+	expectedErr := fmt.Sprintf("state update to \"commit\" failed: EOF, stderr: \"fatal: commit: cannot update ref '%[1]s': "+
+		"trying to write ref '%[1]s' with nonexistent object %[2]s\\n\"", "refs/heads/main", newValue)
 
 	err := updater.Commit()
 	require.NotNil(t, err)
diff --git a/internal/git/version.go b/internal/git/version.go
index db46eef78..5ce5b6cba 100644
--- a/internal/git/version.go
+++ b/internal/git/version.go
@@ -73,17 +73,6 @@ func (v Version) IsSupported() bool {
 	return !v.LessThan(minimumVersion)
 }
 
-// FlushesUpdaterefStatus determines whether the given Git version properly flushes status messages
-// in git-update-ref(1).
-func (v Version) FlushesUpdaterefStatus() bool {
-	// This has been released with v2.33.1 and will be part of v2.34.0. It's also contained in
-	// our custom-patched version v2.33.0-gl3. So everything newer than these versions is
-	// supported.
-	return !v.LessThan(Version{
-		major: 2, minor: 33, patch: 0, gl: 3,
-	})
-}
-
 // 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 9789bd200..b7c5c6cd2 100644
--- a/internal/git/version_test.go
+++ b/internal/git/version_test.go
@@ -124,28 +124,3 @@ func TestVersion_IsSupported(t *testing.T) {
 		})
 	}
 }
-
-func TestVersion_FlushesUpdaterefStatus(t *testing.T) {
-	for _, tc := range []struct {
-		version string
-		expect  bool
-	}{
-		{"2.31.0", false},
-		{"2.31.0-rc0", false},
-		{"2.31.1", false},
-		{"2.37.0", true},
-		{"2.37.0.gl0", true},
-		{"2.37.0.gl2", true},
-		{"2.37.0.gl3", true},
-		{"2.37.0.gl4", true},
-		{"2.33.1", true},
-		{"3.0.0", true},
-		{"3.0.0.gl5", 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())
-		})
-	}
-}
diff --git a/internal/gitaly/service/objectpool/fetch_into_object_pool_test.go b/internal/gitaly/service/objectpool/fetch_into_object_pool_test.go
index 03262537b..63d8f937b 100644
--- a/internal/gitaly/service/objectpool/fetch_into_object_pool_test.go
+++ b/internal/gitaly/service/objectpool/fetch_into_object_pool_test.go
@@ -401,14 +401,7 @@ func testFetchIntoObjectPoolDfConflict(t *testing.T, ctx context.Context) {
 	gittest.Exec(t, cfg, "-C", repoPath, "update-ref", "-d", "refs/heads/branch")
 	gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("branch/conflict"))
 
-	gitVersion, err := gittest.NewCommandFactory(t, cfg).GitVersion(ctx)
-	require.NoError(t, err)
-
-	// Due to a bug in old Git versions we may get an unexpected exit status.
 	expectedExitStatus := 254
-	if !gitVersion.FlushesUpdaterefStatus() {
-		expectedExitStatus = 1
-	}
 
 	// Verify that we can still fetch into the object pool regardless of the D/F conflict. While
 	// it is not possible to store both references at the same time due to the conflict, we
diff --git a/internal/gitaly/service/ref/delete_refs_test.go b/internal/gitaly/service/ref/delete_refs_test.go
index a821999f7..324341368 100644
--- a/internal/gitaly/service/ref/delete_refs_test.go
+++ b/internal/gitaly/service/ref/delete_refs_test.go
@@ -223,10 +223,6 @@ func TestDeleteRefs_refLocked(t *testing.T) {
 func testDeleteRefsRefLocked(t *testing.T, ctx context.Context) {
 	cfg, repoProto, _, client := setupRefService(ctx, t)
 
-	if !gittest.GitSupportsStatusFlushing(t, ctx, cfg) {
-		t.Skip("git does not support flushing yet, which is known to be flaky")
-	}
-
 	request := &gitalypb.DeleteRefsRequest{
 		Repository: repoProto,
 		Refs:       [][]byte{[]byte("refs/heads/master")},