Skip to content

Commit 47e4ba6

Browse files
[3.15] gh-141276: Avoid compiling zipimport source in get_filename (GH-153799) (#159729)
gh-141276: Avoid compiling zipimport source in get_filename (GH-153799) `zipimporter.get_filename()` compiled source modules just to work out the module's file name. `find_spec()` calls it through `spec_from_loader()`, so importing a source module from a ZIP archive compiled it twice and emitted every `SyntaxWarning` twice. `get_filename()` now returns before reading or compiling the source. As a result, a `SyntaxError` in a zipped source module is now raised by `exec_module()` rather than by `find_spec()` or `get_filename()`, and the source isn't compiled twice during import, which matches the filesystem-based loaders. (cherry picked from commit 230917f) Co-authored-by: Harjoth Khara <harjoth.khara@gmail.com>
1 parent 6412d30 commit 47e4ba6

6 files changed

Lines changed: 93 additions & 9 deletions

File tree

‎Doc/library/zipimport.rst‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ zipimporter Objects
124124

125125
Return the value ``__file__`` would be set to if the specified module
126126
was imported. Raise :exc:`ZipImportError` if the module couldn't be
127-
imported.
127+
found.
128128

129129
.. versionadded:: 3.1
130130

‎Lib/test/test_cmd_line_script.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -827,7 +827,7 @@ def test_zipfile_run_filter_syntax_warnings_by_module(self):
827827
'-Werror:::test_pkg.__main__',
828828
os.path.join(zip_name, 'test_pkg')
829829
)
830-
self.assertEqual(err.count(b': SyntaxWarning: '), 12)
830+
self.assertEqual(err.count(b': SyntaxWarning: '), 6)
831831

832832

833833
def tearDownModule():

‎Lib/test/test_zipimport.py‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import time
1010
import unittest
1111
import unittest.mock
12+
import warnings
1213

1314
from test import support
1415
from test.support import import_helper
@@ -229,6 +230,81 @@ def testPy(self):
229230
files = {TESTMOD + ".py": test_src}
230231
self.doTest(".py", files, TESTMOD)
231232

233+
def test_syntax_warning(self):
234+
files = {TESTMOD + ".py": "x = 1 is 1\n"}
235+
self.makeZip(files)
236+
zi = zipimport.zipimporter(TEMP_ZIP)
237+
238+
with warnings.catch_warnings(record=True) as caught:
239+
warnings.simplefilter("always", SyntaxWarning)
240+
spec = zi.find_spec(TESTMOD)
241+
self.assertIsNotNone(spec)
242+
self.assertEqual(caught, [])
243+
244+
mod = importlib.util.module_from_spec(spec)
245+
spec.loader.exec_module(mod)
246+
247+
self.assertEqual(len(caught), 1)
248+
self.assertIsInstance(caught[0].message, SyntaxWarning)
249+
250+
def test_syntax_error_defers_to_exec(self):
251+
files = {TESTMOD + ".py": "x =\n"}
252+
self.makeZip(files)
253+
zi = zipimport.zipimporter(TEMP_ZIP)
254+
expected_path = os.path.join(TEMP_ZIP, TESTMOD + ".py")
255+
256+
spec = zi.find_spec(TESTMOD)
257+
self.assertIsNotNone(spec)
258+
self.assertEqual(spec.origin, expected_path)
259+
self.assertEqual(zi.get_filename(TESTMOD), expected_path)
260+
261+
mod = importlib.util.module_from_spec(spec)
262+
with self.assertRaises(SyntaxError):
263+
spec.loader.exec_module(mod)
264+
265+
def test_get_filename_no_warnings(self):
266+
files = {TESTMOD + ".py": "x = 1 is 1\n"}
267+
self.makeZip(files)
268+
zi = zipimport.zipimporter(TEMP_ZIP)
269+
270+
with warnings.catch_warnings(record=True) as caught:
271+
warnings.simplefilter("always", SyntaxWarning)
272+
zi.get_filename(TESTMOD)
273+
274+
self.assertEqual(caught, [])
275+
276+
def test_syntax_error_in_source_with_valid_pyc(self):
277+
bad_src = "x =\n"
278+
files = {TESTMOD + ".py": bad_src,
279+
TESTMOD + pyc_ext: make_pyc(test_co, NOW, len(bad_src))}
280+
self.makeZip(files)
281+
zi = zipimport.zipimporter(TEMP_ZIP)
282+
expected_path = os.path.join(TEMP_ZIP, TESTMOD + pyc_ext)
283+
284+
spec = zi.find_spec(TESTMOD)
285+
self.assertIsNotNone(spec)
286+
self.assertEqual(spec.origin, expected_path)
287+
288+
mod = importlib.util.module_from_spec(spec)
289+
spec.loader.exec_module(mod)
290+
self.assertEqual(mod.get_file(), expected_path)
291+
292+
def test_syntax_error_in_package_init_no_fallthrough(self):
293+
packdir = TESTPACK + os.sep
294+
files = {packdir + "__init__.py": "x =\n",
295+
TESTPACK + ".py": test_src}
296+
self.makeZip(files)
297+
zi = zipimport.zipimporter(TEMP_ZIP)
298+
expected_path = os.path.join(TEMP_ZIP, TESTPACK, "__init__.py")
299+
300+
spec = zi.find_spec(TESTPACK)
301+
self.assertIsNotNone(spec)
302+
self.assertEqual(spec.origin, expected_path)
303+
304+
mod = importlib.util.module_from_spec(spec)
305+
with self.assertRaises(SyntaxError):
306+
spec.loader.exec_module(mod)
307+
232308
def testPyc(self):
233309
files = {TESTMOD + pyc_ext: test_pyc}
234310
self.doTest(pyc_ext, files, TESTMOD)

