Skip to content

Add the ability to set 'inherit' at all levels of the options cascade - #2928

Open
joerick wants to merge 5 commits into
mainfrom
inherit-extend
Open

Add the ability to set 'inherit' at all levels of the options cascade#2928
joerick wants to merge 5 commits into
mainfrom
inherit-extend

Conversation

@joerick

@joerick joerick commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Adds the ability for options set in

  • [tool.cibuildwheel]
  • [tool.cibuildwheel.platform]
  • env vars

to inherit the values previously set.

Previously one could only inherit in an 'overrides' config.

I always wanted this to be possible for env vars, but I also realised that when we were doing audit-command, it would be useful for projects to be able to extend the default audit (e.g. add twine check to the existing abi3audit command). So I think it makes sense to allow this at every user-configurable level.

Docs + tests still to come.

@joerick
joerick marked this pull request as ready for review August 12, 2026 08:19
@joerick

joerick commented Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

This should be ready for review now!

@henryiii

Copy link
Copy Markdown
Contributor

Here's a Claude review.

🤖 AI text below 🤖

cibuildwheel/options.py
  ● 568 [correctness]       Moving overrides parsing from __init__ to a functools.cached_property makes override validation lazy, so malformed overrides config is silently accepted on paths that never compute per-identifier options.
  ● 445 [correctness]       parse_inherit validates rule values but never the keys against known option names, so typos in the new global/platform/CIBW_INHERIT inherit surfaces are silent no-ops that degrade to replace semantics.
  ● 405 [correctness]       _apply_inherit_rule does not catch OptionFormat.NotSupported, so an append/prepend inherit rule on an allow_merge=False option (container-engine, xbuild-files, test-runtime — all listed in the new schema's inherit keys) crashes with a blank-message traceback.
  ● 874 [correctness]       An inherit append/prepend rule on xbuild-tools merges the user value with the "" unset-sentinel default, defeating the sentinel check and leaking a NUL tool name.
  ● 599 [correctness]       parse_inherit's string path lets shlex's bare ValueError escape: env_inherit catches only OptionsReaderError, and the config-file/override call sites have no wrapper at all.
  ● 460 [correctness]       parse_inherit's value check `v in {"none", "append", "prepend"}` raises TypeError on unhashable TOML values instead of the intended OptionsReaderError.
  ● 604 [api-design]        Platform scoping for env inherit rules is encoded as a '-<platform>' suffix inside CIBW_INHERIT keys instead of the CIBW_<OPTION>_<PLATFORM> convention every other option uses, and the suffix matches against no vocabulary.
cibuildwheel/util/helpers.py
  ● 189 [correctness]       In CIBW_INHERIT, a forgotten colon or a space inside a rule word is silently accepted with the wrong meaning: bare words become keys with default 'append', and parse_inherit's "".join glues multi-token values back into valid rules.
  ● 150 [simplification]    parse_arbitrary_key_value_string is a near-verbatim copy of parse_key_value_string (16 of 27 body lines byte-identical), added in the same PR that deletes the '*' wildcard which already provided arbitrary-key parsing.
bin/generate_schema.py
  ●  43 [schema-divergence] The schema's inherit declaration diverges from runtime acceptance: string form rejected, the 18-key list omits now-reachable list options (enable, archs, test-groups, build, skip), and platform sections $ref the full inherit object including keys invalid for that platform.

Review of PR #2928 is done — 10 findings reported above, ranked by severity. Eight are confirmed with executed reproductions; two (the schema/runtime divergence and the CIBW_INHERIT platform-suffix design concern) are plausible but not mechanically reproduced.

The broad themes:

  • Error handling around the new inherit surfaces is the weak spot. Typo'd keys, missing colons, unbalanced quotes, non-string TOML values, and append/prepend on non-mergeable options all either silently do the wrong thing or escape as raw tracebacks instead of the usual friendly fatal errors. One fix location (parse_inherit) covers several of these.
  • A real regression: moving overrides parsing into a cached_property makes validation lazy, so --print-build-identifiers now exits 0 on config that previously failed fast.
  • Two pre-existing bugs got widened: the OptionFormat.NotSupported crash and the \x00 xbuild-tools sentinel leak existed via override inherit, but the new top-level/platform/env surfaces (and the schema advertising the keys) make them much more reachable.
  • Cleanup: the new parse_arbitrary_key_value_string duplicates the existing parser almost verbatim and could be a default_value kwarg instead; and the schema's 18-key inherit list no longer matches what runtime actually accepts.

The reviewer also verified the checked-in schema matches bin/generate_schema.py output and that all 220 unit tests pass at PR head. Since the CIBW_INHERIT env var is brand new in this PR, the naming/validation concerns (findings about key typos and the -<platform> suffix) are cheap to address now and costly after release — worth deciding before merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants