Skip to content

Commit 14c2d8a

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 96f9188 commit 14c2d8a

File tree

4 files changed

+145
-12
lines changed

4 files changed

+145
-12
lines changed

src/build/__init__.py

Lines changed: 99 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,9 @@
5454
_ExcInfoType = Union[Tuple[Type[BaseException], BaseException, types.TracebackType], Tuple[None, None, None]]
5555

5656

57+
_SDIST_NAME_REGEX = re.compile(r'(?P<distribution>.+)-(?P<version>.+)\.tar.gz')
58+
59+
5760
_WHEEL_NAME_REGEX = re.compile(
5861
r'(?P<distribution>.+)-(?P<version>.+)'
5962
r'(-(?P<build_tag>.+))?-(?P<python_tag>.+)'
@@ -104,6 +107,30 @@ def __str__(self) -> str:
104107
return f'Failed to validate `build-system` in pyproject.toml: {self.args[0]}'
105108

106109

110+
class CircularBuildSystemDependencyError(BuildException):
111+
"""
112+
Exception raised when a ``[build-system]`` requirement in pyproject.toml is circular.
113+
"""
114+
115+
def __init__(
116+
self, project_name: str, ancestral_req_strings: Tuple[str, ...], req_string: str, backend: Optional[str]
117+
) -> None:
118+
super().__init__()
119+
self.project_name: str = project_name
120+
self.ancestral_req_strings: Tuple[str, ...] = ancestral_req_strings
121+
self.req_string: str = req_string
122+
self.backend: Optional[str] = backend
123+
124+
def __str__(self) -> str:
125+
cycle_err_str = f'`{self.project_name}`'
126+
if self.backend:
127+
cycle_err_str += f' -> `{self.backend}`'
128+
for dep in self.ancestral_req_strings:
129+
cycle_err_str += f' -> `{dep}`'
130+
cycle_err_str += f' -> `{self.req_string}`'
131+
return f'Failed to validate `build-system` in pyproject.toml, dependency cycle detected: {cycle_err_str}'
132+
133+
107134
class TypoWarning(Warning):
108135
"""
109136
Warning raised when a potential typo is found
@@ -131,8 +158,17 @@ def _validate_source_directory(srcdir: PathType) -> None:
131158
raise BuildException(f'Source {srcdir} does not appear to be a Python project: no pyproject.toml or setup.py')
132159

133160

161+
# https://www.python.org/dev/peps/pep-0503/#normalized-names
162+
def _normalize(name: str) -> str:
163+
return re.sub(r'[-_.]+', '-', name).lower()
164+
165+
134166
def check_dependency(
135-
req_string: str, ancestral_req_strings: Tuple[str, ...] = (), parent_extras: AbstractSet[str] = frozenset()
167+
req_string: str,
168+
ancestral_req_strings: Tuple[str, ...] = (),
169+
parent_extras: AbstractSet[str] = frozenset(),
170+
project_name: Optional[str] = None,
171+
backend: Optional[str] = None,
136172
) -> Iterator[Tuple[str, ...]]:
137173
"""
138174
Verify that a dependency and all of its dependencies are met.
@@ -159,6 +195,12 @@ def check_dependency(
159195
# dependency is satisfied.
160196
return
161197

198+
# Front ends SHOULD check explicitly for requirement cycles, and
199+
# terminate the build with an informative message if one is found.
200+
# https://www.python.org/dev/peps/pep-0517/#build-requirements
201+
if project_name is not None and _normalize(req.name) == _normalize(project_name):
202+
raise CircularBuildSystemDependencyError(project_name, ancestral_req_strings, req_string, backend)
203+
162204
try:
163205
dist = importlib_metadata.distribution(req.name) # type: ignore[no-untyped-call]
164206
except importlib_metadata.PackageNotFoundError:
@@ -171,7 +213,7 @@ def check_dependency(
171213
elif dist.requires:
172214
for other_req_string in dist.requires:
173215
# yields transitive dependencies that are not satisfied.
174-
yield from check_dependency(other_req_string, ancestral_req_strings + (req_string,), req.extras)
216+
yield from check_dependency(other_req_string, ancestral_req_strings + (req_string,), req.extras, project_name)
175217

176218

177219
def _find_typo(dictionary: Mapping[str, str], expected: str) -> None:
@@ -222,6 +264,23 @@ def _parse_build_system_table(pyproject_toml: Mapping[str, Any]) -> Dict[str, An
222264
return build_system_table
223265

224266

267+
def _parse_project_name(pyproject_toml: Mapping[str, Any]) -> Optional[str]:
268+
if 'project' not in pyproject_toml:
269+
return None
270+
271+
project_table = dict(pyproject_toml['project'])
272+
273+
# If [project] is present, it must have a ``name`` field (per PEP 621)
274+
if 'name' not in project_table:
275+
return None
276+
277+
project_name = project_table['name']
278+
if not isinstance(project_name, str):
279+
return None
280+
281+
return project_name
282+
283+
225284
class ProjectBuilder:
226285
"""
227286
The PEP 517 consumer API.
@@ -267,8 +326,10 @@ def __init__(
267326
except TOMLDecodeError as e:
268327
raise BuildException(f'Failed to parse {spec_file}: {e} ')
269328

329+
self.project_name: Optional[str] = _parse_project_name(spec)
270330
self._build_system = _parse_build_system_table(spec)
271331
self._backend = self._build_system['build-backend']
332+
self._requires_for_build_cache: dict[str, Optional[Set[str]]] = {'wheel': None, 'sdist': None}
272333
self._scripts_dir = scripts_dir
273334
self._hook_runner = runner
274335
self._hook = pep517.wrappers.Pep517HookCaller(
@@ -341,6 +402,15 @@ def get_requires_for_build(self, distribution: str, config_settings: Optional[Co
341402
with self._handle_backend(hook_name):
342403
return set(get_requires(config_settings))
343404

405+
def check_build_dependencies(self) -> Set[Tuple[str, ...]]:
406+
"""
407+
Return the dependencies which are not satisfied from
408+
:attr:`build_system_requires`
409+
410+
:returns: Set of variable-length unmet dependency tuples
411+
"""
412+
return {u for d in self.build_system_requires for u in check_dependency(d, project_name=self.project_name)}
413+
344414
def check_dependencies(
345415
self, distribution: str, config_settings: Optional[ConfigSettingsType] = None
346416
) -> Set[Tuple[str, ...]]:
@@ -353,8 +423,20 @@ def check_dependencies(
353423
:param config_settings: Config settings for the build backend
354424
:returns: Set of variable-length unmet dependency tuples
355425
"""
356-
dependencies = self.get_requires_for_build(distribution, config_settings).union(self.build_system_requires)
357-
return {u for d in dependencies for u in check_dependency(d)}
426+
build_system_dependencies = self.check_build_dependencies()
427+
requires_for_build: Set[str]
428+
requires_for_build_cache: Optional[Set[str]] = self._requires_for_build_cache[distribution]
429+
if requires_for_build_cache is not None:
430+
requires_for_build = requires_for_build_cache
431+
else:
432+
requires_for_build = self.get_requires_for_build(distribution, config_settings)
433+
dependencies = {
434+
u for d in requires_for_build for u in check_dependency(d, project_name=self.project_name, backend=self._backend)
435+
}
436+
unmet_dependencies = dependencies.union(build_system_dependencies)
437+
if len(unmet_dependencies) == 0:
438+
self._requires_for_build_cache[distribution] = requires_for_build
439+
return unmet_dependencies
358440

359441
def prepare(
360442
self, distribution: str, output_directory: PathType, config_settings: Optional[ConfigSettingsType] = None
@@ -399,7 +481,15 @@ def build(
399481
"""
400482
self.log(f'Building {distribution}...')
401483
kwargs = {} if metadata_directory is None else {'metadata_directory': metadata_directory}
402-
return self._call_backend(f'build_{distribution}', output_directory, config_settings, **kwargs)
484+
basename = self._call_backend(f'build_{distribution}', output_directory, config_settings, **kwargs)
485+
match = None
486+
if distribution == 'wheel':
487+
match = _WHEEL_NAME_REGEX.match(os.path.basename(basename))
488+
elif distribution == 'sdist':
489+
match = _SDIST_NAME_REGEX.match(os.path.basename(basename))
490+
if match:
491+
self.project_name = match['distribution']
492+
return basename
403493

404494
def metadata_path(self, output_directory: PathType) -> str:
405495
"""
@@ -413,13 +503,17 @@ def metadata_path(self, output_directory: PathType) -> str:
413503
# prepare_metadata hook
414504
metadata = self.prepare('wheel', output_directory)
415505
if metadata is not None:
506+
match = _WHEEL_NAME_REGEX.match(os.path.basename(metadata))
507+
if match:
508+
self.project_name = match['distribution']
416509
return metadata
417510

418511
# fallback to build_wheel hook
419512
wheel = self.build('wheel', output_directory)
420513
match = _WHEEL_NAME_REGEX.match(os.path.basename(wheel))
421514
if not match:
422515
raise ValueError('Invalid wheel')
516+
self.project_name = match['distribution']
423517
distinfo = f"{match['distribution']}-{match['version']}.dist-info"
424518
member_prefix = f'{distinfo}/'
425519
with zipfile.ZipFile(wheel) as w:

src/build/__main__.py

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -99,16 +99,29 @@ def _format_dep_chain(dep_chain: Sequence[str]) -> str:
9999

100100

101101
def _build_in_isolated_env(
102-
builder: ProjectBuilder, outdir: PathType, distribution: str, config_settings: Optional[ConfigSettingsType]
102+
builder: ProjectBuilder,
103+
outdir: PathType,
104+
distribution: str,
105+
config_settings: Optional[ConfigSettingsType],
106+
skip_dependency_check: bool = False,
103107
) -> str:
104108
with _IsolatedEnvBuilder() as env:
105109
builder.python_executable = env.executable
106110
builder.scripts_dir = env.scripts_dir
107111
# first install the build dependencies
108112
env.install(builder.build_system_requires)
113+
# validate build system dependencies
114+
revalidate = False
115+
if not skip_dependency_check:
116+
builder.check_dependencies(distribution)
117+
if builder.project_name is None:
118+
revalidate = True
109119
# then get the extra required dependencies from the backend (which was installed in the call above :P)
110120
env.install(builder.get_requires_for_build(distribution))
111-
return builder.build(distribution, outdir, config_settings or {})
121+
build_result = builder.build(distribution, outdir, config_settings or {})
122+
if revalidate and builder.project_name is not None:
123+
builder.check_dependencies(distribution)
124+
return build_result
112125

113126

114127
def _build_in_current_env(
@@ -118,14 +131,22 @@ def _build_in_current_env(
118131
config_settings: Optional[ConfigSettingsType],
119132
skip_dependency_check: bool = False,
120133
) -> str:
134+
revalidate = False
121135
if not skip_dependency_check:
122136
missing = builder.check_dependencies(distribution)
123137
if missing:
124138
dependencies = ''.join('\n\t' + dep for deps in missing for dep in (deps[0], _format_dep_chain(deps[1:])) if dep)
125139
print()
126140
_error(f'Missing dependencies:{dependencies}')
141+
elif builder.project_name is None:
142+
revalidate = True
143+
144+
build_result = builder.build(distribution, outdir, config_settings or {})
145+
146+
if revalidate and builder.project_name is not None:
147+
builder.check_dependencies(distribution)
127148

128-
return builder.build(distribution, outdir, config_settings or {})
149+
return build_result
129150

130151

131152
def _build(
@@ -137,7 +158,7 @@ def _build(
137158
skip_dependency_check: bool,
138159
) -> str:
139160
if isolation:
140-
return _build_in_isolated_env(builder, outdir, distribution, config_settings)
161+
return _build_in_isolated_env(builder, outdir, distribution, config_settings, skip_dependency_check)
141162
else:
142163
return _build_in_current_env(builder, outdir, distribution, config_settings, skip_dependency_check)
143164

Lines changed: 7 additions & 0 deletions
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"

tests/test_projectbuilder.py

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import importlib
66
import logging
77
import os
8+
import pathlib
89
import sys
910
import textwrap
1011

@@ -19,8 +20,6 @@
1920
else: # pragma: no cover
2021
import importlib_metadata
2122

22-
import pathlib
23-
2423

2524
build_open_owner = 'builtins'
2625

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

135134

135+
@pytest.mark.parametrize('distribution', ['wheel', 'sdist'])
136+
def test_build_no_isolation_circular_requirements(monkeypatch, package_test_circular_requirements, distribution):
137+
monkeypatch.setattr(importlib_metadata, 'Distribution', MockDistribution)
138+
msg = (
139+
'Failed to validate `build-system` in pyproject.toml, dependency cycle detected: `recursive_unmet_dep` -> '
140+
'`recursive_dep` -> `recursive_unmet_dep`'
141+
)
142+
builder = build.ProjectBuilder(package_test_circular_requirements)
143+
with pytest.raises(build.CircularBuildSystemDependencyError, match=msg):
144+
builder.check_dependencies(distribution)
145+
146+
136147
def test_bad_project(package_test_no_project):
137148
# Passing a nonexistent project directory
138149
with pytest.raises(build.BuildException):
@@ -570,7 +581,7 @@ def test_log(mocker, caplog, package_test_flit):
570581
('INFO', 'something'),
571582
]
572583
if sys.version_info >= (3, 8): # stacklevel
573-
assert caplog.records[-1].lineno == 562
584+
assert caplog.records[-1].lineno == test_log.__code__.co_firstlineno + 11
574585

575586

576587
@pytest.mark.parametrize(

0 commit comments

Comments
 (0)