pypa / pip

The Python package installer
https://pip.pypa.io/
MIT License
9.52k stars 3.03k forks source link

More robust shebang manipulation #12599

Open jaraco opened 7 months ago

jaraco commented 7 months ago

What's the problem this feature will solve?

When pip installs (non-"console script") scripts to an environment with spaces in the pathname, it supplies the interpreter name in the script but doesn't escape the space(s) in any way, leading to failures:

 draft @ mkdir 'foo bar'
 draft @ py -m venv 'foo bar/.venv'
 draft @ 'foo bar/.venv/bin/pip' install docutils
Collecting docutils
  Using cached docutils-0.20.1-py3-none-any.whl.metadata (2.8 kB)
Using cached docutils-0.20.1-py3-none-any.whl (572 kB)
Installing collected packages: docutils
Successfully installed docutils-0.20.1
 draft @ cd 'foo bar'
 foo bar @ .venv/bin/rst2html.py
xonsh: subprocess mode: command not found: '/Users/jaraco/draft/foo'
 foo bar @ head -n 1 .venv/bin/rst2html.py
#!/Users/jaraco/draft/foo bar/.venv/bin/python3.12

Although the root cause is a long-standing problem with Unix itself, some shells like xonsh do support them:

 foo bar @ sed -i -e '1{s/^#!/#!"/;s/$/"/;}' .venv/bin/rst2html.py
 foo bar @ head -n 1 .venv/bin/rst2html.py
#!"/Users/jaraco/draft/foo bar/.venv/bin/python3.12"
 foo bar @ .venv/bin/rst2html.py --help > /dev/null && echo done
done

Moreover, the Python launcher for Windows also supports them.

Lack of support for spaces in filenames leads to problems like https://github.com/pypa/pipx/issues/1198 when applications attempt to install to the platform-recommended directories.

Describe the solution you'd like

When rewriting the shebangs for scripts during install, wrap the interpreter in quotes when it contains spaces.

Other shells like bash and zsh will still choke on the space, but at least they'll have the opportunity to potentially honor them, and in the meantime, users of shells like xonsh can enjoy a working executable. It's one small step toward honoring valid pathnames and strengthening a precedent for a workable solution that those shells could honor.

Alternative Solutions

One might be tempted to suggest that users should just avoid paths with spaces in filenames, and although that kind of bullying is the norm in Linux, on other platforms like Windows and macOS, the platform guidance itself specifies to install scripts and applications and other things in paths with spaces (e.g. C:\Program Files, ~/Library/Application Support).

Another option might be to apply the somewhat wonky exec wrapper used for console scripts:

 foo bar @ head -n 3 .venv/bin/pipx
#!/bin/sh
'''exec' "/Users/jaraco/draft/foo bar/.venv/bin/python3.12" "$0" "$@"
' '''

Additional context

none

Code of Conduct

uranusjr commented 7 months ago

Note that pip already has access to the shell script hack via vendored distlib. So we can maybe reuse that logic to minimise things to implement.

jaraco commented 6 months ago

Today I learned that although the shebang hack in distlib does in fact allow a script with spaces in the interpreter path to be executed by bash:

 jaraco.vcs main @ bash
bash-5.2$ head -n 3 $(which hg)
#!/bin/sh
'''exec' "/Users/jaraco/Library/Application Support/pipx/venvs/mercurial/bin/python" "$0" "$@"
' '''
bash-5.2$ hg --version -q
Mercurial Distributed SCM (version 6.7.2)

It doesn't solve the issue for other forms of execution, such as through subprocess.Popen:

 @ subprocess.check_output(['hg', '--version'])
FileNotFoundError: [Errno 2] No such file or directory: 'hg'

I don't yet understand why the subprocess invocation is unable to honor the sh+exec hack.

Note, I generated that header with:

pip-run distlib -- -c "import distlib.scripts as scr; sm = scr.ScriptMaker(None, None); print(sm._build_shebang(scr.enquote_executable('/Users/jaraco/Library/Application Support/pipx/venvs/mercurial/bin/python').encode(), b'').decode())"
jaraco commented 6 months ago

I don't yet understand why the subprocess invocation is unable to honor the sh+exec hack.

Oh. The issue seems to be related to me using subprocess.*/shutil.which inside a xonsh context. If I execute it in a native Python context, the program executes as expected.

 ~ @ py -c "import subprocess; print(subprocess.check_output(['hg', '--version'])[:41])"
b'Mercurial Distributed SCM (version 6.7.2)'

It took only a small amount of investigation to determine the cause - for whatever reason, under xonsh when launched as the user's default shell, os.environ only contains a few entries.

eabase commented 6 months ago

OMG, I really hope people doesn't have to use and resort to these kinds of hacks:

#!/bin/sh
'''exec' "/Users/jaraco/draft/foo bar/.venv/bin/python3.12" "$0" "$@"
' '''

It breaks all portability possible!
However, much liked creds to those who thought about this.