shell,nix: set original PATH at start of init file #57
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This fixes a bug that causes the devbox shell's init to fail when the user's rcfile tries to call a command outside of devbox. For example, the following line in a ~/.bashrc will trigger an error if the devbox doesn't have Go installed: export PATH="$(go env GOPATH):$PATH" This is because we're adding the ORIGINAL_PATH at the end of their shell init script, which means that anything before that only see Nix packages. Fix this by adding a pre-init hook that lets us restore the PATH before the user's rcfile, while still forcing Nix packages to come first in the post-init hook.
|
@LucilleH do you mind running this to verify that |
loreto
reviewed
Sep 2, 2022
| # but prefer anything installed by Nix. | ||
| export PATH="$PURE_NIX_PATH:$ORIGINAL_PATH" | ||
| sh.PreInitHook = ` | ||
| # Update the $PATH so that the user's init script has access to all of their |
There was a problem hiding this comment.
Should both the PreInit and PostInit hooks set the path to: $PURE_NIX_PATH:$ORIGINAL_PATH?
There was a problem hiding this comment.
I just switched both hooks to use the Nix packages.
I wasn't sure what the correct behavior should be:
- Prefer the non-devbox PATH to guarantee compatibility with the user's rcfile.
- Prefer the devbox PATH so that hooks use the Nix packages.
|
@LucilleH are you able to test again? |
mikeland73
reviewed
Sep 3, 2022
| if runErr != nil && d.Debug() { | ||
| log.Printf("Error: %+v\n", runErr) | ||
| strVal := "" | ||
| if d.flag.Changed { |
There was a problem hiding this comment.
This doesn't work. preRun runs before the command so the flag is not set yet
There was a problem hiding this comment.
Well that explains it. I'll put up another PR to clean up the debug stuff and make the flag work.
| var enabled bool | ||
|
|
||
| func init() { | ||
| enabled, _ = strconv.ParseBool(os.Getenv("DEBUG")) |
There was a problem hiding this comment.
Should this be DEVBOX_DEBUG ?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This fixes a bug that causes the devbox shell's init to fail when the user's rcfile tries to call a command outside of devbox. For example, the following line in a
~/.bashrcwill trigger an error if the devbox doesn't have Go installed:This is because we're adding the ORIGINAL_PATH at the end of their shell init script, which means that anything before that only see Nix packages.
Fix this by adding a pre-init hook that lets us restore the PATH before the user's rcfile, while still forcing Nix packages to come first in the post-init hook.
How was it tested?
Added a command that's only available outside of devbox to my ~/.bashrc and made sure it was called successfully.