Skip to content

Commit 15c943a

Browse files
committed
fix: detect override vs. config-settings
Signed-off-by: Henry Schreiner <henryfs@princeton.edu>
1 parent 208a853 commit 15c943a

2 files changed

Lines changed: 61 additions & 10 deletions

File tree

src/scikit_build_core/settings/skbuild_read_settings.py

Lines changed: 30 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -137,22 +137,28 @@ def _handle_move(
137137

138138
def _validate_overrides(
139139
settings: ScikitBuildSettings,
140+
static_settings: ScikitBuildSettings,
140141
overrides: dict[str, OverrideRecord],
142+
config_setting_keys: set[str],
141143
) -> None:
142144
"""Validate all fields with any override information."""
143145

144146
def validate_field(
145147
field: dataclasses.Field[Any],
146148
value: Any,
149+
static_value: Any,
147150
prefix: str = "",
148151
record: OverrideRecord | None = None,
149152
) -> None:
150153
"""Do the actual validation."""
151154
# Check if we had a hard-coded value in the record
152155
conf_key = field.name.replace("_", "-")
153156
if field.metadata.get("override_only", False):
154-
original_value = record.original_value if record else value
155-
if original_value is not None:
157+
full_key = f"{prefix}{conf_key}"
158+
original_value = record.original_value if record else static_value
159+
if original_value is not None or (
160+
value is not None and full_key not in config_setting_keys
161+
):
156162
msg = f"{prefix}{conf_key} is not allowed to be hard-coded in the pyproject.toml file"
157163
if settings.strict_config:
158164
sys.stdout.flush()
@@ -162,6 +168,7 @@ def validate_field(
162168

163169
def validate_field_recursive(
164170
obj: Any,
171+
static_obj: Any,
165172
record: OverrideRecord | None = None,
166173
prefix: str = "",
167174
) -> None:
@@ -170,20 +177,25 @@ def validate_field_recursive(
170177
conf_key = field.name.replace("_", "-")
171178
closest_record = overrides.get(f"{prefix}{conf_key}", record)
172179
value = getattr(obj, field.name)
180+
static_value = getattr(static_obj, field.name)
173181
# Do the validation of the current field
174182
validate_field(
175183
field=field,
176184
value=value,
185+
static_value=static_value,
177186
prefix=prefix,
178187
record=closest_record,
179188
)
180189
if dataclasses.is_dataclass(value):
181190
validate_field_recursive(
182-
obj=value, record=closest_record, prefix=f"{prefix}{conf_key}."
191+
obj=value,
192+
static_obj=static_value,
193+
record=closest_record,
194+
prefix=f"{prefix}{conf_key}.",
183195
)
184196

185197
# Navigate all fields starting from the top-level
186-
validate_field_recursive(obj=settings)
198+
validate_field_recursive(obj=settings, static_obj=static_settings)
187199

188200

189201
class SettingsReader:
@@ -250,6 +262,9 @@ def __init__(
250262
remaining = {
251263
k: v for k, v in config_settings.items() if not k.startswith("skbuild.")
252264
}
265+
self.config_setting_keys = {
266+
k[8:] if k.startswith("skbuild.") else k for k in config_settings
267+
}
253268
self.sources = SourceChain(
254269
EnvSource("SKBUILD", env=env),
255270
ConfSource("skbuild", settings=prefixed, verify=verify_conf),
@@ -259,7 +274,7 @@ def __init__(
259274
)
260275
self.settings = self.sources.convert_target(ScikitBuildSettings)
261276

262-
static_settings = SourceChain(
277+
self.static_settings = SourceChain(
263278
*toml_srcs, prefixes=["tool", "scikit-build"]
264279
).convert_target(ScikitBuildSettings)
265280

@@ -350,8 +365,8 @@ def __init__(
350365
self.settings.build.verbose,
351366
self.settings.minimum_version,
352367
Version("0.10"),
353-
static=static_settings.cmake.verbose == self.settings.cmake.verbose
354-
and static_settings.build.verbose == self.settings.build.verbose,
368+
static=self.static_settings.cmake.verbose == self.settings.cmake.verbose
369+
and self.static_settings.build.verbose == self.settings.build.verbose,
355370
)
356371
self.settings.build.targets = _handle_move(
357372
"cmake.targets",
@@ -360,8 +375,8 @@ def __init__(
360375
self.settings.build.targets,
361376
self.settings.minimum_version,
362377
Version("0.10"),
363-
static=static_settings.cmake.targets == self.settings.cmake.targets
364-
and static_settings.build.targets == self.settings.build.targets,
378+
static=self.static_settings.cmake.targets == self.settings.cmake.targets
379+
and self.static_settings.build.targets == self.settings.build.targets,
365380
)
366381

367382
if self.settings.sdist.inclusion_mode is not None:
@@ -421,7 +436,12 @@ def validate_may_exit(self) -> None:
421436
self.print_suggestions()
422437
raise SystemExit(7)
423438
logger.warning("Unrecognized options: {}", ", ".join(unrecognized))
424-
_validate_overrides(self.settings, self.overridden_items)
439+
_validate_overrides(
440+
self.settings,
441+
self.static_settings,
442+
self.overridden_items,
443+
self.config_setting_keys,
444+
)
425445

426446
for key, value in self.settings.metadata.items():
427447
if "provider" not in value:

tests/test_settings_overrides.py

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,37 @@ def test_disallow_hardcoded(
7070
assert "is not allowed to be hard-coded in the pyproject.toml file" in out
7171

7272

73+
@pytest.mark.parametrize("prefix", [True, False], ids=["skbuild", "noprefix"])
74+
def test_allow_override_only_in_config_settings(
75+
tmp_path: Path,
76+
caplog: pytest.LogCaptureFixture,
77+
prefix: bool,
78+
):
79+
pyproject_toml = tmp_path / "pyproject.toml"
80+
pyproject_toml.write_text("", encoding="utf-8")
81+
82+
config_settings: dict[str, str] = {
83+
"cmake.toolchain-file": "foo.cmake",
84+
"wheel.tags": "cp312-abi3-win_amd64",
85+
}
86+
if prefix:
87+
config_settings = {f"skbuild.{k}": v for k, v in config_settings.items()}
88+
89+
caplog.set_level(logging.WARNING)
90+
91+
settings_reader = SettingsReader.from_file(pyproject_toml, config_settings)
92+
settings_reader.validate_may_exit()
93+
94+
assert settings_reader.settings.cmake.toolchain_file == Path("foo.cmake")
95+
assert settings_reader.settings.wheel.tags == ["cp312-abi3-win_amd64"]
96+
assert not [
97+
record
98+
for record in caplog.records
99+
if "is not allowed to be hard-coded in the pyproject.toml file"
100+
in str(record.msg)
101+
]
102+
103+
73104
@pytest.mark.parametrize("python_version", ["3.9", "3.10"])
74105
def test_skbuild_overrides_pyver(
75106
python_version: str, tmp_path: Path, monkeypatch: pytest.MonkeyPatch

0 commit comments

Comments
 (0)