bpo-21736: Set __file__ on frozen stdlib modules.

Lib/importlib/_bootstrap.py

@@ -421,7 +421,10 @@ def has_location(self, value):
 
 def spec_from_loader(name, loader, *, origin=None, is_package=None):
     """Return a module spec based on various loader methods."""
-    if hasattr(loader, 'get_filename'):
+    if origin is None:
+        origin = getattr(loader, '_ORIGIN', None)
+
+    if not origin and hasattr(loader, 'get_filename'):
         if _bootstrap_external is None:
             raise NotImplementedError
         spec_from_file_location = _bootstrap_external.spec_from_file_location
@@ -467,12 +470,9 @@ def _spec_from_module(module, loader=None, origin=None):
     except AttributeError:
         location = None
     if origin is None:
-        if location is None:
-            try:
-                origin = loader._ORIGIN
-            except AttributeError:
-                origin = None
-        else:
+        if loader is not None:
+            origin = getattr(loader, '_ORIGIN', None)
+        if not origin and location is not None:
             origin = location
     try:
         cached = module.__cached__
@@ -484,7 +484,7 @@ def _spec_from_module(module, loader=None, origin=None):
         submodule_search_locations = None
 
     spec = ModuleSpec(name, loader, origin=origin)
-    spec._set_fileattr = False if location is None else True
+    spec._set_fileattr = False if location is None else (origin == location)
     spec.cached = cached
     spec.submodule_search_locations = submodule_search_locations
     return spec
@@ -541,6 +541,7 @@ def _init_module_attrs(spec, module, *, override=False):
     # __path__
     if override or getattr(module, '__path__', None) is None:
         if spec.submodule_search_locations is not None:
+            # XXX We should extend __path__ if it's already a list.
             try:
                 module.__path__ = spec.submodule_search_locations
             except AttributeError:
@@ -825,38 +826,127 @@ def module_repr(m):
         return '<module {!r} ({})>'.format(m.__name__, FrozenImporter._ORIGIN)
 
     @classmethod
-    def _setup_module(cls, module):
-        assert not hasattr(module, '__file__'), module.__file__
-        ispkg = hasattr(module, '__path__')
-        assert not ispkg or not module.__path__, module.__path__
+    def _fix_up_module(cls, module):
         spec = module.__spec__
-        assert not ispkg or not spec.submodule_search_locations
+        state = spec.loader_state
+        if state is None:
+            # The module is missing FrozenImporter-specific values.
 
-        if spec.loader_state is None:
-            spec.loader_state = type(sys.implementation)(
-                data=None,
-                origname=None,
-            )
-        elif not hasattr(spec.loader_state, 'data'):
-            spec.loader_state.data = None
-        if not getattr(spec.loader_state, 'origname', None):
+            # Fix up the spec attrs.
             origname = vars(module).pop('__origname__', None)
             assert origname, 'see PyImport_ImportFrozenModuleObject()'
