pypa / pipenv

Python Development Workflow for Humans.
https://pipenv.pypa.io
MIT License
24.78k stars 1.86k forks source link

Linuxbrew shells are detected as `nu` shell #6197

Closed JoshStern closed 3 weeks ago

JoshStern commented 1 month ago

Issue description

When running pipenv shell using a brew-installed shell the cmd will include linuxbrew which trips the nu branch when identifying the activate file and command to run.

Expected result

Shells with nu elsewhere in the path are not detected as nu shell.

Actual result

$ pipenv shell
Launching subshell in virtual environment...
 overlay use <venvpath>/bin/activate.nu

Followed by (I am using zsh):

$  overlay use <venvpath>/bin/activate.nu
zsh: command not found: overlay

Steps to replicate

This can be replicated without a Pipfile:

  1. Install brew from guide.
  2. Install pipenv, and zsh:
    brew install pipenv zsh
  3. Start zsh.
  4. Attempt to start a pipenv shell:
    pipenv shell

Proposed fix

This may require some conversation. Switching the branches to check the end of the path would be a breaking change for anyone who depends on the current behavior. It may be worth adding a zsh branch which does the same check before nu. I'm happy to make the change with some guidance from a contributor.


$ pipenv --support Pipenv version: `'2024.0.1'` Pipenv location: `'/home/linuxbrew/.linuxbrew/Cellar/pipenv/2024.0.1/libexec/lib/python3.12/site-packages/pipenv'` Python location: `'/home/linuxbrew/.linuxbrew/Cellar/pipenv/2024.0.1/libexec/bin/python'` OS Name: `'posix'` User pip version: `'24.0'` user Python installations found: PEP 508 Information: ``` {'implementation_name': 'cpython', 'implementation_version': '3.12.3', 'os_name': 'posix', 'platform_machine': 'x86_64', 'platform_python_implementation': 'CPython', 'platform_release': '6.2.0-33-generic', 'platform_system': 'Linux', 'platform_version': '#33~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC Thu Sep 7 ' '10:33:52 UTC 2', 'python_full_version': '3.12.3', 'python_version': '3.12', 'sys_platform': 'linux'} ``` System environment variables: - `GJS_DEBUG_TOPICS` - `USER` - `XDG_SESSION_TYPE` - `SHLVL` - `HOME` - `DESKTOP_SESSION` - `GIO_LAUNCHED_DESKTOP_FILE` - `GTK_MODULES` - `GNOME_SHELL_SESSION_MODE` - `MANAGERPID` - `SYSTEMD_EXEC_PID` - `DBUS_SESSION_BUS_ADDRESS` - `GIO_LAUNCHED_DESKTOP_FILE_PID` - `LOGNAME` - `GTK_IM_MODULE` - `JOURNAL_STREAM` - `XDG_SESSION_CLASS` - `USERNAME` - `GNOME_DESKTOP_SESSION_ID` - `WINDOWPATH` - `PATH` - `SESSION_MANAGER` - `INVOCATION_ID` - `XDG_RUNTIME_DIR` - `XDG_MENU_PREFIX` - `DISPLAY` - `LANG` - `XDG_CURRENT_DESKTOP` - `XAUTHORITY` - `XDG_SESSION_DESKTOP` - `XMODIFIERS` - `SSH_AGENT_LAUNCHER` - `SSH_AUTH_SOCK` - `SHELL` - `QT_ACCESSIBILITY` - `GDMSESSION` - `GPG_AGENT_INFO` - `GJS_DEBUG_OUTPUT` - `QT_IM_MODULE` - `PWD` - `XDG_DATA_DIRS` - `XDG_CONFIG_DIRS` - `KITTY_WINDOW_ID` - `WINDOWID` - `TERM` - `COLORTERM` - `OLDPWD` - `HOMEBREW_PREFIX` - `HOMEBREW_CELLAR` - `HOMEBREW_REPOSITORY` - `MANPATH` - `INFOPATH` - `ZSH` - `PAGER` - `LESS` - `LSCOLORS` - `LS_COLORS` - `VIRTUAL_ENV_DISABLE_PROMPT` - `FZF_CTRL_R_OPTS` - `NVM_DIR` - `NVM_CD_FLAGS` - `_` - `PIP_DISABLE_PIP_VERSION_CHECK` - `PYTHONDONTWRITEBYTECODE` - `PYTHONFINDER_IGNORE_UNSUPPORTED` - `PIP_PYTHON_PATH` - `PIPENV_ACTIVE` Pipenv–specific environment variables: - `PIPENV_ACTIVE`: `1` Debug–specific environment variables: - `PATH`: `/home/linuxbrew/.linuxbrew/bin:/home/linuxbrew/.linuxbrew/sbin:/home/user/.local/bin:/home/linuxbrew/.linuxbrew/bin:/home/linuxbrew/.linuxbrew/sbin:/home/user/.local/bin:/home/user/.local/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games:/snap/bin:/snap/bin` - `SHELL`: `/home/linuxbrew/.linuxbrew/bin/zsh` - `LANG`: `en_US.UTF-8` - `PWD`: `/home/user` --------------------------- Contents of `Pipfile` ('/home/user/Pipfile'): ```toml [[source]] url = "https://pypi.org/simple" verify_ssl = true name = "pypi" [packages] [dev-packages] [requires] python_version = "3.12" ```
oz123 commented 1 month ago

