Skip to content

Commit 9dc2175

Browse files
committed
build: add circular dependency checker for build requirements
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
1 parent 5bafda8 commit 9dc2175

File tree

7 files changed

+208
-18
lines changed

7 files changed

+208
-18
lines changed

Diff for: src/build/__init__.py

+74-5
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,12 @@
2828
BuildBackendException,
2929
BuildException,
3030
BuildSystemTableValidationError,
31+
CircularBuildDependencyError,
3132
FailedProcessError,
33+
ProjectTableValidationError,
3234
TypoWarning,
3335
)
34-
from ._util import check_dependency, parse_wheel_filename
35-
36+
from ._util import check_dependency, parse_wheel_filename, project_name_from_path
3637

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

128129

130+
def _parse_project_name(pyproject_toml: Mapping[str, Any]) -> str | None:
131+
if 'project' not in pyproject_toml:
132+
return None
133+
134+
project_table = dict(pyproject_toml['project'])
135+
136+
# If [project] is present, it must have a ``name`` field (per PEP 621)
137+
if 'name' not in project_table:
138+
raise ProjectTableValidationError('`project` must have a `name` field')
139+
140+
project_name = project_table['name']
141+
if not isinstance(project_name, str):
142+
raise ProjectTableValidationError('`name` field in `project` must be a string')
143+
144+
return project_name
145+
146+
129147
def _wrap_subprocess_runner(runner: RunnerType, env: env.IsolatedEnv) -> RunnerType:
130148
def _invoke_wrapped_runner(cmd: Sequence[str], cwd: str | None, extra_environ: Mapping[str, str] | None) -> None:
131149
runner(cmd, cwd, {**(env.make_extra_environ() or {}), **(extra_environ or {})})
@@ -170,8 +188,10 @@ def __init__(
170188
pyproject_toml_path = os.path.join(source_dir, 'pyproject.toml')
171189
self._build_system = _parse_build_system_table(_read_pyproject_toml(pyproject_toml_path))
172190

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

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

253+
def get_cache_requires_for_build(self, distribution: str, config_settings: ConfigSettingsType | None = None) -> set[str]:
254+
"""
255+
Return the dependencies defined by the backend in addition to
256+
:attr:`build_system_requires` for a given distribution.
257+
258+
:param distribution: Distribution to get the dependencies of
259+
(``sdist`` or ``wheel``)
260+
:param config_settings: Config settings for the build backend
261+
"""
262+
requires_for_build: set[str]
263+
requires_for_build_cache: set[str] | None = self._requires_for_build_cache[distribution]
264+
if requires_for_build_cache is not None:
265+
requires_for_build = requires_for_build_cache
266+
else:
267+
requires_for_build = self.get_requires_for_build(distribution, config_settings)
268+
self._requires_for_build_cache[distribution] = requires_for_build
269+
return requires_for_build
270+
271+
def check_build_system_dependencies(self) -> set[tuple[str, ...]]:
272+
"""
273+
Return the dependencies which are not satisfied from
274+
:attr:`build_system_requires`
275+
276+
:returns: Set of variable-length unmet dependency tuples
277+
"""
278+
return {u for d in self.build_system_requires for u in check_dependency(d, project_name=self.project_name)}
279+
233280
def check_dependencies(self, distribution: str, config_settings: ConfigSettingsType | None = None) -> set[tuple[str, ...]]:
234281
"""
235282
Return the dependencies which are not satisfied from the combined set of
@@ -240,8 +287,20 @@ def check_dependencies(self, distribution: str, config_settings: ConfigSettingsT
240287
:param config_settings: Config settings for the build backend
241288
:returns: Set of variable-length unmet dependency tuples
242289
"""
243-
dependencies = self.get_requires_for_build(distribution, config_settings).union(self.build_system_requires)
244-
return {u for d in dependencies for u in check_dependency(d)}
290+
build_system_dependencies = self.check_build_system_dependencies()
291+
requires_for_build: set[str]
292+
requires_for_build_cache: set[str] | None = self._requires_for_build_cache[distribution]
293+
if requires_for_build_cache is not None:
294+
requires_for_build = requires_for_build_cache
295+
else:
296+
requires_for_build = self.get_requires_for_build(distribution, config_settings)
297+
# cache if build system dependencies are fully satisfied
298+
if len(build_system_dependencies) == 0:
299+
self._requires_for_build_cache[distribution] = requires_for_build
300+
dependencies = {
301+
u for d in requires_for_build for u in check_dependency(d, project_name=self.project_name, backend=self._backend)
302+
}
303+
return dependencies.union(build_system_dependencies)
245304

246305
def prepare(
247306
self, distribution: str, output_directory: PathType, config_settings: ConfigSettingsType | None = None
@@ -286,7 +345,11 @@ def build(
286345
"""
287346
self.log(f'Building {distribution}...')
288347
kwargs = {} if metadata_directory is None else {'metadata_directory': metadata_directory}
289-
return self._call_backend(f'build_{distribution}', output_directory, config_settings, **kwargs)
348+
basename = self._call_backend(f'build_{distribution}', output_directory, config_settings, **kwargs)
349+
project_name = project_name_from_path(basename, distribution)
350+
if project_name:
351+
self.project_name = project_name
352+
return basename
290353

291354
def metadata_path(self, output_directory: PathType) -> str:
292355
"""
@@ -301,13 +364,17 @@ def metadata_path(self, output_directory: PathType) -> str:
301364
# prepare_metadata hook
302365
metadata = self.prepare('wheel', output_directory)
303366
if metadata is not None:
367+
project_name = project_name_from_path(metadata, 'wheel')
368+
if project_name:
369+
self.project_name = project_name
304370
return metadata
305371

306372
# fallback to build_wheel hook
307373
wheel = self.build('wheel', output_directory)
308374
match = parse_wheel_filename(os.path.basename(wheel))
309375
if not match:
310376
raise ValueError('Invalid wheel')
377+
self.project_name = match['distribution']
311378
distinfo = f"{match['distribution']}-{match['version']}.dist-info"
312379
member_prefix = f'{distinfo}/'
313380
with zipfile.ZipFile(wheel) as w:
@@ -373,9 +440,11 @@ def log(message: str) -> None:
373440
'BuildSystemTableValidationError',
374441
'BuildBackendException',
375442
'BuildException',
443+
'CircularBuildDependencyError',
376444
'ConfigSettingsType',
377445
'FailedProcessError',
378446
'ProjectBuilder',
447+
'ProjectTableValidationError',
379448
'RunnerType',
380449
'TypoWarning',
381450
'check_dependency',

Diff for: src/build/__main__.py

+26-5
Original file line numberDiff line numberDiff line change
@@ -105,15 +105,28 @@ def _format_dep_chain(dep_chain: Sequence[str]) -> str:
105105

106106

107107
def _build_in_isolated_env(
108-
srcdir: PathType, outdir: PathType, distribution: str, config_settings: ConfigSettingsType | None
108+
srcdir: PathType,
109+
outdir: PathType,
110+
distribution: str,
111+
config_settings: ConfigSettingsType | None,
112+
skip_dependency_check: bool = False,
109113
) -> str:
110114
with _DefaultIsolatedEnv() as env:
111115
builder = _ProjectBuilder.from_isolated_env(env, srcdir)
112116
# first install the build dependencies
113117
env.install(builder.build_system_requires)
118+
# validate build system dependencies
119+
revalidate = False
120+
if not skip_dependency_check:
121+
builder.check_dependencies(distribution)
122+
if builder.project_name is None:
123+
revalidate = True
114124
# then get the extra required dependencies from the backend (which was installed in the call above :P)
115-
env.install(builder.get_requires_for_build(distribution))
116-
return builder.build(distribution, outdir, config_settings or {})
125+
env.install(builder.get_cache_requires_for_build(distribution))
126+
build_result = builder.build(distribution, outdir, config_settings or {})
127+
if revalidate and builder.project_name is not None:
128+
builder.check_dependencies(distribution)
129+
return build_result
117130

118131

119132
def _build_in_current_env(
@@ -125,14 +138,22 @@ def _build_in_current_env(
125138
) -> str:
126139
builder = _ProjectBuilder(srcdir)
127140

141+
revalidate = False
128142
if not skip_dependency_check:
129143
missing = builder.check_dependencies(distribution)
130144
if missing:
131145
dependencies = ''.join('\n\t' + dep for deps in missing for dep in (deps[0], _format_dep_chain(deps[1:])) if dep)
132146
_cprint()
133147
_error(f'Missing dependencies:{dependencies}')
148+
elif builder.project_name is None:
149+
revalidate = True
150+
151+
build_result = builder.build(distribution, outdir, config_settings or {})
152+
153+
if revalidate and builder.project_name is not None:
154+
builder.check_dependencies(distribution)
134155

135-
return builder.build(distribution, outdir, config_settings or {})
156+
return build_result
136157

137158

138159
def _build(
@@ -144,7 +165,7 @@ def _build(
144165
skip_dependency_check: bool,
145166
) -> str:
146167
if isolation:
147-
return _build_in_isolated_env(srcdir, outdir, distribution, config_settings)
168+
return _build_in_isolated_env(srcdir, outdir, distribution, config_settings, skip_dependency_check)
148169
else:
149170
return _build_in_current_env(srcdir, outdir, distribution, config_settings, skip_dependency_check)
150171

Diff for: src/build/_exceptions.py

+33
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,15 @@ def __str__(self) -> str:
4343
return f'Failed to validate `build-system` in pyproject.toml: {self.args[0]}'
4444

4545

46+
class ProjectTableValidationError(BuildException):
47+
"""
48+
Exception raised when the ``[project]`` table in pyproject.toml is invalid.
49+
"""
50+
51+
def __str__(self) -> str:
52+
return f'Failed to validate `project` in pyproject.toml: {self.args[0]}'
53+
54+
4655
class FailedProcessError(Exception):
4756
"""
4857
Exception raised when a setup or preparation operation fails.
@@ -64,6 +73,30 @@ def __str__(self) -> str:
6473
return description
6574

6675

76+
class CircularBuildDependencyError(BuildException):
77+
"""
78+
Exception raised when a ``[build-system]`` requirement in pyproject.toml is circular.
79+
"""
80+
81+
def __init__(
82+
self, project_name: str, ancestral_req_strings: tuple[str, ...], req_string: str, backend: str | None
83+
) -> None:
84+
super().__init__()
85+
self.project_name: str = project_name
86+
self.ancestral_req_strings: tuple[str, ...] = ancestral_req_strings
87+
self.req_string: str = req_string
88+
self.backend: str | None = backend
89+
90+
def __str__(self) -> str:
91+
cycle_err_str = f'`{self.project_name}`'
92+
if self.backend:
93+
cycle_err_str += f' -> `{self.backend}`'
94+
for dep in self.ancestral_req_strings:
95+
cycle_err_str += f' -> `{dep}`'
96+
cycle_err_str += f' -> `{self.req_string}`'
97+
return f'Failed to validate `build-system` in pyproject.toml, dependency cycle detected: {cycle_err_str}'
98+
99+
67100
class TypoWarning(Warning):
68101
"""
69102
Warning raised when a possible typo is found.

Diff for: src/build/_util.py

+34-3
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,15 @@
11
from __future__ import annotations
22

3+
import os
34
import re
45
import sys
56

67
from collections.abc import Iterator, Set
78

9+
from ._exceptions import CircularBuildDependencyError
10+
11+
_SDIST_FILENAME_REGEX = re.compile(r'(?P<distribution>.+)-(?P<version>.+)\.tar.gz')
12+
813

914
_WHEEL_FILENAME_REGEX = re.compile(
1015
r'(?P<distribution>.+)-(?P<version>.+)'
@@ -13,8 +18,23 @@
1318
)
1419

1520

16-
def check_dependency(
17-
req_string: str, ancestral_req_strings: tuple[str, ...] = (), parent_extras: Set[str] = frozenset()
21+
def project_name_from_path(basename: str, distribution: str) -> str | None:
22+
match = None
23+
if distribution == 'wheel':
24+
match = _WHEEL_FILENAME_REGEX.match(os.path.basename(basename))
25+
elif distribution == 'sdist':
26+
match = _SDIST_FILENAME_REGEX.match(os.path.basename(basename))
27+
if match:
28+
return match['distribution']
29+
return None
30+
31+
32+
def check_dependency( # noqa: C901
33+
req_string: str,
34+
ancestral_req_strings: tuple[str, ...] = (),
35+
parent_extras: Set[str] = frozenset(),
36+
project_name: str | None = None,
37+
backend: str | None = None,
1838
) -> Iterator[tuple[str, ...]]:
1939
"""
2040
Verify that a dependency and all of its dependencies are met.
@@ -24,6 +44,7 @@ def check_dependency(
2444
:yields: Unmet dependencies
2545
"""
2646
import packaging.requirements
47+
import packaging.utils
2748

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

72+
# Front ends SHOULD check explicitly for requirement cycles, and
73+
# terminate the build with an informative message if one is found.
74+
# https://www.python.org/dev/peps/pep-0517/#build-requirements
75+
if project_name is not None and packaging.utils.canonicalize_name(req.name) == packaging.utils.canonicalize_name(
76+
project_name
77+
):
78+
raise CircularBuildDependencyError(project_name, ancestral_req_strings, req_string, backend)
79+
5180
try:
5281
dist = importlib_metadata.distribution(req.name) # type: ignore[no-untyped-call]
5382
except importlib_metadata.PackageNotFoundError:
@@ -60,7 +89,9 @@ def check_dependency(
6089
elif dist.requires:
6190
for other_req_string in dist.requires:
6291
# yields transitive dependencies that are not satisfied.
63-
yield from check_dependency(other_req_string, (*ancestral_req_strings, normalised_req_string), req.extras)
92+
yield from check_dependency(
93+
other_req_string, (*ancestral_req_strings, normalised_req_string), req.extras, project_name
94+
)
6495

6596

6697
def parse_wheel_filename(filename: str) -> re.Match[str] | None:
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
[build-system]
2+
requires = ["recursive_dep"]
3+
4+
[project]
5+
name = "recursive_unmet_dep"
6+
version = "1.0.0"
7+
description = "circular project"

Diff for: tests/test_main.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -131,11 +131,11 @@ def test_build_isolated(mocker, package_test_flit):
131131
mocker.patch('build.__main__._error')
132132
install = mocker.patch('build.env.DefaultIsolatedEnv.install')
133133

134-
build.__main__.build_package(package_test_flit, '.', ['sdist'])
134+
build.__main__.build_package(package_test_flit, '.', ['sdist'], skip_dependency_check=True)
135135

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

138-
required_cmd.assert_called_with('sdist')
138+
required_cmd.assert_called_with('sdist', None)
139139
install.assert_any_call(['dep1', 'dep2'])
140140

141141
build_cmd.assert_called_with('sdist', '.', {})

0 commit comments

Comments
 (0)