bpo-25711: Rewrite zipimport in pure Python. (GH-6809)

Lib/ctypes/test/test_values.py

@@ -64,6 +64,7 @@ class PythonValuesTestCase(unittest.TestCase):
         bootstrap_expected = [
                 b'_frozen_importlib',
                 b'_frozen_importlib_external',
+                b'zipimport',
                 ]
         for entry in ft:
             # This is dangerous. We *can* iterate over a pointer, but

Lib/importlib/__init__.py

@@ -48,8 +48,8 @@ else:
     sys.modules['importlib._bootstrap_external'] = _bootstrap_external
 
 # To simplify imports in test code
-_w_long = _bootstrap_external._w_long
-_r_long = _bootstrap_external._r_long
+_pack_uint32 = _bootstrap_external._pack_uint32
+_unpack_uint32 = _bootstrap_external._unpack_uint32
 
 # Fully bootstrapped at this point, import whatever you like, circular
 # dependencies and startup overhead minimisation permitting :)

Lib/importlib/_bootstrap_external.py

@@ -43,14 +43,20 @@ def _make_relax_case():
     return _relax_case
 
 
-def _w_long(x):
+def _pack_uint32(x):
     """Convert a 32-bit integer to little-endian."""
     return (int(x) & 0xFFFFFFFF).to_bytes(4, 'little')
 
 
-def _r_long(int_bytes):
+def _unpack_uint32(data):
     """Convert 4 bytes in little-endian to an integer."""
-    return int.from_bytes(int_bytes, 'little')
+    assert len(data) == 4
+    return int.from_bytes(data, 'little')
+
+def _unpack_uint16(data):
+    """Convert 2 bytes in little-endian to an integer."""
+    assert len(data) == 2
+    return int.from_bytes(data, 'little')
 
 
 def _path_join(*path_parts):
@@ -503,7 +509,7 @@ def _classify_pyc(data, name, exc_details):
         message = f'reached EOF while reading pyc header of {name!r}'
         _bootstrap._verbose_message('{}', message)
         raise EOFError(message)
-    flags = _r_long(data[4:8])
+    flags = _unpack_uint32(data[4:8])
     # Only the first two flags are defined.
     if flags & ~0b11:
         message = f'invalid flags {flags!r} in {name!r}'
