Pyenv: Bug in fish configuration

[kennethreitz42]

the following:

set -x PATH "$pyenv_root/shims" $PATH
set -x PYENV_SHELL fish

(from ~/.config/fish/conf.d/pyenv.fish).

Should be changed to:

if status --is-login
    set -x PATH "$pyenv_root/shims" $PATH
    set -x PYENV_SHELL fish
end

This increases compatibility with subshells and tools like pipenv. Other shells should do this too, but let's start with fish.

[blueyed]

for reference: it comes from here: https://github.com/pyenv/pyenv/blob/005a2e8914ff4733bd28f20d000871feca98320e/libexec/pyenv-init#L87-L91

[kennethreitz42]

Ah thanks, I'll send a pull request there :)

[blueyed]

Should go through rbenv maybe then first: https://github.com/rbenv/rbenv/blob/2d7cefe78279f65a6aa3c6e5db1e5daffe934061/libexec/rbenv-init#L87-L90

Does fish have different config files for login vs. non-login shells (as with others)?

[kennethreitz42]

not that i'm aware of, just these lines are required.

[kennethreitz42]

https://github.com/rbenv/rbenv/pull/1038

[joshfriend]

This might be it:
https://github.com/fisherman/pyenv/blob/51a283d589e3e54207003da17671bd2988ffe3fa/conf.d/pyenv.fish#L15-L16

Notice set -x is used there and is what @kennethreitz was seeing, but pyenv-init did set -gx. Also, it comes from a file called conf.d/pyenv.fish which matches the above fisherman plugin.

[kennethreitz42]

ah, interesting.

In any case, these commands should only be run during a login session, not during every shell spawn.

[AlJohri]

@kennethreitz will this work for both macOS and other *nix systems? If I remember correctly, macOS starts each terminal session as interactive, login while standard *nix systems start terminal sessions as interactive, non-login.

if the command only runs for non-login sessions, it would fix child processes on macOS but break terminal sessions on other *nix systems?

I could be off - I don't have experience working with fish

ref: https://github.com/MikeMcQuaid/dotfiles/issues/4

[kennethreitz42]

I have only tested this with MacOS, so I'm not sure, actually. This is the first time I've heard this.

[AlJohri]

yeah, this is a good guide: https://github.com/rbenv/rbenv/wiki/Unix-shell-initialization#shell-modes

would love to solve this issue once and for all since I'm pretty sure it causes issues with subshells in envdir as well: https://github.com/jezdez/envdir/issues/62 cc @blueyed

any program that creates subshells using a python program installed via pyenv will end up with a slightly modified/incorrect PATH

[kennethreitz42]

Can set honor an environment variable maybe, like PYENV_SKIP_ACTIVATION, to skip these activations, if needed?

[AlJohri]

@kennethreitz I confirmed this bug exists for zsh and fish in linux as well. In addition, linux does indeed start every shell as a non-login shell. For some reason this bug doesn't exist for bash. https://github.com/AlJohri/test-pyenv-pipenv#test-pyenv-pipenv

@joshfriend @blueyed would you accept a PR for something like PYENV_SKIP_ACTIVATION which gets set on first pyenv activation preventing it from running in subshells? this would also fix #969

[kennethreitz42]

or PYENV_SKIP_ACTIVATION could be set by a smart program (e.g. pipenv) that launches subshells

[AlJohri]

okay, I figured out why bash still works. turns out @berdario (from pew) added a hack specific for bash which alters the bashrc file for the child process and edits the PATH in the last line: https://github.com/berdario/pew/blob/f10e80f85fddf5c3246d85224fe72af87cc92c92/pew/pew.py#L160

I'd still prefer something that fixes the problem more globally like PYENV_SKIP_ACTIVATION

[blueyed]

Thanks for tackling this.

pyenv's init could indeed export some marker that would prevent it from manipulating the PATH again I think.

=====

FWIW: the best is to setup pyenv in your profile, not rc file (which works around this, and prevents this issue):

~/.profile - sourced from ~/.zprofile (via emulate sh -c 'source ~/.profile'):

if [ -d ~/.pyenv ]; then
  export PYENV_ROOT="$HOME/.pyenv"
  PATH="$PYENV_ROOT/shims:$PYENV_ROOT/bin:$PATH"
  eval "$("$PYENV_ROOT"/bin/pyenv init -)"
  # Unset PYENV_SHELL=lightdm-session etc.  It will use $SHELL then by default.
  unset PYENV_SHELL
fi

And then I am setting up a wrapper function and completion in zshrc (via https://github.com/blueyed/oh-my-zsh/commit/90215b1df328f838f0431b66b84ef88cfe82297c).
I can then use $+functions[zsh_setup_pyenv] in other places to check if pyenv has been loaded already (e.g. in my theme).
Having the wrapper like that might not be necessary anymore, but previously I was loading more plugins there.

I think it boils down to: do the setup as early, and as lazy as possible.

[AlJohri]

Thanks @blueyed!

What would be the path forward on this? Can pyenv use the existing PYENV_SHELL marker it already sets to prevent re-execution? Would this need to go through rbenv first or can pyenv attack this independently?

===

I think your solution is mac specific as you're relying on zprofile which gets run in login sessions only. On mac, each new tab is a login session; on linux this isn't the case. Modifying my test dockerfile above:

$ docker-compose run app zsh
running zshenv
running zshrc
non-login shell
interactive shell
0cac7261ff48# exit

$ docker-compose run app zsh --login
running zshenv
running zprofile
running zshrc
login shell
interactive shell
running zlogin
5db04e3a7a2c# exit
running zlogout

[AlJohri]

actually, wait @blueyed I think I'm wrong - if its moved to the profile I think it'll work on linux as well. I forgot that the GUI terminal sessions on linux are all subshells of the login shell anyway.

while something like PYENV_SKIP_ACTIVATION may be useful, it seems like I just misunderstood how to set up my shell properly

[kennethreitz42]

@AlJohri you're far from alone, though. TONS of people have their shells mis-configured, and are blaming it on pipenv.

[AlJohri]

okay, I reconfigured my shell and everything works now as expected! (except for #985) thanks all! I'll submit a PR to change the README to recommend zprofile instead of zshenv as you suggested @blueyed