Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Profile scripts add duplicate entries to PATH in subshells #5950

Open
lilyball opened this issue Jan 20, 2022 · 17 comments
Open

Profile scripts add duplicate entries to PATH in subshells #5950

lilyball opened this issue Jan 20, 2022 · 17 comments
Labels

Comments

@lilyball
Copy link
Member

Describe the bug

The Nix profile scripts unconditionally add directories to PATH without checking if those directories are already there. The multi-user profile script has a guard variable to prevent it from running multiple times, but this guard variable is not exported and so does not affect subshell behavior. The single-user profile script doesn't even have a guard variable.

The effect of this is that running an interactive subshell (if multi-user, or a login subshell if single-user) will source the profile script and add the directories to PATH again even though they're already there.

Steps To Reproduce

  1. Install Nix in an existing system
  2. Spawn a new shell. Verify that $PATH includes your nix profile bin directories.
  3. Spawn a subshell (a login subshell if you have a single-user install)
  4. Check what $PATH contains

Expected behavior

$PATH should contain your nix profile bin directories exactly once in both the parent shell and the subshell.

Additional context

I have not actually reproduced this problem locally, as I am not currently set up to test standalone Nix installs (the machines I have access to use either NixOS or nix-darwin and I'm not prepared to run a VM at the moment). But this can be verified by reading the profile scripts, and this issue was originally reported to me at lilyball/nix-env.fish#11 in my fish plugin that imports the bash profile.

One potential fix is to just export the __ETC_PROFILE_NIX_SOURCED guard variable (and add this to the single-user profile too as that doesn't even have a guard variable right now), but I don't think that's a good fix as sudo will preserve PATH but not the other variables (and especially not the guard variable) and so running an interactive or login shell via sudo would still add the duplicate PATH entries.

Given that, I think the best approach is to just make sure the profile script is idempotent. I should be able to source it multiple times in a row (unsetting the guard variable each time) and end up with the same environment that I do if I only source it once. The guard variable still has a purpose with this approach, both as an optimization and to prevent re-sourcing the profile from blowing away any subsequent environment modifications (the installer sources the profile from potentially multiple files involved in shell initialization, and I might update the earliest such file to change the environment and so the subsequent re-sourcing of the profile shouldn't blow that away). Making the script idempotent simply requires making it check if the entries are in PATH before setting them, as all other environment changes it does are already idempotent, though it's probably also worth putting a comment in the file noting that it should be idempotent as a caution for anyone modifying it in the future.

@n8henrie
Copy link
Contributor

A little relevant discussion (and my current PATH-deduplication workarounds): #5298

@SuperSandro2000
Copy link
Member

I solved it with two lines in my bashrc which work universally:

DEDUPE_PATH="$(printf %s "$PATH" | awk -v RS=: '{ if (!arr[$0]++) {printf("%s%s",!ln++?"":":",$0)}}')"
export PATH=$DEDUPE_PATH

@n8henrie
Copy link
Contributor

n8henrie commented Feb 4, 2022

Pretty clever! The ternary in the printf confused me for a bit.

I put a similar deduplicator in the same thread, though I didn't care for the extra ms added to my shell spawn time.

In my testing you can shave off a ms or so by using a herestring instead of piping from printf (and I've added a little more spacing, which helped me understand what was going on):

awk -v RS=: '{ if (!arr[$0]++) { printf("%s%s", !ln++ ? "" : ":", $0) }}' <<<"${PATH}"

Also, the version I'm using is no slower and included for comparison:

$ cat run.sh
#!/bin/bash

orig() {
  printf %s "${PATH}" | awk -v RS=: '{ if (!arr[$0]++) {printf("%s%s",!ln++?"":":",$0)}}'
}

herestring() {
  awk -v RS=: '{ if (!arr[$0]++) {printf("%s%s",!ln++?"":":",$0)}}' <<< "${PATH}"
}

mine() {
  env --ignore-environment HOME="${HOME}" sh -c '
  source /etc/static/bashrc
  echo "${PATH}"
  '
}

if [[ -n "$(herestring)" ]] && [[ "$(orig)" = "$(herestring)" ]]; then
  export -f orig herestring mine
  hyperfine -m 500 orig
  hyperfine -m 500 herestring
  hyperfine -m 500 mine
fi
$ ./run.sh
Benchmark 1: orig
  Time (mean ± σ):       4.5 ms ±   0.5 ms    [User: 1.9 ms, System: 1.3 ms]
  Range (min … max):     3.8 ms …   8.4 ms    500 runs
...
Benchmark 1: herestring
  Time (mean ± σ):       3.8 ms ±   0.4 ms    [User: 1.9 ms, System: 1.3 ms]
  Range (min … max):     3.3 ms …   7.2 ms    500 runs
...
Benchmark 1: mine
  Time (mean ± σ):       3.7 ms ±   0.4 ms    [User: 0.9 ms, System: 1.3 ms]
  Range (min … max):     2.4 ms …   8.6 ms    500 runs

@lilyball
Copy link
Member Author

lilyball commented Feb 4, 2022

You could try checking SHLVL and only doing this if it's >1. That way it won't run on your initial login shell.

@n8henrie That said, if you're using nix-darwin, you shouldn't be sourcing the nix profile script at all. Your /etc/bashrc should be overwritten by nix-daemon to be a symlink to /etc/static/bashrc, which sets up your environment appropriately. And child shells should then skip the PATH setup (using __NIX_DARWIN_SET_ENVIRONMENT_DONE). And if you wipe that var and run a child shell it will just replace PATH entirely anyway.

So if you're sourcing the nix profile separately you should clean that up. If you had a single-user install on your machine then it probably added the single-user nix profile to your .bash_profile, .bash_login, or .profile scripts, as well as your .zshenv or .zshrc scripts (if these existed). The multi-user installer will have instead modified /etc/bashrc, which nix-darwin would have replaced.

This is of course assuming that programs.bash.enable is set (which it is by default). Zsh configuration would be programs.zsh.enable but that's actually off by default, so if you are using Zsh then you'll want to turn that on.

@n8henrie
Copy link
Contributor

n8henrie commented Feb 4, 2022

That said, if you're using nix-darwin, you shouldn't be sourcing the nix profile script at all.

Thanks @lilyball -- I don't think I am: #5298 (comment)

I'm sourcing /etc/static/bashrc directly.

I use tmux, and my .bashrc sets up my PATH (synced across multiple distros). Because it appends to PATH, it ends up with an increasingly long PATH every time it gets sourced. My workaround for this for years was to start with PATH=$(getconf PATH) at the beginning of .bashrc, so it builds it up from scratch.

EDIT: For example, .bashrc gets sourced when I open Terminal.app, then again when I launch tmux, and occasionally when I make changes to .bashrc and re source it. My env (including __NIX_DARWIN_SET_ENVIRONMENT_DONE) gets passed along which mucks things up with this approach.

This led to obvious problems with nix and nix-darwin trying to decide whether or not the PATH was correct by looking at __NIX_DARWIN_SET_ENVIRONMENT_DONE and such (which seemed like an odd decision anyway -- why not just look at PATH to see if PATH is correct)? More details in #5298 if interested.

In the end, sourcing /etc/static/bashrc at the beginning of my .bashrc and having it reset my path seems like a reasonable replacement for PATH=$(getconf PATH).

¯\_(ツ)_/¯

@lilyball
Copy link
Member Author

lilyball commented Feb 4, 2022

I'm also curious about the speed of doing a bash-native deduplication, something like

newpath=:
while read -d : path; do
	if [[ $newpath != *:$path:* ]]; then
		newpath+=$path:
	fi
done <<<"$PATH:"
newpath=${newpath#:}
PATH=${newpath%:}

This won't stop the profile script from potentially moving the nix paths in front of anything else of course, but it should behave like the awk script except without spawning any additional processes.

That said, we really should just fix the profile scripts to be idempotent.

@n8henrie
Copy link
Contributor

n8henrie commented Feb 4, 2022

Good call!

$ cat run.sh
───────┬──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
       │ File: run.sh
───────┼──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
   1   │ #!/bin/bash
   2   │
   3   │ orig() {
   4   │   printf %s "${PATH}" | awk -v RS=: '{ if (!arr[$0]++) {printf("%s%s",!ln++?"":":",$0)}}'
   5   │ }
   6   │
   7   │ herestring() {
   8   │   awk -v RS=: '{ if (!arr[$0]++) {printf("%s%s",!ln++?"":":",$0)}}' <<< "${PATH}"
   9   │ }
  10   │
  11   │ mine() {
  12   │   env --ignore-environment HOME="${HOME}" sh -c '
  13   │   source /etc/static/bashrc
  14   │   echo "${PATH}"
  15   │   '
  16   │ }
  17   │
  18   │ bashnative() {
  19   │   newpath=:
  20   │   while read -r -d : path; do
  21   │     if [[ "${newpath}" != *:"${path}":* ]]; then
  22   │       newpath+=${path}:
  23   │     fi
  24   │   done <<< "${PATH}:"
  25   │   newpath=${newpath#:}
  26   │   echo "${newpath%:}"
  27   │ }
  28   │
  29   │ if [[ -n "$(herestring)" ]] && [[ "$(orig)" = "$(herestring)" ]] && [[ "$(bashnative)" = "$(orig)" ]]; then
  30   │   export -f orig herestring mine bashnative
  31   │   hyperfine -m 500 orig
  32   │   hyperfine -m 500 herestring
  33   │   hyperfine -m 500 mine
  34   │   hyperfine -m 500 bashnative
  35   │ fi
───────┴──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
$ ./run.sh
Benchmark 1: orig
  Time (mean ± σ):       4.7 ms ±   0.3 ms    [User: 2.0 ms, System: 1.5 ms]
  Range (min … max):     3.9 ms …   7.5 ms    500 runs
...
Benchmark 1: herestring
  Time (mean ± σ):       4.1 ms ±   0.2 ms    [User: 2.0 ms, System: 1.3 ms]
  Range (min … max):     3.8 ms …   6.5 ms    500 runs
...
Benchmark 1: mine
  Time (mean ± σ):       3.7 ms ±   0.4 ms    [User: 1.0 ms, System: 1.4 ms]
  Range (min … max):     2.8 ms …   6.2 ms    500 runs
...
Benchmark 1: bashnative
  Time (mean ± σ):       1.7 ms ±   0.1 ms    [User: 1.9 ms, System: 0.5 ms]
  Range (min … max):     1.5 ms …   2.8 ms    522 runs
...

@stevecheckoway
Copy link

It seems this issue could be solved similar to how Cargo does it.

This line could become something like

case ":$PATH:" in
  *:"$HOME/.nix-profile/bin:@localstatedir@/nix/profiles/default/bin":*)
    ;;
  *)
    export PATH="$HOME/.nix-profile/bin:@localstatedir@/nix/profiles/default/bin:$PATH"
    ;;
esac

@SuperSandro2000
Copy link
Member

That way it won't run on your initial login shell.

For my usecase I also want to run it in the login shell because I already have duped PATHs at that level.

This won't stop the profile script from potentially moving the nix paths in front of anything else of course, but it should behave like the awk script except without spawning any additional processes.

Ordering for PATH is super important for reboot traps, safe-rm and sudo wrappers.

@n8henrie
Copy link
Contributor

My strategy of "starting PATH fresh" (described above) continues to be problematic.

  • on Raspbian, the default /etc/profile overwrites PATH (starts it from scratch)
  • on MacOS it does not, so I am using the strategies about to start fresh with each shell

On either of these, using nix shell nixpkgs#hello doesn't add hello to resulting shell's PATH -- it tries but is apparently scrubbed by sourcing ~/.bashrc after PATH is modified.

So I think the "forgiveness over permission" approach of cleaning duplicates at the end is probably a better idea, will try out @lilyball's recommendation.

@n8henrie
Copy link
Contributor

n8henrie commented Nov 28, 2022

By no longer creating the PATH from scratch (with a clean env) every time, it looks like I've now traded one problem for another:

$ which -a bash
/run/current-system/sw/bin/bash
/bin/bash
$ bash -c 'which -a bash'
/run/current-system/sw/bin/bash
/bin/bash
$ bash --login -c 'which -a bash'
/bin/bash
/run/current-system/sw/bin/bash

Login shells now "see" that the nix is already in the environment, and I think the MacOS path_helper is placing the system PATH before my nix stuff. I'm a heavy tmux user, which by default spawns a login shell, so inside tmux I'm getting e.g. MacOS's BSD sed instead of nix's GNU-sed. What a pain.

EDIT: Fix for now by having tmux not use a login shell (set -g default-command "${SHELL}").

@madsonic
Copy link

Another approach to the problem on macOS with zsh. I try to avoid manipulating PATH directly which gets quite hairy when when path_helper is involved.

  1. comment out the export PATH line
  2. prepend profile dir to /etc/paths e.g.
/Users/foo/.nix-profile/bin
/nix/var/nix/profiles/default/bin
...

This works well even with tmux. my $PATH are the same in terminal on startup and in tmux. no duplicates

/Users/foo/.nix-profile/bin:/nix/var/nix/profiles/default/bin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/MacGPG2/bin

path_helper is invoked in /etc/zprofile, its behaviour is documented here. Some of its behaviour are not documented but from my testing I believe the constructed PATH follows this format

</etc/paths>:</etc/paths.d/>:<all other paths in current $PATH>

for example, given an existing PATH /sbin:/usr/local/foo/bin:/baz and the following

cat /etc/paths
/sbin

tree /etc/paths.d
.
└── /usr/local/foo/bin

when a shell is spawned the resulting $PATH is

/sbin:/usr/local/foo/bin:/baz

Perhaps the way to fix the dup path issue is to utilise path_helper by adding nix paths to /etc/paths during the installation phase.

@steerio
Copy link

steerio commented Jan 19, 2024

I used the Arch Linux package, so your mileage may vary, but this works for me. You may want to check your /etc/profile.d/nix-daemon.* files whether they use this environment variable.

In your .tmux.conf:

set-environment -g __ETC_PROFILE_NIX_SOURCED 1

In your Bash or Zsh profile:

unset __ETC_PROFILE_NIX_SOURCED

Why the profile.d script doesn't check the existence of $NIX_PROFILES instead is beyond me.

@carschandler
Copy link

carschandler commented Feb 19, 2024

This is a pretty big issue in my opinion. If I have a python environment loaded up that I want to be persisted through an entire tmux session, I can't do that because PATH will just be overwritten with my nix-profile path at the front as soon as the new shell spawns, containing the global python instance I have defined rather than the one I just had loaded up.

@carschandler
Copy link

A bash-native workaround is to add this to a home-manager config:

programs.bash.bashrcExtra = ''
NIX_PATHS="$HOME/.nix-profile/bin:/nix/var/nix/profiles/default/bin:"
NEWPATH=''${PATH/$NIX_PATHS}
while [[ $NEWPATH =~ $NIX_PATHS ]]; do
  PATH=$NEWPATH
  NEWPATH=''${NEWPATH/$NIX_PATHS}
done
''

Simply removes the paths string until it's gone, leaving the result with only one match as the PATH.

@chaoky
Copy link

chaoky commented Apr 26, 2024

similar problem happens in zsh,
nix-daemon.sh keeps prepending to path,
causing other /nix/ paths (set by direnv) to not have priority on new shells

added this as a workarround in zshrc

NIX_PATHS=$(echo $PATH | tr ':' '\n' | grep "/nix/" | tail -n +2 | tr '\n' ':')
if [[ $NIX_PATHS ]]; then
  PATH=$NIX_PATHS$PATH
fi

@ramboman
Copy link
Contributor

ramboman commented May 30, 2024

I have duplicated nix bin paths as well. There are several criterias my solution has to respect while solving the problem:

  • Nix bin paths has to come after $HOME/* paths
  • Should only modify nix bin paths. The solution is meant to fix the nix bin path problem only, so it should not do anything beyond that.
  • Compatible with Bourne shell and other compatible shells. nix-daemon.sh is sourced in /etc/profile/nix.sh, which is sourced in /etc/profile, which needs to be compatible with Bourne shell and other compatible shells.
  • Use NIX_PROFILES from nix-daemon.sh to create nix bin paths.

Here is my solution when placed in a separated script (nix_paths_idempotent.sh):

#!/bin/sh

keep_chosen_lines_unique () {
    # stdin: lines
    local chosen_lines=$1

    local awk_code='
    BEGIN {
    '"$( \
        echo "$chosen_lines" \
        | tr ' ' '\n' \
        | sed 's|\(.*\)|is_presents["\1"]="false"|')"'
    } {
        is_present=is_presents[$0]
        if (length(is_present) > 0) {
            if (is_present == "false") {
                print $0
                is_presents[$0]="true"
            }
        } else {
            print $0
        }
    }
    '
    # stdin
    awk "$awk_code"
}

match () {
    local pattern=$1
    local string=$2

    eval "
    case \"\$string\" in
        $pattern) return 0 ;;
    esac"
    return 1
}

insert_before () {
    # stdin: lines
    local section=$1
    local pattern=$2

    while read line; do
        if match "$pattern" "$line"; then
            echo "$section"
            echo "$line"
            break
        else
            echo "$line"
        fi
    done # stdin
    cat # stdin
}

get_nix_bin_paths () {
    local nix_profiles=$1

    if [ -z "${nix_profiles}" ]; then
        return 0
    fi
    local output=$( \
        echo "x $nix_profiles" \
        | tr ' ' '\n' \
        | tac \
        | sed 's|$|/bin|' \
        | tr '\n' ':')
    echo "${output%:x/bin:}"
}

insert_unique_paths_after_home () {
    local paths=$1
    local new_paths=$2

    if [ -n "${paths}" ]; then
        if [ -n "${new_paths}" ]; then
            local new_lines=$( \
                echo "$new_paths" \
                | tr ':' '\n' \
                | tac)
            local output=$( \
                echo "${new_paths}:${paths}:x" \
                | tr ':' '\n' \
                | tac \
                | insert_before "$new_lines" "$HOME/*" \
                | keep_chosen_lines_unique "$new_lines" \
                | tac \
                | tr '\n' ':')
            echo "${output%:x:}"
        else
            echo "$paths"
        fi
    else
        if [ -n "${new_paths}" ]; then
            echo "$new_paths"
        fi
    fi
}

test_insert_unique_paths_after_home () {
    local nix_profiles=$1
    local paths=$2

    local nix_bin_paths=$(get_nix_bin_paths "$nix_profiles")
    local output=$(insert_unique_paths_after_home "$paths" "$nix_bin_paths")

    echo "input:"
    echo "$paths"
    echo
    echo "output:"
    echo "$output"
    echo
}

test_all () {
    local nix_profiles="/nix/var/nix/profiles/default $HOME/.nix-profile"

    echo "Empty PATH"
    test_insert_unique_paths_after_home "$nix_profiles" ""
    echo

    echo "PATH without $HOME/* + trailing empty paths"
    test_insert_unique_paths_after_home "$nix_profiles" \
        "/usr/local/bin:/usr/bin:/bin:/usr/local/games:/usr/games:::"
    echo

    echo "PATH with repeated $HOME/bin + trailing empty paths"
    test_insert_unique_paths_after_home "$nix_profiles" \
        "$HOME/bin:/usr/local/bin:$HOME/bin:/usr/bin:/bin:/usr/local/games:/usr/games:::"
    echo

    echo "PATH with repeated $HOME/bin + trailing empty paths + repeated nix bin paths"
    local nix_bin_paths=$(get_nix_bin_paths "$nix_profiles")
    test_insert_unique_paths_after_home "$nix_profiles" \
        "$nix_bin_paths:$HOME/bin:/usr/local/bin:$HOME/bin:$nix_bin_paths:/usr/bin:/bin:/usr/local/games:/usr/games:::"
    echo
}

"$@"

insert_unique_paths_after_home is the function that keeps the PATH idempotent while adding nix bin paths.

To test do:

$ ./nix_paths_idempotent.sh test_all

Here is my result:

Empty PATH
input:


output:
/home/user/.nix-profile/bin:/nix/var/nix/profiles/default/bin


PATH without /home/user/* + trailing empty paths
input:
/usr/local/bin:/usr/bin:/bin:/usr/local/games:/usr/games:::

output:
/home/user/.nix-profile/bin:/nix/var/nix/profiles/default/bin:/usr/local/bin:/usr/bin:/bin:/usr/local/games:/usr/games:::


PATH with repeated /home/user/bin + trailing empty paths
input:
/home/user/bin:/usr/local/bin:/home/user/bin:/usr/bin:/bin:/usr/local/games:/usr/games:::

output:
/home/user/bin:/usr/local/bin:/home/user/bin:/home/user/.nix-profile/bin:/nix/var/nix/profiles/default/bin:/usr/bin:/bin:/usr/local/games:/usr/games:::


PATH with repeated /home/user/bin + trailing empty paths + repeated nix bin paths
input:
/home/user/.nix-profile/bin:/nix/var/nix/profiles/default/bin:/home/user/bin:/usr/local/bin:/home/user/bin:/home/user/.nix-profile/bin:/nix/var/nix/profiles/default/bin:/usr/bin:/bin:/usr/local/games:/usr/games:::

output:
/home/user/bin:/usr/local/bin:/home/user/bin:/home/user/.nix-profile/bin:/nix/var/nix/profiles/default/bin:/usr/bin:/bin:/usr/local/games:/usr/games:::

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

9 participants