Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make libs/exec fallback to sh if bash cannot be found #1114

Merged
merged 4 commits into from
Jan 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions libs/exec/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,23 +46,23 @@ func (c *command) Stderr() io.ReadCloser {
}

type Executor struct {
interpreter interpreter
dir string
shell shell
dir string
}

func NewCommandExecutor(dir string) (*Executor, error) {
interpreter, err := findInterpreter()
shell, err := findShell()
if err != nil {
return nil, err
}
return &Executor{
interpreter: interpreter,
dir: dir,
shell: shell,
dir: dir,
}, nil
}

func (e *Executor) StartCommand(ctx context.Context, command string) (Command, error) {
ec, err := e.interpreter.prepare(command)
ec, err := e.shell.prepare(command)
if err != nil {
return nil, err
}
Expand Down
75 changes: 45 additions & 30 deletions libs/exec/exec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,11 @@ package exec

import (
"context"
"errors"
"fmt"
"io"
"os"
osexec "os/exec"
"runtime"
"sync"
"testing"
Expand Down Expand Up @@ -49,51 +52,63 @@ func TestExecutorWithInvalidCommandWithWindowsLikePath(t *testing.T) {
assert.Contains(t, string(out), "C:\\Program Files\\invalid-command.exe: No such file or directory")
}

func TestFindBashInterpreterNonWindows(t *testing.T) {
if runtime.GOOS == "windows" {
t.SkipNow()
func testExecutorWithShell(t *testing.T, shell string) {
p, err := osexec.LookPath(shell)
if err != nil {
if errors.Is(err, osexec.ErrNotFound) {
switch runtime.GOOS {
case "windows":
if shell == "cmd" {
// We must find `cmd.exe` on Windows.
t.Fatal("cmd.exe not found")
}
default:
if shell == "bash" || shell == "sh" {
// We must find `bash` or `sh` on other operating systems.
t.Fatal("bash or sh not found")
}
}
t.Skipf("shell %s not found", shell)
}
t.Fatal(err)
}

interpreter, err := findBashInterpreter()
assert.NoError(t, err)
assert.NotEmpty(t, interpreter)

e, err := NewCommandExecutor(".")
assert.NoError(t, err)
e.interpreter = interpreter
// Create temporary directory with only the shell executable in the PATH.
tmpDir := t.TempDir()
t.Setenv("PATH", tmpDir)
if runtime.GOOS == "windows" {
os.Symlink(p, fmt.Sprintf("%s/%s.exe", tmpDir, shell))
} else {
os.Symlink(p, fmt.Sprintf("%s/%s", tmpDir, shell))
}

executor, err := NewCommandExecutor(".")
assert.NoError(t, err)
out, err := e.Exec(context.Background(), `echo "Hello from bash"`)
out, err := executor.Exec(context.Background(), "echo 'Hello from shell'")
assert.NoError(t, err)

assert.Equal(t, "Hello from bash\n", string(out))
assert.NotNil(t, out)
assert.Contains(t, string(out), "Hello from shell")
}

func TestFindCmdInterpreter(t *testing.T) {
if runtime.GOOS != "windows" {
t.SkipNow()
func TestExecutorWithDifferentShells(t *testing.T) {
for _, shell := range []string{"bash", "sh", "cmd"} {
t.Run(shell, func(t *testing.T) {
testExecutorWithShell(t, shell)
})
}
}

interpreter, err := findCmdInterpreter()
assert.NoError(t, err)
assert.NotEmpty(t, interpreter)

e, err := NewCommandExecutor(".")
assert.NoError(t, err)
e.interpreter = interpreter

assert.NoError(t, err)
out, err := e.Exec(context.Background(), `echo "Hello from cmd"`)
assert.NoError(t, err)

assert.Contains(t, string(out), "Hello from cmd")
func TestExecutorNoShellFound(t *testing.T) {
t.Setenv("PATH", "")
_, err := NewCommandExecutor(".")
assert.ErrorContains(t, err, "no shell found")
}

func TestExecutorCleanupsTempFiles(t *testing.T) {
executor, err := NewCommandExecutor(".")
assert.NoError(t, err)

ec, err := executor.interpreter.prepare("echo 'Hello'")
ec, err := executor.shell.prepare("echo 'Hello'")
assert.NoError(t, err)

cmd, err := executor.start(context.Background(), ec)
Expand Down
123 changes: 0 additions & 123 deletions libs/exec/interpreter.go

This file was deleted.

54 changes: 54 additions & 0 deletions libs/exec/shell.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package exec

import (
"errors"
"io"
"os"
)

type shell interface {
prepare(string) (*execContext, error)
}

type execContext struct {
executable string
args []string
scriptFile string
}

func findShell() (shell, error) {
for _, fn := range []func() (shell, error){
newBashShell,
newShShell,
newCmdShell,
} {
shell, err := fn()
if err != nil {
return nil, err
}

if shell != nil {
return shell, nil
}
}

return nil, errors.New("no shell found")
}

func createTempScript(command string, extension string) (string, error) {
file, err := os.CreateTemp(os.TempDir(), "cli-exec*"+extension)
if err != nil {
return "", err
}

defer file.Close()

_, err = io.WriteString(file, command)
if err != nil {
// Try to remove the file if we failed to write to it
os.Remove(file.Name())
return "", err
}

return file.Name(), nil
}
37 changes: 37 additions & 0 deletions libs/exec/shell_bash.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package exec

import (
"errors"
osexec "os/exec"
)

type bashShell struct {
executable string
}

func (s bashShell) prepare(command string) (*execContext, error) {
filename, err := createTempScript(command, ".sh")
if err != nil {
return nil, err
}

return &execContext{
executable: s.executable,
args: []string{"-e", filename},
scriptFile: filename,
}, nil
}

func newBashShell() (shell, error) {
out, err := osexec.LookPath("bash")
if err != nil && !errors.Is(err, osexec.ErrNotFound) {
return nil, err
}

// `bash` is not found, return early.
if out == "" {
return nil, nil
}

return &bashShell{executable: out}, nil
}
30 changes: 30 additions & 0 deletions libs/exec/shell_bash_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package exec

import (
"runtime"
"testing"

"github.com/stretchr/testify/assert"
)

func TestBashFound(t *testing.T) {
if runtime.GOOS == "windows" {
t.SkipNow()
}

shell, err := newBashShell()
assert.NoError(t, err)
assert.NotNil(t, shell)
}

func TestBashNotFound(t *testing.T) {
if runtime.GOOS == "windows" {
t.SkipNow()
}

t.Setenv("PATH", "")

shell, err := newBashShell()
assert.NoError(t, err)
assert.Nil(t, shell)
}
Loading
Loading