@@ -530,12 +536,12 @@ def _validate_timestamp_pyc(data, source_mtime, source_size, name,
     An ImportError is raised if the bytecode is stale.
 
     """
-    if _r_long(data[8:12]) != (source_mtime & 0xFFFFFFFF):
+    if _unpack_uint32(data[8:12]) != (source_mtime & 0xFFFFFFFF):
         message = f'bytecode is stale for {name!r}'
         _bootstrap._verbose_message('{}', message)
         raise ImportError(message, **exc_details)
     if (source_size is not None and
-        _r_long(data[12:16]) != (source_size & 0xFFFFFFFF)):
+        _unpack_uint32(data[12:16]) != (source_size & 0xFFFFFFFF)):
         raise ImportError(f'bytecode is stale for {name!r}', **exc_details)
 
 
@@ -579,9 +585,9 @@ def _compile_bytecode(data, name=None, bytecode_path=None, source_path=None):
 def _code_to_timestamp_pyc(code, mtime=0, source_size=0):
     "Produce the data for a timestamp-based pyc."
     data = bytearray(MAGIC_NUMBER)
-    data.extend(_w_long(0))
-    data.extend(_w_long(mtime))
-    data.extend(_w_long(source_size))
+    data.extend(_pack_uint32(0))
+    data.extend(_pack_uint32(mtime))
+    data.extend(_pack_uint32(source_size))
     data.extend(marshal.dumps(code))
     return data
 
@@ -590,7 +596,7 @@ def _code_to_hash_pyc(code, source_hash, checked=True):
     "Produce the data for a hash-based pyc."
     data = bytearray(MAGIC_NUMBER)
     flags = 0b1 | checked << 1
-    data.extend(_w_long(flags))
+    data.extend(_pack_uint32(flags))
     assert len(source_hash) == 8
     data.extend(source_hash)
     data.extend(marshal.dumps(code))

Lib/test/test_bdb.py

@@ -726,7 +726,7 @@ class StateTestCase(BaseTestCase):
                 ('line', 2, 'tfunc_import'), ('step', ),
                 ('line', 3, 'tfunc_import'), ('quit', ),
             ]
-            skip = ('importlib*', TEST_MODULE)
+            skip = ('importlib*', 'zipimport', TEST_MODULE)
             with TracerRun(self, skip=skip) as tracer:
                 tracer.runcall(tfunc_import)
 

Lib/test/test_importlib/source/test_file_loader.py

@@ -629,7 +629,7 @@ class SourceLoaderBadBytecodeTest:
                 bytecode_file.write(zeros)
             self.import_(mapping['_temp'], '_temp')
             source_mtime = os.path.getmtime(mapping['_temp'])
-            source_timestamp = self.importlib._w_long(source_mtime)
+            source_timestamp = self.importlib._pack_uint32(source_mtime)
             with open(bytecode_path, 'rb') as bytecode_file:
                 bytecode_file.seek(8)
                 self.assertEqual(bytecode_file.read(4), source_timestamp)

Lib/test/test_importlib/test_abc.py

@@ -712,9 +712,9 @@ class SourceLoader(SourceOnlyLoader):
         if magic is None:
             magic = self.util.MAGIC_NUMBER
         data = bytearray(magic)
-        data.extend(self.init._w_long(0))
-        data.extend(self.init._w_long(self.source_mtime))
-        data.extend(self.init._w_long(self.source_size))
+        data.extend(self.init._pack_uint32(0))
+        data.extend(self.init._pack_uint32(self.source_mtime))
+        data.extend(self.init._pack_uint32(self.source_size))
         code_object = compile(self.source, self.path, 'exec',
                                 dont_inherit=True)
         data.extend(marshal.dumps(code_object))
@@ -876,9 +876,9 @@ class SourceLoaderBytecodeTests(SourceLoaderTestHarness):
         if bytecode_written:
             self.assertIn(self.cached, self.loader.written)
             data = bytearray(self.util.MAGIC_NUMBER)
-            data.extend(self.init._w_long(0))
-            data.extend(self.init._w_long(self.loader.source_mtime))
-            data.extend(self.init._w_long(self.loader.source_size))
+            data.extend(self.init._pack_uint32(0))
+            data.extend(self.init._pack_uint32(self.loader.source_mtime))
+            data.extend(self.init._pack_uint32(self.loader.source_size))
             data.extend(marshal.dumps(code_object))
             self.assertEqual(self.loader.written[self.cached], bytes(data))
 

Lib/test/test_zipimport.py

@@ -551,7 +551,12 @@ class UncompressedZipImportTestCase(ImportHooksBaseTestCase):
             z.writestr(name, data)
             z.close()
             zi = zipimport.zipimporter(TEMP_ZIP)
-            self.assertEqual(data, zi.get_data(FunnyStr(name)))
+            try:
+                data2 = zi.get_data(FunnyStr(name))
+            except AttributeError:
+                pass
+            else:
+                self.assertEqual(data2, data)
         finally:
             z.close()
             os.remove(TEMP_ZIP)
@@ -677,9 +682,9 @@ class UncompressedZipImportTestCase(ImportHooksBaseTestCase):
 
         zipimport.zipimporter(filename)
         zipimport.zipimporter(os.fsencode(filename))
-        with self.assertWarns(DeprecationWarning):
+        with self.assertRaises(TypeError):
             zipimport.zipimporter(bytearray(os.fsencode(filename)))
-        with self.assertWarns(DeprecationWarning):
+        with self.assertRaises(TypeError):
             zipimport.zipimporter(memoryview(os.fsencode(filename)))
 
     @support.cpython_only
@@ -687,14 +692,14 @@ class UncompressedZipImportTestCase(ImportHooksBaseTestCase):
         # The interpreter shouldn't crash in case of calling methods of an
         # uninitialized zipimport.zipimporter object.
         zi = zipimport.zipimporter.__new__(zipimport.zipimporter)
-        self.assertRaises(ValueError, zi.find_module, 'foo')
-        self.assertRaises(ValueError, zi.find_loader, 'foo')
-        self.assertRaises(ValueError, zi.load_module, 'foo')
-        self.assertRaises(ValueError, zi.get_filename, 'foo')
-        self.assertRaises(ValueError, zi.is_package, 'foo')
-        self.assertRaises(ValueError, zi.get_data, 'foo')
-        self.assertRaises(ValueError, zi.get_code, 'foo')
-        self.assertRaises(ValueError, zi.get_source, 'foo')
+        self.assertRaises((ValueError, AttributeError), zi.find_module, 'foo')
+        self.assertRaises((ValueError, AttributeError), zi.find_loader, 'foo')
+        self.assertRaises((ValueError, AttributeError), zi.load_module, 'foo')
+        self.assertRaises((ValueError, AttributeError), zi.get_filename, 'foo')
+        self.assertRaises((ValueError, AttributeError), zi.is_package, 'foo')
+        self.assertRaises((ValueError, AttributeError), zi.get_data, 'foo')
+        self.assertRaises((ValueError, AttributeError), zi.get_code, 'foo')
+        self.assertRaises((ValueError, AttributeError), zi.get_source, 'foo')
 
 
 @support.requires_zlib
@@ -712,7 +717,7 @@ class CompressedZipImportTestCase(UncompressedZipImportTestCase):
             zip_file.writestr('bar.py', b'print("hello world")', ZIP_DEFLATED)
         zi = zipimport.zipimporter(TEMP_ZIP)
         with support.swap_attr(zlib, 'decompress', bad_decompress):
-            self.assertRaises(TypeError, zi.get_source, 'bar')
+            self.assertRaises((TypeError, AttributeError), zi.get_source, 'bar')
 
 
 class BadFileZipImportTestCase(unittest.TestCase):

Makefile.pre.in

@@ -713,16 +713,22 @@ Programs/_freeze_importlib: Programs/_freeze_importlib.o $(LIBRARY_OBJS_OMIT_FRO
 regen-importlib: Programs/_freeze_importlib
 	# Regenerate Python/importlib_external.h
 	# from Lib/importlib/_bootstrap_external.py using _freeze_importlib
-	./Programs/_freeze_importlib \
+	./Programs/_freeze_importlib importlib._bootstrap_external \
 	    $(srcdir)/Lib/importlib/_bootstrap_external.py \
 	    $(srcdir)/Python/importlib_external.h.new
 	$(UPDATE_FILE) $(srcdir)/Python/importlib_external.h $(srcdir)/Python/importlib_external.h.new
 	# Regenerate Python/importlib.h from Lib/importlib/_bootstrap.py
 	# using _freeze_importlib
-	./Programs/_freeze_importlib \
+	./Programs/_freeze_importlib importlib._bootstrap \
 	    $(srcdir)/Lib/importlib/_bootstrap.py \
 	    $(srcdir)/Python/importlib.h.new
 	$(UPDATE_FILE) $(srcdir)/Python/importlib.h $(srcdir)/Python/importlib.h.new
+	# Regenerate Python/importlib_zipimport.h from Lib/zipimport.py
+	# using _freeze_importlib
+	./Programs/_freeze_importlib zipimport \
+	    $(srcdir)/Lib/zipimport.py \
+	    $(srcdir)/Python/importlib_zipimport.h.new
+	$(UPDATE_FILE) $(srcdir)/Python/importlib_zipimport.h $(srcdir)/Python/importlib_zipimport.h.new
 
 
 ############################################################################
@@ -897,7 +903,8 @@ regen-opcode-targets:
 
 Python/ceval.o: $(srcdir)/Python/opcode_targets.h $(srcdir)/Python/ceval_gil.h
 
-Python/frozen.o: $(srcdir)/Python/importlib.h $(srcdir)/Python/importlib_external.h
+Python/frozen.o: $(srcdir)/Python/importlib.h $(srcdir)/Python/importlib_external.h \
+		$(srcdir)/Python/importlib_zipimport.h
 
 # Generate DTrace probe macros, then rename them (PYTHON_ -> PyDTrace_) to
 # follow our naming conventions. dtrace(1) uses the output filename to generate

Misc/NEWS.d/next/Core and Builtins/2018-05-14-18-54-03.bpo-25711.9xfq-v.rst

+The :mod:`zipimport` module has been rewritten in pure Python.

Modules/Setup

@@ -125,10 +125,6 @@ _locale _localemodule.c  # -lintl
 # Standard I/O baseline
 _io -DPy_BUILD_CORE -I$(srcdir)/Modules/_io _io/_iomodule.c _io/iobase.c _io/fileio.c _io/bytesio.c _io/bufferedio.c _io/textio.c _io/stringio.c
 
-# The zipimport module is always imported at startup. Having it as a
-# builtin module avoids some bootstrapping problems and reduces overhead.
-zipimport -DPy_BUILD_CORE zipimport.c
-
 # faulthandler module
 faulthandler faulthandler.c
 

PC/config.c

@@ -35,7 +35,6 @@ extern PyObject* PyInit__weakref(void);
 /* XXX: These two should really be extracted to standalone extensions. */
 extern PyObject* PyInit_xxsubtype(void);
 extern PyObject* PyInit__xxsubinterpreters(void);
-extern PyObject* PyInit_zipimport(void);
 extern PyObject* PyInit__random(void);
 extern PyObject* PyInit_itertools(void);
 extern PyObject* PyInit__collections(void);
@@ -131,7 +130,6 @@ struct _inittab _PyImport_Inittab[] = {
 
     {"xxsubtype", PyInit_xxsubtype},
     {"_xxsubinterpreters", PyInit__xxsubinterpreters},
-    {"zipimport", PyInit_zipimport},
 #ifdef _Py_HAVE_ZLIB
     {"zlib", PyInit_zlib},
 #endif

PCbuild/_freeze_importlib.vcxproj

@@ -77,19 +77,26 @@
   </ItemGroup>
   <ItemGroup>
     <None Include="..\Lib\importlib\_bootstrap.py">
+      <ModName>importlib._bootstrap</ModName>
       <IntFile>$(IntDir)importlib.g.h</IntFile>
       <OutFile>$(PySourcePath)Python\importlib.h</OutFile>
     </None>
     <None Include="..\Lib\importlib\_bootstrap_external.py">
+      <ModName>importlib._bootstrap_external</ModName>
       <IntFile>$(IntDir)importlib_external.g.h</IntFile>
       <OutFile>$(PySourcePath)Python\importlib_external.h</OutFile>
     </None>
+    <None Include="..\Lib\zipimport.py">
+      <ModName>zipimport</ModName>
+      <IntFile>$(IntDir)importlib_zipimport.g.h</IntFile>
+      <OutFile>$(PySourcePath)Python\importlib_zipimport.h</OutFile>
+    </None>
   </ItemGroup>
   <Import Project="$(VCTargetsPath)\Microsoft.Cpp.targets" />
   <ImportGroup Label="ExtensionTargets">
   </ImportGroup>
   <Target Name="_RebuildImportLib">
-    <Exec Command='"$(TargetPath)" "%(None.FullPath)" "%(None.IntFile)"' />
+    <Exec Command='"$(TargetPath)" "%(None.ModName)" "%(None.FullPath)" "%(None.IntFile)"' />
     
     <PropertyGroup>
       <_OldContent Condition="Exists($(OutTargetPath))"></_OldContent>
@@ -114,6 +121,7 @@
     <ItemGroup>
       <Clean Include="$(IntDir)importlib.g.h" />
       <Clean Include="$(IntDir)importlib_external.g.h" />
+      <Clean Include="$(IntDir)importlib_zipimport.g.h" />
     </ItemGroup>
   </Target>
 </Project>

PCbuild/pythoncore.vcxproj

@@ -282,7 +282,6 @@
     <ClCompile Include="..\Modules\timemodule.c" />
     <ClCompile Include="..\Modules\xxsubtype.c" />
     <ClCompile Include="..\Modules\_xxsubinterpretersmodule.c" />
-    <ClCompile Include="..\Modules\zipimport.c" />
     <ClCompile Include="..\Modules\_io\fileio.c" />
     <ClCompile Include="..\Modules\_io\bytesio.c" />
     <ClCompile Include="..\Modules\_io\stringio.c" />

PCbuild/pythoncore.vcxproj.filters

@@ -617,9 +617,6 @@
     <ClCompile Include="..\Modules\xxsubtype.c">
       <Filter>Modules</Filter>
     </ClCompile>
-    <ClCompile Include="..\Modules\zipimport.c">
-      <Filter>Modules</Filter>
-    </ClCompile>
     <ClCompile Include="..\Modules\zlibmodule.c">
       <Filter>Modules</Filter>
     </ClCompile>

Programs/_freeze_importlib.c

@@ -27,28 +27,30 @@ static const struct _frozen _PyImport_FrozenModules[] = {
 const struct _frozen *PyImport_FrozenModules;
 #endif
 
-const char header[] = "/* Auto-generated by Programs/_freeze_importlib.c */";
+static const char header[] =
+    "/* Auto-generated by Programs/_freeze_importlib.c */";
 
 int
 main(int argc, char *argv[])
 {
-    char *inpath, *outpath, *code_name;
+    const char *name, *inpath, *outpath;
+    char buf[100];
     FILE *infile = NULL, *outfile = NULL;
     struct _Py_stat_struct status;
-    size_t text_size, data_size, n;
+    size_t text_size, data_size, i, n;
     char *text = NULL;
     unsigned char *data;
     PyObject *code = NULL, *marshalled = NULL;
-    int is_bootstrap = 1;
 
     PyImport_FrozenModules = _PyImport_FrozenModules;
 
-    if (argc != 3) {
-        fprintf(stderr, "need to specify input and output paths\n");
+    if (argc != 4) {
+        fprintf(stderr, "need to specify the name, input and output paths\n");
         return 2;
     }
-    inpath = argv[1];
-    outpath = argv[2];
+    name = argv[1];
+    inpath = argv[2];
+    outpath = argv[3];
     infile = fopen(inpath, "rb");
     if (infile == NULL) {
         fprintf(stderr, "cannot open '%s' for reading\n", inpath);
@@ -90,14 +92,8 @@ main(int argc, char *argv[])
         _Py_FatalInitError(err);
     }
 
-    if (strstr(inpath, "_external") != NULL) {
-        is_bootstrap = 0;
-    }
-
-    code_name = is_bootstrap ?
-        "<frozen importlib._bootstrap>" :
-        "<frozen importlib._bootstrap_external>";
-    code = Py_CompileStringExFlags(text, code_name, Py_file_input, NULL, 0);
+    sprintf(buf, "<frozen %s>", name);
+    code = Py_CompileStringExFlags(text, buf, Py_file_input, NULL, 0);
     if (code == NULL)
         goto error;
     free(text);
@@ -120,11 +116,13 @@ main(int argc, char *argv[])
         goto error;
     }
     fprintf(outfile, "%s\n", header);
-    if (is_bootstrap)
-        fprintf(outfile, "const unsigned char _Py_M__importlib[] = {\n");
-    else
-        fprintf(outfile,
-                "const unsigned char _Py_M__importlib_external[] = {\n");
+    for (i = n = 0; name[i] != '\0'; i++) {
+        if (name[i] != '.') {
+            buf[n++] = name[i];
+        }
+    }
+    buf[n] = '\0';
+    fprintf(outfile, "const unsigned char _Py_M__%s[] = {\n", buf);
     for (n = 0; n < data_size; n += 16) {
         size_t i, end = Py_MIN(n + 16, data_size);
         fprintf(outfile, "    ");

Python/frozen.c

@@ -4,6 +4,7 @@
 #include "Python.h"
 #include "importlib.h"
 #include "importlib_external.h"
+#include "importlib_zipimport.h"
 
 /* In order to test the support for frozen modules, by default we
    define a single frozen module, __hello__.  Loading it will print
@@ -29,9 +30,12 @@ static unsigned char M___hello__[] = {
 
 static const struct _frozen _PyImport_FrozenModules[] = {
     /* importlib */
-    {"_frozen_importlib", _Py_M__importlib, (int)sizeof(_Py_M__importlib)},
-    {"_frozen_importlib_external", _Py_M__importlib_external,
-        (int)sizeof(_Py_M__importlib_external)},
+    {"_frozen_importlib", _Py_M__importlib_bootstrap,
+        (int)sizeof(_Py_M__importlib_bootstrap)},
+    {"_frozen_importlib_external", _Py_M__importlib_bootstrap_external,
+        (int)sizeof(_Py_M__importlib_bootstrap_external)},
+    {"zipimport", _Py_M__zipimport,
+        (int)sizeof(_Py_M__zipimport)},
     /* Test module */
     {"__hello__", M___hello__, SIZE},
     /* Test package (negative size indicates package-ness) */

Python/importlib.h

 /* Auto-generated by Programs/_freeze_importlib.c */
-const unsigned char _Py_M__importlib[] = {
+const unsigned char _Py_M__importlib_bootstrap[] = {
     99,0,0,0,0,0,0,0,0,0,0,0,0,4,0,0,
     0,64,0,0,0,115,208,1,0,0,100,0,90,0,100,1,
     97,1,100,2,100,3,132,0,90,2,100,4,100,5,132,0,

Python/pylifecycle.c

@@ -191,7 +191,6 @@ initimport(PyInterpreterState *interp, PyObject *sysmod)
     PyObject *importlib;
     PyObject *impmod;
     PyObject *value;
-    _PyInitError err;
 
     /* Import _importlib through its frozen version, _frozen_importlib. */
     if (PyImport_ImportFrozenModule("_frozen_importlib") <= 0) {
@@ -233,11 +232,6 @@ initimport(PyInterpreterState *interp, PyObject *sysmod)
     Py_DECREF(value);
     Py_DECREF(impmod);
 
-    err = _PyImportZip_Init();
-    if (_Py_INIT_FAILED(err)) {
-        return err;
-    }
-
     return _Py_INIT_OK();
 }
 
@@ -252,7 +246,7 @@ initexternalimport(PyInterpreterState *interp)
         return _Py_INIT_ERR("external importer setup failed");
     }
     Py_DECREF(value);
-    return _Py_INIT_OK();
+    return _PyImportZip_Init();
 }
 
 /* Helper functions to better handle the legacy C locale