gh-141276: Avoid compiling zipimport source in get_filename

[harjothkhara]

Summary

• avoid compiling zipimport source while resolving a module filename
• preserve bytecode validation and source fallback behavior
• add a regression test and a NEWS entry

Root cause

zipimporter.get_filename() called _get_module_code(), which compiled source while import machinery was creating the module spec. Module execution then compiled the same source again.

Tests

• ./python.exe -m test test_zipimport -m test_syntax_warning -m testBadMagic -m testBadMTime
• ./python.exe -m test test_zipimport

Refs #141276

Duplicate-work check

No open or merged pull request referencing #141276 was found before submission.

AI assistance

OpenAI Codex assisted with investigation, implementation, testing, and review.

• Issue: Syntax warnings are duplicated in zipimport #141276

[harjothkhara]

This fixes the duplicated SyntaxWarning from #141276, could someone take a look?

[itamaro]

the fix generally looks good to me, thanks for the PR!

see inline comments regarding skipping wasted source load.

a few additional requests:

1. this is an observable behavior change, which needs to be called out in the NEWS.

the semantic change - previously, py files with syntax errors in zip would raise from find_spec() and get_filename(). after this change, find_spec() and get_filename() would succeed, and only exec_module() would raise.

while this sounds like a significant behavior change, I think it's acceptable and even desirable, because it brings zipimport in line with the standard filesystem based finder/loader - but it does warrant more explicit coverage in NEWS.
this also makes it somewhat risky to backport, so I'd like @serhiy-storchaka's opinion here as well.

2. I'd like to see a few more test cases covering the expected behavior, and preventing unintended future regressions, especially around the subtler corners.

a test that with a py with SyntaxError. assert that find_spec() and get_filename() succeed and return expected spec / filename, and exec_module() raises SyntaxError.

perhaps also have a test that calls get_filename() directly and asserts no warnings.

I see testBadMagic/testBadMTime already cover stale/bad-magic .pyc falling back to .py. could you add:

• valid .pyc + .py with a syntax error → spec picks the .pyc and import succeeds
• package __init__.py with a syntax error + a valid sibling module of the same name → origin is __init__.py and exec raises SyntaxError (no fallthrough to the module)

[bedevere-app]

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

[itamaro]

thanks for making the requested changes!

please don't forget to follow the bot instructions about commenting on the PR once you've made the requested changes (it's needed so the bot updates PR labels and makes it show up in review queues again, otherwise it's more likely to go unnoticed).

we also need to update the docs (Doc/library/zipimport.rst) to match the modified get_filename behavior (raises when "module couldn't be found", not when "module couldn't be imported".

[harjothkhara]

I have made the requested changes; please review again.

Pushed 8082336 with the NEWS wording, the docstring, the comment, the redundant modpath line and the Doc/library/zipimport.rst update.

[bedevere-app]

Thanks for making the requested changes!

@itamaro: please review the changes made to this pull request.

[itamaro]

thanks, it looks good to me! I'll merge main since it's been a while, and merge if CI passes.

cc @serhiy-storchaka @hugovk what do you think about backporting this? (see previous comment)

[read-the-docs-community]

Documentation build overview

📚 cpython-previews | 🛠️ Build #35063365 | 📁 Comparing cb21019 against main (5564790)

  🔍 Preview build  

2 files changed

± library/zipimport.html
± whatsnew/changelog.html