shell,nix: set original PATH at start of init file (#57)

This fixes a bug that causes the devbox shell's init to fail when the
user's rcfile tries to call a command outside of devbox. For example,
the following line in a ~/.bashrc will trigger an error if the devbox
doesn't have Go installed:

	export PATH="$(go env GOPATH):$PATH"

This is because we're adding the ORIGINAL_PATH at the end of their shell
init script, which means that anything before that only see Nix
packages.

Fix this by adding a pre-init hook that lets us restore the PATH before
the user's rcfile, while still forcing Nix packages to come first in
the post-init hook.

---

In the process of testing and debugging this PR, I added a whole bunch
of debug logging. It should probably be a separate commit, but I'm
hesitant to touch anything now that things are working.

boxcli/midcobra/debug.go

@@ -4,11 +4,12 @@
 package midcobra
 
 import (
-	"log"
 	"os"
+	"strconv"
 
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
+	"go.jetpack.io/devbox/debug"
 )
 
 type DebugMiddleware struct {
@@ -27,17 +28,25 @@ func (d *DebugMiddleware) AttachToFlag(flags *pflag.FlagSet, flagName string) {
 	d.flag.Hidden = true
 }
 
-func (d *DebugMiddleware) preRun(cmd *cobra.Command, args []string) {}
+func (d *DebugMiddleware) preRun(cmd *cobra.Command, args []string) {
+	if d == nil {
+		return
+	}
 
-func (d *DebugMiddleware) postRun(cmd *cobra.Command, args []string, runErr error) {
-	if runErr != nil && d.Debug() {
-		log.Printf("Error: %+v\n", runErr)
+	strVal := ""
+	if d.flag.Changed {
+		strVal = d.flag.Value.String()
+	} else {
+		strVal = os.Getenv("DEVBOX_DEBUG")
+	}
+	if enabled, _ := strconv.ParseBool(strVal); enabled {
+		debug.Enable()
 	}
 }
 
-func (d *DebugMiddleware) Debug() bool {
-	if d != nil && d.flag.Changed {
-		return d.flag.Value.String() == "true"
+func (d *DebugMiddleware) postRun(cmd *cobra.Command, args []string, runErr error) {
+	if runErr == nil {
+		return
 	}
-	return os.Getenv("DEBUG") != ""
+	debug.Log("Error: %+v\n", runErr)
 }

boxcli/root.go

@@ -6,13 +6,13 @@ package boxcli
 import (
 	"context"
 	"errors"
-	"fmt"
 	"os"
 	"os/exec"
 
 	"github.com/spf13/cobra"
 	"go.jetpack.io/devbox/boxcli/midcobra"
 	"go.jetpack.io/devbox/build"
+	"go.jetpack.io/devbox/debug"
 )
 
 var debugMiddleware *midcobra.DebugMiddleware = &midcobra.DebugMiddleware{}
@@ -51,15 +51,7 @@ func RootCmd() *cobra.Command {
 }
 
 func Execute(ctx context.Context, args []string) int {
-	defer func() {
-		if r := recover(); r != nil {
-			if debugMiddleware.Debug() {
-				fmt.Printf("PANIC (DEBUG MODE ON): %+v\n", r)
-			} else {
-				fmt.Printf("Error: %s\n", r)
-			}
-		}
-	}()
+	defer debug.Recover()
 	exe := midcobra.New(RootCmd())
 	exe.AddMiddleware(midcobra.Telemetry(&midcobra.TelemetryOpts{
 		AppName:      "devbox",

debug/debug.go

@@ -0,0 +1,43 @@
+package debug
+
+import (
+	"fmt"
+	"log"
+	"os"
+	"strconv"
+)
+
+var enabled bool
+
+func init() {
+	enabled, _ = strconv.ParseBool(os.Getenv("DEBUG"))
+}
+
+func IsEnabled() bool { return enabled }
+
+func Enable() {
+	enabled = true
+	log.SetPrefix("[DEBUG] ")
+	log.SetFlags(log.Llongfile | log.Ldate | log.Ltime)
+	_ = log.Output(2, "Debug mode enabled.")
+}
+
+func Log(format string, v ...any) {
+	if !enabled {
+		return
+	}
+	_ = log.Output(2, fmt.Sprintf(format, v...))
+}
+
+func Recover() {
+	r := recover()
+	if r == nil {
+		return
+	}
+
+	if enabled {
+		log.Println("Allowing panic because debug mode is enabled.")
+		panic(r)
+	}
+	fmt.Println("Error:", r)
+}

generate.go

@@ -13,6 +13,7 @@ import (
 	"text/template"
 
 	"github.com/pkg/errors"
+	"go.jetpack.io/devbox/debug"
 	"go.jetpack.io/devbox/planner"
 )
 
@@ -67,4 +68,5 @@ func toJSON(a any) string {
 var templateFuncs = template.FuncMap{
 	"json":     toJSON,
 	"contains": strings.Contains,
+	"debug":    debug.IsEnabled,
 }

nix/nix.go

@@ -11,6 +11,7 @@ import (
 	"os/exec"
 	"strings"
 
+	"go.jetpack.io/devbox/debug"
 	"go.jetpack.io/devbox/shell"
 )
 
@@ -44,20 +45,32 @@ func Shell(path string) error {
 	//
 	// ORIGINAL_PATH is set by sh.StartCommand.
 	// PURE_NIX_PATH is set by the shell hook in shell.nix.tmpl.
-	_ = sh.SetInit(`
-# Update the $PATH so the user can keep using programs that live outside of Nix,
-# but prefer anything installed by Nix.
+	sh.PreInitHook = `
+# Update the $PATH so that the user's init script has access to all of their
+# non-devbox programs.
+export PATH="$PURE_NIX_PATH:$ORIGINAL_PATH"
+`
+	sh.PostInitHook = `
+# Update the $PATH again so that the Nix packages take priority over the
+# programs outside of devbox.
 export PATH="$PURE_NIX_PATH:$ORIGINAL_PATH"
 
 # Prepend to the prompt to make it clear we're in a devbox shell.
 export PS1="(devbox) $PS1"
-`)
+`
+
+	if debug.IsEnabled() {
+		sh.PostInitHook += `echo "POST-INIT PATH=$PATH"
+`
+	}
 
 	cmd := exec.Command("nix-shell", path)
 	cmd.Args = append(cmd.Args, "--pure", "--command", sh.ExecCommand())
 	cmd.Stdin = os.Stdin
 	cmd.Stdout = os.Stdout
 	cmd.Stderr = os.Stderr
+
+	debug.Log("Executing nix-shell command: %v", cmd.Args)
 	return cmd.Run()
 }
 
@@ -66,6 +79,8 @@ func runFallbackShell(path string) error {
 	cmd.Stdin = os.Stdin
 	cmd.Stdout = os.Stdout
 	cmd.Stderr = os.Stderr
+
+	debug.Log("Unrecognized user shell, falling back to: %v", cmd.Args)
 	return cmd.Run()
 }
 

shell/shell.go

@@ -12,6 +12,8 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
+
+	"go.jetpack.io/devbox/debug"
 )
 
 type name string
@@ -30,6 +32,22 @@ type Shell struct {
 	path           string
 	initFile       string
 	devboxInitFile string
+
+	// PreInitHook contains commands that will run before the user's init
+	// files at shell startup.
+	//
+	// The script's environment will contain an ORIGINAL_PATH environment
+	// variable, which will bet set to the PATH before the shell's init
+	// files have had a chance to modify it.
+	PreInitHook string
+
+	// PostInitHook contains commands that will run after the user's init
+	// files at shell startup.
+	//
+	// The script's environment will contain an ORIGINAL_PATH environment
+	// variable, which will bet set to the PATH before the shell's init
+	// files have had a chance to modify it.
+	PostInitHook string
 }
 
 // Detect attempts to determine the user's default shell.
@@ -67,6 +85,9 @@ func Detect() (*Shell, error) {
 	default:
 		sh.name = shUnknown
 	}
+	debug.Log("Detected shell: %s", sh.path)
+	debug.Log("Recognized shell as: %s", sh.path)
+	debug.Log("Looking for user's shell init file at: %s", sh.initFile)
 	return sh, nil
 }
 
@@ -80,33 +101,62 @@ func rcfilePath(basename string) string {
 	return filepath.Join(home, basename)
 }
 
-// SetInit configures the shell to run a script at startup. The script runs
-// after the user's usual init files. The script's environment will contain an
-// ORIGINAL_PATH environment variable, which will bet set to the PATH before
-// the user's init files have had a chance to modify it.
-func (s *Shell) SetInit(script string) error {
-	script = strings.TrimSpace(script)
-	if script == "" {
-		return nil
+func (s *Shell) buildInitFile() ([]byte, error) {
+	prehook := strings.TrimSpace(s.PreInitHook)
+	posthook := strings.TrimSpace(s.PostInitHook)
+	if prehook == "" && posthook == "" {
+		return nil, nil
+	}
+
+	buf := bytes.Buffer{}
+	if prehook != "" {
+		buf.WriteString(`
+# Begin Devbox Pre-init Hook
+
+`)
+		buf.WriteString(prehook)
+		buf.WriteString(`
+
+# End Devbox Pre-init Hook
+
+`)
 	}
 
-	initFile, _ := os.ReadFile(s.initFile)
+	initFile, err := os.ReadFile(s.initFile)
+	if err != nil {
+		return nil, err
+	}
 	initFile = bytes.TrimSpace(initFile)
 	if len(initFile) > 0 {
-		initFile = append(initFile, '\n', '\n')
+		buf.Write(initFile)
 	}
 
-	buf := bytes.NewBuffer(initFile)
-	buf.WriteString(`
+	if posthook != "" {
+		buf.WriteString(`
 
-# Begin Devbox Shell Hook
+# Begin Devbox Pre-init Hook
 
 `)
-	buf.WriteString(script)
-	buf.WriteString(`
+		buf.WriteString(posthook)
+		buf.WriteString(`
 
-# End Devbox Shell Hook
-`)
+# End Devbox Post-init Hook`)
+	}
+
+	b := buf.Bytes()
+	b = bytes.TrimSpace(b)
+	if len(b) == 0 {
+		return nil, nil
+	}
+	b = append(b, '\n')
+	return b, nil
+}
+
+func (s *Shell) writeHooks() error {
+	initContents, err := s.buildInitFile()
+	if err != nil {
+		return err
+	}
 
 	// We need a temp dir (as opposed to a temp file) because zsh uses
 	// ZDOTDIR to point to a new directory containing the .zshrc.
@@ -115,16 +165,20 @@ func (s *Shell) SetInit(script string) error {
 		return fmt.Errorf("create temp dir for shell init file: %v", err)
 	}
 	devboxInitFile := filepath.Join(tmp, filepath.Base(s.initFile))
-	if err := os.WriteFile(devboxInitFile, buf.Bytes(), 0600); err != nil {
+	if err := os.WriteFile(devboxInitFile, initContents, 0600); err != nil {
 		return fmt.Errorf("write to shell init file: %v", err)
 	}
 	s.devboxInitFile = devboxInitFile
+
+	debug.Log("Wrote devbox shell init file to: %s", s.devboxInitFile)
+	debug.Log("--- Begin Devbox Shell Init Contents ---\n%s--- End Devbox Shell Init Contents ---", initContents)
 	return nil
 }
 
 // ExecCommand is a command that replaces the current shell with s.
 func (s *Shell) ExecCommand() string {
-	if s.devboxInitFile == "" {
+	if err := s.writeHooks(); err != nil || s.devboxInitFile == "" {
+		debug.Log("Failed to write shell pre-init and post-init hooks: %v", err)
 		return "exec " + s.path
 	}
 

tmpl/shell.nix.tmpl

@@ -21,6 +21,11 @@ mkShell {
       # Make sure we include a basic path for the devbox shell. Otherwise it may
       # fail to start.
       export PATH=$PATH:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin
+
+      {{ if debug }}
+      echo "PURE_NIX_PATH=$PURE_NIX_PATH"
+      echo "PATH=$PATH"
+      {{- end }}
     '';
   packages = [
   {{- range .Packages}}