Skip to content

Commit

Permalink
fix(setuptools): only inject logic if cmake_* keywords present (#768)
Browse files Browse the repository at this point in the history
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 (cschwan/sage-on-gentoo#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 <henryschreineriii@gmail.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Henry Schreiner <HenrySchreinerIII@gmail.com>
  • Loading branch information
3 people authored Jun 14, 2024
1 parent d9c41f9 commit edddcc7
Showing 1 changed file with 28 additions and 20 deletions.
48 changes: 28 additions & 20 deletions src/scikit_build_core/setuptools/build_cmake.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]:
Expand Down Expand Up @@ -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]

Expand All @@ -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)

0 comments on commit edddcc7

Please sign in to comment.