This repository has been archived by the owner on Apr 26, 2024. It is now read-only.
-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Use importlib.metadata to read requirements #12088
Merged
Merged
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
e49fd50
Pull runtime dep checks into their own module
94ddc49
Changelog
d88bab5
Reimplement `check_requirements` using `importlib`
51e76ec
Type hints for existing code
c7eee4f
Test cases
9bed98f
Use `.get_all`; add distribution pkg name constant
903e95c
needed -> unfulfilled
40dbcb2
isort && flake8
24a4b54
And fix an extra newline
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,14 +1,12 @@ | ||
import logging | ||
from typing import List | ||
from typing import Iterable, NamedTuple, Optional | ||
|
||
from pkg_resources import ( | ||
DistributionNotFound, | ||
Requirement, | ||
VersionConflict, | ||
get_provider, | ||
) | ||
try: | ||
from importlib import metadata | ||
except ImportError: | ||
import importlib_metadata as metadata # type: ignore[no-redef] | ||
|
||
from synapse.python_dependencies import CONDITIONAL_REQUIREMENTS, REQUIREMENTS | ||
from packaging.requirements import Requirement | ||
|
||
|
||
class DependencyException(Exception): | ||
|
@@ -29,79 +27,103 @@ def dependencies(self): | |
yield '"' + i + '"' | ||
|
||
|
||
def check_requirements(for_feature=None): | ||
deps_needed = [] | ||
errors = [] | ||
EXTRAS = { | ||
v | ||
for k, v in metadata.distribution("matrix-synapse").metadata.items() | ||
if k == "Provides-Extra" | ||
} | ||
|
||
|
||
class Dependency(NamedTuple): | ||
requirement: Requirement | ||
must_be_installed: bool | ||
|
||
|
||
def _generic_dependencies() -> Iterable[Dependency]: | ||
"""Yield pairs (requirement, must_be_installed).""" | ||
requirements = metadata.requires("matrix-synapse") | ||
assert requirements is not None | ||
for raw_requirement in requirements: | ||
req = Requirement(raw_requirement) | ||
# https://packaging.pypa.io/en/latest/markers.html#usage notes that | ||
# > Evaluating an extra marker with no environment is an error | ||
# so we pass in a dummy empty extra value here. | ||
must_be_installed = req.marker is None or req.marker.evaluate({"extra": ""}) | ||
yield Dependency(req, must_be_installed) | ||
|
||
|
||
def _dependencies_for_extra(extra: str) -> Iterable[Dependency]: | ||
"""Yield additional dependencies needed for a given `extra`.""" | ||
requirements = metadata.requires("matrix-synapse") | ||
assert requirements is not None | ||
for raw_requirement in requirements: | ||
req = Requirement(raw_requirement) | ||
# Exclude mandatory deps by only selecting deps needed with this extra. | ||
if ( | ||
req.marker is not None | ||
and req.marker.evaluate({"extra": extra}) | ||
and not req.marker.evaluate({"extra": ""}) | ||
): | ||
yield Dependency(req, True) | ||
|
||
|
||
def _not_installed(requirement: Requirement, extra: Optional[str] = None) -> str: | ||
if extra: | ||
return f"Need {requirement.name} for {extra}, but it is not installed" | ||
else: | ||
return f"Need {requirement.name}, but it is not installed" | ||
|
||
if for_feature: | ||
reqs = CONDITIONAL_REQUIREMENTS[for_feature] | ||
|
||
def _incorrect_version( | ||
requirement: Requirement, got: str, extra: Optional[str] = None | ||
) -> str: | ||
if extra: | ||
return f"Need {requirement} for {extra}, but got {requirement.name}=={got}" | ||
else: | ||
reqs = REQUIREMENTS | ||
return f"Need {requirement}, but got {requirement.name}=={got}" | ||
|
||
for dependency in reqs: | ||
try: | ||
_check_requirement(dependency) | ||
except VersionConflict as e: | ||
deps_needed.append(dependency) | ||
errors.append( | ||
"Needed %s, got %s==%s" | ||
% ( | ||
dependency, | ||
e.dist.project_name, # type: ignore[attr-defined] # noqa | ||
e.dist.version, # type: ignore[attr-defined] # noqa | ||
) | ||
) | ||
except DistributionNotFound: | ||
deps_needed.append(dependency) | ||
if for_feature: | ||
errors.append( | ||
"Needed %s for the '%s' feature but it was not installed" | ||
% (dependency, for_feature) | ||
) | ||
else: | ||
errors.append("Needed %s but it was not installed" % (dependency,)) | ||
|
||
if not for_feature: | ||
# Check the optional dependencies are up to date. We allow them to not be | ||
# installed. | ||
OPTS: List[str] = sum(CONDITIONAL_REQUIREMENTS.values(), []) | ||
|
||
for dependency in OPTS: | ||
try: | ||
_check_requirement(dependency) | ||
except VersionConflict as e: | ||
deps_needed.append(dependency) | ||
errors.append( | ||
"Needed optional %s, got %s==%s" | ||
% ( | ||
dependency, | ||
e.dist.project_name, # type: ignore[attr-defined] # noqa | ||
e.dist.version, # type: ignore[attr-defined] # noqa | ||
) | ||
) | ||
except DistributionNotFound: | ||
# If it's not found, we don't care | ||
pass | ||
|
||
if deps_needed: | ||
for err in errors: | ||
logging.error(err) | ||
def check_requirements(extra: Optional[str] = None) -> None: | ||
"""Check Synapse's dependencies are present and correctly versioned. | ||
|
||
raise DependencyException(deps_needed) | ||
If provided, `extra` must be the name of an pacakging extra (e.g. "saml2" in | ||
`pip install matrix-synapse[saml2]`). | ||
|
||
If `extra` is None, this function checks that | ||
- all mandatory dependencies are installed and correctly versioned, and | ||
- each optional dependency that's installed is correctly versioned. | ||
|
||
def _check_requirement(dependency_string): | ||
"""Parses a dependency string, and checks if the specified requirement is installed | ||
If `extra` is not None, this function checks that | ||
- the dependencies needed for that extra are installed and correctly versioned. | ||
|
||
Raises: | ||
VersionConflict if the requirement is installed, but with the the wrong version | ||
DistributionNotFound if nothing is found to provide the requirement | ||
:raises DependencyException: if a dependency is missing or incorrectly versioned. | ||
:raises ValueError: if this extra does not exist. | ||
""" | ||
req = Requirement.parse(dependency_string) | ||
# First work out which dependencies are required, and which are optional. | ||
if extra is None: | ||
dependencies = _generic_dependencies() | ||
elif extra in EXTRAS: | ||
dependencies = _dependencies_for_extra(extra) | ||
else: | ||
raise ValueError(f"Synapse does not provide the feature '{extra}'") | ||
|
||
# first check if the markers specify that this requirement needs installing | ||
if req.marker is not None and not req.marker.evaluate(): | ||
# not required for this environment | ||
return | ||
deps_needed = [] | ||
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. nit: 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. Can do. This came from the original implementation, but finding the right words for all of this stuff is tricky. |
||
errors = [] | ||
|
||
get_provider(req) | ||
for (requirement, must_be_installed) in dependencies: | ||
try: | ||
dist: metadata.Distribution = metadata.distribution(requirement.name) | ||
except metadata.PackageNotFoundError: | ||
if must_be_installed: | ||
deps_needed.append(requirement.name) | ||
errors.append(_not_installed(requirement, extra)) | ||
else: | ||
if not requirement.specifier.contains(dist.version): | ||
deps_needed.append(requirement.name) | ||
errors.append(_incorrect_version(requirement, dist.version, extra)) | ||
|
||
if deps_needed: | ||
for err in errors: | ||
logging.error(err) | ||
|
||
raise DependencyException(deps_needed) |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
set(metadata.distribution("matrix-synapse").metadata.get_all("Provides-Extra"))
perhaps?Or even
set(metadata.metadata("matrix-synapse").get_all("Provides-Extra"))
Disclaimer: I've only confirmed it works on my machine and not in general
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 seems neater---will check this out!
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.
Seems kosher to me: https://importlib-metadata.readthedocs.io/en/latest/api.html#importlib_metadata.PackageMetadata.get_all