[Nix#9441] Assorted peculiarities of the various `nix-profile{,-daemon}.{,fi}sh` files #96

New issue

Open

opened 2024-03-16 06:44:53 +00:00 by lix-bot · 0 comments

lix-bot commented

2024-03-16 06:44:53 +00:00

Member

Upstream-Issue: NixOS/nix#9441

Recently, a patch was introduced to add Nix's paths to XDG_DATA_DIRS: https://github.com/NixOS/nix/pull/8985
This had an oversight, which was fixed in a PR: https://github.com/NixOS/nix/pull/9312
...which also had an oversight which was released as 2.19.0, and fixed in a PR: https://github.com/NixOS/nix/pull/9425

This reminded me of all the reasons the Determinate Nix Installer was created, and inspired me to examine the profile scripts and experiment with porting it to Rust.

Note that this isn't intended to be a "call-out" thread where I drag people through the mud or whatever. I probably introduced several of these personally! This code was written by smart people, trying to do the right thing, but the challenges of scripting languages makes it hard to do it right.

I identified the following inconsistencies:

nix-profile-daemon.{fish,sh} protect against double-loading but nix-profile.{fish,sh} don't:

if [ -n "${__ETC_PROFILE_NIX_SOURCED:-}" ]; then return; fi
__ETC_PROFILE_NIX_SOURCED=1

if test -n "$__ETC_PROFILE_NIX_SOURCED"
exit
end
set __ETC_PROFILE_NIX_SOURCED 1

nix-profile-daemeon.fish sometimes leaks the add_path function, because it registers the function and then detects if Nix was sourced already:

function add_path --argument-names new_path
  # ...snip...
end

# Only execute this file once per shell.
if test -n "$__ETC_PROFILE_NIX_SOURCED"
  exit
end

# ...snip...

functions -e add_path

nix-profile.{fish,sh} check that HOME is defined before using it, but nix-profile-daemon.{fish,sh} don't, exposing users with set -u to a potential crash in the most common use case:
```
if [ -n "$HOME" ] && [ -n "$USER" ]; then
```
nix-profile.{fish,sh} requires that USER is defined, despite never using it. Likely leftover from a refactor around how GC roots were configured.
nix-profile{-daemon}.sh were updated to account for the XDG directory migration, but the Fish equivalent weren't:
```
NIX_LINK="$HOME/.nix-profile"
if [ -n "${XDG_STATE_HOME-}" ]; then
    NIX_LINK_NEW="$XDG_STATE_HOME/nix/profile"
else
    NIX_LINK_NEW="$HOME/.local/state/nix/profile"
fi
```
vs. the naive Fish:
```
set --export NIX_PROFILES "@localstatedir@/nix/profiles/default $HOME/.nix-profile"
```
By way of note, the XDG migration included a useful, user-forward migration path for users who had both a legacy and an XDG-based path. It was made defunct by a logical inversion by mistake,

nix-profile.sh, nix-profile.fish, nix-profile-daemon.sh all include the user's Nix profile in the XDG_DATA_DIRS, but nix-profile-daemon.fish does not:

set --export XDG_DATA_DIRS "$XDG_DATA_DIRS:$NIX_LINK/share:/nix/var/nix/profiles/default/share"

vs.

set --export XDG_DATA_DIRS "$XDG_DATA_DIRS:/nix/var/nix/profiles/default/share"

The profile scripts typically make an attempt at leaving the NIX_SSL_CERT_FILE environment variable alone if the user set it, but...
- nix-profile.sh doesn't bother
- nix-profile.{fish,sh} look to see if NIX_SSH_CERT_FILE (note the H!) is set instead of NIX_SSL_CERT_FILE (note the L).
All but nix-profile-daemon.sh will check to see if $NIX_LINK/etc/ca-bundle.crt exists and use that.
nix-profile-daemon.{sh,fish} both check for a file called etc/ssl/certs/ca-bundle.crt in all the defined NIX_PROFILES, but nix-profile.{sh,fish} don't.
nix-profile.{sh,fish} will extend MANPATH if it is already set, but the -daemon scripts won't.
The nix-profile-daemon.{fish,sh} scripts put /nix/var/nix/profiles/default/bin into the PATH, but the others don't. This is true, despite all four setting up the default profile.

I don't know which of these are intentional or bugs, but since I found them during the Rust port I thought it would be useful food for thought!

Upstream-Issue: https://git.lix.systems/NixOS/nix/issues/9441 Recently, a patch was introduced to add Nix's paths to `XDG_DATA_DIRS`: https://github.com/NixOS/nix/pull/8985 This had an oversight, which was fixed in a PR: https://github.com/NixOS/nix/pull/9312 ...which also had an oversight which was released as 2.19.0, and fixed in a PR: https://github.com/NixOS/nix/pull/9425 This reminded me of all the reasons the Determinate Nix Installer was created, and inspired me to examine the profile scripts and experiment with porting it to Rust. Note that this isn't intended to be a "call-out" thread where I drag people through the mud or whatever. I probably introduced several of these personally! This code was written by smart people, trying to do the right thing, but the challenges of scripting languages makes it hard to do it right. I identified the following inconsistencies: 1. `nix-profile-daemon.{fish,sh}` protect against double-loading but `nix-profile.{fish,sh}` don't: ```sh if [ -n "${__ETC_PROFILE_NIX_SOURCED:-}" ]; then return; fi __ETC_PROFILE_NIX_SOURCED=1 ``` ```fish if test -n "$__ETC_PROFILE_NIX_SOURCED" exit end set __ETC_PROFILE_NIX_SOURCED 1 ``` 2. `nix-profile-daemeon.fish` sometimes leaks the `add_path` function, because it registers the function and then detects if Nix was sourced already: ```fish function add_path --argument-names new_path # ...snip... end # Only execute this file once per shell. if test -n "$__ETC_PROFILE_NIX_SOURCED" exit end # ...snip... functions -e add_path ``` 3. `nix-profile.{fish,sh}` check that `HOME` is defined before using it, but `nix-profile-daemon.{fish,sh}` don't, exposing users with `set -u` to a potential crash in the most common use case: ```sh if [ -n "$HOME" ] && [ -n "$USER" ]; then ``` 4. `nix-profile.{fish,sh}` requires that `USER` is defined, despite never using it. Likely leftover from a refactor around how GC roots were configured. 5. `nix-profile{-daemon}.sh` were updated to account for the XDG directory migration, but the Fish equivalent weren't: ```sh NIX_LINK="$HOME/.nix-profile" if [ -n "${XDG_STATE_HOME-}" ]; then NIX_LINK_NEW="$XDG_STATE_HOME/nix/profile" else NIX_LINK_NEW="$HOME/.local/state/nix/profile" fi ``` vs. the naive Fish: ```fish set --export NIX_PROFILES "@localstatedir@/nix/profiles/default $HOME/.nix-profile" ``` By way of note, the XDG migration included a useful, user-forward migration path for users who had both a legacy and an XDG-based path. It was made defunct by a logical inversion by mistake, 6. `nix-profile.sh`, `nix-profile.fish`, `nix-profile-daemon.sh` all include the user's Nix profile in the `XDG_DATA_DIRS`, but `nix-profile-daemon.fish` does not: ```fish set --export XDG_DATA_DIRS "$XDG_DATA_DIRS:$NIX_LINK/share:/nix/var/nix/profiles/default/share" ``` vs. ```fish set --export XDG_DATA_DIRS "$XDG_DATA_DIRS:/nix/var/nix/profiles/default/share" ``` 7. The profile scripts typically make an attempt at leaving the `NIX_SSL_CERT_FILE` environment variable alone if the user set it, but... * `nix-profile.sh` doesn't bother * `nix-profile.{fish,sh}` look to see if `NIX_SSH_CERT_FILE` (note the `H`!) is set instead of `NIX_SSL_CERT_FILE` (note the `L`). 8. All but `nix-profile-daemon.sh` will check to see if `$NIX_LINK/etc/ca-bundle.crt` exists and use that. 9. `nix-profile-daemon.{sh,fish}` both check for a file called `etc/ssl/certs/ca-bundle.crt` in all the defined `NIX_PROFILES`, but `nix-profile.{sh,fish}` don't. 10. `nix-profile.{sh,fish}` will extend `MANPATH` if it is already set, but the `-daemon` scripts won't. 11. The `nix-profile-daemon.{fish,sh}` scripts put `/nix/var/nix/profiles/default/bin` into the `PATH`, but the others don't. This is true, despite all four setting up the default profile. I don't know which of these are intentional or bugs, but since I found them during [the Rust port](https://github.com/DeterminateSystems/nix-installer/pull/738) I thought it would be useful food for thought!