|
Without the change: With the change: |
|
I thought distutils is frozen. If this change acceptable, it needs an open issue, news entry and tests. |
distutils.spawn.find_executable() has been reimplemented using shutil.which() to fix two issues: fallback on os.defpath if the PATH environment variable is not set, and respect the PATHEXT environment variable on Windows.
I'm not aware of that. Are you refering to the dead distutils2 project? distutils now accepts changes.
done |
|
It’s not 100% frozen anymore, but still very stable. New things like wheel support happens outside of distutils and the stdlib. For example, it would break a lot of code to use shutil.which in distutils and remove find_executable. This PR however fixes two issues but keeps the interface intact, so it is acceptable. |
|
@serhiy-storchaka: I added a NEWS entry and a basic test. I don't want to write a long test suite, since shutil.which() is very well tested in test_shutils, and find_executable() is now really just calling which(). Would you mind to review my change? |
There was a problem hiding this comment.
Thanks. LGTM now. Added just few minor comments.
| pass | ||
| os.chmod(filename, stat.S_IXUSR) | ||
|
|
||
| rv = find_executable(program, path=tmp_dir) |
There was a problem hiding this comment.
Please add also a negative test. find_executable(program) should return None.
| @@ -0,0 +1,4 @@ | |||
| distutils.spawn.find_executable() has been reimplemented using | |||
There was a problem hiding this comment.
Add links? :func:`distutils.spawn.find_executable`
|
I missed the fact that find_executable() always lookup in the current directory, whereas shutil.which() doesn't. I abandon this change in favor of PR #9049. |
distutils.spawn.find_executable() has been reimplemented using
shutil.which() to fix two issues: fallback on os.defpath if the PATH
environment variable is not set, and respect the PATHEXT environment
variable on Windows.
https://bugs.python.org/issue34530