requested changes by Greg and Mike

Author: savil

internal/envir/util.go

@@ -4,8 +4,10 @@
 package envir
 
 import (
+	"fmt"
 	"os"
 	"strconv"
+	"strings"
 )
 
 func IsDevboxCloud() bool {
@@ -43,3 +45,23 @@ func GetValueOrDefault(key, def string) string {
 
 	return val
 }
+
+func MapToPairs(m map[string]string) []string {
+	pairs := []string{}
+	for k, v := range m {
+		pairs = append(pairs, fmt.Sprintf("%s=%s", k, v))
+	}
+	return pairs
+}
+
+func PairsToMap(pairs []string) map[string]string {
+	vars := map[string]string{}
+	for _, p := range pairs {
+		k, v, ok := strings.Cut(p, "=")
+		if !ok {
+			continue
+		}
+		vars[k] = v
+	}
+	return vars
+}

internal/impl/devbox.go

@@ -25,6 +25,7 @@ import (
 	"go.jetpack.io/devbox/internal/searcher"
 	"go.jetpack.io/devbox/internal/shellgen"
 	"go.jetpack.io/devbox/internal/telemetry"
+	"golang.org/x/exp/maps"
 	"golang.org/x/exp/slices"
 
 	"go.jetpack.io/devbox/internal/boxcli/usererr"
@@ -789,8 +790,8 @@ func (d *Devbox) computeNixEnv(ctx context.Context, usePrintDevEnvCache bool) (m
 
 	debug.Log("current environment PATH is: %s", env["PATH"])
 
-	pathStack := envpath.Stack(env)
-	debug.Log("Original path stack is: %s", pathStack)
+	originalEnv := make(map[string]string, len(env))
+	maps.Copy(originalEnv, env)
 
 	vaf, err := d.nix.PrintDevEnv(ctx, &nix.PrintDevEnvArgs{
 		FlakesFilePath:       d.nixFlakesFilePath(),
@@ -896,7 +897,10 @@ func (d *Devbox) computeNixEnv(ctx context.Context, usePrintDevEnvCache bool) (m
 	})
 	debug.Log("PATH after filtering with buildInputs (%v) is: %s", buildInputs, nixEnvPath)
 
-	env = pathStack.PushAndUpdateEnv(env, d.projectDirHash(), nixEnvPath, d.preservePathStack)
+	pathStack := envpath.Stack(originalEnv)
+	pathStack.Push(env, d.projectDirHash(), nixEnvPath, d.preservePathStack)
+	env["PATH"] = pathStack.Path(env)
+
 	debug.Log("New path stack is: %s", pathStack)
 
 	debug.Log("computed environment PATH is: %s", env["PATH"])

internal/impl/envpath/stack.go

@@ -32,7 +32,7 @@ type stack struct {
 
 func Stack(env map[string]string) *stack {
 	stackEnv, ok := env[PathStackEnv]
-	if !ok {
+	if !ok || strings.TrimSpace(stackEnv) == "" {
 		// 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
@@ -48,29 +48,33 @@ func (s *stack) String() string {
 	return strings.Join(s.keys, ":")
 }
 
+func (s *stack) Path(env map[string]string) string {
+	// Look up the paths-list for each 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] })
+	return JoinPathLists(pathLists...)
+}
+
 // 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_ENV_PATH_" + projectHash
 }
 
-// PushAndUpdateEnv adds the new nixEnvPath for the devbox-project identified by projectHash.
-// The nixEnvPath is pushed to the top of the stack (given highest priority), unless preservePathStack
-// is enabled.
+// Push adds the new nixEnvPath for the devbox-project identified by projectHash.
+// The nixEnvPath is pushed to the top of the stack (given highest priority),
+// unless preservePathStack is enabled.
 //
-// It also updated the env by modifying the following env-vars:
+// It also updates the env by modifying the following env-vars:
 // 1. nixEnvPath key
 // 2. PathStack
 // 3. PATH
-//
-// Returns the modified env map
-func (s *stack) PushAndUpdateEnv(
+func (s *stack) Push(
 	env map[string]string,
 	projectHash string,
 	nixEnvPath string,
 	preservePathStack bool,
-) map[string]string {
+) {
 	key := Key(projectHash)
 
 	// Add this nixEnvPath to env
@@ -85,15 +89,10 @@ func (s *stack) PushAndUpdateEnv(
 		s.keys = lo.Uniq(slices.Insert(s.keys, 0, key))
 	}
 	env[PathStackEnv] = s.String()
-
-	// Look up the paths-list for each 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 {
-	return lo.Contains(s.keys, key)
+func (s *stack) Has(projectHash string) bool {
+	return lo.Contains(s.keys, Key(projectHash))
 }

internal/impl/envvars.go

@@ -4,36 +4,16 @@
 package impl
 
 import (
-	"fmt"
 	"os"
 	"sort"
 	"strings"
 
+	"go.jetpack.io/devbox/internal/envir"
 	"go.jetpack.io/devbox/internal/impl/envpath"
 )
 
 const devboxSetPrefix = "__DEVBOX_SET_"
 
-func mapToPairs(m map[string]string) []string {
-	pairs := []string{}
-	for k, v := range m {
-		pairs = append(pairs, fmt.Sprintf("%s=%s", k, v))
-	}
-	return pairs
-}
-
-func pairsToMap(pairs []string) map[string]string {
-	vars := map[string]string{}
-	for _, p := range pairs {
-		k, v, ok := strings.Cut(p, "=")
-		if !ok {
-			continue
-		}
-		vars[k] = v
-	}
-	return vars
-}
-
 // exportify takes a map of [string]string and returns a single string
 // of the form export KEY="VAL"; and escapes all the vals from special characters.
 func exportify(vars map[string]string) string {
@@ -138,11 +118,6 @@ 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 {
-	env := map[string]string{}
-	for _, keyVal := range os.Environ() {
-		parts := strings.Split(keyVal, "=")
-		env[parts[0]] = parts[1]
-	}
-	pathStack := envpath.Stack(env)
-	return pathStack.Has(envpath.Key(d.projectDirHash()))
+	pathStack := envpath.Stack(envir.PairsToMap(os.Environ()))
+	return pathStack.Has(d.projectDirHash())
 }

internal/impl/shell.go

@@ -246,7 +246,7 @@ func (s *DevboxShell) Run() error {
 	}
 
 	cmd = exec.Command(s.binPath)
-	cmd.Env = mapToPairs(env)
+	cmd.Env = envir.MapToPairs(env)
 	cmd.Args = append(cmd.Args, extraArgs...)
 	cmd.Stdin = os.Stdin
 	cmd.Stdout = os.Stdout

internal/impl/shell_test.go

@@ -13,6 +13,7 @@ import (
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
+	"go.jetpack.io/devbox/internal/envir"
 	"go.jetpack.io/devbox/internal/shellgen"
 )
 
@@ -45,7 +46,7 @@ func testWriteDevboxShellrc(t *testing.T, testdirs []string) {
 		test := &tests[i]
 		test.name = filepath.Base(path)
 		if b, err := os.ReadFile(filepath.Join(path, "env")); err == nil {
-			test.env = pairsToMap(strings.Split(string(b), "\n"))
+			test.env = envir.PairsToMap(strings.Split(string(b), "\n"))
 		}
 
 		test.hooksFilePath = shellgen.ScriptPath(projectDir, shellgen.HooksFilename)