From 4b61913ecef7a79cec4bc5b1d2759af67bb2c311 Mon Sep 17 00:00:00 2001 From: Johan Herland Date: Wed, 19 Apr 2023 02:41:20 +0200 Subject: [PATCH 1/9] Add test to demonstrate issue with symlinked packages --- tests/fixtures.py | 26 ++++++++++++++++++++++++++ tests/test_main.py | 26 ++++++++++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/tests/fixtures.py b/tests/fixtures.py index 6d26bb91..b39a1c4b 100644 --- a/tests/fixtures.py +++ b/tests/fixtures.py @@ -169,6 +169,32 @@ def setUp(self): build_files(DistInfoPkg.files, self.site_dir) +class DistInfoSymlinkedPkg(OnSysPath, SiteDir): + files: FilesSpec = { + "symlinked_pkg-1.0.0.dist-info": { + "METADATA": """ + Name: symlinked-pkg + Version: 1.0.0 + """, + "RECORD": "symlinked,,\n", + }, + ".symlink.target": { + "__init__.py": """ + def main(): + print("hello world") + """, + }, + # "symlinked" -> ".symlink.target", see below + } + + def setUp(self): + super().setUp() + build_files(DistInfoSymlinkedPkg.files, self.site_dir) + target = self.site_dir / ".symlink.target" + assert target.is_dir() + (self.site_dir / "symlinked").symlink_to(target, target_is_directory=True) + + class EggInfoPkg(OnSysPath, SiteDir): files: FilesSpec = { "egginfo_pkg.egg-info": { diff --git a/tests/test_main.py b/tests/test_main.py index a7650172..bec1303b 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -387,6 +387,32 @@ def test_packages_distributions_all_module_types(self): assert not any(name.endswith('.dist-info') for name in distributions) +class PackagesDistributionsDistTest( + fixtures.DistInfoPkg, + fixtures.DistInfoSymlinkedPkg, + unittest.TestCase, +): + def test_packages_distributions_on_dist_info(self): + """ + Test _top_level_inferred() on various dist-info packages. + """ + distributions = packages_distributions() + + def import_names_from_package(package_name): + return { + import_name + for import_name, package_names in distributions.items() + if package_name in package_names + } + + # distinfo-pkg has one import ('mod') inferred from RECORD + assert import_names_from_package('distinfo-pkg') == {'mod'} + + # symlinked-pkg has one import ('symlinked') inderred from RECORD which + # references a symlink to the real package dir elsewhere. + assert import_names_from_package('symlinked-pkg') == {'symlinked'} + + class PackagesDistributionsEggTest( fixtures.EggInfoPkg, fixtures.EggInfoPkgPipInstalledNoToplevel, From 0023c15d8fe1664184f6e0f80a9fca29fc3d159e Mon Sep 17 00:00:00 2001 From: Johan Herland Date: Wed, 19 Apr 2023 02:41:44 +0200 Subject: [PATCH 2/9] Attempt to fix issue with symlinked packages --- importlib_metadata/__init__.py | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/importlib_metadata/__init__.py b/importlib_metadata/__init__.py index e9ae0d19..76ccacbc 100644 --- a/importlib_metadata/__init__.py +++ b/importlib_metadata/__init__.py @@ -31,7 +31,7 @@ from importlib import import_module from importlib.abc import MetaPathFinder from itertools import starmap -from typing import List, Mapping, Optional, cast +from typing import Iterable, Iterator, List, Mapping, Optional, cast __all__ = [ @@ -961,10 +961,32 @@ def _top_level_declared(dist): return (dist.read_text('top_level.txt') or '').split() +def _walk_dirs(package_paths: Iterable[PackagePath]) -> Iterator[PackagePath]: + for package_path in package_paths: + + def make_file(name): + result = PackagePath(name) + result.hash = None + result.size = None + result.dist = package_path.dist + return result + + real_path = package_path.locate() + real_sitedir = package_path.dist.locate_file("") # type: ignore + if real_path.is_dir() and real_path.is_symlink(): + # .files only mentions symlink, we must recurse into it ourselves: + for root, dirs, files in os.walk(real_path): + for filename in files: + real_file = pathlib.Path(root, filename) + yield make_file(real_file.relative_to(real_sitedir)) + else: + yield package_path + + def _top_level_inferred(dist): opt_names = { f.parts[0] if len(f.parts) > 1 else inspect.getmodulename(f) - for f in always_iterable(dist.files) + for f in _walk_dirs(always_iterable(dist.files)) } @pass_none From e50ebd77363dd59af06fb0fb1633c5e1e7fa9151 Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Sun, 18 Jun 2023 16:17:57 -0400 Subject: [PATCH 3/9] Extract _topmost and _get_toplevel_name functions. --- importlib_metadata/__init__.py | 56 +++++++++++++++++++--------------- 1 file changed, 31 insertions(+), 25 deletions(-) diff --git a/importlib_metadata/__init__.py b/importlib_metadata/__init__.py index 4bf232ee..37b664f4 100644 --- a/importlib_metadata/__init__.py +++ b/importlib_metadata/__init__.py @@ -31,8 +31,7 @@ from importlib import import_module from importlib.abc import MetaPathFinder from itertools import starmap -from typing import Iterable, Iterator, List, Mapping, Optional, Set, cast - +from typing import Iterable, List, Mapping, Optional, Set, cast __all__ = [ 'Distribution', @@ -974,35 +973,42 @@ def _top_level_declared(dist): return (dist.read_text('top_level.txt') or '').split() -def _walk_dirs(package_paths: Iterable[PackagePath]) -> Iterator[PackagePath]: - for package_path in package_paths: +def _topmost(name: PackagePath) -> Optional[str]: + """ + Return the top-most parent as long as there is a parent. + """ + top, *rest = name.parts + return top if rest else None - def make_file(name): - result = PackagePath(name) - result.hash = None - result.size = None - result.dist = package_path.dist - return result - real_path = package_path.locate() - real_sitedir = package_path.dist.locate_file("") # type: ignore - if real_path.is_dir() and real_path.is_symlink(): - # .files only mentions symlink, we must recurse into it ourselves: - for root, dirs, files in os.walk(real_path): - for filename in files: - real_file = pathlib.Path(root, filename) - yield make_file(real_file.relative_to(real_sitedir)) - else: - yield package_path +def _get_toplevel_name(name: PackagePath) -> str: + """ + Infer a possibly importable module name from a name presumed on + sys.path. + + >>> _get_toplevel_name(PackagePath('foo.py')) + 'foo' + >>> _get_toplevel_name(PackagePath('foo')) + 'foo' + >>> _get_toplevel_name(PackagePath('foo.pyc')) + 'foo' + >>> _get_toplevel_name(PackagePath('foo.dist-info')) + 'foo.dist-info' + >>> _get_toplevel_name(PackagePath('foo.pth')) + 'foo.pth' + >>> _get_toplevel_name(PackagePath('foo/__init__.py')) + 'foo' + """ + return _topmost(name) or ( + # python/typeshed#10328 + inspect.getmodulename(name) # type: ignore + or str(name) + ) def _top_level_inferred(dist): - opt_names = { - f.parts[0] if len(f.parts) > 1 else inspect.getmodulename(f) - for f in _walk_dirs(always_iterable(dist.files)) - } + opt_names = set(map(_get_toplevel_name, always_iterable(dist.files))) - @pass_none def importable_name(name): return '.' not in name From 7a5e025f51e88cf49092fb33e2cb5bea82dc7ff5 Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Sun, 18 Jun 2023 16:46:33 -0400 Subject: [PATCH 4/9] Streamline the test to check one expectation (the standard dist-info expectation is handled by other tests above. --- tests/test_main.py | 22 ++++------------------ 1 file changed, 4 insertions(+), 18 deletions(-) diff --git a/tests/test_main.py b/tests/test_main.py index 5d3f39c6..5f653f3d 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -401,29 +401,15 @@ def test_packages_distributions_all_module_types(self): class PackagesDistributionsDistTest( - fixtures.DistInfoPkg, fixtures.DistInfoSymlinkedPkg, unittest.TestCase, ): - def test_packages_distributions_on_dist_info(self): + def test_packages_distributions_symlinked_top_level(self): """ - Test _top_level_inferred() on various dist-info packages. + Distribution is resolvable from a simple top-level symlink in RECORD. + See #452. """ - distributions = packages_distributions() - - def import_names_from_package(package_name): - return { - import_name - for import_name, package_names in distributions.items() - if package_name in package_names - } - - # distinfo-pkg has one import ('mod') inferred from RECORD - assert import_names_from_package('distinfo-pkg') == {'mod'} - - # symlinked-pkg has one import ('symlinked') inderred from RECORD which - # references a symlink to the real package dir elsewhere. - assert import_names_from_package('symlinked-pkg') == {'symlinked'} + assert packages_distributions()['symlinked'] == ['symlinked-pkg'] class PackagesDistributionsEggTest( From fa705d37265d25581eb9bfd5c1f41ea11c94743b Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Sun, 18 Jun 2023 16:50:32 -0400 Subject: [PATCH 5/9] Inline the symlink setup. --- tests/fixtures.py | 26 -------------------------- tests/test_main.py | 26 ++++++++++++++++++++++++-- 2 files changed, 24 insertions(+), 28 deletions(-) diff --git a/tests/fixtures.py b/tests/fixtures.py index e0413cc8..c0b0fa32 100644 --- a/tests/fixtures.py +++ b/tests/fixtures.py @@ -169,32 +169,6 @@ def setUp(self): build_files(DistInfoPkg.files, self.site_dir) -class DistInfoSymlinkedPkg(OnSysPath, SiteDir): - files: FilesSpec = { - "symlinked_pkg-1.0.0.dist-info": { - "METADATA": """ - Name: symlinked-pkg - Version: 1.0.0 - """, - "RECORD": "symlinked,,\n", - }, - ".symlink.target": { - "__init__.py": """ - def main(): - print("hello world") - """, - }, - # "symlinked" -> ".symlink.target", see below - } - - def setUp(self): - super().setUp() - build_files(DistInfoSymlinkedPkg.files, self.site_dir) - target = self.site_dir / ".symlink.target" - assert target.is_dir() - (self.site_dir / "symlinked").symlink_to(target, target_is_directory=True) - - class EggInfoPkg(OnSysPath, SiteDir): files: FilesSpec = { "egginfo_pkg.egg-info": { diff --git a/tests/test_main.py b/tests/test_main.py index 5f653f3d..0bcb7ac9 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -401,14 +401,36 @@ def test_packages_distributions_all_module_types(self): class PackagesDistributionsDistTest( - fixtures.DistInfoSymlinkedPkg, - unittest.TestCase, + fixtures.OnSysPath, fixtures.SiteDir, unittest.TestCase ): def test_packages_distributions_symlinked_top_level(self): """ Distribution is resolvable from a simple top-level symlink in RECORD. See #452. """ + + files: fixtures.FilesSpec = { + "symlinked_pkg-1.0.0.dist-info": { + "METADATA": """ + Name: symlinked-pkg + Version: 1.0.0 + """, + "RECORD": "symlinked,,\n", + }, + ".symlink.target": { + "__init__.py": """ + def main(): + print("hello world") + """, + }, + # "symlinked" -> ".symlink.target", see below + } + + fixtures.build_files(files, self.site_dir) + target = self.site_dir / ".symlink.target" + assert target.is_dir() + (self.site_dir / "symlinked").symlink_to(target, target_is_directory=True) + assert packages_distributions()['symlinked'] == ['symlinked-pkg'] From 7a19e8a4a933f00b12d26719ddb5b474045817ae Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Sun, 18 Jun 2023 16:52:51 -0400 Subject: [PATCH 6/9] Consolidate PackageDistributions tests. --- tests/test_main.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/test_main.py b/tests/test_main.py index 0bcb7ac9..fbf79a62 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -399,10 +399,6 @@ def test_packages_distributions_all_module_types(self): assert not any(name.endswith('.dist-info') for name in distributions) - -class PackagesDistributionsDistTest( - fixtures.OnSysPath, fixtures.SiteDir, unittest.TestCase -): def test_packages_distributions_symlinked_top_level(self): """ Distribution is resolvable from a simple top-level symlink in RECORD. From d5f723f8ec5623740593011bae63df16be50a1c3 Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Sun, 18 Jun 2023 17:19:51 -0400 Subject: [PATCH 7/9] Utilize the new Symlink in preparing the test case. --- tests/test_main.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/tests/test_main.py b/tests/test_main.py index fbf79a62..4543e21d 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -9,6 +9,7 @@ from . import fixtures from ._context import suppress +from ._path import Symlink from importlib_metadata import ( Distribution, EntryPoint, @@ -419,14 +420,10 @@ def main(): print("hello world") """, }, - # "symlinked" -> ".symlink.target", see below + "symlinked": Symlink(".symlink.target"), } fixtures.build_files(files, self.site_dir) - target = self.site_dir / ".symlink.target" - assert target.is_dir() - (self.site_dir / "symlinked").symlink_to(target, target_is_directory=True) - assert packages_distributions()['symlinked'] == ['symlinked-pkg'] From e8bc802866861ee32049fd60cf9e4f3e30592f26 Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Sun, 18 Jun 2023 17:23:58 -0400 Subject: [PATCH 8/9] Update changelog. --- CHANGES.rst | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGES.rst b/CHANGES.rst index c7e5889c..255bbd32 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -1,3 +1,10 @@ +v6.7.0 +====== + +* #453: When inferring top-level names that are importable for + distributions in ``package_distributions``, now symlinks to + other directories are honored. + v6.6.0 ====== From 53e47d9ae8e7784da051e264376bdb7221a45871 Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Sun, 18 Jun 2023 17:31:33 -0400 Subject: [PATCH 9/9] Remove '__init__.py', not needed. --- tests/test_main.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/tests/test_main.py b/tests/test_main.py index 4543e21d..79181bf4 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -414,12 +414,7 @@ def test_packages_distributions_symlinked_top_level(self): """, "RECORD": "symlinked,,\n", }, - ".symlink.target": { - "__init__.py": """ - def main(): - print("hello world") - """, - }, + ".symlink.target": {}, "symlinked": Symlink(".symlink.target"), }