Fix crash when checking file in a read only directory#21496
Open
karlicoss wants to merge 2 commits into
Open
Conversation
Only applies to new SqliteMetadataStore, which is the default since 1.20; old FilesystemMetadataStore doesn't crash. Fixes python#21495
karlicoss
commented
May 16, 2026
|
|
||
| @unittest.skipIf( | ||
| sys.platform == "win32", | ||
| "POSIX chmod semantics: os.chmod(dir, 0o555) does not prevent writes on Windows", |
Author
There was a problem hiding this comment.
Wasn't sure what's better -- excluding test altogether, or checking the same thing after store = SqliteMetadataStore(cache_dir) and returning early/skipping in the test body.
I figured excluding it altogether is less confusing.
karlicoss
commented
May 16, 2026
| os.makedirs(cache_dir_prefix, exist_ok=True) | ||
| try: | ||
| os.makedirs(cache_dir_prefix, exist_ok=True) | ||
| except OSError: |
Author
There was a problem hiding this comment.
Perhaps a bit too broad, but catching all sort of possible FS read-only states can be difficult, and also this is consistent with what FilesystemMetadataStore was doing..
This comment has been minimized.
This comment has been minimized.
Contributor
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
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
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.
Only applies to new SqliteMetadataStore, which is the default since 1.20; old FilesystemMetadataStore doesn't crash.
Fixes #21495
I added a test for
SqliteMetadataStore-- a bit surprised there weren't any in the first place, perhaps the idea is that this is already covered bycmdlinebased integration tests? I had a stab at implementing one for consistency, but realized that there is no way to issue achmod -wthere, so figured a more specialized unit test is fine. Let me know if there is a better way to test this!I used Claude to bisect when issue started happening and to suggest me best place to place code and make it consistent with existing testbase; but otherwise fully understand it!