-            spec.loader_state.origname = origname
+            ispkg = hasattr(module, '__path__')
+            assert _imp.is_frozen_package(module.__name__) == ispkg, ispkg
+            filename, pkgdir = cls._resolve_filename(origname, spec.name, ispkg)
+            spec.loader_state = type(sys.implementation)(
+                filename=filename,
+                origname=origname,
+            )
+            __path__ = spec.submodule_search_locations
+            if ispkg:
+                assert __path__ == [], __path__
+                if pkgdir:
+                    spec.submodule_search_locations.insert(0, pkgdir)
+            else:
+                assert __path__ is None, __path__
+
+            # Fix up the module attrs (the bare minimum).
+            assert not hasattr(module, '__file__'), module.__file__
+            if filename:
+                try:
+                    module.__file__ = filename
+                except AttributeError:
+                    pass
+            if ispkg:
+                if module.__path__ != __path__:
+                    assert module.__path__ == [], module.__path__
+                    module.__path__.extend(__path__)
+        else:
+            # These checks ensure that _fix_up_module() is only called
+            # in the right places.
+            __path__ = spec.submodule_search_locations
+            ispkg = __path__ is not None
+            # Check the loader state.
+            assert sorted(vars(state)) == ['filename', 'origname'], state
+            if state.origname:
+                # The only frozen modules with "origname" set are stdlib modules.
+                (__file__, pkgdir,
+                 ) = cls._resolve_filename(state.origname, spec.name, ispkg)
+                assert state.filename == __file__, (state.filename, __file__)
+                if pkgdir:
+                    assert __path__ == [pkgdir], (__path__, pkgdir)
+                else:
+                    assert __path__ == ([] if ispkg else None), __path__
+            else:
+                __file__ = None
+                assert state.filename is None, state.filename
+                assert __path__ == ([] if ispkg else None), __path__
+            # Check the file attrs.
+            if __file__:
+                assert hasattr(module, '__file__')
+                assert module.__file__ == __file__, (module.__file__, __file__)
+            else:
+                assert not hasattr(module, '__file__'), module.__file__
+            if ispkg:
+                assert hasattr(module, '__path__')
+                assert module.__path__ == __path__, (module.__path__, __path__)
+            else:
+                assert not hasattr(module, '__path__'), module.__path__
+        assert not spec.has_location
+
+    @classmethod
+    def _resolve_filename(cls, fullname, alias=None, ispkg=False):
+        if not fullname or not getattr(sys, '_stdlib_dir', None):
+            return None, None
+        try:
+            sep = cls._SEP
+        except AttributeError:
+            sep = cls._SEP = '\\' if sys.platform == 'win32' else '/'
+
+        if fullname != alias:
+            if fullname.startswith('<'):
+                fullname = fullname[1:]
+                if not ispkg:
+                    fullname = f'{fullname}.__init__'
+            else:
+                ispkg = False
+        relfile = fullname.replace('.', sep)
+        if ispkg:
+            pkgdir = f'{sys._stdlib_dir}{sep}{relfile}'
+            filename = f'{pkgdir}{sep}__init__.py'
+        else:
+            pkgdir = None
+            filename = f'{sys._stdlib_dir}{sep}{relfile}.py'
+        return filename, pkgdir
 
     @classmethod
     def find_spec(cls, fullname, path=None, target=None):
         info = _call_with_frames_removed(_imp.find_frozen, fullname)
         if info is None:
             return None
-        data, ispkg, origname = info
+        # We get the marshaled data in exec_module() (the loader
+        # part of the importer), instead of here (the finder part).
+        # The loader is the usual place to get the data that will
+        # be loaded into the module.  (For example, see _LoaderBasics
+        # in _bootstra_external.py.)  Most importantly, this importer
+        # is simpler if we wait to get the data.
+        # However, getting as much data in the finder as possible
+        # to later load the module is okay, and sometimes important.
+        # (That's why ModuleSpec.loader_state exists.)  This is
+        # especially true if it avoids throwing away expensive data
+        # the loader would otherwise duplicate later and can be done
+        # efficiently.  In this case it isn't worth it.
+        _, ispkg, origname = info
         spec = spec_from_loader(fullname, cls,
                                 origin=cls._ORIGIN,
                                 is_package=ispkg)
+        filename, pkgdir = cls._resolve_filename(origname, fullname, ispkg)
         spec.loader_state = type(sys.implementation)(
-            data=data,
+            filename=filename,
             origname=origname,
         )
+        if pkgdir:
+            spec.submodule_search_locations.insert(0, pkgdir)
         return spec
 
     @classmethod
@@ -873,26 +963,22 @@ def find_module(cls, fullname, path=None):
 
     @staticmethod
     def create_module(spec):
-        """Use default semantics for module creation."""
+        """Set __file__, if able."""
+        module = _new_module(spec.name)
+        try:
+            filename = spec.loader_state.filename
+        except AttributeError:
+            pass
+        else:
+            if filename:
+                module.__file__ = filename
+        return module
 
     @staticmethod
     def exec_module(module):
         spec = module.__spec__
         name = spec.name
