Skip to content

Commit e4411e5

Browse files
savilsavil
committed
add flag to bin-wrappers to keep path stack in place
1 parent 027bfb8 commit e4411e5

File tree

9 files changed

+54
-34
lines changed

9 files changed

+54
-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: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,11 @@ import (
1313

1414
type shellEnvCmdFlags struct {
1515
envFlag
16-
config configFlags
17-
runInitHook bool
18-
install bool
19-
pure bool
16+
config configFlags
17+
runInitHook bool
18+
install bool
19+
pure bool
20+
pathStackInPlace bool
2021
}
2122

2223
func shellEnvCmd() *cobra.Command {
@@ -69,12 +70,12 @@ func shellEnvFunc(cmd *cobra.Command, flags shellEnvCmdFlags) (string, error) {
6970
}
7071

7172
if flags.install {
72-
if err := box.Install(cmd.Context()); err != nil {
73+
if err := box.Install(cmd.Context(), flags.pathStackInPlace); err != nil {
7374
return "", err
7475
}
7576
}
7677

77-
envStr, err := box.NixEnv(cmd.Context(), flags.runInitHook)
78+
envStr, err := box.NixEnv(cmd.Context(), flags.runInitHook, flags.pathStackInPlace)
7879
if err != nil {
7980
return "", err
8081
}

‎internal/impl/devbox.go

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

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

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

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

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

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

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

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

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

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

790795
pathStack := envpath.NewStack(env)
791-
debug.Log("path stack is: %s", pathStack)
796+
debug.Log("Original path stack is: %s", pathStack)
792797

793798
vaf, err := d.nix.PrintDevEnv(ctx, &nix.PrintDevEnvArgs{
794799
FlakesFilePath: d.nixFlakesFilePath(),
@@ -894,7 +899,8 @@ func (d *Devbox) computeNixEnv(ctx context.Context, usePrintDevEnvCache bool) (m
894899
})
895900
debug.Log("PATH after filtering with buildInputs (%v) is: %s", buildInputs, nixEnvPath)
896901

897-
env = pathStack.AddToEnv(env, d.projectDirHash(), nixEnvPath)
902+
env = pathStack.AddToEnv(env, d.projectDirHash(), nixEnvPath, pathStackInPlace)
903+
debug.Log("New path stack is: %s", pathStack)
898904

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

@@ -917,7 +923,7 @@ var nixEnvCache map[string]string
917923
// nixEnv is a wrapper around computeNixEnv that caches the result.
918924
// Note that this is in-memory cache of the final environment, and not the same
919925
// as the nix print-dev-env cache which is stored in a file.
920-
func (d *Devbox) nixEnv(ctx context.Context) (map[string]string, error) {
926+
func (d *Devbox) nixEnv(ctx context.Context, pathStackInPlace bool) (map[string]string, error) {
921927
defer debug.FunctionTimer().End()
922928
if nixEnvCache != nil {
923929
return nixEnvCache, nil
@@ -934,7 +940,7 @@ func (d *Devbox) nixEnv(ctx context.Context) (map[string]string, error) {
934940
usePrintDevEnvCache = true
935941
}
936942

937-
return d.computeNixEnv(ctx, usePrintDevEnvCache)
943+
return d.computeNixEnv(ctx, usePrintDevEnvCache, pathStackInPlace)
938944
}
939945

940946
func (d *Devbox) nixPrintDevEnvCachePath() string {
@@ -1120,7 +1126,7 @@ func (d *Devbox) setCommonHelperEnvVars(env map[string]string) {
11201126
// give name. This matches how nix flakes behaves if there are conflicts in
11211127
// buildInputs
11221128
func (d *Devbox) NixBins(ctx context.Context) ([]string, error) {
1123-
env, err := d.nixEnv(ctx)
1129+
env, err := d.nixEnv(ctx, false /*pathStackInPlace*/)
11241130
if err != nil {
11251131
return nil, err
11261132
}

‎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-inplace -c {{ .ProjectDir }})"
2525
fi
2626

2727
{{/*

0 commit comments

Comments
 (0)