From 5291db77d2ce7faa1591d86a66ceadcbfa445f41 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Fri, 19 Jul 2024 17:39:37 -0400 Subject: [PATCH] feat: if failed (retry) Signed-off-by: Henry Schreiner --- docs/overrides.md | 13 ++++ src/scikit_build_core/build/_init.py | 2 +- src/scikit_build_core/build/wheel.py | 63 ++++++++++++++-- .../resources/scikit-build.schema.json | 4 + .../settings/skbuild_read_settings.py | 73 +++++++++++++++++-- .../settings/skbuild_schema.py | 4 + tests/conftest.py | 9 +++ tests/packages/broken_fallback/CMakeLists.txt | 18 +++++ tests/packages/broken_fallback/main.c | 29 ++++++++ tests/packages/broken_fallback/pyproject.toml | 14 ++++ tests/test_broken_fallback.py | 36 +++++++++ tests/test_settings_overrides.py | 1 + 12 files changed, 252 insertions(+), 14 deletions(-) create mode 100644 tests/packages/broken_fallback/CMakeLists.txt create mode 100644 tests/packages/broken_fallback/main.c create mode 100644 tests/packages/broken_fallback/pyproject.toml create mode 100644 tests/test_broken_fallback.py 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..96f759ac 100644 --- a/src/scikit_build_core/settings/skbuild_read_settings.py +++ b/src/scikit_build_core/settings/skbuild_read_settings.py @@ -72,6 +72,7 @@ def override_match( "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,13 +82,19 @@ def override_match( env: dict[str, str] | None = None, state: str | None = None, from_sdist: bool | None = None, -) -> bool: + failed: bool | None = None, +) -> set[str]: + """ + Check if the current environment matches the overrides. Returns the set of matched keys. + """ # 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 = [] + passed_keys = set() + if current_env is None: current_env = os.environ @@ -95,11 +102,15 @@ def override_match( 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_keys.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_keys.add("implementation-name") if implementation_version is not None: info = sys.implementation.version @@ -111,25 +122,45 @@ def override_match( version, implementation_version, "Python implementation" ) matches.append(match_msg) + if match_msg: + passed_keys.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_keys.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_keys.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_keys.add("platform-node") if state is not None: match_msg = regex_match(current_state, state) matches.append(match_msg) + if match_msg: + passed_keys.add("state") + + if failed is not None: + if failed and retry: + matches.append("Previous run failed") + passed_keys.add("failed") + elif not failed and not retry: + matches.append("Running on a fresh run") + passed_keys.add("failed") + else: + matches.append("") if from_sdist is not None: if has_dist_info: @@ -140,17 +171,21 @@ def override_match( if env: for key, value in env.items(): if isinstance(value, bool): - matches.append( + match_msg = ( f"env {key} is {value}" if strtobool(current_env.get(key, "")) == value else "" ) + matches.append(match_msg) + passed_keys.add(f"env.{key}") elif key not in current_env: matches.append("") else: current_value = current_env.get(key, "") match_msg = regex_match(current_value, value) matches.append(match_msg and f"env {key}: {match_msg}") + if match_msg: + passed_keys.add(f"env.{key}") if not matches: msg = "At least one override must be provided" @@ -164,7 +199,8 @@ def override_match( matched = any(matches) if matched: logger.info("Overrides {}", " or ".join([m for m in matches if m])) - return matched + + return passed_keys if matched else set() def _handle_minimum_version( @@ -273,17 +309,20 @@ 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 + matched: set[str] | None = None if_override = override.pop("if", None) if not if_override: msg = "At least one 'if' override must be provided" @@ -299,6 +338,7 @@ def process_overides( current_env=env, current_state=state, has_dist_info=has_dist_info, + retry=retry, **select, ) @@ -309,13 +349,23 @@ def process_overides( select = {k.replace("-", "_"): v for k, v in if_override.items()} if select: - matched = matched and override_match( + each_matched = override_match( match_all=True, current_env=env, current_state=state, has_dist_info=has_dist_info, + retry=retry, **select, ) + if matched is None: + matched = each_matched + elif matched and each_matched: + matched |= each_matched + else: + matched = set() + if matched is None: + matched = set() + global_matched |= matched if matched: for key, value in override.items(): inherit1 = inherit_override.get(key, {}) @@ -334,6 +384,7 @@ def process_overides( tool_skb[key] = inherit_join( value, tool_skb.get(key), inherit_override_tmp ) + return global_matched class SettingsReader: @@ -348,12 +399,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 +442,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 = { 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..b15308a8 100644 --- a/tests/test_settings_overrides.py +++ b/tests/test_settings_overrides.py @@ -79,6 +79,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