Merge branch 'fix-determine-root-dir' into 'master'

Correctly determine the root directory for gitlab-shell

See merge request gitlab-org/gitlab-shell!294

go/cmd/gitlab-shell/main.go

@@ -11,22 +11,24 @@ import (
 	"gitlab.com/gitlab-org/gitlab-shell/go/internal/config"
 )
 
-var (
-	binDir     string
-	rootDir    string
-	readWriter *readwriter.ReadWriter
-)
+// findRootDir determines the root directory (and so, the location of the config
+// file) from os.Executable()
+func findRootDir() (string, error) {
+	path, err := os.Executable()
+	if err != nil {
+		return "", err
+	}
 
-func init() {
-	binDir = filepath.Dir(os.Args[0])
-	rootDir = filepath.Dir(binDir)
-	readWriter = &readwriter.ReadWriter{Out: os.Stdout, In: os.Stdin, ErrOut: os.Stderr}
+	// Start: /opt/.../gitlab-shell/bin/gitlab-shell
+	// Ends:  /opt/.../gitlab-shell
+	return filepath.Dir(filepath.Dir(path)), nil
 }
 
 // rubyExec will never return. It either replaces the current process with a
 // Ruby interpreter, or outputs an error and kills the process.
-func execRuby() {
-	cmd := &fallback.Command{}
+func execRuby(rootDir string, readWriter *readwriter.ReadWriter) {
+	cmd := &fallback.Command{RootDir: rootDir, Args: os.Args}
+
 	if err := cmd.Execute(readWriter); err != nil {
 		fmt.Fprintf(readWriter.ErrOut, "Failed to exec: %v\n", err)
 		os.Exit(1)
@@ -34,12 +36,24 @@ func execRuby() {
 }
 
 func main() {
+	readWriter := &readwriter.ReadWriter{
+		Out:    os.Stdout,
+		In:     os.Stdin,
+		ErrOut: os.Stderr,
+	}
+
+	rootDir, err := findRootDir()
+	if err != nil {
+		fmt.Fprintln(readWriter.ErrOut, "Failed to determine root directory, exiting")
+		os.Exit(1)
+	}
+
 	// Fall back to Ruby in case of problems reading the config, but issue a
 	// warning as this isn't something we can sustain indefinitely
 	config, err := config.NewFromDir(rootDir)
 	if err != nil {
 		fmt.Fprintln(readWriter.ErrOut, "Failed to read config, falling back to gitlab-shell-ruby")
-		execRuby()
+		execRuby(rootDir, readWriter)
 	}
 
 	cmd, err := command.New(os.Args, config)

go/internal/command/command.go

@@ -24,7 +24,7 @@ func New(arguments []string, config *config.Config) (Command, error) {
 		return buildCommand(args, config), nil
 	}
 
-	return &fallback.Command{}, nil
+	return &fallback.Command{RootDir: config.RootDir, Args: arguments}, nil
 }
 
 func buildCommand(args *commandargs.CommandArgs, config *config.Config) Command {

go/internal/command/fallback/fallback.go

@@ -8,14 +8,25 @@ import (
 	"gitlab.com/gitlab-org/gitlab-shell/go/internal/command/readwriter"
 )
 
-type Command struct{}
+type Command struct {
+	RootDir string
+	Args    []string
+}
 
 var (
-	binDir = filepath.Dir(os.Args[0])
+	// execFunc is overridden in tests
+	execFunc = syscall.Exec
+)
+
+const (
+	RubyProgram = "gitlab-shell-ruby"
 )
 
-func (c *Command) Execute(_ *readwriter.ReadWriter) error {
-	rubyCmd := filepath.Join(binDir, "gitlab-shell-ruby")
-	execErr := syscall.Exec(rubyCmd, os.Args, os.Environ())
-	return execErr
+func (c *Command) Execute(*readwriter.ReadWriter) error {
+	rubyCmd := filepath.Join(c.RootDir, "bin", RubyProgram)
+
+	// Ensure rubyArgs[0] is the full path to gitlab-shell-ruby
+	rubyArgs := append([]string{rubyCmd}, c.Args[1:]...)
+
+	return execFunc(rubyCmd, rubyArgs, os.Environ())
 }

go/internal/command/fallback/fallback_test.go

+package fallback
+
+import (
+	"errors"
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+type fakeExec struct {
+	OldExec func(string, []string, []string) error
+	Error   error
+	Called  bool
+
+	Filename string
+	Args     []string
+	Env      []string
+}
+
+var (
+	fakeArgs = []string{"./test", "foo", "bar"}
+)
+
+func (f *fakeExec) Exec(filename string, args []string, env []string) error {
+	f.Called = true
+
+	f.Filename = filename
+	f.Args = args
+	f.Env = env
+
+	return f.Error
+}
+
+func (f *fakeExec) Setup() {
+	f.OldExec = execFunc
+	execFunc = f.Exec
+}
+
+func (f *fakeExec) Cleanup() {
+	execFunc = f.OldExec
+}
+
+func TestExecuteExecsCommandSuccesfully(t *testing.T) {
+	cmd := &Command{RootDir: "/tmp", Args: fakeArgs}
+
+	// Override the exec func
+	fake := &fakeExec{}
+	fake.Setup()
+	defer fake.Cleanup()
+
+	require.NoError(t, cmd.Execute(nil))
+	require.True(t, fake.Called)
+	require.Equal(t, fake.Filename, "/tmp/bin/gitlab-shell-ruby")
+	require.Equal(t, fake.Args, []string{"/tmp/bin/gitlab-shell-ruby", "foo", "bar"})
+	require.Equal(t, fake.Env, os.Environ())
+}
+
+func TestExecuteExecsCommandOnError(t *testing.T) {
+	cmd := &Command{RootDir: "/test", Args: fakeArgs}
+
+	// Override the exec func
+	fake := &fakeExec{Error: errors.New("Test error")}
+	fake.Setup()
+	defer fake.Cleanup()
+
+	require.Error(t, cmd.Execute(nil))
+	require.True(t, fake.Called)
+}
+
+func TestExecuteGivenNonexistentCommand(t *testing.T) {
+	cmd := &Command{RootDir: "/tmp/does/not/exist", Args: fakeArgs}
+
+	require.Error(t, cmd.Execute(nil))
+}