Merge branch 'pks-ff-remove-bundled-git' into 'master'

git: Remove feature flag for bundled Git

Closes #4064

See merge request gitlab-org/gitaly!4429

4 files changed, 7 insertions, 18 deletions

diff --git a/internal/git/command_factory_test.go b/internal/git/command_factory_test.go
index f960c2c2e..067dace1e 100644
--- a/internal/git/command_factory_test.go
+++ b/internal/git/command_factory_test.go
@@ -17,6 +17,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git/gittest"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/config"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/helper/text"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper/testcfg"
 )
@@ -180,6 +181,10 @@ func TestCommandFactory_ExecutionEnvironment(t *testing.T) {
 	})
 
 	t.Run("set using GITALY_TESTING_BUNDLED_GIT_PATH", func(t *testing.T) {
+		// Force use of Git v2.35.1 so that we can easily handle the expected suffix.
+		ctx := featureflag.ContextWithFeatureFlag(ctx, featureflag.GitV2351WithFetchSpeedups, true)
+		suffix := "-v2.35.1.gl1"
+
 		bundledGitDir := testhelper.TempDir(t)
 
 		var bundledGitConstructors []git.BundledGitEnvironmentConstructor
@@ -191,8 +196,6 @@ func TestCommandFactory_ExecutionEnvironment(t *testing.T) {
 		}
 		require.NotEmpty(t, bundledGitConstructors)
 
-		suffix := bundledGitConstructors[0].Suffix
-
 		bundledGitExecutable := filepath.Join(bundledGitDir, "gitaly-git"+suffix)
 		bundledGitRemoteExecutable := filepath.Join(bundledGitDir, "gitaly-git-remote-http"+suffix)
 
diff --git a/internal/git/execution_environment.go b/internal/git/execution_environment.go
index f4d981a51..e6e0fe363 100644
--- a/internal/git/execution_environment.go
+++ b/internal/git/execution_environment.go
@@ -28,7 +28,6 @@ var (
 		BundledGitEnvironmentConstructor{
 			Suffix: "-v2.35.1.gl1",
 			FeatureFlags: []featureflag.FeatureFlag{
-				featureflag.UseBundledGit,
 				featureflag.GitV2351WithFetchSpeedups,
 			},
 		},
@@ -36,9 +35,6 @@ var (
 			// This is the current default bundled Git environment, which does not yet
 			// have a version suffix.
 			Suffix: "",
-			FeatureFlags: []featureflag.FeatureFlag{
-				featureflag.UseBundledGit,
-			},
 		},
 		DistributedGitEnvironmentConstructor{},
 		FallbackGitEnvironmentConstructor{},
diff --git a/internal/metadata/featureflag/ff_use_bundled_git.go b/internal/metadata/featureflag/ff_use_bundled_git.go
deleted file mode 100644
index e833c4e78..000000000
--- a/internal/metadata/featureflag/ff_use_bundled_git.go
+++ /dev/null
@@ -1,5 +0,0 @@
-package featureflag
-
-// UseBundledGit enables the use of bundled Git if the Gitaly configuration has both a binary path
-// and the bundled Git enabled.
-var UseBundledGit = NewFeatureFlag("use_bundled_git", true)
diff --git a/internal/testhelper/testhelper.go b/internal/testhelper/testhelper.go
index 496404cba..6bf7d3839 100644
--- a/internal/testhelper/testhelper.go
+++ b/internal/testhelper/testhelper.go
@@ -175,13 +175,8 @@ func ContextWithoutCancel(opts ...ContextOpt) context.Context {
 	// ConcurrencyQueueMaxWait is in the codepath of every RPC call since it's in the limithandler
 	// middleware.
 	ctx = featureflag.ContextWithFeatureFlag(ctx, featureflag.ConcurrencyQueueMaxWait, true)
-	// We support using both bundled and non-bundled Git, which can be toggled via a feature
-	// flag if both are configured. Naturally, this kicks in whenever we spawn a Git command,
-	// and thus it's not feasible to inject the feature flag everywhere. Instead, we just use
-	// one of both randomly.
-	ctx = featureflag.ContextWithFeatureFlag(ctx, featureflag.UseBundledGit, mrand.Int()%2 == 0)
-	// Same as with the preceding feature flag, this flag is checked whenever we execute a Git
-	// command.
+	// This flag is checked whenever we execute a Git command, making it infeasible to inject it
+	// at every callsite.
 	ctx = featureflag.ContextWithFeatureFlag(ctx, featureflag.GitV2351WithFetchSpeedups, mrand.Int()%2 == 0)
 
 	for _, opt := range opts {