pyenv / pyenv-installer

This tool is used to install `pyenv` and friends.
MIT License
3.96k stars 428 forks source link

Suggest recommending editing of .profile, not .bashrc #97

Closed rbtcollins closed 3 years ago

rbtcollins commented 4 years ago

~/.bashrc is invoked on every new shell, so the path changes are additive, whereas ~/.profile's contents only run once per session, which is why most PATH changes are done to ~/.profile.

When pyenv is added to the path in ~/.bashrc, running 4 or five nested bash shells will result in a path like this:

/home/robertc/.pyenv/plugins/pyenv-virtualenv/shims:/home/robertc/.pyenv/shims:/home/robertc/.pyenv/bin:/home/robertc/.pyenv/plugins/pyenv-virtualenv/shims:/home/robertc/.pyenv/shims:/home/robertc/.pyenv/bin:/home/robertc/.pyenv/plugins/pyenv-virtualenv/shims:/home/robertc/.pyenv/shims:/home/robertc/.pyenv/bin:/home/robertc/.pyenv/plugins/pyenv-virtualenv/shims:/home/robertc/.pyenv/shims:/home/robertc/.pyenv/bin:/home/robertc/.cargo/bin:/home/robertc/bin:/home/robertc/.pyenv/plugins/pyenv-virtualenv/shims:/home/robertc/.pyenv/shims:/home/robertc/.pyenv/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games:/snap/bin

Which is clearly undesirable - in extreme cases this will cause errors when the environment block becomes exhausted, and even before then will cause performance issues as the same dir is searched again and again for commands.

joshfriend commented 4 years ago

thanks for the suggestion, that sounds good to me.

wouldn't you also want things loaded for login shells, in which case .bash_profile would need to be udpated instead of .profile?

rbtcollins commented 4 years ago

I suspect there is some per-distribution variation here, but at least for Debian and Ubuntu, the default is ~/.profile for login shells, whereas ~/.bash_profile is for bash-specificisms for login shells and typically not used at all.

e.g. from the bash man page:

              Do not read either the system-wide startup file /etc/profile or any of the personal initialization files ~/.bash_profile, ~/.bash_login, or ~/.profile.  By default, bash reads these files when it is invoked as a login shell (see INVOCATION below).

In rustup we only edit ~/.profile and the issues we run into are a) folk not understanding that they need to set the PATH directly for their current session and b) zsh and other off the beaten path shells.

faraz5040 commented 4 years ago

Woudn't it be better to check if it's already in PATH? Like this:

PYENV_PATH="$HOME/.pyenv/bin"
grep -E -q "(^|:)$PYENV_PATH($|:)" && PATH="$PATH:$PYENV_PATH"
rbtcollins commented 4 years ago

@alireza4050 doing it without invoking a subprocess would be ok I guess. I think the Google SDK has some array glue that does that in a nifty fashion.

workingjubilee commented 4 years ago

The following trick works to "grep" PATH without a subshell (afaik?) and is part of https://github.com/rust-lang/rustup/pull/2387

#!/bin/sh
# rustup shell setup
# affix colons on either side of $PATH to simplify matching
case ":${PATH}:" in
    *:"${cargo_bin}":*)
        ;;
    *)
        # Prepending path in case a system-installed rustc must be overwritten
        export PATH="${cargo_bin}:${PATH}"
        ;;
esac
native-api commented 3 years ago

The instructions have been moved into Pyenv's codebase and fixed in https://github.com/pyenv/pyenv/pull/1920