From d64e68f7db91a1adee1d6d4ae281899a4f45b2a8 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Mon, 30 Jan 2023 12:00:44 -0500 Subject: [PATCH] feat: build directory setting & reuse (#181) Support for custom build directory, opt-in. Using `FORCE` for the moment. Doesn't try to rewrite the paths for moving CMake yet, but the info for this is there in the new json file. Maybe this shouldn't have a `cmake.` prefix? Edit: yes, looks like we could sync-up with meson-python if we just use `build-dir`, so let's go with that. See https://github.com/mesonbuild/meson-python/issues/275 for `builddir` -> `build-dir` there. - [x] TODO: this should be able to (maybe via substitution?) have a unique path per interpreter. Might adjust this later. My thought is we can make this opt-in for now, then if we make it opt-out, we can protect the change on a minimum version selection. --------- Signed-off-by: Henry Schreiner --- README.md | 3 ++ src/scikit_build_core/build/wheel.py | 9 +++- src/scikit_build_core/builder/builder.py | 2 + src/scikit_build_core/cmake.py | 47 +++++++++++++++++-- .../settings/skbuild_model.py | 3 ++ tests/test_cmake_config.py | 6 +-- tests/test_pyproject_pep518.py | 44 +++++++++++++++++ tests/test_skbuild_settings.py | 7 +++ 8 files changed, 114 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 99960fe7..063e7744 100644 --- a/README.md +++ b/README.md @@ -179,6 +179,9 @@ strict-config = true # scikit-build-core version. minimum-version = "0.1" # current version +# Build directory (empty will use a temporary directory) +build-dir = "" + [tool.scikit-build.cmake.define] # Put CMake defines in this table. ``` diff --git a/src/scikit_build_core/build/wheel.py b/src/scikit_build_core/build/wheel.py index f5501ebd..44dc7ed8 100644 --- a/src/scikit_build_core/build/wheel.py +++ b/src/scikit_build_core/build/wheel.py @@ -128,7 +128,14 @@ def build_wheel( with tempfile.TemporaryDirectory() as tmpdir: build_tmp_folder = Path(tmpdir) wheel_dir = build_tmp_folder / "wheel" - build_dir = build_tmp_folder / "build" + + # A build dir can be specified, otherwise use a temporary directory + build_dir = ( + Path(settings.build_dir.format(cache_tag=sys.implementation.cache_tag)) + if settings.build_dir + else build_tmp_folder / "build" + ) + logger.info("Build directory: {}", build_dir.resolve()) wheel_dirs = { "platlib": wheel_dir / "platlib", diff --git a/src/scikit_build_core/builder/builder.py b/src/scikit_build_core/builder/builder.py index fc75e160..74abbc8d 100644 --- a/src/scikit_build_core/builder/builder.py +++ b/src/scikit_build_core/builder/builder.py @@ -77,11 +77,13 @@ def configure( cmake_defines = dict(defines) cmake_args: list[str] = [] + # Add site-packages to the prefix path for CMake site_packages = Path(sysconfig.get_path("purelib")) self.config.prefix_dirs.append(site_packages) if site_packages != DIR.parent.parent: self.config.prefix_dirs.append(DIR.parent.parent) + # Add the FindPython backport if needed fp_backport = self.settings.backport.find_python if fp_backport and self.config.cmake.version < Version(fp_backport): self.config.module_dirs.append(Path(find_python.__file__).parent.resolve()) diff --git a/src/scikit_build_core/cmake.py b/src/scikit_build_core/cmake.py index ac8216f5..6979e052 100644 --- a/src/scikit_build_core/cmake.py +++ b/src/scikit_build_core/cmake.py @@ -1,7 +1,10 @@ from __future__ import annotations +import contextlib import dataclasses +import json import os +import shutil import subprocess import sys import textwrap @@ -13,6 +16,7 @@ from ._logging import logger from ._shutil import Run +from ._version import __version__ from .errors import CMakeConfigError, CMakeNotFoundError, FailedLiveProcessError from .program_search import best_program, get_cmake_programs @@ -77,6 +81,43 @@ def __post_init__(self) -> None: msg = f"build directory {self.build_dir} must be a (creatable) directory" raise CMakeConfigError(msg) + # If these were the same, the following check could wipe the source directory! + if self.build_dir.resolve() == self.source_dir.resolve(): + msg = "build directory must be different from source directory" + raise CMakeConfigError(msg) + + skbuild_info = self.build_dir / ".skbuild-info.json" + # If building via SDist, this could be pre-filled, so delete it if it exists + with contextlib.suppress(FileNotFoundError): + with skbuild_info.open("r", encoding="utf-8") as f: + info = json.load(f) + + cached_source_dir = Path(info["source_dir"]) + if cached_source_dir.resolve() != self.source_dir.resolve(): + logger.warning( + "Original src {} != {}, wiping build directory", + cached_source_dir, + self.source_dir, + ) + shutil.rmtree(self.build_dir) + self.build_dir.mkdir() + + with skbuild_info.open("w", encoding="utf-8") as f: + json.dump(self._info_dict(), f, indent=2) + + def _info_dict(self) -> dict[str, str]: + """ + Produce an information dict about the current run that can be stored in a json file. + """ + return { + "source_dir": os.fspath(self.source_dir.resolve()), + "build_dir": os.fspath(self.build_dir.resolve()), + "cmake_path": os.fspath(self.cmake), + "skbuild_path": os.fspath(DIR), + "skbuild_version": __version__, + "python_executable": sys.executable, + } + def init_cache( self, cache_settings: Mapping[str, str | os.PathLike[str] | bool] ) -> None: @@ -84,13 +125,13 @@ def init_cache( for key, value in cache_settings.items(): if isinstance(value, bool): value = "ON" if value else "OFF" - f.write(f'set({key} {value} CACHE BOOL "")\n') + f.write(f'set({key} {value} CACHE BOOL "" FORCE)\n') elif isinstance(value, os.PathLike): # Convert to CMake's internal path format value = str(value).replace("\\", "/") - f.write(f'set({key} [===[{value}]===] CACHE PATH "")\n') + f.write(f'set({key} [===[{value}]===] CACHE PATH "" FORCE)\n') else: - f.write(f'set({key} [===[{value}]===] CACHE STRING "")\n') + f.write(f'set({key} [===[{value}]===] CACHE STRING "" FORCE)\n') contents = self.init_cache_file.read_text(encoding="utf-8").strip() logger.debug( "{}:\n{}", diff --git a/src/scikit_build_core/settings/skbuild_model.py b/src/scikit_build_core/settings/skbuild_model.py index ec07a835..7f154a02 100644 --- a/src/scikit_build_core/settings/skbuild_model.py +++ b/src/scikit_build_core/settings/skbuild_model.py @@ -120,3 +120,6 @@ class ScikitBuildSettings: #: If set, this will provide a method for backward compatibility. minimum_version: Optional[str] = None + + #: The build directory. Defaults to a temporary directory, but can be set. + build_dir: str = "" diff --git a/tests/test_cmake_config.py b/tests/test_cmake_config.py index f7ef0185..0c7cf561 100644 --- a/tests/test_cmake_config.py +++ b/tests/test_cmake_config.py @@ -60,9 +60,9 @@ def test_init_cache(fp, tmp_path): assert ( cmake_init.read_text() == f"""\ -set(SKBUILD ON CACHE BOOL "") -set(SKBUILD_VERSION [===[1.0.0]===] CACHE STRING "") -set(SKBUILD_PATH [===[{source_dir_str}]===] CACHE PATH "") +set(SKBUILD ON CACHE BOOL "" FORCE) +set(SKBUILD_VERSION [===[1.0.0]===] CACHE STRING "" FORCE) +set(SKBUILD_PATH [===[{source_dir_str}]===] CACHE PATH "" FORCE) """ ) diff --git a/tests/test_pyproject_pep518.py b/tests/test_pyproject_pep518.py index 6be9a089..ec5459b3 100644 --- a/tests/test_pyproject_pep518.py +++ b/tests/test_pyproject_pep518.py @@ -105,6 +105,50 @@ def test_pep518_wheel(isolated, build_args, monkeypatch): assert add == "3" +@pytest.mark.compile() +@pytest.mark.configure() +@pytest.mark.integration() +@pytest.mark.parametrize( + "build_args", [(), ("--wheel",)], ids=["sdist_to_wheel", "wheel_directly"] +) +def test_pep518_rebuild_build_dir(isolated, monkeypatch, tmp_path, build_args): + dist = HELLO_PEP518 / "dist" + monkeypatch.chdir(HELLO_PEP518) + isolated.install("build[virtualenv]") + + build_dir = tmp_path.joinpath("build") + build_dir.mkdir() + build_dir = build_dir.resolve() + + for _ in range(2): + shutil.rmtree(dist, ignore_errors=True) + isolated.module( + "build", + *build_args, + "--config-setting=logging.level=DEBUG", + f"--config-setting=build-dir={build_dir}", + ) + (wheel,) = dist.glob("cmake_example-0.0.1-*.whl") + + if sys.version_info >= (3, 8): + with wheel.open("rb") as f: + p = zipfile.Path(f) + file_names = [p.name for p in p.iterdir()] + + assert len(file_names) == 2 + assert "cmake_example-0.0.1.dist-info" in file_names + file_names.remove("cmake_example-0.0.1.dist-info") + (so_file,) = file_names + + assert so_file.startswith("cmake_example") + print("SOFILE:", so_file) + + isolated.install(wheel) + + version = isolated.execute("import cmake_example; print(cmake_example.__version__)") + assert version == "0.0.1" + + @pytest.mark.compile() @pytest.mark.configure() @pytest.mark.integration() diff --git a/tests/test_skbuild_settings.py b/tests/test_skbuild_settings.py index f94a3069..e5ed4fe4 100644 --- a/tests/test_skbuild_settings.py +++ b/tests/test_skbuild_settings.py @@ -37,6 +37,7 @@ def test_skbuild_settings_default(tmp_path): assert settings.strict_config assert not settings.experimental assert settings.minimum_version is None + assert settings.build_dir == "" def test_skbuild_settings_envvar(tmp_path, monkeypatch): @@ -62,6 +63,7 @@ def test_skbuild_settings_envvar(tmp_path, monkeypatch): monkeypatch.setenv("SKBUILD_EXPERIMENTAL", "1") monkeypatch.setenv("SKBUILD_MINIMUM_VERSION", "0.1") monkeypatch.setenv("SKBUILD_CMAKE_VERBOSE", "TRUE") + monkeypatch.setenv("SKBUILD_BUILD_DIR", "a/b/c") pyproject_toml = tmp_path / "pyproject.toml" pyproject_toml.write_text("", encoding="utf-8") @@ -90,6 +92,7 @@ def test_skbuild_settings_envvar(tmp_path, monkeypatch): assert not settings.strict_config assert settings.experimental assert settings.minimum_version == "0.1" + assert settings.build_dir == "a/b/c" def test_skbuild_settings_config_settings(tmp_path, monkeypatch): @@ -120,6 +123,7 @@ def test_skbuild_settings_config_settings(tmp_path, monkeypatch): "strict-config": "false", "experimental": "1", "minimum-version": "0.1", + "build-dir": "a/b/c", } settings_reader = SettingsReader(pyproject_toml, config_settings) @@ -144,6 +148,7 @@ def test_skbuild_settings_config_settings(tmp_path, monkeypatch): assert not settings.strict_config assert settings.experimental assert settings.minimum_version == "0.1" + assert settings.build_dir == "a/b/c" def test_skbuild_settings_pyproject_toml(tmp_path, monkeypatch): @@ -173,6 +178,7 @@ def test_skbuild_settings_pyproject_toml(tmp_path, monkeypatch): strict-config = false experimental = true minimum-version = "0.1" + build-dir = "a/b/c" """ ), encoding="utf-8", @@ -202,6 +208,7 @@ def test_skbuild_settings_pyproject_toml(tmp_path, monkeypatch): assert not settings.strict_config assert settings.experimental assert settings.minimum_version == "0.1" + assert settings.build_dir == "a/b/c" def test_skbuild_settings_pyproject_toml_broken(tmp_path, capsys):