-        try:
-            data = spec.loader_state.data
-        except AttributeError:
-            if not _imp.is_frozen(name):
-                raise ImportError('{!r} is not a frozen module'.format(name),
-                                  name=name)
-            data = None
-        else:
-            # We clear the extra data we got from the finder, to save memory.
-            # Note that if this method is called again (e.g. by
-            # importlib.reload()) then _imp.get_frozen_object() will notice
-            # no data was provided and will look it up.
-            spec.loader_state.data = None
-        code = _call_with_frames_removed(_imp.get_frozen_object, name, data)
+        code = _call_with_frames_removed(_imp.get_frozen_object, name)
         exec(code, module.__dict__)
 
     @classmethod
@@ -903,7 +989,16 @@ def load_module(cls, fullname):
 
         """
         # Warning about deprecation implemented in _load_module_shim().
-        return _load_module_shim(cls, fullname)
+        module = _load_module_shim(cls, fullname)
+        info = _imp.find_frozen(fullname)
+        assert info is not None
+        _, ispkg, origname = info
+        module.__origname__ = origname
+        vars(module).pop('__file__', None)
+        if ispkg:
+            module.__path__ = []
+        cls._fix_up_module(module)
+        return module
 
     @classmethod
     @_requires_frozen
@@ -1244,7 +1339,7 @@ def _setup(sys_module, _imp_module):
             spec = _spec_from_module(module, loader)
             _init_module_attrs(spec, module)
             if loader is FrozenImporter:
-                loader._setup_module(module)
+                loader._fix_up_module(module)
 
     # Directly load built-in modules needed during bootstrap.
     self_module = sys.modules[__name__]

Lib/test/test_frozen.py

@@ -39,9 +39,6 @@ def test_frozen_submodule_in_unfrozen_package(self):
         self.assertIs(spam.__spec__.loader,
                       importlib.machinery.FrozenImporter)
 
-    # This is not possible until frozen packages have __path__ set properly.
-    # See https://bugs.python.org/issue21736.
-    @unittest.expectedFailure
     def test_unfrozen_submodule_in_frozen_package(self):
         with import_helper.CleanImport('__phello__', '__phello__.spam'):
             with import_helper.frozen_modules(enabled=True):

Lib/test/test_importlib/frozen/test_finder.py

@@ -44,30 +44,31 @@ def check_loader_state(self, spec, origname=None, filename=None):
         if not filename:
             if not origname:
                 origname = spec.name
+            filename = resolve_stdlib_file(origname)
 
         actual = dict(vars(spec.loader_state))
 
-        # Check the code object used to import the frozen module.
-        # We can't compare the marshaled data directly because
-        # marshal.dumps() would mark "expected" (below) as a ref,
-        # which slightly changes the output.
-        # (See https://bugs.python.org/issue34093.)
-        data = actual.pop('data')
-        with import_helper.frozen_modules():
-            expected = _imp.get_frozen_object(spec.name)
-        code = marshal.loads(data)
-        self.assertEqual(code, expected)
-
         # Check the rest of spec.loader_state.
         expected = dict(
             origname=origname,
+            filename=filename if origname else None,
         )
         self.assertDictEqual(actual, expected)
 
     def check_search_locations(self, spec):
-        # Frozen packages do not have any path entries.
-        # (See https://bugs.python.org/issue21736.)
-        expected = []
+        """This is only called when testing packages."""
+        missing = object()
+        filename = getattr(spec.loader_state, 'filename', missing)
+        origname = getattr(spec.loader_state, 'origname', None)
+        if not origname or filename is missing:
+            # We deal with this in check_loader_state().
+            return
+        if not filename:
+            expected = []
+        elif origname != spec.name and not origname.startswith('<'):
+            expected = []
+        else:
+            expected = [os.path.dirname(filename)]
         self.assertListEqual(spec.submodule_search_locations, expected)
 
     def test_module(self):