diff --git a/test/BUILD b/test/BUILD index d12cad3f..d270fdae 100644 --- a/test/BUILD +++ b/test/BUILD @@ -185,6 +185,17 @@ python_test( srcs = ["interpreter_not_included_test.py"], ) +# Regression test for ModuleDirImport not setting submodule_search_locations on the specs it +# creates for module_dir packages, which breaks importlib.resources. +python_test( + name = "module_dir_import_resources_test", + srcs = ["module_dir_import_resources_test.py"], + deps = [ + "//third_party/python:pygments", + ], + labels = ["manual"], +) + python_test( name = "module_dir_import_test", srcs = ["module_dir_import_test.py"], @@ -199,7 +210,7 @@ python_test( deps = [ "//third_party/python:confluent-kafka", ], - labels = ["manual"], + labels = ["manual"] if CONFIG.OS == "freebsd" else [], ) plugin_e2e_test( diff --git a/test/module_dir_import_resources_test.py b/test/module_dir_import_resources_test.py new file mode 100644 index 00000000..bc8fb407 --- /dev/null +++ b/test/module_dir_import_resources_test.py @@ -0,0 +1,26 @@ +import importlib.resources +import unittest + + +class ModuleDirImportResourcesTest(unittest.TestCase): + """Tests that importlib.resources works for a package imported via its Please + module_dir alias (e.g. `third_party.python.pygments`), not just its plain top-level + name (`pygments`). + + ModuleDirImport aliases third_party.python.X to the same module object as X, but the + ModuleSpec it hands back for the aliased name doesn't carry over submodule_search_locations + from the real module's spec. Because both names share the same module object, + importlib._bootstrap unconditionally overwrites module.__spec__ with that incomplete spec + as soon as the aliased name is imported, which then breaks importlib.resources for both + names (see https://docs.python.org/3/library/importlib.html#importlib.machinery.ModuleSpec). + """ + + def test_read_resource_text_file_via_module_dir_alias(self): + init = importlib.resources.files("third_party.python.pygments") / "__init__.py" + self.assertIn("__version__ = '2.19.2'", init.read_text()) + + def test_read_resource_text_file_via_plain_name_after_alias_import(self): + import third_party.python.pygments # noqa: F401 (import triggers the aliasing) + + init = importlib.resources.files("pygments") / "__init__.py" + self.assertIn("__version__ = '2.19.2'", init.read_text()) diff --git a/third_party/python/BUILD b/third_party/python/BUILD index 1da39021..e5c8b1c3 100644 --- a/third_party/python/BUILD +++ b/third_party/python/BUILD @@ -505,7 +505,7 @@ python_wheel( pip_library( name = "confluent-kafka", licences = ["Apache-2.0"], - version = "2.6.1", + version = "2.15.1", test_only = True, zip_safe = False, ) diff --git a/tools/please_pex/ChangeLog b/tools/please_pex/ChangeLog index 4dfbdd6c..429937e6 100644 --- a/tools/please_pex/ChangeLog +++ b/tools/please_pex/ChangeLog @@ -1,3 +1,7 @@ +Version 3.0.3 +------------- + * Fix `importlib.resources` for some usages relating to packages in module_dir (#324) + Version 3.0.2 ------------- * Search `PATH` when .pex is invoked at a relative path with no `/` (#299) diff --git a/tools/please_pex/VERSION b/tools/please_pex/VERSION index b5021469..75a22a26 100644 --- a/tools/please_pex/VERSION +++ b/tools/please_pex/VERSION @@ -1 +1 @@ -3.0.2 +3.0.3 diff --git a/tools/please_pex/pex/plz.py b/tools/please_pex/pex/plz.py index 25eda0d4..aa396e1a 100644 --- a/tools/please_pex/pex/plz.py +++ b/tools/please_pex/pex/plz.py @@ -123,6 +123,8 @@ class ModuleDirImport(MetaPathFinder): def __init__(self, module_dir): self.prefix = module_dir.replace("/", ".") + "." self._distributions = self._find_all_distributions(module_dir) + # Real (unaliased) specs of modules we've imported, keyed by their aliased name. + self._real_specs = {} def _find_all_distributions(self, module_dir): pex_file = sys.argv[0] @@ -149,12 +151,26 @@ def find_spec(self, name, path, target=None): def create_module(self, spec): """Actually load a module that we said we'd handle in find_module.""" module = import_module(spec.name.removeprefix(self.prefix)) + # Carry over submodule_search_locations from the real module's spec, otherwise + # importlib.resources (which relies on it to find a package's files) breaks once + # the module's __spec__ gets overwritten with this one. + real_spec = module.__spec__ + spec.submodule_search_locations = real_spec.submodule_search_locations + self._real_specs[spec.name] = real_spec sys.modules[spec.name] = module return module def exec_module(self, mod): """Nothing to do, create_module already did the work.""" + def get_resource_reader(self, fullname): + """Delegates to the real module's loader so importlib.resources can find its files.""" + real_spec = self._real_specs.get(fullname) + if real_spec is not None: + reader = getattr(real_spec.loader, "get_resource_reader", None) + if reader is not None: + return reader(real_spec.name) + def find_distributions(self, context): """Return an iterable of all Distribution instances capable of loading the metadata for packages for the indicated ``context``.