Skip to content
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

GH-75586: Fix case where PATHEXT isn't applied to items in PATH (Windows) #103179

Merged
merged 31 commits into from Apr 4, 2023

Conversation

csm10495
Copy link
Contributor

@csm10495 csm10495 commented Apr 2, 2023

GH-75586 - Fix case where PATHEXT isn't applied to items in PATH.

The logic change here makes it so that if the command passed into shutil.which on windows, it will be verified against how it is passed in and against the various items in PATHEXT.

For example:

shutil.which('C:/windows/system32/cmd') 

will now return

'C:/windows/system32/cmd.EXE'

Instead of None.

If this looks good, I'd be happy to create a changelog entry, etc. Just feeling the water with this first commit.

@arhadthedev arhadthedev added OS-windows stdlib Python modules in the Lib dir labels Apr 2, 2023
@arhadthedev arhadthedev changed the title GH-75586 - Fix case where PATHEXT isn't applied to items in PATH (Win… GH-75586: Fix case where PATHEXT isn't applied to items in PATH (Windows) Apr 2, 2023
@arhadthedev arhadthedev requested a review from a team April 2, 2023 03:58
Lib/shutil.py Outdated Show resolved Hide resolved
Lib/shutil.py Outdated Show resolved Hide resolved
Lib/shutil.py Outdated Show resolved Hide resolved
Lib/shutil.py Outdated Show resolved Hide resolved
Lib/shutil.py Outdated Show resolved Hide resolved
@csm10495
Copy link
Contributor Author

csm10495 commented Apr 2, 2023

@eryksun I think i fixed everything. Check again. Thanks!

Lib/shutil.py Outdated Show resolved Hide resolved
csm10495 and others added 2 commits April 2, 2023 15:36
Co-authored-by: Eryk Sun <eryksun@gmail.com>
Lib/shutil.py Outdated Show resolved Hide resolved
Lib/test/test_shutil.py Outdated Show resolved Hide resolved
Lib/test/test_shutil.py Outdated Show resolved Hide resolved
csm10495 and others added 2 commits April 2, 2023 16:10
Co-authored-by: Eryk Sun <eryksun@gmail.com>
Co-authored-by: Eryk Sun <eryksun@gmail.com>
@eryksun eryksun added the 3.12 bugs and security fixes label Apr 2, 2023
Doc/library/shutil.rst Outdated Show resolved Hide resolved
Copy link
Member

@zooba zooba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with one doc fix

Doc/library/shutil.rst Outdated Show resolved Hide resolved
Co-authored-by: Steve Dower <steve.dower@microsoft.com>
@csm10495 csm10495 requested a review from zooba April 4, 2023 16:52
Lib/shutil.py Outdated Show resolved Hide resolved
Lib/shutil.py Outdated Show resolved Hide resolved
csm10495 and others added 2 commits April 4, 2023 13:16
Co-authored-by: Steve Dower <steve.dower@microsoft.com>
@csm10495
Copy link
Contributor Author

csm10495 commented Apr 4, 2023

@zooba updated and added a corresponding test.

@csm10495 csm10495 requested a review from zooba April 4, 2023 20:27
@zooba
Copy link
Member

zooba commented Apr 4, 2023

Looks good, and the tests all patch the call to that API so we shouldn't be impacted if the test machine has it configured differently.

Once CI passes, and unless someone else speaks up, I'll merge. Thanks for all your work on this!

@csm10495
Copy link
Contributor Author

csm10495 commented Apr 4, 2023

Got a doctest failure... The error is odd.. maybe its transient? I'll push an empty commit to see if it helps.

Testing of doctests in the sources finished, look at the results in build/doctest/output.txt.
python: ./Include/internal/pycore_typeobject.h:75: _PyType_GetModuleState: Assertion `et->ht_module' failed.
Aborted (core dumped)
make[1]: *** [Makefile:49: build] Error 134
make[1]: Leaving directory '/home/runner/work/cpython/cpython/Doc'
Testing of doctests in the sources finished, look at the results in build/doctest/output.txt
make: *** [Makefile:127: doctest] Error 1
make: Leaving directory '/home/runner/work/cpython/cpython/Doc'
Error: Process completed with exit code 2.

Doc/library/shutil.rst Outdated Show resolved Hide resolved
@csm10495
Copy link
Contributor Author

csm10495 commented Apr 4, 2023

@eryksun added. Thanks.

Copy link
Contributor

@eryksun eryksun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thank you, Charles.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.12 bugs and security fixes OS-windows stdlib Python modules in the Lib dir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants