Skip to content

Commit

Permalink
build: add circular dependency checker for build requirements
Browse files Browse the repository at this point in the history
Implement a basic build requirement cycle detector per PEP-517:

- Project build requirements will define a directed graph of
requirements (project A needs B to build, B needs C and D, etc.)
This graph MUST NOT contain cycles. If (due to lack of co-ordination
between projects, for example) a cycle is present, front ends MAY
refuse to build the project.

- Front ends SHOULD check explicitly for requirement cycles, and
terminate the build with an informative message if one is found.

See:
https://www.python.org/dev/peps/pep-0517/#build-requirements
  • Loading branch information
jameshilliard committed Dec 12, 2021
1 parent cb91293 commit 4ec3f30
Show file tree
Hide file tree
Showing 7 changed files with 98 additions and 7 deletions.
67 changes: 63 additions & 4 deletions src/build/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,28 @@ def __str__(self) -> str:
return f'Failed to validate `build-system` in pyproject.toml: {self.args[0]}'


class CircularBuildSystemDependencyError(BuildException):
"""
Exception raised when a ``[build-system]`` requirement in pyproject.toml is circular.
"""

def __str__(self) -> str:
cycle_deps = self.args[0]
cycle_err_str = f'`{cycle_deps[0]}`'
for dep in cycle_deps[1:]:
cycle_err_str += f' -> `{dep}`'
return f'Failed to validate `build-system` in pyproject.toml, dependency cycle detected: {cycle_err_str}'


class ProjectTableValidationError(BuildException):
"""
Exception raised when the ``[project]`` table in pyproject.toml is invalid.
"""

def __str__(self) -> str:
return f'Failed to validate `project` in pyproject.toml: {self.args[0]}'


