cpython: Issue #18070: importlib.util.module_for_loader() now sets __loader__
http://hg.python.org/cpython/rev/185a0e649fb8 changeset: 83963:185a0e649fb8 user: Brett Cannon <brett@python.org> date: Tue May 28 18:35:54 2013 -0400 summary: Issue #18070: importlib.util.module_for_loader() now sets __loader__ and __package__ unconditionally in order to do the right thing for reloading. files: Doc/library/importlib.rst | 22 +- Doc/whatsnew/3.4.rst | 7 +- Lib/importlib/_bootstrap.py | 35 +- Lib/test/test_importlib/test_util.py | 13 +- Misc/NEWS | 3 + Python/importlib.h | 6879 ++++++------- 6 files changed, 3468 insertions(+), 3491 deletions(-) diff --git a/Doc/library/importlib.rst b/Doc/library/importlib.rst --- a/Doc/library/importlib.rst +++ b/Doc/library/importlib.rst @@ -811,12 +811,11 @@ The decorated method will take in the **name** of the module to be loaded as expected for a :term:`loader`. If the module is not found in - :data:`sys.modules` then a new one is constructed with its - :attr:`__name__` attribute set to **name**, :attr:`__loader__` set to - **self**, and :attr:`__package__` set based on what - :meth:`importlib.abc.InspectLoader.is_package` returns (if available). If a - new module is not needed then the module found in :data:`sys.modules` will - be passed into the method. + :data:`sys.modules` then a new one is constructed. Regardless of where the + module came from, :attr:`__loader__` set to **self** and :attr:`__package__` + is set based on what :meth:`importlib.abc.InspectLoader.is_package` returns + (if available). These attributes are set unconditionally to support + reloading. If an exception is raised by the decorated method and a module was added to :data:`sys.modules` it will be removed to prevent a partially initialized @@ -831,6 +830,10 @@ :attr:`__loader__` and :attr:`__package__` are automatically set (when possible). + .. versionchanged:: 3.4 + Set :attr:`__loader__` :attr:`__package__` unconditionally to support + reloading. + .. decorator:: set_loader A :term:`decorator` for :meth:`importlib.abc.Loader.load_module` @@ -843,12 +846,13 @@ .. note:: As this decorator sets :attr:`__loader__` after loading the module, it is recommended to use :func:`module_for_loader` instead when appropriate. + This decorator is also redundant as of Python 3.3 as import itself will + set these attributes post-import if necessary. .. versionchanged:: 3.4 Set ``__loader__`` if set to ``None``, as if the attribute does not exist. - .. decorator:: set_package A :term:`decorator` for :meth:`importlib.abc.Loader.load_module` to set the :attr:`__package__` attribute on the returned module. If :attr:`__package__` @@ -856,4 +860,6 @@ .. note:: As this decorator sets :attr:`__package__` after loading the module, it is - recommended to use :func:`module_for_loader` instead when appropriate. + recommended to use :func:`module_for_loader` instead when appropriate. As + of Python 3.3 this decorator is also redundant as import will set + :attr:`__package__` post-import if necessary. diff --git a/Doc/whatsnew/3.4.rst b/Doc/whatsnew/3.4.rst --- a/Doc/whatsnew/3.4.rst +++ b/Doc/whatsnew/3.4.rst @@ -250,4 +250,9 @@ * The module type now initializes the :attr:`__package__` and :attr:`__loader__` attributes to ``None`` by default. To determine if these attributes were set in a backwards-compatible fashion, use e.g. - ``getattr(module, '__loader__', None) is not None``. \ No newline at end of file + ``getattr(module, '__loader__', None) is not None``. + +* :meth:`importlib.util.module_for_loader` now sets ``__loader__`` and + ``__package__`` unconditionally to properly support reloading. If this is not + desired then you will need to set these attributes manually. You can use + :class:`importlib.util.ModuleManager` for module management. diff --git a/Lib/importlib/_bootstrap.py b/Lib/importlib/_bootstrap.py --- a/Lib/importlib/_bootstrap.py +++ b/Lib/importlib/_bootstrap.py @@ -37,23 +37,13 @@ return _relax_case -# TODO: Expose from marshal def _w_long(x): - """Convert a 32-bit integer to little-endian. - - XXX Temporary until marshal's long functions are exposed. - - """ + """Convert a 32-bit integer to little-endian.""" return (int(x) & 0xFFFFFFFF).to_bytes(4, 'little') -# TODO: Expose from marshal def _r_long(int_bytes): - """Convert 4 bytes in little-endian to an integer. - - XXX Temporary until marshal's long function are exposed. - - """ + """Convert 4 bytes in little-endian to an integer.""" return int.from_bytes(int_bytes, 'little') @@ -569,17 +559,7 @@ """ def module_for_loader_wrapper(self, fullname, *args, **kwargs): - module = sys.modules.get(fullname) - is_reload = module is not None - if not is_reload: - # This must be done before open() is called as the 'io' module - # implicitly imports 'locale' and would otherwise trigger an - # infinite loop. - module = new_module(fullname) - # This must be done before putting the module in sys.modules - # (otherwise an optimization shortcut in import.c becomes wrong) - module.__initializing__ = True - sys.modules[fullname] = module + with ModuleManager(fullname) as module: module.__loader__ = self try: is_package = self.is_package(fullname) @@ -590,17 +570,8 @@ module.__package__ = fullname else: module.__package__ = fullname.rpartition('.')[0] - else: - module.__initializing__ = True - try: # If __package__ was not set above, __import__() will do it later. return fxn(self, module, *args, **kwargs) - except: - if not is_reload: - del sys.modules[fullname] - raise - finally: - module.__initializing__ = False _wrap(module_for_loader_wrapper, fxn) return module_for_loader_wrapper diff --git a/Lib/test/test_importlib/test_util.py b/Lib/test/test_importlib/test_util.py --- a/Lib/test/test_importlib/test_util.py +++ b/Lib/test/test_importlib/test_util.py @@ -85,12 +85,23 @@ def test_reload(self): # Test that a module is reused if already in sys.modules. + class FakeLoader: + def is_package(self, name): + return True + @util.module_for_loader + def load_module(self, module): + return module name = 'a.b.c' module = imp.new_module('a.b.c') + module.__loader__ = 42 + module.__package__ = 42 with test_util.uncache(name): sys.modules[name] = module - returned_module = self.return_module(name) + loader = FakeLoader() + returned_module = loader.load_module(name) self.assertIs(returned_module, sys.modules[name]) + self.assertEqual(module.__loader__, loader) + self.assertEqual(module.__package__, name) def test_new_module_failure(self): # Test that a module is removed from sys.modules if added but an diff --git a/Misc/NEWS b/Misc/NEWS --- a/Misc/NEWS +++ b/Misc/NEWS @@ -96,6 +96,9 @@ Library ------- +- Issue #18070: Have importlib.util.module_for_loader() set attributes + unconditionally in order to properly support reloading. + - Add importlib.util.ModuleManager as a context manager to provide the proper module object to load. diff --git a/Python/importlib.h b/Python/importlib.h --- a/Python/importlib.h +++ b/Python/importlib.h [stripped] -- Repository URL: http://hg.python.org/cpython
participants (1)
-
brett.cannon