Skip to content
Snippets Groups Projects
Unverified Commit bf9d549e authored by David Robertson's avatar David Robertson Committed by GitHub
Browse files

Try to detect borked package installations. (#12244)


* Try to detect borked package installations.

Fixes #12223.

Co-authored-by: default avatarSean Quah <8349537+squahtx@users.noreply.github.com>
parent 8fe930c2
No related branches found
No related tags found
No related merge requests found
Improve error message when dependencies check finds a broken installation.
\ No newline at end of file
...@@ -128,6 +128,19 @@ def _incorrect_version( ...@@ -128,6 +128,19 @@ def _incorrect_version(
) )
def _no_reported_version(requirement: Requirement, extra: Optional[str] = None) -> str:
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: def check_requirements(extra: Optional[str] = None) -> None:
"""Check Synapse's dependencies are present and correctly versioned. """Check Synapse's dependencies are present and correctly versioned.
...@@ -163,8 +176,17 @@ def check_requirements(extra: Optional[str] = None) -> None: ...@@ -163,8 +176,17 @@ def check_requirements(extra: Optional[str] = None) -> None:
deps_unfulfilled.append(requirement.name) deps_unfulfilled.append(requirement.name)
errors.append(_not_installed(requirement, extra)) errors.append(_not_installed(requirement, extra))
else: 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.
# Type-ignore: the annotations don't reflect reality: see
# https://github.com/python/typeshed/issues/7513
# https://bugs.python.org/issue47060
deps_unfulfilled.append(requirement.name) # type: ignore[unreachable]
errors.append(_no_reported_version(requirement, extra))
# We specify prereleases=True to allow prereleases such as RCs. # 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) deps_unfulfilled.append(requirement.name)
errors.append(_incorrect_version(requirement, dist.version, extra)) errors.append(_incorrect_version(requirement, dist.version, extra))
......
...@@ -12,7 +12,7 @@ from tests.unittest import TestCase ...@@ -12,7 +12,7 @@ from tests.unittest import TestCase
class DummyDistribution(metadata.Distribution): class DummyDistribution(metadata.Distribution):
def __init__(self, version: str): def __init__(self, version: object):
self._version = version self._version = version
@property @property
...@@ -30,6 +30,7 @@ old = DummyDistribution("0.1.2") ...@@ -30,6 +30,7 @@ old = DummyDistribution("0.1.2")
old_release_candidate = DummyDistribution("0.1.2rc3") old_release_candidate = DummyDistribution("0.1.2rc3")
new = DummyDistribution("1.2.3") new = DummyDistribution("1.2.3")
new_release_candidate = DummyDistribution("1.2.3rc4") new_release_candidate = DummyDistribution("1.2.3rc4")
distribution_with_no_version = DummyDistribution(None)
# could probably use stdlib TestCase --- no need for twisted here # could probably use stdlib TestCase --- no need for twisted here
...@@ -67,6 +68,18 @@ class TestDependencyChecker(TestCase): ...@@ -67,6 +68,18 @@ class TestDependencyChecker(TestCase):
# should not raise # should not raise
check_requirements() check_requirements()
def test_version_reported_as_none(self) -> None:
"""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: def test_checks_ignore_dev_dependencies(self) -> None:
"""Bot generic and per-extra checks should ignore dev dependencies.""" """Bot generic and per-extra checks should ignore dev dependencies."""
with patch( with patch(
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment