From 17ddcd0a5211099e3010f9d72b84629082a0d8e6 Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Sat, 8 Oct 2022 15:07:21 -0400 Subject: [PATCH 1/9] gh-93334: Fix homonym edge case in PathFinder.find_spec --- Lib/importlib/_bootstrap_external.py | 2 +- .../test_importlib/namespace_pkgs/homonym/README.md | 2 ++ .../test_importlib/namespace_pkgs/project4/homonym.py | 1 + Lib/test/test_importlib/test_namespace_pkgs.py | 11 +++++++++++ .../2022-10-08-14-56-07.gh-issue-93334.0KUm8d.rst | 3 +++ 5 files changed, 18 insertions(+), 1 deletion(-) create mode 100644 Lib/test/test_importlib/namespace_pkgs/homonym/README.md create mode 100644 Lib/test/test_importlib/namespace_pkgs/project4/homonym.py create mode 100644 Misc/NEWS.d/next/Library/2022-10-08-14-56-07.gh-issue-93334.0KUm8d.rst diff --git a/Lib/importlib/_bootstrap_external.py b/Lib/importlib/_bootstrap_external.py index fc50e70539cacf..fd25d27cc91eb5 100644 --- a/Lib/importlib/_bootstrap_external.py +++ b/Lib/importlib/_bootstrap_external.py @@ -1347,7 +1347,7 @@ def __init__(self, name, path, path_finder): def _find_parent_path_names(self): """Returns a tuple of (parent-module-name, parent-path-attr-name)""" parent, dot, me = self._name.rpartition('.') - if dot == '': + if dot == '' or parent not in sys.modules: # This is a top-level module. sys.path contains the parent path. return 'sys', 'path' # Not a top-level module. parent-module.__path__ contains the diff --git a/Lib/test/test_importlib/namespace_pkgs/homonym/README.md b/Lib/test/test_importlib/namespace_pkgs/homonym/README.md new file mode 100644 index 00000000000000..9f6bc74941d0ce --- /dev/null +++ b/Lib/test/test_importlib/namespace_pkgs/homonym/README.md @@ -0,0 +1,2 @@ +This directory should not be a package, but should share a name with an +unrelated subpackage. diff --git a/Lib/test/test_importlib/namespace_pkgs/project4/homonym.py b/Lib/test/test_importlib/namespace_pkgs/project4/homonym.py new file mode 100644 index 00000000000000..089a239dff7525 --- /dev/null +++ b/Lib/test/test_importlib/namespace_pkgs/project4/homonym.py @@ -0,0 +1 @@ +attr = 'homonym' diff --git a/Lib/test/test_importlib/test_namespace_pkgs.py b/Lib/test/test_importlib/test_namespace_pkgs.py index f451f7547b35bb..30ad00e86ce52a 100644 --- a/Lib/test/test_importlib/test_namespace_pkgs.py +++ b/Lib/test/test_importlib/test_namespace_pkgs.py @@ -316,6 +316,17 @@ def test_module_before_namespace_package(self): self.assertEqual(a_test.attr, 'in module') +class DirectoryHomonym(NamespacePackageTest): + paths = [''] + + def test_namespace_package_submodule_homonym(self): + submodule_path = 'project4.homonym' + got = importlib.machinery.PathFinder.find_spec(submodule_path) + self.assertEqual(got.name, submodule_path) + self.assertIsNone(got.loader) + self.assertTrue(got.submodule_search_locations[0].endswith('homonym')) + + class ReloadTests(NamespacePackageTest): paths = ['portion1'] diff --git a/Misc/NEWS.d/next/Library/2022-10-08-14-56-07.gh-issue-93334.0KUm8d.rst b/Misc/NEWS.d/next/Library/2022-10-08-14-56-07.gh-issue-93334.0KUm8d.rst new file mode 100644 index 00000000000000..0fd3c520aee875 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2022-10-08-14-56-07.gh-issue-93334.0KUm8d.rst @@ -0,0 +1,3 @@ +Prevent :exc:`KeyError` thrown by :meth:`importlib.machinery.PathFinder.find_spec` +when a directory that is not a module shares a name with a namespace package +submodule, and where a `path` argument is not provided. From 7ee7ebe576c4bd053cd484e4525dc094bf6aef0e Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Sat, 8 Oct 2022 15:14:53 -0400 Subject: [PATCH 2/9] fixup! Fix homonym edge case double backticks in news entry --- .../next/Library/2022-10-08-14-56-07.gh-issue-93334.0KUm8d.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Library/2022-10-08-14-56-07.gh-issue-93334.0KUm8d.rst b/Misc/NEWS.d/next/Library/2022-10-08-14-56-07.gh-issue-93334.0KUm8d.rst index 0fd3c520aee875..69407120446486 100644 --- a/Misc/NEWS.d/next/Library/2022-10-08-14-56-07.gh-issue-93334.0KUm8d.rst +++ b/Misc/NEWS.d/next/Library/2022-10-08-14-56-07.gh-issue-93334.0KUm8d.rst @@ -1,3 +1,3 @@ Prevent :exc:`KeyError` thrown by :meth:`importlib.machinery.PathFinder.find_spec` when a directory that is not a module shares a name with a namespace package -submodule, and where a `path` argument is not provided. +submodule, and where a ``path`` argument is not provided. From 8092f01e04d6ce32b7fd61c4a461d7aa84d43e73 Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Sat, 24 Feb 2024 10:05:07 -0500 Subject: [PATCH 3/9] Update expected result --- Lib/importlib/_bootstrap_external.py | 8 ++++++-- Lib/test/test_importlib/test_namespace_pkgs.py | 4 ++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/Lib/importlib/_bootstrap_external.py b/Lib/importlib/_bootstrap_external.py index 9d4d21dfe32f05..3981198b5154ca 100644 --- a/Lib/importlib/_bootstrap_external.py +++ b/Lib/importlib/_bootstrap_external.py @@ -1358,7 +1358,7 @@ def __init__(self, name, path, path_finder): def _find_parent_path_names(self): """Returns a tuple of (parent-module-name, parent-path-attr-name)""" parent, dot, me = self._name.rpartition('.') - if dot == '' or parent not in sys.modules: + if dot == '': # This is a top-level module. sys.path contains the parent path. return 'sys', 'path' # Not a top-level module. parent-module.__path__ contains the @@ -1367,7 +1367,11 @@ def _find_parent_path_names(self): def _get_parent_path(self): parent_module_name, path_attr_name = self._find_parent_path_names() - return getattr(sys.modules[parent_module_name], path_attr_name) + try: + module = sys.modules[parent_module_name] + except KeyError: + return None + return getattr(module, path_attr_name) def _recalculate(self): # If the parent's path has changed, recalculate _path diff --git a/Lib/test/test_importlib/test_namespace_pkgs.py b/Lib/test/test_importlib/test_namespace_pkgs.py index 6a27b665d3cc1f..6dc981c56f081d 100644 --- a/Lib/test/test_importlib/test_namespace_pkgs.py +++ b/Lib/test/test_importlib/test_namespace_pkgs.py @@ -325,9 +325,9 @@ class DirectoryHomonym(NamespacePackageTest): def test_namespace_package_submodule_homonym(self): submodule_path = 'project4.homonym' got = importlib.machinery.PathFinder.find_spec(submodule_path) - self.assertEqual(got.name, submodule_path) + self.assertEqual(got.name, 'homonym') self.assertIsNone(got.loader) - self.assertTrue(got.submodule_search_locations[0].endswith('homonym')) + self.assertNotIn('project4', got.submodule_search_locations[0]) class ReloadTests(NamespacePackageTest): From d0f5a82563ac468aea3e9a9b7f809030bd097fdc Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Sat, 24 Feb 2024 10:08:39 -0500 Subject: [PATCH 4/9] Fix expected result --- Lib/test/test_importlib/test_namespace_pkgs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_importlib/test_namespace_pkgs.py b/Lib/test/test_importlib/test_namespace_pkgs.py index 6dc981c56f081d..21bc6e8cc0a974 100644 --- a/Lib/test/test_importlib/test_namespace_pkgs.py +++ b/Lib/test/test_importlib/test_namespace_pkgs.py @@ -325,7 +325,7 @@ class DirectoryHomonym(NamespacePackageTest): def test_namespace_package_submodule_homonym(self): submodule_path = 'project4.homonym' got = importlib.machinery.PathFinder.find_spec(submodule_path) - self.assertEqual(got.name, 'homonym') + self.assertEqual(got.name, 'project4.homonym') self.assertIsNone(got.loader) self.assertNotIn('project4', got.submodule_search_locations[0]) From 78a1232ff8825e650061fe64616443148194235c Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Sat, 24 Feb 2024 10:27:25 -0500 Subject: [PATCH 5/9] fix failing test --- Lib/importlib/_bootstrap_external.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/importlib/_bootstrap_external.py b/Lib/importlib/_bootstrap_external.py index 3981198b5154ca..61ccd025ca04e8 100644 --- a/Lib/importlib/_bootstrap_external.py +++ b/Lib/importlib/_bootstrap_external.py @@ -1370,7 +1370,7 @@ def _get_parent_path(self): try: module = sys.modules[parent_module_name] except KeyError: - return None + return sys.path return getattr(module, path_attr_name) def _recalculate(self): From efb61c1e7c744ff0d2f500cb4584ede6df0d3efa Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Sat, 24 Feb 2024 10:28:21 -0500 Subject: [PATCH 6/9] reuse string --- Lib/test/test_importlib/test_namespace_pkgs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_importlib/test_namespace_pkgs.py b/Lib/test/test_importlib/test_namespace_pkgs.py index 21bc6e8cc0a974..e0bfb576ff1283 100644 --- a/Lib/test/test_importlib/test_namespace_pkgs.py +++ b/Lib/test/test_importlib/test_namespace_pkgs.py @@ -325,7 +325,7 @@ class DirectoryHomonym(NamespacePackageTest): def test_namespace_package_submodule_homonym(self): submodule_path = 'project4.homonym' got = importlib.machinery.PathFinder.find_spec(submodule_path) - self.assertEqual(got.name, 'project4.homonym') + self.assertEqual(got.name, submodule_path) self.assertIsNone(got.loader) self.assertNotIn('project4', got.submodule_search_locations[0]) From 7b77fac3e412af695390ec1e7c7ad63c66bca79e Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Sat, 13 Jul 2024 18:39:31 -0400 Subject: [PATCH 7/9] Raise ModuleNotFoundError, update news and tests --- Lib/importlib/_bootstrap_external.py | 7 +++++-- .../namespace_pkgs/{homonym => foo}/README.md | 0 .../namespace_pkgs/project4/homonym.py | 1 - Lib/test/test_importlib/test_namespace_pkgs.py | 13 ++++++------- .../2022-10-08-14-56-07.gh-issue-93334.0KUm8d.rst | 6 +++--- 5 files changed, 14 insertions(+), 13 deletions(-) rename Lib/test/test_importlib/namespace_pkgs/{homonym => foo}/README.md (100%) delete mode 100644 Lib/test/test_importlib/namespace_pkgs/project4/homonym.py diff --git a/Lib/importlib/_bootstrap_external.py b/Lib/importlib/_bootstrap_external.py index ccd5bc76cd299f..11aa76213cf953 100644 --- a/Lib/importlib/_bootstrap_external.py +++ b/Lib/importlib/_bootstrap_external.py @@ -1379,8 +1379,11 @@ def _get_parent_path(self): parent_module_name, path_attr_name = self._find_parent_path_names() try: module = sys.modules[parent_module_name] - except KeyError: - return sys.path + except KeyError as e: + raise ModuleNotFoundError( + f"{parent_module_name} must be imported before finding {self._name}.", + name=parent_module_name, + ) from e return getattr(module, path_attr_name) def _recalculate(self): diff --git a/Lib/test/test_importlib/namespace_pkgs/homonym/README.md b/Lib/test/test_importlib/namespace_pkgs/foo/README.md similarity index 100% rename from Lib/test/test_importlib/namespace_pkgs/homonym/README.md rename to Lib/test/test_importlib/namespace_pkgs/foo/README.md diff --git a/Lib/test/test_importlib/namespace_pkgs/project4/homonym.py b/Lib/test/test_importlib/namespace_pkgs/project4/homonym.py deleted file mode 100644 index 089a239dff7525..00000000000000 --- a/Lib/test/test_importlib/namespace_pkgs/project4/homonym.py +++ /dev/null @@ -1 +0,0 @@ -attr = 'homonym' diff --git a/Lib/test/test_importlib/test_namespace_pkgs.py b/Lib/test/test_importlib/test_namespace_pkgs.py index 79ad12fe7f6f34..0d552367cdbe74 100644 --- a/Lib/test/test_importlib/test_namespace_pkgs.py +++ b/Lib/test/test_importlib/test_namespace_pkgs.py @@ -318,15 +318,14 @@ def test_module_before_namespace_package(self): self.assertEqual(a_test.attr, 'in module') -class DirectoryHomonym(NamespacePackageTest): +class NamespaceSubpackageSameName(NamespacePackageTest): paths = [''] - def test_namespace_package_submodule_homonym(self): - submodule_path = 'project4.homonym' - got = importlib.machinery.PathFinder.find_spec(submodule_path) - self.assertEqual(got.name, submodule_path) - self.assertIsNone(got.loader) - self.assertNotIn('project4', got.submodule_search_locations[0]) + def test_namespace_subpackage_shares_name_with_directory(self): + submodule_path = 'project4.foo' + msg = 'project4 must be imported before finding project4.foo.' + with self.assertRaisesRegex(ModuleNotFoundError, msg): + importlib.machinery.PathFinder.find_spec(submodule_path) class ReloadTests(NamespacePackageTest): diff --git a/Misc/NEWS.d/next/Library/2022-10-08-14-56-07.gh-issue-93334.0KUm8d.rst b/Misc/NEWS.d/next/Library/2022-10-08-14-56-07.gh-issue-93334.0KUm8d.rst index 69407120446486..c3a3b7a6812252 100644 --- a/Misc/NEWS.d/next/Library/2022-10-08-14-56-07.gh-issue-93334.0KUm8d.rst +++ b/Misc/NEWS.d/next/Library/2022-10-08-14-56-07.gh-issue-93334.0KUm8d.rst @@ -1,3 +1,3 @@ -Prevent :exc:`KeyError` thrown by :meth:`importlib.machinery.PathFinder.find_spec` -when a directory that is not a module shares a name with a namespace package -submodule, and where a ``path`` argument is not provided. +Reraise :exc:`KeyError` as :exc:`ModuleNotFoundError` when +:meth:`importlib.machinery.PathFinder.find_spec` is called on a submodule +without importing the parent (and without a ``path`` argument). From 1d59e5c223ebc577d4f24152d0e4fd91fa8c5a29 Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Sat, 13 Jul 2024 18:54:37 -0400 Subject: [PATCH 8/9] Let build know about new test package --- Makefile.pre.in | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile.pre.in b/Makefile.pre.in index d380c422714a32..e38b889249303f 100644 --- a/Makefile.pre.in +++ b/Makefile.pre.in @@ -2427,6 +2427,7 @@ TESTSUBDIRS= idlelib/idle_test \ test/test_importlib/namespace_pkgs \ test/test_importlib/namespace_pkgs/both_portions \ test/test_importlib/namespace_pkgs/both_portions/foo \ + test/test_importlib/namespace_pkgs/foo \ test/test_importlib/namespace_pkgs/module_and_namespace_package \ test/test_importlib/namespace_pkgs/module_and_namespace_package/a_test \ test/test_importlib/namespace_pkgs/not_a_namespace_pkg \ From fd24f8794c3fdd5b38f3e8934e1dec9c9889b52f Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Sun, 15 Sep 2024 09:22:24 -0400 Subject: [PATCH 9/9] Address review --- Lib/importlib/_bootstrap_external.py | 5 +++-- Lib/test/test_importlib/test_namespace_pkgs.py | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/Lib/importlib/_bootstrap_external.py b/Lib/importlib/_bootstrap_external.py index c95e106b5f3b0e..4a441a2ad43dfa 100644 --- a/Lib/importlib/_bootstrap_external.py +++ b/Lib/importlib/_bootstrap_external.py @@ -1109,10 +1109,11 @@ def _get_parent_path(self): module = sys.modules[parent_module_name] except KeyError as e: raise ModuleNotFoundError( - f"{parent_module_name} must be imported before finding {self._name}.", + f"{parent_module_name!r} must be imported before finding {self._name!r}.", name=parent_module_name, ) from e - return getattr(module, path_attr_name) + else: + return getattr(module, path_attr_name) def _recalculate(self): # If the parent's path has changed, recalculate _path diff --git a/Lib/test/test_importlib/test_namespace_pkgs.py b/Lib/test/test_importlib/test_namespace_pkgs.py index 0d552367cdbe74..a4bb27fa1a383e 100644 --- a/Lib/test/test_importlib/test_namespace_pkgs.py +++ b/Lib/test/test_importlib/test_namespace_pkgs.py @@ -323,10 +323,11 @@ class NamespaceSubpackageSameName(NamespacePackageTest): def test_namespace_subpackage_shares_name_with_directory(self): submodule_path = 'project4.foo' - msg = 'project4 must be imported before finding project4.foo.' - with self.assertRaisesRegex(ModuleNotFoundError, msg): + with self.assertRaises(ModuleNotFoundError) as cm: importlib.machinery.PathFinder.find_spec(submodule_path) + self.assertEqual(cm.exception.name, 'project4') + class ReloadTests(NamespacePackageTest): paths = ['portion1']