build: add circular dependency checker for build requirements

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,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
@@ -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.
@@ -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:
@@ -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:
@@ -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.
@@ -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(
@@ -341,6 +402,35 @@ 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 get_cache_requires_for_build(
+        self, distribution: str, config_settings: Optional[ConfigSettingsType] = 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: 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)
+            self._requires_for_build_cache[distribution] = requires_for_build
+        return requires_for_build
+
+    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, ...]]:
@@ -353,8 +443,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)
+            # 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: Optional[ConfigSettingsType] = None
@@ -399,7 +501,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 +523,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_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(
@@ -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(
@@ -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_main.py

@@ -127,11 +127,11 @@ def test_build_isolated(mocker, package_test_flit):
     mocker.patch('build.__main__._error')
     install = mocker.patch('build.env._IsolatedEnvVenvPip.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', '.', {})

tests/test_projectbuilder.py

@@ -5,6 +5,7 @@
 import importlib
 import logging
 import os
+import pathlib
 import sys
 import textwrap
 
@@ -19,8 +20,6 @@
 else:  # pragma: no cover
     import importlib_metadata
 
-import pathlib
-
 
 build_open_owner = 'builtins'
 
@@ -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):
@@ -400,7 +411,7 @@ def test_build_with_dep_on_console_script(tmp_path, demo_pkg_inline, capfd, mock
     from build.__main__ import main
 
     with pytest.raises(SystemExit):
-        main(['--wheel', '--outdir', str(tmp_path / 'dist'), str(tmp_path)])
+        main(['--wheel', '--skip-dependency-check', '--outdir', str(tmp_path / 'dist'), str(tmp_path)])
 
     out, err = capfd.readouterr()
     lines = [line[3:] for line in out.splitlines() if line.startswith('BB ')]  # filter for our markers
@@ -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(