nix: inherit all environment variables in shell #706
Conversation
Before release v0.4.0, devbox handled environment variables differently depending on the command: - `devbox shell` would start with a clean environment. Nothing was inherited from the parent shell (but shellrc files would still be sourced) - `devbox shell -- cmd` would inherit the environment. - `devbox run` would inherit the environment. With v0.4.0, all commands adopted the `devbox shell` behavior of starting with a clean environment. However, this broke some existing users and the workaround of manually specifying environment variables to inherit would be cumbersome. As discussed, this change flips the behavior such that all commands will now inherit their parent shell's environment. Eventually there will be a flag to enable an "isolated" mode to restore the previous behavior. ## Changes to `devbox shell` and `devbox run` environments Devbox environments are now built with environment variables from the following steps: 1. Variables exported in the user's `~/.bashrc`, `~/.zshrc`, etc. 2. Variables from the current environment except for PWD, OLDPWD, SHLVL, and SHELL. 3. Variables from the `nix print-dev-env` environment except for BASHOPTS, HOME, NIX_BUILD_TOP, NIX_ENFORCE_PURITY, NIX_LOG_FD, NIX_REMOTE, PPID, SHELL, SHELLOPTS, TEMP, TEMPDIR, TERM, TMP, TMPDIR, TZ, and UID. The SSL_CERT_FILE variable is also ignored if it's set to `/no-cert-file.crt`. 4. Variables from Devbox plugins. 5. PATH is set to the concatenation of the PATHs from step 4, step 3, and step 2 (in that order). The PATH from the user's init files in step 1 is overwritten. The final PATH is cleaned and deduplicated. 5. HISTFILE is set to a Devbox project-specific file. 6. PS1 is set to "(devbox) $PS1" Each step adds to the previous step, overwriting any keys that already exist. ## Bug fixes and improvements - As suggested by @ipince, `shellrc.tmpl` simply sources the user's rcfile instead of copying it. - The result of `PrintEnv` now escapes all environment variable values. This means that values such as `FOO=$(echo bar)` will become `FOO="\$(echo bar)"` instead of being evaluated by the shell. Allowing variables and command substitution would require implementing a full parser to determine which characters to escape. - Some unquoted variables in `shellrc.tmpl` are now quoted so that devbox directories with spaces work. - The `cd` commands in `shellrc.tmpl` exit on failure to prevent running further commands in the wrong directory.
…tis/impure-shell
| // Devbox may change the working directory of the shell, so using the | ||
| // original PWD and OLDPWD would be wrong. | ||
| "PWD": true, | ||
| "OLDPWD": true, |
There was a problem hiding this comment.
what scenario leads to working directory changing?
I thought doing devbox --config=/path/to/config shell would still start you in the current directory but using the devbox.json of that config path. But maybe there's some other scenario I'm not considering.
There was a problem hiding this comment.
I think it's primarily plugin + init hooks that always run from the devbox.json directory. So if you're in devbox/internal and run devbox shell, the shell changes to the parent directory, runs the init hooks, then switches back.
| if !seen[path] { | ||
| cleaned = append(cleaned, path) | ||
| } | ||
| seen[path] = true |
There was a problem hiding this comment.
below: filepath.Join(cleaned...)
There was a problem hiding this comment.
Are you referring to line 638? That's joining on the list separator (:) instead of the path separator.
|
@savil the original motivation was to properly handle variables with special characters. We were previously doing FOO="$(ls "dir 01")" # lists a directory named `dir 01`and then you only escape quotes, you get: FOO="$(ls \"dir 01\")" # lists a directory named `"dir 01"`Because quotes within a command substitution shouldn't be escaped, we'd need to implement a parser. Instead, if we just say all env var values need to be literals then it becomes easy. We just do: FOO="\$(ls \"dir 01\")" # doesn't list anything, value is `$(ls "dir 01")` |
…tis/impure-shell
## Summary * Applies the same changes from #706 to the fish shellrc. * Ignores the `_` variable. It's read-only and `fish` complains if we try to write it. * Added a note in `shellrc` so we remember do edit `shellrc_fish` too. If we had tests for fish we could remove this silly note. * Alphabetized vars so the generated shellrc is a bit cleaner (imo). Fixes #719 ## How was it tested? ``` SHELL=fish ./devbox shell ``` I'll add some shellrc_fish tests in a future PR
This is a long description, but I wanted to document the exact changes being made to Devbox environments for reference.
Context
Before release v0.4.0, devbox handled environment variables differently depending on the command:
devbox shellwould start with a clean environment. Nothing was inherited from the parent shell (but shellrc files would still be sourced)devbox shell -- cmdwould inherit the environment.devbox runwould inherit the environment.With v0.4.0, all commands adopted the
devbox shellbehavior of starting with a clean environment. However, this broke some existing users and the workaround of manually specifying environment variables to inherit would be cumbersome. As discussed, this change flips the behavior such that all commands will now inherit their parent shell's environment. Eventually there will be a flag to enable an "isolated" mode to restore the previous behavior.Changes to
devbox shellanddevbox runenvironmentsDevbox environments are now built with environment variables from the following steps:
~/.bashrc,~/.zshrc, etc.nix print-dev-envenvironment except for BASHOPTS, HOME, NIX_BUILD_TOP, NIX_ENFORCE_PURITY, NIX_LOG_FD, NIX_REMOTE, PPID, SHELL, SHELLOPTS, TEMP, TEMPDIR, TERM, TMP, TMPDIR, TZ, and UID. The SSL_CERT_FILE variable is also ignored if it's set to/no-cert-file.crt.Each step adds to the previous step, overwriting any keys that already exist.
Bug fixes and improvements
shellrc.tmplsimply sources the user's rcfile instead of copying it.PrintEnvnow escapes all environment variable values. This means that values such asFOO=$(echo bar)will becomeFOO="\$(echo bar)"instead of being evaluated by the shell. Allowing variables and command substitution would require implementing a full parser to determine which characters to escape.shellrc.tmplare now quoted so that devbox directories with spaces work.cdcommands inshellrc.tmplexit on failure to prevent running further commands in the wrong directory.