From 4ec3f30b4d54d60ae53f044dbac73a8f982b55f4 Mon Sep 17 00:00:00 2001 From: James Hilliard Date: Sun, 12 Dec 2021 07:46:59 -0700 Subject: [PATCH] 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 --- src/build/__init__.py | 67 +++++++++++++++++-- src/build/__main__.py | 11 ++- .../test-circular-requirements/pyproject.toml | 8 +++ tests/requirements.txt | 1 + tests/test_integration.py | 2 + tests/test_main.py | 14 ++++ tox.ini | 2 +- 7 files changed, 98 insertions(+), 7 deletions(-) create mode 100644 tests/packages/test-circular-requirements/pyproject.toml create mode 100644 tests/requirements.txt diff --git a/src/build/__init__.py b/src/build/__init__.py index 41adff723..c1323f927 100644 --- a/src/build/__init__.py +++ b/src/build/__init__.py @@ -104,6 +104,28 @@ 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 ProjectTableValidationError(BuildException): + """ + Exception raised when the ``[project]`` table in pyproject.toml is invalid. + """ + + def __str__(self) -> str: + return f'Failed to validate `project` in pyproject.toml: {self.args[0]}' + + class TypoWarning(Warning): """ Warning raised when a potential typo is found @@ -132,7 +154,10 @@ def _validate_source_directory(srcdir: PathType) -> None: 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 +175,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 req.name == project_name: + raise CircularBuildSystemDependencyError(ancestral_req_strings + (req_string,)) + if req.marker: extras = frozenset(('',)).union(parent_extras) # a requirement can have multiple extras but ``evaluate`` can @@ -171,7 +202,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 +253,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: + raise ProjectTableValidationError('`name` is a required property') + + project_name = project_table['name'] + if not isinstance(project_name, str): + raise ProjectTableValidationError('`name` must be a string') + + return project_name + + class ProjectBuilder: """ The PEP 517 consumer API. @@ -268,6 +316,7 @@ def __init__( raise BuildException(f'Failed to parse {spec_file}: {e} ') self._build_system = _parse_build_system_table(spec) + self._project_name = _parse_project_name(spec) self._backend = self._build_system['build-backend'] self._scripts_dir = scripts_dir self._hook_runner = runner @@ -341,6 +390,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, ...]]: @@ -353,8 +411,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 diff --git a/src/build/__main__.py b/src/build/__main__.py index d5685cd9d..f802288d4 100644 --- a/src/build/__main__.py +++ b/src/build/__main__.py @@ -99,13 +99,20 @@ 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 + if not skip_dependency_check: + builder.check_build_dependencies() # 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 {}) @@ -137,7 +144,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) diff --git a/tests/packages/test-circular-requirements/pyproject.toml b/tests/packages/test-circular-requirements/pyproject.toml new file mode 100644 index 000000000..3b59fc41e --- /dev/null +++ b/tests/packages/test-circular-requirements/pyproject.toml @@ -0,0 +1,8 @@ +[build-system] +requires = ["flit_core==3.5.1"] +build-backend = "flit_core.buildapi" + +[project] +name = "tomli" +version = "1.2.2" +description = "A lil' TOML parser" diff --git a/tests/requirements.txt b/tests/requirements.txt new file mode 100644 index 000000000..6cde04720 --- /dev/null +++ b/tests/requirements.txt @@ -0,0 +1 @@ +flit_core==3.5.1 diff --git a/tests/test_integration.py b/tests/test_integration.py index 0db1b00e9..aaccba425 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -123,6 +123,8 @@ def test_build(monkeypatch, project, args, call, tmp_path): def test_isolation(tmp_dir, package_test_flit, mocker): + if 'flit_core ' in sys.modules: + del sys.modules["flit_core"] try: import flit_core # noqa: F401 except ModuleNotFoundError: diff --git a/tests/test_main.py b/tests/test_main.py index d34f1bc23..5fc6608d2 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -147,6 +147,20 @@ def test_build_no_isolation_check_deps_empty(mocker, package_test_flit): build_cmd.assert_called_with('sdist', '.', {}) +@pytest.mark.parametrize('distribution', ['wheel', 'sdist']) +def test_build_no_isolation_circular_requirements(package_test_circular_requirements, distribution): + msg = 'Failed to validate `build-system` in pyproject.toml, dependency cycle detected: `flit_core==3.5.1` -> `tomli`' + with pytest.raises(build.CircularBuildSystemDependencyError, match=msg): + build.__main__.build_package(package_test_circular_requirements, '.', [distribution], isolation=False) + + +@pytest.mark.parametrize('distribution', ['wheel', 'sdist']) +def test_build_circular_requirements(package_test_circular_requirements, distribution): + msg = 'Failed to validate `build-system` in pyproject.toml, dependency cycle detected: `flit_core==3.5.1` -> `tomli`' + with pytest.raises(build.CircularBuildSystemDependencyError, match=msg): + build.__main__.build_package(package_test_circular_requirements, '.', [distribution]) + + @pytest.mark.parametrize( ['missing_deps', 'output'], [ diff --git a/tox.ini b/tox.ini index c51c3bc33..664e1b8e3 100644 --- a/tox.ini +++ b/tox.ini @@ -60,7 +60,7 @@ commands = description = check minimum versions required of all dependencies skip_install = true commands = - pip install .[test] -c tests/constraints.txt + pip install .[test] -c tests/constraints.txt -r tests/requirements.txt pytest -rsx tests {posargs:-n auto} [testenv:docs]