From 8611ede6194282e76b0698aa01a9ef0268aff438 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Mon, 22 Jan 2024 18:09:21 -0500 Subject: [PATCH 1/6] feat: add inherit to override Signed-off-by: Henry Schreiner --- bin/generate_schema.py | 57 ++++++------- cibuildwheel/options.py | 64 ++++++++++----- .../resources/cibuildwheel.schema.json | 19 +++++ docs/options.md | 17 ++++ unit_test/options_toml_test.py | 80 ++++++++++++++----- unit_test/validate_schema_test.py | 29 +++++++ 6 files changed, 195 insertions(+), 71 deletions(-) diff --git a/bin/generate_schema.py b/bin/generate_schema.py index 08680b831..4de427e47 100755 --- a/bin/generate_schema.py +++ b/bin/generate_schema.py @@ -153,9 +153,6 @@ schema = yaml.safe_load(starter) -if args.schemastore: - schema["$id"] += "#" - string_array = yaml.safe_load( """ - type: string @@ -214,19 +211,29 @@ additionalProperties: false properties: select: {} + inherit: + type: array + items: + enum: + - before-all + - before-build + - before-test + - config-settings + - container-engine + - environment + - environment-pass + - repair-wheel-command + - test-command + - test-extras + - test-requires + uniqueItems: true """ ) for key, value in schema["properties"].items(): value["title"] = f'CIBW_{key.replace("-", "_").upper()}' -if args.schemastore: - non_global_options = { - k: {"$ref": f"#/properties/tool/properties/cibuildwheel/properties/{k}"} - for k in schema["properties"] - } -else: - non_global_options = {k: {"$ref": f"#/properties/{k}"} for k in schema["properties"]} +non_global_options = {k: {"$ref": f"#/properties/{k}"} for k in schema["properties"]} del non_global_options["build"] del non_global_options["skip"] del non_global_options["container-engine"] @@ -273,27 +280,11 @@ def as_object(d: dict[str, Any]) -> dict[str, Any]: schema["properties"]["overrides"] = overrides schema["properties"] |= oses -if not args.schemastore: - print(json.dumps(schema, indent=2)) - raise SystemExit(0) - -schema_store_txt = """ -$id: https://json.schemastore.org/cibuildwheel.json -$schema: http://json-schema.org/draft-07/schema# -additionalProperties: false -description: cibuildwheel's toml file, generated with ./bin/generate_schema.py --schemastore from cibuildwheel. -type: object -properties: - tool: - type: object - properties: - cibuildwheel: - type: object -""" -schema_store = yaml.safe_load(schema_store_txt) - -schema_store["properties"]["tool"]["properties"]["cibuildwheel"]["properties"] = schema[ - "properties" -] +if args.schemastore: + schema["$id"] = "https://json.schemastore.org/partial-cibuildwheel.json" + schema["$id"] = "http://json-schema.org/draft-07/schema#" + schema[ + "description" + ] = "cibuildwheel's toml file, generated with ./bin/generate_schema.py --schemastore from cibuildwheel." -print(json.dumps(schema_store, indent=2)) +print(json.dumps(schema, indent=2)) diff --git a/cibuildwheel/options.py b/cibuildwheel/options.py index 418b19f52..50ba86f93 100644 --- a/cibuildwheel/options.py +++ b/cibuildwheel/options.py @@ -10,9 +10,9 @@ import sys import textwrap import traceback -from collections.abc import Callable, Generator, Iterable, Iterator, Mapping, Set +from collections.abc import Callable, Generator, Iterable, Iterator, Set from pathlib import Path -from typing import Any, Dict, List, Literal, TypedDict, Union +from typing import Any, Literal, Mapping, Sequence, TypedDict, Union # noqa: TID251 from packaging.specifiers import SpecifierSet @@ -116,13 +116,14 @@ def architectures(self) -> set[Architecture]: return self.globals.architectures -Setting = Union[Dict[str, str], List[str], str, int] +Setting = Union[Mapping[str, str], Sequence[str], str, int] @dataclasses.dataclass(frozen=True) class Override: select_pattern: str options: dict[str, Setting] + inherit: list[str] MANYLINUX_OPTIONS = {f"manylinux-{build_platform}-image" for build_platform in MANYLINUX_ARCHS} @@ -150,25 +151,43 @@ class ConfigOptionError(KeyError): pass -def _dig_first(*pairs: tuple[Mapping[str, Setting], str], ignore_empty: bool = False) -> Setting: +def _dig_first( + *pairs: tuple[Mapping[str, Setting], str, bool], ignore_empty: bool = False +) -> Setting: """ - Return the first dict item that matches from pairs of dicts and keys. - Will throw a KeyError if missing. + Return the dict items that match from pairs of dicts and keys. The third value (boolean) indicates if values should merge. - _dig_first((dict1, "key1"), (dict2, "key2"), ...) + The following idiom can be used to get the first matching value: + + _dig_first((dict1, "key1", False), (dict2, "key2", False), ...))) """ if not pairs: msg = "pairs cannot be empty" raise ValueError(msg) - for dict_like, key in pairs: + old_value: None | Setting = None + + for dict_like, key, merge in pairs: if key in dict_like: value = dict_like[key] - if ignore_empty and value == "": continue - return value + if old_value is None: + old_value = value + elif isinstance(value, list) and isinstance(old_value, list): + old_value = value + old_value + elif isinstance(value, dict) and isinstance(old_value, dict): + old_value = {**value, **old_value} + else: + msg = f"Cannot merge {value!r} with {dict_like[key]!r}" + raise ValueError(msg) + + if not merge: + return old_value + + if old_value is not None: + return old_value last_key = pairs[-1][1] raise KeyError(last_key) @@ -244,7 +263,12 @@ def __init__( if isinstance(select, list): select = " ".join(select) - self.overrides.append(Override(select, config_override)) + inherit = config_override.pop("inherit", []) + if not isinstance(inherit, list) or not all(isinstance(i, str) for i in inherit): + msg = "'inherit' must be a list of strings" + raise ConfigOptionError(msg) + + self.overrides.append(Override(select, config_override, inherit)) def _validate_global_option(self, name: str) -> None: """ @@ -341,17 +365,17 @@ def get( # get the option from the environment, then the config file, then finally the default. # platform-specific options are preferred, if they're allowed. result = _dig_first( - (self.env if env_plat else {}, plat_envvar), - (self.env, envvar), - *[(o.options, name) for o in active_config_overrides], - (self.config_platform_options, name), - (self.config_options, name), - (self.default_platform_options, name), - (self.default_options, name), + (self.env if env_plat else {}, plat_envvar, False), + (self.env, envvar, False), + *[(o.options, name, name in o.inherit) for o in active_config_overrides], + (self.config_platform_options, name, False), + (self.config_options, name, False), + (self.default_platform_options, name, False), + (self.default_options, name, False), ignore_empty=ignore_empty, ) - if isinstance(result, dict): + if isinstance(result, Mapping): if table is None: msg = f"{name!r} does not accept a table" raise ConfigOptionError(msg) @@ -359,7 +383,7 @@ def get( item for k, v in result.items() for item in _inner_fmt(k, v, table) ) - if isinstance(result, list): + if not isinstance(result, str) and isinstance(result, Sequence): if sep is None: msg = f"{name!r} does not accept a list" raise ConfigOptionError(msg) diff --git a/cibuildwheel/resources/cibuildwheel.schema.json b/cibuildwheel/resources/cibuildwheel.schema.json index 7e9c3901d..b2b86a65b 100644 --- a/cibuildwheel/resources/cibuildwheel.schema.json +++ b/cibuildwheel/resources/cibuildwheel.schema.json @@ -423,6 +423,25 @@ } ] }, + "inherit": { + "type": "array", + "items": { + "enum": [ + "before-all", + "before-build", + "before-test", + "config-settings", + "container-engine", + "environment", + "environment-pass", + "repair-wheel-command", + "test-command", + "test-extras", + "test-requires" + ], + "uniqueItems": true + } + }, "before-all": { "$ref": "#/properties/before-all" }, diff --git a/docs/options.md b/docs/options.md index a339e28ee..9dc63ce85 100644 --- a/docs/options.md +++ b/docs/options.md @@ -135,6 +135,9 @@ trigger new containers, one per image. Some commands are not supported; `output-dir`, build/skip/test_skip selectors, and architectures cannot be overridden. +You can specify a list of options in `inherit=[]`, any list or table in this +list will inherit from previous overrides or the main configuration. + ##### Examples: ```toml @@ -169,6 +172,20 @@ This example will build CPython 3.6 wheels on manylinux1, CPython 3.7-3.9 wheels on manylinux2010, and manylinux2014 wheels for any newer Python (like 3.10). +```toml +[tool.cibuildwheel] +environment = {FOO="BAR", "HAM"="EGGS"} +test-command = ["pyproject"] + +[[tool.cibuildwheel.overrides]] +select = "cp311*" +inherit = ["test-command", "environment"] +test-command = ["pyproject-override"] +environment = {FOO="BAZ", "PYTHON"="MONTY"} +``` + +This example will provide the command `["pyproject", "pyproject-override"]` on +Python 3.11, and will have `environment = {FOO="BAZ", "PYTHON"="MONTY", "HAM"="EGGS"}`. ## Options summary diff --git a/unit_test/options_toml_test.py b/unit_test/options_toml_test.py index 4d5b4f43e..996e4db7b 100644 --- a/unit_test/options_toml_test.py +++ b/unit_test/options_toml_test.py @@ -270,39 +270,77 @@ def test_dig_first(ignore_empty): d4 = {"this": "d4", "empty": "not"} answer = _dig_first( - (d1, "empty"), - (d2, "empty"), - (d3, "empty"), - (d4, "empty"), + (d1, "empty", False), + (d2, "empty", False), + (d3, "empty", False), + (d4, "empty", False), ignore_empty=ignore_empty, ) assert answer == ("not" if ignore_empty else "") answer = _dig_first( - (d1, "this"), - (d2, "this"), - (d3, "this"), - (d4, "this"), + (d1, "this", False), + (d2, "this", False), + (d3, "this", False), + (d4, "this", False), ignore_empty=ignore_empty, ) assert answer == "that" with pytest.raises(KeyError): _dig_first( - (d1, "this"), - (d2, "other"), - (d3, "this"), - (d4, "other"), + (d1, "this", False), + (d2, "other", False), + (d3, "this", False), + (d4, "other", False), ignore_empty=ignore_empty, ) +@pytest.mark.parametrize("ignore_empty", [True, False]) +@pytest.mark.parametrize("end", [True, False]) +def test_dig_first_merge_list(ignore_empty, end): + d1 = {"random": ["thing"]} + d2 = {"this": ["that"], "empty": ""} + d3 = {"other": ["hi"]} + d4 = {"this": ["d4"], "empty": ["not"]} + + answer = _dig_first( + (d1, "this", False), + (d2, "this", True), + (d3, "this", False), + (d4, "this", end), + ignore_empty=ignore_empty, + ) + + assert answer == ["d4", "that"] + + +@pytest.mark.parametrize("ignore_empty", [True, False]) +@pytest.mark.parametrize("end", [True, False]) +def test_dig_first_merge_dict(ignore_empty, end): + d1 = {"random": {"a": "thing"}} + d2 = {"this": {"b": "that"}} + d3 = {"other": {"c": "ho"}} + d4 = {"this": {"d": "d4"}, "empty": {"d": "not"}} + + answer = _dig_first( + (d1, "this", False), + (d2, "this", True), + (d3, "this", False), + (d4, "this", end), + ignore_empty=ignore_empty, + ) + + assert answer == {"b": "that", "d": "d4"} + + PYPROJECT_2 = """ [tool.cibuildwheel] build = ["cp38*", "cp37*"] -environment = {FOO="BAR"} +environment = {FOO="BAR", "HAM"="EGGS"} -test-command = "pyproject" +test-command = ["pyproject"] manylinux-x86_64-image = "manylinux1" @@ -311,8 +349,10 @@ def test_dig_first(ignore_empty): [[tool.cibuildwheel.overrides]] select = "cp37*" -test-command = "pyproject-override" +inherit = ["test-command", "environment"] +test-command = ["pyproject-override"] manylinux-x86_64-image = "manylinux2014" +environment = {FOO="BAZ", "PYTHON"="MONTY"} """ @@ -321,13 +361,17 @@ def test_pyproject_2(tmp_path, platform): pyproject_toml.write_text(PYPROJECT_2) options_reader = OptionsReader(config_file_path=pyproject_toml, platform=platform, env={}) - assert options_reader.get("test-command") == "pyproject" + assert options_reader.get("test-command", sep=" && ") == "pyproject" with options_reader.identifier("random"): - assert options_reader.get("test-command") == "pyproject" + assert options_reader.get("test-command", sep=" && ") == "pyproject" with options_reader.identifier("cp37-something"): - assert options_reader.get("test-command") == "pyproject-override" + assert options_reader.get("test-command", sep=" && ") == "pyproject && pyproject-override" + assert ( + options_reader.get("environment", table={"item": '{k}="{v}"', "sep": " "}) + == 'FOO="BAZ" HAM="EGGS" PYTHON="MONTY"' + ) def test_overrides_not_a_list(tmp_path, platform): diff --git a/unit_test/validate_schema_test.py b/unit_test/validate_schema_test.py index 66c95f25a..f58c94338 100644 --- a/unit_test/validate_schema_test.py +++ b/unit_test/validate_schema_test.py @@ -72,6 +72,35 @@ def test_overrides_only_select(): validator(example) +def test_overrides_valid_inherit(): + example = tomllib.loads( + """ + [[tool.cibuildwheel.overrides]] + inherit = ["repair-wheel-command"] + select = "somestring" + repair-wheel-command = ["something"] + """ + ) + + validator = validate_pyproject.api.Validator() + assert validator(example) is not None + + +def test_overrides_invalid_inherit(): + example = tomllib.loads( + """ + [[tool.cibuildwheel.overrides]] + inherit = ["something"] + select = "somestring" + repair-wheel-command = "something" + """ + ) + + validator = validate_pyproject.api.Validator() + with pytest.raises(validate_pyproject.error_reporting.ValidationError): + validator(example) + + def test_docs_examples(): """ Parse out all the configuration examples, build valid TOML out of them, and From 8b15f4c5a4063984ec1ffc5f0297c8b0bb2fc056 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Mon, 29 Jan 2024 11:36:37 -0500 Subject: [PATCH 2/6] refactor: use dict and support prepend Signed-off-by: Henry Schreiner --- bin/generate_schema.py | 37 ++++--- cibuildwheel/options.py | 103 +++++++++++++----- .../resources/cibuildwheel.schema.json | 63 ++++++++--- docs/options.md | 21 +++- unit_test/options_toml_test.py | 91 ++++++++++------ unit_test/validate_schema_test.py | 19 +++- 6 files changed, 234 insertions(+), 100 deletions(-) diff --git a/bin/generate_schema.py b/bin/generate_schema.py index 4de427e47..ed3154fd1 100755 --- a/bin/generate_schema.py +++ b/bin/generate_schema.py @@ -17,6 +17,14 @@ additionalProperties: false description: cibuildwheel's settings. type: object +defines: + inherit: + enum: + - none + - prepend + - append + default: none + description: How to inherit the parent's value. properties: archs: description: Change the architectures built on your machine by default. @@ -212,21 +220,20 @@ properties: select: {} inherit: - type: array - items: - enum: - - before-all - - before-build - - before-test - - config-settings - - container-engine - - environment - - environment-pass - - repair-wheel-command - - test-command - - test-extras - - test-requires - uniqueItems: true + type: object + additionalProperties: false + properties: + before-all: {"$ref": "#/defines/inherit"} + before-build: {"$ref": "#/defines/inherit"} + before-test: {"$ref": "#/defines/inherit"} + config-settings: {"$ref": "#/defines/inherit"} + container-engine: {"$ref": "#/defines/inherit"} + environment: {"$ref": "#/defines/inherit"} + environment-pass: {"$ref": "#/defines/inherit"} + repair-wheel-command: {"$ref": "#/defines/inherit"} + test-command: {"$ref": "#/defines/inherit"} + test-extras: {"$ref": "#/defines/inherit"} + test-requires: {"$ref": "#/defines/inherit"} """ ) diff --git a/cibuildwheel/options.py b/cibuildwheel/options.py index 50ba86f93..28ff59a6e 100644 --- a/cibuildwheel/options.py +++ b/cibuildwheel/options.py @@ -5,6 +5,7 @@ import contextlib import dataclasses import difflib +import enum import functools import shlex import sys @@ -123,7 +124,7 @@ def architectures(self) -> set[Architecture]: class Override: select_pattern: str options: dict[str, Setting] - inherit: list[str] + inherit: dict[str, Inherit] MANYLINUX_OPTIONS = {f"manylinux-{build_platform}-image" for build_platform in MANYLINUX_ARCHS} @@ -151,21 +152,28 @@ class ConfigOptionError(KeyError): pass +class Inherit(enum.Enum): + NONE = enum.auto() + APPEND = enum.auto() + PREPEND = enum.auto() + + def _dig_first( - *pairs: tuple[Mapping[str, Setting], str, bool], ignore_empty: bool = False + *pairs: tuple[Mapping[str, Setting], str, Inherit], ignore_empty: bool = False ) -> Setting: """ - Return the dict items that match from pairs of dicts and keys. The third value (boolean) indicates if values should merge. + Return the dict items that match from pairs of dicts and keys. The third + value (enum) indicates if values should merge. The following idiom can be used to get the first matching value: - _dig_first((dict1, "key1", False), (dict2, "key2", False), ...))) + _dig_first((dict1, "key1", Inherit.NONE), (dict2, "key2", Inherit.NONE), ...))) """ if not pairs: msg = "pairs cannot be empty" raise ValueError(msg) - old_value: None | Setting = None + commands: list[tuple[Setting, Inherit]] = [] for dict_like, key, merge in pairs: if key in dict_like: @@ -173,21 +181,49 @@ def _dig_first( if ignore_empty and value == "": continue - if old_value is None: - old_value = value - elif isinstance(value, list) and isinstance(old_value, list): - old_value = value + old_value - elif isinstance(value, dict) and isinstance(old_value, dict): - old_value = {**value, **old_value} - else: - msg = f"Cannot merge {value!r} with {dict_like[key]!r}" - raise ValueError(msg) - - if not merge: - return old_value + commands.append((value, merge)) + if merge == Inherit.NONE: + break + + if len(commands) == 1: + return commands[0][0] + + if len(commands) > 1: + if isinstance(commands[0][0], list): + retlist: list[str] = [] + for c in commands: + if c[1] == Inherit.PREPEND: + assert isinstance(c[0], list) + retlist.extend(c[0]) + for c in commands: + if c[1] == Inherit.NONE: + assert isinstance(c[0], list) + retlist.extend(c[0]) + for c in commands[::-1]: + if c[1] == Inherit.APPEND: + assert isinstance(c[0], list) + retlist.extend(c[0]) + return retlist + + if isinstance(commands[0][0], dict): + retdict: dict[str, str] = {} + for c in commands: + if c[1] == Inherit.PREPEND: + assert isinstance(c[0], dict) + retdict.update(c[0]) + for c in commands: + if c[1] == Inherit.NONE: + assert isinstance(c[0], dict) + retdict.update(c[0]) + for c in commands[::-1]: + if c[1] == Inherit.APPEND: + assert isinstance(c[0], dict) + retdict.update(c[0]) + return retdict - if old_value is not None: - return old_value + else: + msg = f"Must be list or dict, got {type(commands[0])}" + raise TypeError(msg) last_key = pairs[-1][1] raise KeyError(last_key) @@ -263,12 +299,16 @@ def __init__( if isinstance(select, list): select = " ".join(select) - inherit = config_override.pop("inherit", []) - if not isinstance(inherit, list) or not all(isinstance(i, str) for i in inherit): - msg = "'inherit' must be a list of strings" + inherit = config_override.pop("inherit", {}) + if not isinstance(inherit, dict) or not all( + i in {"none", "append", "prepend"} for i in inherit.values() + ): + msg = "'inherit' must be a dict containing only {'none', 'append', 'prepend'} values" raise ConfigOptionError(msg) - self.overrides.append(Override(select, config_override, inherit)) + inherit_enum = {k: Inherit[v.upper()] for k, v in inherit.items()} + + self.overrides.append(Override(select, config_override, inherit_enum)) def _validate_global_option(self, name: str) -> None: """ @@ -365,13 +405,16 @@ def get( # get the option from the environment, then the config file, then finally the default. # platform-specific options are preferred, if they're allowed. result = _dig_first( - (self.env if env_plat else {}, plat_envvar, False), - (self.env, envvar, False), - *[(o.options, name, name in o.inherit) for o in active_config_overrides], - (self.config_platform_options, name, False), - (self.config_options, name, False), - (self.default_platform_options, name, False), - (self.default_options, name, False), + (self.env if env_plat else {}, plat_envvar, Inherit.NONE), + (self.env, envvar, Inherit.NONE), + *[ + (o.options, name, o.inherit.get(name, Inherit.NONE)) + for o in active_config_overrides + ], + (self.config_platform_options, name, Inherit.NONE), + (self.config_options, name, Inherit.NONE), + (self.default_platform_options, name, Inherit.NONE), + (self.default_options, name, Inherit.NONE), ignore_empty=ignore_empty, ) diff --git a/cibuildwheel/resources/cibuildwheel.schema.json b/cibuildwheel/resources/cibuildwheel.schema.json index b2b86a65b..1dd890990 100644 --- a/cibuildwheel/resources/cibuildwheel.schema.json +++ b/cibuildwheel/resources/cibuildwheel.schema.json @@ -4,6 +4,17 @@ "additionalProperties": false, "description": "cibuildwheel's settings.", "type": "object", + "defines": { + "inherit": { + "enum": [ + "none", + "prepend", + "append" + ], + "default": "none", + "description": "How to inherit the parent's value." + } + }, "properties": { "archs": { "description": "Change the architectures built on your machine by default.", @@ -424,22 +435,42 @@ ] }, "inherit": { - "type": "array", - "items": { - "enum": [ - "before-all", - "before-build", - "before-test", - "config-settings", - "container-engine", - "environment", - "environment-pass", - "repair-wheel-command", - "test-command", - "test-extras", - "test-requires" - ], - "uniqueItems": true + "type": "object", + "additionalProperties": false, + "properties": { + "before-all": { + "$ref": "#/defines/inherit" + }, + "before-build": { + "$ref": "#/defines/inherit" + }, + "before-test": { + "$ref": "#/defines/inherit" + }, + "config-settings": { + "$ref": "#/defines/inherit" + }, + "container-engine": { + "$ref": "#/defines/inherit" + }, + "environment": { + "$ref": "#/defines/inherit" + }, + "environment-pass": { + "$ref": "#/defines/inherit" + }, + "repair-wheel-command": { + "$ref": "#/defines/inherit" + }, + "test-command": { + "$ref": "#/defines/inherit" + }, + "test-extras": { + "$ref": "#/defines/inherit" + }, + "test-requires": { + "$ref": "#/defines/inherit" + } } }, "before-all": { diff --git a/docs/options.md b/docs/options.md index 9dc63ce85..a57e0f1f4 100644 --- a/docs/options.md +++ b/docs/options.md @@ -135,8 +135,9 @@ trigger new containers, one per image. Some commands are not supported; `output-dir`, build/skip/test_skip selectors, and architectures cannot be overridden. -You can specify a list of options in `inherit=[]`, any list or table in this -list will inherit from previous overrides or the main configuration. +You can specify a table of overrides in `inherit={}`, any list or table in this +list will inherit from previous overrides or the main configuration. The valid +options are `"none"` (the default), `"append"`, and `"prepend"`. ##### Examples: @@ -179,13 +180,21 @@ test-command = ["pyproject"] [[tool.cibuildwheel.overrides]] select = "cp311*" -inherit = ["test-command", "environment"] -test-command = ["pyproject-override"] + +inherit.test-command="prepend" +test-command = ["pyproject-before"] + +inherit.environment="append" environment = {FOO="BAZ", "PYTHON"="MONTY"} + +[[tool.cibuildwheel.overrides]] +select = "cp311*" +inherit.test-command="append" +test-command = ["pyproject-after"] ``` -This example will provide the command `["pyproject", "pyproject-override"]` on -Python 3.11, and will have `environment = {FOO="BAZ", "PYTHON"="MONTY", "HAM"="EGGS"}`. +This example will provide the command `"pyproject-before && pyproject && pyproject-after"` +on Python 3.11, and will have `environment = {FOO="BAZ", "PYTHON"="MONTY", "HAM"="EGGS"}`. ## Options summary diff --git a/unit_test/options_toml_test.py b/unit_test/options_toml_test.py index 996e4db7b..547a77352 100644 --- a/unit_test/options_toml_test.py +++ b/unit_test/options_toml_test.py @@ -4,7 +4,7 @@ import pytest -from cibuildwheel.options import ConfigOptionError, OptionsReader, _dig_first +from cibuildwheel.options import ConfigOptionError, Inherit, OptionsReader, _dig_first PYPROJECT_1 = """ [tool.cibuildwheel] @@ -262,7 +262,7 @@ def test_environment_override_empty(tmp_path): assert options_reader.get("manylinux-aarch64-image", ignore_empty=True) == "manylinux1" -@pytest.mark.parametrize("ignore_empty", [True, False]) +@pytest.mark.parametrize("ignore_empty", [True, False], ids=["ignore_empty", "no_ignore_empty"]) def test_dig_first(ignore_empty): d1 = {"random": "thing"} d2 = {"this": "that", "empty": ""} @@ -270,54 +270,55 @@ def test_dig_first(ignore_empty): d4 = {"this": "d4", "empty": "not"} answer = _dig_first( - (d1, "empty", False), - (d2, "empty", False), - (d3, "empty", False), - (d4, "empty", False), + (d1, "empty", Inherit.NONE), + (d2, "empty", Inherit.NONE), + (d3, "empty", Inherit.NONE), + (d4, "empty", Inherit.NONE), ignore_empty=ignore_empty, ) assert answer == ("not" if ignore_empty else "") answer = _dig_first( - (d1, "this", False), - (d2, "this", False), - (d3, "this", False), - (d4, "this", False), + (d1, "this", Inherit.NONE), + (d2, "this", Inherit.NONE), + (d3, "this", Inherit.NONE), + (d4, "this", Inherit.NONE), ignore_empty=ignore_empty, ) assert answer == "that" with pytest.raises(KeyError): _dig_first( - (d1, "this", False), - (d2, "other", False), - (d3, "this", False), - (d4, "other", False), + (d1, "this", Inherit.NONE), + (d2, "other", Inherit.NONE), + (d3, "this", Inherit.NONE), + (d4, "other", Inherit.NONE), ignore_empty=ignore_empty, ) -@pytest.mark.parametrize("ignore_empty", [True, False]) -@pytest.mark.parametrize("end", [True, False]) -def test_dig_first_merge_list(ignore_empty, end): +@pytest.mark.parametrize("ignore_empty", [True, False], ids=["ignore_empty", "no_ignore_empty"]) +@pytest.mark.parametrize("end", [Inherit.PREPEND, Inherit.NONE, Inherit.APPEND]) +@pytest.mark.parametrize("append", [True, False], ids=["append", "prepend"]) +def test_dig_first_merge_list(ignore_empty, end, append): d1 = {"random": ["thing"]} - d2 = {"this": ["that"], "empty": ""} + d2 = {"this": ["d2a", "d2b"], "empty": ""} d3 = {"other": ["hi"]} - d4 = {"this": ["d4"], "empty": ["not"]} + d4 = {"this": ["d4a", "d4b"], "empty": ["not"]} answer = _dig_first( - (d1, "this", False), - (d2, "this", True), - (d3, "this", False), + (d1, "this", Inherit.NONE), + (d2, "this", Inherit.APPEND if append else Inherit.PREPEND), + (d3, "this", Inherit.NONE), (d4, "this", end), ignore_empty=ignore_empty, ) - assert answer == ["d4", "that"] + assert answer == (["d4a", "d4b", "d2a", "d2b"] if append else ["d2a", "d2b", "d4a", "d4b"]) -@pytest.mark.parametrize("ignore_empty", [True, False]) -@pytest.mark.parametrize("end", [True, False]) +@pytest.mark.parametrize("ignore_empty", [True, False], ids=["ignore_empty", "no_ignore_empty"]) +@pytest.mark.parametrize("end", [Inherit.PREPEND, Inherit.NONE, Inherit.APPEND]) def test_dig_first_merge_dict(ignore_empty, end): d1 = {"random": {"a": "thing"}} d2 = {"this": {"b": "that"}} @@ -325,9 +326,9 @@ def test_dig_first_merge_dict(ignore_empty, end): d4 = {"this": {"d": "d4"}, "empty": {"d": "not"}} answer = _dig_first( - (d1, "this", False), - (d2, "this", True), - (d3, "this", False), + (d1, "this", Inherit.NONE), + (d2, "this", Inherit.APPEND), + (d3, "this", Inherit.NONE), (d4, "this", end), ignore_empty=ignore_empty, ) @@ -349,10 +350,25 @@ def test_dig_first_merge_dict(ignore_empty, end): [[tool.cibuildwheel.overrides]] select = "cp37*" -inherit = ["test-command", "environment"] -test-command = ["pyproject-override"] +inherit = {test-command="prepend", environment="append"} +test-command = ["pyproject-override", "override2"] manylinux-x86_64-image = "manylinux2014" environment = {FOO="BAZ", "PYTHON"="MONTY"} + +[[tool.cibuildwheel.overrides]] +select = "*-final" +inherit = {test-command="append"} +test-command = ["pyproject-finalize", "finalize2"] + +[[tool.cibuildwheel.overrides]] +select = "*-final" +inherit = {test-command="append"} +test-command = ["extra-finalize"] + +[[tool.cibuildwheel.overrides]] +select = "*-final" +inherit = {test-command="prepend"} +test-command = ["extra-prepend"] """ @@ -367,7 +383,20 @@ def test_pyproject_2(tmp_path, platform): assert options_reader.get("test-command", sep=" && ") == "pyproject" with options_reader.identifier("cp37-something"): - assert options_reader.get("test-command", sep=" && ") == "pyproject && pyproject-override" + assert ( + options_reader.get("test-command", sep=" && ") + == "pyproject-override && override2 && pyproject" + ) + assert ( + options_reader.get("environment", table={"item": '{k}="{v}"', "sep": " "}) + == 'FOO="BAZ" HAM="EGGS" PYTHON="MONTY"' + ) + + with options_reader.identifier("cp37-final"): + assert ( + options_reader.get("test-command", sep=" && ") + == "extra-prepend && pyproject-override && override2 && pyproject && pyproject-finalize && finalize2 && extra-finalize" + ) assert ( options_reader.get("environment", table={"item": '{k}="{v}"', "sep": " "}) == 'FOO="BAZ" HAM="EGGS" PYTHON="MONTY"' diff --git a/unit_test/validate_schema_test.py b/unit_test/validate_schema_test.py index f58c94338..581ac9a7b 100644 --- a/unit_test/validate_schema_test.py +++ b/unit_test/validate_schema_test.py @@ -76,7 +76,7 @@ def test_overrides_valid_inherit(): example = tomllib.loads( """ [[tool.cibuildwheel.overrides]] - inherit = ["repair-wheel-command"] + inherit.repair-wheel-command = "append" select = "somestring" repair-wheel-command = ["something"] """ @@ -90,7 +90,22 @@ def test_overrides_invalid_inherit(): example = tomllib.loads( """ [[tool.cibuildwheel.overrides]] - inherit = ["something"] + inherit.something = "append" + select = "somestring" + repair-wheel-command = "something" + """ + ) + + validator = validate_pyproject.api.Validator() + with pytest.raises(validate_pyproject.error_reporting.ValidationError): + validator(example) + + +def test_overrides_invalid_inherit_value(): + example = tomllib.loads( + """ + [[tool.cibuildwheel.overrides]] + inherit.repair-wheel-command = "nothing" select = "somestring" repair-wheel-command = "something" """ From e03078ea1a7d33381cd09009381837a84e6f6cf7 Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Mon, 19 Feb 2024 09:26:08 +0000 Subject: [PATCH 3/6] Refactor: simplifying by splitting the responsibilities of _dig_first --- cibuildwheel/options.py | 129 +++++++++++++++------------------ unit_test/options_toml_test.py | 103 +++++++++++++------------- 2 files changed, 108 insertions(+), 124 deletions(-) diff --git a/cibuildwheel/options.py b/cibuildwheel/options.py index 28ff59a6e..2e254a904 100644 --- a/cibuildwheel/options.py +++ b/cibuildwheel/options.py @@ -124,7 +124,7 @@ def architectures(self) -> set[Architecture]: class Override: select_pattern: str options: dict[str, Setting] - inherit: dict[str, Inherit] + inherit: dict[str, InheritRule] MANYLINUX_OPTIONS = {f"manylinux-{build_platform}-image" for build_platform in MANYLINUX_ARCHS} @@ -152,81 +152,73 @@ class ConfigOptionError(KeyError): pass -class Inherit(enum.Enum): +class InheritRule(enum.Enum): NONE = enum.auto() APPEND = enum.auto() PREPEND = enum.auto() -def _dig_first( - *pairs: tuple[Mapping[str, Setting], str, Inherit], ignore_empty: bool = False +def _resolve_cascade( + *pairs: tuple[Setting | None, InheritRule], ignore_empty: bool = False ) -> Setting: """ - Return the dict items that match from pairs of dicts and keys. The third - value (enum) indicates if values should merge. + Given a cascade of values with inherit rules, resolve them into a single + value. + + 'None' values mean that the option was not set at that level. + + Values start with defaults, followed by more specific rules. If rules + are NONE, the last non-null value is returned. If a rule is APPEND or + PREPEND, the value is concatenated with the previous value, according to + the rules in _merge_values. The following idiom can be used to get the first matching value: - _dig_first((dict1, "key1", Inherit.NONE), (dict2, "key2", Inherit.NONE), ...))) + _resolve_cascade(("value1", Inherit.NONE), ("value2", Inherit.NONE), ...))) """ if not pairs: msg = "pairs cannot be empty" raise ValueError(msg) - commands: list[tuple[Setting, Inherit]] = [] + result: Setting | None = None - for dict_like, key, merge in pairs: - if key in dict_like: - value = dict_like[key] - if ignore_empty and value == "": - continue + for value, rule in pairs: + if value is None: + continue - commands.append((value, merge)) - if merge == Inherit.NONE: - break - - if len(commands) == 1: - return commands[0][0] - - if len(commands) > 1: - if isinstance(commands[0][0], list): - retlist: list[str] = [] - for c in commands: - if c[1] == Inherit.PREPEND: - assert isinstance(c[0], list) - retlist.extend(c[0]) - for c in commands: - if c[1] == Inherit.NONE: - assert isinstance(c[0], list) - retlist.extend(c[0]) - for c in commands[::-1]: - if c[1] == Inherit.APPEND: - assert isinstance(c[0], list) - retlist.extend(c[0]) - return retlist - - if isinstance(commands[0][0], dict): - retdict: dict[str, str] = {} - for c in commands: - if c[1] == Inherit.PREPEND: - assert isinstance(c[0], dict) - retdict.update(c[0]) - for c in commands: - if c[1] == Inherit.NONE: - assert isinstance(c[0], dict) - retdict.update(c[0]) - for c in commands[::-1]: - if c[1] == Inherit.APPEND: - assert isinstance(c[0], dict) - retdict.update(c[0]) - return retdict + if ignore_empty and not value: + continue + if result is None: # noqa: SIM108 + result = value else: - msg = f"Must be list or dict, got {type(commands[0])}" - raise TypeError(msg) + result = _merge_values(result, value, rule) - last_key = pairs[-1][1] - raise KeyError(last_key) + if result is None: + msg = "a setting should at least have a default value" + raise ValueError(msg) + + return result + + +def _merge_values(before: Setting, after: Setting, rule: InheritRule) -> Setting: + if rule == InheritRule.NONE: + return after + + if isinstance(before, list) and isinstance(after, list): + if rule == InheritRule.APPEND: + return before + after + else: + return after + before + + if isinstance(before, dict) and isinstance(after, dict): + if rule == InheritRule.APPEND: + return {**before, **after} + else: + return {**after, **before} + + msg = f"Cannot merge {type(before)} and {type(after)} with {rule}" + raise TypeError(msg) class OptionsReader: @@ -306,7 +298,7 @@ def __init__( msg = "'inherit' must be a dict containing only {'none', 'append', 'prepend'} values" raise ConfigOptionError(msg) - inherit_enum = {k: Inherit[v.upper()] for k, v in inherit.items()} + inherit_enum = {k: InheritRule[v.upper()] for k, v in inherit.items()} self.overrides.append(Override(select, config_override, inherit_enum)) @@ -399,22 +391,19 @@ def get( envvar = f"CIBW_{name.upper().replace('-', '_')}" plat_envvar = f"{envvar}_{self.platform.upper()}" - # later overrides take precedence over earlier ones, so reverse the list - active_config_overrides = reversed(self.active_config_overrides) - - # get the option from the environment, then the config file, then finally the default. + # get the option from the default, then the config file, then finally the environment. # platform-specific options are preferred, if they're allowed. - result = _dig_first( - (self.env if env_plat else {}, plat_envvar, Inherit.NONE), - (self.env, envvar, Inherit.NONE), + result = _resolve_cascade( + (self.default_options.get(name), InheritRule.NONE), + (self.default_platform_options.get(name), InheritRule.NONE), + (self.config_options.get(name), InheritRule.NONE), + (self.config_platform_options.get(name), InheritRule.NONE), *[ - (o.options, name, o.inherit.get(name, Inherit.NONE)) - for o in active_config_overrides + (o.options.get(name), o.inherit.get(name, InheritRule.NONE)) + for o in self.active_config_overrides ], - (self.config_platform_options, name, Inherit.NONE), - (self.config_options, name, Inherit.NONE), - (self.default_platform_options, name, Inherit.NONE), - (self.default_options, name, Inherit.NONE), + (self.env.get(envvar), InheritRule.NONE), + (self.env.get(plat_envvar) if env_plat else None, InheritRule.NONE), ignore_empty=ignore_empty, ) diff --git a/unit_test/options_toml_test.py b/unit_test/options_toml_test.py index 547a77352..7a69b4574 100644 --- a/unit_test/options_toml_test.py +++ b/unit_test/options_toml_test.py @@ -4,7 +4,7 @@ import pytest -from cibuildwheel.options import ConfigOptionError, Inherit, OptionsReader, _dig_first +from cibuildwheel.options import ConfigOptionError, InheritRule, OptionsReader, _resolve_cascade PYPROJECT_1 = """ [tool.cibuildwheel] @@ -263,77 +263,72 @@ def test_environment_override_empty(tmp_path): @pytest.mark.parametrize("ignore_empty", [True, False], ids=["ignore_empty", "no_ignore_empty"]) -def test_dig_first(ignore_empty): - d1 = {"random": "thing"} - d2 = {"this": "that", "empty": ""} - d3 = {"other": "hi"} - d4 = {"this": "d4", "empty": "not"} - - answer = _dig_first( - (d1, "empty", Inherit.NONE), - (d2, "empty", Inherit.NONE), - (d3, "empty", Inherit.NONE), - (d4, "empty", Inherit.NONE), +def test_resolve_cascade(ignore_empty): + answer = _resolve_cascade( + ("not", InheritRule.NONE), + (None, InheritRule.NONE), + ("", InheritRule.NONE), + (None, InheritRule.NONE), ignore_empty=ignore_empty, ) assert answer == ("not" if ignore_empty else "") - answer = _dig_first( - (d1, "this", Inherit.NONE), - (d2, "this", Inherit.NONE), - (d3, "this", Inherit.NONE), - (d4, "this", Inherit.NONE), + answer = _resolve_cascade( + ("d4", InheritRule.NONE), + (None, InheritRule.NONE), + ("that", InheritRule.NONE), + (None, InheritRule.NONE), ignore_empty=ignore_empty, ) assert answer == "that" - with pytest.raises(KeyError): - _dig_first( - (d1, "this", Inherit.NONE), - (d2, "other", Inherit.NONE), - (d3, "this", Inherit.NONE), - (d4, "other", Inherit.NONE), + with pytest.raises(ValueError, match="a setting should at least have a default value"): + _resolve_cascade( + (None, InheritRule.NONE), + (None, InheritRule.NONE), + (None, InheritRule.NONE), + (None, InheritRule.NONE), ignore_empty=ignore_empty, ) @pytest.mark.parametrize("ignore_empty", [True, False], ids=["ignore_empty", "no_ignore_empty"]) -@pytest.mark.parametrize("end", [Inherit.PREPEND, Inherit.NONE, Inherit.APPEND]) -@pytest.mark.parametrize("append", [True, False], ids=["append", "prepend"]) -def test_dig_first_merge_list(ignore_empty, end, append): - d1 = {"random": ["thing"]} - d2 = {"this": ["d2a", "d2b"], "empty": ""} - d3 = {"other": ["hi"]} - d4 = {"this": ["d4a", "d4b"], "empty": ["not"]} - - answer = _dig_first( - (d1, "this", Inherit.NONE), - (d2, "this", Inherit.APPEND if append else Inherit.PREPEND), - (d3, "this", Inherit.NONE), - (d4, "this", end), +@pytest.mark.parametrize("rule", [InheritRule.PREPEND, InheritRule.NONE, InheritRule.APPEND]) +def test_resolve_cascade_merge_list(ignore_empty, rule): + answer = _resolve_cascade( + (["a1", "a2"], InheritRule.NONE), + ([], InheritRule.NONE), + (["b1", "b2"], rule), + (None, InheritRule.NONE), ignore_empty=ignore_empty, ) - assert answer == (["d4a", "d4b", "d2a", "d2b"] if append else ["d2a", "d2b", "d4a", "d4b"]) - - -@pytest.mark.parametrize("ignore_empty", [True, False], ids=["ignore_empty", "no_ignore_empty"]) -@pytest.mark.parametrize("end", [Inherit.PREPEND, Inherit.NONE, Inherit.APPEND]) -def test_dig_first_merge_dict(ignore_empty, end): - d1 = {"random": {"a": "thing"}} - d2 = {"this": {"b": "that"}} - d3 = {"other": {"c": "ho"}} - d4 = {"this": {"d": "d4"}, "empty": {"d": "not"}} - - answer = _dig_first( - (d1, "this", Inherit.NONE), - (d2, "this", Inherit.APPEND), - (d3, "this", Inherit.NONE), - (d4, "this", end), - ignore_empty=ignore_empty, + if not ignore_empty: + assert answer == ["b1", "b2"] + else: + if rule == InheritRule.PREPEND: + assert answer == ["b1", "b2", "a1", "a2"] + elif rule == InheritRule.NONE: + assert answer == ["b1", "b2"] + elif rule == InheritRule.APPEND: + assert answer == ["a1", "a2", "b1", "b2"] + + +@pytest.mark.parametrize("rule", [InheritRule.PREPEND, InheritRule.NONE, InheritRule.APPEND]) +def test_resolve_cascade_merge_dict(rule): + answer = _resolve_cascade( + ({"value": "a1", "base": "b1"}, InheritRule.NONE), + (None, InheritRule.NONE), + ({"value": "override"}, rule), + (None, InheritRule.NONE), ) - assert answer == {"b": "that", "d": "d4"} + if rule == InheritRule.PREPEND: + assert answer == {"value": "a1", "base": "b1"} + elif rule == InheritRule.NONE: + assert answer == {"value": "override"} + elif rule == InheritRule.APPEND: + assert answer == {"value": "override", "base": "b1"} PYPROJECT_2 = """ From 5f3a5decabad009e90b10f0e462863910098acda Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Tue, 20 Feb 2024 22:41:31 +0000 Subject: [PATCH 4/6] Refactor to allow merging of string settings, and preserve table cascades --- cibuildwheel/options.py | 155 +++++++++++++++++++-------------- unit_test/options_test.py | 68 +++++++++++++++ unit_test/options_toml_test.py | 104 +++++++++++++++------- 3 files changed, 233 insertions(+), 94 deletions(-) diff --git a/cibuildwheel/options.py b/cibuildwheel/options.py index 2e254a904..a69664554 100644 --- a/cibuildwheel/options.py +++ b/cibuildwheel/options.py @@ -18,7 +18,7 @@ from packaging.specifiers import SpecifierSet from ._compat import tomllib -from ._compat.typing import NotRequired +from ._compat.typing import NotRequired, assert_never from .architecture import Architecture from .environment import EnvironmentParseError, ParsedEnvironment, parse_environment from .logger import log @@ -159,18 +159,21 @@ class InheritRule(enum.Enum): def _resolve_cascade( - *pairs: tuple[Setting | None, InheritRule], ignore_empty: bool = False -) -> Setting: + *pairs: tuple[Setting | None, InheritRule], + ignore_empty: bool = False, + list_sep: str | None = None, + table_format: TableFmt | None = None, +) -> str: """ Given a cascade of values with inherit rules, resolve them into a single value. - 'None' values mean that the option was not set at that level. + 'None' values mean that the option was not set at that level, and are + ignored. If `ignore_empty` is True, empty values are ignored too. - Values start with defaults, followed by more specific rules. If rules - are NONE, the last non-null value is returned. If a rule is APPEND or - PREPEND, the value is concatenated with the previous value, according to - the rules in _merge_values. + Values start with defaults, followed by more specific rules. If rules are + NONE, the last non-null value is returned. If a rule is APPEND or PREPEND, + the value is concatenated with the previous value. The following idiom can be used to get the first matching value: @@ -180,7 +183,14 @@ def _resolve_cascade( msg = "pairs cannot be empty" raise ValueError(msg) - result: Setting | None = None + result: str | None = None + + if table_format is not None: + merge_sep = table_format["sep"] + elif list_sep is not None: + merge_sep = list_sep + else: + merge_sep = None for value, rule in pairs: if value is None: @@ -189,10 +199,14 @@ def _resolve_cascade( if ignore_empty and not value: continue - if result is None: # noqa: SIM108 - result = value - else: - result = _merge_values(result, value, rule) + value_string = _stringify_setting(value, list_sep, table_format) + + result = _merge_values( + result, + value_string, + rule=rule, + merge_sep=merge_sep, + ) if result is None: msg = "a setting should at least have a default value" @@ -201,24 +215,52 @@ def _resolve_cascade( return result -def _merge_values(before: Setting, after: Setting, rule: InheritRule) -> Setting: +def _merge_values(before: str | None, after: str, rule: InheritRule, merge_sep: str | None) -> str: if rule == InheritRule.NONE: return after - if isinstance(before, list) and isinstance(after, list): - if rule == InheritRule.APPEND: - return before + after - else: - return after + before + if not before: + # if before is None, we can just return after + # if before is an empty string, we shouldn't add any separator + return after - if isinstance(before, dict) and isinstance(after, dict): - if rule == InheritRule.APPEND: - return {**before, **after} - else: - return {**after, **before} + if not after: + # if after is an empty string, we shouldn't add any separator + return before + + if not merge_sep: + msg = f"Don't know how to merge {before!r} and {after!r} with {rule}" + raise ConfigOptionError(msg) + + if rule == InheritRule.APPEND: + return f"{before}{merge_sep}{after}" + elif rule == InheritRule.PREPEND: + return f"{after}{merge_sep}{before}" + else: + assert_never(rule) + + +def _stringify_setting( + setting: Setting, list_sep: str | None, table_format: TableFmt | None +) -> str: + if isinstance(setting, Mapping): + if table_format is None: + msg = f"Error converting {setting!r} to a string: this setting doesn't accept a table" + raise ConfigOptionError(msg) + return table_format["sep"].join( + item for k, v in setting.items() for item in _inner_fmt(k, v, table_format) + ) + + if not isinstance(setting, str) and isinstance(setting, Sequence): + if list_sep is None: + msg = f"Error converting {setting!r} to a string: this setting doesn't accept a list" + raise ConfigOptionError(msg) + return list_sep.join(setting) - msg = f"Cannot merge {type(before)} and {type(after)} with {rule}" - raise TypeError(msg) + if isinstance(setting, int): + return str(setting) + + return setting class OptionsReader: @@ -368,8 +410,8 @@ def get( name: str, *, env_plat: bool = True, - sep: str | None = None, - table: TableFmt | None = None, + list_sep: str | None = None, + table_format: TableFmt | None = None, ignore_empty: bool = False, ) -> str: """ @@ -393,7 +435,7 @@ def get( # get the option from the default, then the config file, then finally the environment. # platform-specific options are preferred, if they're allowed. - result = _resolve_cascade( + return _resolve_cascade( (self.default_options.get(name), InheritRule.NONE), (self.default_platform_options.get(name), InheritRule.NONE), (self.config_options.get(name), InheritRule.NONE), @@ -405,27 +447,10 @@ def get( (self.env.get(envvar), InheritRule.NONE), (self.env.get(plat_envvar) if env_plat else None, InheritRule.NONE), ignore_empty=ignore_empty, + list_sep=list_sep, + table_format=table_format, ) - if isinstance(result, Mapping): - if table is None: - msg = f"{name!r} does not accept a table" - raise ConfigOptionError(msg) - return table["sep"].join( - item for k, v in result.items() for item in _inner_fmt(k, v, table) - ) - - if not isinstance(result, str) and isinstance(result, Sequence): - if sep is None: - msg = f"{name!r} does not accept a list" - raise ConfigOptionError(msg) - return sep.join(result) - - if isinstance(result, int): - return str(result) - - return result - def _inner_fmt(k: str, v: Any, table: TableFmt) -> Iterator[str]: quote_function = table.get("quote", lambda a: a) @@ -486,9 +511,9 @@ def globals(self) -> GlobalOptions: package_dir = args.package_dir output_dir = args.output_dir - build_config = self.reader.get("build", env_plat=False, sep=" ") or "*" - skip_config = self.reader.get("skip", env_plat=False, sep=" ") - test_skip = self.reader.get("test-skip", env_plat=False, sep=" ") + build_config = self.reader.get("build", env_plat=False, list_sep=" ") or "*" + skip_config = self.reader.get("skip", env_plat=False, list_sep=" ") + test_skip = self.reader.get("test-skip", env_plat=False, list_sep=" ") prerelease_pythons = args.prerelease_pythons or strtobool( self.env.get("CIBW_PRERELEASE_PYTHONS", "0") @@ -501,7 +526,7 @@ def globals(self) -> GlobalOptions: ) requires_python = None if requires_python_str is None else SpecifierSet(requires_python_str) - archs_config_str = args.archs or self.reader.get("archs", sep=" ") + archs_config_str = args.archs or self.reader.get("archs", list_sep=" ") architectures = Architecture.parse_config(archs_config_str, platform=self.platform) # Process `--only` @@ -520,7 +545,8 @@ def globals(self) -> GlobalOptions: test_selector = TestSelector(skip_config=test_skip) container_engine_str = self.reader.get( - "container-engine", table={"item": "{k}:{v}", "sep": "; ", "quote": shlex.quote} + "container-engine", + table_format={"item": "{k}:{v}", "sep": "; ", "quote": shlex.quote}, ) try: @@ -545,29 +571,30 @@ def build_options(self, identifier: str | None) -> BuildOptions: """ with self.reader.identifier(identifier): - before_all = self.reader.get("before-all", sep=" && ") + before_all = self.reader.get("before-all", list_sep=" && ") environment_config = self.reader.get( - "environment", table={"item": '{k}="{v}"', "sep": " "} + "environment", table_format={"item": '{k}="{v}"', "sep": " "} ) - environment_pass = self.reader.get("environment-pass", sep=" ").split() - before_build = self.reader.get("before-build", sep=" && ") - repair_command = self.reader.get("repair-wheel-command", sep=" && ") + environment_pass = self.reader.get("environment-pass", list_sep=" ").split() + before_build = self.reader.get("before-build", list_sep=" && ") + repair_command = self.reader.get("repair-wheel-command", list_sep=" && ") config_settings = self.reader.get( - "config-settings", table={"item": "{k}={v}", "sep": " ", "quote": shlex.quote} + "config-settings", + table_format={"item": "{k}={v}", "sep": " ", "quote": shlex.quote}, ) dependency_versions = self.reader.get("dependency-versions") - test_command = self.reader.get("test-command", sep=" && ") - before_test = self.reader.get("before-test", sep=" && ") - test_requires = self.reader.get("test-requires", sep=" ").split() - test_extras = self.reader.get("test-extras", sep=",") + test_command = self.reader.get("test-command", list_sep=" && ") + before_test = self.reader.get("before-test", list_sep=" && ") + test_requires = self.reader.get("test-requires", list_sep=" ").split() + test_extras = self.reader.get("test-extras", list_sep=",") build_verbosity_str = self.reader.get("build-verbosity") build_frontend_str = self.reader.get( "build-frontend", env_plat=False, - table={"item": "{k}:{v}", "sep": "; ", "quote": shlex.quote}, + table_format={"item": "{k}:{v}", "sep": "; ", "quote": shlex.quote}, ) build_frontend: BuildFrontendConfig | None if not build_frontend_str or build_frontend_str == "default": diff --git a/unit_test/options_test.py b/unit_test/options_test.py index c2163090f..cf03f8548 100644 --- a/unit_test/options_test.py +++ b/unit_test/options_test.py @@ -347,3 +347,71 @@ def test_build_frontend_option(tmp_path: Path, toml_assignment, result_name, res assert parsed_build_frontend.args == result_args else: assert parsed_build_frontend is None + + +def test_override_inherit_environment(tmp_path: Path): + args = CommandLineArguments.defaults() + args.package_dir = tmp_path + + pyproject_toml: Path = tmp_path / "pyproject.toml" + pyproject_toml.write_text( + textwrap.dedent( + """\ + [tool.cibuildwheel] + environment = {FOO="BAR", "HAM"="EGGS"} + + [[tool.cibuildwheel.overrides]] + select = "cp37*" + inherit.environment = "append" + environment = {FOO="BAZ", "PYTHON"="MONTY"} + """ + ) + ) + + options = Options(platform="linux", command_line_arguments=args, env={}) + parsed_environment = options.build_options(identifier=None).environment + assert parsed_environment.as_dictionary(prev_environment={}) == { + "FOO": "BAR", + "HAM": "EGGS", + } + + assert options.build_options("cp37-manylinux_x86_64").environment.as_dictionary( + prev_environment={} + ) == { + "FOO": "BAZ", + "HAM": "EGGS", + "PYTHON": "MONTY", + } + + +def test_override_inherit_environment_with_references(tmp_path: Path): + args = CommandLineArguments.defaults() + args.package_dir = tmp_path + + pyproject_toml: Path = tmp_path / "pyproject.toml" + pyproject_toml.write_text( + textwrap.dedent( + """\ + [tool.cibuildwheel] + environment = {PATH="/opt/bin:$PATH"} + + [[tool.cibuildwheel.overrides]] + select = "cp37*" + inherit.environment = "append" + environment = {PATH="/opt/local/bin:$PATH"} + """ + ) + ) + + options = Options(platform="linux", command_line_arguments=args, env={"MONTY": "PYTHON"}) + parsed_environment = options.build_options(identifier=None).environment + prev_environment = {"PATH": "/usr/bin:/bin"} + assert parsed_environment.as_dictionary(prev_environment=prev_environment) == { + "PATH": "/opt/bin:/usr/bin:/bin", + } + + assert options.build_options("cp37-manylinux_x86_64").environment.as_dictionary( + prev_environment=prev_environment + ) == { + "PATH": "/opt/local/bin:/opt/bin:/usr/bin:/bin", + } diff --git a/unit_test/options_toml_test.py b/unit_test/options_toml_test.py index 7a69b4574..6d3fc50e9 100644 --- a/unit_test/options_toml_test.py +++ b/unit_test/options_toml_test.py @@ -37,27 +37,31 @@ def test_simple_settings(tmp_path, platform, fname): options_reader = OptionsReader(config_file_path, platform=platform, env={}) - assert options_reader.get("build", env_plat=False, sep=" ") == "cp39*" + assert options_reader.get("build", env_plat=False, list_sep=" ") == "cp39*" assert options_reader.get("test-command") == "pyproject" - assert options_reader.get("archs", sep=" ") == "auto" + assert options_reader.get("archs", list_sep=" ") == "auto" assert ( - options_reader.get("test-requires", sep=" ") + options_reader.get("test-requires", list_sep=" ") == {"windows": "something", "macos": "else", "linux": "other many"}[platform] ) # Also testing options for support for both lists and tables assert ( - options_reader.get("environment", table={"item": '{k}="{v}"', "sep": " "}) + options_reader.get("environment", table_format={"item": '{k}="{v}"', "sep": " "}) == 'THING="OTHER" FOO="BAR"' ) assert ( - options_reader.get("environment", sep="x", table={"item": '{k}="{v}"', "sep": " "}) + options_reader.get( + "environment", list_sep="x", table_format={"item": '{k}="{v}"', "sep": " "} + ) == 'THING="OTHER" FOO="BAR"' ) - assert options_reader.get("test-extras", sep=",") == "one,two" + assert options_reader.get("test-extras", list_sep=",") == "one,two" assert ( - options_reader.get("test-extras", sep=",", table={"item": '{k}="{v}"', "sep": " "}) + options_reader.get( + "test-extras", list_sep=",", table_format={"item": '{k}="{v}"', "sep": " "} + ) == "one,two" ) @@ -65,10 +69,10 @@ def test_simple_settings(tmp_path, platform, fname): assert options_reader.get("manylinux-i686-image") == "manylinux2014" with pytest.raises(ConfigOptionError): - options_reader.get("environment", sep=" ") + options_reader.get("environment", list_sep=" ") with pytest.raises(ConfigOptionError): - options_reader.get("test-extras", table={"item": '{k}="{v}"', "sep": " "}) + options_reader.get("test-extras", table_format={"item": '{k}="{v}"', "sep": " "}) def test_envvar_override(tmp_path, platform): @@ -87,14 +91,14 @@ def test_envvar_override(tmp_path, platform): }, ) - assert options_reader.get("archs", sep=" ") == "auto" + assert options_reader.get("archs", list_sep=" ") == "auto" - assert options_reader.get("build", sep=" ") == "cp38*" + assert options_reader.get("build", list_sep=" ") == "cp38*" assert options_reader.get("manylinux-x86_64-image") == "manylinux_2_24" assert options_reader.get("manylinux-i686-image") == "manylinux2014" assert ( - options_reader.get("test-requires", sep=" ") + options_reader.get("test-requires", list_sep=" ") == {"windows": "docs", "macos": "docs", "linux": "scod"}[platform] ) assert options_reader.get("test-command") == "mytest" @@ -215,7 +219,7 @@ def test_unsupported_join(tmp_path): ) options_reader = OptionsReader(pyproject_toml, platform="linux", env={}) - assert options_reader.get("build", sep=", ") == "1, 2" + assert options_reader.get("build", list_sep=", ") == "1, 2" with pytest.raises(ConfigOptionError): options_reader.get("build") @@ -301,17 +305,18 @@ def test_resolve_cascade_merge_list(ignore_empty, rule): (["b1", "b2"], rule), (None, InheritRule.NONE), ignore_empty=ignore_empty, + list_sep=" ", ) if not ignore_empty: - assert answer == ["b1", "b2"] + assert answer == "b1 b2" else: if rule == InheritRule.PREPEND: - assert answer == ["b1", "b2", "a1", "a2"] + assert answer == "b1 b2 a1 a2" elif rule == InheritRule.NONE: - assert answer == ["b1", "b2"] + assert answer == "b1 b2" elif rule == InheritRule.APPEND: - assert answer == ["a1", "a2", "b1", "b2"] + assert answer == "a1 a2 b1 b2" @pytest.mark.parametrize("rule", [InheritRule.PREPEND, InheritRule.NONE, InheritRule.APPEND]) @@ -321,14 +326,24 @@ def test_resolve_cascade_merge_dict(rule): (None, InheritRule.NONE), ({"value": "override"}, rule), (None, InheritRule.NONE), + table_format={"item": "{k}={v}", "sep": " "}, ) if rule == InheritRule.PREPEND: - assert answer == {"value": "a1", "base": "b1"} + assert answer == "value=override value=a1 base=b1" elif rule == InheritRule.NONE: - assert answer == {"value": "override"} + assert answer == "value=override" elif rule == InheritRule.APPEND: - assert answer == {"value": "override", "base": "b1"} + assert answer == "value=a1 base=b1 value=override" + + +def test_resolve_cascade_merge_different_types(): + answer = _resolve_cascade( + ({"value": "a1", "base": "b1"}, InheritRule.NONE), + ({"value": "override"}, InheritRule.APPEND), + table_format={"item": "{k}={v}", "sep": " "}, + ) + assert answer == "value=a1 base=b1 value=override" PYPROJECT_2 = """ @@ -372,29 +387,29 @@ def test_pyproject_2(tmp_path, platform): pyproject_toml.write_text(PYPROJECT_2) options_reader = OptionsReader(config_file_path=pyproject_toml, platform=platform, env={}) - assert options_reader.get("test-command", sep=" && ") == "pyproject" + assert options_reader.get("test-command", list_sep=" && ") == "pyproject" with options_reader.identifier("random"): - assert options_reader.get("test-command", sep=" && ") == "pyproject" + assert options_reader.get("test-command", list_sep=" && ") == "pyproject" with options_reader.identifier("cp37-something"): assert ( - options_reader.get("test-command", sep=" && ") + options_reader.get("test-command", list_sep=" && ") == "pyproject-override && override2 && pyproject" ) assert ( - options_reader.get("environment", table={"item": '{k}="{v}"', "sep": " "}) - == 'FOO="BAZ" HAM="EGGS" PYTHON="MONTY"' + options_reader.get("environment", table_format={"item": '{k}="{v}"', "sep": " "}) + == 'FOO="BAR" HAM="EGGS" FOO="BAZ" PYTHON="MONTY"' ) with options_reader.identifier("cp37-final"): assert ( - options_reader.get("test-command", sep=" && ") + options_reader.get("test-command", list_sep=" && ") == "extra-prepend && pyproject-override && override2 && pyproject && pyproject-finalize && finalize2 && extra-finalize" ) assert ( - options_reader.get("environment", table={"item": '{k}="{v}"', "sep": " "}) - == 'FOO="BAZ" HAM="EGGS" PYTHON="MONTY"' + options_reader.get("environment", table_format={"item": '{k}="{v}"', "sep": " "}) + == 'FOO="BAR" HAM="EGGS" FOO="BAZ" PYTHON="MONTY"' ) @@ -427,7 +442,7 @@ def test_config_settings(tmp_path): options_reader = OptionsReader(config_file_path=pyproject_toml, platform="linux", env={}) assert ( - options_reader.get("config-settings", table={"item": '{k}="{v}"', "sep": " "}) + options_reader.get("config-settings", table_format={"item": '{k}="{v}"', "sep": " "}) == 'example="one" other="two" other="three"' ) @@ -444,7 +459,36 @@ def test_pip_config_settings(tmp_path): options_reader = OptionsReader(config_file_path=pyproject_toml, platform="linux", env={}) assert ( options_reader.get( - "config-settings", table={"item": "--config-settings='{k}=\"{v}\"'", "sep": " "} + "config-settings", table_format={"item": "--config-settings='{k}=\"{v}\"'", "sep": " "} ) == "--config-settings='--build-option=\"--use-mypyc\"'" ) + + +def test_overrides_inherit(tmp_path): + pyproject_toml: Path = tmp_path / "pyproject.toml" + pyproject_toml.write_text( + """\ +[tool.cibuildwheel] +before-all = ["before-all"] + +[[tool.cibuildwheel.overrides]] +select = "cp37*" +inherit.before-all = "append" +before-all = ["override1"] + +[[tool.cibuildwheel.overrides]] +select = "cp37*" +inherit.before-all = "prepend" +before-all = ["override2"] +""" + ) + + options_reader = OptionsReader(config_file_path=pyproject_toml, platform="linux", env={}) + with options_reader.identifier("cp38-something"): + assert options_reader.get("before-all", list_sep=" && ") == "before-all" + with options_reader.identifier("cp37-something"): + assert ( + options_reader.get("before-all", list_sep=" && ") + == "override2 && before-all && override1" + ) From f84ca3707c898f30d0ea3a7d27d4dc6d3478f29a Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Thu, 29 Feb 2024 09:56:44 -0500 Subject: [PATCH 5/6] docs: add some docs for inherit Signed-off-by: Henry Schreiner --- bin/generate_schema.py | 2 +- docs/options.md | 41 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/bin/generate_schema.py b/bin/generate_schema.py index ed3154fd1..a7007886a 100755 --- a/bin/generate_schema.py +++ b/bin/generate_schema.py @@ -289,7 +289,7 @@ def as_object(d: dict[str, Any]) -> dict[str, Any]: if args.schemastore: schema["$id"] = "https://json.schemastore.org/partial-cibuildwheel.json" - schema["$id"] = "http://json-schema.org/draft-07/schema#" + schema["$schema"] = "http://json-schema.org/draft-07/schema#" schema[ "description" ] = "cibuildwheel's toml file, generated with ./bin/generate_schema.py --schemastore from cibuildwheel." diff --git a/docs/options.md b/docs/options.md index a57e0f1f4..84380974d 100644 --- a/docs/options.md +++ b/docs/options.md @@ -196,6 +196,47 @@ test-command = ["pyproject-after"] This example will provide the command `"pyproject-before && pyproject && pyproject-after"` on Python 3.11, and will have `environment = {FOO="BAZ", "PYTHON"="MONTY", "HAM"="EGGS"}`. + +### Extending existing options {: #inherit } + +In the TOML configuration, you can choose how tables and lists are inherited. +By default, all values are overridden completely (`"none"`) but sometimes you'd +rather `"append"` or `"prepend"` to an existing list or table. You can do this +with the `inherit` table in overrides. For example, if you want to add an environment +variable for CPython 3.11, without `inherit` you'd have to repeat all the +original environment variables in the override. With `inherit`, it's just: + +```toml +[[tool.cibuildwheel.overrides]] +select = "cp311*" +inherit.environment = "append" +environment.NEWVAR = "Added!" +``` + +For a table, `"append"` will replace a key if it exists, while `"prepend"` will +only add a new key, older keys take precedence. + +Lists are also supported (and keep in mind that commands are lists). For +example, you can print a message before and after a wheel is repaired: + +```toml +[[tool.cibuildwheel.overrides]] +select = "*" +inherit.repair-wheel-command = "prepend" +repair-wheel-command = "echo 'Before repair'" + +[[tool.cibuildwheel.overrides]] +select = "*" +inherit.repair-wheel-command = "append" +repair-wheel-command = "echo 'After repair'" +``` + +As seen in this example, you can have multiple overrides match - they match top +to bottom, with the config being accumulated. If you need platform-specific +inheritance, you can use `select = "*-????linux_*"` for Linux, `select = +"*-win_*"` for Windows, and `select = "*-macosx_*"` for macOS. As always, +environment variables will completely override any TOML configuration. + ## Options summary
From e662058ce996a49a5333639cf30ca7ccfbfbc7c1 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Sat, 2 Mar 2024 14:07:37 -0500 Subject: [PATCH 6/6] Apply suggestions from code review Co-authored-by: Joe Rickerby --- docs/options.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/options.md b/docs/options.md index 84380974d..b7d8008ed 100644 --- a/docs/options.md +++ b/docs/options.md @@ -181,7 +181,7 @@ test-command = ["pyproject"] [[tool.cibuildwheel.overrides]] select = "cp311*" -inherit.test-command="prepend" +inherit.test-command = "prepend" test-command = ["pyproject-before"] inherit.environment="append" @@ -189,7 +189,7 @@ environment = {FOO="BAZ", "PYTHON"="MONTY"} [[tool.cibuildwheel.overrides]] select = "cp311*" -inherit.test-command="append" +inherit.test-command = "append" test-command = ["pyproject-after"] ```