Skip to content

Commit faa40d5

Browse files
mikeland73mikeland73
committed
[services] Fix issue where env var is ignored
1 parent c694277 commit faa40d5

File tree

7 files changed

+97
-52
lines changed

7 files changed

+97
-52
lines changed

‎devbox.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,12 @@ type Devbox interface {
3434
Update(ctx context.Context, opts devopt.UpdateOpts) error
3535

3636
// Interact with services
37-
ListServices(ctx context.Context) error
38-
RestartServices(ctx context.Context, services ...string) error
37+
ListServices(ctx context.Context, runInCurrentShell bool) error
38+
RestartServices(ctx context.Context, runInCurrentShell bool, services ...string) error
3939
Services() (services.Services, error)
40-
StartProcessManager(ctx context.Context, requestedServices []string, background bool, processComposeFileOrDir string) error
41-
StartServices(ctx context.Context, services ...string) error
42-
StopServices(ctx context.Context, allProjects bool, services ...string) error
40+
StartProcessManager(ctx context.Context, runInCurrentShell bool, requestedServices []string, background bool, processComposeFileOrDir string) error
41+
StartServices(ctx context.Context, runInCurrentShell bool, services ...string) error
42+
StopServices(ctx context.Context, runInCurrentShell bool, allProjects bool, services ...string) error
4343

4444
// Generate files
4545
Generate(ctx context.Context) error

‎internal/boxcli/run.go

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,7 @@ package boxcli
55

66
import (
77
"fmt"
8-
"os"
98
"slices"
10-
"strconv"
119
"strings"
1210

1311
"github.com/samber/lo"
@@ -99,19 +97,12 @@ func runScriptCmd(cmd *cobra.Command, args []string, flags runCmdFlags) error {
9997
return err
10098
}
10199

102-
omitBinWrappersFromPath := true
103-
// This is for testing. Once we completely remove bin wrappers we can remove
104-
// this. It helps simulate shell using "run".
105-
if ok, _ := strconv.ParseBool(os.Getenv("DEVBOX_INCLUDE_BIN_WRAPPERS_IN_PATH")); ok {
106-
omitBinWrappersFromPath = false
107-
}
108100
// Check the directory exists.
109101
box, err := devbox.Open(&devopt.Opts{
110-
Dir: path,
111-
Stderr: cmd.ErrOrStderr(),
112-
Pure: flags.pure,
113-
Env: env,
114-
OmitBinWrappersFromPath: omitBinWrappersFromPath,
102+
Dir: path,
103+
Stderr: cmd.ErrOrStderr(),
104+
Pure: flags.pure,
105+
Env: env,
115106
})
116107
if err != nil {
117108
return redact.Errorf("error reading devbox.json: %w", err)

‎internal/boxcli/services.go

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@ import (
1212

1313
type servicesCmdFlags struct {
1414
envFlag
15-
config configFlags
15+
config configFlags
16+
runInCurrentShell bool
1617
}
1718

1819
type serviceUpFlags struct {
@@ -105,6 +106,13 @@ func servicesCmd(persistentPreRunE ...cobraFunc) *cobra.Command {
105106

106107
flags.envFlag.register(servicesCommand)
107108
flags.config.registerPersistent(servicesCommand)
109+
servicesCommand.PersistentFlags().BoolVar(
110+
&flags.runInCurrentShell,
111+
"run-in-current-shell",
112+
false,
113+
"Run the command in the current shell instead of a new shell",
114+
)
115+
servicesCommand.Flag("run-in-current-shell").Hidden = true
108116
serviceUpFlags.register(upCommand)
109117
serviceStopFlags.register(stopCommand)
110118
servicesCommand.AddCommand(lsCommand)
@@ -124,7 +132,7 @@ func listServices(cmd *cobra.Command, flags servicesCmdFlags) error {
124132
return errors.WithStack(err)
125133
}
126134

127-
return box.ListServices(cmd.Context())
135+
return box.ListServices(cmd.Context(), flags.runInCurrentShell)
128136
}
129137

130138
func startServices(cmd *cobra.Command, services []string, flags servicesCmdFlags) error {
@@ -141,7 +149,7 @@ func startServices(cmd *cobra.Command, services []string, flags servicesCmdFlags
141149
return errors.WithStack(err)
142150
}
143151

144-
return box.StartServices(cmd.Context(), services...)
152+
return box.StartServices(cmd.Context(), flags.runInCurrentShell, services...)
145153
}
146154

147155
func stopServices(
@@ -165,7 +173,8 @@ func stopServices(
165173
if len(services) > 0 && flags.allProjects {
166174
return errors.New("cannot use both services and --all-projects arguments simultaneously")
167175
}
168-
return box.StopServices(cmd.Context(), flags.allProjects, services...)
176+
return box.StopServices(
177+
cmd.Context(), servicesFlags.runInCurrentShell, flags.allProjects, services...)
169178
}
170179

171180
func restartServices(
@@ -186,7 +195,7 @@ func restartServices(
186195
return errors.WithStack(err)
187196
}
188197

189-
return box.RestartServices(cmd.Context(), services...)
198+
return box.RestartServices(cmd.Context(), flags.runInCurrentShell, services...)
190199
}
191200

192201
func startProcessManager(
@@ -209,5 +218,11 @@ func startProcessManager(
209218
return errors.WithStack(err)
210219
}
211220

212-
return box.StartProcessManager(cmd.Context(), args, flags.background, flags.processComposeFile)
221+
return box.StartProcessManager(
222+
cmd.Context(),
223+
servicesFlags.runInCurrentShell,
224+
args,
225+
flags.background,
226+
flags.processComposeFile,
227+
)
213228
}

‎internal/impl/devbox.go

Lines changed: 45 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import (
3030
"go.jetpack.io/devbox/internal/searcher"
3131
"go.jetpack.io/devbox/internal/shellgen"
3232
"go.jetpack.io/devbox/internal/telemetry"
33+
"go.jetpack.io/devbox/internal/wrapnix"
3334

3435
"go.jetpack.io/devbox/internal/boxcli/usererr"
3536
"go.jetpack.io/devbox/internal/cmdutil"
@@ -67,7 +68,6 @@ type Devbox struct {
6768
pure bool
6869
allowInsecureAdds bool
6970
customProcessComposeFile string
70-
OmitBinWrappersFromPath bool
7171

7272
// This is needed because of the --quiet flag.
7373
stderr io.Writer
@@ -98,7 +98,6 @@ func Open(opts *devopt.Opts) (*Devbox, error) {
9898
pure: opts.Pure,
9999
customProcessComposeFile: opts.CustomProcessComposeFile,
100100
allowInsecureAdds: opts.AllowInsecureAdds,
101-
OmitBinWrappersFromPath: opts.OmitBinWrappersFromPath,
102101
}
103102

104103
lock, err := lock.GetFile(box)
@@ -221,6 +220,19 @@ func (d *Devbox) RunScript(ctx context.Context, cmdName string, cmdArgs []string
221220
if err != nil {
222221
return err
223222
}
223+
224+
// By default we always remove bin wrappers when using `run`. This env var is
225+
// for testing. Once we completely remove bin wrappers we can remove this.
226+
// It helps simulate shell using "run".
227+
if includeBinWrappers, _ := strconv.ParseBool(
228+
os.Getenv("DEVBOX_INCLUDE_BIN_WRAPPERS_IN_PATH"),
229+
); !includeBinWrappers {
230+
env["PATH"] = envpath.RemoveFromPath(
231+
env["PATH"],
232+
wrapnix.WrapperBinPath(d.projectDir),
233+
)
234+
}
235+
224236
// Used to determine whether we're inside a shell (e.g. to prevent shell inception)
225237
// This is temporary because StartServices() needs it but should be replaced with
226238
// better alternative since devbox run and devbox shell are not the same.
@@ -526,15 +538,21 @@ func (d *Devbox) Services() (services.Services, error) {
526538
return result, nil
527539
}
528540

529-
func (d *Devbox) StartServices(ctx context.Context, serviceNames ...string) error {
530-
if !d.IsEnvEnabled() {
531-
return d.RunScript(ctx, "devbox", append([]string{"services", "start"}, serviceNames...))
541+
func (d *Devbox) StartServices(
542+
ctx context.Context, runInCurrentShell bool, serviceNames ...string) error {
543+
if !runInCurrentShell {
544+
return d.RunScript(ctx, "devbox",
545+
append(
546+
[]string{"services", "start", "--run-in-current-shell"},
547+
serviceNames...,
548+
),
549+
)
532550
}
533551

534552
if !services.ProcessManagerIsRunning(d.projectDir) {
535553
fmt.Fprintln(d.stderr, "Process-compose is not running. Starting it now...")
536554
fmt.Fprintln(d.stderr, "\nNOTE: We recommend using `devbox services up` to start process-compose and your services")
537-
return d.StartProcessManager(ctx, serviceNames, true, "")
555+
return d.StartProcessManager(ctx, runInCurrentShell, serviceNames, true, "")
538556
}
539557

540558
svcSet, err := d.Services()
@@ -563,9 +581,9 @@ func (d *Devbox) StartServices(ctx context.Context, serviceNames ...string) erro
563581
return nil
564582
}
565583

566-
func (d *Devbox) StopServices(ctx context.Context, allProjects bool, serviceNames ...string) error {
567-
if !d.IsEnvEnabled() {
568-
args := []string{"services", "stop"}
584+
func (d *Devbox) StopServices(ctx context.Context, runInCurrentShell bool, allProjects bool, serviceNames ...string) error {
585+
if !runInCurrentShell {
586+
args := []string{"services", "stop", "--run-in-current-shell"}
569587
args = append(args, serviceNames...)
570588
if allProjects {
571589
args = append(args, "--all-projects")
@@ -602,9 +620,10 @@ func (d *Devbox) StopServices(ctx context.Context, allProjects bool, serviceName
602620
return nil
603621
}
604622

605-
func (d *Devbox) ListServices(ctx context.Context) error {
606-
if !d.IsEnvEnabled() {
607-
return d.RunScript(ctx, "devbox", []string{"services", "ls"})
623+
func (d *Devbox) ListServices(ctx context.Context, runInCurrentShell bool) error {
624+
if !runInCurrentShell {
625+
return d.RunScript(ctx,
626+
"devbox", []string{"services", "ls", "--run-in-current-shell"})
608627
}
609628

610629
svcSet, err := d.Services()
@@ -640,15 +659,21 @@ func (d *Devbox) ListServices(ctx context.Context) error {
640659
return nil
641660
}
642661

643-
func (d *Devbox) RestartServices(ctx context.Context, serviceNames ...string) error {
644-
if !d.IsEnvEnabled() {
645-
return d.RunScript(ctx, "devbox", append([]string{"services", "restart"}, serviceNames...))
662+
func (d *Devbox) RestartServices(
663+
ctx context.Context, runInCurrentShell bool, serviceNames ...string) error {
664+
if !runInCurrentShell {
665+
return d.RunScript(ctx, "devbox",
666+
append(
667+
[]string{"services", "restart", "--run-in-current-shell"},
668+
serviceNames...,
669+
),
670+
)
646671
}
647672

648673
if !services.ProcessManagerIsRunning(d.projectDir) {
649674
fmt.Fprintln(d.stderr, "Process-compose is not running. Starting it now...")
650675
fmt.Fprintln(d.stderr, "\nTip: We recommend using `devbox services up` to start process-compose and your services")
651-
return d.StartProcessManager(ctx, serviceNames, true, "")
676+
return d.StartProcessManager(ctx, runInCurrentShell, serviceNames, true, "")
652677
}
653678

654679
// 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 +699,13 @@ func (d *Devbox) RestartServices(ctx context.Context, serviceNames ...string) er
674699

675700
func (d *Devbox) StartProcessManager(
676701
ctx context.Context,
702+
runInCurrentShell bool,
677703
requestedServices []string,
678704
background bool,
679705
processComposeFileOrDir string,
680706
) error {
681-
if !d.IsEnvEnabled() {
682-
args := []string{"services", "up"}
707+
if !runInCurrentShell {
708+
args := []string{"services", "up", "--run-in-current-shell"}
683709
args = append(args, requestedServices...)
684710
if processComposeFileOrDir != "" {
685711
args = append(args, "--process-compose-file", processComposeFileOrDir)
@@ -853,17 +879,11 @@ func (d *Devbox) computeNixEnv(ctx context.Context, usePrintDevEnvCache bool) (m
853879
addEnvIfNotPreviouslySetByDevbox(env, pluginEnv)
854880

855881
env["PATH"] = envpath.JoinPathLists(
882+
filepath.Join(d.projectDir, plugin.WrapperBinPath),
856883
nix.ProfileBinPath(d.projectDir),
857884
env["PATH"],
858885
)
859886

860-
if !d.OmitBinWrappersFromPath {
861-
env["PATH"] = envpath.JoinPathLists(
862-
filepath.Join(d.projectDir, plugin.WrapperBinPath),
863-
env["PATH"],
864-
)
865-
}
866-
867887
env["PATH"], err = d.addUtilitiesToPath(ctx, env["PATH"])
868888
if err != nil {
869889
return nil, err

‎internal/impl/devopt/devboxopts.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ type Opts struct {
1212
Pure bool
1313
IgnoreWarnings bool
1414
CustomProcessComposeFile string
15-
OmitBinWrappersFromPath bool
1615
Stderr io.Writer
1716
}
1817

‎internal/impl/envpath/pathlists.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package envpath
22

33
import (
4+
"os"
45
"path/filepath"
56
"strings"
67
)
@@ -35,3 +36,22 @@ func JoinPathLists(pathLists ...string) string {
3536
}
3637
return strings.Join(cleaned, string(filepath.ListSeparator))
3738
}
39+
40+
func RemoveFromPath(path, pathToRemove string) string {
41+
paths := strings.Split(path, string(os.PathListSeparator))
42+
43+
// Create a new slice to store the modified paths
44+
var newPaths []string
45+
46+
// Iterate through the paths and add them to the newPaths slice if they are not equal to pathToRemove
47+
for _, p := range paths {
48+
if p != pathToRemove {
49+
newPaths = append(newPaths, p)
50+
}
51+
}
52+
53+
// Join the modified paths using ":" as the delimiter
54+
newPath := strings.Join(newPaths, string(os.PathListSeparator))
55+
56+
return newPath
57+
}

‎internal/wrapnix/wrapper.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ func CreateWrappers(ctx context.Context, args CreateWrappersArgs) error {
4444
_ = os.RemoveAll(filepath.Join(args.ProjectDir, plugin.WrapperPath))
4545

4646
// Recreate the bin wrapper directory
47-
destPath := filepath.Join(wrapperBinPath(args.ProjectDir))
47+
destPath := filepath.Join(WrapperBinPath(args.ProjectDir))
4848
_ = os.MkdirAll(destPath, 0o755)
4949

5050
bashPath := cmdutil.GetPathOrDefault("bash", "/bin/bash")
@@ -195,6 +195,6 @@ func createSymlinksForSupportDirs(projectDir string) error {
195195
return nil
196196
}
197197

198-
func wrapperBinPath(projectDir string) string {
198+
func WrapperBinPath(projectDir string) string {
199199
return filepath.Join(projectDir, plugin.WrapperBinPath)
200200
}

0 commit comments

Comments
 (0)