From edddcc755950ac60e5b9787fd0179bb8c46dc911 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Bissey?= Date: Fri, 14 Jun 2024 17:21:13 +1200 Subject: [PATCH] fix(setuptools): only inject logic if `cmake_*` keywords present (#768) This is a sort of follow up to #413 and #414. In sage-on-gentoo we discovered that the presence of scikit-build-core breaks the building sagemath (https://github.com/cschwan/sage-on-gentoo/issues/783). scikit-build-core also happens to be an indirect dependency for building sagemath's documentation. On the basis that the presence of setuptolls_rust does not break sagemath's build, I studied the difference with scikit-build-core. I noticed that the elements `bundled in the single "finalize_distribution_options" entrypoint in scikit is split between two different setuptools entrypoints. This PR reproduce that setup and does a bit of clean up. The bundling of the old _prepare_extension_detection and _prepare_build_cmake_command into finalize_distribution_options being redundant after attributing them directly to separate entrypoints. I also renamed the functions to better characterise them. Using this PR, sagemath builds without problems and pyzmq, which uses scikit-build-core also builds properly. --------- Signed-off-by: Henry Schreiner Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Henry Schreiner --- .../setuptools/build_cmake.py | 48 +++++++++++-------- 1 file changed, 28 insertions(+), 20 deletions(-) diff --git a/src/scikit_build_core/setuptools/build_cmake.py b/src/scikit_build_core/setuptools/build_cmake.py index a7efd007..63427875 100644 --- a/src/scikit_build_core/setuptools/build_cmake.py +++ b/src/scikit_build_core/setuptools/build_cmake.py @@ -19,7 +19,12 @@ from .._compat.typing import Literal -__all__ = ["BuildCMake", "cmake_source_dir", "finalize_distribution_options"] +__all__ = [ + "BuildCMake", + "cmake_args", + "cmake_source_dir", + "finalize_distribution_options", +] def __dir__() -> list[str]: @@ -176,9 +181,25 @@ def _has_cmake(dist: Distribution) -> bool: ) -def _prepare_extension_detection(dist: Distribution) -> None: - # Setuptools needs to know that it has extensions modules +def finalize_distribution_options(dist: Distribution) -> None: + # Prepare new build_cmake command and make sure build calls it + build = dist.get_command_class("build") + assert build is not None + if "build_cmake" not in {x for x, _ in build.sub_commands}: + build.sub_commands.append( + ("build_cmake", lambda cmd: _has_cmake(cmd.distribution)) + ) + +def _cmake_extension(dist: Distribution) -> None: + # Every keyword argument needs to call this + # Run this only once + if getattr(dist, "_has_cmake_extensions", False): + return + # pylint: disable-next=protected-access + dist._has_cmake_extensions = True # type: ignore[attr-defined] + + # Setuptools needs to know that it has extensions modules orig_has_ext_modules = dist.has_ext_modules dist.has_ext_modules = lambda: orig_has_ext_modules() or _has_cmake(dist) # type: ignore[method-assign] @@ -192,34 +213,21 @@ def __len__(self) -> int: dist.ext_modules = getattr(dist, "ext_modules", []) or EvilList() -def _prepare_build_cmake_command(dist: Distribution) -> None: - # Prepare new build_cmake command and make sure build calls it - build = dist.get_command_class("build") - assert build is not None - if "build_cmake" not in {x for x, _ in build.sub_commands}: - build.sub_commands.append( - ("build_cmake", lambda cmd: _has_cmake(cmd.distribution)) - ) - - def cmake_args( - _dist: Distribution, attr: Literal["cmake_args"], value: list[str] + dist: Distribution, attr: Literal["cmake_args"], value: list[str] ) -> None: assert attr == "cmake_args" + _cmake_extension(dist) if not isinstance(value, list): msg = "cmake_args must be a list" raise setuptools.errors.SetupError(msg) def cmake_source_dir( - _dist: Distribution, attr: Literal["cmake_source_dir"], value: str + dist: Distribution, attr: Literal["cmake_source_dir"], value: str ) -> None: assert attr == "cmake_source_dir" + _cmake_extension(dist) if not Path(value).is_dir(): msg = "cmake_source_dir must be an existing directory" raise setuptools.errors.SetupError(msg) - - -def finalize_distribution_options(dist: Distribution) -> None: - _prepare_extension_detection(dist) - _prepare_build_cmake_command(dist)