Include some more shell-specific vars, remove nix shell hook #635
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
savil
approved these changes
Feb 15, 2023
internal/impl/devbox.go
Outdated
| env[name] = os.Getenv(name) | ||
| // Copy over (and overwrite) vars that we explicitly "leak", as well as DEVBOX_ vars. | ||
| for _, kv := range os.Environ() { | ||
| parts := strings.SplitN(kv, "=", 2) |
There was a problem hiding this comment.
key, val, found := strings.Cut(kv, "=")
if !found {
return errors.Errorsf("expected = in kv: %s", kv)
}
| } | ||
| } | ||
|
|
||
| // These variables are only needed for shell, but we include them here in the computed env | ||
| // for both shell and run in order to be as identical as possible. | ||
| env["NIX_PKGS_ALLOW_UNFREE"] = "1" // Only for shell because we don't expect nix calls within run |
There was a problem hiding this comment.
I think its good to include, because someone's script may do devbox add for them.
| // TODO: add shell-specific vars, including: | ||
| // - NIXPKGS_ALLOW_UNFREE=1 (not needed in run because we don't expect nix calls there) | ||
| // - __ETC_PROFILE_NIX_SOURCED=1 (not needed in run because we don't expect rc files to try to load nix profiles) | ||
| // - HISTFILE (not needed in run because it's non-interactive) |
There was a problem hiding this comment.
would it be good to maintain somewhere a list of env vars like HISTFILE that are deliberately omitted from devbox run but included in devbox shell?
There was a problem hiding this comment.
maybe in LeakedVarsForShell with a value of false ?
There was a problem hiding this comment.
Well, I think looking at shellrc will be good enough for now (there's no other place in which the env can be changed)
ipince
force-pushed
the
rodrigo/shell-vars
branch
from
71481b5 to
a1de3c8
Compare
savil
added a commit
that referenced
this pull request
Feb 23, 2023
## Summary #635 hid the shell hook from `shell.nix` behind the unified-env flag (where shell hook is active when flag is OFF). So, this PR removes it from `flake.nix`, since flakes depend on unified-env being turned on. ## How was it tested? opened `devbox shell` in `examples/testdata/go/go-1.19` project
savil
added a commit
that referenced
this pull request
Jun 14, 2024
## Summary This was introduced in #635 (cc @ipince). I don't think its needed anymore. It seems to be used in the `/nix/var/nix/profiles/default/etc/profile.d/nix-daemon.sh` script, at the very top to prevent re-execution: ``` 1 # Only execute this file once per shell. 2 if [ -n "${__ETC_PROFILE_NIX_SOURCED:-}" ]; then return; fi 3 __ETC_PROFILE_NIX_SOURCED=1 4 ``` Perhaps it has other use-cases as well? RFC. From my casual usage of a Devbox binary built with this change, it all seems to still work fine. ## How was it tested?
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
Add some more variables that are required for shell: DEVBOX_ vars, a couple hard-coded nix vars, and a few more vars to leak.
Also removes the
shell.nixhook, which we no longer need if we're using unified env.How was it tested?
go tests