[services] Fix issue where env var is ignored

devbox.go

@@ -34,12 +34,12 @@ type Devbox interface {
 	Update(ctx context.Context, opts devopt.UpdateOpts) error
 
 	// Interact with services
-	ListServices(ctx context.Context) error
-	RestartServices(ctx context.Context, services ...string) error
+	ListServices(ctx context.Context, runInCurrentShell bool) error
+	RestartServices(ctx context.Context, runInCurrentShell bool, services ...string) error
 	Services() (services.Services, error)
-	StartProcessManager(ctx context.Context, requestedServices []string, background bool, processComposeFileOrDir string) error
-	StartServices(ctx context.Context, services ...string) error
-	StopServices(ctx context.Context, allProjects bool, services ...string) error
+	StartProcessManager(ctx context.Context, runInCurrentShell bool, requestedServices []string, background bool, processComposeFileOrDir string) error
+	StartServices(ctx context.Context, runInCurrentShell bool, services ...string) error
+	StopServices(ctx context.Context, runInCurrentShell, allProjects bool, services ...string) error
 
 	// Generate files
 	Generate(ctx context.Context) error

examples/stacks/lapp-stack/devbox.json

@@ -7,7 +7,8 @@
     "[email protected]"
   ],
   "env": {
-    "PGPORT": "5432"
+    "PGPORT": "5432",
+    "PGHOST": "/tmp/devbox/lapp"
   },
   "shell": {
     "scripts": {
@@ -19,7 +20,6 @@
       "init_db": "initdb",
       "run_test": [
         "mkdir -p /tmp/devbox/lapp",
-        "export PGHOST=/tmp/devbox/lapp",
         "initdb",
         "devbox services start",
         "echo 'sleep 1 second for the postgres server to initialize.' && sleep 1",

examples/stacks/lepp-stack/devbox.json

@@ -9,7 +9,8 @@
   "env": {
     "NGINX_WEB_PORT": "8089",
     "NGINX_WEB_ROOT": "../../../my_app",
-    "PGPORT": "5433"
+    "PGPORT": "5433",
+    "PGHOST": "/tmp/devbox/lepp"
   },
   "shell": {
     "scripts": {
@@ -21,7 +22,7 @@
       "init_db": "initdb",
       "run_test": [
         "mkdir -p /tmp/devbox/lepp",
-        "export PGHOST=/tmp/devbox/lepp",
+        "rm -rf .devbox/virtenv/postgresql/data",
         "initdb",
         "devbox services up -b",
         "echo 'sleep 2 second for the postgres server to initialize.' && sleep 2",

examples/stacks/lepp-stack/devbox.lock

@@ -22,7 +22,7 @@
     },
     "nginx@latest": {
       "last_modified": "2023-09-04T16:24:30Z",
-      "plugin_version": "0.0.3",
+      "plugin_version": "0.0.4",
       "resolved": "github:NixOS/nixpkgs/3c15feef7770eb5500a4b8792623e2d6f598c9c1#nginx",
       "source": "devbox-search",
       "version": "1.24.0",

internal/boxcli/run.go

@@ -5,9 +5,7 @@ package boxcli
 
 import (
 	"fmt"
-	"os"
 	"slices"
-	"strconv"
 	"strings"
 
 	"github.com/samber/lo"
@@ -99,19 +97,12 @@ func runScriptCmd(cmd *cobra.Command, args []string, flags runCmdFlags) error {
 		return err
 	}
 
-	omitBinWrappersFromPath := true
-	// This is for testing. Once we completely remove bin wrappers we can remove
-	// this. It helps simulate shell using "run".
-	if ok, _ := strconv.ParseBool(os.Getenv("DEVBOX_INCLUDE_BIN_WRAPPERS_IN_PATH")); ok {
-		omitBinWrappersFromPath = false
-	}
 	// Check the directory exists.
 	box, err := devbox.Open(&devopt.Opts{
-		Dir:                     path,
-		Stderr:                  cmd.ErrOrStderr(),
-		Pure:                    flags.pure,
-		Env:                     env,
-		OmitBinWrappersFromPath: omitBinWrappersFromPath,
+		Dir:    path,
+		Stderr: cmd.ErrOrStderr(),
+		Pure:   flags.pure,
+		Env:    env,
 	})
 	if err != nil {
 		return redact.Errorf("error reading devbox.json: %w", err)

internal/boxcli/services.go

@@ -12,7 +12,8 @@ import (
 
 type servicesCmdFlags struct {
 	envFlag
-	config configFlags
+	config            configFlags
+	runInCurrentShell bool
 }
 
 type serviceUpFlags struct {
@@ -47,7 +48,13 @@ func servicesCmd(persistentPreRunE ...cobraFunc) *cobra.Command {
 	serviceStopFlags := serviceStopFlags{}
 	servicesCommand := &cobra.Command{
 		Use:   "services",
-		Short: "Interact with devbox services",
+		Short: "Interact with devbox services.",
+		Long: "Interact with devbox services. Services start in a new shell. " +
+			"Plugin services use environment variables specified by plugin unless " +
+			"overridden by the user. To override plugin environment variables, use " +
+			"the --env or --env-file flag. You may also override in devbox.json by " +
+			"using the `env` field or exporting an environment variable in the " +
+			"init hook.",
 		PersistentPreRunE: func(cmd *cobra.Command, args []string) error {
 			preruns := append([]cobraFunc{ensureNixInstalled}, persistentPreRunE...)
 			for _, fn := range preruns {
@@ -105,6 +112,13 @@ func servicesCmd(persistentPreRunE ...cobraFunc) *cobra.Command {
 
 	flags.envFlag.register(servicesCommand)
 	flags.config.registerPersistent(servicesCommand)
+	servicesCommand.PersistentFlags().BoolVar(
+		&flags.runInCurrentShell,
+		"run-in-current-shell",
+		false,
+		"Run the command in the current shell instead of a new shell",
+	)
+	servicesCommand.Flag("run-in-current-shell").Hidden = true
 	serviceUpFlags.register(upCommand)
 	serviceStopFlags.register(stopCommand)
 	servicesCommand.AddCommand(lsCommand)
@@ -124,7 +138,7 @@ func listServices(cmd *cobra.Command, flags servicesCmdFlags) error {
 		return errors.WithStack(err)
 	}
 
-	return box.ListServices(cmd.Context())
+	return box.ListServices(cmd.Context(), flags.runInCurrentShell)
 }
 
 func startServices(cmd *cobra.Command, services []string, flags servicesCmdFlags) error {
@@ -141,7 +155,7 @@ func startServices(cmd *cobra.Command, services []string, flags servicesCmdFlags
 		return errors.WithStack(err)
 	}
 
-	return box.StartServices(cmd.Context(), services...)
+	return box.StartServices(cmd.Context(), flags.runInCurrentShell, services...)
 }
 
 func stopServices(
@@ -165,7 +179,8 @@ func stopServices(
 	if len(services) > 0 && flags.allProjects {
 		return errors.New("cannot use both services and --all-projects arguments simultaneously")
 	}
-	return box.StopServices(cmd.Context(), flags.allProjects, services...)
+	return box.StopServices(
+		cmd.Context(), servicesFlags.runInCurrentShell, flags.allProjects, services...)
 }
 
 func restartServices(
@@ -186,7 +201,7 @@ func restartServices(
 		return errors.WithStack(err)
 	}
 
-	return box.RestartServices(cmd.Context(), services...)
+	return box.RestartServices(cmd.Context(), flags.runInCurrentShell, services...)
 }
 
 func startProcessManager(
@@ -209,5 +224,11 @@ func startProcessManager(
 		return errors.WithStack(err)
 	}
 
-	return box.StartProcessManager(cmd.Context(), args, flags.background, flags.processComposeFile)
+	return box.StartProcessManager(
+		cmd.Context(),
+		servicesFlags.runInCurrentShell,
+		args,
+		flags.background,
+		flags.processComposeFile,
+	)
 }

internal/impl/devbox.go

@@ -30,6 +30,7 @@ import (
 	"go.jetpack.io/devbox/internal/searcher"
 	"go.jetpack.io/devbox/internal/shellgen"
 	"go.jetpack.io/devbox/internal/telemetry"
+	"go.jetpack.io/devbox/internal/wrapnix"
 
 	"go.jetpack.io/devbox/internal/boxcli/usererr"
 	"go.jetpack.io/devbox/internal/cmdutil"
@@ -67,7 +68,6 @@ type Devbox struct {
 	pure                     bool
 	allowInsecureAdds        bool
 	customProcessComposeFile string
-	OmitBinWrappersFromPath  bool
 
 	// This is needed because of the --quiet flag.
 	stderr io.Writer
@@ -98,7 +98,6 @@ func Open(opts *devopt.Opts) (*Devbox, error) {
 		pure:                     opts.Pure,
 		customProcessComposeFile: opts.CustomProcessComposeFile,
 		allowInsecureAdds:        opts.AllowInsecureAdds,
-		OmitBinWrappersFromPath:  opts.OmitBinWrappersFromPath,
 	}
 
 	lock, err := lock.GetFile(box)
@@ -221,6 +220,19 @@ func (d *Devbox) RunScript(ctx context.Context, cmdName string, cmdArgs []string
 	if err != nil {
 		return err
 	}
+
+	// By default we always remove bin wrappers when using `run`. This env var is
+	// for testing. Once we completely remove bin wrappers we can remove this.
+	// It helps simulate shell using "run".
+	if includeBinWrappers, _ := strconv.ParseBool(
+		os.Getenv("DEVBOX_INCLUDE_BIN_WRAPPERS_IN_PATH"),
+	); !includeBinWrappers {
+		env["PATH"] = envpath.RemoveFromPath(
+			env["PATH"],
+			wrapnix.WrapperBinPath(d.projectDir),
+		)
+	}
+
 	// Used to determine whether we're inside a shell (e.g. to prevent shell inception)
 	// This is temporary because StartServices() needs it but should be replaced with
 	// better alternative since devbox run and devbox shell are not the same.
@@ -526,15 +538,22 @@ func (d *Devbox) Services() (services.Services, error) {
 	return result, nil
 }
 
-func (d *Devbox) StartServices(ctx context.Context, serviceNames ...string) error {
-	if !d.IsEnvEnabled() {
-		return d.RunScript(ctx, "devbox", append([]string{"services", "start"}, serviceNames...))
+func (d *Devbox) StartServices(
+	ctx context.Context, runInCurrentShell bool, serviceNames ...string,
+) error {
+	if !runInCurrentShell {
+		return d.RunScript(ctx, "devbox",
+			append(
+				[]string{"services", "start", "--run-in-current-shell"},
+				serviceNames...,
+			),
+		)
 	}
 
 	if !services.ProcessManagerIsRunning(d.projectDir) {
 		fmt.Fprintln(d.stderr, "Process-compose is not running. Starting it now...")
 		fmt.Fprintln(d.stderr, "\nNOTE: We recommend using `devbox services up` to start process-compose and your services")
-		return d.StartProcessManager(ctx, serviceNames, true, "")
+		return d.StartProcessManager(ctx, runInCurrentShell, serviceNames, true, "")
 	}
 
 	svcSet, err := d.Services()
@@ -563,9 +582,9 @@ func (d *Devbox) StartServices(ctx context.Context, serviceNames ...string) erro
 	return nil
 }
 
-func (d *Devbox) StopServices(ctx context.Context, allProjects bool, serviceNames ...string) error {
-	if !d.IsEnvEnabled() {
-		args := []string{"services", "stop"}
+func (d *Devbox) StopServices(ctx context.Context, runInCurrentShell, allProjects bool, serviceNames ...string) error {
+	if !runInCurrentShell {
+		args := []string{"services", "stop", "--run-in-current-shell"}
 		args = append(args, serviceNames...)
 		if allProjects {
 			args = append(args, "--all-projects")
@@ -602,9 +621,10 @@ func (d *Devbox) StopServices(ctx context.Context, allProjects bool, serviceName
 	return nil
 }
 
-func (d *Devbox) ListServices(ctx context.Context) error {
-	if !d.IsEnvEnabled() {
-		return d.RunScript(ctx, "devbox", []string{"services", "ls"})
+func (d *Devbox) ListServices(ctx context.Context, runInCurrentShell bool) error {
+	if !runInCurrentShell {
+		return d.RunScript(ctx,
+			"devbox", []string{"services", "ls", "--run-in-current-shell"})
 	}
 
 	svcSet, err := d.Services()
@@ -640,15 +660,22 @@ func (d *Devbox) ListServices(ctx context.Context) error {
 	return nil
 }
 
-func (d *Devbox) RestartServices(ctx context.Context, serviceNames ...string) error {
-	if !d.IsEnvEnabled() {
-		return d.RunScript(ctx, "devbox", append([]string{"services", "restart"}, serviceNames...))
+func (d *Devbox) RestartServices(
+	ctx context.Context, runInCurrentShell bool, serviceNames ...string,
+) error {
+	if !runInCurrentShell {
+		return d.RunScript(ctx, "devbox",
+			append(
+				[]string{"services", "restart", "--run-in-current-shell"},
+				serviceNames...,
+			),
+		)
 	}
 
 	if !services.ProcessManagerIsRunning(d.projectDir) {
 		fmt.Fprintln(d.stderr, "Process-compose is not running. Starting it now...")
 		fmt.Fprintln(d.stderr, "\nTip: We recommend using `devbox services up` to start process-compose and your services")
-		return d.StartProcessManager(ctx, serviceNames, true, "")
+		return d.StartProcessManager(ctx, runInCurrentShell, serviceNames, true, "")
 	}
 
 	// TODO: Restart with no services should restart the _currently running_ services. This means we should get the list of running services from the process-compose, then restart them all.
@@ -674,12 +701,13 @@ func (d *Devbox) RestartServices(ctx context.Context, serviceNames ...string) er
 
 func (d *Devbox) StartProcessManager(
 	ctx context.Context,
+	runInCurrentShell bool,
 	requestedServices []string,
 	background bool,
 	processComposeFileOrDir string,
 ) error {
-	if !d.IsEnvEnabled() {
-		args := []string{"services", "up"}
+	if !runInCurrentShell {
+		args := []string{"services", "up", "--run-in-current-shell"}
 		args = append(args, requestedServices...)
 		if processComposeFileOrDir != "" {
 			args = append(args, "--process-compose-file", processComposeFileOrDir)
@@ -853,17 +881,11 @@ func (d *Devbox) computeNixEnv(ctx context.Context, usePrintDevEnvCache bool) (m
 	addEnvIfNotPreviouslySetByDevbox(env, pluginEnv)
 
 	env["PATH"] = envpath.JoinPathLists(
+		filepath.Join(d.projectDir, plugin.WrapperBinPath),
 		nix.ProfileBinPath(d.projectDir),
 		env["PATH"],
 	)
 
-	if !d.OmitBinWrappersFromPath {
-		env["PATH"] = envpath.JoinPathLists(
-			filepath.Join(d.projectDir, plugin.WrapperBinPath),
-			env["PATH"],
-		)
-	}
-
 	env["PATH"], err = d.addUtilitiesToPath(ctx, env["PATH"])
 	if err != nil {
 		return nil, err

internal/impl/devopt/devboxopts.go

@@ -12,7 +12,6 @@ type Opts struct {
 	Pure                     bool
 	IgnoreWarnings           bool
 	CustomProcessComposeFile string
-	OmitBinWrappersFromPath  bool
 	Stderr                   io.Writer
 }
 

internal/impl/envpath/pathlists.go

@@ -1,6 +1,7 @@
 package envpath
 
 import (
+	"os"
 	"path/filepath"
 	"strings"
 )
@@ -35,3 +36,22 @@ func JoinPathLists(pathLists ...string) string {
 	}
 	return strings.Join(cleaned, string(filepath.ListSeparator))
 }
+
+func RemoveFromPath(path, pathToRemove string) string {
+	paths := filepath.SplitList(path)
+
+	// Create a new slice to store the modified paths
+	var newPaths []string
+
+	// Iterate through the paths and add them to the newPaths slice if they are not equal to pathToRemove
+	for _, p := range paths {
+		if p != pathToRemove {
+			newPaths = append(newPaths, p)
+		}
+	}
+
+	// Join the modified paths using ":" as the delimiter
+	newPath := strings.Join(newPaths, string(os.PathListSeparator))
+
+	return newPath
+}