From d88bab5ce4ea04a27afab609e52c8927f291db7d Mon Sep 17 00:00:00 2001 From: David Robertson Date: Fri, 25 Feb 2022 15:47:33 +0000 Subject: [PATCH] Reimplement `check_requirements` using `importlib` I've tried to make this clearer. We start by working out which of Synapse's requirements we need to be installed here and now. I was surprised that there wasn't an easier way to see which packages were installed by a given extra. I've pulled out the error messages into functions that deal with "is this for an extra or not". And I've rearranged the loop over two different sets of requirements into one loop with a "must be instaled" flag. I hope you agree that this is clearer. --- synapse/python_dependencies.py | 2 + synapse/util/check_dependencies.py | 168 ++++++++++++++++------------- 2 files changed, 97 insertions(+), 73 deletions(-) diff --git a/synapse/python_dependencies.py b/synapse/python_dependencies.py index 17f28b333066..8f48a33936c1 100644 --- a/synapse/python_dependencies.py +++ b/synapse/python_dependencies.py @@ -83,6 +83,8 @@ # ijson 3.1.4 fixes a bug with "." in property names "ijson>=3.1.4", "matrix-common~=1.1.0", + # For runtime introspection of our dependencies + "packaging~=21.3", ] CONDITIONAL_REQUIREMENTS = { diff --git a/synapse/util/check_dependencies.py b/synapse/util/check_dependencies.py index 0cc1a6333754..1ba25011eef8 100644 --- a/synapse/util/check_dependencies.py +++ b/synapse/util/check_dependencies.py @@ -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 = [] + 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)