Repository: pypa/build
Pull request #512 (2022-09-02 20:05:10)

build: add circular dependency protection


Patch:
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index f918d90..8e9f768 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -3,6 +3,16 @@ Changelog
 +++++++++
 
 
+Unreleased
+==========
+
+- Fix infinite recursion error in ``check_dependency`` with circular
+  dependencies (`PR #512`_, Fixes `#511`_)
+
+.. _PR #512: https://github.com/pypa/build/pull/512
+.. _#511: https://github.com/pypa/build/issues/511
+
+
 0.8.0 (2022-05-22)
 ==================
 
diff --git a/src/build/__init__.py b/src/build/__init__.py
index ea534b3..549f738 100644
--- a/src/build/__init__.py
+++ b/src/build/__init__.py
@@ -176,6 +176,13 @@ def check_dependency(
         import importlib_metadata
 
     req = packaging.requirements.Requirement(req_string)
+    normalised_req_string = str(req)
+
+    # ``Requirement`` doesn't implement ``__eq__`` so we cannot compare reqs for
+    # equality directly but the string representation is stable.
+    if normalised_req_string in ancestral_req_strings:
+        # cyclical dependency, already checked.
+        return
 
     if req.marker:
         extras = frozenset(('',)).union(parent_extras)
@@ -190,15 +197,15 @@ def check_dependency(
         dist = importlib_metadata.distribution(req.name)  # type: ignore[no-untyped-call]
     except importlib_metadata.PackageNotFoundError:
         # dependency is not installed in the environment.
-        yield ancestral_req_strings + (req_string,)
+        yield ancestral_req_strings + (normalised_req_string,)
     else:
         if req.specifier and not req.specifier.contains(dist.version, prereleases=True):
             # the installed version is incompatible.
-            yield ancestral_req_strings + (req_string,)
+            yield ancestral_req_strings + (normalised_req_string,)
         elif dist.requires:
             for other_req_string in dist.requires:
                 # yields transitive dependencies that are not satisfied.
-                yield from check_dependency(other_req_string, ancestral_req_strings + (req_string,), req.extras)
+                yield from check_dependency(other_req_string, ancestral_req_strings + (normalised_req_string,), req.extras)
 
 
 def _find_typo(dictionary: Mapping[str, str], expected: str) -> None:


Test patch:
diff --git a/tests/test_projectbuilder.py b/tests/test_projectbuilder.py
index a05b6ca..5475d7e 100644
--- a/tests/test_projectbuilder.py
+++ b/tests/test_projectbuilder.py
@@ -45,55 +45,93 @@ class MockDistribution(importlib_metadata.Distribution):
             return RecursiveMockDistribution()
         elif name == 'prerelease_dep':
             return PrereleaseMockDistribution()
+        elif name == 'circular_dep':
+            return CircularMockDistribution()
+        elif name == 'nested_circular_dep':
+            return NestedCircularMockDistribution()
         raise importlib_metadata.PackageNotFoundError
 
 
 class ExtraMockDistribution(MockDistribution):
     def read_text(self, filename):
         if filename == 'METADATA':
-            return """
-Metadata-Version: 2.2
-Name: extras_dep
-Version: 1.0.0
-Provides-Extra: extra_without_associated_deps
-Provides-Extra: extra_with_unmet_deps
-Requires-Dist: unmet_dep; extra == 'extra_with_unmet_deps'
-Provides-Extra: extra_with_met_deps
-Requires-Dist: extras_dep; extra == 'extra_with_met_deps'
-Provides-Extra: recursive_extra_with_unmet_deps
-Requires-Dist: recursive_dep; extra == 'recursive_extra_with_unmet_deps'
-""".strip()
+            return textwrap.dedent(
+                """
+                Metadata-Version: 2.2
+                Name: extras_dep
+                Version: 1.0.0
+                Provides-Extra: extra_without_associated_deps
+                Provides-Extra: extra_with_unmet_deps
+                Requires-Dist: unmet_dep; extra == 'extra_with_unmet_deps'
+                Provides-Extra: extra_with_met_deps
+                Requires-Dist: extras_dep; extra == 'extra_with_met_deps'
+                Provides-Extra: recursive_extra_with_unmet_deps
+                Requires-Dist: recursive_dep; extra == 'recursive_extra_with_unmet_deps'
+                """
+            ).strip()
 
 
 class RequirelessMockDistribution(MockDistribution):
     def read_text(self, filename):
         if filename == 'METADATA':
-            return """
-Metadata-Version: 2.2
-Name: requireless_dep
-Version: 1.0.0
-""".strip()
+            return textwrap.dedent(
+                """
+                Metadata-Version: 2.2
+                Name: requireless_dep
+                Version: 1.0.0
+                """
+            ).strip()
 
 
 class RecursiveMockDistribution(MockDistribution):
     def read_text(self, filename):
         if filename == 'METADATA':
-            return """
-Metadata-Version: 2.2
-Name: recursive_dep
-Version: 1.0.0
-Requires-Dist: recursive_unmet_dep
-""".strip()
+            return textwrap.dedent(
+                """
+                Metadata-Version: 2.2
+                Name: recursive_dep
+                Version: 1.0.0
+                Requires-Dist: recursive_unmet_dep
+                """
+            ).strip()
 
 
 class PrereleaseMockDistribution(MockDistribution):
     def read_text(self, filename):
         if filename == 'METADATA':
-            return """
-Metadata-Version: 2.2
-Name: prerelease_dep
-Version: 1.0.1a0
-""".strip()
+            return textwrap.dedent(
+                """
+                Metadata-Version: 2.2
+                Name: prerelease_dep
+                Version: 1.0.1a0
+                """
+            ).strip()
+
+
+class CircularMockDistribution(MockDistribution):
+    def read_text(self, filename):
+        if filename == 'METADATA':
+            return textwrap.dedent(
+                """
+                Metadata-Version: 2.2
+                Name: circular_dep
+                Version: 1.0.0
+                Requires-Dist: nested_circular_dep
+                """
+            ).strip()
+
+
+class NestedCircularMockDistribution(MockDistribution):
+    def read_text(self, filename):
+        if filename == 'METADATA':
+            return textwrap.dedent(
+                """
+                Metadata-Version: 2.2
+                Name: nested_circular_dep
+                Version: 1.0.0
+                Requires-Dist: circular_dep
+                """
+            ).strip()
 
 
 @pytest.mark.parametrize(
@@ -107,25 +145,26 @@ Version: 1.0.1a0
         ('extras_dep[extra_without_associated_deps]', None),
         (
             'extras_dep[extra_with_unmet_deps]',
-            ('extras_dep[extra_with_unmet_deps]', "unmet_dep; extra == 'extra_with_unmet_deps'"),
+            ('extras_dep[extra_with_unmet_deps]', 'unmet_dep; extra == "extra_with_unmet_deps"'),
         ),
         (
             'extras_dep[recursive_extra_with_unmet_deps]',
             (
                 'extras_dep[recursive_extra_with_unmet_deps]',
-                "recursive_dep; extra == 'recursive_extra_with_unmet_deps'",
+                'recursive_dep; extra == "recursive_extra_with_unmet_deps"',
                 'recursive_unmet_dep',
             ),
         ),
         ('extras_dep[extra_with_met_deps]', None),
         ('missing_dep; python_version>"10"', None),
         ('missing_dep; python_version<="1"', None),
-        ('missing_dep; python_version>="1"', ('missing_dep; python_version>="1"',)),
+        ('missing_dep; python_version>="1"', ('missing_dep; python_version >= "1"',)),
         ('extras_dep == 1.0.0', None),
-        ('extras_dep == 2.0.0', ('extras_dep == 2.0.0',)),
+        ('extras_dep == 2.0.0', ('extras_dep==2.0.0',)),
         ('extras_dep[extra_without_associated_deps] == 1.0.0', None),
-        ('extras_dep[extra_without_associated_deps] == 2.0.0', ('extras_dep[extra_without_associated_deps] == 2.0.0',)),
+        ('extras_dep[extra_without_associated_deps] == 2.0.0', ('extras_dep[extra_without_associated_deps]==2.0.0',)),
         ('prerelease_dep >= 1.0.0', None),
+        ('circular_dep', None),
     ],
 )
 def test_check_dependency(monkeypatch, requirement_string, expected):
@@ -232,12 +271,12 @@ def test_check_dependencies(mocker, package_test_flit):
     builder._hook.get_requires_for_build_wheel.side_effect = copy.copy(side_effects)
 
     # requires = []
-    assert builder.check_dependencies('sdist') == {('flit_core >=2,<3',)}
-    assert builder.check_dependencies('wheel') == {('flit_core >=2,<3',)}
+    assert builder.check_dependencies('sdist') == {('flit_core<3,>=2',)}
+    assert builder.check_dependencies('wheel') == {('flit_core<3,>=2',)}
 
     # requires = ['something']
-    assert builder.check_dependencies('sdist') == {('flit_core >=2,<3',), ('something',)}
-    assert builder.check_dependencies('wheel') == {('flit_core >=2,<3',), ('something',)}
+    assert builder.check_dependencies('sdist') == {('flit_core<3,>=2',), ('something',)}
+    assert builder.check_dependencies('wheel') == {('flit_core<3,>=2',), ('something',)}
 
     # BackendUnavailable
     with pytest.raises(build.BuildBackendException):
@@ -570,7 +609,7 @@ def test_log(mocker, caplog, package_test_flit):
         ('INFO', 'something'),
     ]
     if sys.version_info >= (3, 8):  # stacklevel
-        assert caplog.records[-1].lineno == 562
+        assert caplog.records[-1].lineno == 601
 
 
 @pytest.mark.parametrize(