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

NT escaping in shebang in easy_install #188

Closed
bb-migration opened this Issue Apr 15, 2014 · 26 comments

Comments

Projects
None yet
1 participant
@bb-migration

bb-migration commented Apr 15, 2014

Originally reported by: Anonymous


Hi.
As far as I can see, there is still escaping of spaces in the shebang in the current version:
https://bitbucket.org/pypa/setuptools/src/0e53d50b76aabfdee7b6166593e41d41a16e6a25/setuptools/command/easy_install.py?at=default#cl-1546
I find this quite unfortunate, as this is of very limited use, but prevents some valid use cases.

The current code quotes all shebang strings that contain spaces. On all POSIX systems that I tried (current Ubuntu, ancient redhat, current OS X) this is illegal and simply doesn't work. Given the name of the function, it seems this was intended for windows NT (?).

However, this escaping prevents valid use cases, for example it doesn't allow the specification of "/bin/env /bin/python". I think there is a workaround for this specific case. However I have a use case where I need a different command to produce the correct python executable. This would be easy, except for the escaping.

If it is possible to detect being on a posix platform (probably not that easy?), we could just skip the escaping, as it is never beneficial. If this is not possible, maybe just skip the escaping all together? The benefit seems very platform specific.

Thanks,
Andy


@bb-migration

This comment has been minimized.

bb-migration commented Apr 15, 2014

Original comment by t3kcit (Bitbucket: t3kcit, GitHub: Unknown):


Sorry I was not logged in :-/

@bb-migration

This comment has been minimized.

bb-migration commented Apr 15, 2014

Original comment by guyroz (Bitbucket: guyroz, GitHub: Unknown):


well, escaping doesn't work either:

11:55:09 ⮀ Guys-MacBook-Air ⮀ ~/infinigit ⮀ easy_install
zsh: /Users/guy/.virtualenvs/with space/bin/easy_install: bad interpreter: /Users/guy/.virtualenvs/with\: no such file or directory
error: No urls, filenames, or requirements specified (see --help)

11:55:32 ⮀ Guys-MacBook-Air ⮀ ~/infinigit ⮀ cat ~/.virtualenvs/with\ space/bin/easy_install
#!/Users/guy/.virtualenvs/with\ space/bin/python
# EASY-INSTALL-ENTRY-SCRIPT: 'setuptools==3.4.4','console_scripts','easy_install'
__requires__ = 'setuptools==3.4.4'
import sys
from pkg_resources import load_entry_point

if __name__ == '__main__':
    sys.exit(
        load_entry_point('setuptools==3.4.4', 'console_scripts', 'easy_install')()
    )
@bb-migration

This comment has been minimized.

bb-migration commented Apr 15, 2014

Original comment by guyroz (Bitbucket: guyroz, GitHub: Unknown):


you should avoid directories with spaces on unix systems, this won't be the only incompatibility problem you'll run into.

@bb-migration

This comment has been minimized.

bb-migration commented Apr 15, 2014

Original comment by guyroz (Bitbucket: guyroz, GitHub: Unknown):


escaping does not work either; I don't know of a solution to white-spaces in shebang lines on Unix. if there is one, re-open and we'll see how to implement it. but from a quick search spaces are not supported in shebangs (again, on Unix).

@bb-migration

This comment has been minimized.

bb-migration commented Apr 15, 2014

Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco):


Indeed, I wrote a blog about how Unix doesn't support spaces in filenames. You may be right that skipping the quoting behavior (and just raising an error) would be better on Unix systems.

@bb-migration

This comment has been minimized.

bb-migration commented Apr 15, 2014

Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco):


Spaces are supported in shebangs on Windows.

@bb-migration

This comment has been minimized.

bb-migration commented Apr 15, 2014

Original comment by guyroz (Bitbucket: guyroz, GitHub: Unknown):


yes, of-course, I was referring to Unix; modified my comment so it'll be clear

@bb-migration

This comment has been minimized.

bb-migration commented Apr 15, 2014

Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco):


Guy, I think the OP is not requesting that Setuptools support spaces in filenames in shebang lines, but rather requesting that Setuptools not escape the shebang so that one can provide multiple entities in the shebang line.

@bb-migration

This comment has been minimized.

