diff --git a/importlib_metadata/__init__.py b/importlib_metadata/__init__.py index 08ebcb34..ac0969d3 100644 --- a/importlib_metadata/__init__.py +++ b/importlib_metadata/__init__.py @@ -27,7 +27,7 @@ from importlib import import_module from importlib.abc import MetaPathFinder from itertools import starmap -from typing import Any +from typing import Any, TypeVar from ._collections import FreezableDefaultDict, Pair from ._compat import ( @@ -35,7 +35,6 @@ install, localize, ) -from ._context import ExceptionTrap from ._functools import apply, compose, method_cache, noop, pass_none, passthrough from ._itertools import always_iterable, bucket, unique_everseen from ._meta import ( @@ -67,6 +66,9 @@ ] +_D = TypeVar('_D', bound=IDistribution) + + class PackageNotFoundError(ModuleNotFoundError): """The package was not found.""" @@ -520,18 +522,14 @@ def discover( ) @staticmethod - def _prefer_valid(dists: Iterable[Distribution]) -> Iterable[Distribution]: + def _prefer_valid(dists: Iterable[_D]) -> Iterable[_D]: """ Prefer (move to the front) distributions that have metadata. Ref python/importlib_resources#489. """ - has_metadata = ExceptionTrap(MetadataNotFound).passes( - operator.attrgetter('metadata') - ) - - buckets = bucket(dists, has_metadata) + buckets = bucket(dists, _safe_has_metadata) return itertools.chain(buckets[True], buckets[False]) @staticmethod @@ -566,7 +564,18 @@ def metadata(self) -> PackageMetadata: :raises MetadataNotFound: If no metadata file is present. """ - text = ( + text = self._metadata_text() + return self._assemble_message(self._ensure_metadata_present(text)) + + def _metadata_text(self) -> str | None: + """ + The metadata text, if any. + + Separate from ``metadata`` so that ``_has_metadata`` can name + the same files without repeating them and without paying to + parse what it finds. + """ + return ( self.read_text('METADATA') or self.read_text('PKG-INFO') # This last clause is here to support old egg-info files. Its @@ -574,7 +583,18 @@ def metadata(self) -> PackageMetadata: # (which points to the egg-info file) attribute unchanged. or self.read_text('') ) - return self._assemble_message(self._ensure_metadata_present(text)) + + def _has_metadata(self) -> bool: + """ + Whether this distribution supplies metadata. + + Reads the metadata, as the interface offers nothing cheaper: + ``locate_file`` resolves against the package root and not the + metadata, and every other member loads the metadata or parses + it. A subclass able to detect the metadata without reading it + should override this (python/importlib_metadata#534). + """ + return bool(self._metadata_text()) @staticmethod def _assemble_message(text: str) -> PackageMetadata: @@ -1078,6 +1098,24 @@ def read_text(self, filename: str | os.PathLike[str]) -> str | None: read_text.__doc__ = Distribution.read_text.__doc__ + def _has_metadata(self) -> bool: + """ + Detect the metadata file without opening it + (python/importlib_metadata#534). + + The last clause is for an old-style egg-info file, whose + metadata is the distribution's own path; a metadata directory + is not itself metadata. ``SimplePath`` doesn't name + ``is_file``, but only these legacy paths reach that clause and + they arrive as a ``pathlib.Path`` or a ``zipfile.Path``, both + of which supply it. + """ + return ( + self._path.joinpath('METADATA').exists() + or self._path.joinpath('PKG-INFO').exists() + or self._path.is_file() # type: ignore[attr-defined] + ) + def locate_file(self, path: str | os.PathLike[str]) -> SimplePath: return self._path.parent / path @@ -1148,6 +1186,25 @@ def version(distribution_name: str) -> str: return distribution(distribution_name).version +def _safe_has_metadata(dist: IDistribution) -> bool: + """ + ``dist._has_metadata()``, for a distribution that may not supply it. + + A provider's own ``Distribution`` subclass derives from + ``importlib.metadata`` and is passed through unadapted (#486), so + it has none; read its metadata as the default would. It supplies + no ``_metadata_text`` either, hence the unbound call. + + Such a distribution cannot be asked for ``metadata`` behind a trap + instead. The ``MetadataNotFound`` it raises is the one from + ``importlib.metadata``, a different class from this package's, + which no trap of ours would catch. + """ + with suppress(AttributeError): + return dist._has_metadata() # type: ignore[attr-defined] + return bool(Distribution._metadata_text(dist)) # type: ignore[arg-type] + + _unique = functools.partial( unique_everseen, key=operator.attrgetter('_normalized_name'), @@ -1167,7 +1224,8 @@ def entry_points(**params) -> EntryPoints: :return: EntryPoints for all installed packages. """ eps = itertools.chain.from_iterable( - dist.entry_points for dist in _unique(distributions()) + dist.entry_points + for dist in _unique(Distribution._prefer_valid(distributions())) ) return EntryPoints(eps).select(**params) diff --git a/newsfragments/+foreign-metadata-not-found.bugfix.rst b/newsfragments/+foreign-metadata-not-found.bugfix.rst new file mode 100644 index 00000000..b41cb9bf --- /dev/null +++ b/newsfragments/+foreign-metadata-not-found.bugfix.rst @@ -0,0 +1,5 @@ +The preference for valid distributions no longer propagates the +``MetadataNotFound`` of a distribution that a provider derived from +``importlib.metadata``, whose exception is a different class from this +package's and so escaped the preference rather than marking the distribution +invalid. diff --git a/newsfragments/+has-metadata.feature.rst b/newsfragments/+has-metadata.feature.rst new file mode 100644 index 00000000..607448b2 --- /dev/null +++ b/newsfragments/+has-metadata.feature.rst @@ -0,0 +1,4 @@ +Detecting whether a distribution supplies metadata no longer reads or parses +that metadata where the metadata directory can be inspected instead, so +retrieving a distribution by name, which prefers the valid candidates, is +substantially faster. diff --git a/newsfragments/534.bugfix.rst b/newsfragments/534.bugfix.rst new file mode 100644 index 00000000..67e0cba0 --- /dev/null +++ b/newsfragments/534.bugfix.rst @@ -0,0 +1,5 @@ +``entry_points()`` now prefers distributions whose metadata is readable, so +an empty metadata directory left behind by an unclean upgrade no longer +shadows the valid one of the same name and suppresses its entry points. +Invalid distributions are deferred to the end of the enumeration rather than +dropped. diff --git a/tests/compat/test_localize.py b/tests/compat/test_localize.py index 36ed50ac..ad26bf0e 100644 --- a/tests/compat/test_localize.py +++ b/tests/compat/test_localize.py @@ -119,6 +119,24 @@ def test_conforming_provider_is_not_warned_about(self): assert distribution('distinfo-pkg').version == '1.0.0' assert not [str(warning.message) for warning in caught] + def test_dist_without_metadata_is_deferred(self): + """ + A provider's own distribution supplies no detection of its own, + so the metadata is sought through ``read_text``. Asking for + ``metadata`` instead would raise, as the ``MetadataNotFound`` + of a distribution deriving from ``importlib.metadata`` is a + different class. + """ + + class Empty(stdlib.Distribution): + def read_text(self, filename): + return None + + def locate_file(self, path): + return None + + assert not importlib_metadata._safe_has_metadata(Empty()) + class ConformanceCheckTests(unittest.TestCase): @unittest.skipIf( diff --git a/tests/test_main.py b/tests/test_main.py index cbcb5cbb..b03911a3 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -56,6 +56,25 @@ def test_abc_enforced(self): with self.assertRaises(TypeError): type('DistributionSubclass', (Distribution,), {})() + @fixtures.parameterize( + dict(text='Name: foo\nVersion: 1.0\n', expected=True), + dict(text=None, expected=False), + ) + def test_detects_metadata_by_reading(self, text, expected): + """ + A distribution that isn't path-based takes the default + detection, which reads what it cannot otherwise detect. + """ + + class Provided(Distribution): + def read_text(self, filename): + return text if filename == 'METADATA' else None + + def locate_file(self, path): + return None # pragma: no cover + + assert Provided()._has_metadata() is expected + @fixtures.parameterize( dict(name=None), dict(name=''), @@ -160,6 +179,40 @@ def test_valid_dists_preferred(self): dist = Distribution.from_name('foo') assert dist.version == "1.0" + def test_invalid_dists_deferred(self): + """ + Preferring the valid dists defers the invalid ones to the end + rather than dropping them. + """ + fixtures.build_files(self.make_pkg('foo-4.0', files={}), self.site_dir) + fixtures.build_files(self.make_pkg('foo-4.1'), self.site_dir) + dists = list(Distribution._prefer_valid(Distribution.discover(name='foo'))) + assert len(dists) == 2 + assert dists[0].version == "1.0" + + def test_entry_points_from_valid_dist(self): + """ + Entry points come from the valid dist even when an invalid one + of the same name is discovered first. + + Ref python/importlib_metadata#534. + """ + # as in test_valid_dists_preferred, bracket the valid dist so that + # on most file systems it is never naturally first. + fixtures.build_files(self.make_pkg('foo-4.0', files={}), self.site_dir) + fixtures.build_files( + self.make_pkg( + 'foo-4.1', + files={ + 'METADATA': "VERSION: 1.0", + 'entry_points.txt': '[shadowed]\nep = foo:main\n', + }, + ), + self.site_dir, + ) + fixtures.build_files(self.make_pkg('foo-4.2', files={}), self.site_dir) + assert entry_points(group='shadowed').names == {'ep'} + def test_missing_metadata(self): """ Dists with a missing metadata file should raise ``MetadataNotFound``.