First, I am suprised to learn that we don't have a bransh for zsh. It is quite popular. Second, the place where the decision is taking place is here:

https://github.com/pypa/pipenv/blob/5eb672a6153c11a161420229373dcfa115a08050/pipenv/shells.py#L47

Supported shells are found here:

https://github.com/pypa/virtualenv/tree/main/src/virtualenv/activation

JoshStern commented 3 weeks ago

@oz123 thank you for taking a look! zsh works fine with the else branch activate script.

The issue is that elif "nu" in cmd: will trip on any nu present in the cmd path. In my case li[nu]xbrew sends it down the nu branch. The other branches have the same problem.

I think a 'deep' fix would only detect based on the end of the cmd path but that's a breaking change (maybe that's okay? I can draft it up if so).

JoshStern commented 3 weeks ago

As a workaround (if anyone else runs into this) I've set up a symlink to a path without nu in it and made sure that's found before the brew installation.

oz123 commented 3 weeks ago

Please draft it. I was just taking a look on my Ubuntu VM at this issue. Unfortunately, despite your very detailed steps to construct the issue, I was not able to recreate it:

oznt@xubuntu-vm:~$ which pipenv
/home/linuxbrew/.linuxbrew/bin/pipenv
oznt@xubuntu-vm:~$ pipenv --python /home/linuxbrew/.linuxbrew/Cellar/pipenv/2024.0.1/libexec/bin/python shell
Launching subshell in virtual environment...
 . /home/oznt/.local/share/virtualenvs/oznt-8gl3RSgU/bin/activate
oznt@xubuntu-vm:~$  . /home/oznt/.local/share/virtualenvs/oznt-8gl3RSgU/bin/activate
(oznt) oznt@xubuntu-vm:~$ exit
exit
oznt@xubuntu-vm:~$ zsh
xubuntu-vm%  pipenv --python /home/linuxbrew/.linuxbrew/Cellar/pipenv/2024.0.1/libexec/bin/python shell
Launching subshell in virtual environment...
xubuntu-vm%  . /home/oznt/.local/share/virtualenvs/oznt-8gl3RSgU/bin/activate
(oznt) xubuntu-vm% 
JoshStern commented 3 weeks ago

Do you already have zsh installed somewhere else? echo $SHELL should be the linuxbrew path for this to ~work~ repro.

oz123 commented 3 weeks ago

Well:

$ zsh
xubuntu-vm% echo $SHELL
/bin/bash

That seems wrong...

JoshStern commented 3 weeks ago

if which zsh is showing the right place you may need to chsh -s $(which zsh)

oz123 commented 3 weeks ago

OK, I set my default shell to zsh, and after new login I can recreate the issue.

oz123 commented 3 weeks ago

@JoshStern can you add to your patch:

cmd = cmd.split(".linuxbrew")[-1]

I believe this will not break the behaviour, as the current order of parsing does not change.

>>> cmd = "/bin/foo"
>>> cmd.split(".linuxbrew")[-1]
'/bin/foo'
>>> cmd.split('/home/linuxbrew/.linuxbrew/bin/zsh')[-1]
'/bin/foo'
JoshStern commented 3 weeks ago

Just pushed up a PR with my original fix, running the tests now.

Your idea is definitely more backwards compatible, I was hoping to fix it for any path that has a shell as a substring and not just the linuxbrew case.

JoshStern commented 3 weeks ago

I can't seem to get the suite running using the container option. I tried 3.7, 3.8, and 3.9 (got the furthest with 3.8).

I've tried most of the options here, let me know if there's something else I need to set up.

JoshStern commented 3 weeks ago

@oz123 I've pushed both versions of the fix - feel free to pick either (or modify as needed).

Sorry I can't verify the suite locally :disappointed:. I may spend some time trying to get the containerized run working on my machine on another day.

oz123 commented 3 weeks ago

Thanks for your efforts. No worries about the suite. We only support Python 3.8+.

oz123 commented 3 weeks ago

Fixed in https://github.com/pypa/pipenv/pull/6215