Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork33.7k
gh-127001: Fix PATHEXT issues in shutil.which() on Windows#127035
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
…h() on WindowsName without a PATHEXT extension is only searched if the mode does notinclude X_OK.
zooba commentedNov 19, 2024
Ah, nice. I approve of this logic. I have a vague feeling it was discussed at some point, but I think it makes sense to require an extension from |
serhiy-storchaka commentedNov 19, 2024
I'll rewrite tests tomorrow. |
serhiy-storchaka commentedNov 22, 2024
Some of existing tests did not make sense because they are grounded on wrong model. I replaced them. Also I refactored other tests to make them more uniform and reuse the same files and directories. Fixed also cases when PATHEXT contains a multicomponent extension (e.g. |
zooba 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.
One comment I'd like to see added, but otherwise looks good
Lib/shutil.py Outdated
| pathext_source=os.getenv("PATHEXT")or_WIN_DEFAULT_PATHEXT | ||
| pathext= [extforextinpathext_source.split(os.pathsep)ifext] | ||
| pathext=pathext_source.split(os.pathsep) | ||
| pathext= [ext.rstrip('.')andextforextinpathextifext] |
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 really dislike usingand for value selection. I trust that you've got it right here, but I can't read it easily. Can we maybe comment?
# Replace ext of '.' with empty string, but keep trailing dots on '.ext.'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.
Actually, I think that it is better to remove dots in all cases. The behavior of cmd.exe ifPATHEXT contains.BAT. and there isa.bat in the path:
a-- hangsa.bat-- founda.-- not founda.bat.-- not found
which() will now return the same result, except for the first case. I think this is the best option for now.
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.
Sounds good to me
8899e85 intopython:mainUh oh!
There was an error while loading.Please reload this page.
Thanks@serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13. |
…honGH-127035)* Name without a PATHEXT extension is only searched if the mode does not include X_OK.* Support multi-component PATHEXT extensions (e.g. ".foo.bar").* Support files without extensions in PATHEXT contains dot-only extension (".", "..", etc).* Support PATHEXT extensions that end with a dot (e.g. ".foo.").(cherry picked from commit8899e85)Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
Sorry,@serhiy-storchaka, I could not cleanly backport this to |
GH-127156 is a backport of this pull request to the3.13 branch. |
…ws (pythonGH-127035)* Name without a PATHEXT extension is only searched if the mode does not include X_OK.* Support multi-component PATHEXT extensions (e.g. ".foo.bar").* Support files without extensions in PATHEXT contains dot-only extension (".", "..", etc).* Support PATHEXT extensions that end with a dot (e.g. ".foo.").(cherry picked from commit8899e85)Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
GH-127158 is a backport of this pull request to the3.12 branch. |
…-127035) (GH-127156)* Name without a PATHEXT extension is only searched if the mode does not include X_OK.* Support multi-component PATHEXT extensions (e.g. ".foo.bar").* Support files without extensions in PATHEXT contains dot-only extension (".", "..", etc).* Support PATHEXT extensions that end with a dot (e.g. ".foo.").(cherry picked from commit8899e85)Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
…-127035) (GH-127158)* Name without a PATHEXT extension is only searched if the mode does not include X_OK.* Support multi-component PATHEXT extensions (e.g. ".foo.bar").* Support files without extensions in PATHEXT contains dot-only extension (".", "..", etc).* Support PATHEXT extensions that end with a dot (e.g. ".foo.").(cherry picked from commit8899e85)
…Windows (#2408)This PR aims to address a behavior change in shutil.which (in Windows) introduced inpython/cpython#127035.For more context, in test_run_model_exe_rel_path, the mf6 executable is copied to a relative path, ../bin/mf6 (without extension). Within the test, shutil.which is used to locate the executable path. However, with the new changes in shutil.which, by default, it would only search for the path with an extension in PATHEXT, excluding paths without extension.
…honGH-127035)* Name without a PATHEXT extension is only searched if the mode does not include X_OK.* Support multi-component PATHEXT extensions (e.g. ".foo.bar").* Support files without extensions in PATHEXT contains dot-only extension (".", "..", etc).* Support PATHEXT extensions that end with a dot (e.g. ".foo.").
Uh oh!
There was an error while loading.Please reload this page.
include X_OK.
(".", "..", etc).