From e06968330ebfed90cb1d6e43b32aed3349e11c15 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 29 Oct 2025 14:28:33 +0000 Subject: [PATCH 1/7] Update `check_dependencies` to support markers --- synapse/util/check_dependencies.py | 130 +++++++++++++++++++++++++++-- 1 file changed, 123 insertions(+), 7 deletions(-) diff --git a/synapse/util/check_dependencies.py b/synapse/util/check_dependencies.py index 1c79c0be48d..f14ded73979 100644 --- a/synapse/util/check_dependencies.py +++ b/synapse/util/check_dependencies.py @@ -27,9 +27,11 @@ """ import logging +from functools import lru_cache 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 +67,24 @@ def dependencies(self) -> Iterable[str]: VERSION = metadata.version(DISTRIBUTION_NAME) +@lru_cache(maxsize=None) +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 ) @@ -95,6 +112,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,7 +121,7 @@ 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) @@ -111,6 +129,8 @@ 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): @@ -118,12 +138,83 @@ def _dependencies_for_extra(extra: str) -> Iterable[Dependency]: # 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] +) -> Sequence[str]: + """ + Augment `base_candidates` with extras explicitly mentioned in `marker`. + + Markers can mention extras (e.g. `extra == "saml2"`). + """ + + extras = list(dict.fromkeys(base_candidates)) + for candidate in _extras_from_marker(marker): + if candidate not in extras: + extras.append(candidate) + return extras + + +def _marker_applies_for_any_extra( + requirement: Requirement, extras: Sequence[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 +255,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 +265,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 +288,23 @@ def check_requirements(extra: Optional[str] = None) -> None: deps_unfulfilled = [] errors = [] + if extra is None: + base_extra_candidates: Sequence[str] = ("", *sorted(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: From 23e057101182a89bc017d4b37a3827241af8e740 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 29 Oct 2025 14:29:42 +0000 Subject: [PATCH 2/7] Add a unit test for checking dep versions based on python version A regression test for the issue we were seeing in https://github.com/element-hq/synapse/pull/19071#discussion_r2469055358 --- tests/util/test_check_dependencies.py | 54 ++++++++++++++++++++++++++- 1 file changed, 53 insertions(+), 1 deletion(-) diff --git a/tests/util/test_check_dependencies.py b/tests/util/test_check_dependencies.py index c052ba2b75f..a587a07285c 100644 --- a/tests/util/test_check_dependencies.py +++ b/tests/util/test_check_dependencies.py @@ -22,11 +22,14 @@ from contextlib import contextmanager from os import PathLike from pathlib import Path -from typing import Generator, Optional, Union +from typing import Generator, Optional, Union, cast from unittest.mock import patch +from packaging.markers import default_environment as packaging_default_environment + from synapse.util.check_dependencies import ( DependencyException, + _marker_environment, check_requirements, metadata, ) @@ -80,6 +83,27 @@ def mock_distribution(name: str) -> DummyDistribution: ): yield + @contextmanager + def mock_python_version(self, version: str) -> Generator[None, None, None]: + """Override the marker environment to report the supplied `python_version`.""" + + _marker_environment.cache_clear() + + def fake_default_environment() -> dict[str, str]: + env = cast(dict[str, str], dict(packaging_default_environment())) + env["python_version"] = version + env["python_full_version"] = f"{version}.0" + return env + + with patch( + "synapse.util.check_dependencies.default_environment", + side_effect=fake_default_environment, + ): + try: + yield + finally: + _marker_environment.cache_clear() + def test_mandatory_dependency(self) -> None: """Complain if a required package is missing or old.""" with patch( @@ -191,3 +215,31 @@ def test_setuptools_rust_ignored(self) -> None: with self.mock_installed_package(old): # We also ignore old versions of setuptools_rust check_requirements() + + def test_python_version_markers_respected(self) -> None: + """ + Tests that python_version markers are properly respected. + + Specifically that older versions of dependencies can be installed in + environments with older Python versions. + """ + requirements = [ + "pydantic ~= 2.8; python_version < '3.14'", + "pydantic ~= 2.12; python_version >= '3.14'", + ] + + with patch( + "synapse.util.check_dependencies.metadata.requires", + return_value=requirements, + ): + with self.mock_python_version("3.9"): + with self.mock_installed_package(DummyDistribution("2.8.1")): + check_requirements() + with self.mock_installed_package(DummyDistribution("2.7.0")): + self.assertRaises(DependencyException, check_requirements) + + with self.mock_python_version("3.14"): + with self.mock_installed_package(DummyDistribution("2.12.3")): + check_requirements() + with self.mock_installed_package(DummyDistribution("2.8.1")): + self.assertRaises(DependencyException, check_requirements) From a6bfc1187e796d341fa9f4e744e872ceb47e4267 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 29 Oct 2025 14:52:06 +0000 Subject: [PATCH 3/7] newsfile --- changelog.d/19110.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/19110.misc diff --git a/changelog.d/19110.misc b/changelog.d/19110.misc new file mode 100644 index 00000000000..dc45eef17ca --- /dev/null +++ b/changelog.d/19110.misc @@ -0,0 +1 @@ +Allow Synapse's runtime dependency checking code to take packaging markers (i.e. `python <= 3.14`) into account when checking dependencies. \ No newline at end of file From 40fa1eea2942c7e6faca547e5acdf72a5fcc9e3f Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 30 Oct 2025 18:50:07 +0000 Subject: [PATCH 4/7] Drop unnecessary `lru_cache` --- synapse/util/check_dependencies.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/synapse/util/check_dependencies.py b/synapse/util/check_dependencies.py index f14ded73979..335d6ce068b 100644 --- a/synapse/util/check_dependencies.py +++ b/synapse/util/check_dependencies.py @@ -27,7 +27,6 @@ """ import logging -from functools import lru_cache from importlib import metadata from typing import Any, Iterable, NamedTuple, Optional, Sequence, cast @@ -67,7 +66,6 @@ def dependencies(self) -> Iterable[str]: VERSION = metadata.version(DISTRIBUTION_NAME) -@lru_cache(maxsize=None) def _marker_environment(extra: str) -> dict[str, str]: """Return the marker environment for `extra`, seeded with the current interpreter.""" From 227bfdc4ef7b1d5dea3fb3b0559a8225f3b128b9 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 30 Oct 2025 18:50:47 +0000 Subject: [PATCH 5/7] Remove unnecessary `dict.fromkeys` and use a set Small perf tweak. --- synapse/util/check_dependencies.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/synapse/util/check_dependencies.py b/synapse/util/check_dependencies.py index 335d6ce068b..05ad691705c 100644 --- a/synapse/util/check_dependencies.py +++ b/synapse/util/check_dependencies.py @@ -186,23 +186,24 @@ def collect(tree: object) -> None: def _extras_to_consider_for_requirement( marker: Marker, base_candidates: Sequence[str] -) -> Sequence[str]: +) -> set[str]: """ Augment `base_candidates` with extras explicitly mentioned in `marker`. Markers can mention extras (e.g. `extra == "saml2"`). """ - extras = list(dict.fromkeys(base_candidates)) + # Avoid modifying the input sequence. + # Use a set to efficiently avoid duplicate extras. + extras = set(base_candidates) + for candidate in _extras_from_marker(marker): - if candidate not in extras: - extras.append(candidate) + extras.add(candidate) + return extras -def _marker_applies_for_any_extra( - requirement: Requirement, extras: Sequence[str] -) -> bool: +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: From 47418e86a9a1a0121b17ed20dff7a798c6a3b137 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 30 Oct 2025 18:51:11 +0000 Subject: [PATCH 6/7] Drop `sorted` and add explanation comment --- synapse/util/check_dependencies.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/synapse/util/check_dependencies.py b/synapse/util/check_dependencies.py index 05ad691705c..715240c8cee 100644 --- a/synapse/util/check_dependencies.py +++ b/synapse/util/check_dependencies.py @@ -288,7 +288,9 @@ def check_requirements(extra: Optional[str] = None) -> None: errors = [] if extra is None: - base_extra_candidates: Sequence[str] = ("", *sorted(RUNTIME_EXTRAS)) + # 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,) From 0a34d524a1393a1018c5d27a9464d5fcab0999f0 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 30 Oct 2025 18:55:39 +0000 Subject: [PATCH 7/7] Update unit tests to include all package ver variants --- tests/util/test_check_dependencies.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/tests/util/test_check_dependencies.py b/tests/util/test_check_dependencies.py index a587a07285c..ab2e2f6291b 100644 --- a/tests/util/test_check_dependencies.py +++ b/tests/util/test_check_dependencies.py @@ -29,7 +29,6 @@ from synapse.util.check_dependencies import ( DependencyException, - _marker_environment, check_requirements, metadata, ) @@ -87,8 +86,6 @@ def mock_distribution(name: str) -> DummyDistribution: def mock_python_version(self, version: str) -> Generator[None, None, None]: """Override the marker environment to report the supplied `python_version`.""" - _marker_environment.cache_clear() - def fake_default_environment() -> dict[str, str]: env = cast(dict[str, str], dict(packaging_default_environment())) env["python_version"] = version @@ -99,10 +96,7 @@ def fake_default_environment() -> dict[str, str]: "synapse.util.check_dependencies.default_environment", side_effect=fake_default_environment, ): - try: - yield - finally: - _marker_environment.cache_clear() + yield def test_mandatory_dependency(self) -> None: """Complain if a required package is missing or old.""" @@ -233,6 +227,8 @@ def test_python_version_markers_respected(self) -> None: return_value=requirements, ): with self.mock_python_version("3.9"): + with self.mock_installed_package(DummyDistribution("2.12.3")): + check_requirements() with self.mock_installed_package(DummyDistribution("2.8.1")): check_requirements() with self.mock_installed_package(DummyDistribution("2.7.0")): @@ -243,3 +239,5 @@ def test_python_version_markers_respected(self) -> None: check_requirements() with self.mock_installed_package(DummyDistribution("2.8.1")): self.assertRaises(DependencyException, check_requirements) + with self.mock_installed_package(DummyDistribution("2.7.0")): + self.assertRaises(DependencyException, check_requirements)