Skip to content

Commit

Permalink
Don't require setuptools_rust at runtime (matrix-org#13952)
Browse files Browse the repository at this point in the history
  • Loading branch information
David Robertson authored and Oleg Girko committed Oct 11, 2022
1 parent 5b83f77 commit 422a148
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 3 deletions.
1 change: 1 addition & 0 deletions changelog.d/13952.bugfix
Original file line number Diff line number Diff line change
@@ -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.
17 changes: 16 additions & 1 deletion synapse/util/check_dependencies.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,21 @@ 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.
# 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


class Dependency(NamedTuple):
requirement: Requirement
must_be_installed: bool
Expand All @@ -77,7 +92,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
Expand Down
20 changes: 18 additions & 2 deletions tests/util/test_check_dependencies.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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'"],
Expand Down Expand Up @@ -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()

0 comments on commit 422a148

Please sign in to comment.