Skip to content

Conversation

@wyattscarpenter
Copy link
Contributor

@wyattscarpenter wyattscarpenter commented Oct 24, 2025

#16019, which fixed this issue (for linux) was not accompanied by a regression test. Thus, nobody noticed that it doesn't work on Windows!

This fixes an Internal Error where namespace packages were not supported properly. This fix was inspired by @sterliakov noticing that #18128 was very similar to #19843, which has a similar fix. Note that we use os.path.isdir(tree.path) instead of trying to catch an IsADirectoryError exception because of a bug on Windows which causes it to throw a PermissionError instead in the relevant situation, which makes except IsADirectoryError unreliable. (We also can't just except (IsADirectoryError, PermissionError) because what if there is an actual permission error?)

python#16019, which fixed this issue (for linux) was not accompanied by a regression test
@wyattscarpenter wyattscarpenter changed the title [WIP] Add a regression test for #15979, and fix it for Windows [WIP] Add a regression test for #15979, and fix linecount-report for Windows Oct 24, 2025
@wyattscarpenter wyattscarpenter marked this pull request as ready for review October 24, 2025 03:44
@wyattscarpenter wyattscarpenter changed the title [WIP] Add a regression test for #15979, and fix linecount-report for Windows Add a regression test for #15979, and fix linecount-report for Windows Oct 24, 2025
@github-actions
Copy link
Contributor

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

Copy link
Collaborator

@sterliakov sterliakov left a comment

Choose a reason for hiding this comment

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

This has a TOCTOU problem, but I think that's fine, report generation is anyway not robust enough to support concurrent file modifications. Thanks!

@wyattscarpenter
Copy link
Contributor Author

I considered this TOCTOU problem, but I don't think there's a better way around it. I'm sure by the time someone files a bug report to complain about a crash caused by them replacing one of their files with a directory in the middle of report generation, python will have fixed its file-access bug :^)

Copy link
Member

@emmatyping emmatyping left a comment

Choose a reason for hiding this comment

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

Thanks! I think this looks good.

@emmatyping
Copy link
Member

Oh and

This has a TOCTOU problem, but I think that's fine

yeah I don't think there's much to be done here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants