gh-141276: Avoid compiling zipimport source in get_filename by harjothkhara · Pull Request #153799 · python/cpython

harjothkhara · 2026-07-16T05:16:08Z

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 · 2026-09-03T00:55:31Z

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:

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.

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)

itamaro · 2026-10-03T18:20:28Z

+                if not compile_source:
+                    return None, ispackage, modpath


not needed with the suggestion above

Suggested change

if not compile_source:

return None, ispackage, modpath

itamaro · 2026-10-03T19:07:44Z

+Fix duplicate :exc:`SyntaxWarning` messages when importing source modules
+from ZIP archives.


some prose you could use here:

zipimporter.get_filename() and find_spec() no longer compile source modules; a SyntaxError in a zipped source module is now raised when the module is executed, matching the behavior of filesystem-based loaders.

bedevere-app · 2026-10-03T19:08:22Z

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.

Move the compile_source=False early return above the archive read, so get_filename() no longer decompresses source it immediately discards. Also update the NEWS entry with itamaro's suggested wording and add tests covering the deferred-SyntaxError behavior: find_spec()/ get_filename() succeed and get_filename() emits no warnings for a module with a syntax error, a valid .pyc still wins over a bad sibling .py, and a package's __init__.py error doesn't fall through to a same-named sibling module.

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".

itamaro · 2026-10-10T01:09:06Z

+``zipimporter.get_filename()`` and ``find_spec()`` no longer compile source
+modules; a :exc:`SyntaxError` in a zipped source module is now raised when the
+module is executed, matching the behavior of filesystem-based loaders.


The new NEWS entry is now missing the original bugfix description

Suggested change

``zipimporter.get_filename()`` and ``find_spec()`` no longer compile source

modules; a :exc:`SyntaxError` in a zipped source module is now raised when the

module is executed, matching the behavior of filesystem-based loaders.

Fix duplicate :exc:`SyntaxWarning` messages when importing source modules

from ZIP archives. :meth:`zipimport.zipimporter.get_filename`, and therefore

:meth:`~zipimport.zipimporter.find_spec`, no longer compile source modules.

A :exc:`SyntaxError` in a zipped source module is now raised when the module

is executed rather than when it is found, matching the behavior of

filesystem-based loaders.

itamaro · 2026-10-10T01:12:34Z

                # bad magic number or non-matching mtime
                # in byte code, try next
                continue
            modpath = toc_entry[0]


nit (pre-existing), but worth doing since you're here -- the second modpath = toc_entry[0] is redundant and can be removed

itamaro · 2026-10-10T01:13:37Z

-# 'fullname'.
-def _get_module_code(self, fullname):
+# Get the code object associated with the module specified by 'fullname'.
+# If compile_source is false, return None for source code without compiling it.


nit

Suggested change

# If compile_source is false, return None for source code without compiling it.

# If compile_source is false, return None for source code without reading or compiling it.

itamaro · 2026-10-10T01:20:40Z

        Return the filename for the specified module or raise ZipImportError
        if it couldn't be imported.


the documented contract is now inaccurate

Suggested change

Return the filename for the specified module or raise ZipImportError

if it couldn't be found.

GitHub looks buggy, at least in how it's rendering this suggested change for me.

the only change I made was s/imported/found/ on the second line

pythongh-141276: Avoid compiling zipimport source in get_filename

0d5505d

harjothkhara requested review from FFY00, brettcannon, ericsnowcurrently, ncoghlan and warsaw as code owners July 16, 2026 05:16

bedevere-app Bot added the awaiting review label Jul 16, 2026

bedevere-app Bot mentioned this pull request Jul 16, 2026

Syntax warnings are duplicated in zipimport #141276

Open

pythongh-141276: Update zipimport warning expectations

05f92d2

brettcannon removed their request for review July 17, 2026 19:12

itamaro requested changes Oct 3, 2026

View reviewed changes

bedevere-app Bot added awaiting changes and removed awaiting review labels Oct 3, 2026

itamaro reviewed Oct 10, 2026

View reviewed changes

Navigation Menu

Repository navigation

gh-141276: Avoid compiling zipimport source in get_filename - #153799

gh-141276: Avoid compiling zipimport source in get_filename#153799
harjothkhara wants to merge 3 commits into
python:mainfrom
harjothkhara:codex/gh-141276-zipimport

harjothkhara commented Jul 16, 2026 •

edited

Loading

Uh oh!

harjothkhara commented Sep 3, 2026 •

edited

Loading

Uh oh!

itamaro left a comment

Uh oh!

Uh oh!

itamaro Oct 3, 2026

Uh oh!

itamaro Oct 3, 2026

Uh oh!

bedevere-app Bot commented Oct 3, 2026

Uh oh!

itamaro left a comment

Uh oh!

itamaro Oct 10, 2026

Uh oh!

itamaro Oct 10, 2026

Uh oh!

itamaro Oct 10, 2026

Uh oh!

itamaro Oct 10, 2026

Uh oh!

itamaro Oct 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		Fix duplicate :exc:`SyntaxWarning` messages when importing source modules
		from ZIP archives.

	# If compile_source is false, return None for source code without compiling it.
	# If compile_source is false, return None for source code without reading or compiling it.

		Return the filename for the specified module or raise ZipImportError
		if it couldn't be imported.


	Return the filename for the specified module or raise ZipImportError
	if it couldn't be found.

Conversation

harjothkhara commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Root cause

Tests

Duplicate-work check

AI assistance

Uh oh!

harjothkhara commented Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

itamaro left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

itamaro Oct 3, 2026

Choose a reason for hiding this comment

Uh oh!

itamaro Oct 3, 2026

Choose a reason for hiding this comment

Uh oh!

bedevere-app Bot commented Oct 3, 2026

Uh oh!

itamaro left a comment

Choose a reason for hiding this comment

Uh oh!

itamaro Oct 10, 2026

Choose a reason for hiding this comment

Uh oh!

itamaro Oct 10, 2026

Choose a reason for hiding this comment

Uh oh!

itamaro Oct 10, 2026

Choose a reason for hiding this comment

Uh oh!

itamaro Oct 10, 2026

Choose a reason for hiding this comment

Uh oh!

itamaro Oct 10, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

harjothkhara commented Jul 16, 2026 •

edited

Loading

harjothkhara commented Sep 3, 2026 •

edited

Loading