bb-migration commented Apr 15, 2014

Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco):


Reading through the source, it appears to me as if "executable" is allowed to be supplied, but options to that executable should be present as options to the original script text.

What this ticket is requesting is that "executable" be interpreted to allow multiple entities on Unix and be considered to be a single entity on Windows. That would be a bad choice.

@t3kcit would it be possible in your use case to instead pass a script with something like script_text="#!/usr/bin/env script param", executable="/usr/bin/env" ?

The way I read the code, the 'script' and 'param' would be included in the resulting script and /usr/bin/env would be replaced by /usr/bin/env and there wouldn't be any quoted characters.

Let me know what you find.

@bb-migration

This comment has been minimized.

bb-migration commented Apr 15, 2014

Original comment by guyroz (Bitbucket: guyroz, GitHub: Unknown):


@t3kcit could you describe in more details what is your use-case? what are you trying to replace and with what?

@bb-migration

This comment has been minimized.

bb-migration commented Apr 15, 2014

Original comment by t3kcit (Bitbucket: t3kcit, GitHub: Unknown):


Thanks a lot for your replies and sorry for being unclear. I think Jason got it right. I basically want to pass parameters to the shebang executable.
Unfortunately I didn't understand the proposed solution. Could you please point me to the documentation of "script_text" and which version it was added in?

Thanks a lot!

Andy

@bb-migration

This comment has been minimized.

bb-migration commented Apr 20, 2014

Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco):


When I mention 'script_text', I'm referring to the parameter to the get_script_header function.

It's hard to infer from the description how you're using that call, but because you referenced it explicitly, I'm presuming you're calling that function directly or you've at least traced the behavior in your usage to that function call.

@t3kcit can you provide a description of what you're trying to accomplish (i.e. what are your inputs)?

@bb-migration

This comment has been minimized.

bb-migration commented Apr 27, 2014

Original comment by t3kcit (Bitbucket: t3kcit, GitHub: Unknown):


Sorry to be unclear. I use (or want to use) something like the below:

#!python

   options={
       'build_scripts': {
           'executable': '/my/own/env give-me-python',
       },
   }

in the call to setuptools setup function.
The intention is to have the shebang in my scripts be "#!/my/own/env give-me-python".
(similar to "/bin/env python").

This ends up in the function "get_script_header" function, where it is escaped.
Basically what I want is to set the shebang at setup time to be the above. I thought the options I give above were the way to achieve this, but I might be mistaken.

@bb-migration

This comment has been minimized.

bb-migration commented Apr 30, 2014

Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco):


@t3kcit: what if you set executable to '/my/own/env', but then in the scripts themselves, have a shebang:

#!/any/executable/will/become/my/own/env give-me-python
...

Will that work for your purposes?

@bb-migration

This comment has been minimized.

bb-migration commented Apr 30, 2014

Original comment by t3kcit (Bitbucket: t3kcit, GitHub: Unknown):


I would prefer not to change the scripts if possible. I have to do this to a lot of packages and limiting my changes to the setup.py script will hopefully make it easier to maintain forks.

@bb-migration

This comment has been minimized.

bb-migration commented Apr 30, 2014

Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco):


That clarifies things. So it seems that the issue is that you want to provide "options" (a.k.a. parameters) to the executable and simply passing those options as the executable is causing them to be escaped (as one might expect). In any case, setuptools can probably support "executable" meaning more than simply an executable.

@bb-migration

This comment has been minimized.

bb-migration commented Apr 30, 2014

Original comment by t3kcit (Bitbucket: t3kcit, GitHub: Unknown):


Sorry for being unclear in the beginning.
I disagree with escaping being expected, as this is a non-feature on basically all platforms.
How would would you like to add this functionality? Maybe I can come up with a patch.

@bb-migration

This comment has been minimized.

bb-migration commented Oct 30, 2014

Original comment by wojdyr (Bitbucket: wojdyr, GitHub: wojdyr):


I think I have the same problem, but I'd put it differently.
Handling of the "executable" option in setuptools (in easy_install.py) is not compatible with handling of this option in distutils.

For example, if I have in ~/.pydistutils.cfg:

[build]
executable = /usr/bin/env my-python

distutils will, as expected, change shebang in scripts to:

