{{ message }}
Repository navigation
fix(run): run scripts and init hooks with bash, not sh (#2607) - #2928
Draft
mikeland73 wants to merge 2 commits into
Draft
mikeland73 wants to merge 2 commits into
mikeland73 wants to merge 2 commits into
Conversation
`devbox run` executed generated scripts (and the init hook they source)
with `sh`, which on many systems is a POSIX shell such as dash. Init
hooks that use bash features — most commonly the `source` builtin —
work inside `devbox shell` (which runs them under the user's interactive
shell, usually bash) but failed under `devbox run` with:
.hooks.sh: source: not found
This inconsistency between `devbox shell` and `devbox run` is surprising.
Have nix.RunScript prefer bash, falling back to POSIX sh (and finally a
well-known absolute path) when bash isn't installed. Running scripts
under bash matches the interactive shell and is a strict superset of the
previous behavior for POSIX scripts.
Adds a unit test in the nix package and an integration testscript that
reproduces the sourced-init-hook case, and updates the now-stale
"scripts always use sh" comments.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WEsmatXnTzGGBLG9H5ySrw
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes devbox run executing generated scripts (and sourced init hooks) under sh—often dash on Debian/Ubuntu—by preferring bash so bash-specific init hooks (notably source) behave consistently with devbox shell.
Changes:
- Update
internal/nix.RunScriptto execute viabashwhen available (fallback tosh). - Refresh
internal/shellgencomments/docs to reflect the new runner behavior. - Add unit + integration tests reproducing and preventing the
source: not foundregression.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+47
to
+52
| func scriptRunnerPath() string { | ||
| if bashPath := cmdutil.GetPathOrDefault("bash", ""); bashPath != "" { | ||
| return bashPath | ||
| } | ||
| return cmdutil.GetPathOrDefault("sh", "/bin/sh") | ||
| } |
Comment on lines
+6
to
+29
| import ( | ||
| "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. | ||
| err := RunScript(t.TempDir(), "source /dev/null", map[string]string{}) | ||
| if err != nil { | ||
| t.Fatalf("RunScript should run bashisms under bash, got error: %v", err) | ||
| } |
The CI testscript sandbox runs devbox with a minimal PATH that excludes
/bin and /usr/bin, so exec.LookPath("bash") failed and scriptRunnerPath
fell back to /bin/sh (dash), reintroducing the "source: not found"
failure this change is meant to fix (and failing the new
init_hook_source testscript).
Check well-known absolute locations (/bin/bash, /usr/bin/bash,
/usr/local/bin/bash) before falling back to sh, so bash is used even
when it isn't on PATH. Also addresses Copilot review feedback: source a
temp file instead of /dev/null in the unit test.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WEsmatXnTzGGBLG9H5ySrw
Collaborator
Author
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #2607.
devbox runexecuted generated scripts — and the init hook theysource— withsh. On many systems (Debian/Ubuntu, and the CI containers used here)/bin/shis a POSIX shell such as dash. Init hooks that use bash features, most commonly thesourcebuiltin, work insidedevbox shell(which runs them under the user's interactive shell, usually bash) but fail underdevbox runwith:So the same
devbox.jsonbehaves differently betweendevbox shellanddevbox run, which is surprising and hard to debug.Change
nix.RunScriptnow prefers bash, falling back to POSIXsh(and finally a well-known absolute path) when bash isn't installed. Running scripts under bash matches the interactive shell and is a strict superset of the previous behavior for POSIX scripts.Scripts always use sh to runcomments ininternal/shellgento reflect the new behavior.The fallback keeps things working on the rare system without bash, so no POSIX script regresses.
How was it tested?
go build ./...,go vet, andgofmtare clean.nixpackage (internal/nix/run_test.go) that runs thesourcebuiltin throughRunScriptand asserts it succeeds — this fails when scripts run under dash and passes under bash.testscripts/run/init_hook_source.test.txt) reproducing the exact issue: aninit_hookthatsources a file, with a script that reads the sourced variable.dash -c 'source /dev/null'→source: not found(exit 127);bash -c 'source /dev/null'→ ok.cc @ashishkurian (issue reporter)
Community Contribution License
All community contributions in this pull request are licensed to the project
maintainers under the terms of the
Apache 2 License.
By creating this pull request, I represent that I have the right to license the
contributions to the project maintainers under the Apache 2 License as stated in
the
Community Contribution License.
Generated by Claude Code