git: Create separate constructor for distributed Git environments

Move the logic which creates execution environments for distributed Git
into its own structure. This will eventually allow us to have a simple
list of all supported Git execution environments in a central location.

3 files changed, 114 insertions, 16 deletions

diff --git a/internal/git/command_factory.go b/internal/git/command_factory.go
index 197f7b859..62ec937bb 100644
--- a/internal/git/command_factory.go
+++ b/internal/git/command_factory.go
@@ -49,14 +49,6 @@ type executionEnvironments struct {
 	bundledGit  ExecutionEnvironment
 }
 
-// ExecutionEnvironment describes the environment required to execute a Git command
-type ExecutionEnvironment struct {
-	// BinaryPath is the path to the Git binary.
-	BinaryPath string
-	// EnvironmentVariables are variables which must be set when running the Git binary.
-	EnvironmentVariables []string
-}
-
 // CommandFactory is designed to create and run git commands in a protected and fully managed manner.
 type CommandFactory interface {
 	// New creates a new command for the repo repository.
@@ -187,11 +179,6 @@ func setupGitExecutionEnvironments(cfg config.Cfg, factoryCfg execCommandFactory
 		}, func() {}, nil
 	}
 
-	binaryPath := cfg.Git.BinPath
-	if override := os.Getenv("GITALY_TESTING_GIT_BINARY"); binaryPath == "" && override != "" {
-		binaryPath = override
-	}
-
 	useBundledBinaries := cfg.Git.UseBundledBinaries
 	if bundledGitPath := os.Getenv("GITALY_TESTING_BUNDLED_GIT_PATH"); bundledGitPath != "" {
 		if cfg.BinDir == "" {
@@ -225,10 +212,13 @@ func setupGitExecutionEnvironments(cfg config.Cfg, factoryCfg execCommandFactory
 	var execEnvs executionEnvironments
 	var cleanups []func()
 
-	if binaryPath != "" {
-		execEnvs.externalGit = ExecutionEnvironment{
-			BinaryPath: binaryPath,
+	if execEnv, cleanup, err := (DistributedGitEnvironmentConstructor{}).Construct(cfg); err != nil {
+		if !errors.Is(err, ErrNotConfigured) {
+			return executionEnvironments{}, nil, fmt.Errorf("constructing distributed Git environment: %w", err)
 		}
+	} else {
+		execEnvs.externalGit = execEnv
+		cleanups = append(cleanups, cleanup)
 	}
 
 	if useBundledBinaries {
diff --git a/internal/git/execution_environment.go b/internal/git/execution_environment.go
new file mode 100644
index 000000000..619622abb
--- /dev/null
+++ b/internal/git/execution_environment.go
@@ -0,0 +1,48 @@
+package git
+
+import (
+	"errors"
+	"os"
+
+	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/config"
+)
+
+// ErrNotConfigured may be returned by an ExecutionEnvironmentConstructor in case an environment
+// was not configured.
+var ErrNotConfigured = errors.New("execution environment is not configured")
+
+// ExecutionEnvironment describes the environment required to execute a Git command
+type ExecutionEnvironment struct {
+	// BinaryPath is the path to the Git binary.
+	BinaryPath string
+	// EnvironmentVariables are variables which must be set when running the Git binary.
+	EnvironmentVariables []string
+}
+
+// DistributedGitEnvironmentConstructor creates ExecutionEnvironments via the Git binary path
+// configured in the Gitaly configuration. This expects a complete Git installation with all its
+// components. The installed distribution must either have its prefix compiled into the binaries or
+// alternatively be compiled with runtime-detection of the prefix such that Git is able to locate
+// its auxiliary helper binaries correctly.
+type DistributedGitEnvironmentConstructor struct{}
+
+// Construct sets up an ExecutionEnvironment for a complete Git distribution. No setup needs to be
+// performed given that the Git environment is expected to be self-contained. The returned function
+// is a cleanup function that shall be executed when the ExecutionEnvironment is no longer used.
+//
+// For testing purposes, this function overrides the configured Git binary path if the
+// `GITALY_TESTING_GIT_BINARY` environment variable is set.
+func (c DistributedGitEnvironmentConstructor) Construct(cfg config.Cfg) (ExecutionEnvironment, func(), error) {
+	binaryPath := cfg.Git.BinPath
+	if override := os.Getenv("GITALY_TESTING_GIT_BINARY"); binaryPath == "" && override != "" {
+		binaryPath = override
+	}
+
+	if binaryPath == "" {
+		return ExecutionEnvironment{}, nil, ErrNotConfigured
+	}
+
+	return ExecutionEnvironment{
+		BinaryPath: binaryPath,
+	}, func() {}, nil
+}
diff --git a/internal/git/execution_environment_test.go b/internal/git/execution_environment_test.go
new file mode 100644
index 000000000..467eacab7
--- /dev/null
+++ b/internal/git/execution_environment_test.go
@@ -0,0 +1,60 @@
+package git_test
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/git"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/config"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper"
+)
+
+func TestDistributedGitEnvironmentConstructor(t *testing.T) {
+	constructor := git.DistributedGitEnvironmentConstructor{}
+
+	testhelper.ModifyEnvironment(t, "GITALY_TESTING_GIT_BINARY", "")
+
+	t.Run("empty configuration fails", func(t *testing.T) {
+		_, _, err := constructor.Construct(config.Cfg{})
+		require.Equal(t, git.ErrNotConfigured, err)
+	})
+
+	t.Run("configuration with Git binary path succeeds", func(t *testing.T) {
+		execEnv, cleanup, err := constructor.Construct(config.Cfg{
+			Git: config.Git{
+				BinPath: "/foo/bar",
+			},
+		})
+		require.NoError(t, err)
+		defer cleanup()
+
+		require.Equal(t, "/foo/bar", execEnv.BinaryPath)
+		require.Equal(t, []string(nil), execEnv.EnvironmentVariables)
+	})
+
+	t.Run("empty configuration with environment override", func(t *testing.T) {
+		testhelper.ModifyEnvironment(t, "GITALY_TESTING_GIT_BINARY", "/foo/bar")
+
+		execEnv, cleanup, err := constructor.Construct(config.Cfg{})
+		require.NoError(t, err)
+		defer cleanup()
+
+		require.Equal(t, "/foo/bar", execEnv.BinaryPath)
+		require.Equal(t, []string(nil), execEnv.EnvironmentVariables)
+	})
+
+	t.Run("configuration overrides environment variable", func(t *testing.T) {
+		testhelper.ModifyEnvironment(t, "GITALY_TESTING_GIT_BINARY", "envvar")
+
+		execEnv, cleanup, err := constructor.Construct(config.Cfg{
+			Git: config.Git{
+				BinPath: "config",
+			},
+		})
+		require.NoError(t, err)
+		defer cleanup()
+
+		require.Equal(t, "config", execEnv.BinaryPath)
+		require.Equal(t, []string(nil), execEnv.EnvironmentVariables)
+	})
+}