move to envpath.Stack struct

Author: savil

internal/impl/devbox.go

@@ -20,6 +20,7 @@ import (
 	"github.com/pkg/errors"
 	"github.com/samber/lo"
 	"go.jetpack.io/devbox/internal/devpkg"
+	"go.jetpack.io/devbox/internal/impl/envpath"
 	"go.jetpack.io/devbox/internal/impl/generate"
 	"go.jetpack.io/devbox/internal/searcher"
 	"go.jetpack.io/devbox/internal/shellgen"
@@ -784,16 +785,10 @@ func (d *Devbox) computeNixEnv(ctx context.Context, usePrintDevEnvCache bool) (m
 		}
 	}
 
-	currentEnvPath := env["PATH"]
-	debug.Log("current environment PATH is: %s", currentEnvPath)
+	debug.Log("current environment PATH is: %s", env["PATH"])
 
-	pathStackEnv, ok := env["PATH_STACK"]
-	if !ok {
-		// if path stack is empty, then set the first element
-		pathStackEnv = d.ogPathKey()
-		env[d.ogPathKey()] = currentEnvPath
-	}
-	debug.Log("path stack is: %s", pathStackEnv)
+	pathStack := envpath.NewStack(env)
+	debug.Log("path stack is: %s", pathStack)
 
 	vaf, err := d.nix.PrintDevEnv(ctx, &nix.PrintDevEnvArgs{
 		FlakesFilePath:       d.nixFlakesFilePath(),
@@ -853,13 +848,13 @@ func (d *Devbox) computeNixEnv(ctx context.Context, usePrintDevEnvCache bool) (m
 
 	addEnvIfNotPreviouslySetByDevbox(env, pluginEnv)
 
-	env["PATH"] = JoinPathLists(
+	env["PATH"] = envpath.JoinPathLists(
 		nix.ProfileBinPath(d.projectDir),
 		env["PATH"],
 	)
 
 	if !d.OmitBinWrappersFromPath {
-		env["PATH"] = JoinPathLists(
+		env["PATH"] = envpath.JoinPathLists(
 			filepath.Join(d.projectDir, plugin.WrapperBinPath),
 			env["PATH"],
 		)
@@ -899,32 +894,15 @@ func (d *Devbox) computeNixEnv(ctx context.Context, usePrintDevEnvCache bool) (m
 	})
 	debug.Log("PATH after filtering with buildInputs (%v) is: %s", buildInputs, nixEnvPath)
 
-	// The PathStack enables:
-	// 1. Tracking which sub-paths in PATH come from which devbox-project
-	// 2. The priority order of these sub-paths: devbox-projects encountered later get higher priority.
-	pathStack := strings.Split(pathStackEnv, ":")
-	nixEnvPathKey := "DEVBOX_NIX_ENV_PATH_" + d.projectDirHash()
-	if !lo.Contains(pathStack, nixEnvPathKey) {
-		// if pathStack does not contain this project's nixEnvPath already,
-		// then we add it
-		pathStack = append(pathStack, nixEnvPathKey)
-	}
-	// Store the nixEnvPath for this devbox-project. This lets us replace this sub-path in the eventual PATH,
-	// without affecting the sub-paths from other devbox-projects.
-	env[nixEnvPathKey] = nixEnvPath
-	env["PATH_STACK"] = strings.Join(pathStack, ":")
-
-	// Look up the path-list for each path-stack element, and join them together to get the final PATH.
-	pathLists := lo.Map(pathStack, func(part string, idx int) string { return env[part] })
-	env["PATH"] = JoinPathLists(lo.Reverse(pathLists)...) // reverse to give priority to the most recent project
+	env = pathStack.AddToEnv(env, d.projectDirHash(), nixEnvPath)
 
 	debug.Log("computed environment PATH is: %s", env["PATH"])
 
 	d.setCommonHelperEnvVars(env)
 
 	if !d.pure {
 		// preserve the original XDG_DATA_DIRS by prepending to it
-		env["XDG_DATA_DIRS"] = JoinPathLists(env["XDG_DATA_DIRS"], os.Getenv("XDG_DATA_DIRS"))
+		env["XDG_DATA_DIRS"] = envpath.JoinPathLists(env["XDG_DATA_DIRS"], os.Getenv("XDG_DATA_DIRS"))
 	}
 
 	for k, v := range d.env {
@@ -959,10 +937,6 @@ func (d *Devbox) nixEnv(ctx context.Context) (map[string]string, error) {
 	return d.computeNixEnv(ctx, usePrintDevEnvCache)
 }
 
-func (d *Devbox) ogPathKey() string {
-	return "DEVBOX_OG_PATH_" + d.projectDirHash()
-}
-
 func (d *Devbox) nixPrintDevEnvCachePath() string {
 	return filepath.Join(d.projectDir, ".devbox/.nix-print-dev-env-cache")
 }
@@ -1137,8 +1111,8 @@ var ignoreDevEnvVar = map[string]bool{
 // common setups (e.g. gradio, rust)
 func (d *Devbox) setCommonHelperEnvVars(env map[string]string) {
 	profileLibDir := filepath.Join(d.projectDir, nix.ProfilePath, "lib")
-	env["LD_LIBRARY_PATH"] = JoinPathLists(profileLibDir, env["LD_LIBRARY_PATH"])
-	env["LIBRARY_PATH"] = JoinPathLists(profileLibDir, env["LIBRARY_PATH"])
+	env["LD_LIBRARY_PATH"] = envpath.JoinPathLists(profileLibDir, env["LD_LIBRARY_PATH"])
+	env["LIBRARY_PATH"] = envpath.JoinPathLists(profileLibDir, env["LIBRARY_PATH"])
 }
 
 // NixBins returns the paths to all the nix binaries that are installed by
@@ -1214,7 +1188,7 @@ func (d *Devbox) parseEnvAndExcludeSpecialCases(currentEnv []string) (map[string
 			return nil, err
 		}
 		includedInPath = append(includedInPath, dotdevboxBinPath(d))
-		env["PATH"] = JoinPathLists(includedInPath...)
+		env["PATH"] = envpath.JoinPathLists(includedInPath...)
 	}
 	return env, nil
 }

internal/impl/devbox_test.go

@@ -14,6 +14,7 @@ import (
 	"github.com/bmatcuk/doublestar/v4"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"go.jetpack.io/devbox/internal/impl/envpath"
 
 	"go.jetpack.io/devbox/internal/devconfig"
 	"go.jetpack.io/devbox/internal/envir"
@@ -85,11 +86,9 @@ func TestComputeNixPathIsIdempotent(t *testing.T) {
 	assert.NotEmpty(t, path, "path should not be nil")
 
 	t.Setenv("PATH", path)
-	t.Setenv(
-		"DEVBOX_OG_PATH_"+devbox.projectDirHash(),
-		env["DEVBOX_OG_PATH_"+devbox.projectDirHash()],
-	)
-	t.Setenv("PATH_STACK", env["PATH_STACK"])
+	t.Setenv(envpath.InitPathEnv, env[envpath.InitPathEnv])
+	t.Setenv(envpath.PathStackEnv, env[envpath.PathStackEnv])
+	t.Setenv(envpath.Key(devbox.projectDirHash()), env[envpath.Key(devbox.projectDirHash())])
 
 	env, err = devbox.computeNixEnv(ctx, false /*use cache*/)
 	require.NoError(t, err, "computeNixEnv should not fail")
@@ -109,11 +108,9 @@ func TestComputeNixPathWhenRemoving(t *testing.T) {
 	assert.Contains(t, path, "/tmp/my/path", "path should contain /tmp/my/path")
 
 	t.Setenv("PATH", path)
-	t.Setenv(
-		"DEVBOX_OG_PATH_"+devbox.projectDirHash(),
-		env["DEVBOX_OG_PATH_"+devbox.projectDirHash()],
-	)
-	t.Setenv("PATH_STACK", env["PATH_STACK"])
+	t.Setenv(envpath.InitPathEnv, env[envpath.InitPathEnv])
+	t.Setenv(envpath.PathStackEnv, env[envpath.PathStackEnv])
+	t.Setenv(envpath.Key(devbox.projectDirHash()), env[envpath.Key(devbox.projectDirHash())])
 
 	devbox.nix.(*testNix).path = ""
 	env, err = devbox.computeNixEnv(ctx, false /*use cache*/)

internal/impl/envpath/envpath.go

@@ -0,0 +1,37 @@
+package envpath
+
+import (
+	"path/filepath"
+	"strings"
+)
+
+// JoinPathLists joins and cleans PATH-style strings of
+// [os.ListSeparator] delimited paths. To clean a path list, it splits it and
+// does the following for each element:
+//
+//  1. Applies [filepath.Clean].
+//  2. Removes the path if it's relative (must begin with '/' and not be '.').
+//  3. Removes the path if it's a duplicate.
+func JoinPathLists(pathLists ...string) string {
+	if len(pathLists) == 0 {
+		return ""
+	}
+
+	seen := make(map[string]bool)
+	var cleaned []string
+	for _, path := range pathLists {
+		for _, path := range filepath.SplitList(path) {
+			path = filepath.Clean(path)
+			if path == "." || path[0] != '/' {
+				// Remove empty paths and don't allow relative
+				// paths for security reasons.
+				continue
+			}
+			if !seen[path] {
+				cleaned = append(cleaned, path)
+			}
+			seen[path] = true
+		}
+	}
+	return strings.Join(cleaned, string(filepath.ListSeparator))
+}

internal/impl/envpath/pathstack.go

@@ -0,0 +1,86 @@
+package envpath
+
+import (
+	"strings"
+
+	"github.com/samber/lo"
+	"golang.org/x/exp/slices"
+)
+
+const (
+	PathStackEnv = "DEVBOX_PATH_STACK"
+
+	// InitPathEnv stores the path prior to any devbox shellenv modifying the environment
+	InitPathEnv = "DEVBOX_INIT_PATH"
+)
+
+// Stack has the following design:
+// 1. The PathStack enables tracking which sub-paths in PATH come from which devbox-project
+// 2. What it stores: The PathStack is an ordered-list of nixEnvPathKeys
+// 3. Each nixEnvPathKey is set as an env-var with the value of the nixEnvPath for that devbox-project.
+// 4. The final PATH is reconstructed by concatenating the env-var values of each nixEnvPathKey env-var.
+// 5. The Stack is stored in its own env-var shared by all devbox-projects in this shell.
+type Stack struct {
+
+	// keys holds the stack elements.
+	// Earlier (lower index number) keys get higher priority.
+	// This keeps the string representation of the Stack aligned with the PATH value.
+	keys []string
+}
+
+func NewStack(env map[string]string) *Stack {
+	stackEnv, ok := env[PathStackEnv]
+	if !ok {
+		// if path stack is empty, then push the current PATH, which is the
+		// external environment prior to any devbox-shellenv being applied to it.
+		stackEnv = InitPathEnv
+		env[InitPathEnv] = env["PATH"]
+	}
+	return &Stack{
+		keys: strings.Split(stackEnv, ":"),
+	}
+}
+
+// String is the value of the Stack stored in its env-var.
+func (s *Stack) String() string {
+	return strings.Join(s.keys, ":")
+}
+
+// Key is the element stored in the Stack for a devbox-project. It represents
+// a pointer to the nixEnvPath value stored in its own env-var, also using this same
+// Key.
+func Key(projectHash string) string {
+	return "DEVBOX_NIX_" + projectHash
+}
+
+// AddToEnv adds the new nixEnvPath for the devbox-project identified by projectHash to the env.
+// It does so by modifying the following env-vars:
+// 1. nixEnvPath key
+// 2. PathStack
+// 3. PATH
+func (s *Stack) AddToEnv(env map[string]string, projectHash, nixEnvPath string) map[string]string {
+	key := Key(projectHash)
+
+	// Add this nixEnvPath to env
+	env[key] = nixEnvPath
+
+	// Add this key to the stack b/c earlier keys get priority
+	s.keys = lo.Uniq(slices.Insert(s.keys, 0, key))
+	env[PathStackEnv] = s.String()
+
+	// Look up the paths-list for each paths-stack element, and join them together to get the final PATH.
+	pathLists := lo.Map(s.keys, func(part string, idx int) string { return env[part] })
+	env["PATH"] = JoinPathLists(pathLists...)
+	return env
+}
+
+// Has tests if the stack has the specified key. Refer to the Key function for constructing
+// the appropriate key for any devbox-project.
+func (s *Stack) Has(key string) bool {
+	for _, k := range s.keys {
+		if k == key {
+			return true
+		}
+	}
+	return false
+}

internal/impl/envvars.go

@@ -8,6 +8,8 @@ import (
 	"os"
 	"sort"
 	"strings"
+
+	"go.jetpack.io/devbox/internal/impl/envpath"
 )
 
 const devboxSetPrefix = "__DEVBOX_SET_"
@@ -136,5 +138,11 @@ func markEnvsAsSetByDevbox(envs ...map[string]string) {
 // as a proxy for this. This allows us to differentiate between global and
 // individual project shells.
 func (d *Devbox) IsEnvEnabled() bool {
-	return os.Getenv(d.ogPathKey()) != ""
+	env := map[string]string{}
+	for _, keyVal := range os.Environ() {
+		parts := strings.Split(keyVal, "=")
+		env[parts[0]] = parts[1]
+	}
+	pathStack := envpath.NewStack(env)
+	return pathStack.Has(envpath.Key(d.projectDirHash()))
 }

internal/impl/shell.go

@@ -386,37 +386,6 @@ func (s *DevboxShell) linkShellStartupFiles(shellSettingsDir string) {
 	}
 }
 
-// JoinPathLists joins and cleans PATH-style strings of
-// [os.ListSeparator] delimited paths. To clean a path list, it splits it and
-// does the following for each element:
-//
-//  1. Applies [filepath.Clean].
-//  2. Removes the path if it's relative (must begin with '/' and not be '.').
-//  3. Removes the path if it's a duplicate.
-func JoinPathLists(pathLists ...string) string {
-	if len(pathLists) == 0 {
-		return ""
-	}
-
-	seen := make(map[string]bool)
-	var cleaned []string
-	for _, path := range pathLists {
-		for _, path := range filepath.SplitList(path) {
-			path = filepath.Clean(path)
-			if path == "." || path[0] != '/' {
-				// Remove empty paths and don't allow relative
-				// paths for security reasons.
-				continue
-			}
-			if !seen[path] {
-				cleaned = append(cleaned, path)
-			}
-			seen[path] = true
-		}
-	}
-	return strings.Join(cleaned, string(filepath.ListSeparator))
-}
-
 func filterPathList(pathList string, keep func(string) bool) string {
 	filtered := []string{}
 	for _, path := range filepath.SplitList(pathList) {

internal/impl/shell_test.go

@@ -13,6 +13,7 @@ import (
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
+	"go.jetpack.io/devbox/internal/impl/envpath"
 	"go.jetpack.io/devbox/internal/shellgen"
 )
 
@@ -125,7 +126,7 @@ func TestCleanEnvPath(t *testing.T) {
 	}
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			got := JoinPathLists(test.inPath)
+			got := envpath.JoinPathLists(test.inPath)
 			if got != test.outPath {
 				t.Errorf("Got incorrect cleaned PATH.\ngot:  %s\nwant: %s", got, test.outPath)
 			}