From 13fc768552153a9a740e16156b056d44728227ca Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Sat, 17 Sep 2022 11:15:00 +0100 Subject: [PATCH] Revert to the previous quoting behaviour for environment option --- cibuildwheel/environment.py | 7 ++++++- cibuildwheel/options.py | 23 +++++++++++++---------- cibuildwheel/typing.py | 5 +++++ pyproject.toml | 1 + unit_test/options_test.py | 16 ++++++++++++---- 5 files changed, 37 insertions(+), 15 deletions(-) diff --git a/cibuildwheel/environment.py b/cibuildwheel/environment.py index a544852a..23ea9877 100644 --- a/cibuildwheel/environment.py +++ b/cibuildwheel/environment.py @@ -4,6 +4,7 @@ import dataclasses from typing import Any, Mapping, Sequence import bashlex +import bashlex.errors from cibuildwheel.typing import Protocol @@ -33,7 +34,11 @@ def split_env_items(env_string: str) -> list[str]: if not env_string: return [] - command_node = bashlex.parsesingle(env_string) + try: + command_node = bashlex.parsesingle(env_string) + except bashlex.errors.ParsingError as e: + raise EnvironmentParseError(env_string) from e + result = [] for word_node in command_node.parts: diff --git a/cibuildwheel/options.py b/cibuildwheel/options.py index 75e367c6..5a03c6b8 100644 --- a/cibuildwheel/options.py +++ b/cibuildwheel/options.py @@ -10,7 +10,7 @@ from configparser import ConfigParser from contextlib import contextmanager from dataclasses import asdict, dataclass from pathlib import Path -from typing import Any, Dict, Generator, Iterator, List, Mapping, Union, cast +from typing import Any, Callable, Dict, Generator, Iterator, List, Mapping, Union, cast if sys.version_info >= (3, 11): import tomllib @@ -23,7 +23,7 @@ from .architecture import Architecture from .environment import EnvironmentParseError, ParsedEnvironment, parse_environment from .oci_container import ContainerEngine from .projectfiles import get_requires_python_str -from .typing import PLATFORMS, Literal, PlatformName, TypedDict +from .typing import PLATFORMS, Literal, NotRequired, PlatformName, TypedDict from .util import ( MANYLINUX_ARCHS, MUSLLINUX_ARCHS, @@ -123,6 +123,7 @@ DISALLOWED_OPTIONS = { class TableFmt(TypedDict): item: str sep: str + quote: NotRequired[Callable[[str], str]] class ConfigOptionError(KeyError): @@ -329,7 +330,7 @@ class OptionsReader: if table is None: raise ConfigOptionError(f"{name!r} does not accept a table") return table["sep"].join( - item for k, v in result.items() for item in _inner_fmt(k, v, table["item"]) + item for k, v in result.items() for item in _inner_fmt(k, v, table) ) if isinstance(result, list): @@ -343,14 +344,16 @@ class OptionsReader: return result -def _inner_fmt(k: str, v: Any, table_item: str) -> Iterator[str]: +def _inner_fmt(k: str, v: Any, table: TableFmt) -> Iterator[str]: + quote_function = table.get("quote", lambda a: a) + if isinstance(v, list): for inner_v in v: - qv = shlex.quote(inner_v) - yield table_item.format(k=k, v=qv) + qv = quote_function(inner_v) + yield table["item"].format(k=k, v=qv) else: - qv = shlex.quote(v) - yield table_item.format(k=k, v=qv) + qv = quote_function(v) + yield table["item"].format(k=k, v=qv) class Options: @@ -449,13 +452,13 @@ class Options: build_frontend_str = self.reader.get("build-frontend", env_plat=False) environment_config = self.reader.get( - "environment", table={"item": "{k}={v}", "sep": " "} + "environment", table={"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=" && ") config_settings = self.reader.get( - "config-settings", table={"item": "{k}={v}", "sep": " "} + "config-settings", table={"item": "{k}={v}", "sep": " ", "quote": shlex.quote} ) dependency_versions = self.reader.get("dependency-versions") diff --git a/cibuildwheel/typing.py b/cibuildwheel/typing.py index a546d7bf..3ffa5476 100644 --- a/cibuildwheel/typing.py +++ b/cibuildwheel/typing.py @@ -10,6 +10,10 @@ if sys.version_info < (3, 8): else: from typing import Final, Literal, OrderedDict, Protocol, TypedDict +if sys.version_info < (3, 11): + from typing_extensions import NotRequired +else: + from typing import NotRequired __all__ = ( "Final", @@ -26,6 +30,7 @@ __all__ = ( "OrderedDict", "Union", "assert_never", + "NotRequired", ) diff --git a/pyproject.toml b/pyproject.toml index 6c78cdb0..92c086e5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -66,6 +66,7 @@ module = [ "setuptools", "pytest", # ignored in pre-commit to speed up check "bashlex", + "bashlex.*", "importlib_resources", "ghapi.*", ] diff --git a/unit_test/options_test.py b/unit_test/options_test.py index 59a46228..73d558f4 100644 --- a/unit_test/options_test.py +++ b/unit_test/options_test.py @@ -62,7 +62,7 @@ test_command: 'pyproject' default_build_options = options.build_options(identifier=None) - assert default_build_options.environment == parse_environment("FOO=BAR") + assert default_build_options.environment == parse_environment('FOO="BAR"') all_pinned_container_images = _get_pinned_container_images() pinned_x86_64_container_image = all_pinned_container_images["x86_64"] @@ -122,14 +122,19 @@ def test_passthrough_evil(tmp_path, monkeypatch, env_var_value): assert parsed_environment.as_dictionary(prev_environment={}) == {"ENV_VAR": env_var_value} +xfail_env_parse = pytest.mark.xfail( + raises=SystemExit, reason="until we can figure out the right way to quote these values" +) + + @pytest.mark.parametrize( "env_var_value", [ "normal value", - '"value wrapped in quotes"', - 'an unclosed double-quote: "', + pytest.param('"value wrapped in quotes"', marks=[xfail_env_parse]), + pytest.param('an unclosed double-quote: "', marks=[xfail_env_parse]), "string\nwith\ncarriage\nreturns\n", - "a trailing backslash \\", + pytest.param("a trailing backslash \\", marks=[xfail_env_parse]), ], ) def test_toml_environment_evil(tmp_path, monkeypatch, env_var_value): @@ -163,6 +168,9 @@ def test_toml_environment_evil(tmp_path, monkeypatch, env_var_value): ('TEST_VAR="before:$PARAM:after"', "before:spam:after"), # env var extension with spaces ('TEST_VAR="before $PARAM after"', "before spam after"), + # literal $ - this test is just for reference, I'm not sure if this + # syntax will work if we change the TOML quoting behaviour + (r'TEST_VAR="before\\$after"', "before$after"), ], ) def test_toml_environment_quoting(tmp_path: Path, toml_assignment, result_value):