Merge branch 'jc-add-git-2.38' into 'master'

Add git 2.38 as a build target in Makefile

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

Merged-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Approved-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Co-authored-by: John Cai <jcai@gitlab.com>

5 files changed, 52 insertions, 13 deletions

diff --git a/Makefile b/Makefile
index c7f307f82..bd66f8d4e 100644
--- a/Makefile
+++ b/Makefile
@@ -323,13 +323,16 @@ install: build
 .PHONY: build-bundled-git
 ## Build bundled Git binaries.
 build-bundled-git: build-bundled-git-v2.37.1.gl1
+build-bundled-git: build-bundled-git-v2.38
 build-bundled-git-v2.37.1.gl1: $(patsubst %,${BUILD_DIR}/bin/gitaly-%-v2.37.1.gl1,${GIT_EXECUTABLES})
+build-bundled-git-v2.38: $(patsubst %,${BUILD_DIR}/bin/gitaly-%-v2.38,${GIT_EXECUTABLES})
 
 .PHONY: install-bundled-git
 ## Install bundled Git binaries. The target directory can be modified by
 ## setting PREFIX and DESTDIR.
-install-bundled-git: install-bundled-git-v2.37.1.gl1
+install-bundled-git: install-bundled-git-v2.37.1.gl1 install-bundled-git-v2.38
 install-bundled-git-v2.37.1.gl1: $(patsubst %,${INSTALL_DEST_DIR}/gitaly-%-v2.37.1.gl1,${GIT_EXECUTABLES})
+install-bundled-git-v2.38: $(patsubst %,${INSTALL_DEST_DIR}/gitaly-%-v2.38,${GIT_EXECUTABLES})
 
 ifdef WITH_BUNDLED_GIT
 build: build-bundled-git
@@ -579,6 +582,11 @@ ${BUILD_DIR}/bin/gitaly-%-v2.37.1.gl1: override GIT_EXTRA_VERSION = gl1
 ${BUILD_DIR}/bin/gitaly-%-v2.37.1.gl1: ${DEPENDENCY_DIR}/git-v2.37.1.gl1/% | ${BUILD_DIR}/bin
 	${Q}install $< $@
 
+${BUILD_DIR}/bin/gitaly-%-v2.38: override GIT_VERSION = v2.38.1
+${BUILD_DIR}/bin/gitaly-%-v2.38: override GIT_EXTRA_VERSION = gl0
+${BUILD_DIR}/bin/gitaly-%-v2.38: ${DEPENDENCY_DIR}/git-v2.38/% | ${BUILD_DIR}/bin
+	${Q}install $< $@
+
 ${BUILD_DIR}/bin/%: ${BUILD_DIR}/intermediate/% | ${BUILD_DIR}/bin
 	@ # To compute a unique and deterministic value for GNU build-id, we use an
 	@ # intermediate binary which has a fixed build ID of "TEMP_GITALY_BUILD_ID",
diff --git a/internal/git/command_factory_test.go b/internal/git/command_factory_test.go
index 66737b181..45a2b8e2e 100644
--- a/internal/git/command_factory_test.go
+++ b/internal/git/command_factory_test.go
@@ -20,6 +20,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git/gittest"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/helper/text"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testcfg"
 )