#!python
#!/usr/bin/env my-python

but setuptools will start script with:

#!python
#!"/usr/bin/env my-python"

Could it be changed just for the sake of consistency?
Simply don't use nt_quote_arg() on unix in
https://bitbucket.org/pypa/setuptools/src/dd8bb6bb99e0ec6ae4218bccc8e77e3a2d39bdb6/setuptools/command/easy_install.py?at=default#cl-1601

@bb-migration

This comment has been minimized.

bb-migration commented Nov 2, 2014

Original comment by t3kcit (Bitbucket: t3kcit, GitHub: Unknown):


Another good point. I really don't see any reason for this call, as it clearly breaks things, but does not seem to have an beneficial effect on any platform.
Can someone explain why these lines are there? As I said above, they don't work for any OS I tried.

@bb-migration

This comment has been minimized.

bb-migration commented Dec 3, 2014

Original comment by yarbelk (Bitbucket: yarbelk, GitHub: yarbelk):


This is related to #272 and #291, or those should be marked as duplicates of this.

@bb-migration

This comment has been minimized.

bb-migration commented Jan 5, 2015

Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco):


Issue #291 was marked as a duplicate of this issue.

@bb-migration

This comment has been minimized.

bb-migration commented Jan 5, 2015

Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco):


I believe I have a fix for this. In the latest commits, I've updated Setuptools to allow for the build_scripts parameter to be a CommandSpec instance, which allows specifying the command line entities explicitly:

'executable': easy_install.CommandSpec(['/my/own/env', 'give-me-python'])

I quickly realized that this verbose syntax isn't particularly friendly, so I allowed it to also accept just a simple list:

'executable': ['/my/own/env', 'give-me-python']

I then realized that still won't fix the situation where the executable is specified in a metadata file (such as pydistutils.cfg), so I made a backward-incompatible change which treats the executable if it's a string as a command line, requiring quoting or escaping if spaces are expected in the path. So now the syntax should be exactly as users have expected:

'executable': '/my/own/env give-me-python'

I haven't made a release, but do please download and install the latest source and report your experience with this change.

@bb-migration

This comment has been minimized.

bb-migration commented Jan 5, 2015

Original comment by t3kcit (Bitbucket: t3kcit, GitHub: Unknown):


Thank you for the fix.
That should work.

@bb-migration

This comment has been minimized.

bb-migration commented May 27, 2015

Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco):


I believe this was fixed in the 12.0 release. Please comment/re-open if not.

@bb-migration

This comment has been minimized.

bb-migration commented Jun 14, 2015

Original comment by joeyespo (Bitbucket: joeyespo, GitHub: joeyespo):


I think this fix broke something else.

I have Python installed in C:\Program Files (x86)\Python (for legacy reasons--it's kind of late to move it now), and any console_script I install from PyPI breaks now. Running the script causes a "failed to create process" error.

I noticed that older console_scripts were working, and turns out it's because they were quoted.

#!"C:\Program Files (x86)\Python\python.exe"

Adding quotes around the newly installed scripts fixes it.

Any suggestions? Should I open a new issue? Thanks.

@bb-migration

This comment has been minimized.

bb-migration commented Jun 14, 2015

Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco):


Yes, please create a new issue. Thanks.

fkrull added a commit to fkrull/setuptools that referenced this issue Jun 25, 2016

Add test to verify shebang generation on non-Windows systems.
Check that even when supplied a --executable argument containing spaces,
non-Windows systems should never escape paths in shebangs since they can't
deal with it anyway and it prevents use cases as described in issue pypa#188.

fkrull added a commit to fkrull/setuptools that referenced this issue Jun 25, 2016

Add test to verify shebang generation on non-Windows systems.
Check that even when supplied a --executable argument containing spaces,
non-Windows systems should never escape paths in shebangs since they can't
deal with it anyway and it prevents use cases as described in issue pypa#188.

fkrull added a commit to fkrull/setuptools that referenced this issue Jun 25, 2016

Test quoting of shebang lines.
See issues pypa#188 and pypa#398; on Windows, the launcher executables support shebangs
that use quotes to escape spaces, so these should be used when necessary. On
the other hand, Unix systems don't support shebangs with quotes so they should
never have quotes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment