Skip to content

Commit df4820e

Browse files
savilsavil
committed
add flag to bin-wrappers to keep path stack in place
1 parent 0cbc154 commit df4820e

File tree

9 files changed

+59
-34
lines changed

9 files changed

+59
-34
lines changed

‎devbox.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,10 @@ type Devbox interface {
2020
Config() *devconfig.Config
2121
EnvVars(ctx context.Context) ([]string, error)
2222
Info(ctx context.Context, pkg string, markdown bool) (string, error)
23-
Install(ctx context.Context) error
23+
Install(ctx context.Context, pathStackInPlace bool) error
2424
IsEnvEnabled() bool
2525
ListScripts() []string
26-
NixEnv(ctx context.Context, includeHooks bool) (string, error)
26+
NixEnv(ctx context.Context, includeHooks, pathStackInPlace bool) (string, error)
2727
PackageNames() []string
2828
ProjectDir() string
2929
Pull(ctx context.Context, opts devopt.PullboxOpts) error

‎internal/boxcli/install.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ func installCmdFunc(cmd *cobra.Command, flags runCmdFlags) error {
3939
if err != nil {
4040
return errors.WithStack(err)
4141
}
42-
if err = box.Install(cmd.Context()); err != nil {
42+
if err = box.Install(cmd.Context(), false /*pathStackInPlace*/); err != nil {
4343
return errors.WithStack(err)
4444
}
4545
fmt.Fprintln(cmd.ErrOrStderr(), "Finished installing packages.")

‎internal/boxcli/shell.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ func runShellCmd(cmd *cobra.Command, flags shellCmdFlags) error {
6666
if flags.printEnv {
6767
// false for includeHooks is because init hooks is not compatible with .envrc files generated
6868
// by versions older than 0.4.6
69-
script, err := box.NixEnv(cmd.Context(), false /*includeHooks*/)
69+
script, err := box.NixEnv(cmd.Context(), false /*includeHooks*/, false /*pathStackInPlace*/)
7070
if err != nil {
7171
return err
7272
}

‎internal/boxcli/shellenv.go

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,11 @@ import (
1515

1616
type shellEnvCmdFlags struct {
1717
envFlag
18-
config configFlags
19-
runInitHook bool
20-
install bool
21-
pure bool
18+
config configFlags
19+
runInitHook bool
20+
install bool
21+
pure bool
22+
pathStackInPlace bool
2223
}
2324

2425
func shellEnvCmd() *cobra.Command {
@@ -48,6 +49,11 @@ func shellEnvCmd() *cobra.Command {
4849

4950
command.Flags().BoolVar(
5051
&flags.pure, "pure", false, "If this flag is specified, devbox creates an isolated environment inheriting almost no variables from the current environment. A few variables, in particular HOME, USER and DISPLAY, are retained.")
52+
command.Flags().BoolVar(
53+
&flags.pure, "path-stack-in-place", true,
54+
"If true, Devbox will not give top priority to the PATH of this project's shellenv. "+
55+
"This project's place in the path stack will be unchanged.")
56+
_ = command.Flags().MarkHidden("path-stack-in-place")
5157

5258
flags.config.register(command)
5359
flags.envFlag.register(command)
@@ -73,12 +79,12 @@ func shellEnvFunc(cmd *cobra.Command, flags shellEnvCmdFlags) (string, error) {
7379
}
7480

7581
if flags.install {
76-
if err := box.Install(cmd.Context()); err != nil {
82+
if err := box.Install(cmd.Context(), flags.pathStackInPlace); err != nil {
7783
return "", err
7884
}
7985
}
8086

81-
envStr, err := box.NixEnv(cmd.Context(), flags.runInitHook)
87+
envStr, err := box.NixEnv(cmd.Context(), flags.runInitHook, flags.pathStackInPlace)
8288
if err != nil {
8389
return "", err
8490
}

‎internal/impl/devbox.go

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ func (d *Devbox) Shell(ctx context.Context) error {
175175
return err
176176
}
177177

178-
envs, err := d.nixEnv(ctx)
178+
envs, err := d.nixEnv(ctx, false /*pathStackInPlace*/)
179179
if err != nil {
180180
return err
181181
}
@@ -219,7 +219,7 @@ func (d *Devbox) RunScript(ctx context.Context, cmdName string, cmdArgs []string
219219
return err
220220
}
221221

222-
env, err := d.nixEnv(ctx)
222+
env, err := d.nixEnv(ctx, false /*pathStackInPlace*/)
223223
if err != nil {
224224
return err
225225
}
@@ -261,11 +261,11 @@ func (d *Devbox) RunScript(ctx context.Context, cmdName string, cmdArgs []string
261261
// Install ensures that all the packages in the config are installed and
262262
// creates all wrappers, but does not run init hooks. It is used to power
263263
// devbox install cli command.
264-
func (d *Devbox) Install(ctx context.Context) error {
264+
func (d *Devbox) Install(ctx context.Context, pathStackInPlace bool) error {
265265
ctx, task := trace.NewTask(ctx, "devboxInstall")
266266
defer task.End()
267267

268-
if _, err := d.NixEnv(ctx, false /*includeHooks*/); err != nil {
268+
if _, err := d.NixEnv(ctx, false /*includeHooks*/, pathStackInPlace); err != nil {
269269
return err
270270
}
271271
return wrapnix.CreateWrappers(ctx, d)
@@ -282,15 +282,15 @@ func (d *Devbox) ListScripts() []string {
282282
return keys
283283
}
284284

285-
func (d *Devbox) NixEnv(ctx context.Context, includeHooks bool) (string, error) {
285+
func (d *Devbox) NixEnv(ctx context.Context, includeHooks, pathStackInPlace bool) (string, error) {
286286
ctx, task := trace.NewTask(ctx, "devboxNixEnv")
287287
defer task.End()
288288

289289
if err := d.ensurePackagesAreInstalled(ctx, ensure); err != nil {
290290
return "", err
291291
}
292292

293-
envs, err := d.nixEnv(ctx)
293+
envs, err := d.nixEnv(ctx, pathStackInPlace)
294294
if err != nil {
295295
return "", err
296296
}
@@ -314,15 +314,16 @@ func (d *Devbox) EnvVars(ctx context.Context) ([]string, error) {
314314
return nil, err
315315
}
316316

317-
envs, err := d.nixEnv(ctx)
317+
envs, err := d.nixEnv(ctx, false /*pathStackInPlace*/)
318318
if err != nil {
319319
return nil, err
320320
}
321321
return mapToPairs(envs), nil
322322
}
323323

324324
func (d *Devbox) ShellEnvHash(ctx context.Context) (string, error) {
325-
envs, err := d.nixEnv(ctx)
325+
// TODO savil: correct?
326+
envs, err := d.nixEnv(ctx, false /*pathStackInPlace*/)
326327
if err != nil {
327328
return "", err
328329
}
@@ -765,7 +766,11 @@ func (d *Devbox) StartProcessManager(
765766
// Note that the shellrc.tmpl template (which sources this environment) does
766767
// some additional processing. The computeNixEnv environment won't necessarily
767768
// represent the final "devbox run" or "devbox shell" environments.
768-
func (d *Devbox) computeNixEnv(ctx context.Context, usePrintDevEnvCache bool) (map[string]string, error) {
769+
func (d *Devbox) computeNixEnv(
770+
ctx context.Context,
771+
usePrintDevEnvCache bool,
772+
pathStackInPlace bool,
773+
) (map[string]string, error) {
769774
defer trace.StartRegion(ctx, "computeNixEnv").End()
770775

771776
// Append variables from current env if --pure is not passed
@@ -786,7 +791,7 @@ func (d *Devbox) computeNixEnv(ctx context.Context, usePrintDevEnvCache bool) (m
786791
debug.Log("current environment PATH is: %s", env["PATH"])
787792

788793
pathStack := envpath.NewStack(env)
789-
debug.Log("path stack is: %s", pathStack)
794+
debug.Log("Original path stack is: %s", pathStack)
790795

791796
vaf, err := d.nix.PrintDevEnv(ctx, &nix.PrintDevEnvArgs{
792797
FlakesFilePath: d.nixFlakesFilePath(),
@@ -892,7 +897,8 @@ func (d *Devbox) computeNixEnv(ctx context.Context, usePrintDevEnvCache bool) (m
892897
})
893898
debug.Log("PATH after filtering with buildInputs (%v) is: %s", buildInputs, nixEnvPath)
894899

895-
env = pathStack.AddToEnv(env, d.projectDirHash(), nixEnvPath)
900+
env = pathStack.AddToEnv(env, d.projectDirHash(), nixEnvPath, pathStackInPlace)
901+
debug.Log("New path stack is: %s", pathStack)
896902

897903
debug.Log("computed environment PATH is: %s", env["PATH"])
898904

@@ -915,7 +921,7 @@ var nixEnvCache map[string]string
915921
// nixEnv is a wrapper around computeNixEnv that caches the result.
916922
// Note that this is in-memory cache of the final environment, and not the same
917923
// as the nix print-dev-env cache which is stored in a file.
918-
func (d *Devbox) nixEnv(ctx context.Context) (map[string]string, error) {
924+
func (d *Devbox) nixEnv(ctx context.Context, pathStackInPlace bool) (map[string]string, error) {
919925
defer debug.FunctionTimer().End()
920926
if nixEnvCache != nil {
921927
return nixEnvCache, nil
@@ -932,7 +938,7 @@ func (d *Devbox) nixEnv(ctx context.Context) (map[string]string, error) {
932938
usePrintDevEnvCache = true
933939
}
934940

935-
return d.computeNixEnv(ctx, usePrintDevEnvCache)
941+
return d.computeNixEnv(ctx, usePrintDevEnvCache, pathStackInPlace)
936942
}
937943

938944
func (d *Devbox) nixPrintDevEnvCachePath() string {
@@ -1119,7 +1125,7 @@ func (d *Devbox) setCommonHelperEnvVars(env map[string]string) {
11191125
// give name. This matches how nix flakes behaves if there are conflicts in
11201126
// buildInputs
11211127
func (d *Devbox) NixBins(ctx context.Context) ([]string, error) {
1122-
env, err := d.nixEnv(ctx)
1128+
env, err := d.nixEnv(ctx, false /*pathStackInPlace*/)
11231129
if err != nil {
11241130
return nil, err
11251131
}

‎internal/impl/devbox_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ func TestComputeNixEnv(t *testing.T) {
7171
d := devboxForTesting(t)
7272
d.nix = &testNix{}
7373
ctx := context.Background()
74-
env, err := d.computeNixEnv(ctx, false /*use cache*/)
74+
env, err := d.computeNixEnv(ctx, false /*use cache*/, false /*pathStackInPlace*/)
7575
require.NoError(t, err, "computeNixEnv should not fail")
7676
assert.NotNil(t, env, "computeNixEnv should return a valid env")
7777
}
@@ -80,7 +80,7 @@ func TestComputeNixPathIsIdempotent(t *testing.T) {
8080
devbox := devboxForTesting(t)
8181
devbox.nix = &testNix{"/tmp/my/path"}
8282
ctx := context.Background()
83-
env, err := devbox.computeNixEnv(ctx, false /*use cache*/)
83+
env, err := devbox.computeNixEnv(ctx, false /*use cache*/, false /*pathStackInPlace*/)
8484
require.NoError(t, err, "computeNixEnv should not fail")
8585
path := env["PATH"]
8686
assert.NotEmpty(t, path, "path should not be nil")
@@ -90,7 +90,7 @@ func TestComputeNixPathIsIdempotent(t *testing.T) {
9090
t.Setenv(envpath.PathStackEnv, env[envpath.PathStackEnv])
9191
t.Setenv(envpath.Key(devbox.projectDirHash()), env[envpath.Key(devbox.projectDirHash())])
9292

93-
env, err = devbox.computeNixEnv(ctx, false /*use cache*/)
93+
env, err = devbox.computeNixEnv(ctx, false /*use cache*/, false /*pathStackInPlace*/)
9494
require.NoError(t, err, "computeNixEnv should not fail")
9595
path2 := env["PATH"]
9696

@@ -101,7 +101,7 @@ func TestComputeNixPathWhenRemoving(t *testing.T) {
101101
devbox := devboxForTesting(t)
102102
devbox.nix = &testNix{"/tmp/my/path"}
103103
ctx := context.Background()
104-
env, err := devbox.computeNixEnv(ctx, false /*use cache*/)
104+
env, err := devbox.computeNixEnv(ctx, false /*use cache*/, false /*pathStackInPlace*/)
105105
require.NoError(t, err, "computeNixEnv should not fail")
106106
path := env["PATH"]
107107
assert.NotEmpty(t, path, "path should not be nil")
@@ -113,7 +113,7 @@ func TestComputeNixPathWhenRemoving(t *testing.T) {
113113
t.Setenv(envpath.Key(devbox.projectDirHash()), env[envpath.Key(devbox.projectDirHash())])
114114

115115
devbox.nix.(*testNix).path = ""
116-
env, err = devbox.computeNixEnv(ctx, false /*use cache*/)
116+
env, err = devbox.computeNixEnv(ctx, false /*use cache*/, false /*pathStackInPlace*/)
117117
require.NoError(t, err, "computeNixEnv should not fail")
118118
path2 := env["PATH"]
119119
assert.NotContains(t, path2, "/tmp/my/path", "path should not contain /tmp/my/path")

‎internal/impl/envpath/pathstack.go

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,14 +58,27 @@ func Key(projectHash string) string {
5858
// 1. nixEnvPath key
5959
// 2. PathStack
6060
// 3. PATH
61-
func (s *Stack) AddToEnv(env map[string]string, projectHash, nixEnvPath string) map[string]string {
61+
//
62+
// Returns the modified env map
63+
func (s *Stack) AddToEnv(
64+
env map[string]string,
65+
projectHash string,
66+
nixEnvPath string,
67+
pathStackInPlace bool,
68+
) map[string]string {
6269
key := Key(projectHash)
6370

6471
// Add this nixEnvPath to env
6572
env[key] = nixEnvPath
6673

67-
// Add this key to the stack b/c earlier keys get priority
68-
s.keys = lo.Uniq(slices.Insert(s.keys, 0, key))
74+
// Common case: ensure this key is at the top of the stack
75+
if !pathStackInPlace ||
76+
// Case pathStackInPlace == true, usually from bin-wrapper or (in future) shell hook.
77+
// Add this key only if absent from the stack
78+
!lo.Contains(s.keys, key) {
79+
80+
s.keys = lo.Uniq(slices.Insert(s.keys, 0, key))
81+
}
6982
env[PathStackEnv] = s.String()
7083

7184
// Look up the paths-list for each paths-stack element, and join them together to get the final PATH.

‎internal/impl/packages.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ func (d *Devbox) ensurePackagesAreInstalled(ctx context.Context, mode installMod
234234
}
235235

236236
// Force print-dev-env cache to be recomputed.
237-
if _, err := d.computeNixEnv(ctx, false /*use cache*/); err != nil {
237+
if _, err := d.computeNixEnv(ctx, false /*use cache*/, false /*pathStackInPlace*/); err != nil {
238238
return err
239239
}
240240

‎internal/wrapnix/wrapper.sh.tmpl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ DO_NOT_TRACK=1 can be removed once we optimize segment to queue events.
2121

2222
if [[ "${{ .ShellEnvHashKey }}" != "{{ .ShellEnvHash }}" ]] && [[ -z "${{ .ShellEnvHashKey }}_GUARD" ]]; then
2323
export {{ .ShellEnvHashKey }}_GUARD=true
24-
eval "$(DO_NOT_TRACK=1 devbox shellenv -c {{ .ProjectDir }})"
24+
eval "$(DO_NOT_TRACK=1 devbox shellenv --path-stack-in-place -c {{ .ProjectDir }})"
2525
fi
2626

2727
{{/*

0 commit comments

Comments
 (0)