‎Lib/test/test_zipimport_support.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,7 @@ def test_import_filter_syntax_warnings_by_module(self):
253253
warnings.filterwarnings('error', module='test_mod')
254254
import test_pkg.test_mod
255255
self.assertEqual(sorted(wm.lineno for wm in wlog),
256-
sorted([4, 7, 10, 13, 14, 21]*2))
256+
[4, 7, 10, 13, 14, 21])
257257
filename = test_pkg.test_mod.__file__
258258
for wm in wlog:
259259
self.assertEqual(wm.filename, filename)

‎Lib/zipimport.py‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -159,11 +159,12 @@ def get_filename(self, fullname):
159159
"""get_filename(fullname) -> filename string.
160160
161161
Return the filename for the specified module or raise ZipImportError
162-
if it couldn't be imported.
162+
if it couldn't be found.
163163
"""
164164
# Deciding the filename requires working out where the code
165165
# would come from if the module was actually loaded
166-
code, ispackage, modpath = _get_module_code(self, fullname)
166+
_, _, modpath = _get_module_code(
167+
self, fullname, compile_source=False)
167168
return modpath
168169

169170

@@ -793,9 +794,9 @@ def _get_pyc_source(self, path):
793794
return _get_data(self.archive, toc_entry)
794795

795796

796-
# Get the code object associated with the module specified by
797-
# 'fullname'.
798-
def _get_module_code(self, fullname):
797+
# Get the code object associated with the module specified by 'fullname'.
798+
# If compile_source is false, return None for source code without reading or compiling it.
799+
def _get_module_code(self, fullname, *, compile_source=True):
799800
path = _get_module_path(self, fullname)
800801
import_error = None
801802
for suffix, isbytecode, ispackage in _zip_searchorder:
@@ -807,6 +808,8 @@ def _get_module_code(self, fullname):
807808
pass
808809
else:
809810
modpath = toc_entry[0]
811+
if not isbytecode and not compile_source:
812+
return None, ispackage, modpath
810813
data = _get_data(self.archive, toc_entry)
811814
code = None
812815
if isbytecode:
@@ -820,7 +823,6 @@ def _get_module_code(self, fullname):
820823
# bad magic number or non-matching mtime
821824
# in byte code, try next
822825
continue
823-
modpath = toc_entry[0]
824826
return code, ispackage, modpath
825827
else:
826828
if import_error:
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
Fix duplicate :exc:`SyntaxWarning` messages when importing source modules
2+
from ZIP archives. :meth:`zipimport.zipimporter.get_filename`, and therefore
3+
:meth:`~zipimport.zipimporter.find_spec`, no longer compile source modules. A
4+
:exc:`SyntaxError` in a zipped source module is now raised when the module is
5+
executed rather than when it is found, matching the behavior of
6+
filesystem-based loaders.

0 commit comments

Comments
 (0)