Refactor env vars as map[string]string instead of []string (#621)

## Summary
This makes it easier to manipulate vars in functions (like
`computeNixEnv`). On the flip side, it does mean we need to convert them
to `[]string` in order to pass them to a `Command`, which is a bit
annoying, but I think it's reasonable. We can introduce a helper
function `asPairs(map[string]string)` or something like that later on if
we feel the need to clean up.

## How was it tested?
CICD

Author: ipince

internal/impl/devbox.go

@@ -309,7 +309,7 @@ func (d *Devbox) RunScript(cmdName string, cmdArgs []string) error {
 			return err
 		}
 		cmdWithArgs = []string{d.scriptPath(d.scriptFilename(arbitraryCmdFilename))}
-		env = append(env, fmt.Sprintf("DEVBOX_RUN_CMD=%s", strings.Join(append([]string{cmdName}, cmdArgs...), " ")))
+		env["DEVBOX_RUN_CMD"] = strings.Join(append([]string{cmdName}, cmdArgs...), " ")
 	}
 
 	return nix.RunScript(d.projectDir, strings.Join(cmdWithArgs, " "), env)
@@ -696,7 +696,7 @@ func (d *Devbox) printPackageUpdateMessage(
 // 3. Append the cleaned host PATH (tradeoff between reproducibility and ease of use).
 // 4. Prepend the devbox-managed nix profile path (which is needed to support devbox add inside shell--can we do without it?).
 // 5. Prepend the paths of any plugins (tbd whether it's actually needed).
-func (d *Devbox) computeNixEnv() ([]string, error) {
+func (d *Devbox) computeNixEnv() (map[string]string, error) {
 
 	vaf, err := nix.PrintDevEnv(d.nixShellFilePath(), d.nixFlakesFilePath())
 	if err != nil {
@@ -722,35 +722,32 @@ func (d *Devbox) computeNixEnv() ([]string, error) {
 		}
 	}
 
-	// PATH handling.
-	pluginVirtenvPath := d.pluginVirtenvPath() // TODO: consider removing this; not being used?
-	nixProfilePath, err := d.profileBinPath()
-	if err != nil {
-		return nil, err
-	}
-	nixPath := env["PATH"]
-	hostPath := nix.CleanEnvPath(os.Getenv("PATH"), os.Getenv("NIX_PROFILES"))
-
-	env["PATH"] = fmt.Sprintf("%s:%s:%s:%s", pluginVirtenvPath, nixProfilePath, nixPath, hostPath)
-
-	envPairs := []string{}
-	for k, v := range env {
-		envPairs = append(envPairs, fmt.Sprintf("%s=%s", k, v))
-	}
-
 	pluginEnv, err := plugin.Env(d.packages(), d.projectDir)
 	if err != nil {
 		return nil, err
 	}
-	envPairs = append(envPairs, pluginEnv...)
+	for k, v := range pluginEnv {
+		env[k] = v
+	}
 
 	// TODO: add shell-specific vars, including:
 	// - NIXPKGS_ALLOW_UNFREE=1 (not needed in run because we don't expect nix calls there)
 	// - __ETC_PROFILE_NIX_SOURCED=1 (not needed in run because we don't expect rc files to try to load nix profiles)
 	// - HISTFILE (not needed in run because it's non-interactive)
 	// - (some of) nix.envToKeep.
 
-	return envPairs, nil
+	// PATH handling.
+	pluginVirtenvPath := d.pluginVirtenvPath() // TODO: consider removing this; not being used?
+	nixProfilePath, err := d.profileBinPath()
+	if err != nil {
+		return nil, err
+	}
+	nixPath := env["PATH"]
+	hostPath := nix.CleanEnvPath(os.Getenv("PATH"), os.Getenv("NIX_PROFILES"))
+
+	env["PATH"] = fmt.Sprintf("%s:%s:%s:%s", pluginVirtenvPath, nixProfilePath, nixPath, hostPath)
+
+	return env, nil
 }
 
 // TODO savil. move to packages.go

internal/nix/nix.go

@@ -50,13 +50,18 @@ func (i *Info) String() string {
 	return fmt.Sprintf("%s-%s", i.Name, i.Version)
 }
 
-func Exec(path string, command []string, env []string) error {
+func Exec(path string, command []string, envPairs map[string]string) error {
+	env := DefaultEnv()
+	for k, v := range envPairs {
+		env = append(env, fmt.Sprintf("%s=%s", k, v))
+	}
+
 	runCmd := strings.Join(command, " ")
 	cmd := exec.Command("nix-shell", path, "--run", runCmd)
 	cmd.Stdin = os.Stdin
 	cmd.Stdout = os.Stdout
 	cmd.Stderr = os.Stderr
-	cmd.Env = append(DefaultEnv(), env...)
+	cmd.Env = env
 	return errors.WithStack(usererr.NewExecError(cmd.Run()))
 }
 

internal/nix/run.go

@@ -1,6 +1,7 @@
 package nix
 
 import (
+	"fmt"
 	"os"
 	"os/exec"
 
@@ -9,18 +10,23 @@ import (
 	"go.jetpack.io/devbox/internal/debug"
 )
 
-func RunScript(projectDir string, cmdWithArgs string, env []string) error {
+func RunScript(projectDir string, cmdWithArgs string, env map[string]string) error {
 	if cmdWithArgs == "" {
 		return errors.New("attempted to run an empty command or script")
 	}
 
+	envPairs := []string{}
+	for k, v := range env {
+		envPairs = append(envPairs, fmt.Sprintf("%s=%s", k, v))
+	}
+
 	// Try to find sh in the PATH, if not, default to a well known absolute path.
 	shPath, err := exec.LookPath("sh")
 	if err != nil {
 		shPath = "/bin/sh"
 	}
 	cmd := exec.Command(shPath, "-c", cmdWithArgs)
-	cmd.Env = env
+	cmd.Env = envPairs
 	cmd.Dir = projectDir
 	cmd.Stdin = os.Stdin
 	cmd.Stdout = os.Stdout

internal/nix/shell.go

@@ -129,9 +129,11 @@ func WithHistoryFile(historyFile string) ShellOption {
 
 // TODO: Consider removing this once plugins add env vars directly to binaries
 // via wrapper scripts.
-func WithEnvVariables(envVariables []string) ShellOption {
+func WithEnvVariables(envVariables map[string]string) ShellOption {
 	return func(s *Shell) {
-		s.env = append(s.env, envVariables...)
+		for k, v := range envVariables {
+			s.env = append(s.env, fmt.Sprintf("%s=%s", k, v))
+		}
 	}
 }
 

internal/plugin/pkgcfg.go

@@ -107,8 +107,8 @@ func (m *Manager) CreateFilesAndShowReadme(pkg, projectDir string) error {
 // Env returns the environment variables for the given plugins.
 // TODO: We should associate the env variables with the individual plugin
 // binaries via wrappers instead of adding to the environment everywhere.
-func Env(pkgs []string, projectDir string) ([]string, error) {
-	env := []string{}
+func Env(pkgs []string, projectDir string) (map[string]string, error) {
+	env := map[string]string{}
 	for _, pkg := range pkgs {
 		cfg, err := getConfigIfAny(pkg, projectDir)
 		if err != nil {
@@ -118,7 +118,7 @@ func Env(pkgs []string, projectDir string) ([]string, error) {
 			continue
 		}
 		for k, v := range cfg.Env {
-			env = append(env, fmt.Sprintf("%s=%s", k, v))
+			env[k] = v
 		}
 	}
 	return env, nil

internal/services/services.go

@@ -46,6 +46,10 @@ func toggleServices(
 	if err != nil {
 		return err
 	}
+	env := []string{}
+	for k, v := range envVars {
+		env = append(env, fmt.Sprintf("%s=%s", k, v))
+	}
 	contextChannels := []<-chan struct{}{}
 	for _, name := range serviceNames {
 		service, found := services[name]
@@ -59,7 +63,7 @@ func toggleServices(
 		)
 		cmd.Stdout = w
 		cmd.Stderr = w
-		cmd.Env = envVars
+		cmd.Env = env
 		cmd.Env = append(cmd.Env, os.Environ()...)
 		if err = cmd.Run(); err != nil {
 			actionString := lo.Ternary(action == startService, "start", "stop")