Improve "run" behavior on Windows #2727
Conversation
Some Windows users are used to launch files without specifying a command,
e.g.::
> my-script.py
This works in the shell because Windows will automatically choose an
command based on file association, and with newer Python versions, the Py
Launcher (py.exe) automatically chooses the correct Python based on
shebang-parsing.
A similar syntax, unfortunately, does not currently work in Pipenv::
> pipenv run my-script.py
Since my-script.py will be treated as a real application by the subprocess
module.
This patch catch Windows error 193 during subprocess initialization, and
fall back to use COMSPEC (shell=True) when it happens, to provide better
support for this use case.
uranusjr
force-pushed
the
windows-run-non-executable
branch
from
d460a21 to
87ab5e8
Compare
| # Windows error 193 "Command is not a valid Win32 application" to handle | ||
| # a "command" that is non-executable. See pypa/pipenv#2727. | ||
| try: | ||
| return subprocess.Popen([command] + script.args, **options) |
There was a problem hiding this comment.
So it's a command, but not an executable? I think pythonfinder.which actually handles this correctly then on windows whenever we get around to figuring that out => it will find things even if the execute bit isn't set as just from testing I found that to be unreliable
There was a problem hiding this comment.
Yeah, the cause here is that the DOS CLI does not really have a notion of executable files. As far as it concerns, everything can be launched, and it’s up to the operating system to decide what to do. As a consequence, where.exe find all sorts of things, not just applications. It would happily return django-admin.py if it’s in PATH, and subprocess.Popen (actually the CreateProcess API it uses under the hood) would be hung to dry.
So the root issue here is that we are mixing two paradigms when implementing Pipenv for Windows because the deep roots of Windows lacking a real user-facing “executable” flag. I’d doubt even a re-implemented which function would help, unless you dig deep into Windows API to actually detect if a file is a real application (by peeking into the PE header, for example).
|
Not sure if we need to complicate it that much. In pythonfinder I think I handle everything with sort order. So prefer by path order and execute bit with priority to known executable extensions. At the end of the day we are talking about this: users can run any file on windows from the command line. File get passed to the CreateProcess API which fails if the file can’t be executed. This produces an error message with information. We can let pipenv act like a cli application and trust that the user is trying to rub something because they want to, and just try running if if it exists and providing that error if it can’t be executed. Or we can rebuild the API and manually introspect the file to try and check against the api and achieve the exact same result of printing a slightly different error. Let ms know Iif I’m missing something here In case we just match an entry in |
|
hm had a chance to look at this properly, i swiped the logic and built it into vistir as well but it looks right to me |
Provide better error-handling for #2718 (so the command never throws OSError).
Some Windows users are used to launch files without specifying a command, e.g.
This works in the shell because Windows will automatically choose an command based on file association, and with newer Python versions, the Py Launcher (py.exe) automatically chooses the correct Python based on shebang-parsing.
A similar syntax, unfortunately, does not currently work in Pipenv
Since my-script.py will be treated as a real application by the subprocess module.
This patch catch Windows error 193 during subprocess initialization, and fall back to use COMSPEC (shell=True) when it happens, to provide better support for this use case.
The checklist
news/directory to describe this fix with the extension.bugfix,.feature,.behavior,.doc..vendor. or.trivial(this will appear in the release changelog). Use semantic line breaks and name the file after the issue number or the PR #.