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

PATH not extended within virtualenv #1024

Closed
rgilton opened this issue Jul 26, 2023 · 7 comments
Closed

PATH not extended within virtualenv #1024

rgilton opened this issue Jul 26, 2023 · 7 comments
Labels
bug Something isn't working

Comments

@rgilton
Copy link

rgilton commented Jul 26, 2023

Describe the bug

If some python running within pipx attempts to run an executable that's provided by one of its dependencies that is installed within the pipx virtualenv, it fails. I believe this is because the PATH environment variable is not extended with the bin directory of the virtualenv.

How to reproduce

Here's a simple package that runs an executable provided by one of its dependencies:

demo.py

from subprocess import check_call
def main():
    check_call(["cowsay", "hi!"])

setup.cfg

[metadata]                
name = demo               
version = 0.0.1           
                          
[options]                 
install_requires = cowsay 
                          
[options.entry_points]    
console_scripts =         
    demo = demo:main      

Build and run with:

% python -m build -w
% pipx run --spec ./dist/demo-0.0.1-py3-none-any.whl 

Result:

Traceback (most recent call last):
  File "/home/rob/.local/pipx/.cache/a2d81caa3c1d24d/bin/demo", line 8, in <module>
    sys.exit(main())
             ^^^^^^
  File "/home/rob/.local/pipx/.cache/a2d81caa3c1d24d/lib64/python3.11/site-packages/demo.py", line 5, in main
    check_call(["cowsay", "hi!"])
  File "/usr/lib64/python3.11/subprocess.py", line 408, in check_call
    retcode = call(*popenargs, **kwargs)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib64/python3.11/subprocess.py", line 389, in call
    with Popen(*popenargs, **kwargs) as p:
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib64/python3.11/subprocess.py", line 1026, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
  File "/usr/lib64/python3.11/subprocess.py", line 1950, in _execute_child
    raise child_exception_type(errno_num, err_msg, err_filename)
FileNotFoundError: [Errno 2] No such file or directory: 'cowsay'

Expected behavior

By the time main function in demo.py is called, the PATH environment variable should contain the bin directory of the virtualenv. So we should see:

  ___
| hi! |
  ===
   \
    \
      ^__^
      (oo)\_______
      (__)\       )\/\
          ||----w |
          ||     ||

Workaround

A workaround we're currently using is to invoke the target python binary using sys.executable. It's not ideal, as it means that the package needs to be explicitly modified to run in pipx.

from subprocess import check_call
import sys

def main():
    check_call([sys.executable, "-m", "cowsay", "hi!"])
@dukecat0
Copy link
Member

dukecat0 commented Aug 6, 2023

This works for pipx install --include-deps:

$ pipx install --include-deps ./dist/demo-0.0.1-py3-none-any.whl
  installed package demo 0.0.1, installed using Python 3.9.2
  These apps are now globally available
    - cowsay
    - demo
done! ✨ 🌟 ✨

Probably we need to apply this behaviour to pipx run as well.

@dukecat0 dukecat0 added the bug Something isn't working label Aug 6, 2023
@rgilton
Copy link
Author

rgilton commented Aug 6, 2023

Ah, it sounds like adding --include-deps support to pipx run might fix some use-cases, however in this one I wasn't looking for the dependency's executables to be accessible from the "outside".

In my toy example from above, I would not expect cowsay to become a command I could run from my shell. I would only expect demo to be. I can understand that some people might want the other behaviour too.

Seems to me that making cowsay accessible from the outside is a bit like making the python packages that something depends on also accessible/importable from the outside -- definitely not something that one usually wants.

@uranusjr
Copy link
Member

Note that PEP 405 does not require virtual environment activation to add the script directory to PATH. I would say programs relying on the behaviour are designed suboptimally and should be updated.

@rgilton
Copy link
Author

rgilton commented Aug 10, 2023

Note that PEP 405 does not require virtual environment activation to add the script directory to PATH. I would say programs relying on the behaviour are designed suboptimally and should be updated.

Oh, that seems odd to me. What is the "optimal" implementation in your opinion here?

@uranusjr
Copy link
Member

uranusjr commented Aug 10, 2023

Depending on how you want the user to set up the dependency, either use sys.executable in combination with -m, or explicitly look up for an executable in the same script directory with shutil.which(..., path=dirname(sys.argv[0])).

@Andrysire

This comment was marked as spam.

@gaborbernat
Copy link
Contributor

Depending on how you want the user to set up the dependency, either use sys.executable in combination with -m, or explicitly look up for an executable in the same script directory with shutil.which(..., path=dirname(sys.argv[0])).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants