feat: add a setting to disallow hard-coding some setting keys in the pyproject.toml (#1078)

This complements the discussion in #1050 that we should not allow some
fields to be hard-coded. We should allow them in the context of
overrides, so here is a rather convoluted way of adding that check.

This will also make it possible to extend it further if we want more
specific checks for specific overrides matchers as well. E.g. we can add
a restriction that `build.requires` cannot have `@` in an `sdist`

---------

Signed-off-by: Cristian Le <git@lecris.dev>
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
Signed-off-by: Henry Schreiner <henryfs@princeton.edu>
Co-authored-by: Henry Schreiner <henryschreineriii@gmail.com>
Co-authored-by: Henry Schreiner <henryfs@princeton.edu>

Author: LecrisUT

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
-
 ```
 
 <!-- [[[end]]] -->

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.
 ```

src/scikit_build_core/resources/scikit-build.schema.json

@@ -489,11 +489,6 @@
       "default": "",
       "description": "The CMake build directory. Defaults to a unique temporary directory."
     },
-    "fail": {
-      "type": "boolean",
-      "default": false,
-      "description": "Immediately fail the build. This is only useful in overrides."
-    },
     "overrides": {
       "type": "array",
       "description": "A list of overrides to apply to the settings, based on the `if` selector.",
@@ -648,7 +643,8 @@
             "$ref": "#/properties/build-dir"
           },
           "fail": {
-            "$ref": "#/properties/fail"
+            "type": "boolean",
+            "description": "Immediately fail the build. This is only useful in overrides."
           }
         }
       }

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),
         )

src/scikit_build_core/settings/json_schema.py

@@ -97,6 +97,8 @@ def to_json_schema(dclass: type[Any], *, normalize_keys: bool) -> dict[str, Any]
         props[field.name]["description"] = docs[field.name].split("\n", maxsplit=1)[0]
         if field.metadata.get("deprecated"):
             props[field.name]["deprecated"] = True
+        if field.metadata.get("override_only"):
+            props[field.name]["scikit-build:override-only"] = True
 
     if normalize_keys:
         props = {k.replace("_", "-"): v for k, v in props.items()}

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()
 

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):
@@ -507,7 +509,10 @@ 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.
     """

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,17 +435,33 @@ def process_overrides(
                 inherit1 = inherit_override.get(key, {})
                 if isinstance(value, dict):
                     for key2, value2 in value.items():
+                        record_override(
+                            *[key, key2],
+                            value=value2,
+                            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(
                             value2, inner.get(key2, None), inherit2
                         )
                         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

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:

src/scikit_build_core/settings/skbuild_schema.py

@@ -82,7 +82,21 @@ def generate_skbuild_schema(tool_name: str = "scikit-build") -> dict[str, Any]:
         m: {"$ref": "#/$defs/metadata"} for m in METADATA
     }
 
+    inherit_only_props = {
+        k: v
+        for k, v in schema["properties"].items()
+        if v.get("scikit-build:override-only")
+    }
+    schema["properties"] = {
+        k: v
+        for k, v in schema["properties"].items()
+        if not v.get("scikit-build:override-only")
+    }
+    for v in inherit_only_props.values():
+        del v["scikit-build:override-only"]
     props = {k: {"$ref": f"#/properties/{k}"} for k in schema["properties"]}
+    props.update(inherit_only_props)
+
     schema["$defs"]["if_overrides"] = {
         "type": "object",
         "minProperties": 1,