pypa / pipx

Install and Run Python Applications in Isolated Environments
https://pipx.pypa.io
MIT License
10.52k stars 416 forks source link

virtualenv's `/bin` is not in `PATH` when executing command #1559

Closed jerome-pouiller closed 1 month ago

jerome-pouiller commented 1 month ago

I have noticed the bin directory associated with a venv is not included in $PATH when the command is run.

My initial issue was I get some trouble to run west command:

$ west
... <does not work as expected> ...

(note the error looked absolutely unrelated to $PATH)

I found it did work if I sourced the venv first:

$ source /home/jerome/.local/pipx/venvs/west/bin/activate
$ west
... <work as expected> ...

After some investigations, I found:

$ PATH=$PATH:/home/jerome/.local/pipx/venvs/west/bin west
... <work as expected> ...

I believe the pipx promise is to allow to run west directly exactly as if the venv was activated before.

Note this issue has already been noticed by #1369. However, I don't expect the same behavior.

Gitznik commented 1 month ago

I could not replicate this behavior, neither on my PC, nor on a fresh docker container. Could you provide some more detail like your pipx version? Have you run pipx ensurepath?

➜  ~ docker run -it python:3.10 bash
➜ root@9271eba9d728:/# python3 -m pip install -U pipx
Collecting pipx
  Downloading pipx-1.7.1-py3-none-any.whl (78 kB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 78.7/78.7 kB 2.5 MB/s eta 0:00:00
Collecting argcomplete>=1.9.4
  Downloading argcomplete-3.5.1-py3-none-any.whl (43 kB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 43.5/43.5 kB 8.4 MB/s eta 0:00:00
Collecting tomli
  Downloading tomli-2.0.2-py3-none-any.whl (13 kB)
Collecting userpath!=1.9,>=1.6
  Downloading userpath-1.9.2-py3-none-any.whl (9.1 kB)
Collecting packaging>=20
  Downloading packaging-24.1-py3-none-any.whl (53 kB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 54.0/54.0 kB 8.9 MB/s eta 0:00:00
Collecting platformdirs>=2.1
  Downloading platformdirs-4.3.6-py3-none-any.whl (18 kB)
Collecting click
  Downloading click-8.1.7-py3-none-any.whl (97 kB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 97.9/97.9 kB 13.9 MB/s eta 0:00:00
Installing collected packages: tomli, platformdirs, packaging, click, argcomplete, userpath, pipx
Successfully installed argcomplete-3.5.1 click-8.1.7 packaging-24.1 pipx-1.7.1 platformdirs-4.3.6 tomli-2.0.2 userpath-1.9.2
WARNING: Running pip as the 'root' user can result in broken permissions and conflicting behaviour with the system package manager. It is recommended to use a virtual environment instead: https://pip.pypa.io/warnings/venv

➜ root@9271eba9d728:/# pipx ensurepath
Success! Added /root/.local/bin to the PATH environment variable.

Consider adding shell completions for pipx. Run 'pipx completions' for instructions.

You will need to open a new terminal or re-login for the PATH changes to take effect. Alternatively, you can source your shell's config file with e.g.
'source ~/.bashrc'.

Otherwise pipx is ready to go! ✨ 🌟 ✨
➜ root@9271eba9d728:/# source ~/.bashrc
➜ root@9271eba9d728:/# pipx install west
  installed package west 1.2.0, installed using Python 3.10.13
  These apps are now globally available
    - west
done! ✨ 🌟 ✨
➜ root@9271eba9d728:/# west --version
West version: v1.2.0
jerome-pouiller commented 1 month ago

I believe trying to reproduce the issue with west is not relevant (the issue appears in a corner case that is a bit difficult to reproduce). You can probably just observe the difference with a python package containing:

import os; print(os.environ['PATH'])

That's said, here is the requested technical information:

$ pipx ensurepath
/home/jerome/.local/bin is already in PATH.

⚠  All pipx binary directories have been added to PATH. If you are sure you want to proceed, try again with the '--force' flag.

Otherwise pipx is ready to go! ✨ 🌟 ✨
$ pipx --version
1.1.0
$ cat /etc/os-release
PRETTY_NAME="Debian GNU/Linux 12 (bookworm)"
NAME="Debian GNU/Linux"
VERSION_ID="12"
VERSION="12 (bookworm)"
VERSION_CODENAME=bookworm
ID=debian
HOME_URL="https://www.debian.org/"
SUPPORT_URL="https://www.debian.org/support"
BUG_REPORT_URL="https://bugs.debian.org/"
jerome-pouiller commented 1 month ago

You may also be interested by that:

$ ls ~/.local/bin
commander@            dt-extract-example@  emailproxy@
dtb2py@               dt-extract-props@    pwclient@
dt-check-compatible@  dt-mk-schema@        west@
dt-doc-validate@      dt-validate@         zed@
$ ls /home/jerome/.local/pipx/venvs/west/bin/
activate               get_gprof*               __pycache__/
activate.csh           get_objgraph*            pykwalify*
activate.fish          git-clang-format*        pylint*
Activate.ps1           gitlint*                 pylint-config*
bin2hex.py*            hex2bin.py*              pyocd*
can_logconvert*        hex2dump.py*             pyocd-gdbserver*
can_logger*            hexdiff.py*              pyreverse*
can_player*            hexinfo.py*              pysemver*
can_viewer*            hexmerge.py*             python@
cbor2*                 imgtool*                 python3@
clang-format*          isort*                   python3.11@
clang-format-diff.py*  isort-identify-imports*  sphinx-lint*
coverage*              junit2html*              symilar*
coverage3*             lpc_checksum*            undill*
coverage-3.11*         natsort*                 west*
crc*                   pack-manager*            zcbor*
gcovr*                 patool*
Gitznik commented 1 month ago

Right sorry I think I misunderstood. You can run west itself, just fine, but whatever you tried to do required the bin folder of the virtual environment itself to be on the PATH, not west itself?

jerome-pouiller commented 1 month ago

That's it.

Gitznik commented 1 month ago

TBH I'm not sure how we would attempt to fix this. If you look at the file of the application in the bin folder, it'll look something like this:

│ File: /home/gitznik/.local/bin/black

   1   │ #!/home/gitznik/.local/pipx/venvs/black/bin/python
   2   │ # -*- coding: utf-8 -*-
   3   │ import re
   4   │ import sys
   5   │ from black import patched_main
   6   │ if __name__ == '__main__':
   7   │     sys.argv[0] = re.sub(r'(-script\.pyw|\.exe)?$', '', sys.argv[0])
   8   │     sys.exit(patched_main())

This file is generated by pip, and pipx is only responsible for handling the virtual environment around it and making the executable available to the user. We don't actually touch this file in any way.

This might even be a feature request on pip, as they're the ones responsible for how the application is actually run.

jerome-pouiller commented 1 month ago

Arf, I thought this file was generated by pipx. I am going to forward the bug to pip

notatallshaw commented 1 month ago

Coming from https://github.com/pypa/pip/issues/12998 I don't understand what OP is reporting.

If I create a virtual environment and install west, west is available, if I pipx install west, west is available.

Is there actually an issue identified with pip? Or is this just an issue with OPs environment such as directories on PATH (pip does not change environmental variables, that's left up to the user and environment management tools)?

pfmoore commented 1 month ago

The OP still hasn't actually provided the output of the failing command here, so all we have to go on is speculation.

Is the problem that the west command expects to find a virtualenv command on the user's PATH? Because if so, then that's an issue with the west command, not with pipx or pip. I searched the west source code repo, though and I coulddn't find any reference to virtualenv, so that seems unlikely. The only subprocess calls I see in west are to git (which aren't going to be affected by whether a virtualenv's bin directory is on PATH...).

pfmoore commented 1 month ago

It's worth noting that the documentation for west doesn't say that installing it via pipx is supported. @jerome-pouiller have you confirmed with the west project that this is something they support?

jerome-pouiller commented 1 month ago

To be faire, I don't understand the relation between venv, pip and pipx. I expected to get the same result if run my command directly or if I source activate script first.

Unfortunately, there is a difference with the value of $PATH. So, this line executes differently:

import subprocess; subprocess.run(["command-provided-by-a-required-package"])

west or its dependencies have hundreds of call to subprocess which will behave differently.

notatallshaw commented 1 month ago

To reword what has already said, without the steps you tried (e.g. install commands), and without the output to those steps, no one can help.

jerome-pouiller commented 1 month ago

I believe it is not relevant and you are going to waste your time by reading this, but since you insist:

sudo apt install --no-install-recommends git cmake ninja-build gperf \
  ccache dfu-util device-tree-compiler wget python3-dev python3-pip \
  python3-setuptools python3-tk python3-wheel xz-utils file make gcc \
  gcc-multilib g++-multilib libsdl2-dev libmagic1
pipx install west
mkdir workspace
cd workspace
west init -m git@github.com:siliconlabssoftware/zephyr-silabs
west update
pipx runpip west install -r zephyr/scripts/requirements.txt

The following commands are only here to get a success at the end but they are not involved in the issue:

west blobs fetch
wget https://github.com/zephyrproject-rtos/sdk-ng/releases/download/v0.16.8/zephyr-sdk-0.16.8_linux-$(uname -m).tar.xz
wget -O - https://github.com/zephyrproject-rtos/sdk-ng/releases/download/v0.16.8/sha256.sum | shasum --check --ignore-missing
tar -xvf zephyr-sdk-0.16.8_linux-$(uname -m).tar.xz
zephyr-sdk-0.16.8/setup.sh -h -c
ARCH=x86_64 # Also consider "aarch32" or "aarch64"
wget https://www.silabs.com/documents/login/software/SimplicityCommander-Linux.zip
unzip SimplicityCommander-Linux.zip
sudo mkdir -p /opt/commander
sudo chown $(id -un):$(id -gn) /opt/commander
tar -C /opt -xvf SimplicityCommander-Linux/Commander_linux_${ARCH}_*.tar.bz
sudo ln -sfn /opt/commander/commander /usr/local/bin/

Then, the line below does not work as expected:

$ west twister --test sample.basic.helloworld -p siwx917_rb4338a
Renaming output directory to /home/jerome/zephyr/zephyr-silabs/twister-out.4
INFO    - Using Ninja..
INFO    - Zephyr version: v3.7.0-4150-g029540abece0
INFO    - Using 'zephyr' toolchain.
INFO    - Building initial testsuite list...
INFO    - Writing JSON report /home/jerome/zephyr/zephyr-silabs/twister-out/testplan.json
INFO    - JOBS: 8
INFO    - Adding tasks to the queue...
INFO    - Added initial list of jobs to queue
ERROR   - Cmake build failure: /home/jerome/zephyr/zephyr/samples/hello_world for siwx917_rb4338a
ERROR   - siwx917_rb4338a           samples/hello_world/sample.basic.helloworld         ERROR : Cmake build failure
ERROR   - see: /home/jerome/zephyr/zephyr-silabs/twister-out/siwx917_rb4338a/samples/hello_world/sample.basic.helloworld/build.log
INFO    - Total complete:    1/   1  100%  skipped:    0, failed:    0, error:    1
INFO    - 1 test scenarios (1 test instances) selected, 0 configurations skipped (0 by static filter, 0 at runtime).
INFO    - 0 of 1 test configurations passed (0.00%), 0 failed, 1 errored, 0 skipped with 0 warnings in 3.52 seconds
INFO    - In total 1 test cases were executed, 0 skipped on 1 out of total 1 platforms (100.00%)
INFO    - 0 test configurations executed on platforms, 1 test configurations were only built.
INFO    - Saving reports...
INFO    - Writing JSON report /home/jerome/zephyr/zephyr-silabs/twister-out/twister.json
INFO    - Writing xunit report /home/jerome/zephyr/zephyr-silabs/twister-out/twister.xml...
INFO    - Writing xunit report /home/jerome/zephyr/zephyr-silabs/twister-out/twister_report.xml...
INFO    - -+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
INFO    - The following issues were found (showing the top 10 items):
INFO    - 1) samples/hello_world/sample.basic.helloworld on siwx917_rb4338a error (Cmake build failure)
INFO    -
INFO    - To rerun the tests, call twister using the following commandline:
INFO    - west twister -p <PLATFORM> -s <TEST ID>, for example:
INFO    -
INFO    - west twister -p siwx917_rb4338a -s samples/hello_world/sample.basic.helloworld
INFO    - or with west:
INFO    - west build -p -b siwx917_rb4338a ../zephyr/samples/hello_world -T sample.basic.helloworld
INFO    - -+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
INFO    - Run completed

while this one work:

$ source /home/jerome/.local/pipx/venvs/west/bin/activate
(west) $ west twister --test sample.basic.helloworld -p siwx917_rb4338a
Renaming output directory to /home/jerome/zephyr/zephyr-silabs/twister-out.5
INFO    - Using Ninja..
INFO    - Zephyr version: v3.7.0-4150-g029540abece0
INFO    - Using 'zephyr' toolchain.
INFO    - Building initial testsuite list...
INFO    - Writing JSON report /home/jerome/zephyr/zephyr-silabs/twister-out/testplan.json
INFO    - JOBS: 8
INFO    - Adding tasks to the queue...
INFO    - Added initial list of jobs to queue
INFO    - Total complete:    1/   1  100%  skipped:    0, failed:    0, error:    0
INFO    - 1 test scenarios (1 test instances) selected, 0 configurations skipped (0 by static filter, 0 at runtime).
INFO    - 1 of 1 test configurations passed (100.00%), 0 failed, 0 errored, 0 skipped with 0 warnings in 10.68 seconds
INFO    - In total 0 test cases were executed, 1 skipped on 1 out of total 1 platforms (100.00%)
INFO    - 0 test configurations executed on platforms, 1 test configurations were only built.
INFO    - Saving reports...
INFO    - Writing JSON report /home/jerome/zephyr/zephyr-silabs/twister-out/twister.json
INFO    - Writing xunit report /home/jerome/zephyr/zephyr-silabs/twister-out/twister.xml...
INFO    - Writing xunit report /home/jerome/zephyr/zephyr-silabs/twister-out/twister_report.xml...
INFO    - Run completed
pfmoore commented 1 month ago
ERROR   -  Cmake build failure: /home/jerome/zephyr/zephyr/samples/hello_world for siwx917_rb4338a
ERROR   - siwx917_rb4338a           samples/hello_world/sample.basic.helloworld         ERROR : Cmake build failure
ERROR   - see: /home/jerome/zephyr/zephyr-silabs/twister-out/siwx917_rb4338a/samples/hello_world/sample.basic.helloworld/build.log

OK, so what's in the build.log file?

jerome-pouiller commented 1 month ago
$ cat /home/jerome/zephyr/zephyr-silabs/twister-out/siwx917_rb4338a/samples/hello_world/sample.basic.helloworld/build.log
Loading Zephyr default modules (Zephyr base).
-- Application: /home/jerome/zephyr/zephyr/samples/hello_world
-- CMake version: 3.25.1
-- Found Python3: /usr/bin/python3 (found suitable version "3.11.2", minimum required is "3.10") found components: Interpreter
-- Cache files will be written to: /home/jerome/.cache/zephyr
-- Zephyr version: 3.7.99 (/home/jerome/zephyr/zephyr)
No board named 'siwx917_rb4338a' found.

Please choose one of the following boards:

[...list 600 boards...]
CMake Error at /home/jerome/zephyr/zephyr/cmake/modules/boards.cmake:244 (message):
  Invalid BOARD; see above.
Call Stack (most recent call first):
  /home/jerome/zephyr/zephyr/cmake/modules/zephyr_default.cmake:133 (include)
  /home/jerome/zephyr/zephyr/share/zephyr-package/cmake/ZephyrConfig.cmake:66 (include)
  /home/jerome/zephyr/zephyr/share/zephyr-package/cmake/ZephyrConfig.cmake:92 (include_boilerplate)
  CMakeLists.txt:5 (find_package)

-- Configuring incomplete, errors occurred!
$
notatallshaw commented 1 month ago

Importantly, it looks like west installs fine, so it seems like it doesn't support being called the way pipx installs it. It really seems to me this is something you need to discuss with the west project.

jerome-pouiller commented 1 month ago

Note this scenario also work as expected:

    $ PATH=$PATH:/home/jerome/.local/pipx/venvs/west/bin
    $ west twister --test sample.basic.helloworld -p siwx917_rb4338a
notatallshaw commented 1 month ago

Yeah, that really seems like west expects to have an environment activated with west installed, but that's not what you get from pipx installing tools.

pfmoore commented 1 month ago

So where is west looking for the board siwx917_rb4338a? Is it expecting to find it on PATH? Because if it is, there's nothing that requires that to be the case.

I suspect that west (the application) is expecting to be run with your PATH set to include the directory where pip installs the west.exe wrapper. But that's not something that pip ensures, nor is it something that pipx enforces. In fact, quite the opposite - pipx relies on being able to copy (or link) the west.exe application into a different directory (which is on PATH).

Or, to put it another way, west doesn't support being installed via pipx. So you should either follow the official west installation instructions, or ask the west developers to change the application to support being installed via pipx.

jerome-pouiller commented 1 month ago

nor is it something that pipx enforces.

I believe this is not entirely true. PATH is changed in the activate script.

pfmoore commented 1 month ago

pipx doesn't run the activate script, by design.

jerome-pouiller commented 1 month ago

Do you know where this script come from?

pfmoore commented 1 month ago

It's created by virtualenv, but ignored by pipx, as it's not relevant to how pipx works.

I'll repeat what I said before - this is simply a case where west isn't designed to work with pipx. If you want to use it that way, you'll need to either use the workaround you found (of manually running the activate script) or ask the west developers to make changes to support installation with pipx.

jerome-pouiller commented 1 month ago

What changes do you suggest to west to support installation with pipx?

jerome-pouiller commented 1 month ago

I mean, any call to subprocess is impacted. What is the proper way to use subprocess to allow pipx usage?

pfmoore commented 1 month ago

I don't know. The first time I even heard of west was when it came up here. Working out what's needed to support pipx is up to the west developers. Or they may simply say no, they aren't willing to support this installation method. Ask them rather than asking here.

What is the proper way to use subprocess to allow pipx usage?

Only run things that you have ensured are available on the user's PATH.

Gitznik commented 1 month ago

Will close this as it does not seem to be something that we should/will support. Thanks @pfmoore for jumping on the discussion.

jerome-pouiller commented 1 month ago

FYI, Alyssa Coghlan gave a very useful answer.