-
-
Notifications
You must be signed in to change notification settings - Fork 33.8k
bpo-42041: Clarify how Windows path search affects virtual environments #22715
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
Conversation
sfc-gh-mkeller
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚢 🚢
|
I've added the "skip news" label as this is simply a docs clarification. |
|
Updated based on discussions on the tracker. |
|
@vsajip Are you OK with the revised version? |
vsajip
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Doc/library/subprocess.rst
Outdated
|
|
||
| .. warning:: | ||
|
|
||
| Using the string ``"python"`` as the program to execute is not a good |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I personally feel like we should not single out python executable because wouldn't the same apply with any other program? IMHO we should point out that on Windows non-absolute path executables don't resolve by using the PATH environment variable and as such we encourage users to pass executables as absolute paths. And then we can mention to use shutil.which if they want a discover executable using the PATH environment variable, and in particular if they want to execute a script with the running python sys.executable. Wouldn't that be more accurate and general?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will only apply to anything inside the same directory as python.exe, which is basically just python. In a venv, it's also anything that installed a script wrapper, so pip or virtualenv, among others.
That said, it's been "broken" for 2-3 full releases now (depending on how you count), and the only issues I've heard of were people who thought "python" would be found on PATH. So I don't think switching it back is super important. Arguably, we made it closer to "search PATH".
| arguments for additional differences from the default behavior. Unless | ||
| otherwise stated, it is recommended to pass *args* as a sequence. | ||
|
|
||
| .. warning:: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you also change os.execvp to os.execvpe at the beginning of the Popen docs?
Also, further down in the cwd section it claims "In particular, the function looks for executable (or for the first item in args) relative to cwd if the executable path is a relative path". That should be changed to restrict it to just POSIX. You could change "In particular" to "In POSIX".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
…GH-22715) Clarify in the subprocess documentation how searching for the executable to run works, noting that ``sys.executable`` is the recommended way to find the current interpreter.
https://bugs.python.org/issue42041