@@ -219,8 +220,7 @@ func TestCommandFactory_ExecutionEnvironment(t *testing.T) {
 	})
 
 	t.Run("set using GITALY_TESTING_BUNDLED_GIT_PATH", func(t *testing.T) {
-		suffix := "-v2.37.1.gl1"
-
+		suffixes := []string{"-v2.37.1.gl1", "-v2.38"}
 		bundledGitDir := testhelper.TempDir(t)
 
 		var bundledGitConstructors []git.BundledGitEnvironmentConstructor
@@ -232,8 +232,11 @@ func TestCommandFactory_ExecutionEnvironment(t *testing.T) {
 		}
 		require.NotEmpty(t, bundledGitConstructors)
 
-		bundledGitExecutable := filepath.Join(bundledGitDir, "gitaly-git"+suffix)
-		bundledGitRemoteExecutable := filepath.Join(bundledGitDir, "gitaly-git-remote-http"+suffix)
+		var bundledGitExecutables, bundledGitRemoteExecutables []string
+		for _, suffix := range suffixes {
+			bundledGitExecutables = append(bundledGitExecutables, filepath.Join(bundledGitDir, "gitaly-git"+suffix))
+			bundledGitRemoteExecutables = append(bundledGitRemoteExecutables, filepath.Join(bundledGitDir, "gitaly-git-remote-http"+suffix))
+		}
 
 		t.Setenv("GITALY_TESTING_BUNDLED_GIT_PATH", bundledGitDir)
 
@@ -245,30 +248,41 @@ func TestCommandFactory_ExecutionEnvironment(t *testing.T) {
 		t.Run("missing gitaly-git executable", func(t *testing.T) {
 			_, _, err := git.NewExecCommandFactory(config.Cfg{BinDir: testhelper.TempDir(t)}, git.WithSkipHooks())
 			require.Error(t, err)
-			require.Contains(t, err.Error(), fmt.Sprintf(`statting "gitaly-git%s":`, suffix))
+			require.Contains(t, err.Error(), `statting "gitaly-git-v2.38"`)
 			require.True(t, errors.Is(err, os.ErrNotExist))
 		})
 
 		t.Run("missing git-remote-http executable", func(t *testing.T) {
-			require.NoError(t, os.WriteFile(bundledGitExecutable, []byte{}, 0o777))
+			for _, bundledGitExecutable := range bundledGitExecutables {
+				require.NoError(t, os.WriteFile(bundledGitExecutable, []byte{}, 0o777))
+			}
 
 			_, _, err := git.NewExecCommandFactory(config.Cfg{BinDir: testhelper.TempDir(t)}, git.WithSkipHooks())
 			require.Error(t, err)
-			require.Contains(t, err.Error(), fmt.Sprintf("statting \"gitaly-git-remote-http%s\":", suffix))
+			require.Contains(t, err.Error(), `statting "gitaly-git-remote-http-v2.38"`)
 			require.True(t, errors.Is(err, os.ErrNotExist))
 		})
 
 		t.Run("missing git-http-backend executable", func(t *testing.T) {
-			require.NoError(t, os.WriteFile(bundledGitExecutable, []byte{}, 0o777))
-			require.NoError(t, os.WriteFile(bundledGitRemoteExecutable, []byte{}, 0o777))
+			for _, bundledGitExecutable := range bundledGitExecutables {
+				require.NoError(t, os.WriteFile(bundledGitExecutable, []byte{}, 0o777))
+			}
+			for _, bundledGitRemoteExecutable := range bundledGitRemoteExecutables {
+				require.NoError(t, os.WriteFile(bundledGitRemoteExecutable, []byte{}, 0o777))
+			}
 
 			_, _, err := git.NewExecCommandFactory(config.Cfg{BinDir: testhelper.TempDir(t)}, git.WithSkipHooks())
 			require.Error(t, err)
-			require.Contains(t, err.Error(), fmt.Sprintf("statting \"gitaly-git-http-backend%s\":", suffix))
+			require.Contains(t, err.Error(), `statting "gitaly-git-http-backend-v2.38"`)
 			require.True(t, errors.Is(err, os.ErrNotExist))
 		})
 
 		t.Run("bin_dir with executables", func(t *testing.T) {
+			expectedSuffix := "-v2.37.1.gl1"
+			if featureflag.GitV238.IsEnabled(ctx) {
+				expectedSuffix = "-v2.38"
+			}
+
 			for _, bundledGitConstructor := range bundledGitConstructors {
 				for _, gitBinary := range []string{"gitaly-git", "gitaly-git-remote-http", "gitaly-git-http-backend"} {
 					require.NoError(t, os.WriteFile(filepath.Join(bundledGitDir, gitBinary+bundledGitConstructor.Suffix), []byte{}, 0o777))
@@ -289,14 +303,14 @@ func TestCommandFactory_ExecutionEnvironment(t *testing.T) {
 				// executable in Gitaly's BinDir.
 				target, err := os.Readlink(symlinkPath)
 				require.NoError(t, err)
-				require.Equal(t, filepath.Join(binDir, "gitaly-"+executable+suffix), target)
+				require.Equal(t, filepath.Join(binDir, "gitaly-"+executable+expectedSuffix), target)
 
 				// And in a test setup, the symlink in Gitaly's BinDir must point to
 				// the Git binary pointed to by the GITALY_TESTING_BUNDLED_GIT_PATH
 				// environment variable.
 				target, err = os.Readlink(target)
 				require.NoError(t, err)
-				require.Equal(t, filepath.Join(bundledGitDir, "gitaly-"+executable+suffix), target)
+				require.Equal(t, filepath.Join(bundledGitDir, "gitaly-"+executable+expectedSuffix), target)
 			}
 		})
 	})
diff --git a/internal/git/execution_environment.go b/internal/git/execution_environment.go
index b313dce85..8ce1fbb7f 100644
--- a/internal/git/execution_environment.go
+++ b/internal/git/execution_environment.go
@@ -26,6 +26,12 @@ var (
 	// case `IsEnabled()` returns `false` though.
 	ExecutionEnvironmentConstructors = []ExecutionEnvironmentConstructor{
 		BundledGitEnvironmentConstructor{
+			Suffix: "-v2.38",
+			FeatureFlags: []featureflag.FeatureFlag{
+				featureflag.GitV238,
+			},
+		},
+		BundledGitEnvironmentConstructor{
 			Suffix: "-v2.37.1.gl1",
 		},
 		DistributedGitEnvironmentConstructor{},
diff --git a/internal/metadata/featureflag/ff_git_v238.go b/internal/metadata/featureflag/ff_git_v238.go
new file mode 100644
index 000000000..158e61448
--- /dev/null
+++ b/internal/metadata/featureflag/ff_git_v238.go
@@ -0,0 +1,9 @@
+package featureflag
+
+// GitV238 will enable the use of Git v2.38
+var GitV238 = NewFeatureFlag(
+	"git_v238",
+	"v15.6.0",
+	"https://gitlab.com/gitlab-org/gitaly/-/issues/4553",
+	false,
+)
diff --git a/internal/testhelper/testhelper.go b/internal/testhelper/testhelper.go
index d449367e0..c247a949e 100644
--- a/internal/testhelper/testhelper.go
+++ b/internal/testhelper/testhelper.go
@@ -200,6 +200,8 @@ func ContextWithoutCancel(opts ...ContextOpt) context.Context {
 	// Randomly enable the flag to exercise both paths to some extent.
 	ctx = featureflag.ContextWithFeatureFlag(ctx, featureflag.NodeErrorCancelsVoter, rnd.Int()%2 == 0)
 
+	ctx = featureflag.ContextWithFeatureFlag(ctx, featureflag.GitV238, rnd.Int()%2 == 0)
+
 	for _, opt := range opts {
 		ctx = opt(ctx)
 	}