Added support for ZDOTDIR handling in Zsh shell initialization. (#2712)

## Summary

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.

## How was it tested?

Unit tests and manual testing

## Community Contribution License

All community contributions in this pull request are licensed to the
project
maintainers under the terms of the
[Apache 2 License](https://www.apache.org/licenses/LICENSE-2.0).

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](https://github.com/jetify-com/opensource/blob/main/CONTRIBUTING.md#community-contribution-license).

---------

Co-authored-by: Savil Srivastava <676452+savil@users.noreply.github.com>

Author: r00tdaemon

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
@@ -364,21 +367,52 @@ func (s *DevboxShell) linkShellStartupFiles(shellSettingsDir string) {
 
 		for _, filename := range filenames {
 			// The userShellrcPath should be set to ZDOTDIR already.
-			fileOld := filepath.Join(filepath.Dir(s.userShellrcPath), filename)
-			_, err := os.Stat(fileOld)
+			userFile := filepath.Join(filepath.Dir(s.userShellrcPath), filename)
+			_, err := os.Stat(userFile)
 			if errors.Is(err, fs.ErrNotExist) {
 				// this file may not be relevant for the user's setup.
 				continue
 			}
 			if err != nil {
-				slog.Debug("os.Stat error for %s is %v", fileOld, err)
+				slog.Debug("os.Stat error for %s is %v", userFile, err)
 			}
 
 			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 "{{.UserFile}}" ]]; then
+    local DEVBOX_ZDOTDIR="$ZDOTDIR"
+    export ZDOTDIR="{{.ZDOTDIR}}"
+    . "{{.UserFile}}"
+    export ZDOTDIR="$DEVBOX_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 {
+				UserFile string
+				ZDOTDIR  string
+			}{
+				UserFile: userFile,
+				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
 			}
 		}

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,239 @@ 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 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), 0o644)
+		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, "DEVBOX_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), 0o644)
+	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, "DEVBOX_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), 0o644)
+	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, "DEVBOX_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")
+	}
+}

internal/devbox/shellrc.tmpl

@@ -19,9 +19,18 @@ content readable.
 */ -}}
 
 {{- if .OriginalInitPath -}}
+{{- if eq .ShellName "zsh" -}}
 if [ -f {{ .OriginalInitPath }} ]; then
+  local DEVBOX_ZDOTDIR="$ZDOTDIR"
+  export ZDOTDIR="{{dirPath .OriginalInitPath}}"
   . "{{ .OriginalInitPath }}"
+  export ZDOTDIR="$DEVBOX_ZDOTDIR"
 fi
+{{ else -}}
+if [ -f {{ .OriginalInitPath }} ]; then
+  . "{{ .OriginalInitPath }}"
+fi
+{{ end -}}
 {{ end -}}
 
 # Begin Devbox Post-init Hook

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}"; }