refactor: deduplicate config-setting handling

henryiii · henryiii · commit 8ccfee3dda4a · 2026-07-22T15:11:00.000-04:00
Generate the declaration entry schema from the dataclass, fold the
triplicated post-override handling into one helper, and drop a
duplicated test constant.

Assisted-by: ClaudeCode:claude-fable-5
diff --git a/src/scikit_build_core/resources/scikit-build.schema.json b/src/scikit_build_core/resources/scikit-build.schema.json
@@ -621,6 +621,7 @@
           "properties": {
             "help": {
               "type": "string",
+              "default": "",
               "description": "A description of the setting."
             },
             "type": {
@@ -644,8 +645,8 @@
             },
             "env": {
               "type": "string",
-              "minLength": 1,
-              "description": "An environment variable also read for this setting; it takes precedence over `-C`."
+              "description": "An environment variable also read for this setting; it takes precedence over `-C`.",
+              "minLength": 1
             }
           }
         }
diff --git a/src/scikit_build_core/settings/config_settings.py b/src/scikit_build_core/settings/config_settings.py
@@ -1,4 +1,5 @@
-from __future__ import annotations
+# No `from __future__ import annotations` here: `to_json_schema` reads real
+# runtime types from the dataclass fields, like `skbuild_model.py`.
 
 __lazy_modules__ = {
     f"{(__spec__.parent or '').rsplit('.', 1)[0]}._logging",
@@ -9,7 +10,7 @@
 
 import dataclasses
 import re
-from typing import Any, Literal
+from typing import Any, Dict, List, Literal, Optional, Union
 
 from .._logging import rich_error
 from .skbuild_model import ScikitBuildSettings
@@ -27,7 +28,7 @@
 ]
 
 
-def __dir__() -> list[str]:
+def __dir__() -> List[str]:
     return __all__
 
 
@@ -36,10 +37,7 @@ def __dir__() -> list[str]:
 _NAME_REGEX = re.compile(r"^[A-Za-z0-9_-]+(\.[A-Za-z0-9_-]+)+$")
 
 # A "choices" key (allowed values, str type only) was cut from the initial
-# release to keep the surface minimal. To re-add: a `choices: list[str] | None`
-# field here, load-time validation (list of strings, not with type = 'bool',
-# default must be a member), a resolved-value check in resolve_config_settings,
-# and the entry-schema property in skbuild_schema.py.
+# release to keep the surface minimal; see git history to re-add.
 _DECLARATION_KEYS = frozenset({"help", "type", "default", "env"})
 
 
@@ -54,28 +52,45 @@ class ConfigSettingDeclaration:
     """
 
     help: str = ""
+    """
+    A description of the setting.
+    """
+
     type: Literal["str", "bool"] = "str"
-    default: str | bool | None = None
-    env: str | None = None
+    """
+    The type of the setting.
+    """
+
+    default: Optional[Union[str, bool]] = None
+    """
+    The value used when the setting is not passed; must match the type.
+    """
+
+    env: Optional[str] = None
+    """
+    An environment variable also read for this setting; it takes precedence over `-C`.
+    """
 
 
 def _error_context(name: str) -> str:
     return f"tool.scikit-build.config-setting.{name!r}"
 
 
-def load_declarations(raw: Any) -> dict[str, ConfigSettingDeclaration]:
+def load_declarations(raw: Any) -> Dict[str, ConfigSettingDeclaration]:
     """
     Validate and load the raw ``tool.scikit-build.config-setting`` table.
     """
     if not isinstance(raw, dict):
         rich_error("tool.scikit-build.config-setting must be a table")
+    if not raw:
+        return {}
 
     reserved = {
         field.name.replace("_", "-")
         for field in dataclasses.fields(ScikitBuildSettings)
     } | {"overrides", "config-setting", "skbuild"}
 
-    decls: dict[str, ConfigSettingDeclaration] = {}
+    decls: Dict[str, ConfigSettingDeclaration] = {}
     for name, entry in raw.items():
         if not _NAME_REGEX.match(name):
             rich_error(
@@ -130,17 +145,17 @@ def load_declarations(raw: Any) -> dict[str, ConfigSettingDeclaration]:
 
 
 def resolve_config_settings(
-    decls: Mapping[str, ConfigSettingDeclaration],
-    config_settings: Mapping[str, str | list[str] | bool],
-    env: Mapping[str, str],
-) -> dict[str, str | bool | None]:
+    decls: "Mapping[str, ConfigSettingDeclaration]",
+    config_settings: "Mapping[str, Union[str, List[str], bool]]",
+    env: "Mapping[str, str]",
+) -> Dict[str, Optional[Union[str, bool]]]:
     """
     Resolve declared config-settings; precedence env var > config-setting >
     default, with ``None`` meaning "unset".
     """
-    values: dict[str, str | bool | None] = {}
+    values: Dict[str, Optional[Union[str, bool]]] = {}
     for name, decl in decls.items():
-        raw: str | bool | None
+        raw: Optional[Union[str, bool]]
         if decl.env is not None and decl.env in env:
             raw = env[decl.env]
         elif name in config_settings:
@@ -165,7 +180,7 @@ def resolve_config_settings(
 
 
 def resolve_define_references(
-    tool_skb: dict[str, Any], values: Mapping[str, str | bool | None]
+    tool_skb: Dict[str, Any], values: "Mapping[str, Optional[Union[str, bool]]]"
 ) -> None:
     """
     Replace ``{config-setting = "..."}`` values in the raw ``cmake.define``
