Add config to specify a git binary path

12 files changed, 112 insertions, 9 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index e41864c5d..5d9f14a5b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,10 @@
 # Gitaly changelog
 
+UNRELEASED
+
+- Add config to specify a git binary path
+  https://gitlab.com/gitlab-org/gitaly/merge_requests/177
+
 v0.17.0
 
 - Rename auth 'unenforced' to 'transitioning'
diff --git a/config.toml.example b/config.toml.example
index b10a37523..ff867cdec 100644
--- a/config.toml.example
+++ b/config.toml.example
@@ -10,6 +10,10 @@ socket_path = "/home/git/gitlab/tmp/sockets/private/gitaly.socket"
 # prometheus_listen_addr = "localhost:9236"
 #
 
+# # Git executable settings
+# [git]
+# bin_path = "/usr/bin/git"
+
 [[storage]]
 name = "default"
 path = "/home/git/repositories"
diff --git a/internal/config/config.go b/internal/config/config.go
index c7fab2f1b..8727cd2a3 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -3,6 +3,7 @@ package config
 import (
 	"fmt"
 	"io"
+	"os/exec"
 
 	log "github.com/Sirupsen/logrus"
 
@@ -19,12 +20,18 @@ type config struct {
 	SocketPath           string     `toml:"socket_path" split_words:"true"`
 	ListenAddr           string     `toml:"listen_addr" split_words:"true"`
 	PrometheusListenAddr string     `toml:"prometheus_listen_addr" split_words:"true"`
+	Git                  Git        `toml:"git" envconfig:"git"`
 	Storages             []Storage  `toml:"storage" envconfig:"storage"`
 	Logging              Logging    `toml:"logging" envconfig:"logging"`
 	Prometheus           Prometheus `toml:"prometheus"`
 	Auth                 Auth       `toml:"auth"`
 }
 
+// Git contains the settings for the Git executable
+type Git struct {
+	BinPath string `toml:"bin_path"`
+}
+
 // Storage contains a single storage-shard
 type Storage struct {
 	Name string
@@ -64,7 +71,7 @@ func Load(file io.Reader) error {
 
 // Validate checks the current Config for sanity.
 func Validate() error {
-	for _, err := range []error{validateStorages(), validateToken()} {
+	for _, err := range []error{validateStorages(), validateToken(), SetGitPath()} {
 		if err != nil {
 			return err
 		}
@@ -93,6 +100,27 @@ func validateStorages() error {
 	return nil
 }
 
+// SetGitPath populates the variable GitPath with the path to the `git`
+// executable. It warns if no path was specified in the configuration.
+func SetGitPath() error {
+	if Config.Git.BinPath != "" {
+		return nil
+	}
+
+	resolvedPath, err := exec.LookPath("git")
+	if err != nil {
+		return err
+	}
+
+	log.WithFields(log.Fields{
+		"resolvedPath": resolvedPath,
+	}).Warn("git path not configured. Using default path resolution")
+
+	Config.Git.BinPath = resolvedPath
+
+	return nil
+}
+
 // StoragePath looks up the base path for storageName. The second boolean
 // return value indicates if anything was found.
 func StoragePath(storageName string) (string, bool) {
diff --git a/internal/config/config_test.go b/internal/config/config_test.go
index e2aaff85c..d31fb3e97 100644
--- a/internal/config/config_test.go
+++ b/internal/config/config_test.go
@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"io"
 	"os"
+	"os/exec"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -238,3 +239,50 @@ func TestStoragePath(t *testing.T) {
 		assert.Equal(t, tc.out, out, "%+v", tc)
 	}
 }
+
+func TestLoadGit(t *testing.T) {
+	tmpFile := configFileReader(`[git]
+bin_path = "/my/git/path"`)
+
+	err := Load(tmpFile)
+	assert.NoError(t, err)
+
+	assert.Equal(t, "/my/git/path", Config.Git.BinPath)
+}
+
+func TestSetGitPath(t *testing.T) {
+	defer func(oldGitSettings Git) {
+		Config.Git = oldGitSettings
+	}(Config.Git)
+
+	resolvedGitPath, err := exec.LookPath("git")
+
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	testCases := []struct {
+		desc       string
+		gitBinPath string
+		expected   string
+	}{
+		{
+			desc:       "With a Git Path set through the settings",
+			gitBinPath: "/path/to/myGit",
+			expected:   "/path/to/myGit",
+		},
+		{
+			desc:       "When a git path hasn't been set",
+			gitBinPath: "",
+			expected:   resolvedGitPath,
+		},
+	}
+
+	for _, tc := range testCases {
+		Config.Git.BinPath = tc.gitBinPath
+
+		SetGitPath()
+
+		assert.Equal(t, tc.expected, Config.Git.BinPath, tc.desc)
+	}
+}
diff --git a/internal/helper/command.go b/internal/helper/command.go
index d66eed92a..48d8e8b43 100644
--- a/internal/helper/command.go
+++ b/internal/helper/command.go
@@ -6,6 +6,10 @@ import (
 	"os"
 	"os/exec"
 	"syscall"
+
+	"gitlab.com/gitlab-org/gitaly/internal/config"
+
+	log "github.com/Sirupsen/logrus"
 )
 
 // Command encapsulates operations with commands creates with NewCommand
@@ -14,6 +18,20 @@ type Command struct {
 	*exec.Cmd
 }
 
+// GitPath returns the path to the `git` binary. See `SetGitPath` for details
+// on how this is set
+func GitPath() string {
+	if config.Config.Git.BinPath == "" {
+		// This shouldn't happen outside of testing, SetGitPath should be called by
+		// main.go to ensure correctness of the configuration on start-up.
+		if err := config.SetGitPath(); err != nil {
+			log.Fatal(err) // Bail out.
+		}
+	}
+
+	return config.Config.Git.BinPath
+}
+
 // Kill cleans the subprocess group of the command. Callers should defer a call
 // to kill after they get the command from NewCommand
 func (c *Command) Kill() {
@@ -22,7 +40,7 @@ func (c *Command) Kill() {
 
 // GitCommandReader creates a git Command with the given args
 func GitCommandReader(args ...string) (*Command, error) {
-	return NewCommand(exec.Command("git", args...), nil, nil, nil)
+	return NewCommand(exec.Command(GitPath(), args...), nil, nil, nil)
 }
 
 // NewCommand creates a Command from an exec.Cmd
diff --git a/internal/service/blob/get_blob.go b/internal/service/blob/get_blob.go
index 9ed179cf9..eeb7b677f 100644
--- a/internal/service/blob/get_blob.go
+++ b/internal/service/blob/get_blob.go
@@ -29,7 +29,7 @@ func (s *server) GetBlob(in *pb.GetBlobRequest, stream pb.BlobService_GetBlobSer
 	stdinReader, stdinWriter := io.Pipe()
 
 	cmdArgs := []string{"--git-dir", repoPath, "cat-file", "--batch"}
-	cmd, err := helper.NewCommand(exec.Command("git", cmdArgs...), stdinReader, nil, nil)
+	cmd, err := helper.NewCommand(exec.Command(helper.GitPath(), cmdArgs...), stdinReader, nil, nil)
 	if err != nil {
 		return grpc.Errorf(codes.Internal, "GetBlob: cmd: %v", err)
 	}
diff --git a/internal/service/commit/isancestor.go b/internal/service/commit/isancestor.go
index 27084b3c9..5ad32885f 100644
--- a/internal/service/commit/isancestor.go
+++ b/internal/service/commit/isancestor.go
@@ -33,7 +33,7 @@ func (s *server) CommitIsAncestor(ctx context.Context, in *pb.CommitIsAncestorRe
 
 // Assumes that `path`, `ancestorID` and `childID` are populated :trollface:
 func commitIsAncestorName(path, ancestorID, childID string) (bool, error) {
-	osCommand := exec.Command("git", "--git-dir", path, "merge-base", "--is-ancestor", ancestorID, childID)
+	osCommand := exec.Command(helper.GitPath(), "--git-dir", path, "merge-base", "--is-ancestor", ancestorID, childID)
 	cmd, err := helper.NewCommand(osCommand, nil, ioutil.Discard, nil)
 	if err != nil {
 		return false, grpc.Errorf(codes.Internal, err.Error())
diff --git a/internal/service/commit/tree_entry.go b/internal/service/commit/tree_entry.go
index c14c530c3..6cbd6610b 100644
--- a/internal/service/commit/tree_entry.go
+++ b/internal/service/commit/tree_entry.go
@@ -40,7 +40,7 @@ func (s *server) TreeEntry(in *pb.TreeEntryRequest, stream pb.CommitService_Tree
 		"cat-file",
 		"--batch",
 	}
-	cmd, err := helper.NewCommand(exec.Command("git", cmdArgs...), stdinReader, nil, nil)
+	cmd, err := helper.NewCommand(exec.Command(helper.GitPath(), cmdArgs...), stdinReader, nil, nil)
 	if err != nil {
 		return grpc.Errorf(codes.Internal, "TreeEntry: cmd: %v", err)
 	}
diff --git a/internal/service/smarthttp/receive_pack.go b/internal/service/smarthttp/receive_pack.go
index 107f731b5..c6881cfe3 100644
--- a/internal/service/smarthttp/receive_pack.go
+++ b/internal/service/smarthttp/receive_pack.go
@@ -47,7 +47,7 @@ func (s *server) PostReceivePack(stream pb.SmartHTTPService_PostReceivePackServe
 		"GlRepository": req.GlRepository,
 	}).Debug("PostReceivePack")
 
-	osCommand := exec.Command("git", "receive-pack", "--stateless-rpc", repoPath)
+	osCommand := exec.Command(helper.GitPath(), "receive-pack", "--stateless-rpc", repoPath)
 	cmd, err := helper.NewCommand(osCommand, stdin, stdout, nil, env...)
 
 	if err != nil {
diff --git a/internal/service/smarthttp/upload_pack.go b/internal/service/smarthttp/upload_pack.go
index 6f3f3733a..833feb989 100644
--- a/internal/service/smarthttp/upload_pack.go
+++ b/internal/service/smarthttp/upload_pack.go
@@ -61,7 +61,7 @@ func (s *server) PostUploadPack(stream pb.SmartHTTPService_PostUploadPackServer)
 		"RepoPath": repoPath,
 	}).Debug("PostUploadPack")
 
-	osCommand := exec.Command("git", "upload-pack", "--stateless-rpc", repoPath)
+	osCommand := exec.Command(helper.GitPath(), "upload-pack", "--stateless-rpc", repoPath)
 	cmd, err := helper.NewCommand(osCommand, stdin, stdout, nil)
 
 	if err != nil {
diff --git a/internal/service/ssh/receive_pack.go b/internal/service/ssh/receive_pack.go
index 8b6f329b2..d848a653f 100644
--- a/internal/service/ssh/receive_pack.go
+++ b/internal/service/ssh/receive_pack.go
@@ -51,7 +51,7 @@ func (s *server) SSHReceivePack(stream pb.SSHService_SSHReceivePackServer) error
 		"GlRepository": req.GlRepository,
 	}).Debug("SSHReceivePack")
 
-	osCommand := exec.Command("git-receive-pack", repoPath)
+	osCommand := exec.Command(helper.GitPath(), "receive-pack", repoPath)
 	cmd, err := helper.NewCommand(osCommand, stdin, stdout, stderr, env...)
 
 	if err != nil {
diff --git a/internal/service/ssh/upload_pack.go b/internal/service/ssh/upload_pack.go
index 05aec775c..94b848c38 100644
--- a/internal/service/ssh/upload_pack.go
+++ b/internal/service/ssh/upload_pack.go
@@ -39,7 +39,7 @@ func (s *server) SSHUploadPack(stream pb.SSHService_SSHUploadPackServer) error {
 		"RepoPath": repoPath,
 	}).Debug("SSHUploadPack")
 
-	osCommand := exec.Command("git-upload-pack", repoPath)
+	osCommand := exec.Command(helper.GitPath(), "upload-pack", repoPath)
 	cmd, err := helper.NewCommand(osCommand, stdin, stdout, stderr)
 
 	if err != nil {