Skip to content

Commit

Permalink
feat: build directory setting & reuse (#181)
Browse files Browse the repository at this point in the history
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 mesonbuild/meson-python#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 <henryschreineriii@gmail.com>
  • Loading branch information
henryiii authored Jan 30, 2023
1 parent 10f9a59 commit d64e68f
Show file tree
Hide file tree
Showing 8 changed files with 114 additions and 7 deletions.
3 changes: 3 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
```
Expand Down
9 changes: 8 additions & 1 deletion src/scikit_build_core/build/wheel.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
2 changes: 2 additions & 0 deletions src/scikit_build_core/builder/builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
47 changes: 44 additions & 3 deletions src/scikit_build_core/cmake.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
from __future__ import annotations

import contextlib
import dataclasses
import json
import os
import shutil
import subprocess
import sys
import textwrap
Expand All @@ -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

Expand Down Expand Up @@ -77,20 +81,57 @@ 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:
with self.init_cache_file.open("w", encoding="utf-8") as f:
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{}",
Expand Down
3 changes: 3 additions & 0 deletions src/scikit_build_core/settings/skbuild_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = ""
6 changes: 3 additions & 3 deletions tests/test_cmake_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
"""
)

Expand Down
44 changes: 44 additions & 0 deletions tests/test_pyproject_pep518.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
7 changes: 7 additions & 0 deletions tests/test_skbuild_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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")
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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)
Expand All @@ -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):
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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):
Expand Down

0 comments on commit d64e68f

Please sign in to comment.