NIX_PATH: don't prepend $HOME-based value in session variable, set later #45400
Conversation
environment.sessionVariables cannot refer to the values of env vars, and as a result this has caused problems in a variety of scenarios. One use for these is that they're injected into /etc/profile, elewhere these are used to populate an 'envfile' for pam (`pam 5 pam_env.conf`) which mentions use of HOME being potentially problematic. Anyway if the goal is to make things easier for users, simply do the NIX_PATH modification as extraInit. This fixes the annoying problems generated by the current approach (NixOS#40165 and others) while hopefully serving the original goal. One way to check if things are borked is to try: $ sudo env | grep NIX_PATH Which (before this change) prints NIX_PATH variable with an unexpanded $HOME in the value. ------- This does mean the following won't contain user channels for 'will': $ sudo -u will nix-instantiate --eval -E builtins.nixPath However AFAICT currently they won't be present either, due to unescaped $HOME. Unsure if similar situation for other users of sessionVariables (not sudo) work with current situation (if they exist they will regress after this change AFAIK).
| @@ -446,6 +445,8 @@ in | |||
| if [ "$USER" != root -o ! -w /nix/var/nix/db ]; then | |||
| export NIX_REMOTE=daemon | |||
| fi | |||
| '' + '' | |||
| export NIX_PATH="$HOME/.nix-defexpr/channels''${NIX_PATH:+:$NIX_PATH}" | |||
There was a problem hiding this comment.
Are these two single quotes necessary here?
There was a problem hiding this comment.
Within the double-single-quote they escape the dollar-sign ( '' ... ``${ .. ''), if you're asking about the two single quotes in the middle of the line? Or are you suggesting dropping the outer '' in favor of something else? I'm a fan of using '' in most places for consistency if nothing else, but if there's a reason that's bad or there's a better alternative please LMK! :)
There was a problem hiding this comment.
Agreed. Not sure what you mean about 'test -e' but happy if you want to just add a commit doing so? :D
There was a problem hiding this comment.
Well nix shows a warning if NIX_PATH entries don't exist, so only extend it if ~/.nix-defexpr exists.
if [ -e "$HOME/.nix-defexpr/channels" ]; then
export NIX_PATH="$HOME/.nix-defexpr/channels''${NIX_PATH:+:$NIX_PATH}"
fiThere was a problem hiding this comment.
Oh, right. Thanks, updated!
There was a problem hiding this comment.
Unfortunately this means the user needs to log out before the NIX_PATH gets updated after they add their first channel, but this is an improvement.
|
Ping! Can we gather some examples/motivation for the current state of things? Ideally we can find a way to fix this breakage while still addressing those concerns...? |
|
Should this be merged or are we waiting for something? Anyone have suggestions for how to proceed? Tests to run, people to check with? Thanks! |
Per reviewer comment (thanks!).
environment.sessionVariables cannot refer to the values of env vars,
and as a result this has caused problems in a variety of scenarios.
One use for these is that they're injected into /etc/profile,
elewhere these are used to populate an 'envfile' for pam
(
pam 5 pam_env.conf) which mentions use of HOME beingpotentially problematic.
Anyway if the goal is to make things easier for users,
simply do the NIX_PATH modification as extraInit.
This fixes the annoying problems generated by the current approach
(#40165 and others) while hopefully serving the original goal.
One way to check if things are borked is to try:
$ sudo env | grep NIX_PATH
Which (before this change) prints NIX_PATH variable with
an unexpanded $HOME in the value.
This does mean the following won't contain user channels for 'will':
$ sudo -u will nix-instantiate --eval -E builtins.nixPath
However AFAICT currently they won't be present either,
due to unescaped $HOME. Unsure if similar situation for other users
of sessionVariables (not sudo) work with current situation
(if they exist they will regress after this change AFAIK).
Tired of seeing warning, been running this change locally for a week
and seems to work for my machines. Comments/testing appreciated.
sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)nix path-info -Sbefore and after)