From 95dd8642ab93d69e9f65d3f882a2ef45fbcc1bec Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Tue, 8 Aug 2023 20:32:10 -0400 Subject: [PATCH] feat: SDist CMake option Signed-off-by: Henry Schreiner --- README.md | 11 ++- docs/configuration.md | 46 +++++++++++ noxfile.py | 10 ++- src/scikit_build_core/build/__init__.py | 6 ++ src/scikit_build_core/build/sdist.py | 6 ++ src/scikit_build_core/build/wheel.py | 10 ++- src/scikit_build_core/builder/get_requires.py | 13 ++- .../resources/scikit-build.schema.json | 7 +- .../settings/skbuild_model.py | 7 ++ .../setuptools/build_meta.py | 14 ++-- tests/conftest.py | 11 +++ tests/packages/sdist_config/.gitignore | 1 + tests/packages/sdist_config/CMakeLists.txt | 22 +++++ tests/packages/sdist_config/main.cpp | 7 ++ tests/packages/sdist_config/pyproject.toml | 18 +++++ tests/test_get_requires.py | 6 ++ tests/test_pyproject_pep518.py | 80 +++++++++++++++++++ tests/test_skbuild_settings.py | 7 ++ 18 files changed, 267 insertions(+), 15 deletions(-) create mode 100644 tests/packages/sdist_config/.gitignore create mode 100644 tests/packages/sdist_config/CMakeLists.txt create mode 100644 tests/packages/sdist_config/main.cpp create mode 100644 tests/packages/sdist_config/pyproject.toml diff --git a/README.md b/README.md index 554854a1..ae2ee1eb 100644 --- a/README.md +++ b/README.md @@ -178,6 +178,9 @@ sdist.exclude = [] # value if not set. sdist.reproducible = true +# If set to True, CMake will be run before building the SDist. +sdist.cmake = false + # A list of packages to auto-copy into the wheel. If this is not set, it will # default to the first of ``src/`` or ```` if they exist. The # prefix(s) will be stripped from the package name inside the wheel. @@ -195,9 +198,11 @@ wheel.py-api = "" # (before 21.0.1) find the correct wheel. wheel.expand-macos-universal-tags = false -# The install directory for the wheel. This is relative to the platlib root. -# EXPERIMENTAL: An absolute path will be one level higher than the platlib root, -# giving access to "/platlib", "/data", "/headers", and "/scripts". +# The install directory for the wheel. This is relative to the platlib root. You +# might set this to the package name. The original dir is still at +# SKBUILD_PLATLIB_DIR (also SKBUILD_DATA_DIR, etc. are available). EXPERIMENTAL: +# An absolute path will be one level higher than the platlib root, giving access +# to "/platlib", "/data", "/headers", and "/scripts". wheel.install-dir = "" # A list of license files to include in the wheel. Supports glob patterns. diff --git a/docs/configuration.md b/docs/configuration.md index e67cc43c..81be30fc 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -152,6 +152,52 @@ reproducible builds if you prefer, however: sdist.reproducible = false ``` +You can also request CMake to run during this step: + +```toml +[tool.scikit-build] +sdist.cmake = true +``` + +:::{note} + +If you do this, you'll want to have some artifact from the configure in your +source directory; for example: + +```cmake +include(FetchContent) + +if(NOT SKBUILD_STATE STREQUAL "sdist" + AND EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/pybind11/CMakeLists.txt") + message(STATUS "Using integrated pybind11") + set(FETCHCONTENT_FULLY_DISCONNECTED ON) +endif() + +FetchContent_Declare( + pybind11 + GIT_REPOSITORY https://github.com/pybind/pybind11.git + GIT_TAG v2.11.1 + SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR}/pybind11) + +set(PYBIND11_FINDPYTHON ON) +FetchContent_MakeAvailable(pybind11) +``` + +The `/pybind11` directory is in the `.gitignore` and important parts are in +`sdist.include`: + +```toml +[tool.scikit-build] +sdist.cmake = true +sdist.include = [ + "pybind11/tools", + "pybind11/include", + "pybind11/CMakeLists.txt", +] +``` + +::: + ## Customizing the built wheel The wheel will automatically look for Python packages at `` and diff --git a/noxfile.py b/noxfile.py index 7089def6..5746a8b0 100644 --- a/noxfile.py +++ b/noxfile.py @@ -10,7 +10,15 @@ DIR = Path(__file__).parent.resolve() -nox.options.sessions = ["lint", "pylint", "tests"] +nox.options.sessions = [ + "lint", + "pylint", + "generate_schema", + "readme", + "build_api_docs", + "tests", + "test_doc_examples", +] @nox.session(reuse_venv=True) diff --git a/src/scikit_build_core/build/__init__.py b/src/scikit_build_core/build/__init__.py index 44297787..70c68a8a 100644 --- a/src/scikit_build_core/build/__init__.py +++ b/src/scikit_build_core/build/__init__.py @@ -104,9 +104,15 @@ def get_requires_for_build_sdist( requires = GetRequires(config_settings) + # These are only injected if cmake is required for the SDist step + cmake_requires = ( + [*requires.cmake(), *requires.ninja()] if requires.settings.sdist.cmake else [] + ) + return [ "pathspec", "pyproject_metadata", + *cmake_requires, *requires.dynamic_metadata(), ] diff --git a/src/scikit_build_core/build/sdist.py b/src/scikit_build_core/build/sdist.py index a6451c28..45e42747 100644 --- a/src/scikit_build_core/build/sdist.py +++ b/src/scikit_build_core/build/sdist.py @@ -18,6 +18,7 @@ from ..settings.skbuild_read_settings import SettingsReader from ._file_processor import each_unignored_file from ._init import setup_logging +from .wheel import _build_wheel_impl __all__: list[str] = ["build_sdist"] @@ -110,6 +111,11 @@ def build_sdist( srcdirname = f"{sdist_name}-{metadata.version}" filename = f"{srcdirname}.tar.gz" + if settings.sdist.cmake: + _build_wheel_impl( + None, config_settings, None, exit_after_config=True, editable=False + ) + sdist_dir.mkdir(parents=True, exist_ok=True) with contextlib.ExitStack() as stack: gzip_container = stack.enter_context( diff --git a/src/scikit_build_core/build/wheel.py b/src/scikit_build_core/build/wheel.py index a3e71fc1..94b7a183 100644 --- a/src/scikit_build_core/build/wheel.py +++ b/src/scikit_build_core/build/wheel.py @@ -71,6 +71,7 @@ def _build_wheel_impl( config_settings: dict[str, list[str] | str] | None, metadata_directory: str | None, *, + exit_after_config: bool = False, editable: bool, ) -> WheelImplReturn: """ @@ -97,6 +98,8 @@ def _build_wheel_impl( action = "editable" if editable else "wheel" if wheel_directory is None: action = f"metadata_{action}" + if exit_after_config: + action = "sdist" cmake = CMake.default_search(minimum_version=settings.cmake.minimum_version) @@ -176,7 +179,7 @@ def _build_wheel_impl( config=config, ) - if wheel_directory is None: + if wheel_directory is None and not exit_after_config: if metadata_directory is None: msg = "metadata_directory must be specified if wheel_directory is None" raise AssertionError(msg) @@ -209,6 +212,11 @@ def _build_wheel_impl( version=metadata.version, ) + if exit_after_config: + return WheelImplReturn("") + + assert wheel_directory is not None + generator = builder.config.env.get( "CMAKE_GENERATOR", "MSVC" diff --git a/src/scikit_build_core/builder/get_requires.py b/src/scikit_build_core/builder/get_requires.py index ec85ade4..50dcfd8d 100644 --- a/src/scikit_build_core/builder/get_requires.py +++ b/src/scikit_build_core/builder/get_requires.py @@ -19,6 +19,7 @@ ) from ..resources import resources from ..settings._load_provider import load_provider +from ..settings.skbuild_model import ScikitBuildSettings from ..settings.skbuild_read_settings import SettingsReader __all__ = ["GetRequires"] @@ -48,8 +49,12 @@ def __post_init__(self) -> None: "pyproject.toml", self.config_settings ).settings + @property + def settings(self) -> ScikitBuildSettings: + return self._settings + def cmake(self) -> Generator[str, None, None]: - cmake_min = self._settings.cmake.minimum_version + cmake_min = self.settings.cmake.minimum_version cmake = best_program( get_cmake_programs(module=False), minimum_version=cmake_min ) @@ -73,7 +78,7 @@ def ninja(self) -> Generator[str, None, None]: if os.environ.get("CMAKE_MAKE_PROGRAM", ""): return - ninja_min = self._settings.ninja.minimum_version + ninja_min = self.settings.ninja.minimum_version ninja = best_program( get_ninja_programs(module=False), minimum_version=ninja_min ) @@ -82,7 +87,7 @@ def ninja(self) -> Generator[str, None, None]: return if ( - self._settings.ninja.make_fallback + self.settings.ninja.make_fallback and not is_known_platform(known_wheels("ninja")) and list(get_make_programs()) ): @@ -93,7 +98,7 @@ def ninja(self) -> Generator[str, None, None]: yield f"ninja>={ninja_min}" def dynamic_metadata(self) -> Generator[str, None, None]: - for dynamic_metadata in self._settings.metadata.values(): + for dynamic_metadata in self.settings.metadata.values(): if "provider" in dynamic_metadata: config = dynamic_metadata.copy() provider = config.pop("provider") diff --git a/src/scikit_build_core/resources/scikit-build.schema.json b/src/scikit_build_core/resources/scikit-build.schema.json index b016bbe9..e639e886 100644 --- a/src/scikit_build_core/resources/scikit-build.schema.json +++ b/src/scikit_build_core/resources/scikit-build.schema.json @@ -110,6 +110,11 @@ "type": "boolean", "default": true, "description": "If set to True, try to build a reproducible distribution (Unix and Python 3.9+ recommended). ``SOURCE_DATE_EPOCH`` will be used for timestamps, or a fixed value if not set." + }, + "cmake": { + "type": "boolean", + "default": false, + "description": "If set to True, CMake will be run before building the SDist." } } }, @@ -137,7 +142,7 @@ "install-dir": { "type": "string", "default": "", - "description": "The install directory for the wheel. This is relative to the platlib root. EXPERIMENTAL: An absolute path will be one level higher than the platlib root, giving access to \"/platlib\", \"/data\", \"/headers\", and \"/scripts\"." + "description": "The install directory for the wheel. This is relative to the platlib root. You might set this to the package name. The original dir is still at SKBUILD_PLATLIB_DIR (also SKBUILD_DATA_DIR, etc. are available). EXPERIMENTAL: An absolute path will be one level higher than the platlib root, giving access to \"/platlib\", \"/data\", \"/headers\", and \"/scripts\"." }, "license-files": { "type": "array", diff --git a/src/scikit_build_core/settings/skbuild_model.py b/src/scikit_build_core/settings/skbuild_model.py index 53209791..93352759 100644 --- a/src/scikit_build_core/settings/skbuild_model.py +++ b/src/scikit_build_core/settings/skbuild_model.py @@ -113,6 +113,11 @@ class SDistSettings: fixed value if not set. """ + cmake: bool = False + """ + If set to True, CMake will be run before building the SDist. + """ + @dataclasses.dataclass class WheelSettings: @@ -142,6 +147,8 @@ class WheelSettings: install_dir: str = "" """ The install directory for the wheel. This is relative to the platlib root. + You might set this to the package name. The original dir is still at + SKBUILD_PLATLIB_DIR (also SKBUILD_DATA_DIR, etc. are available). EXPERIMENTAL: An absolute path will be one level higher than the platlib root, giving access to "/platlib", "/data", "/headers", and "/scripts". """ diff --git a/src/scikit_build_core/setuptools/build_meta.py b/src/scikit_build_core/setuptools/build_meta.py index 7ddd88ea..fc199d02 100644 --- a/src/scikit_build_core/setuptools/build_meta.py +++ b/src/scikit_build_core/setuptools/build_meta.py @@ -7,6 +7,8 @@ prepare_metadata_for_build_wheel, ) +from ..builder.get_requires import GetRequires + if hasattr(setuptools.build_meta, "build_editable"): from setuptools.build_meta import build_editable @@ -38,14 +40,18 @@ def get_requires_for_build_sdist( setuptools_reqs = setuptools.build_meta.get_requires_for_build_sdist( config_settings ) - return [*setuptools_reqs] + requires = GetRequires(config_settings) + + # These are only injected if cmake is required for the SDist step + cmake_requires = ( + [*requires.cmake(), *requires.ninja()] if requires.settings.sdist.cmake else [] + ) + return [*setuptools_reqs, *cmake_requires] def get_requires_for_build_wheel( config_settings: dict[str, str | list[str]] | None = None ) -> list[str]: - from ..builder.get_requires import GetRequires - requires = GetRequires(config_settings) setuptools_reqs = setuptools.build_meta.get_requires_for_build_wheel( @@ -60,8 +66,6 @@ def get_requires_for_build_wheel( def get_requires_for_build_editable( config_settings: dict[str, str | list[str]] | None = None ) -> list[str]: - from ..builder.get_requires import GetRequires - requires = GetRequires(config_settings) setuptools_reqs = setuptools.build_meta.get_requires_for_build_editable( config_settings diff --git a/tests/conftest.py b/tests/conftest.py index f965e2fb..1afccd54 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -296,6 +296,17 @@ def package_simplest_c(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Packa return package +@pytest.fixture() +def package_sdist_config( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> PackageInfo: + package = PackageInfo( + "sdist_config", + ) + process_package(package, tmp_path, monkeypatch) + return package + + def pytest_collection_modifyitems(items: list[pytest.Item]) -> None: for item in items: # Ensure all tests using virtualenv are marked as such diff --git a/tests/packages/sdist_config/.gitignore b/tests/packages/sdist_config/.gitignore new file mode 100644 index 00000000..70533e31 --- /dev/null +++ b/tests/packages/sdist_config/.gitignore @@ -0,0 +1 @@ +/pybind11 diff --git a/tests/packages/sdist_config/CMakeLists.txt b/tests/packages/sdist_config/CMakeLists.txt new file mode 100644 index 00000000..1a596707 --- /dev/null +++ b/tests/packages/sdist_config/CMakeLists.txt @@ -0,0 +1,22 @@ +cmake_minimum_required(VERSION 3.15...3.27) +project(sdist_config LANGUAGES CXX) + +include(FetchContent) + +if(NOT SKBUILD_STATE STREQUAL "sdist" + AND EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/pybind11/CMakeLists.txt") + message(STATUS "Using integrated pybind11") + set(FETCHCONTENT_FULLY_DISCONNECTED ON) +endif() + +FetchContent_Declare( + pybind11 + GIT_REPOSITORY https://github.com/pybind/pybind11.git + GIT_TAG v2.11.1 + SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR}/pybind11) + +set(PYBIND11_FINDPYTHON ON) +FetchContent_MakeAvailable(pybind11) + +pybind11_add_module(sdist_config main.cpp) +install(TARGETS sdist_config DESTINATION .) diff --git a/tests/packages/sdist_config/main.cpp b/tests/packages/sdist_config/main.cpp new file mode 100644 index 00000000..9937ab9a --- /dev/null +++ b/tests/packages/sdist_config/main.cpp @@ -0,0 +1,7 @@ +#include + +namespace py = pybind11; + +PYBIND11_MODULE(sdist_config, m) { + m.def("life", []() { return 42; }); +} diff --git a/tests/packages/sdist_config/pyproject.toml b/tests/packages/sdist_config/pyproject.toml new file mode 100644 index 00000000..f3ae48bf --- /dev/null +++ b/tests/packages/sdist_config/pyproject.toml @@ -0,0 +1,18 @@ +[build-system] +requires = ["scikit-build-core[pyproject]"] +build-backend = "scikit_build_core.build" + +[project] +name = "sdist_config" +version = "0.1.0" + +[tool.scikit-build] +sdist.cmake = true +sdist.include = [ + "pybind11/tools", + "pybind11/include", + "pybind11/CMakeLists.txt", +] +wheel.license-files = [] +wheel.packages = [] +cmake.define.FETCHCONTENT_QUIET = false diff --git a/tests/test_get_requires.py b/tests/test_get_requires.py index c8b55549..27187cd8 100644 --- a/tests/test_get_requires.py +++ b/tests/test_get_requires.py @@ -74,6 +74,12 @@ def test_get_requires_for_build_sdist(fp): assert set(get_requires_for_build_sdist({})) == {"pathspec", "pyproject_metadata"} +def test_get_requires_for_build_sdist_cmake(fp): + expected = {"pathspec", "pyproject_metadata", "cmake>=3.15", *ninja} + fp.register([Path("cmake/path"), "--version"], stdout="3.14.0") + assert set(get_requires_for_build_sdist({"sdist.cmake": "True"})) == expected + + def test_get_requires_for_build_wheel(fp): expected = {"pathspec", "pyproject_metadata", "cmake>=3.15", *ninja} fp.register([Path("cmake/path"), "--version"], stdout="3.14.0") diff --git a/tests/test_pyproject_pep518.py b/tests/test_pyproject_pep518.py index 5b514a20..d44fe9ca 100644 --- a/tests/test_pyproject_pep518.py +++ b/tests/test_pyproject_pep518.py @@ -50,6 +50,86 @@ def test_pep518_sdist(isolated, package_simple_pyproject_ext): assert correct_metadata == pkg_info_contents +@pytest.mark.network() +@pytest.mark.configure() +@pytest.mark.integration() +@pytest.mark.usefixtures("package_sdist_config") +def test_pep518_sdist_with_cmake_config(isolated): + correct_metadata = textwrap.dedent( + """\ + Metadata-Version: 2.1 + Name: sdist_config + Version: 0.1.0 + """ + ) + + isolated.install("build[virtualenv]") + isolated.module("build", "--sdist") + (sdist,) = Path("dist").iterdir() + assert sdist.name == "sdist_config-0.1.0.tar.gz" + + with tarfile.open(sdist) as f: + file_names = set(f.getnames()) + assert file_names > { + f"sdist_config-0.1.0/{x}" + for x in ( + "CMakeLists.txt", + "pyproject.toml", + "main.cpp", + "PKG-INFO", + ) + } + assert sum("pybind11" in x for x in file_names) >= 10 + pkg_info = f.extractfile("sdist_config-0.1.0/PKG-INFO") + assert pkg_info + pkg_info_contents = pkg_info.read().decode() + assert correct_metadata == pkg_info_contents + + +@pytest.mark.network() +@pytest.mark.compile() +@pytest.mark.configure() +@pytest.mark.integration() +@pytest.mark.usefixtures("package_sdist_config") +@pytest.mark.parametrize( + "build_args", [(), ("--wheel",)], ids=["sdist_to_wheel", "wheel_directly"] +) +def test_pep518_wheel_sdist_with_cmake_config(isolated, build_args, capfd): + isolated.install("build[virtualenv]") + isolated.module( + "build", + "--config-setting=logging.level=DEBUG", + *build_args, + ) + out, err = capfd.readouterr() + if not sys.platform.startswith("win32"): + assert "Cloning into 'pybind11'..." in err + if build_args: + assert "Using integrated pybind11" not in out + else: + assert "Using integrated pybind11" in out + + (wheel,) = Path("dist").glob("sdist_config-0.1.0-*.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 "sdist_config-0.1.0.dist-info" in file_names + file_names.remove("sdist_config-0.1.0.dist-info") + (so_file,) = file_names + + assert so_file.startswith("sdist_config") + print("SOFILE:", so_file) + + isolated.install(wheel) + + life = isolated.execute("import sdist_config; print(sdist_config.life())") + assert life == "42" + + @pytest.mark.compile() @pytest.mark.configure() @pytest.mark.integration() diff --git a/tests/test_skbuild_settings.py b/tests/test_skbuild_settings.py index a7455729..52b7ea58 100644 --- a/tests/test_skbuild_settings.py +++ b/tests/test_skbuild_settings.py @@ -37,6 +37,7 @@ def test_skbuild_settings_default(tmp_path: Path): assert settings.sdist.include == [] assert settings.sdist.exclude == [] assert settings.sdist.reproducible + assert not settings.sdist.cmake assert settings.wheel.packages is None assert settings.wheel.py_api == "" assert not settings.wheel.expand_macos_universal_tags @@ -76,6 +77,7 @@ def test_skbuild_settings_envvar(tmp_path: Path, monkeypatch: pytest.MonkeyPatch monkeypatch.setenv("SKBUILD_SDIST_INCLUDE", "a;b; c") monkeypatch.setenv("SKBUILD_SDIST_EXCLUDE", "d;e;f") monkeypatch.setenv("SKBUILD_SDIST_REPRODUCIBLE", "OFF") + monkeypatch.setenv("SKBUILD_SDIST_CMAKE", "ON") monkeypatch.setenv("SKBUILD_WHEEL_PACKAGES", "j; k; l") monkeypatch.setenv("SKBUILD_WHEEL_PY_API", "cp39") monkeypatch.setenv("SKBUILD_WHEEL_EXPAND_MACOS_UNIVERSAL_TAGS", "True") @@ -113,6 +115,7 @@ def test_skbuild_settings_envvar(tmp_path: Path, monkeypatch: pytest.MonkeyPatch assert settings.sdist.include == ["a", "b", "c"] assert settings.sdist.exclude == ["d", "e", "f"] assert not settings.sdist.reproducible + assert settings.sdist.cmake assert settings.wheel.packages == ["j", "k", "l"] assert settings.wheel.py_api == "cp39" assert settings.wheel.expand_macos_universal_tags @@ -155,6 +158,7 @@ def test_skbuild_settings_config_settings( "sdist.include": ["a", "b", "c"], "sdist.exclude": "d;e;f", "sdist.reproducible": "false", + "sdist.cmake": "true", "wheel.packages": ["j", "k", "l"], "wheel.py-api": "cp39", "wheel.expand-macos-universal-tags": "True", @@ -188,6 +192,7 @@ def test_skbuild_settings_config_settings( assert settings.sdist.include == ["a", "b", "c"] assert settings.sdist.exclude == ["d", "e", "f"] assert not settings.sdist.reproducible + assert settings.sdist.cmake assert settings.wheel.packages == ["j", "k", "l"] assert settings.wheel.py_api == "cp39" assert settings.wheel.expand_macos_universal_tags @@ -229,6 +234,7 @@ def test_skbuild_settings_pyproject_toml( sdist.include = ["a", "b", "c"] sdist.exclude = ["d", "e", "f"] sdist.reproducible = false + sdist.cmake = true wheel.packages = ["j", "k", "l"] wheel.py-api = "cp39" wheel.expand-macos-universal-tags = true @@ -268,6 +274,7 @@ def test_skbuild_settings_pyproject_toml( assert settings.sdist.include == ["a", "b", "c"] assert settings.sdist.exclude == ["d", "e", "f"] assert not settings.sdist.reproducible + assert settings.sdist.cmake assert settings.wheel.packages == ["j", "k", "l"] assert settings.wheel.py_api == "cp39" assert settings.wheel.expand_macos_universal_tags