[fish] Fix init hooks for fish (#1741)

## Summary

Fixes issue introduced in #1709

`devbox run` uses `sh` instead of native shell. So init hooks must
always be `sh` compatible for `run`. On the other hand, `devbox shell`
uses native shell so init hooks must be `fish` if the native shell is
fish.

This is ugly, but it's how it has always worked. 

#1709 introduced recursion
protection that would be `fish` or `sh` depending on native shell. This
worked fine for `devbox shell` but would break `devbox run`.

This change fixes that by creating 2 hooks files, one fish and one sh
and sourcing the correct one in shellrc, while still always using the
`sh` for run.

cc: @Lagoja 

## How was it tested?

```
devbox run echo 5
devbox shell
SHELL=fish devbox run echo 5
SHELL=fish devbox shell
```

Author: mikeland73

internal/devbox/devbox.go

@@ -204,7 +204,6 @@ func (d *Devbox) Shell(ctx context.Context) error {
 	}
 
 	opts := []ShellOption{
-		WithHooksFilePath(shellgen.ScriptPath(d.ProjectDir(), shellgen.HooksFilename)),
 		WithHistoryFile(filepath.Join(d.projectDir, shellHistoryFile)),
 		WithProjectDir(d.projectDir),
 		WithEnvVariables(envs),

internal/devbox/shell.go

@@ -17,6 +17,7 @@ import (
 
 	"github.com/alessio/shellescape"
 	"github.com/pkg/errors"
+	"go.jetpack.io/devbox/internal/shellgen"
 	"go.jetpack.io/devbox/internal/telemetry"
 
 	"go.jetpack.io/devbox/internal/debug"
@@ -59,8 +60,7 @@ type DevboxShell struct {
 	env             map[string]string
 	userShellrcPath string
 
-	hooksFilePath string
-	historyFile   string
+	historyFile string
 
 	// shellStartTime is the unix timestamp for when the command was invoked
 	shellStartTime time.Time
@@ -183,12 +183,6 @@ func WithHistoryFile(historyFile string) ShellOption {
 	}
 }
 
-func WithHooksFilePath(hooksFilePath string) ShellOption {
-	return func(s *DevboxShell) {
-		s.hooksFilePath = hooksFilePath
-	}
-}
-
 // TODO: Consider removing this once plugins add env vars directly to binaries via wrapper scripts.
 func WithEnvVariables(envVariables map[string]string) ShellOption {
 	return func(s *DevboxShell) {
@@ -317,8 +311,10 @@ func (s *DevboxShell) writeDevboxShellrc() (path string, err error) {
 	}()
 
 	tmpl := shellrcTmpl
+	hooksFilePath := shellgen.ScriptPath(s.projectDir, shellgen.HooksFilename)
 	if s.name == shFish {
 		tmpl = fishrcTmpl
+		hooksFilePath = shellgen.ScriptPath(s.projectDir, shellgen.HooksFishFilename)
 	}
 
 	err = tmpl.Execute(shellrcf, struct {
@@ -337,7 +333,7 @@ func (s *DevboxShell) writeDevboxShellrc() (path string, err error) {
 		ProjectDir:         s.projectDir,
 		OriginalInit:       string(bytes.TrimSpace(userShellrc)),
 		OriginalInitPath:   s.userShellrcPath,
-		HooksFilePath:      s.hooksFilePath,
+		HooksFilePath:      hooksFilePath,
 		ShellStartTime:     telemetry.FormatShellStart(s.shellStartTime),
 		HistoryFile:        strings.TrimSpace(s.historyFile),
 		ExportEnv:          exportify(s.env),

internal/devbox/shell_test.go

@@ -67,9 +67,8 @@ func testWriteDevboxShellrc(t *testing.T, testdirs []string) {
 			s := &DevboxShell{
 				devbox:          &Devbox{projectDir: projectDir},
 				env:             test.env,
-				projectDir:      "path/to/projectDir",
+				projectDir:      "/path/to/projectDir",
 				userShellrcPath: test.shellrcPath,
-				hooksFilePath:   test.hooksFilePath,
 			}
 			gotPath, err := s.writeDevboxShellrc()
 			if err != nil {

internal/devbox/testdata/shellrc/basic/shellrc.golden

@@ -15,7 +15,7 @@ export PS1="(devbox) $PS1"
 
 # Run plugin and user init hooks from the devbox.json directory.
 working_dir="$(pwd)"
-cd "path/to/projectDir" || exit
+cd "/path/to/projectDir" || exit
 
 # Source the hooks file, which contains the project's init hooks and plugin hooks.
 . /path/to/projectDir/.devbox/gen/scripts/.hooks.sh

internal/devbox/testdata/shellrc/noshellrc/shellrc.golden

@@ -9,7 +9,7 @@ export PS1="(devbox) $PS1"
 
 # Run plugin and user init hooks from the devbox.json directory.
 working_dir="$(pwd)"
-cd "path/to/projectDir" || exit
+cd "/path/to/projectDir" || exit
 
 # Source the hooks file, which contains the project's init hooks and plugin hooks.
 . /path/to/projectDir/.devbox/gen/scripts/.hooks.sh

internal/shellgen/scripts.go

@@ -21,11 +21,18 @@ import (
 //go:embed tmpl/init-hook.tmpl
 var initHookTmpl string
 
+//go:embed tmpl/init-hook-fish.tmpl
+var initHookFishTmpl string
+
 const scriptsDir = ".devbox/gen/scripts"
 
 // HooksFilename is the name of the file that contains the project's init-hooks and plugin hooks
 const HooksFilename = ".hooks"
 
+// This is only used in shellrc_fish.tmpl. A bit of a hack, because scripts use
+// sh instead of fish.
+const HooksFishFilename = ".hooks.fish"
+
 type devboxer interface {
 	Config() *devconfig.Config
 	Lockfile() *lock.File
@@ -62,11 +69,16 @@ 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 = writeInitHookFile(devbox, hooks)
+	err = writeInitHookFile(devbox, hooks, initHookTmpl, HooksFilename)
 	if err != nil {
 		return errors.WithStack(err)
 	}
 	written[HooksFilename] = struct{}{}
+	err = writeInitHookFile(devbox, hooks, initHookFishTmpl, HooksFishFilename)
+	if err != nil {
+		return errors.WithStack(err)
+	}
+	written[HooksFishFilename] = struct{}{}
 
 	// Write scripts to files.
 	for name, body := range devbox.Config().Scripts() {
@@ -91,25 +103,21 @@ func WriteScriptsToFiles(devbox devboxer) error {
 	return nil
 }
 
-func writeInitHookFile(devbox devboxer, body string) (err error) {
-	script, err := createScriptFile(devbox, HooksFilename)
+func writeInitHookFile(devbox devboxer, body, tmpl, filename string) (err error) {
+	script, err := createScriptFile(devbox, filename)
 	if err != nil {
 		return errors.WithStack(err)
 	}
 	defer script.Close() // best effort: close file
 
-	t, err := template.New("init-hook-template").Parse(initHookTmpl)
+	t, err := template.New(filename).Parse(tmpl)
 	if err != nil {
 		return errors.WithStack(err)
 	}
 
 	return t.Execute(script, map[string]any{
 		"Body":         body,
 		"InitHookHash": "__DEVBOX_INIT_HOOK_" + devbox.ProjectDirHash(),
-		// TODO put IsFish() in common place so we can call here and in devbox package
-		// without adding more stuff to interface
-		"IsFish": filepath.Base(os.Getenv("SHELL")) == "fish" ||
-			os.Getenv("FISH_VERSION") != "",
 	})
 }
 

internal/shellgen/tmpl/init-hook-fish.tmpl

@@ -0,0 +1,12 @@
+{{/* Fish version */ -}}
+{{/* if hash is set, we're in recursive loop, just exit */ -}}
+
+if set -q {{ .InitHookHash }}; then
+    return
+end
+
+export {{ .InitHookHash }}=true
+
+{{ .Body }}
+
+set -e {{ .InitHookHash }}

internal/shellgen/tmpl/init-hook.tmpl

@@ -1,22 +1,12 @@
 
 {{/* if hash is set, we're in recursive loop, just exit */ -}}
 
-{{ if .IsFish }}
-if set -q {{ .InitHookHash }}; then
-    return
-end
-{{ else }}
 if [ -n "${{ .InitHookHash }}" ]; then
     return
 fi
-{{ end }}
 
 export {{ .InitHookHash }}=true
 
 {{ .Body }}
 
-{{ if .IsFish }}
-set -e {{ .InitHookHash }}
-{{ else }}
 unset {{ .InitHookHash }}
-{{ end }}