Experiment: Determine necessity of save_cwd() in PR #116131 #124091
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
On macOS `getcwd(3)` can return EACCES if a path component isn't readable, resulting in PermissionError. `PathFinder.find_spec()` now catches these and ignores them - the same treatment as a missing/deleted cwd. Introduces 2 new `test.support.os_helper` context managers - `save_mode(path, ...)` - restores the mode of a path on exit - `save_cwd(...)` - restores the current directory on exit Unlike `change_cwd(path)` (which always performs a `chdir(path)`), `save_cwd()` allows the code inside the `with` block to control this. The new context manager just restores the original working directory afterward. This is allows finer control of exception handling and robust environment restoration across platforms in `FinderTests.test_permission_error_cwd()`.
Apply suggestions from code review Co-authored-by: Brett Cannon <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
During review of #116131 the necessity of introducing
test.support.os_helper.save_cwd()andtest.support.os_helper.save_mode()has been rightfully queried. I have verifiedsave_mode()is necessary, as described in #116131 (comment).This PR is intended to run a variation of PR #116131 without
save_cwd()through the CPython CI. If this variation passes, thensave_cwd()is unnecessary. If a failure oftest.test_import_lib.import_.test_permission_error_cwd()is introduced, then it will inform the reviews, and serve as a reference for updating comments/docstrings.📚 Documentation preview 📚: https://cpython-previews--124091.org.readthedocs.build/