Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Don't require setuptools_rust at runtime #13952

Merged
merged 3 commits into from
Sep 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Comment on lines +70 to +71
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we link to a poetry bug here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

python-poetry/poetry#6154 I guess would be this and is linked to someplace else, so probably OK not to.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't 100% sure if this is considered a bug by poetry's authors, but the issue is a good point of reference---I'll gladly add it

# 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()