-
Notifications
You must be signed in to change notification settings - Fork 3.2k
Final purge of direct pkg_resources usages #10675
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
cd01e4f
498f313
e6b9ddc
91f7267
e50c07e
6ed6aa8
496343e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,19 +1,19 @@ | ||
| import email.message | ||
| import email.parser | ||
| import logging | ||
| import os | ||
| import pathlib | ||
| from typing import Collection, Iterable, Iterator, List, NamedTuple, Optional | ||
| from zipfile import BadZipFile | ||
| import zipfile | ||
| from typing import Collection, Iterable, Iterator, List, Mapping, NamedTuple, Optional | ||
|
|
||
| from pip._vendor import pkg_resources | ||
| from pip._vendor.packaging.requirements import Requirement | ||
| from pip._vendor.packaging.utils import NormalizedName, canonicalize_name | ||
| from pip._vendor.packaging.version import parse as parse_version | ||
|
|
||
| from pip._internal.exceptions import InvalidWheel | ||
| from pip._internal.utils import misc # TODO: Move definition here. | ||
| from pip._internal.utils.packaging import get_installer, get_metadata | ||
| from pip._internal.utils.wheel import pkg_resources_distribution_for_wheel | ||
| from pip._internal.exceptions import InvalidWheel, NoneMetadataError, UnsupportedWheel | ||
| from pip._internal.utils.misc import display_path | ||
| from pip._internal.utils.wheel import parse_wheel, read_wheel_metadata_file | ||
|
|
||
| from .base import ( | ||
| BaseDistribution, | ||
|
|
@@ -33,6 +33,41 @@ class EntryPoint(NamedTuple): | |
| group: str | ||
|
|
||
|
|
||
| class WheelMetadata: | ||
| """IMetadataProvider that reads metadata files from a dictionary. | ||
|
|
||
| This also maps metadata decoding exceptions to our internal exception type. | ||
| """ | ||
|
|
||
| def __init__(self, metadata: Mapping[str, bytes], wheel_name: str) -> None: | ||
| self._metadata = metadata | ||
| self._wheel_name = wheel_name | ||
|
|
||
| def has_metadata(self, name: str) -> bool: | ||
| return name in self._metadata | ||
|
|
||
| def get_metadata(self, name: str) -> str: | ||
| try: | ||
| return self._metadata[name].decode() | ||
| except UnicodeDecodeError as e: | ||
| # Augment the default error with the origin of the file. | ||
| raise UnsupportedWheel( | ||
| f"Error decoding metadata for {self._wheel_name}: {e} in {name} file" | ||
| ) | ||
|
|
||
| def get_metadata_lines(self, name: str) -> Iterable[str]: | ||
| return pkg_resources.yield_lines(self.get_metadata(name)) | ||
|
|
||
| def metadata_isdir(self, name: str) -> bool: | ||
| return False | ||
|
|
||
| def metadata_listdir(self, name: str) -> List[str]: | ||
| return [] | ||
|
|
||
| def run_script(self, script_name: str, namespace: str) -> None: | ||
| pass | ||
|
|
||
|
|
||
| class Distribution(BaseDistribution): | ||
| def __init__(self, dist: pkg_resources.Distribution) -> None: | ||
| self._dist = dist | ||
|
|
@@ -63,12 +98,26 @@ def from_wheel(cls, wheel: Wheel, name: str) -> "Distribution": | |
|
|
||
| :raises InvalidWheel: Whenever loading of the wheel causes a | ||
| :py:exc:`zipfile.BadZipFile` exception to be thrown. | ||
| :raises UnsupportedWheel: If the wheel is a valid zip, but malformed | ||
| internally. | ||
| """ | ||
| try: | ||
| with wheel.as_zipfile() as zf: | ||
| dist = pkg_resources_distribution_for_wheel(zf, name, wheel.location) | ||
| except BadZipFile as e: | ||
| info_dir, _ = parse_wheel(zf, name) | ||
| metadata_text = { | ||
| path.split("/", 1)[-1]: read_wheel_metadata_file(zf, path) | ||
| for path in zf.namelist() | ||
| if path.startswith(f"{info_dir}/") | ||
| } | ||
| except zipfile.BadZipFile as e: | ||
| raise InvalidWheel(wheel.location, name) from e | ||
| except UnsupportedWheel as e: | ||
| raise UnsupportedWheel(f"{name} has an invalid wheel, {e}") | ||
|
Comment on lines
+114
to
+115
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not let this error pass through as-is? The caller has the
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, but I don’t really want to think about changing behaviour right now; I’ll get too many chances for that when we try to re-implement everything in importlib.metadata and deal with those subtle differences. |
||
| dist = pkg_resources.DistInfoDistribution( | ||
| location=wheel.location, | ||
| metadata=WheelMetadata(metadata_text, wheel.location), | ||
| project_name=name, | ||
| ) | ||
| return cls(dist) | ||
|
|
||
| @property | ||
|
|
@@ -97,25 +146,6 @@ def canonical_name(self) -> NormalizedName: | |
| def version(self) -> DistributionVersion: | ||
| return parse_version(self._dist.version) | ||
|
|
||
| @property | ||
| def installer(self) -> str: | ||
| try: | ||
| return get_installer(self._dist) | ||
| except (OSError, ValueError): | ||
| return "" # Fail silently if the installer file cannot be read. | ||
|
|
||
| @property | ||
| def local(self) -> bool: | ||
| return misc.dist_is_local(self._dist) | ||
|
|
||
| @property | ||
| def in_usersite(self) -> bool: | ||
| return misc.dist_in_usersite(self._dist) | ||
|
|
||
| @property | ||
| def in_site_packages(self) -> bool: | ||
| return misc.dist_in_site_packages(self._dist) | ||
|
|
||
| def is_file(self, path: InfoPath) -> bool: | ||
| return self._dist.has_metadata(str(path)) | ||
|
|
||
|
|
@@ -132,7 +162,10 @@ def read_text(self, path: InfoPath) -> str: | |
| name = str(path) | ||
| if not self._dist.has_metadata(name): | ||
| raise FileNotFoundError(name) | ||
| return self._dist.get_metadata(name) | ||
| content = self._dist.get_metadata(name) | ||
| if content is None: | ||
| raise NoneMetadataError(self, name) | ||
| return content | ||
|
|
||
| def iter_entry_points(self) -> Iterable[BaseEntryPoint]: | ||
| for group, entries in self._dist.get_entry_map().items(): | ||
|
|
@@ -142,7 +175,26 @@ def iter_entry_points(self) -> Iterable[BaseEntryPoint]: | |
|
|
||
| @property | ||
| def metadata(self) -> email.message.Message: | ||
| return get_metadata(self._dist) | ||
| """ | ||
| :raises NoneMetadataError: if the distribution reports `has_metadata()` | ||
| True but `get_metadata()` returns None. | ||
| """ | ||
| if isinstance(self._dist, pkg_resources.DistInfoDistribution): | ||
| metadata_name = "METADATA" | ||
| else: | ||
| metadata_name = "PKG-INFO" | ||
| try: | ||
| metadata = self.read_text(metadata_name) | ||
| except FileNotFoundError: | ||
| if self.location: | ||
| displaying_path = display_path(self.location) | ||
| else: | ||
| displaying_path = repr(self.location) | ||
| logger.warning("No metadata found in %s", displaying_path) | ||
| metadata = "" | ||
| feed_parser = email.parser.FeedParser() | ||
| feed_parser.feed(metadata) | ||
| return feed_parser.close() | ||
|
|
||
| def iter_dependencies(self, extras: Collection[str] = ()) -> Iterable[Requirement]: | ||
| if extras: # pkg_resources raises on invalid extras, so we sanitize. | ||
|
|
@@ -178,7 +230,6 @@ def _search_distribution(self, name: str) -> Optional[BaseDistribution]: | |
| return None | ||
|
|
||
| def get_distribution(self, name: str) -> Optional[BaseDistribution]: | ||
|
|
||
| # Search the distribution by looking through the working set. | ||
| dist = self._search_distribution(name) | ||
| if dist: | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For a follow up, we should probably give this a more meaningful name --
DeclaredMetadataFileIsMissingErroror something similar.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW this exception never actually makes sense; it’s caught in exactly two places, one just sets the result to an empty string, and the other is the outmost
except PipErrorblock that prints the exception as an error message. It will probably go away entirely when the importlib.metadata backend gets implemented.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, if I remember correctly, the situation which this was originally raised in was something weird, like a metadata file that has a truth has_metadata but get_metadata returned None. I'm not quite sure how this could happen, TBH. I do remember seeing this exception get raised in another situation in one of these porting PRs, which might have changed the meaning of it?
If we're sure that this will never happen with importlib metadata, then yea, this can probably go away.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That’s correct, and the exception can happen for permission reasons. But the thing is that this discrepency isn’t really necessary due to how the exception is handled. And this also does not happen in importlib.metadata because has_metadata does not even exist in the API—the only way to know if a metadata file exists in importlib.metadata is try to read it.