-
Notifications
You must be signed in to change notification settings - Fork 227
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[shellenv] filter out buildInput paths from PATH #1091
Conversation
Current dependencies on/for this PR: This comment was auto-generated by Graphite. |
65e3518
to
c1385e1
Compare
internal/impl/devbox.go
Outdated
// packages around if they are used somewhere else or if the user hasn't triggered | ||
// garbage collection. | ||
nixEnvPath = filterPathList(nixEnvPath, func(path string) bool { | ||
return !strings.HasPrefix(path, "/nix/store") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A problem with filtering all the /nix/store
paths is that we also remove many utilities that are automatically included by pkgs.mkShell
in the generated flake.nix
.
As a result, basic functionality breaks, like invoking mkdir
.
@mikeland86 thoughts on fixing this?
A (bad) idea is to generate print-dev-env
PATH without any packages in the flake.nix. Keep the /nix/store/
paths from this, while removing any new /nix/store paths added from print-dev-env using the generated flake.nix. BUT this would involve calling print-dev-env
twice, which is slow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you filter out $BuildInputs
only?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep, this works I think. PTAL!
96c855d
to
3dd9cbf
Compare
a3312f6
to
319b0ab
Compare
319b0ab
to
03443a6
Compare
@mikeland86 going to land, since the change is as we'd discussed. |
Summary
problem
Doing
devbox rm <package>
within a devbox shell will usually still have the package's binary accessible in PATH. This is confusing and unexpected.cause
In shellenv's
PATH
, we now have 3 sources of paths:.devbox/virtenv/.wrappers/bin
.devbox/nix/profile/default/bin
/nix/store/<hash>-<package>-<version>/bin
When we do
devbox rm <package>
, the first two (wrappers and profile bins) are removed, but the/nix/store
remains in PATH.The user would have to do
eval $(devbox shellenv)
to remove it. But that is not a good UX.background context
Heres’ why we need/don’t need each one:
nix print-dev-env
. We've never removed them, but need to ensure nothing breaks.Thanks to @mikeland86 for explaining this. I've copy-pasted this from a chat conversation.
this PR
This PR removes from PATH the subset of
/nix/store
paths that belong to the buildInputs, duringshellenv
.Fixes #1060
TODO:
How was it tested?