diff --git a/internal/impl/devbox.go b/internal/impl/devbox.go index af30b6e3304..4775fc0bf4b 100644 --- a/internal/impl/devbox.go +++ b/internal/impl/devbox.go @@ -210,17 +210,6 @@ func (d *Devbox) Shell(ctx context.Context) error { return shell.Run() } -// IsUserShellFish returns true if the user's shell is fish. -// This wrapper function over DevboxShell enables querying from other packages that -// make a devboxer interface. -func (d *Devbox) IsUserShellFish() (bool, error) { - sh, err := NewDevboxShell(d) - if err != nil { - return false, err - } - return sh.IsFish(), nil -} - func (d *Devbox) RunScript(ctx context.Context, cmdName string, cmdArgs []string) error { ctx, task := trace.NewTask(ctx, "devboxRun") defer task.End() diff --git a/internal/impl/shell.go b/internal/impl/shell.go index 547d45e7d1a..40fb73f410d 100644 --- a/internal/impl/shell.go +++ b/internal/impl/shell.go @@ -269,12 +269,6 @@ func (s *DevboxShell) Run() error { return errors.WithStack(err) } -// IsFish returns whether this DevboxShell wraps a fish shell. Fish shells are non-posix compatible, -// and so sometimes we may need to switch logic based on this function's result. -func (s *DevboxShell) IsFish() bool { - return s.name == shFish -} - func (s *DevboxShell) shellRCOverrides(shellrc string) (extraEnv map[string]string, extraArgs []string) { // Shells have different ways of overriding the shellrc, so we need to // look at the name to know which env vars or args to set when launching the shell. @@ -325,7 +319,7 @@ func (s *DevboxShell) writeDevboxShellrc() (path string, err error) { }() tmpl := shellrcTmpl - if s.IsFish() { + if s.name == shFish { tmpl = fishrcTmpl } diff --git a/internal/shellgen/scripts.go b/internal/shellgen/scripts.go index 0dae00b9a5e..698190d4ea7 100644 --- a/internal/shellgen/scripts.go +++ b/internal/shellgen/scripts.go @@ -25,7 +25,6 @@ type devboxer interface { Lockfile() *lock.File AllInstallablePackages() ([]*devpkg.Package, error) InstallablePackages() []*devpkg.Package - IsUserShellFish() (bool, error) PluginManager() *plugin.Manager ProjectDir() string } @@ -53,7 +52,7 @@ func WriteScriptsToFiles(devbox devboxer) error { } hooks := strings.Join(append(pluginHooks, devbox.Config().InitHook().String()), "\n\n") // always write it, even if there are no hooks, because scripts will source it. - err = WriteScriptFile(devbox, HooksFilename, hooks) + err = writeHookFile(devbox, hooks) if err != nil { return errors.WithStack(err) } @@ -82,39 +81,53 @@ func WriteScriptsToFiles(devbox devboxer) error { return nil } -func WriteScriptFile(devbox devboxer, name string, body string) (err error) { - script, err := os.Create(ScriptPath(devbox.ProjectDir(), name)) +func writeHookFile(devbox devboxer, body string) (err error) { + script, err := createScriptFile(devbox, HooksFilename) if err != nil { return errors.WithStack(err) } - defer func() { - cerr := script.Close() - if err == nil { - err = cerr - } - }() - err = script.Chmod(0755) + defer script.Close() // best effort: close file + + _, err = script.WriteString(body) + return errors.WithStack(err) +} + +func WriteScriptFile(devbox devboxer, name string, body string) (err error) { + script, err := createScriptFile(devbox, name) if err != nil { return errors.WithStack(err) } + defer script.Close() // best effort: close file if featureflag.ScriptExitOnError.Enabled() { - // Fish cannot run scripts with `set -e`. - // NOTE: Devbox scripts will run using `sh` for consistency. However, - // init_hooks in a fish shell will run using `fish` shell, and need this - // check. - isFish, err := devbox.IsUserShellFish() - if err != nil { - return errors.WithStack(err) - } - if !isFish { - body = fmt.Sprintf("set -e\n\n%s", body) - } + // NOTE: Devbox scripts will run using `sh` for consistency. + // However, we need to disable this for `fish` shell if/when we allow this for init_hooks, + // since init_hooks run in the host shell, and not `sh`. + body = fmt.Sprintf("set -e\n\n%s", body) } _, err = script.WriteString(body) return errors.WithStack(err) } +func createScriptFile(devbox devboxer, name string) (script *os.File, err error) { + script, err = os.Create(ScriptPath(devbox.ProjectDir(), name)) + if err != nil { + return nil, errors.WithStack(err) + } + defer func() { + // best effort: close file if there was some subsequent error + if err != nil { + _ = script.Close() + } + }() + + err = script.Chmod(0755) + if err != nil { + return nil, errors.WithStack(err) + } + return script, nil +} + func ScriptPath(projectDir, scriptName string) string { return filepath.Join(projectDir, scriptsDir, scriptName+".sh") }