class TypoWarning(Warning):
"""
Warning raised when a potential typo is found
Expand Down Expand Up @@ -132,7 +154,10 @@ def _validate_source_directory(srcdir: PathType) -> None:


def check_dependency(
req_string: str, ancestral_req_strings: Tuple[str, ...] = (), parent_extras: AbstractSet[str] = frozenset()
req_string: str,
ancestral_req_strings: Tuple[str, ...] = (),
parent_extras: AbstractSet[str] = frozenset(),
project_name: Optional[str] = None,
) -> Iterator[Tuple[str, ...]]:
"""
Verify that a dependency and all of its dependencies are met.
Expand All @@ -150,6 +175,12 @@ def check_dependency(

req = packaging.requirements.Requirement(req_string)

# Front ends SHOULD check explicitly for requirement cycles, and
# terminate the build with an informative message if one is found.
# https://www.python.org/dev/peps/pep-0517/#build-requirements
if project_name is not None and req.name == project_name:
raise CircularBuildSystemDependencyError(ancestral_req_strings + (req_string,))

if req.marker:
extras = frozenset(('',)).union(parent_extras)
# a requirement can have multiple extras but ``evaluate`` can
Expand All @@ -171,7 +202,7 @@ def check_dependency(
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 + (req_string,), req.extras, project_name)


def _find_typo(dictionary: Mapping[str, str], expected: str) -> None:
Expand Down Expand Up @@ -222,6 +253,23 @@ def _parse_build_system_table(pyproject_toml: Mapping[str, Any]) -> Dict[str, An
return build_system_table


def _parse_project_name(pyproject_toml: Mapping[str, Any]) -> Optional[str]:
if 'project' not in pyproject_toml:
return None

project_table = dict(pyproject_toml['project'])

# If [project] is present, it must have a ``name`` field (per PEP 621)
if 'name' not in project_table:
raise ProjectTableValidationError('`name` is a required property')

project_name = project_table['name']
if not isinstance(project_name, str):
raise ProjectTableValidationError('`name` must be a string')

return project_name


class ProjectBuilder:
"""
The PEP 517 consumer API.
Expand Down Expand Up @@ -268,6 +316,7 @@ def __init__(
raise BuildException(f'Failed to parse {spec_file}: {e} ')

self._build_system = _parse_build_system_table(spec)
self._project_name = _parse_project_name(spec)
self._backend = self._build_system['build-backend']
self._scripts_dir = scripts_dir
self._hook_runner = runner
Expand Down Expand Up @@ -341,6 +390,15 @@ def get_requires_for_build(self, distribution: str, config_settings: Optional[Co
with self._handle_backend(hook_name):
return set(get_requires(config_settings))

def check_build_dependencies(self) -> Set[Tuple[str, ...]]:
"""
Return the dependencies which are not satisfied from
:attr:`build_system_requires`
:returns: Set of variable-length unmet dependency tuples
"""
return {u for d in self.build_system_requires for u in check_dependency(d, project_name=self._project_name)}

def check_dependencies(
self, distribution: str, config_settings: Optional[ConfigSettingsType] = None
) -> Set[Tuple[str, ...]]:
Expand All @@ -353,8 +411,9 @@ def check_dependencies(
:param config_settings: Config settings for the build backend
:returns: Set of variable-length unmet dependency tuples
"""
dependencies = self.get_requires_for_build(distribution, config_settings).union(self.build_system_requires)
return {u for d in dependencies for u in check_dependency(d)}
build_system_dependencies = self.check_build_dependencies()
dependencies = {u for d in self.get_requires_for_build(distribution, config_settings) for u in check_dependency(d)}
return dependencies.union(build_system_dependencies)

def prepare(
self, distribution: str, output_directory: PathType, config_settings: Optional[ConfigSettingsType] = None
Expand Down
11 changes: 9 additions & 2 deletions src/build/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,13 +99,20 @@ def _format_dep_chain(dep_chain: Sequence[str]) -> str:


def _build_in_isolated_env(
builder: ProjectBuilder, outdir: PathType, distribution: str, config_settings: Optional[ConfigSettingsType]
builder: ProjectBuilder,
outdir: PathType,
distribution: str,
config_settings: Optional[ConfigSettingsType],
skip_dependency_check: bool = False,
) -> str:
with _IsolatedEnvBuilder() as env:
builder.python_executable = env.executable
builder.scripts_dir = env.scripts_dir
# first install the build dependencies
env.install(builder.build_system_requires)
# validate build system dependencies
if not skip_dependency_check:
builder.check_build_dependencies()
# then get the extra required dependencies from the backend (which was installed in the call above :P)
env.install(builder.get_requires_for_build(distribution))
return builder.build(distribution, outdir, config_settings or {})
Expand Down Expand Up @@ -137,7 +144,7 @@ def _build(
skip_dependency_check: bool,
) -> str:
if isolation:
return _build_in_isolated_env(builder, outdir, distribution, config_settings)
return _build_in_isolated_env(builder, outdir, distribution, config_settings, skip_dependency_check)
else:
return _build_in_current_env(builder, outdir, distribution, config_settings, skip_dependency_check)

Expand Down
8 changes: 8 additions & 0 deletions tests/packages/test-circular-requirements/pyproject.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[build-system]
requires = ["flit_core==3.5.1"]
build-backend = "flit_core.buildapi"

[project]
name = "tomli"
version = "1.2.2"
description = "A lil' TOML parser"
1 change: 1 addition & 0 deletions tests/requirements.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
flit_core==3.5.1
2 changes: 2 additions & 0 deletions tests/test_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,8 @@ def test_build(monkeypatch, project, args, call, tmp_path):


def test_isolation(tmp_dir, package_test_flit, mocker):
if 'flit_core ' in sys.modules:
del sys.modules["flit_core"]
try:
import flit_core # noqa: F401
except ModuleNotFoundError:
Expand Down
14 changes: 14 additions & 0 deletions tests/test_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,20 @@ def test_build_no_isolation_check_deps_empty(mocker, package_test_flit):
build_cmd.assert_called_with('sdist', '.', {})


@pytest.mark.parametrize('distribution', ['wheel', 'sdist'])
def test_build_no_isolation_circular_requirements(package_test_circular_requirements, distribution):
msg = 'Failed to validate `build-system` in pyproject.toml, dependency cycle detected: `flit_core==3.5.1` -> `tomli`'
with pytest.raises(build.CircularBuildSystemDependencyError, match=msg):
build.__main__.build_package(package_test_circular_requirements, '.', [distribution], isolation=False)


@pytest.mark.parametrize('distribution', ['wheel', 'sdist'])
def test_build_circular_requirements(package_test_circular_requirements, distribution):
msg = 'Failed to validate `build-system` in pyproject.toml, dependency cycle detected: `flit_core==3.5.1` -> `tomli`'
with pytest.raises(build.CircularBuildSystemDependencyError, match=msg):
build.__main__.build_package(package_test_circular_requirements, '.', [distribution])


@pytest.mark.parametrize(
['missing_deps', 'output'],
[
Expand Down
2 changes: 1 addition & 1 deletion tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ commands =
description = check minimum versions required of all dependencies
skip_install = true
commands =
pip install .[test] -c tests/constraints.txt
pip install .[test] -c tests/constraints.txt -r tests/requirements.txt
pytest -rsx tests {posargs:-n auto}

[testenv:docs]
Expand Down

0 comments on commit 4ec3f30

Please sign in to comment.