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 Jan 10, 2022
1 parent 96f9188 commit 14c2d8a
Show file tree
Hide file tree
Showing 4 changed files with 145 additions and 12 deletions.
104 changes: 99 additions & 5 deletions src/build/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@
_ExcInfoType = Union[Tuple[Type[BaseException], BaseException, types.TracebackType], Tuple[None, None, None]]


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


_WHEEL_NAME_REGEX = re.compile(
r'(?P<distribution>.+)-(?P<version>.+)'
r'(-(?P<build_tag>.+))?-(?P<python_tag>.+)'
Expand Down Expand Up @@ -104,6 +107,30 @@ 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 __init__(
self, project_name: str, ancestral_req_strings: Tuple[str, ...], req_string: str, backend: Optional[str]
) -> 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: Optional[str] = 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 potential typo is found
Expand Down Expand Up @@ -131,8 +158,17 @@ def _validate_source_directory(srcdir: PathType) -> None:
raise BuildException(f'Source {srcdir} does not appear to be a Python project: no pyproject.toml or setup.py')


# https://www.python.org/dev/peps/pep-0503/#normalized-names
def _normalize(name: str) -> str:
return re.sub(r'[-_.]+', '-', name).lower()


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,
backend: Optional[str] = None,
) -> Iterator[Tuple[str, ...]]:
"""
Verify that a dependency and all of its dependencies are met.
Expand All @@ -159,6 +195,12 @@ 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 _normalize(req.name) == _normalize(project_name):
raise CircularBuildSystemDependencyError(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 @@ -171,7 +213,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 +264,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:
return None

project_name = project_table['name']
if not isinstance(project_name, str):
return None

return project_name


class ProjectBuilder:
"""
The PEP 517 consumer API.
Expand Down Expand Up @@ -267,8 +326,10 @@ def __init__(
except TOMLDecodeError as e:
raise BuildException(f'Failed to parse {spec_file}: {e} ')

self.project_name: Optional[str] = _parse_project_name(spec)
self._build_system = _parse_build_system_table(spec)
self._backend = self._build_system['build-backend']
self._requires_for_build_cache: dict[str, Optional[Set[str]]] = {'wheel': None, 'sdist': None}
self._scripts_dir = scripts_dir
self._hook_runner = runner
self._hook = pep517.wrappers.Pep517HookCaller(
Expand Down Expand Up @@ -341,6 +402,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 +423,20 @@ 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()
requires_for_build: Set[str]
requires_for_build_cache: Optional[Set[str]] = 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)
dependencies = {
u for d in requires_for_build for u in check_dependency(d, project_name=self.project_name, backend=self._backend)
}
unmet_dependencies = dependencies.union(build_system_dependencies)
if len(unmet_dependencies) == 0:
self._requires_for_build_cache[distribution] = requires_for_build
return unmet_dependencies

def prepare(
self, distribution: str, output_directory: PathType, config_settings: Optional[ConfigSettingsType] = None
Expand Down Expand Up @@ -399,7 +481,15 @@ 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)
match = None
if distribution == 'wheel':
match = _WHEEL_NAME_REGEX.match(os.path.basename(basename))
elif distribution == 'sdist':
match = _SDIST_NAME_REGEX.match(os.path.basename(basename))
if match:
self.project_name = match['distribution']
return basename

def metadata_path(self, output_directory: PathType) -> str:
"""
Expand All @@ -413,13 +503,17 @@ def metadata_path(self, output_directory: PathType) -> str:
# prepare_metadata hook
metadata = self.prepare('wheel', output_directory)
if metadata is not None:
match = _WHEEL_NAME_REGEX.match(os.path.basename(metadata))
if match:
self.project_name = match['distribution']
return metadata

# fallback to build_wheel hook
wheel = self.build('wheel', output_directory)
match = _WHEEL_NAME_REGEX.match(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
29 changes: 25 additions & 4 deletions src/build/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,16 +99,29 @@ 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
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 {})
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 @@ -118,14 +131,22 @@ def _build_in_current_env(
config_settings: Optional[ConfigSettingsType],
skip_dependency_check: bool = False,
) -> str:
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)
print()
_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 @@ -137,7 +158,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
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"
17 changes: 14 additions & 3 deletions tests/test_projectbuilder.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import importlib
import logging
import os
import pathlib
import sys
import textwrap

Expand All @@ -19,8 +20,6 @@
else: # pragma: no cover
import importlib_metadata

import pathlib


build_open_owner = 'builtins'

Expand Down Expand Up @@ -133,6 +132,18 @@ def test_check_dependency(monkeypatch, requirement_string, expected):
assert next(build.check_dependency(requirement_string), None) == expected


@pytest.mark.parametrize('distribution', ['wheel', 'sdist'])
def test_build_no_isolation_circular_requirements(monkeypatch, package_test_circular_requirements, distribution):
monkeypatch.setattr(importlib_metadata, 'Distribution', MockDistribution)
msg = (
'Failed to validate `build-system` in pyproject.toml, dependency cycle detected: `recursive_unmet_dep` -> '
'`recursive_dep` -> `recursive_unmet_dep`'
)
builder = build.ProjectBuilder(package_test_circular_requirements)
with pytest.raises(build.CircularBuildSystemDependencyError, match=msg):
builder.check_dependencies(distribution)


def test_bad_project(package_test_no_project):
# Passing a nonexistent project directory
with pytest.raises(build.BuildException):
Expand Down Expand Up @@ -570,7 +581,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 == test_log.__code__.co_firstlineno + 11


@pytest.mark.parametrize(
Expand Down

0 comments on commit 14c2d8a

Please sign in to comment.