Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: add circular dependency protection #512

Merged
merged 5 commits into from Sep 3, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 10 additions & 0 deletions CHANGELOG.rst
Expand Up @@ -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)
==================

Expand Down
13 changes: 10 additions & 3 deletions src/build/__init__.py
Expand Up @@ -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)
Expand All @@ -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:
Expand Down
115 changes: 77 additions & 38 deletions tests/test_projectbuilder.py
Expand Up @@ -45,55 +45,93 @@ def from_name(cls, name):
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(
Expand All @@ -107,25 +145,26 @@ def read_text(self, filename):
('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):
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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(
Expand Down