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 Mar 26, 2023
1 parent 5bafda8 commit 7c77bf5
Show file tree
Hide file tree
Showing 7 changed files with 209 additions and 18 deletions.
79 changes: 74 additions & 5 deletions src/build/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,12 @@
BuildBackendException,
BuildException,
BuildSystemTableValidationError,
CircularBuildDependencyError,
FailedProcessError,
ProjectTableValidationError,
TypoWarning,
)
from ._util import check_dependency, parse_wheel_filename

from ._util import check_dependency, parse_wheel_filename, project_name_from_path

if sys.version_info >= (3, 11):
import tomllib
Expand Down Expand Up @@ -126,6 +127,23 @@ def _parse_build_system_table(pyproject_toml: Mapping[str, Any]) -> Mapping[str,
return build_system_table


def _parse_project_name(pyproject_toml: Mapping[str, Any]) -> str | None:
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('`project` must have a `name` field')

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

return project_name


def _wrap_subprocess_runner(runner: RunnerType, env: env.IsolatedEnv) -> RunnerType:
def _invoke_wrapped_runner(cmd: Sequence[str], cwd: str | None, extra_environ: Mapping[str, str] | None) -> None:
runner(cmd, cwd, {**(env.make_extra_environ() or {}), **(extra_environ or {})})
Expand Down Expand Up @@ -170,8 +188,10 @@ def __init__(
pyproject_toml_path = os.path.join(source_dir, 'pyproject.toml')
self._build_system = _parse_build_system_table(_read_pyproject_toml(pyproject_toml_path))

self.project_name: str | None = _parse_project_name(_read_pyproject_toml(pyproject_toml_path))
self._backend = self._build_system['build-backend']

self._requires_for_build_cache: dict[str, set[str] | None] = {'wheel': None, 'sdist': None}
self._hook = pyproject_hooks.BuildBackendHookCaller(
self._source_dir,
self._backend,
Expand Down Expand Up @@ -230,6 +250,33 @@ def get_requires_for_build(self, distribution: str, config_settings: ConfigSetti
with self._handle_backend(hook_name):
return set(get_requires(config_settings))

def get_cache_requires_for_build(self, distribution: str, config_settings: ConfigSettingsType | None = None) -> set[str]:
"""
Return the dependencies defined by the backend in addition to
:attr:`build_system_requires` for a given distribution.
:param distribution: Distribution to get the dependencies of
(``sdist`` or ``wheel``)
:param config_settings: Config settings for the build backend
"""
requires_for_build: set[str]
requires_for_build_cache: set[str] | None = self._requires_for_build_cache[distribution]
if requires_for_build_cache is not None:
requires_for_build = requires_for_build_cache
else:
requires_for_build = self.get_requires_for_build(distribution, config_settings)
self._requires_for_build_cache[distribution] = requires_for_build
return requires_for_build

def check_build_system_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: ConfigSettingsType | None = None) -> set[tuple[str, ...]]:
"""
Return the dependencies which are not satisfied from the combined set of
Expand All @@ -240,8 +287,20 @@ def check_dependencies(self, distribution: str, config_settings: ConfigSettingsT
: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_system_dependencies()
requires_for_build: set[str]
requires_for_build_cache: set[str] | None = self._requires_for_build_cache[distribution]
if requires_for_build_cache is not None:
requires_for_build = requires_for_build_cache
else:
requires_for_build = self.get_requires_for_build(distribution, config_settings)
# cache if build system dependencies are fully satisfied
if len(build_system_dependencies) == 0:
self._requires_for_build_cache[distribution] = requires_for_build
dependencies = {
u for d in requires_for_build for u in check_dependency(d, project_name=self.project_name, backend=self._backend)
}
return dependencies.union(build_system_dependencies)

def prepare(
self, distribution: str, output_directory: PathType, config_settings: ConfigSettingsType | None = None
Expand Down Expand Up @@ -286,7 +345,11 @@ def build(
"""
self.log(f'Building {distribution}...')
kwargs = {} if metadata_directory is None else {'metadata_directory': metadata_directory}
return self._call_backend(f'build_{distribution}', output_directory, config_settings, **kwargs)
basename = self._call_backend(f'build_{distribution}', output_directory, config_settings, **kwargs)
project_name = project_name_from_path(basename, distribution)
if project_name:
self.project_name = project_name
return basename

def metadata_path(self, output_directory: PathType) -> str:
"""
Expand All @@ -301,13 +364,17 @@ def metadata_path(self, output_directory: PathType) -> str:
# prepare_metadata hook
metadata = self.prepare('wheel', output_directory)
if metadata is not None:
project_name = project_name_from_path(metadata, 'wheel')
if project_name:
self.project_name = project_name
return metadata

# fallback to build_wheel hook
wheel = self.build('wheel', output_directory)
match = parse_wheel_filename(os.path.basename(wheel))
if not match:
raise ValueError('Invalid wheel')
self.project_name = match['distribution']
distinfo = f"{match['distribution']}-{match['version']}.dist-info"
member_prefix = f'{distinfo}/'
with zipfile.ZipFile(wheel) as w:
Expand Down Expand Up @@ -373,9 +440,11 @@ def log(message: str) -> None:
'BuildSystemTableValidationError',
'BuildBackendException',
'BuildException',
'CircularBuildDependencyError',
'ConfigSettingsType',
'FailedProcessError',
'ProjectBuilder',
'ProjectTableValidationError',
'RunnerType',
'TypoWarning',
'check_dependency',
Expand Down
31 changes: 26 additions & 5 deletions src/build/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,15 +105,28 @@ def _format_dep_chain(dep_chain: Sequence[str]) -> str:


def _build_in_isolated_env(
srcdir: PathType, outdir: PathType, distribution: str, config_settings: ConfigSettingsType | None
srcdir: PathType,
outdir: PathType,
distribution: str,
config_settings: ConfigSettingsType | None,
skip_dependency_check: bool = False,
) -> str:
with _DefaultIsolatedEnv() as env:
builder = _ProjectBuilder.from_isolated_env(env, srcdir)
# first install the build dependencies
env.install(builder.build_system_requires)
# validate build system dependencies
revalidate = False
if not skip_dependency_check:
builder.check_dependencies(distribution)
if builder.project_name is None:
revalidate = True
# 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 {})
env.install(builder.get_cache_requires_for_build(distribution))
build_result = builder.build(distribution, outdir, config_settings or {})
if revalidate and builder.project_name is not None:
builder.check_dependencies(distribution)
return build_result


def _build_in_current_env(
Expand All @@ -125,14 +138,22 @@ def _build_in_current_env(
) -> str:
builder = _ProjectBuilder(srcdir)

revalidate = False
if not skip_dependency_check:
missing = builder.check_dependencies(distribution)
if missing:
dependencies = ''.join('\n\t' + dep for deps in missing for dep in (deps[0], _format_dep_chain(deps[1:])) if dep)
_cprint()
_error(f'Missing dependencies:{dependencies}')
elif builder.project_name is None:
revalidate = True

build_result = builder.build(distribution, outdir, config_settings or {})

if revalidate and builder.project_name is not None:
builder.check_dependencies(distribution)

return builder.build(distribution, outdir, config_settings or {})
return build_result


def _build(
Expand All @@ -144,7 +165,7 @@ def _build(
skip_dependency_check: bool,
) -> str:
if isolation:
return _build_in_isolated_env(srcdir, outdir, distribution, config_settings)
return _build_in_isolated_env(srcdir, outdir, distribution, config_settings, skip_dependency_check)
else:
return _build_in_current_env(srcdir, outdir, distribution, config_settings, skip_dependency_check)

Expand Down
33 changes: 33 additions & 0 deletions src/build/_exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,15 @@ def __str__(self) -> str:
return f'Failed to validate `build-system` in pyproject.toml: {self.args[0]}'


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 FailedProcessError(Exception):
"""
Exception raised when a setup or preparation operation fails.
Expand All @@ -64,6 +73,30 @@ def __str__(self) -> str:
return description


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

def __init__(
self, project_name: str, ancestral_req_strings: tuple[str, ...], req_string: str, backend: str | None
) -> None:
super().__init__()
self.project_name: str = project_name
self.ancestral_req_strings: tuple[str, ...] = ancestral_req_strings
self.req_string: str = req_string
self.backend: str | None = backend

def __str__(self) -> str:
cycle_err_str = f'`{self.project_name}`'
if self.backend:
cycle_err_str += f' -> `{self.backend}`'
for dep in self.ancestral_req_strings:
cycle_err_str += f' -> `{dep}`'
cycle_err_str += f' -> `{self.req_string}`'
return f'Failed to validate `build-system` in pyproject.toml, dependency cycle detected: {cycle_err_str}'


class TypoWarning(Warning):
"""
Warning raised when a possible typo is found.
Expand Down
37 changes: 34 additions & 3 deletions src/build/_util.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
from __future__ import annotations

import os
import re
import sys

from collections.abc import Iterator, Set

from ._exceptions import CircularBuildDependencyError

_SDIST_FILENAME_REGEX = re.compile(r'(?P<distribution>.+)-(?P<version>.+)\.tar.gz')


_WHEEL_FILENAME_REGEX = re.compile(
r'(?P<distribution>.+)-(?P<version>.+)'
Expand All @@ -13,8 +18,23 @@
)


def check_dependency(
req_string: str, ancestral_req_strings: tuple[str, ...] = (), parent_extras: Set[str] = frozenset()
def project_name_from_path(basename: str, distribution: str) -> str | None:
match = None
if distribution == 'wheel':
match = _WHEEL_FILENAME_REGEX.match(os.path.basename(basename))
elif distribution == 'sdist':
match = _SDIST_FILENAME_REGEX.match(os.path.basename(basename))
if match:
return match['distribution']
return None


def check_dependency( # noqa: C901
req_string: str,
ancestral_req_strings: tuple[str, ...] = (),
parent_extras: Set[str] = frozenset(),
project_name: str | None = None,
backend: str | None = None,
) -> Iterator[tuple[str, ...]]:
"""
Verify that a dependency and all of its dependencies are met.
Expand All @@ -24,6 +44,7 @@ def check_dependency(
:yields: Unmet dependencies
"""
import packaging.requirements
import packaging.utils

if sys.version_info >= (3, 8):
import importlib.metadata as importlib_metadata
Expand All @@ -48,6 +69,14 @@ def check_dependency(
# dependency is satisfied.
return

# 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 packaging.utils.canonicalize_name(req.name) == packaging.utils.canonicalize_name(
project_name
):
raise CircularBuildDependencyError(project_name, ancestral_req_strings, req_string, backend)

try:
dist = importlib_metadata.distribution(req.name) # type: ignore[no-untyped-call]
except importlib_metadata.PackageNotFoundError:
Expand All @@ -60,7 +89,9 @@ 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, normalised_req_string), req.extras)
yield from check_dependency(
other_req_string, (*ancestral_req_strings, normalised_req_string), req.extras, project_name
)


def parse_wheel_filename(filename: str) -> re.Match[str] | None:
Expand Down
7 changes: 7 additions & 0 deletions tests/packages/test-circular-requirements/pyproject.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[build-system]
requires = ["recursive_dep"]

[project]
name = "recursive_unmet_dep"
version = "1.0.0"
description = "circular project"
4 changes: 2 additions & 2 deletions tests/test_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,11 +131,11 @@ def test_build_isolated(mocker, package_test_flit):
mocker.patch('build.__main__._error')
install = mocker.patch('build.env.DefaultIsolatedEnv.install')

build.__main__.build_package(package_test_flit, '.', ['sdist'])
build.__main__.build_package(package_test_flit, '.', ['sdist'], skip_dependency_check=True)

install.assert_any_call({'flit_core >=2,<3'})

required_cmd.assert_called_with('sdist')
required_cmd.assert_called_with('sdist', None)
install.assert_any_call(['dep1', 'dep2'])

build_cmd.assert_called_with('sdist', '.', {})
Expand Down
Loading

0 comments on commit 7c77bf5

Please sign in to comment.