Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initial support for cmake.preset #994

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,14 @@ cmake.source-dir = "."
# DEPRECATED in 0.10; use build.targets instead.
cmake.targets = ""

# Configure preset to use. ``cmake.source-dir`` must still be appropriately
# defined and it must contain a ``CMake(User)Presets.json``. The preset's
# ``binaryDir`` is ignored and is always overwritten by the ``build-dir``
# defined by scikit-build-core. ``cmake.define``, generator values are still
# passed if defined and take precedence over preset's value according to CMake
# logic.
cmake.preset = ""

# The versions of Ninja to allow. If Ninja is not present on the system or does
# not pass this specifier, it will be downloaded via PyPI if possible. An empty
# string will disable this check.
Expand Down
1 change: 1 addition & 0 deletions src/scikit_build_core/builder/builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,7 @@ def configure(
cmake_defines.update(self.settings.cmake.define)

self.config.configure(
preset=self.settings.cmake.preset,
defines=cmake_defines,
cmake_args=[*self.get_cmake_args(), *configure_args],
)
Expand Down
1 change: 1 addition & 0 deletions src/scikit_build_core/builder/generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ def set_environment_for_gen(

If gen is not None, then that will be the target generator.
"""
# TODO: How does make_fallback interact when `preset` is set?
allow_make_fallback = ninja_settings.make_fallback

if generator:
Expand Down
17 changes: 15 additions & 2 deletions src/scikit_build_core/cmake.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,23 @@
from pathlib import Path
from typing import TYPE_CHECKING, Any

from packaging.version import Version

from . import __version__
from ._logging import logger
from ._shutil import Run
from .errors import CMakeConfigError, CMakeNotFoundError, FailedLiveProcessError
from .errors import (
CMakeConfigError,
CMakeNotFoundError,
CMakeVersionError,
FailedLiveProcessError,
)
from .program_search import Program, best_program, get_cmake_program, get_cmake_programs

if TYPE_CHECKING:
from collections.abc import Generator, Iterable, Mapping, Sequence

from packaging.specifiers import SpecifierSet
from packaging.version import Version

from ._compat.typing import Self

Expand Down Expand Up @@ -222,12 +228,19 @@
def configure(
self,
*,
preset: str | None = None,
defines: Mapping[str, str | os.PathLike[str] | bool] | None = None,
cmake_args: Sequence[str] = (),
) -> None:
_cmake_args = self._compute_cmake_args(defines or {})
all_args = [*_cmake_args, *cmake_args]

if preset:
if self.cmake.version < Version("3.19"):
msg = f"CMake version ({self.cmake.version}) is too old to support presets."
raise CMakeVersionError(msg)

Check warning on line 241 in src/scikit_build_core/cmake.py

View check run for this annotation

Codecov / codecov/patch

src/scikit_build_core/cmake.py#L240-L241

Added lines #L240 - L241 were not covered by tests
all_args.append(f"--preset={preset}")

gen = self.get_generator(*all_args)
if gen:
self.single_config = gen == "Ninja" or "Makefiles" in gen
Expand Down
4 changes: 4 additions & 0 deletions src/scikit_build_core/resources/scikit-build.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,10 @@
},
"description": "DEPRECATED in 0.10; use build.targets instead.",
"deprecated": true
},
"preset": {
"type": "string",
"description": "Configure preset to use. ``cmake.source-dir`` must still be appropriately defined and it must contain a ``CMake(User)Presets.json``. The preset's ``binaryDir`` is ignored and is always overwritten by the ``build-dir`` defined by scikit-build-core. ``cmake.define``, generator values are still passed if defined and take precedence over preset's value according to CMake logic."
}
}
},
Expand Down
9 changes: 9 additions & 0 deletions src/scikit_build_core/settings/skbuild_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,15 @@ class CMakeSettings:
DEPRECATED in 0.10; use build.targets instead.
"""

preset: Optional[str] = None
"""
Configure preset to use. ``cmake.source-dir`` must still be appropriately defined
and it must contain a ``CMake(User)Presets.json``. The preset's ``binaryDir`` is
ignored and is always overwritten by the ``build-dir`` defined by scikit-build-core.
``cmake.define``, generator values are still passed if defined and take precedence
over preset's value according to CMake logic.
"""


@dataclasses.dataclass
class NinjaSettings:
Expand Down
6 changes: 5 additions & 1 deletion src/scikit_build_core/settings/skbuild_read_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -273,8 +273,12 @@ def __init__(
new_min_cmake = "3.15"
self.settings.cmake.version = SpecifierSet(f">={new_min_cmake}")

default_cmake_minimum = "3.15"
if self.settings.cmake.preset:
default_cmake_minimum = "3.19"
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think I am handling this part correctly. Generally it should not be tied to the auto-cmake version, but not sure where to feed it to override potential values set by the user. Or we just fail it at builder.cmake?


_handle_minimum_version(
self.settings.cmake, self.settings.minimum_version, "3.15"
self.settings.cmake, self.settings.minimum_version, default_cmake_minimum
)
_handle_minimum_version(self.settings.ninja, self.settings.minimum_version)

Expand Down
9 changes: 9 additions & 0 deletions tests/packages/cmake_defines/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,19 @@ set(ONE_LEVEL_LIST
set(NESTED_LIST
""
CACHE STRING "")
set(PRESET_ONLY_VAR
""
CACHE STRING "")
set(OVERWRITTEN_VAR
""
CACHE STRING "")

set(out_file "${CMAKE_CURRENT_BINARY_DIR}/log.txt")
file(WRITE "${out_file}" "")

file(APPEND "${out_file}" "PRESET_ONLY_VAR=${PRESET_ONLY_VAR}\n")
file(APPEND "${out_file}" "OVERWRITTEN_VAR=${OVERWRITTEN_VAR}\n")

foreach(list IN ITEMS ONE_LEVEL_LIST NESTED_LIST)
list(LENGTH ${list} length)
file(APPEND "${out_file}" "${list}.LENGTH = ${length}\n")
Expand Down
14 changes: 14 additions & 0 deletions tests/packages/cmake_defines/CMakePresets.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"version": 1,
"configurePresets": [
{
"name": "scikit",
"cacheVariables": {
"PRESET_ONLY_VAR": "defined",
"OVERWRITTEN_VAR": "original"
},
"binaryDir": "/dev/null",
"generator": "Ninja"
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Windows disapproves, but empty generator is only available in version 3. Not sure if I can bump it and satisfy the other builders. What generator value is available on windows?

}
]
}
5 changes: 5 additions & 0 deletions tests/packages/cmake_defines/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,8 @@ ONE_LEVEL_LIST = [
"Baz",
]
NESTED_LIST = [ "Apple", "Lemon;Lime", "Banana" ]
OVERWRITTEN_VAR = "overwritten"

[[tool.scikit-build.overrides]]
if.env.WITH_PRESET = true
cmake.preset = "scikit"
26 changes: 25 additions & 1 deletion tests/test_cmake_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,14 @@
from scikit_build_core.builder.builder import Builder
from scikit_build_core.cmake import CMake, CMaker
from scikit_build_core.errors import CMakeNotFoundError
from scikit_build_core.program_search import best_program, get_cmake_programs
from scikit_build_core.settings.skbuild_read_settings import SettingsReader

if TYPE_CHECKING:
from collections.abc import Generator

DIR = Path(__file__).parent.resolve()
cmake_preset_info = best_program(get_cmake_programs(), version=SpecifierSet(">=3.19"))


def single_config(param: None | str) -> bool:
Expand Down Expand Up @@ -204,10 +206,26 @@ def test_cmake_paths(
assert len(fp.calls) == 2


@pytest.mark.parametrize(
"with_preset",
[
pytest.param(
True,
marks=pytest.mark.skipif(
cmake_preset_info is None,
reason="CMake version does not support presets.",
),
),
False,
],
)
@pytest.mark.configure
def test_cmake_defines(
monkeypatch,
tmp_path: Path,
with_preset: bool,
):
monkeypatch.setenv("WITH_PRESET", f"{with_preset}")
source_dir = DIR / "packages" / "cmake_defines"
binary_dir = tmp_path / "build"

Expand All @@ -224,8 +242,14 @@ def test_cmake_defines(
builder.configure(defines={})

configure_log = Path.read_text(binary_dir / "log.txt")

# This var is always overwritten
overwritten_var = "overwritten"
preset_only_var = "defined" if with_preset else ""
assert configure_log == dedent(
"""\
f"""\
PRESET_ONLY_VAR={preset_only_var}
OVERWRITTEN_VAR={overwritten_var}
ONE_LEVEL_LIST.LENGTH = 4
Foo
Bar
Expand Down
1 change: 1 addition & 0 deletions tests/test_skbuild_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -764,4 +764,5 @@ def test_skbuild_settings_cmake_define_list():
assert settings.cmake.define == {
"NESTED_LIST": r"Apple;Lemon\;Lime;Banana",
"ONE_LEVEL_LIST": "Foo;Bar;ExceptionallyLargeListEntryThatWouldOverflowTheLine;Baz",
"OVERWRITTEN_VAR": "overwritten",
}
Loading