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

Author: jameshilliard

src/build/__init__.py

@@ -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>.+)'
@@ -104,6 +107,19 @@ 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 TypoWarning(Warning):
     """
     Warning raised when a potential typo is found
@@ -131,8 +147,16 @@ 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,
 ) -> Iterator[Tuple[str, ...]]:
     """
     Verify that a dependency and all of its dependencies are met.
@@ -150,6 +174,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 _normalize(req.name) == _normalize(project_name):
+        raise CircularBuildSystemDependencyError((project_name,) + ancestral_req_strings + (req_string,))
+
     if req.marker:
         extras = frozenset(('',)).union(parent_extras)
         # a requirement can have multiple extras but ``evaluate`` can
@@ -171,7 +201,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:
@@ -222,6 +252,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.
@@ -267,10 +314,14 @@ 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._scripts_dir = scripts_dir
         self._hook_runner = runner
+        self._in_tree_build = False
+        if 'backend-path' in self._build_system:
+            self._in_tree_build = True
         self._hook = pep517.wrappers.Pep517HookCaller(
             self.srcdir,
             self._backend,
@@ -341,6 +392,17 @@ 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
+        """
+        if self._in_tree_build:
+            return set()
+        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, ...]]:
@@ -353,8 +415,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
@@ -399,7 +462,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:
         """
@@ -413,13 +484,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:

src/build/__main__.py

@@ -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_build_dependencies()
+            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_build_dependencies()
+        return build_result
 
 
 def _build_in_current_env(
@@ -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_build_dependencies()
 
-    return builder.build(distribution, outdir, config_settings or {})
+    return build_result
 
 
 def _build(
@@ -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)
 

tests/packages/test-circular-requirements/pyproject.toml

@@ -0,0 +1,7 @@
+[build-system]
+requires = ["recursive_dep"]
+
+[project]
+name = "recursive_unmet_dep"
+version = "1.0.0"
+description = "circular project"

tests/test_projectbuilder.py

@@ -5,9 +5,12 @@
 import importlib
 import logging
 import os
+import pathlib
 import sys
 import textwrap
 
+from inspect import currentframe, getframeinfo
+
 import pep517.wrappers
 import pytest
 
@@ -19,8 +22,6 @@
 else:  # pragma: no cover
     import importlib_metadata
 
-import pathlib
-
 
 build_open_owner = 'builtins'
 
@@ -133,6 +134,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_build_dependencies()
+
+
 def test_bad_project(package_test_no_project):
     # Passing a nonexistent project directory
     with pytest.raises(build.BuildException):
@@ -570,7 +583,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 == getframeinfo(currentframe()).lineno - 11
 
 
 @pytest.mark.parametrize(