bpo-35755: shutil.which() uses os.confstr("CS_PATH")#12858
bpo-35755: shutil.which() uses os.confstr("CS_PATH")#12858vstinner merged 1 commit intopython:masterfrom vstinner:which
Conversation
|
I'm not comfortable to backport this change to stable Python branches (2.7 and 3.7). I propose to only modify master (Python 3.8). |
|
@asottile: Would you mind to review this change? |
yep, will review this when I get home from work 👍 |
asottile
left a comment
asottile
left a comment
There was a problem hiding this comment.
shutil.which looks good, find_executable is still discovering from pwd though
|
@asottile: Would you mind to review the updated PR? find_executable() first looks if the program exists in the current directory. My PR doesn't change that. I have no opinion if it's a good thing or not, but I don't want to change that in this PR. If you want to change it, please open a separated issue on bugs.python.org since it will be backward incompatible change not directly related to https://bugs.python.org/issue35755 |
|
Hum, I propose to backport this change (use confstr("CS_PATH")) to Python 3.7. Or at least, backport additional tests. |
asottile
left a comment
asottile
left a comment
There was a problem hiding this comment.
looks great! thanks for the additional test
shutil.which() and distutils.spawn.find_executable() now use
os.confstr("CS_PATH") if available instead of os.defpath, if the PATH
environment variable is not set.
Don't use os.confstr("CS_PATH") nor os.defpath if the PATH
environment variable is set to an empty string to mimick Unix 'which'
command behavior.
Changes:
* find_executable() now starts by checking for the exeuctable in the
current working directly case first. Add an explicit
"if not path: return None".
* Add tests for PATH='', PATH=':' and for PATHEXT.
|
I rebased my PR, squashed commits and just fixed a comment. |
|
Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7. |
|
Sorry, @vstinner, I could not cleanly backport this to |
* bpo-35755: shutil.which() uses os.confstr("CS_PATH") (GH-12858) shutil.which() and distutils.spawn.find_executable() now use os.confstr("CS_PATH") if available instead of os.defpath, if the PATH environment variable is not set. Don't use os.confstr("CS_PATH") nor os.defpath if the PATH environment variable is set to an empty string. Changes: * find_executable() now starts by checking for the executable in the current working directly case. Add an explicit "if not path: return None". * Add tests for PATH='' (empty string), PATH=':' and for PATHEXT. (cherry picked from commit 228a3c9) * bpo-35755: Remove current directory from posixpath.defpath (GH-11586) Document the change in a NEWS entry of the Security category. (cherry picked from commit 2c4c02f)
5 participants
shutil.which() and distutils.spawn.find_executable() now use
os.confstr("CS_PATH") if available instead of os.defpath, if the PATH
environment variable is not set.
Don't use os.confstr("CS_PATH") nor os.defpath if the PATH
environment variable is set to an empty string to mimick Unix 'which'
command behavior.
https://bugs.python.org/issue35755