-
Notifications
You must be signed in to change notification settings - Fork 508
Update check_dependencies to support markers
#19110
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
e069683
23e0571
a6bfc11
40fa1ee
227bfdc
47418e8
0a34d52
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 |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Allow Synapse's runtime dependency checking code to take packaging markers (i.e. `python <= 3.14`) into account when checking dependencies. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,8 +28,9 @@ | |
|
|
||
| import logging | ||
| from importlib import metadata | ||
| from typing import Iterable, NamedTuple, Optional | ||
| from typing import Any, Iterable, NamedTuple, Optional, Sequence, cast | ||
|
|
||
| from packaging.markers import Marker, Value, Variable, default_environment | ||
| from packaging.requirements import Requirement | ||
|
|
||
| DISTRIBUTION_NAME = "matrix-synapse" | ||
|
|
@@ -65,9 +66,23 @@ def dependencies(self) -> Iterable[str]: | |
| VERSION = metadata.version(DISTRIBUTION_NAME) | ||
|
|
||
|
|
||
| def _marker_environment(extra: str) -> dict[str, str]: | ||
| """Return the marker environment for `extra`, seeded with the current interpreter.""" | ||
|
|
||
| env = cast(dict[str, str], dict(default_environment())) | ||
| env["extra"] = extra | ||
| return env | ||
|
|
||
|
|
||
| def _is_dev_dependency(req: Requirement) -> bool: | ||
| return req.marker is not None and any( | ||
| req.marker.evaluate({"extra": e}) for e in DEV_EXTRAS | ||
| """Return True if `req` is a development dependency.""" | ||
| if req.marker is None: | ||
| return False | ||
|
|
||
| marker_extras = _extras_from_marker(req.marker) | ||
| return any( | ||
| extra in DEV_EXTRAS and req.marker.evaluate(_marker_environment(extra)) | ||
| for extra in marker_extras | ||
| ) | ||
|
Comment on lines
+83
to
86
Contributor
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. This looks similar to the logic in Although we have an extra Just a note and I think this is fine ⏩
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, it's a bit of a shame. But the only ideal way to combine them would be adding a boolean argument to the function signature, and that feels dirtier than the two (simple) methods IMO. |
||
|
|
||
|
|
||
|
|
@@ -95,6 +110,7 @@ def _generic_dependencies() -> Iterable[Dependency]: | |
| """Yield pairs (requirement, must_be_installed).""" | ||
| requirements = metadata.requires(DISTRIBUTION_NAME) | ||
| assert requirements is not None | ||
| env_no_extra = _marker_environment("") | ||
| for raw_requirement in requirements: | ||
| req = Requirement(raw_requirement) | ||
| if _is_dev_dependency(req) or _should_ignore_runtime_requirement(req): | ||
|
|
@@ -103,27 +119,101 @@ def _generic_dependencies() -> Iterable[Dependency]: | |
| # 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": ""}) | ||
| must_be_installed = req.marker is None or req.marker.evaluate(env_no_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(DISTRIBUTION_NAME) | ||
| assert requirements is not None | ||
| env_no_extra = _marker_environment("") | ||
| env_for_extra = _marker_environment(extra) | ||
| for raw_requirement in requirements: | ||
| req = Requirement(raw_requirement) | ||
| if _is_dev_dependency(req): | ||
| continue | ||
| # 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": ""}) | ||
| and req.marker.evaluate(env_for_extra) | ||
| and not req.marker.evaluate(env_no_extra) | ||
| ): | ||
| yield Dependency(req, True) | ||
|
|
||
|
|
||
| def _values_from_marker_value(value: Value) -> set[str]: | ||
| """Extract text values contained in a marker `Value`.""" | ||
|
|
||
| raw: Any = value.value | ||
| if isinstance(raw, str): | ||
| return {raw} | ||
| if isinstance(raw, (tuple, list)): | ||
| return {str(item) for item in raw} | ||
| return {str(raw)} | ||
|
|
||
|
|
||
| def _extras_from_marker(marker: Optional[Marker]) -> set[str]: | ||
| """Return every `extra` referenced in the supplied marker tree.""" | ||
|
|
||
| extras: set[str] = set() | ||
|
|
||
| if marker is None: | ||
| return extras | ||
|
|
||
| def collect(tree: object) -> None: | ||
| if isinstance(tree, list): | ||
| for item in tree: | ||
| collect(item) | ||
| elif isinstance(tree, tuple) and len(tree) == 3: | ||
| lhs, _op, rhs = tree | ||
| if ( | ||
| isinstance(lhs, Variable) | ||
| and lhs.value == "extra" | ||
| and isinstance(rhs, Value) | ||
| ): | ||
| extras.update(_values_from_marker_value(rhs)) | ||
| elif ( | ||
| isinstance(rhs, Variable) | ||
| and rhs.value == "extra" | ||
| and isinstance(lhs, Value) | ||
| ): | ||
| extras.update(_values_from_marker_value(lhs)) | ||
|
|
||
| collect(marker._markers) | ||
| return extras | ||
|
|
||
|
|
||
| def _extras_to_consider_for_requirement( | ||
| marker: Marker, base_candidates: Sequence[str] | ||
| ) -> set[str]: | ||
| """ | ||
| Augment `base_candidates` with extras explicitly mentioned in `marker`. | ||
|
|
||
| Markers can mention extras (e.g. `extra == "saml2"`). | ||
| """ | ||
|
|
||
| # Avoid modifying the input sequence. | ||
| # Use a set to efficiently avoid duplicate extras. | ||
| extras = set(base_candidates) | ||
|
|
||
| for candidate in _extras_from_marker(marker): | ||
| extras.add(candidate) | ||
|
|
||
| return extras | ||
|
|
||
|
|
||
| def _marker_applies_for_any_extra(requirement: Requirement, extras: set[str]) -> bool: | ||
| """Check whether a requirement's marker matches any evaluated `extra`.""" | ||
|
|
||
| if requirement.marker is None: | ||
| return True | ||
|
|
||
| return any( | ||
| requirement.marker.evaluate(_marker_environment(extra)) for extra in extras | ||
| ) | ||
|
|
||
|
|
||
| def _not_installed(requirement: Requirement, extra: Optional[str] = None) -> str: | ||
| if extra: | ||
| return ( | ||
|
|
@@ -164,7 +254,7 @@ def _no_reported_version(requirement: Requirement, extra: Optional[str] = None) | |
| def check_requirements(extra: Optional[str] = None) -> None: | ||
| """Check Synapse's dependencies are present and correctly versioned. | ||
|
|
||
| If provided, `extra` must be the name of an pacakging extra (e.g. "saml2" in | ||
| If provided, `extra` must be the name of an packaging extra (e.g. "saml2" in | ||
| `pip install matrix-synapse[saml2]`). | ||
|
|
||
| If `extra` is None, this function checks that | ||
|
|
@@ -174,6 +264,15 @@ def check_requirements(extra: Optional[str] = None) -> None: | |
| If `extra` is not None, this function checks that | ||
| - the dependencies needed for that extra are installed and correctly versioned. | ||
|
|
||
| `marker`s are optional attributes on each requirement which specify | ||
| conditions under which the requirement applies. For example, a requirement | ||
| might only be needed on Windows, or with Python < 3.14. Markers can | ||
| additionally mention `extras` themselves, meaning a requirement may not | ||
| apply if the marker mentions an extra that the user has not asked for. | ||
|
|
||
| This function skips a requirement when its markers do not apply in the | ||
| current environment. | ||
|
|
||
| :raises DependencyException: if a dependency is missing or incorrectly versioned. | ||
| :raises ValueError: if this extra does not exist. | ||
| """ | ||
|
|
@@ -188,7 +287,25 @@ def check_requirements(extra: Optional[str] = None) -> None: | |
| deps_unfulfilled = [] | ||
| errors = [] | ||
|
|
||
| if extra is None: | ||
| # Default to all mandatory dependencies (non-dev extras). | ||
| # "" means all dependencies that aren't conditional on an extra. | ||
| base_extra_candidates: Sequence[str] = ("", *RUNTIME_EXTRAS) | ||
| else: | ||
| base_extra_candidates = (extra,) | ||
|
|
||
| for requirement, must_be_installed in dependencies: | ||
| if requirement.marker is not None: | ||
| candidate_extras = _extras_to_consider_for_requirement( | ||
| requirement.marker, base_extra_candidates | ||
| ) | ||
| # Skip checking this dependency if the requirement's marker object | ||
| # (i.e. `python_version < "3.14" and os_name == "win32"`) does not | ||
| # apply for any of the extras we're considering. | ||
| if not _marker_applies_for_any_extra(requirement, candidate_extras): | ||
| continue | ||
|
|
||
| # Check if the requirement is installed and correctly versioned. | ||
| try: | ||
| dist: metadata.Distribution = metadata.distribution(requirement.name) | ||
| except metadata.PackageNotFoundError: | ||
|
|
||
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.
Nothing seems too crazy. And the background from the PR description makes sense. I haven't made sure all of the pieces here are aligning in terms of giving a complete picture but I assume the tests cover this.
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, and the fact that the linked Pydantic v2 PR now passes CI won me over.