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

Try to detect borked package installations. #12244

Merged
merged 8 commits into from Mar 18, 2022
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/12244.misc
@@ -0,0 +1 @@
Improve error message when dependencies check finds a broken installation.
23 changes: 22 additions & 1 deletion synapse/util/check_dependencies.py
Expand Up @@ -128,6 +128,21 @@ def _incorrect_version(
)


def _no_reported_version(
requirement: Requirement, extra: Optional[str] = None
):
if extra:
return (
f"Synapse {VERSION} needs {requirement} for {extra}, "
f"but can't determine {requirement.name}'s version"
)
else:
return (
f"Synapse {VERSION} needs {requirement}, "
f"but can't determine {requirement.name}'s version"
)


def check_requirements(extra: Optional[str] = None) -> None:
"""Check Synapse's dependencies are present and correctly versioned.

Expand Down Expand Up @@ -163,8 +178,14 @@ def check_requirements(extra: Optional[str] = None) -> None:
deps_unfulfilled.append(requirement.name)
errors.append(_not_installed(requirement, extra))
else:
if dist.version is None:
# This shouldn't happen---it suggests a borked virtualenv. (See #12223)
# Try to give a vaguely helpful error message anyway.
deps_unfulfilled.append(requirement.name)
errors.append(_no_reported_version(requirement, extra))

# We specify prereleases=True to allow prereleases such as RCs.
if not requirement.specifier.contains(dist.version, prereleases=True):
elif not requirement.specifier.contains(dist.version, prereleases=True):
deps_unfulfilled.append(requirement.name)
errors.append(_incorrect_version(requirement, dist.version, extra))

Expand Down
15 changes: 14 additions & 1 deletion tests/util/test_check_dependencies.py
Expand Up @@ -12,7 +12,7 @@


class DummyDistribution(metadata.Distribution):
def __init__(self, version: str):
def __init__(self, version: object):
self._version = version

@property
Expand All @@ -30,6 +30,7 @@ def read_text(self, filename):
old_release_candidate = DummyDistribution("0.1.2rc3")
new = DummyDistribution("1.2.3")
new_release_candidate = DummyDistribution("1.2.3rc4")
distribution_with_no_version = DummyDistribution(None)

# could probably use stdlib TestCase --- no need for twisted here

Expand Down Expand Up @@ -67,6 +68,18 @@ def test_mandatory_dependency(self) -> None:
# should not raise
check_requirements()

def test_version_reported_as_None(self) -> None:
DMRobertson marked this conversation as resolved.
Show resolved Hide resolved
"""Complain if importlib.metadata.version() returns None.

This shouldn't normally happen, but it was seen in the wild (#12223).
"""
with patch(
"synapse.util.check_dependencies.metadata.requires",
return_value=["dummypkg >= 1"],
):
with self.mock_installed_package(distribution_with_no_version):
self.assertRaises(DependencyException, check_requirements)

def test_checks_ignore_dev_dependencies(self) -> None:
"""Bot generic and per-extra checks should ignore dev dependencies."""
with patch(
Expand Down