diff --git a/README.md b/README.md index 8bab031c6..9f5093614 100644 --- a/README.md +++ b/README.md @@ -286,9 +286,6 @@ minimum-version = "0.11" # current version # The CMake build directory. Defaults to a unique temporary directory. build-dir = "" -# Immediately fail the build. This is only useful in overrides. -fail = false - ``` diff --git a/docs/reference/configs.md b/docs/reference/configs.md index 8c6e8b857..fcaa30bd9 100644 --- a/docs/reference/configs.md +++ b/docs/reference/configs.md @@ -78,7 +78,6 @@ print(mk_skbuild_docs()) ```{eval-rst} .. confval:: fail :type: ``bool`` - :default: false Immediately fail the build. This is only useful in overrides. ``` diff --git a/src/scikit_build_core/resources/scikit-build.schema.json b/src/scikit_build_core/resources/scikit-build.schema.json index 5065b4153..322a9ea0d 100644 --- a/src/scikit_build_core/resources/scikit-build.schema.json +++ b/src/scikit_build_core/resources/scikit-build.schema.json @@ -491,7 +491,6 @@ }, "fail": { "type": "boolean", - "default": false, "description": "Immediately fail the build. This is only useful in overrides." }, "overrides": { diff --git a/src/scikit_build_core/settings/documentation.py b/src/scikit_build_core/settings/documentation.py index 2d7ea0dac..67a736e2e 100644 --- a/src/scikit_build_core/settings/documentation.py +++ b/src/scikit_build_core/settings/documentation.py @@ -56,6 +56,7 @@ class DCDoc: docs: str field: dataclasses.Field[typing.Any] deprecated: bool = False + override_only: bool = False def sanitize_default_field(text: str) -> str: @@ -134,4 +135,5 @@ def mk_docs(dc: type[object], prefix: str = "") -> Generator[DCDoc, None, None]: docs=docs[field.name], field=field, deprecated=field.metadata.get("deprecated", False), + override_only=field.metadata.get("override_only", False), ) diff --git a/src/scikit_build_core/settings/skbuild_docs_readme.py b/src/scikit_build_core/settings/skbuild_docs_readme.py index 4bf45c65e..b731cdf90 100644 --- a/src/scikit_build_core/settings/skbuild_docs_readme.py +++ b/src/scikit_build_core/settings/skbuild_docs_readme.py @@ -46,7 +46,11 @@ def mk_skbuild_docs() -> str: Makes documentation for the skbuild model. """ doc = Document( - [Item(item) for item in mk_docs(ScikitBuildSettings) if not item.deprecated] + [ + Item(item) + for item in mk_docs(ScikitBuildSettings) + if not item.deprecated and not item.override_only + ] ) return doc.format() diff --git a/src/scikit_build_core/settings/skbuild_model.py b/src/scikit_build_core/settings/skbuild_model.py index a83abd2f8..953c18e31 100644 --- a/src/scikit_build_core/settings/skbuild_model.py +++ b/src/scikit_build_core/settings/skbuild_model.py @@ -33,6 +33,8 @@ def __dir__() -> List[str]: class SettingsFieldMetadata(TypedDict, total=False): display_default: Optional[str] deprecated: bool + override_only: bool + """Do not allow the field to be a top-level table.""" class CMakeSettingsDefine(str): @@ -505,7 +507,12 @@ class ScikitBuildSettings: This can be set to reuse the build directory from previous runs. """ - fail: bool = False + fail: Optional[bool] = dataclasses.field( + default=None, + metadata=SettingsFieldMetadata( + override_only=True, + ), + ) """ Immediately fail the build. This is only useful in overrides. """ diff --git a/src/scikit_build_core/settings/skbuild_overrides.py b/src/scikit_build_core/settings/skbuild_overrides.py index 53ec87edb..cea43e771 100644 --- a/src/scikit_build_core/settings/skbuild_overrides.py +++ b/src/scikit_build_core/settings/skbuild_overrides.py @@ -1,5 +1,6 @@ from __future__ import annotations +import dataclasses import os import platform import re @@ -18,7 +19,7 @@ from ..errors import CMakeNotFoundError from ..resources import resources -__all__ = ["process_overrides", "regex_match"] +__all__ = ["OverrideRecord", "process_overrides", "regex_match"] def __dir__() -> list[str]: @@ -29,6 +30,34 @@ def __dir__() -> list[str]: from collections.abc import Mapping +@dataclasses.dataclass +class OverrideRecord: + """ + Record of the override action. + + Saves the original and final values, and the override reasons. + """ + + key: str + """Settings key that is overridden.""" + + original_value: Any | None + """ + Original value in the pyproject table. + + If the pyproject table did not have the key, this is a ``None``. + """ + + value: Any + """Final value.""" + + passed_all: dict[str, str] | None + """All if statements that passed (except the effective ``match_any``).""" + + passed_any: dict[str, str] | None + """All if.any statements that passed.""" + + def strtobool(value: str) -> bool: """ Converts a environment variable string into a boolean value. @@ -257,20 +286,72 @@ def inherit_join( raise TypeError(msg) +def record_override( + *keys: str, + value: Any, + tool_skb: dict[str, Any], + overriden_items: dict[str, OverrideRecord], + passed_all: dict[str, str] | None, + passed_any: dict[str, str] | None, +) -> None: + full_key = ".".join(keys) + # Get the original_value to construct the record + if full_key in overriden_items: + # We found the original value from a previous override record + original_value = overriden_items[full_key].original_value + else: + # Otherwise navigate the original pyproject table until we resolved all keys + _dict_or_value = tool_skb + keys_list = [*keys] + while keys_list: + k = keys_list.pop(0) + if k not in _dict_or_value: + # We hit a dead end so we imply the original_value was not set (`None`) + original_value = None + break + _dict_or_value = _dict_or_value[k] + if isinstance(_dict_or_value, dict): + # If the value is a dict it is either the final value or we continue + # to navigate it + continue + # Otherwise it should be the final value + original_value = _dict_or_value + if keys_list: + msg = f"Could not navigate to the key {full_key} because {k} is a {type(_dict_or_value)}" + raise TypeError(msg) + break + else: + # We exhausted all keys so the current value should be the table key we are + # interested in + original_value = _dict_or_value + # Now save the override record + overriden_items[full_key] = OverrideRecord( + key=keys[-1], + original_value=original_value, + value=value, + passed_any=passed_any, + passed_all=passed_all, + ) + + def process_overrides( tool_skb: dict[str, Any], *, state: Literal["sdist", "wheel", "editable", "metadata_wheel", "metadata_editable"], retry: bool, env: Mapping[str, str] | None = None, -) -> set[str]: +) -> tuple[set[str], dict[str, OverrideRecord]]: """ Process overrides into the main dictionary if they match. Modifies the input dictionary. Must be run from the package directory. + + :return: A tuple of the set of matching overrides and a dict of changed keys and + override record """ has_dist_info = Path("PKG-INFO").is_file() global_matched: set[str] = set() + overriden_items: dict[str, OverrideRecord] = {} for override in tool_skb.pop("overrides", []): passed_any: dict[str, str] | None = None passed_all: dict[str, str] | None = None @@ -354,6 +435,14 @@ def process_overrides( inherit1 = inherit_override.get(key, {}) if isinstance(value, dict): for key2, value2 in value.items(): + record_override( + *[key, key2], + value=value, + tool_skb=tool_skb, + overriden_items=overriden_items, + passed_all=passed_all, + passed_any=passed_any, + ) inherit2 = inherit1.get(key2, "none") inner = tool_skb.get(key, {}) inner[key2] = inherit_join( @@ -361,10 +450,18 @@ def process_overrides( ) tool_skb[key] = inner else: + record_override( + key, + value=value, + tool_skb=tool_skb, + overriden_items=overriden_items, + passed_all=passed_all, + passed_any=passed_any, + ) inherit_override_tmp = inherit_override or "none" if isinstance(inherit_override_tmp, dict): assert not inherit_override_tmp tool_skb[key] = inherit_join( value, tool_skb.get(key), inherit_override_tmp ) - return global_matched + return global_matched, overriden_items diff --git a/src/scikit_build_core/settings/skbuild_read_settings.py b/src/scikit_build_core/settings/skbuild_read_settings.py index 33e4dfba4..e0b5ede87 100644 --- a/src/scikit_build_core/settings/skbuild_read_settings.py +++ b/src/scikit_build_core/settings/skbuild_read_settings.py @@ -24,6 +24,8 @@ import os from collections.abc import Generator, Mapping + from .skbuild_overrides import OverrideRecord + __all__ = ["SettingsReader"] @@ -133,6 +135,57 @@ def _handle_move( return before +def _validate_overrides( + settings: ScikitBuildSettings, + overrides: dict[str, OverrideRecord], +) -> None: + """Validate all fields with any override information.""" + + def validate_field( + field: dataclasses.Field[Any], + value: Any, + prefix: str = "", + record: OverrideRecord | None = None, + ) -> None: + """Do the actual validation.""" + # Check if we had a hard-coded value in the record + conf_key = field.name.replace("_", "-") + if field.metadata.get("override_only", False): + original_value = record.original_value if record else value + if original_value is not None: + msg = f"{prefix}{conf_key} is not allowed to be hard-coded in the pyproject.toml file" + if settings.strict_config: + sys.stdout.flush() + rich_print(f"{{bold.red}}ERROR:{{normal}} {msg}") + raise SystemExit(7) + logger.warning(msg) + + def validate_field_recursive( + obj: Any, + record: OverrideRecord | None = None, + prefix: str = "", + ) -> None: + """Navigate through all the keys and validate each field.""" + for field in dataclasses.fields(obj): + conf_key = field.name.replace("_", "-") + closest_record = overrides.get(f"{prefix}{conf_key}", record) + value = getattr(obj, field.name) + # Do the validation of the current field + validate_field( + field=field, + value=value, + prefix=prefix, + record=closest_record, + ) + if dataclasses.is_dataclass(value): + validate_field_recursive( + obj=value, record=closest_record, prefix=f"{prefix}{conf_key}." + ) + + # Navigate all fields starting from the top-level + validate_field_recursive(obj=settings) + + class SettingsReader: def __init__( self, @@ -151,7 +204,7 @@ def __init__( # Handle overrides pyproject = copy.deepcopy(pyproject) - self.overrides = process_overrides( + self.overrides, self.overriden_items = process_overrides( pyproject.get("tool", {}).get("scikit-build", {}), state=state, env=env, @@ -352,6 +405,7 @@ def validate_may_exit(self) -> None: self.print_suggestions() raise SystemExit(7) logger.warning("Unrecognized options: {}", ", ".join(unrecognized)) + _validate_overrides(self.settings, self.overriden_items) for key, value in self.settings.metadata.items(): if "provider" not in value: diff --git a/tests/test_settings_docs.py b/tests/test_settings_docs.py index 8c8da9613..8ae98ad18 100644 --- a/tests/test_settings_docs.py +++ b/tests/test_settings_docs.py @@ -18,7 +18,7 @@ def test_skbuild_docs_readme() -> None: "A table of defines to pass to CMake when configuring the project. Additive." in docs ) - assert "fail = false" in docs + assert "fail = " not in docs # Deprecated items are not included here assert "ninja.minimum-version" not in docs diff --git a/tests/test_settings_overrides.py b/tests/test_settings_overrides.py index 098414a52..c4d3cec14 100644 --- a/tests/test_settings_overrides.py +++ b/tests/test_settings_overrides.py @@ -1,5 +1,6 @@ from __future__ import annotations +import logging import sysconfig import typing from pathlib import Path @@ -22,6 +23,53 @@ class VersionInfo(typing.NamedTuple): releaselevel: str = "final" +def test_disallow_hardcoded( + tmp_path: Path, + caplog: pytest.LogCaptureFixture, + capsys: pytest.CaptureFixture[str], +): + caplog.set_level(logging.WARNING) + pyproject_toml = tmp_path / "pyproject.toml" + template = dedent( + """\ + [tool.scikit-build] + strict-config = {strict_config} + fail = false + """ + ) + + # First check without strict-config to make sure all fields are disallowed + strict_config = "false" + pyproject_toml.write_text( + template.format(strict_config=strict_config), + encoding="utf-8", + ) + + settings_reader = SettingsReader.from_file(pyproject_toml) + settings_reader.validate_may_exit() + assert caplog.records + for idx, key in enumerate(["fail"]): + assert ( + f"{key} is not allowed to be hard-coded in the pyproject.toml file" + in str(caplog.records[idx].msg) + ) + + # Next check that this exits if string-config is set + strict_config = "true" + pyproject_toml.write_text( + template.format(strict_config=strict_config), + encoding="utf-8", + ) + # Flush the capsys just in case + capsys.readouterr() + settings_reader = SettingsReader.from_file(pyproject_toml) + with pytest.raises(SystemExit) as exc: + settings_reader.validate_may_exit() + assert exc.value.code == 7 + out, _ = capsys.readouterr() + assert "is not allowed to be hard-coded in the pyproject.toml file" in out + + @pytest.mark.parametrize("python_version", ["3.9", "3.10"]) def test_skbuild_overrides_pyver( python_version: str, tmp_path: Path, monkeypatch: pytest.MonkeyPatch