diff --git a/docs/overrides.md b/docs/overrides.md index 585d310a..835a97fc 100644 --- a/docs/overrides.md +++ b/docs/overrides.md @@ -132,6 +132,19 @@ SDist. This will be true if the `PKG-INFO` file exists, that is, if this is coming from an SDist. Takes a bool. +### `failed` (bool) + +This override is a bit special. If a build fails, scikit-build-core will check +to see if there'a a matching `failed = true` override. If there is, the the build will +be retried once with the new settings. This can be used to build a pure-Python fallback +if a build fails, for example: + +```toml +[[tool.scikit-build.overrides]] +if.failed = true +wheel.cmake = false +``` + ## Any matching condition If you use `if.any` instead of `if`, then the override is true if any one of the diff --git a/src/scikit_build_core/build/_init.py b/src/scikit_build_core/build/_init.py index e295922c..06153399 100644 --- a/src/scikit_build_core/build/_init.py +++ b/src/scikit_build_core/build/_init.py @@ -15,9 +15,9 @@ def __dir__() -> list[str]: @functools.lru_cache(1) def setup_logging(log_level: str) -> None: level_value = LEVEL_VALUE[log_level] + logger.setLevel(level_value) ch = logging.StreamHandler() - ch.setLevel(level_value) # create formatter and add it to the handlers formatter = logging.Formatter( "%(asctime)s - %(name)s - %(levelname)s - %(message)s" diff --git a/src/scikit_build_core/build/wheel.py b/src/scikit_build_core/build/wheel.py index 8deb4e7c..aed79661 100644 --- a/src/scikit_build_core/build/wheel.py +++ b/src/scikit_build_core/build/wheel.py @@ -7,7 +7,7 @@ import sysconfig import tempfile from pathlib import Path -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Any from packaging.requirements import Requirement from packaging.utils import canonicalize_name @@ -15,11 +15,12 @@ from .. import __version__ from .._compat import tomllib from .._compat.typing import Literal, assert_never -from .._logging import logger, rich_print +from .._logging import LEVEL_VALUE, logger, rich_print from .._shutil import fix_win_37_all_permissions from ..builder.builder import Builder, archs_to_tags, get_archs from ..builder.wheel_tag import WheelTag from ..cmake import CMake, CMaker +from ..errors import FailedLiveProcessError from ..settings.skbuild_read_settings import SettingsReader from ._editable import editable_redirect, libdir_to_installed, mapping_to_modules from ._init import setup_logging @@ -123,6 +124,7 @@ def _build_wheel_impl( ) -> WheelImplReturn: """ Build a wheel or just prepare metadata (if wheel dir is None). Can be editable. + Handles one retry attempt if "failed" override present. """ state: Literal["sdist", "wheel", "editable", "metadata_wheel", "metadata_editable"] if exit_after_config: @@ -136,9 +138,10 @@ def _build_wheel_impl( with pyproject_path.open("rb") as ft: pyproject = tomllib.load(ft) - settings_reader = SettingsReader(pyproject, config_settings or {}, state=state) - settings = settings_reader.settings - setup_logging(settings.logging.level) + settings_reader = SettingsReader( + pyproject, config_settings or {}, state=state, retry=False + ) + setup_logging(settings_reader.settings.logging.level) settings_reader.validate_may_exit() @@ -156,6 +159,56 @@ def _build_wheel_impl( "ninja should not be in build-system.requires - scikit-build-core will inject it as needed" ) + try: + return _build_wheel_impl_impl( + wheel_directory, + metadata_directory, + exit_after_config=exit_after_config, + editable=editable, + state=state, + settings=settings_reader.settings, + pyproject=pyproject, + ) + except FailedLiveProcessError as err: + settings_reader = SettingsReader( + pyproject, config_settings or {}, state=state, retry=True + ) + if "failed" not in settings_reader.overrides: + raise + + rich_print( + f"\n[yellow bold]*** {' '.join(err.args)} - retrying due to override..." + ) + + logger.setLevel(LEVEL_VALUE[settings_reader.settings.logging.level]) + + settings_reader.validate_may_exit() + + return _build_wheel_impl_impl( + wheel_directory, + metadata_directory, + exit_after_config=exit_after_config, + editable=editable, + state=state, + settings=settings_reader.settings, + pyproject=pyproject, + ) + + +def _build_wheel_impl_impl( + wheel_directory: str | None, + metadata_directory: str | None, + *, + exit_after_config: bool = False, + editable: bool, + state: Literal["sdist", "wheel", "editable", "metadata_wheel", "metadata_editable"], + settings: ScikitBuildSettings, + pyproject: dict[str, Any], +) -> WheelImplReturn: + """ + Build a wheel or just prepare metadata (if wheel dir is None). Can be editable. + """ + metadata = get_standard_metadata(pyproject, settings) if metadata.version is None: diff --git a/src/scikit_build_core/resources/scikit-build.schema.json b/src/scikit_build_core/resources/scikit-build.schema.json index 25c48e9f..8255f0cb 100644 --- a/src/scikit_build_core/resources/scikit-build.schema.json +++ b/src/scikit_build_core/resources/scikit-build.schema.json @@ -628,6 +628,10 @@ "type": "boolean", "description": "Whether the build is from an sdist." }, + "failed": { + "type": "boolean", + "description": "Matches if the build fails. A build will be retried if there is at least one matching override with this set to true." + }, "env": { "type": "object", "patternProperties": { diff --git a/src/scikit_build_core/settings/skbuild_read_settings.py b/src/scikit_build_core/settings/skbuild_read_settings.py index f59697ef..dde50fd7 100644 --- a/src/scikit_build_core/settings/skbuild_read_settings.py +++ b/src/scikit_build_core/settings/skbuild_read_settings.py @@ -66,12 +66,12 @@ def regex_match(value: str, match: str) -> str: def override_match( *, - match_all: bool, current_env: Mapping[str, str] | None, current_state: Literal[ "sdist", "wheel", "editable", "metadata_wheel", "metadata_editable" ], has_dist_info: bool, + retry: bool, python_version: str | None = None, implementation_name: str | None = None, implementation_version: str | None = None, @@ -81,12 +81,15 @@ def override_match( env: dict[str, str] | None = None, state: str | None = None, from_sdist: bool | None = None, -) -> bool: - # This makes a matches list. A match is a string; if it's an empty string, - # then it did not match. If it's not empty, it did match, and the string - # will be printed in the logs - it's the match reason. If match_all is True, - # then all must match, otherwise any match will count. - matches = [] + failed: bool | None = None, +) -> tuple[dict[str, str], set[str]]: + """ + Check if the current environment matches the overrides. Returns a dict + of passed matches, with reasons for values, and a set of non-matches. + """ + + passed_dict = {} + failed_set: set[str] = set() if current_env is None: current_env = os.environ @@ -94,12 +97,18 @@ def override_match( if python_version is not None: current_python_version = ".".join(str(x) for x in sys.version_info[:2]) match_msg = version_match(current_python_version, python_version, "Python") - matches.append(match_msg) + if match_msg: + passed_dict["python-version"] = match_msg + else: + failed_set.add("python-version") if implementation_name is not None: current_impementation_name = sys.implementation.name match_msg = regex_match(current_impementation_name, implementation_name) - matches.append(match_msg) + if match_msg: + passed_dict["implementation-name"] = match_msg + else: + failed_set.add("implementation-name") if implementation_version is not None: info = sys.implementation.version @@ -110,61 +119,84 @@ def override_match( match_msg = version_match( version, implementation_version, "Python implementation" ) - matches.append(match_msg) + if match_msg: + passed_dict["implementation-version"] = match_msg + else: + failed_set.add("implementation-version") if platform_system is not None: current_platform_system = sys.platform match_msg = regex_match(current_platform_system, platform_system) - matches.append(match_msg) + if match_msg: + passed_dict["platform-system"] = match_msg + else: + failed_set.add("platform-system") if platform_machine is not None: current_platform_machine = platform.machine() match_msg = regex_match(current_platform_machine, platform_machine) - matches.append(match_msg) + if match_msg: + passed_dict["platform-machine"] = match_msg + else: + failed_set.add("platform-machine") if platform_node is not None: current_platform_node = platform.node() match_msg = regex_match(current_platform_node, platform_node) - matches.append(match_msg) + if match_msg: + passed_dict["platform-node"] = match_msg + else: + failed_set.add("platform-node") if state is not None: match_msg = regex_match(current_state, state) - matches.append(match_msg) + if match_msg: + passed_dict["state"] = match_msg + else: + failed_set.add("state") + + if failed is not None: + if failed and retry: + passed_dict["failed"] = "Previous run failed" + elif not failed and not retry: + passed_dict["failed"] = "Running on a fresh run" + else: + failed_set.add("failed") if from_sdist is not None: if has_dist_info: - matches.append("from sdist due to PKG-INFO" if from_sdist else "") + if from_sdist: + passed_dict["from-sdist"] = "from sdist due to PKG-INFO" + else: + failed_set.add("from-sdist") + elif not from_sdist: + passed_dict["from-sdist"] = "not from sdist, no PKG-INFO" else: - matches.append("" if from_sdist else "not from sdist, no PKG-INFO") + failed_set.add("from-sdist") if env: for key, value in env.items(): - if isinstance(value, bool): - matches.append( - f"env {key} is {value}" - if strtobool(current_env.get(key, "")) == value - else "" - ) - elif key not in current_env: - matches.append("") + if key not in current_env: + failed_set.add(f"env.{key}") + elif isinstance(value, bool): + if strtobool(current_env[key]) == value: + passed_dict[f"env.{key}"] = f"env {key} is {value}" + else: + failed_set.add(f"env.{key}") else: - current_value = current_env.get(key, "") + current_value = current_env[key] match_msg = regex_match(current_value, value) - matches.append(match_msg and f"env {key}: {match_msg}") - if not matches: + if match_msg: + passed_dict[f"env.{key}"] = f"env {key}: {match_msg}" + else: + failed_set.add(f"env.{key}") + + if not passed_dict and not failed_set: msg = "At least one override must be provided" raise ValueError(msg) - if match_all: - matched = all(matches) - if matched: - logger.info("Overrides {}", " and ".join(matches)) - else: - matched = any(matches) - if matched: - logger.info("Overrides {}", " or ".join([m for m in matches if m])) - return matched + return passed_dict, failed_set def _handle_minimum_version( @@ -273,17 +305,22 @@ def inherit_join( def process_overides( tool_skb: dict[str, Any], + *, state: Literal["sdist", "wheel", "editable", "metadata_wheel", "metadata_editable"], + retry: bool, env: Mapping[str, str] | None = None, -) -> None: +) -> set[str]: """ Process overrides into the main dictionary if they match. Modifies the input dictionary. Must be run from the package directory. """ has_dist_info = Path("PKG-INFO").is_file() + global_matched: set[str] = set() for override in tool_skb.pop("overrides", []): - matched = True + passed_any: dict[str, str] | None = None + passed_all: dict[str, str] | None = None + failed: set[str] = set() if_override = override.pop("if", None) if not if_override: msg = "At least one 'if' override must be provided" @@ -294,11 +331,11 @@ def process_overides( if "any" in if_override: any_override = if_override.pop("any") select = {k.replace("-", "_"): v for k, v in any_override.items()} - matched = override_match( - match_all=False, + passed_any, _ = override_match( current_env=env, current_state=state, has_dist_info=has_dist_info, + retry=retry, **select, ) @@ -309,14 +346,37 @@ def process_overides( select = {k.replace("-", "_"): v for k, v in if_override.items()} if select: - matched = matched and override_match( - match_all=True, + passed_all, failed = override_match( current_env=env, current_state=state, has_dist_info=has_dist_info, + retry=retry, **select, ) - if matched: + + # If no overrides are passed, do nothing + if passed_any is None and passed_all is None: + continue + + # If normal overrides are passed and one or more fails, do nothing + if passed_all is not None and failed: + continue + + # If any is passed, at least one always needs to pass. + if passed_any is not None and not passed_any: + continue + + local_matched = set(passed_any or []) | set(passed_all or []) + global_matched |= local_matched + if local_matched: + all_str = " and ".join( + [ + *(passed_all or {}).values(), + *([" or ".join(passed_any.values())] if passed_any else []), + ] + ) + logger.info("Overrides {}", all_str) + for key, value in override.items(): inherit1 = inherit_override.get(key, {}) if isinstance(value, dict): @@ -334,6 +394,7 @@ def process_overides( tool_skb[key] = inherit_join( value, tool_skb.get(key), inherit_override_tmp ) + return global_matched class SettingsReader: @@ -348,12 +409,18 @@ def __init__( extra_settings: Mapping[str, Any] | None = None, verify_conf: bool = True, env: Mapping[str, str] | None = None, + retry: bool = False, ) -> None: self.state = state # Handle overrides pyproject = copy.deepcopy(pyproject) - process_overides(pyproject.get("tool", {}).get("scikit-build", {}), state, env) + self.overrides = process_overides( + pyproject.get("tool", {}).get("scikit-build", {}), + state=state, + env=env, + retry=retry, + ) # Support for minimum-version='build-system.requires' tmp_min_v = ( @@ -385,7 +452,7 @@ def __init__( if extra_settings is not None: extra_skb = copy.deepcopy(dict(extra_settings)) - process_overides(extra_skb, state, env) + process_overides(extra_skb, state=state, env=env, retry=retry) toml_srcs.insert(0, TOMLSource(settings=extra_skb)) prefixed = { @@ -561,6 +628,7 @@ def from_file( verify_conf: bool = True, extra_settings: Mapping[str, Any] | None = None, env: Mapping[str, str] | None = None, + retry: bool = False, ) -> SettingsReader: with Path(pyproject_path).open("rb") as f: pyproject = tomllib.load(f) @@ -572,4 +640,5 @@ def from_file( state=state, extra_settings=extra_settings, env=env, + retry=retry, ) diff --git a/src/scikit_build_core/settings/skbuild_schema.py b/src/scikit_build_core/settings/skbuild_schema.py index 6062d774..797b3369 100644 --- a/src/scikit_build_core/settings/skbuild_schema.py +++ b/src/scikit_build_core/settings/skbuild_schema.py @@ -120,6 +120,10 @@ def generate_skbuild_schema(tool_name: str = "scikit-build") -> dict[str, Any]: "type": "boolean", "description": "Whether the build is from an sdist.", }, + "failed": { + "type": "boolean", + "description": "Matches if the build fails. A build will be retried if there is at least one matching override with this set to true.", + }, "env": { "type": "object", "patternProperties": { diff --git a/tests/conftest.py b/tests/conftest.py index ad2d7c64..6a445675 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -302,6 +302,15 @@ def navigate_editable(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Packag return package +@pytest.fixture() +def broken_fallback(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> PackageInfo: + package = PackageInfo( + "broken_fallback", + ) + process_package(package, tmp_path, monkeypatch) + return package + + @pytest.fixture() def package_sdist_config( tmp_path: Path, monkeypatch: pytest.MonkeyPatch diff --git a/tests/packages/broken_fallback/CMakeLists.txt b/tests/packages/broken_fallback/CMakeLists.txt new file mode 100644 index 00000000..a0f12eee --- /dev/null +++ b/tests/packages/broken_fallback/CMakeLists.txt @@ -0,0 +1,18 @@ +cmake_minimum_required(VERSION 3.15...3.26) +project(${SKBUILD_PROJECT_NAME} LANGUAGES C) + +if(DEFINED BROKEN_CMAKE) + message(FATAL_ERROR "Broken CMake") +endif() + +find_package( + Python + COMPONENTS Interpreter Development.Module + REQUIRED) + +python_add_library(example MODULE main.c WITH_SOABI) +if(DEFINED BROKEN_CODE) + target_compile_definitions(example PRIVATE BROKEN_CODE) +endif() + +install(TARGETS example DESTINATION .) diff --git a/tests/packages/broken_fallback/main.c b/tests/packages/broken_fallback/main.c new file mode 100644 index 00000000..fb7b8d26 --- /dev/null +++ b/tests/packages/broken_fallback/main.c @@ -0,0 +1,29 @@ +#define PY_SSIZE_T_CLEAN +#include + +#ifdef BROKEN_CODE +#error "Broken code" +#endif + +float square(float x) { return x * x; } + +static PyObject *square_wrapper(PyObject *self, PyObject *args) { + float input, result; + if (!PyArg_ParseTuple(args, "f", &input)) { + return NULL; + } + result = square(input); + return PyFloat_FromDouble(result); +} + +static PyMethodDef example_methods[] = { + {"square", square_wrapper, METH_VARARGS, "Square function"}, + {NULL, NULL, 0, NULL}}; + +static struct PyModuleDef example_module = {PyModuleDef_HEAD_INIT, "example", + NULL, -1, example_methods}; + +/* name here must match extension name, with PyInit_ prefix */ +PyMODINIT_FUNC PyInit_example(void) { + return PyModule_Create(&example_module); +} diff --git a/tests/packages/broken_fallback/pyproject.toml b/tests/packages/broken_fallback/pyproject.toml new file mode 100644 index 00000000..8ec62201 --- /dev/null +++ b/tests/packages/broken_fallback/pyproject.toml @@ -0,0 +1,14 @@ +[build-system] +requires = ["scikit-build-core"] +build-backend = "scikit_build_core.build" + +[project] +name = "broken_fallback" +version = "0.0.1" + +[tool.scikit-build] +wheel.license-files = [] + +[[tool.scikit-build.overrides]] +if.failed = true +wheel.cmake = false diff --git a/tests/test_broken_fallback.py b/tests/test_broken_fallback.py new file mode 100644 index 00000000..12cf11d0 --- /dev/null +++ b/tests/test_broken_fallback.py @@ -0,0 +1,36 @@ +import zipfile +from pathlib import Path + +import pytest + +from scikit_build_core.build import ( + build_wheel, +) + + +@pytest.mark.compile() +@pytest.mark.configure() +@pytest.mark.usefixtures("broken_fallback") +@pytest.mark.parametrize("broken_define", ["BROKEN_CMAKE", "BROKEN_CODE"]) +def test_broken_code(broken_define: str, capfd: pytest.CaptureFixture[str]): + build_wheel("dist", {f"cmake.define.{broken_define}": "1"}) + wheel = Path("dist") / "broken_fallback-0.0.1-py3-none-any.whl" + with zipfile.ZipFile(wheel) as f: + file_names = set(f.namelist()) + + assert file_names == { + "broken_fallback-0.0.1.dist-info/RECORD", + "broken_fallback-0.0.1.dist-info/WHEEL", + "broken_fallback-0.0.1.dist-info/METADATA", + } + + out, err = capfd.readouterr() + assert "retrying due to override..." in out + + if broken_define == "BROKEN_CMAKE": + assert "Broken CMake" in err + assert "CMake Error at CMakeLists.txt" in err + assert "CMake configuration failed" in out + else: + assert "Broken code" in out + assert "CMake build failed" in out diff --git a/tests/test_settings_overrides.py b/tests/test_settings_overrides.py index 8e93dfea..3baad1d7 100644 --- a/tests/test_settings_overrides.py +++ b/tests/test_settings_overrides.py @@ -11,6 +11,13 @@ from pathlib import Path +class VersionInfo(typing.NamedTuple): + major: int + minor: int + micro: int + releaselevel: str = "final" + + @pytest.mark.parametrize("python_version", ["3.9", "3.10"]) def test_skbuild_overrides_pyver( python_version: str, tmp_path: Path, monkeypatch: pytest.MonkeyPatch @@ -43,7 +50,37 @@ def test_skbuild_overrides_pyver( assert not settings.cmake.args assert not settings.cmake.define assert not settings.experimental - assert not settings.sdist.cmake + + +@pytest.mark.parametrize("implementation_version", ["7.3.14", "7.3.15"]) +def test_skbuild_overrides_implver( + implementation_version: str, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +): + monkeypatch.setattr("sys.implementation.name", "pypy") + monkeypatch.setattr( + "sys.implementation.version", + VersionInfo(*(int(x) for x in implementation_version.split("."))), # type: ignore[arg-type] + ) + pyproject_toml = tmp_path / "pyproject.toml" + pyproject_toml.write_text( + dedent( + """\ + [[tool.scikit-build.overrides]] + if.implementation-name = "pypy" + if.implementation-version = ">=7.3.15" + experimental = true + """ + ), + encoding="utf-8", + ) + + settings_reader = SettingsReader.from_file(pyproject_toml) + settings = settings_reader.settings + + if implementation_version == "7.3.15": + assert settings.experimental + else: + assert not settings.experimental @pytest.mark.parametrize("implementation_name", ["cpython", "pypy"]) @@ -79,6 +116,7 @@ def test_skbuild_overrides_dual( settings_reader = SettingsReader.from_file(pyproject_toml) settings = settings_reader.settings + print(settings_reader.overrides) if implementation_name == "pypy" and platform_system == "darwin": assert not settings.editable.verbose @@ -530,3 +568,37 @@ def test_skbuild_overrides_from_sdist( assert settings.wheel.cmake != from_sdist assert settings.sdist.cmake == from_sdist + + +def test_failed_retry(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + pyproject_toml = tmp_path / "pyproject.toml" + pyproject_toml.write_text( + dedent( + """\ + [tool.scikit-build] + wheel.cmake = false + sdist.cmake = false + + [[tool.scikit-build.overrides]] + if.failed = true + wheel.cmake = true + + [[tool.scikit-build.overrides]] + if.failed = false + sdist.cmake = true + """ + ), + encoding="utf-8", + ) + + monkeypatch.chdir(tmp_path) + + settings_reader = SettingsReader.from_file(pyproject_toml, retry=False) + settings = settings_reader.settings + assert not settings.wheel.cmake + assert settings.sdist.cmake + + settings_reader = SettingsReader.from_file(pyproject_toml, retry=True) + settings = settings_reader.settings + assert settings.wheel.cmake + assert not settings.sdist.cmake