Skip to content

Commit

Permalink
[3.6] bpo-32303 - Consistency fixes for namespace loaders (GH-5481) (#…
Browse files Browse the repository at this point in the history
…5504)

* Make sure ``__spec__.loader`` matches ``__loader__`` for namespace packages.
* Make sure ``__spec__.origin` matches ``__file__`` for namespace packages.

https://bugs.python.org/issue32303
https://bugs.python.org/issue32305.
(cherry picked from commit bbbcf86)

Co-authored-by: Barry Warsaw <[email protected]>
  • Loading branch information
warsaw authored Feb 3, 2018
1 parent 7e4cf8e commit a71397f
Show file tree
Hide file tree
Showing 9 changed files with 1,473 additions and 1,439 deletions.
2 changes: 1 addition & 1 deletion Doc/library/importlib.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1077,7 +1077,7 @@ find and load modules.
Name of the place from which the module is loaded, e.g. "builtin" for
built-in modules and the filename for modules loaded from source.
Normally "origin" should be set, but it may be ``None`` (the default)
which indicates it is unspecified.
which indicates it is unspecified (e.g. for namespace packages).

.. attribute:: submodule_search_locations

Expand Down
12 changes: 12 additions & 0 deletions Lib/importlib/_bootstrap.py
Original file line number Diff line number Diff line change
Expand Up @@ -522,6 +522,18 @@ def _init_module_attrs(spec, module, *, override=False):

loader = _NamespaceLoader.__new__(_NamespaceLoader)
loader._path = spec.submodule_search_locations
spec.loader = loader
# While the docs say that module.__file__ is not set for
# built-in modules, and the code below will avoid setting it if
# spec.has_location is false, this is incorrect for namespace
# packages. Namespace packages have no location, but their
# __spec__.origin is None, and thus their module.__file__
# should also be None for consistency. While a bit of a hack,
# this is the best place to ensure this consistency.
#
# See # https://docs.python.org/3/library/importlib.html#importlib.abc.Loader.load_module
# and bpo-32305
module.__file__ = None
try:
module.__loader__ = loader
except AttributeError:
Expand Down
6 changes: 3 additions & 3 deletions Lib/importlib/_bootstrap_external.py
Original file line number Diff line number Diff line change
Expand Up @@ -1160,9 +1160,9 @@ def find_spec(cls, fullname, path=None, target=None):
elif spec.loader is None:
namespace_path = spec.submodule_search_locations
if namespace_path:
# We found at least one namespace path. Return a
# spec which can create the namespace package.
spec.origin = 'namespace'
# We found at least one namespace path. Return a spec which
# can create the namespace package.
spec.origin = None
spec.submodule_search_locations = _NamespacePath(fullname, namespace_path, cls._get_spec)
return spec
else:
Expand Down
6 changes: 4 additions & 2 deletions Lib/test/test_importlib/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,7 @@ def test_reload_namespace_changed(self):
expected = {'__name__': name,
'__package__': name,
'__doc__': None,
'__file__': None,
}
os.mkdir(name)
with open(bad_path, 'w') as init_file:
Expand All @@ -318,8 +319,9 @@ def test_reload_namespace_changed(self):
spec = ns.pop('__spec__')
ns.pop('__builtins__', None) # An implementation detail.
self.assertEqual(spec.name, name)
self.assertIs(spec.loader, None)
self.assertIsNot(loader, None)
self.assertIsNotNone(spec.loader)
self.assertIsNotNone(loader)
self.assertEqual(spec.loader, loader)
self.assertEqual(set(path),
set([os.path.dirname(bad_path)]))
with self.assertRaises(AttributeError):
Expand Down
16 changes: 16 additions & 0 deletions Lib/test/test_importlib/test_namespace_pkgs.py
Original file line number Diff line number Diff line change
Expand Up @@ -317,5 +317,21 @@ def test_dynamic_path(self):
self.assertEqual(foo.two.attr, 'portion2 foo two')


class LoaderTests(NamespacePackageTest):
paths = ['portion1']

def test_namespace_loader_consistency(self):
# bpo-32303
import foo
self.assertEqual(foo.__loader__, foo.__spec__.loader)
self.assertIsNotNone(foo.__loader__)

def test_namespace_origin_consistency(self):
# bpo-32305
import foo
self.assertIsNone(foo.__spec__.origin)
self.assertIsNone(foo.__file__)


if __name__ == "__main__":
unittest.main()
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Make sure ``__spec__.loader`` matches ``__loader__`` for namespace packages.
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
For namespace packages, ensure that both ``__file__`` and
``__spec__.origin`` are set to None.
1,891 changes: 946 additions & 945 deletions Python/importlib.h

Large diffs are not rendered by default.

976 changes: 488 additions & 488 deletions Python/importlib_external.h

Large diffs are not rendered by default.

3 comments on commit a71397f

@EdwardBetts
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change breaks the gunicorn reloader, it doesn't expect module.__file__ to be None.

I've patched gunicorn so it works with this change to cpython. (benoitc/gunicorn#1708).

$ gunicorn3 autoapp:app -b [::1] --reload
[2018-02-19 12:16:56 +0000] [23897] [INFO] Starting gunicorn 19.7.1
[2018-02-19 12:16:56 +0000] [23897] [INFO] Listening at: http://[::1]:8000 (23897)
[2018-02-19 12:16:56 +0000] [23901] [INFO] Booting worker with pid: 23901
Exception in thread Thread-1:
Traceback (most recent call last):
  File "/usr/lib/python3.6/threading.py", line 916, in _bootstrap_inner
    self.run()
  File "/usr/lib/python3/dist-packages/gunicorn/reloader.py", line 42, in run
    for filename in self.get_files():
  File "/usr/lib/python3/dist-packages/gunicorn/reloader.py", line 30, in get_files
    for module in list(sys.modules.values())
  File "/usr/lib/python3/dist-packages/gunicorn/reloader.py", line 31, in <listcomp>
    if hasattr(module, '__file__')
  File "/usr/lib/python3.6/re.py", line 191, in sub
    return _compile(pattern, flags).sub(repl, string, count)
TypeError: expected string or bytes-like object

@warsaw
Copy link
Member Author

@warsaw warsaw commented on a71397f Feb 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gunicorn should use if getattr(module, '__file__', None).

@EdwardBetts
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@warsaw Agreed, that is my fix.

Please sign in to comment.