diff --git a/internal/nix/run.go b/internal/nix/run.go index 2625cddc6f1..e51ef706b92 100644 --- a/internal/nix/run.go +++ b/internal/nix/run.go @@ -24,9 +24,7 @@ func RunScript(projectDir, cmdWithArgs string, env map[string]string) error { envPairs = append(envPairs, fmt.Sprintf("%s=%s", k, v)) } - // Try to find sh in the PATH, if not, default to a well known absolute path. - shPath := cmdutil.GetPathOrDefault("sh", "/bin/sh") - cmd := exec.Command(shPath, "-c", cmdWithArgs) + cmd := exec.Command(scriptRunnerPath(), "-c", cmdWithArgs) cmd.Env = envPairs cmd.Dir = projectDir cmd.Stdin = os.Stdin @@ -37,3 +35,28 @@ func RunScript(projectDir, cmdWithArgs string, env map[string]string) error { // Report error as exec error when executing scripts. return usererr.NewExecError(cmd.Run()) } + +// scriptRunnerPath returns the shell used to execute Devbox scripts and init +// hooks. It prefers bash so that scripts run with `devbox run` behave the same +// as they do inside `devbox shell`, which sources the init hooks from the +// user's interactive shell (usually bash). Init hooks commonly rely on bash +// features such as the `source` builtin, which POSIX sh implementations like +// dash don't provide. We fall back to `sh` (and finally a well-known absolute +// path) when bash isn't available. +// See https://github.com/jetify-com/devbox/issues/2607 +func scriptRunnerPath() string { + // Prefer bash on PATH. + if bashPath := cmdutil.GetPathOrDefault("bash", ""); bashPath != "" { + return bashPath + } + // bash may be installed at a well-known location even when it isn't on + // PATH (e.g. a restricted environment where PATH doesn't include /bin or + // /usr/bin). Check those before giving up on bash. + for _, p := range []string{"/bin/bash", "/usr/bin/bash", "/usr/local/bin/bash"} { + if fi, err := os.Stat(p); err == nil && !fi.IsDir() { + return p + } + } + // Fall back to POSIX sh. + return cmdutil.GetPathOrDefault("sh", "/bin/sh") +} diff --git a/internal/nix/run_test.go b/internal/nix/run_test.go new file mode 100644 index 00000000000..60555e6c0a6 --- /dev/null +++ b/internal/nix/run_test.go @@ -0,0 +1,46 @@ +// Copyright 2024 Jetify Inc. and contributors. All rights reserved. +// Use of this source code is governed by the license in the LICENSE file. + +package nix + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "go.jetify.com/devbox/internal/cmdutil" +) + +// TestRunScriptUsesBashForBashisms verifies that RunScript executes scripts +// with bash so that init hooks and scripts relying on bash features work with +// `devbox run`, matching the behavior of `devbox shell`. `source` is a bash +// builtin that POSIX sh implementations like dash don't provide, so it makes a +// good proxy for "are we running under bash?". +// +// Regression test for https://github.com/jetify-com/devbox/issues/2607 +func TestRunScriptUsesBashForBashisms(t *testing.T) { + if !cmdutil.Exists("bash") { + t.Skip("bash is not installed; RunScript falls back to sh") + } + + // `source` fails under dash (`source: not found`) but succeeds under bash. + dir := t.TempDir() + sourced := filepath.Join(dir, "sourced.sh") + if err := os.WriteFile(sourced, []byte("SOURCED=1\n"), 0o644); err != nil { + t.Fatal(err) + } + err := RunScript(dir, "source "+sourced, map[string]string{}) + if err != nil { + t.Fatalf("RunScript should run bashisms under bash, got error: %v", err) + } +} + +func TestScriptRunnerPathPrefersBash(t *testing.T) { + if !cmdutil.Exists("bash") { + t.Skip("bash is not installed") + } + if got := scriptRunnerPath(); !strings.HasSuffix(got, "bash") { + t.Errorf("scriptRunnerPath() = %q, want a path ending in \"bash\"", got) + } +} diff --git a/internal/shellgen/scripts.go b/internal/shellgen/scripts.go index 35154ad1700..0a4f6e0075f 100644 --- a/internal/shellgen/scripts.go +++ b/internal/shellgen/scripts.go @@ -106,7 +106,8 @@ func WriteScriptFile(devbox devboxer, name, body string) (err error) { defer script.Close() // best effort: close file if featureflag.ScriptExitOnError.Enabled() { - // NOTE: Devbox scripts run using `sh` for consistency. + // NOTE: Devbox scripts run using bash when available (see + // nix.scriptRunnerPath), falling back to POSIX sh; `set -e` works in both. body = fmt.Sprintf("set -e\n\n%s", body) } _, err = script.WriteString(body) diff --git a/internal/shellgen/tmpl/script-wrapper.tmpl b/internal/shellgen/tmpl/script-wrapper.tmpl index de191c431be..7b3a3a4e687 100644 --- a/internal/shellgen/tmpl/script-wrapper.tmpl +++ b/internal/shellgen/tmpl/script-wrapper.tmpl @@ -3,8 +3,10 @@ hooks once, even if the init hook calls devbox run again. This will also protect against using devbox service in the init hook. - Scripts always use sh to run, so POSIX is OK. We don't (yet) support fish - scripts. (though users can run a fish script within their script) + Scripts run with bash when it's available (falling back to POSIX sh), so + that they behave the same as init hooks do inside `devbox shell`. We don't + (yet) support fish scripts. (though users can run a fish script within their + script) */ -}} if [ -z "${{ .SkipInitHookHash }}" ]; then diff --git a/testscripts/run/init_hook_source.test.txt b/testscripts/run/init_hook_source.test.txt new file mode 100644 index 00000000000..a940f0a03fc --- /dev/null +++ b/testscripts/run/init_hook_source.test.txt @@ -0,0 +1,28 @@ +# An init hook that uses the bash `source` builtin should work with +# `devbox run`, just like it does inside `devbox shell`. Previously this +# failed with "source: not found" on systems where /bin/sh is a POSIX shell +# such as dash, because scripts were run with sh instead of bash. +# Regression test for https://github.com/jetify-com/devbox/issues/2607 + +# A script that relies on state set up by a sourced init hook should run. +exec devbox run print_sourced +stdout 'sourced-value' + +# Running the `source` builtin directly should also work. +exec devbox run source_directly +stdout 'ran source ok' + +-- devbox.json -- +{ + "packages": [], + "shell": { + "init_hook": "source ./configure.sh", + "scripts": { + "print_sourced": "echo $SOURCED_VAR", + "source_directly": "source ./configure.sh && echo \"ran source ok\"" + } + } +} + +-- configure.sh -- +export SOURCED_VAR=sourced-value