From bf62b1bf98c5ec081ec4b76a4d31743f6df437e4 Mon Sep 17 00:00:00 2001 From: Ujjwal Verma <16747982+r00tdaemon@users.noreply.github.com> Date: Tue, 30 Sep 2025 23:58:31 -0700 Subject: [PATCH] Added support for ZDOTDIR handling in Zsh shell initialization. Fixes #1715 #2297 This commit adds a patch to correctly handle ZDOTDIR env var for zsh. Instead of copying .z* files directly, we now source them inside devbox zsh config after setting ZDOTDIR to user's config dir. This makes sure any user config referencing ZDOTDIR doesn't break. --- internal/devbox/shell.go | 56 ++- internal/devbox/shell_test.go | 318 ++++++++++++++++++ internal/devbox/shellrc.tmpl | 9 + .../devbox/testdata/shellrc/zsh_zdotdir/env | 5 + .../testdata/shellrc/zsh_zdotdir/shellrc | 37 ++ .../shellrc/zsh_zdotdir/shellrc.golden | 37 ++ 6 files changed, 451 insertions(+), 11 deletions(-) create mode 100644 internal/devbox/testdata/shellrc/zsh_zdotdir/env create mode 100644 internal/devbox/testdata/shellrc/zsh_zdotdir/shellrc create mode 100644 internal/devbox/testdata/shellrc/zsh_zdotdir/shellrc.golden diff --git a/internal/devbox/shell.go b/internal/devbox/shell.go index 098a10593e5..34f81cc7c60 100644 --- a/internal/devbox/shell.go +++ b/internal/devbox/shell.go @@ -29,7 +29,7 @@ import ( //go:embed shellrc.tmpl var shellrcText string -var shellrcTmpl = template.Must(template.New("shellrc").Parse(shellrcText)) +var shellrcTmpl = template.Must(template.New("shellrc").Funcs(template.FuncMap{"dirPath": filepath.Dir}).Parse(shellrcText)) //go:embed shellrc_fish.tmpl var fishrcText string @@ -228,8 +228,8 @@ func (s *DevboxShell) Run() error { return errors.WithStack(err) } - // Link other files that affect the shell settings and environments. - s.linkShellStartupFiles(filepath.Dir(shellrc)) + // Setup other files that affect the shell settings and environments. + s.setupShellStartupFiles(filepath.Dir(shellrc)) extraEnv, extraArgs := s.shellRCOverrides(shellrc) env := s.env for k, v := range extraEnv { @@ -323,6 +323,7 @@ func (s *DevboxShell) writeDevboxShellrc() (path string, err error) { ShellStartTime string HistoryFile string ExportEnv string + ShellName string RefreshAliasName string RefreshCmd string @@ -335,6 +336,7 @@ func (s *DevboxShell) writeDevboxShellrc() (path string, err error) { ShellStartTime: telemetry.FormatShellStart(s.shellStartTime), HistoryFile: strings.TrimSpace(s.historyFile), ExportEnv: exportify(s.env), + ShellName: string(s.name), RefreshAliasName: s.devbox.refreshAliasName(), RefreshCmd: s.devbox.refreshCmd(), RefreshAliasEnvVar: s.devbox.refreshAliasEnvVar(), @@ -347,12 +349,13 @@ func (s *DevboxShell) writeDevboxShellrc() (path string, err error) { return path, nil } -// linkShellStartupFiles will link files used by the shell for initialization. -// We choose to link instead of copy so that changes made outside can be reflected -// within the devbox shell. +// setupShellStartupFiles creates initialization files for the shell by sourcing the user's originals. +// We do this instead of linking or copying, so that we can set correct ZDOTDIR when sourcing +// user's config files which may use the ZDOTDIR env var inside them. +// This also allows us to make sure any devbox config is run after correctly sourcing the user's config. // // We do not link the .{shell}rc files, since devbox modifies them. See writeDevboxShellrc -func (s *DevboxShell) linkShellStartupFiles(shellSettingsDir string) { +func (s *DevboxShell) setupShellStartupFiles(shellSettingsDir string) { // For now, we only need to do this for zsh shell if s.name == shZsh { // List of zsh startup files: https://zsh.sourceforge.io/Intro/intro_3.html @@ -375,10 +378,41 @@ func (s *DevboxShell) linkShellStartupFiles(shellSettingsDir string) { } fileNew := filepath.Join(shellSettingsDir, filename) - cmd := exec.Command("cp", fileOld, fileNew) - if err := cmd.Run(); err != nil { - // This is a best-effort operation. If there's an error then log it for visibility but continue. - slog.Error("error copying zsh setting file", "from", fileOld, "to", fileNew, "err", err) + + // Create template content that sources the original file + templateContent := `if [[ -f "{{.FileOld}}" ]]; then + local OLD_ZDOTDIR="$ZDOTDIR" + export ZDOTDIR="{{.ZDOTDIR}}" + . "{{.FileOld}}" + export ZDOTDIR="$OLD_ZDOTDIR" +fi` + + // Parse and execute the template + tmpl, err := template.New("shellrc").Parse(templateContent) + if err != nil { + slog.Error("error parsing template for zsh setting file", "filename", filename, "err", err) + continue + } + + // Create the new file with template content + file, err := os.Create(fileNew) + if err != nil { + slog.Error("error creating zsh setting file", "filename", filename, "err", err) + continue + } + defer file.Close() + + // Execute template with data + data := struct { + FileOld string + ZDOTDIR string + }{ + FileOld: fileOld, + ZDOTDIR: filepath.Dir(s.userShellrcPath), + } + + if err := tmpl.Execute(file, data); err != nil { + slog.Error("error executing template for zsh setting file", "filename", filename, "err", err) continue } } diff --git a/internal/devbox/shell_test.go b/internal/devbox/shell_test.go index fc5ddf4e7b3..aaaa1a15cdb 100644 --- a/internal/devbox/shell_test.go +++ b/internal/devbox/shell_test.go @@ -17,6 +17,7 @@ import ( "go.jetify.com/devbox/internal/devbox/devopt" "go.jetify.com/devbox/internal/envir" "go.jetify.com/devbox/internal/shellgen" + "go.jetify.com/devbox/internal/xdg" ) // updateFlag overwrites golden files with the new test results. @@ -72,6 +73,10 @@ func testWriteDevboxShellrc(t *testing.T, testdirs []string) { projectDir: "/path/to/projectDir", userShellrcPath: test.shellrcPath, } + // Set shell name based on test name for zsh tests + if strings.Contains(test.name, "zsh") { + s.name = shZsh + } gotPath, err := s.writeDevboxShellrc() if err != nil { t.Fatal("Got writeDevboxShellrc error:", err) @@ -165,3 +170,316 @@ func TestShellPath(t *testing.T) { }) } } + +func TestInitShellBinaryFields(t *testing.T) { + tests := []struct { + name string + path string + env map[string]string + expectedName name + expectedRcPath string + expectedRcPathBase string + }{ + { + name: "bash shell", + path: "/usr/bin/bash", + expectedName: shBash, + expectedRcPathBase: ".bashrc", + }, + { + name: "zsh shell without ZDOTDIR", + path: "/usr/bin/zsh", + expectedName: shZsh, + expectedRcPathBase: ".zshrc", + }, + { + name: "zsh shell with ZDOTDIR", + path: "/usr/bin/zsh", + env: map[string]string{ + "ZDOTDIR": "/custom/zsh/config", + }, + expectedName: shZsh, + expectedRcPath: "/custom/zsh/config/.zshrc", + }, + { + name: "ksh shell", + path: "/usr/bin/ksh", + expectedName: shKsh, + expectedRcPathBase: ".kshrc", + }, + { + name: "fish shell", + path: "/usr/bin/fish", + expectedName: shFish, + expectedRcPath: xdg.ConfigSubpath("fish/config.fish"), + }, + { + name: "dash shell", + path: "/usr/bin/dash", + expectedName: shPosix, + expectedRcPath: ".shinit", + }, + { + name: "unknown shell", + path: "/usr/bin/unknown", + expectedName: shUnknown, + expectedRcPathBase: "", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + // Set up environment variables + for k, v := range test.env { + t.Setenv(k, v) + } + + shell := initShellBinaryFields(test.path) + + if shell.name != test.expectedName { + t.Errorf("Expected shell name %v, got %v", test.expectedName, shell.name) + } + + if test.expectedRcPath != "" { + if shell.userShellrcPath != test.expectedRcPath { + t.Errorf("Expected rc path %s, got %s", test.expectedRcPath, shell.userShellrcPath) + } + } else if test.expectedRcPathBase != "" { + // For tests that expect a path relative to home directory, + // check that the path ends with the expected basename + expectedBasename := test.expectedRcPathBase + actualBasename := filepath.Base(shell.userShellrcPath) + if actualBasename != expectedBasename { + t.Errorf("Expected rc path basename %s, got %s (full path: %s)", expectedBasename, actualBasename, shell.userShellrcPath) + } + } + }) + } +} + +func TestShellRCOverrides(t *testing.T) { + tests := []struct { + name string + shellName name + shellrcPath string + expectedEnv map[string]string + expectedArgs []string + }{ + { + name: "bash shell", + shellName: shBash, + shellrcPath: "/tmp/devbox123/.bashrc", + expectedArgs: []string{"--rcfile", "/tmp/devbox123/.bashrc"}, + }, + { + name: "zsh shell", + shellName: shZsh, + shellrcPath: "/tmp/devbox123/.zshrc", + expectedEnv: map[string]string{"ZDOTDIR": "/tmp/devbox123"}, + }, + { + name: "ksh shell", + shellName: shKsh, + shellrcPath: "/tmp/devbox123/.kshrc", + expectedEnv: map[string]string{"ENV": "/tmp/devbox123/.kshrc"}, + }, + { + name: "posix shell", + shellName: shPosix, + shellrcPath: "/tmp/devbox123/.shinit", + expectedEnv: map[string]string{"ENV": "/tmp/devbox123/.shinit"}, + }, + { + name: "fish shell", + shellName: shFish, + shellrcPath: "/tmp/devbox123/config.fish", + expectedArgs: []string{"-C", ". /tmp/devbox123/config.fish"}, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + shell := &DevboxShell{name: test.shellName} + extraEnv, extraArgs := shell.shellRCOverrides(test.shellrcPath) + + if test.expectedEnv != nil { + if len(extraEnv) != len(test.expectedEnv) { + t.Errorf("Expected %d env vars, got %d", len(test.expectedEnv), len(extraEnv)) + } + for k, v := range test.expectedEnv { + if extraEnv[k] != v { + t.Errorf("Expected env var %s=%s, got %s", k, v, extraEnv[k]) + } + } + } else { + if len(extraEnv) != 0 { + t.Errorf("Expected no env vars, got %v", extraEnv) + } + } + + if test.expectedArgs != nil { + if len(extraArgs) != len(test.expectedArgs) { + t.Errorf("Expected %d args, got %d", len(test.expectedArgs), len(extraArgs)) + } + for i, arg := range test.expectedArgs { + if i >= len(extraArgs) || extraArgs[i] != arg { + t.Errorf("Expected arg %d to be %s, got %s", i, arg, extraArgs[i]) + } + } + } else { + if len(extraArgs) != 0 { + t.Errorf("Expected no args, got %v", extraArgs) + } + } + }) + } +} + +func TestSetupShellStartupFiles(t *testing.T) { + tmpDir := t.TempDir() + + // Create a mock zsh shell + shell := &DevboxShell{ + name: shZsh, + userShellrcPath: filepath.Join(tmpDir, ".zshrc"), + } + + // Create some test zsh startup files + startupFiles := []string{".zshenv", ".zprofile", ".zlogin", ".zlogout", ".zimrc"} + for _, filename := range startupFiles { + filePath := filepath.Join(tmpDir, filename) + err := os.WriteFile(filePath, []byte("# Test content for "+filename), 0644) + if err != nil { + t.Fatalf("Failed to create test file %s: %v", filename, err) + } + } + + // Create a temporary directory for shell settings + shellSettingsDir := t.TempDir() + + // Call setupShellStartupFiles + shell.setupShellStartupFiles(shellSettingsDir) + + // Check that all startup files were created in the shell settings directory + for _, filename := range startupFiles { + expectedPath := filepath.Join(shellSettingsDir, filename) + _, err := os.Stat(expectedPath) + if err != nil { + t.Errorf("Expected startup file %s to be created, but got error: %v", filename, err) + } + + // Check that the file contains the expected template content + content, err := os.ReadFile(expectedPath) + if err != nil { + t.Errorf("Failed to read created file %s: %v", filename, err) + continue + } + + contentStr := string(content) + expectedOldPath := filepath.Join(tmpDir, filename) + if !strings.Contains(contentStr, expectedOldPath) { + t.Errorf("Expected file %s to contain path %s, but content was: %s", filename, expectedOldPath, contentStr) + } + + if !strings.Contains(contentStr, "OLD_ZDOTDIR") { + t.Errorf("Expected file %s to contain ZDOTDIR handling, but content was: %s", filename, contentStr) + } + } +} +func TestWriteDevboxShellrcBash(t *testing.T) { + tmpDir := t.TempDir() + + // Create a test bash rc file + bashrcPath := filepath.Join(tmpDir, ".bashrc") + bashrcContent := "# Test bash configuration\nexport TEST_VAR=value" + err := os.WriteFile(bashrcPath, []byte(bashrcContent), 0644) + if err != nil { + t.Fatalf("Failed to create test .bashrc: %v", err) + } + + // Create a mock devbox + devbox := &Devbox{projectDir: "/test/project"} + + // Create a bash shell + shell := &DevboxShell{ + devbox: devbox, + name: shBash, + userShellrcPath: bashrcPath, + projectDir: "/test/project", + env: map[string]string{"TEST_ENV": "test_value"}, + } + + // Write the devbox shellrc + shellrcPath, err := shell.writeDevboxShellrc() + if err != nil { + t.Fatalf("Failed to write devbox shellrc: %v", err) + } + + // Read and verify the content + content, err := os.ReadFile(shellrcPath) + if err != nil { + t.Fatalf("Failed to read generated shellrc: %v", err) + } + + contentStr := string(content) + + // Check that it does NOT contain zsh-specific ZDOTDIR handling + if strings.Contains(contentStr, "OLD_ZDOTDIR") { + t.Error("Expected shellrc to NOT contain ZDOTDIR handling for bash") + } + + // Check that it sources the original .bashrc + if !strings.Contains(contentStr, bashrcPath) { + t.Error("Expected shellrc to source the original .bashrc file") + } +} + +func TestWriteDevboxShellrcWithZDOTDIR(t *testing.T) { + tmpDir := t.TempDir() + + // Set up ZDOTDIR environment + t.Setenv("ZDOTDIR", tmpDir) + + // Create a test zsh rc file in the custom ZDOTDIR + customZshrcPath := filepath.Join(tmpDir, ".zshrc") + zshrcContent := "# Custom zsh configuration\nexport CUSTOM_VAR=value" + err := os.WriteFile(customZshrcPath, []byte(zshrcContent), 0644) + if err != nil { + t.Fatalf("Failed to create test .zshrc: %v", err) + } + + // Create a mock devbox + devbox := &Devbox{projectDir: "/test/project"} + + // Create a zsh shell - this should pick up the ZDOTDIR + shell := initShellBinaryFields("/usr/bin/zsh") + shell.devbox = devbox + shell.projectDir = "/test/project" + + if shell.userShellrcPath != customZshrcPath { + t.Error("Expected shellrc path to respect ZDOTDIR") + } + + // Write the devbox shellrc + shellrcPath, err := shell.writeDevboxShellrc() + if err != nil { + t.Fatalf("Failed to write devbox shellrc: %v", err) + } + + // Read and verify the content + content, err := os.ReadFile(shellrcPath) + if err != nil { + t.Fatalf("Failed to read generated shellrc: %v", err) + } + + contentStr := string(content) + // Check that it contains zsh-specific ZDOTDIR handling + if !strings.Contains(contentStr, "OLD_ZDOTDIR") { + t.Error("Expected shellrc to contain ZDOTDIR handling for zsh") + } + + // Check that it sources the custom .zshrc + if !strings.Contains(contentStr, customZshrcPath) { + t.Error("Expected shellrc to source the custom .zshrc file") + } +} diff --git a/internal/devbox/shellrc.tmpl b/internal/devbox/shellrc.tmpl index 44b4780d19a..d24bce5aa5a 100644 --- a/internal/devbox/shellrc.tmpl +++ b/internal/devbox/shellrc.tmpl @@ -19,9 +19,18 @@ content readable. */ -}} {{- if .OriginalInitPath -}} +{{- if eq .ShellName "zsh" -}} if [ -f {{ .OriginalInitPath }} ]; then + local OLD_ZDOTDIR="$ZDOTDIR" + export ZDOTDIR="{{dirPath .OriginalInitPath}}" . "{{ .OriginalInitPath }}" + export ZDOTDIR="$OLD_ZDOTDIR" fi +{{ else -}} +if [ -f {{ .OriginalInitPath }} ]; then + . "{{ .OriginalInitPath }}" +fi +{{ end -}} {{ end -}} # Begin Devbox Post-init Hook diff --git a/internal/devbox/testdata/shellrc/zsh_zdotdir/env b/internal/devbox/testdata/shellrc/zsh_zdotdir/env new file mode 100644 index 00000000000..19f513577a5 --- /dev/null +++ b/internal/devbox/testdata/shellrc/zsh_zdotdir/env @@ -0,0 +1,5 @@ +simple=value +space=quote me +quote=they said, "lasers" +special=$`"\ +BASH_FUNC_echo_simple%%=() { echo "${simple}"; } diff --git a/internal/devbox/testdata/shellrc/zsh_zdotdir/shellrc b/internal/devbox/testdata/shellrc/zsh_zdotdir/shellrc new file mode 100644 index 00000000000..85834d546fd --- /dev/null +++ b/internal/devbox/testdata/shellrc/zsh_zdotdir/shellrc @@ -0,0 +1,37 @@ +# Set up the prompt + +autoload -Uz promptinit +promptinit +#prompt adam1 + +setopt histignorealldups sharehistory + +# Use emacs keybindings even if our EDITOR is set to vi +bindkey -e + +# Keep 1000 lines of history within the shell and save it to ~/.zsh_history: +HISTSIZE=1000 +SAVEHIST=1000 +HISTFILE=~/.zsh_history + +# Use modern completion system +autoload -Uz compinit +compinit + +zstyle ':completion:*' auto-description 'specify: %d' +zstyle ':completion:*' completer _expand _complete _correct _approximate +zstyle ':completion:*' format 'Completing %d' +zstyle ':completion:*' group-name '' +zstyle ':completion:*' menu select=2 +eval "$(dircolors -b)" +zstyle ':completion:*:default' list-colors ${(s.:.)LS_COLORS} +zstyle ':completion:*' list-colors '' +zstyle ':completion:*' list-prompt %SAt %p: Hit TAB for more, or the character to insert%s +zstyle ':completion:*' matcher-list '' 'm:{a-z}={A-Z}' 'm:{a-zA-Z}={A-Za-z}' 'r:|[._-]=* r:|=* l:|=*' +zstyle ':completion:*' menu select=long +zstyle ':completion:*' select-prompt %SScrolling active: current selection at %p%s +zstyle ':completion:*' use-compctl false +zstyle ':completion:*' verbose true + +zstyle ':completion:*:*:kill:*:processes' list-colors '=(#b) #([0-9]#)*=0=01;31' +zstyle ':completion:*:kill:*' command 'ps -u $USER -o pid,%cpu,tty,cputime,cmd' diff --git a/internal/devbox/testdata/shellrc/zsh_zdotdir/shellrc.golden b/internal/devbox/testdata/shellrc/zsh_zdotdir/shellrc.golden new file mode 100644 index 00000000000..f3fa4cb16c2 --- /dev/null +++ b/internal/devbox/testdata/shellrc/zsh_zdotdir/shellrc.golden @@ -0,0 +1,37 @@ +if [ -f testdata/shellrc/zsh_zdotdir/shellrc ]; then + local OLD_ZDOTDIR="$ZDOTDIR" + export ZDOTDIR="testdata/shellrc/zsh_zdotdir" + . "testdata/shellrc/zsh_zdotdir/shellrc" + export ZDOTDIR="$OLD_ZDOTDIR" +fi +# Begin Devbox Post-init Hook + +echo_simple () { echo "${simple}"; } +export -f echo_simple +export quote="they said, \"lasers\""; +export simple="value"; +export space="quote me"; +export special="\$\`\"\\"; + +# If the user hasn't specified they want to handle the prompt themselves, +# prepend to the prompt to make it clear we're in a devbox shell. +if [ -z "$DEVBOX_NO_PROMPT" ]; then + export PS1="(devbox) $PS1" +fi + +# End Devbox Post-init Hook + +# Run plugin and user init hooks from the devbox.json directory. +working_dir="$(pwd)" +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" + +cd "$working_dir" || exit + +# Add refresh alias (only if it doesn't already exist) +if ! type refresh >/dev/null 2>&1; then + export DEVBOX_REFRESH_ALIAS_11c3c7a2e9a24e16e714a53a46351e31be8beac32de3f19854be1ef14e556903='eval "$(devbox shellenv --preserve-path-stack -c "/path/to/projectDir")" && hash -r' + alias refresh='eval "$(devbox shellenv --preserve-path-stack -c "/path/to/projectDir")" && hash -r' +fi