diff --git a/src/scikit_build_core/settings/skbuild_read_settings.py b/src/scikit_build_core/settings/skbuild_read_settings.py
@@ -296,26 +296,33 @@ def __init__(
             self.config_setting_decls, config_settings, environ
         )
 
+        def process_table(
+            skb: dict[str, Any], declaration_error: str
+        ) -> tuple[set[str], dict[str, OverrideRecord]]:
+            """
+            Apply overrides to one settings table, reject stray config-setting
+            declarations (the static table was popped from pyproject.toml above,
+            so any survivor is misplaced), and resolve define references.
+            """
+            matched, overridden = process_overrides(
+                skb,
+                state=state,
+                env=env,
+                retry=retry,
+                config_settings=self.custom_config_settings,
+            )
+            if "config-setting" in skb:
+                rich_error(declaration_error)
+            resolve_define_references(skb, self.custom_config_settings)
+            return matched, overridden
+
         # Handle overrides
-        self.overrides, self.overridden_items = process_overrides(
-            tool_skb,
-            state=state,
-            env=env,
-            retry=retry,
-            config_settings=self.custom_config_settings,
+        self.overrides, self.overridden_items = process_table(
+            tool_skb, "config-setting declarations may not be set by overrides"
         )
-        # The static table was popped above, so any survivor came from an
-        # override body.
-        if "config-setting" in tool_skb:
-            rich_error("config-setting declarations may not be set by overrides")
-        resolve_define_references(tool_skb, self.custom_config_settings)
 
         # Support for minimum-version='build-system.requires'
-        tmp_min_v = (
-            pyproject.get("tool", {})
-            .get("scikit-build", {})
-            .get("minimum-version", None)
-        )
+        tmp_min_v = tool_skb.get("minimum-version")
         if tmp_min_v == "build-system.requires":
             reqlist = pyproject["build-system"]["requires"]
             min_v = get_min_requires("scikit-build-core", reqlist)
@@ -324,7 +331,7 @@ def __init__(
                     "scikit-build-core needs a min version in "
                     "build-system.requires to use minimum-version='build-system.requires'"
                 )
-            pyproject["tool"]["scikit-build"]["minimum-version"] = str(min_v)
+            tool_skb["minimum-version"] = str(min_v)
         toml_srcs = [TOMLSource("tool", "scikit-build", settings=pyproject)]
         # Human-readable names parallel to ``toml_srcs`` (used by suggestions).
         toml_src_names = ["pyproject.toml"]
@@ -346,18 +353,10 @@ def __init__(
 
         if extra_settings is not None:
             extra_skb = copy.deepcopy(dict(extra_settings))
-            extra_matched, extra_overridden = process_overrides(
+            extra_matched, extra_overridden = process_table(
                 extra_skb,
-                state=state,
-                env=env,
-                retry=retry,
-                config_settings=self.custom_config_settings,
+                "config-setting declarations are only allowed in pyproject.toml",
             )
-            if "config-setting" in extra_skb:
-                rich_error(
-                    "config-setting declarations are only allowed in pyproject.toml"
-                )
-            resolve_define_references(extra_skb, self.custom_config_settings)
             self.overrides |= extra_matched
             self.overridden_items.update(extra_overridden)
             toml_srcs.insert(0, TOMLSource(settings=extra_skb))
@@ -390,18 +389,10 @@ def __init__(
                 # which merges dicts but takes lists wholesale from the
                 # highest-precedence source. A future refactor could process
                 # overrides on the merged view instead.
-                ep_matched, ep_overridden = process_overrides(
+                ep_matched, ep_overridden = process_table(
                     ep_skb,
-                    state=state,
-                    env=env,
-                    retry=retry,
-                    config_settings=self.custom_config_settings,
+                    "config-setting declarations are only allowed in pyproject.toml",
                 )
-                if "config-setting" in ep_skb:
-                    rich_error(
-                        "config-setting declarations are only allowed in pyproject.toml"
-                    )
-                resolve_define_references(ep_skb, self.custom_config_settings)
                 self.overrides |= ep_matched
                 ep_source = TOMLSource(settings=ep_skb)
                 ep_srcs.append(ep_source)
diff --git a/src/scikit_build_core/settings/skbuild_schema.py b/src/scikit_build_core/settings/skbuild_schema.py
@@ -42,6 +42,7 @@ def generate_skbuild_schema(tool_name: str = "scikit-build") -> dict[str, Any]:
     "Generate the complete schema for scikit-build settings."
     assert tool_name == "scikit-build", "Only scikit-build is supported."
 
+    from .config_settings import _NAME_REGEX, ConfigSettingDeclaration
     from .json_schema import to_json_schema
     from .skbuild_model import ScikitBuildSettings
 
@@ -226,36 +227,13 @@ def generate_skbuild_schema(tool_name: str = "scikit-build") -> dict[str, Any]:
     )
 
     # Added after ``props`` is collected so overrides cannot target it.
+    declaration_entry = to_json_schema(ConfigSettingDeclaration, normalize_keys=True)
+    declaration_entry["properties"]["env"]["minLength"] = 1
     schema["properties"]["config-setting"] = {
         "type": "object",
         "description": "Declare package-specific config-settings, settable via `-C name=value` or a bound environment variable.",
         "additionalProperties": False,
-        "patternProperties": {
-            r"^[A-Za-z0-9_-]+(\.[A-Za-z0-9_-]+)+$": {
-                "type": "object",
-                "additionalProperties": False,
-                "properties": {
-                    "help": {
-                        "type": "string",
-                        "description": "A description of the setting.",
-                    },
-                    "type": {
-                        "enum": ["str", "bool"],
-                        "default": "str",
-                        "description": "The type of the setting.",
-                    },
-                    "default": {
-                        "oneOf": [{"type": "string"}, {"type": "boolean"}],
-                        "description": "The value used when the setting is not passed; must match the type.",
-                    },
-                    "env": {
-                        "type": "string",
-                        "minLength": 1,
-                        "description": "An environment variable also read for this setting; it takes precedence over `-C`.",
-                    },
-                },
-            }
-        },
+        "patternProperties": {_NAME_REGEX.pattern: declaration_entry},
     }
 
     schema["properties"]["overrides"] = {
diff --git a/tests/test_config_settings.py b/tests/test_config_settings.py
@@ -289,15 +289,6 @@ def test_define_conf_beats_toml_reference(tmp_path: Path):
     assert settings_reader.settings.cmake.define["ZMQ_PREFIX"] == "/explicit"
 
 
-PREFIX_DECLARATION = dedent(
-    """\
-    [tool.scikit-build.config-setting."zmq.prefix"]
-    help = "Prefix to search for libzmq"
-    env = "ZMQ_PREFIX"
-    """
-)
-
-
 @pytest.mark.parametrize("with_decl", [True, False])
 @pytest.mark.parametrize(
     "malformed",
@@ -311,7 +302,7 @@ def test_malformed_cmake_table(tmp_path: Path, malformed: str, with_decl: bool):
     internal AttributeError in the config-setting resolution helpers."""
     content = f"[tool.scikit-build]\n{malformed}\n"
     if with_decl:
-        content += PREFIX_DECLARATION
+        content += ENV_DECLARATION
     pyproject_toml = write_pyproject(tmp_path, content)
     with pytest.raises(Exception, match="Failed converting") as exc:
         SettingsReader.from_file(pyproject_toml, {"zmq.prefix": "/foo"})

Original file line number	Diff line number	Diff line change
`@@ -621,6 +621,7 @@`
`621`	`621`	`"properties": {`
`622`	`622`	`"help": {`
`623`	`623`	`"type": "string",`
	`624`	`+ "default": "",`
`624`	`625`	`"description": "A description of the setting."`
`625`	`626`	`},`
`626`	`627`	`"type": {`
`@@ -644,8 +645,8 @@`
`644`	`645`	`},`
`645`	`646`	`"env": {`
`646`	`647`	`"type": "string",`
`647`		`- "minLength": 1,`
`648`		- "description": "An environment variable also read for this setting; it takes precedence over `-C`."
	`648`	+ "description": "An environment variable also read for this setting; it takes precedence over `-C`.",
	`649`	`+ "minLength": 1`
`649`	`650`	`}`
`650`	`651`	`}`
`651`	`652`	`}`