bpo-32303 - Consistency fixes for namespace loaders (GH-5481) (#5503)
* 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 bbbcf8693b876daae4469765aa62f8924f39a7d2)
Co-authored-by: Barry Warsaw <barry@python.org>
diff --git a/Lib/importlib/_bootstrap.py b/Lib/importlib/_bootstrap.py
index 5df6aa0..2bdd192 100644
--- a/Lib/importlib/_bootstrap.py
+++ b/Lib/importlib/_bootstrap.py
@@ -522,6 +522,18 @@
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:
diff --git a/Lib/importlib/_bootstrap_external.py b/Lib/importlib/_bootstrap_external.py
index d3f58af..f9a708c 100644
--- a/Lib/importlib/_bootstrap_external.py
+++ b/Lib/importlib/_bootstrap_external.py
@@ -1276,9 +1276,9 @@
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:
diff --git a/Lib/importlib/resources.py b/Lib/importlib/resources.py
index bf6d703..c4f6bbd 100644
--- a/Lib/importlib/resources.py
+++ b/Lib/importlib/resources.py
@@ -66,6 +66,11 @@
return None
+def _check_location(package):
+ if package.__spec__.origin is None or not package.__spec__.has_location:
+ raise FileNotFoundError(f'Package has no location {package!r}')
+
+
def open_binary(package: Package, resource: Resource) -> BinaryIO:
"""Return a file-like object opened for binary reading of the resource."""
resource = _normalize_path(resource)
@@ -73,6 +78,7 @@
reader = _get_resource_reader(package)
if reader is not None:
return reader.open_resource(resource)
+ _check_location(package)
absolute_package_path = os.path.abspath(package.__spec__.origin)
package_path = os.path.dirname(absolute_package_path)
full_path = os.path.join(package_path, resource)
@@ -106,6 +112,7 @@
reader = _get_resource_reader(package)
if reader is not None:
return TextIOWrapper(reader.open_resource(resource), encoding, errors)
+ _check_location(package)
absolute_package_path = os.path.abspath(package.__spec__.origin)
package_path = os.path.dirname(absolute_package_path)
full_path = os.path.join(package_path, resource)
@@ -172,6 +179,8 @@
return
except FileNotFoundError:
pass
+ else:
+ _check_location(package)
# Fall-through for both the lack of resource_path() *and* if
# resource_path() raises FileNotFoundError.
package_directory = Path(package.__spec__.origin).parent
@@ -232,9 +241,9 @@
yield from reader.contents()
return
# Is the package a namespace package? By definition, namespace packages
- # cannot have resources.
- if (package.__spec__.origin == 'namespace' and
- not package.__spec__.has_location):
+ # cannot have resources. We could use _check_location() and catch the
+ # exception, but that's extra work, so just inline the check.
+ if package.__spec__.origin is None or not package.__spec__.has_location:
return []
package_directory = Path(package.__spec__.origin).parent
yield from os.listdir(str(package_directory))
diff --git a/Lib/test/test_importlib/test_api.py b/Lib/test/test_importlib/test_api.py
index 50dbfe1..8beb424 100644
--- a/Lib/test/test_importlib/test_api.py
+++ b/Lib/test/test_importlib/test_api.py
@@ -305,6 +305,7 @@
expected = {'__name__': name,
'__package__': name,
'__doc__': None,
+ '__file__': None,
}
os.mkdir(name)
with open(bad_path, 'w') as init_file:
@@ -316,8 +317,9 @@
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):
diff --git a/Lib/test/test_importlib/test_namespace_pkgs.py b/Lib/test/test_importlib/test_namespace_pkgs.py
index e37d8a1..8e4b5d6 100644
--- a/Lib/test/test_importlib/test_namespace_pkgs.py
+++ b/Lib/test/test_importlib/test_namespace_pkgs.py
@@ -317,5 +317,21 @@
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()