From ceff48c7bfc5ff9b738c539d02b4590e4ec26d24 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Thu, 29 Sep 2022 19:26:15 +0100 Subject: [PATCH 1/3] Don't require `setuptools_rust` at runtime --- synapse/util/check_dependencies.py | 13 ++++++++++++- tests/util/test_check_dependencies.py | 20 ++++++++++++++++++-- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/synapse/util/check_dependencies.py b/synapse/util/check_dependencies.py index 66f1da75028..0fb1a8fb72a 100644 --- a/synapse/util/check_dependencies.py +++ b/synapse/util/check_dependencies.py @@ -66,6 +66,17 @@ def _is_dev_dependency(req: Requirement) -> bool: ) +def _should_ignore_runtime_requirement(req: Requirement) -> bool: + # This is a build-time dependency. Irritatingly, `poetry build` ignores the + # requirements listed in the [build-system] section of pyproject.toml, so in order + # to support `poetry install --no-dev` we have to mark it as a runtime dependency. + # Workaround this by ignoring it here. (It might be slightly cleaner to put + # `setuptools_rust` in a `build` extra or similar, but . But for now I' + if req.name == "setuptools_rust": + return True + return False + + class Dependency(NamedTuple): requirement: Requirement must_be_installed: bool @@ -77,7 +88,7 @@ def _generic_dependencies() -> Iterable[Dependency]: assert requirements is not None for raw_requirement in requirements: req = Requirement(raw_requirement) - if _is_dev_dependency(req): + if _is_dev_dependency(req) or _should_ignore_runtime_requirement(req): continue # https://packaging.pypa.io/en/latest/markers.html#usage notes that diff --git a/tests/util/test_check_dependencies.py b/tests/util/test_check_dependencies.py index 5d1aa025d12..6913de24b9c 100644 --- a/tests/util/test_check_dependencies.py +++ b/tests/util/test_check_dependencies.py @@ -40,7 +40,10 @@ class TestDependencyChecker(TestCase): def mock_installed_package( self, distribution: Optional[DummyDistribution] ) -> Generator[None, None, None]: - """Pretend that looking up any distribution yields the given `distribution`.""" + """Pretend that looking up any package yields the given `distribution`. + + If `distribution = None`, we pretend that the package is not installed. + """ def mock_distribution(name: str): if distribution is None: @@ -81,7 +84,7 @@ def test_version_reported_as_none(self) -> None: self.assertRaises(DependencyException, check_requirements) def test_checks_ignore_dev_dependencies(self) -> None: - """Bot generic and per-extra checks should ignore dev dependencies.""" + """Both generic and per-extra checks should ignore dev dependencies.""" with patch( "synapse.util.check_dependencies.metadata.requires", return_value=["dummypkg >= 1; extra == 'mypy'"], @@ -142,3 +145,16 @@ def test_release_candidates_satisfy_dependency(self) -> None: with self.mock_installed_package(new_release_candidate): # should not raise check_requirements() + + def test_setuptools_rust_ignored(self) -> None: + """Test a workaround for a `poetry build` problem. Reproduces #13926.""" + with patch( + "synapse.util.check_dependencies.metadata.requires", + return_value=["setuptools_rust >= 1.3"], + ): + with self.mock_installed_package(None): + # should not raise, even if setuptools_rust is not installed + check_requirements() + with self.mock_installed_package(old): + # We also ignore old versions of setuptools_rust + check_requirements() From b7dab6f99ac46ce35f90f8cd25eab56a8ebd67ec Mon Sep 17 00:00:00 2001 From: David Robertson Date: Thu, 29 Sep 2022 19:32:02 +0100 Subject: [PATCH 2/3] Changelog --- changelog.d/13952.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/13952.bugfix diff --git a/changelog.d/13952.bugfix b/changelog.d/13952.bugfix new file mode 100644 index 00000000000..a6af20f0518 --- /dev/null +++ b/changelog.d/13952.bugfix @@ -0,0 +1 @@ +Fix a bug introduced in v1.68.0 where Synapse would require `setuptools_rust` at runtime, even though the package is only required at build time. From 76abcb27b7f21e0978f1ad7019b816fe9731a816 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Thu, 29 Sep 2022 19:43:04 +0100 Subject: [PATCH 3/3] Finish your sentence, boy; poetry issue reference --- synapse/util/check_dependencies.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/synapse/util/check_dependencies.py b/synapse/util/check_dependencies.py index 0fb1a8fb72a..3b1e2057002 100644 --- a/synapse/util/check_dependencies.py +++ b/synapse/util/check_dependencies.py @@ -70,8 +70,12 @@ def _should_ignore_runtime_requirement(req: Requirement) -> bool: # This is a build-time dependency. Irritatingly, `poetry build` ignores the # requirements listed in the [build-system] section of pyproject.toml, so in order # to support `poetry install --no-dev` we have to mark it as a runtime dependency. - # Workaround this by ignoring it here. (It might be slightly cleaner to put - # `setuptools_rust` in a `build` extra or similar, but . But for now I' + # See discussion on https://github.com/python-poetry/poetry/issues/6154 (it sounds + # like the poetry authors don't consider this a bug?) + # + # In any case, workaround this by ignoring setuptools_rust here. (It might be + # slightly cleaner to put `setuptools_rust` in a `build` extra or similar, but for + # now let's do something quick and dirty. if req.name == "setuptools_rust": return True return False