From 787fe1896c97b3f861d3007c33457a4973a77f1e Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Thu, 16 Sep 2021 22:29:11 -0400 Subject: [PATCH 1/2] fix: fall through if empty for manylinux --- cibuildwheel/__main__.py | 4 +-- cibuildwheel/options.py | 24 ++++++++++--- docs/options.md | 3 +- unit_test/options_toml_test.py | 64 ++++++++++++++++++++++++++++++++-- 4 files changed, 83 insertions(+), 12 deletions(-) diff --git a/cibuildwheel/__main__.py b/cibuildwheel/__main__.py index a1cb85cd0..9d1421ed9 100644 --- a/cibuildwheel/__main__.py +++ b/cibuildwheel/__main__.py @@ -291,9 +291,9 @@ def main() -> None: for build_platform in MANYLINUX_ARCHS: pinned_images = all_pinned_docker_images[build_platform] - config_value = options(f"manylinux-{build_platform}-image") + config_value = options(f"manylinux-{build_platform}-image", ignore_empty=True) - if config_value is None: + if not config_value: # default to manylinux2010 if it's available, otherwise manylinux2014 image = pinned_images.get("manylinux2010") or pinned_images.get("manylinux2014") elif config_value in pinned_images: diff --git a/cibuildwheel/options.py b/cibuildwheel/options.py index 6631d762e..e0c9023cf 100644 --- a/cibuildwheel/options.py +++ b/cibuildwheel/options.py @@ -19,7 +19,7 @@ class ConfigOptionError(KeyError): pass -def _dig_first(*pairs: Tuple[Mapping[str, Any], str]) -> Setting: +def _dig_first(*pairs: Tuple[Mapping[str, Any], str], ignore_empty: bool = False) -> Setting: """ Return the first dict item that matches from pairs of dicts and keys. Final result is will throw a KeyError if missing. @@ -27,7 +27,18 @@ def _dig_first(*pairs: Tuple[Mapping[str, Any], str]) -> Setting: _dig_first((dict1, "key1"), (dict2, "key2"), ...) """ (dict_like, key), *others = pairs - return dict_like.get(key, _dig_first(*others)) if others else dict_like[key] + if ignore_empty: + return ( + (dict_like.get(key, "") or _dig_first(*others, ignore_empty=ignore_empty)) + if others + else dict_like[key] + ) + else: + return ( + dict_like.get(key, _dig_first(*others, ignore_empty=ignore_empty)) + if others + else dict_like[key] + ) class ConfigOptions: @@ -62,7 +73,7 @@ def __init__( defaults_path = resources_dir / "defaults.toml" self.default_options, self.default_platform_options = self._load_file(defaults_path) - # load the project config file + # Load the project config file config_options: Dict[str, Any] = {} config_platform_options: Dict[str, Any] = {} @@ -75,7 +86,7 @@ def __init__( if pyproject_toml_path.exists(): config_options, config_platform_options = self._load_file(pyproject_toml_path) - # validate project config + # Validate project config for option_name in config_options: if not self._is_valid_global_option(option_name): raise ConfigOptionError(f'Option "{option_name}" not supported in a config file') @@ -129,6 +140,7 @@ def __call__( env_plat: bool = True, sep: Optional[str] = None, table: Optional[TableFmt] = None, + ignore_empty: bool = False, ) -> str: """ Get and return the value for the named option from environment, @@ -136,7 +148,8 @@ def __call__( accept platform versions of the environment variable. If this is an array it will be merged with "sep" before returning. If it is a table, it will be formatted with "table['item']" using {k} and {v} and merged - with "table['sep']". + with "table['sep']". Empty variables will not override if empty_invalid + is True. """ if name not in self.default_options and name not in self.default_platform_options: @@ -155,6 +168,7 @@ def __call__( (self.config_options, name), (self.default_platform_options, name), (self.default_options, name), + ignore_empty=ignore_empty, ) if isinstance(result, dict): diff --git a/docs/options.md b/docs/options.md index 53637f1d5..6fe44c4ca 100644 --- a/docs/options.md +++ b/docs/options.md @@ -797,8 +797,7 @@ The available options are: Set an alternative Docker image to be used for building [manylinux](https://github.com/pypa/manylinux) wheels. cibuildwheel will then pull these instead of the default images, [`quay.io/pypa/manylinux2010_x86_64`](https://quay.io/pypa/manylinux2010_x86_64), [`quay.io/pypa/manylinux2010_i686`](https://quay.io/pypa/manylinux2010_i686), [`quay.io/pypa/manylinux2010_x86_64`](https://quay.io/pypa/manylinux2010_x86_64), [`quay.io/pypa/manylinux2014_aarch64`](https://quay.io/pypa/manylinux2014_aarch64), [`quay.io/pypa/manylinux2014_ppc64le`](https://quay.io/pypa/manylinux2014_ppc64le), and [`quay.io/pypa/manylinux2014_s390x`](https://quay.io/pypa/manylinux2010_s390x). The value of this option can either be set to `manylinux1`, `manylinux2010`, `manylinux2014` or `manylinux_2_24` to use a pinned version of the [official manylinux images](https://github.com/pypa/manylinux). Alternatively, set these options to any other valid Docker image name. For PyPy, the `manylinux1` image is not available. For architectures other -than x86 (x86\_64 and i686) `manylinux2014` or `manylinux_2_24` must be used, because the first version of the manylinux specification that supports additional architectures is `manylinux2014`. - +than x86 (x86\_64 and i686) `manylinux2014` or `manylinux_2_24` must be used, because the first version of the manylinux specification that supports additional architectures is `manylinux2014`. If this option is blank, it will fall though to the next available definition (environment variable -> pyproject.toml -> default). If setting a custom Docker image, you'll need to make sure it can be used in the same way as the official, default Docker images: all necessary Python and pip versions need to be present in `/opt/python/`, and the auditwheel tool needs to be present for cibuildwheel to work. Apart from that, the architecture and relevant shared system libraries need to be compatible to the relevant standard to produce valid manylinux1/manylinux2010/manylinux2014/manylinux_2_24 wheels (see [pypa/manylinux on GitHub](https://github.com/pypa/manylinux), [PEP 513](https://www.python.org/dev/peps/pep-0513/), [PEP 571](https://www.python.org/dev/peps/pep-0571/), [PEP 599](https://www.python.org/dev/peps/pep-0599/) and [PEP 600](https://www.python.org/dev/peps/pep-0600/) for more details). diff --git a/unit_test/options_toml_test.py b/unit_test/options_toml_test.py index dce9f8f5e..d002d2098 100644 --- a/unit_test/options_toml_test.py +++ b/unit_test/options_toml_test.py @@ -1,6 +1,6 @@ import pytest -from cibuildwheel.options import ConfigOptionError, ConfigOptions +from cibuildwheel.options import ConfigOptionError, ConfigOptions, _dig_first PYPROJECT_1 = """ [tool.cibuildwheel] @@ -181,10 +181,68 @@ def test_disallowed_a(tmp_path): tmp_path.joinpath("pyproject.toml").write_text( """ [tool.cibuildwheel.windows] -manylinux-x64_86-image = "manylinux1" +manylinux-x86_64-image = "manylinux1" """ ) - disallow = {"windows": {"manylinux-x64_86-image"}} + disallow = {"windows": {"manylinux-x86_64-image"}} ConfigOptions(tmp_path, platform="linux", disallow=disallow) with pytest.raises(ConfigOptionError): ConfigOptions(tmp_path, platform="windows", disallow=disallow) + + +def test_environment_override_empty(tmp_path, monkeypatch): + tmp_path.joinpath("pyproject.toml").write_text( + """ +[tool.cibuildwheel] +manylinux-i686-image = "manylinux1" +manylinux-x86_64-image = "" +""" + ) + + monkeypatch.setenv("CIBW_MANYLINUX_I686_IMAGE", "") + monkeypatch.setenv("CIBW_MANYLINUX_AARCH64_IMAGE", "manylinux1") + + options = ConfigOptions(tmp_path, platform="linux") + + assert options("manylinux-x86_64-image") == "" + assert options("manylinux-i686-image") == "" + assert options("manylinux-aarch64-image") == "manylinux1" + + assert options("manylinux-x86_64-image", ignore_empty=True) == "manylinux2010" + assert options("manylinux-i686-image", ignore_empty=True) == "manylinux1" + assert options("manylinux-aarch64-image", ignore_empty=True) == "manylinux1" + + +@pytest.mark.parametrize("ignore_empty", (True, False)) +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"), + (d2, "empty"), + (d3, "empty"), + (d4, "empty"), + ignore_empty=ignore_empty, + ) + assert answer == ("not" if ignore_empty else "") + + answer = _dig_first( + (d1, "this"), + (d2, "this"), + (d3, "this"), + (d4, "this"), + ignore_empty=ignore_empty, + ) + assert answer == "that" + + with pytest.raises(KeyError): + _dig_first( + (d1, "this"), + (d2, "other"), + (d3, "this"), + (d4, "other"), + ignore_empty=ignore_empty, + ) From 0fccf3ad1817c6f3f67996ebe6f2ebf97be4c623 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Fri, 17 Sep 2021 13:23:29 -0400 Subject: [PATCH 2/2] refactor: review cleanup --- cibuildwheel/options.py | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/cibuildwheel/options.py b/cibuildwheel/options.py index e0c9023cf..5ee9c475d 100644 --- a/cibuildwheel/options.py +++ b/cibuildwheel/options.py @@ -19,26 +19,26 @@ class ConfigOptionError(KeyError): pass -def _dig_first(*pairs: Tuple[Mapping[str, Any], str], ignore_empty: bool = False) -> Setting: +def _dig_first(*pairs: Tuple[Mapping[str, Setting], str], ignore_empty: bool = False) -> Setting: """ Return the first dict item that matches from pairs of dicts and keys. - Final result is will throw a KeyError if missing. + Will throw a KeyError if missing. _dig_first((dict1, "key1"), (dict2, "key2"), ...) """ - (dict_like, key), *others = pairs - if ignore_empty: - return ( - (dict_like.get(key, "") or _dig_first(*others, ignore_empty=ignore_empty)) - if others - else dict_like[key] - ) - else: - return ( - dict_like.get(key, _dig_first(*others, ignore_empty=ignore_empty)) - if others - else dict_like[key] - ) + if not pairs: + raise ValueError("pairs cannot be empty") + + for dict_like, key in pairs: + if key in dict_like: + value = dict_like[key] + + if ignore_empty and value == "": + continue + + return value + + raise KeyError(key) class ConfigOptions: @@ -148,7 +148,7 @@ def __call__( accept platform versions of the environment variable. If this is an array it will be merged with "sep" before returning. If it is a table, it will be formatted with "table['item']" using {k} and {v} and merged - with "table['sep']". Empty variables will not override if empty_invalid + with "table['sep']". Empty variables will not override if ignore_empty is True. """