diff --git a/cibuildwheel/environment.py b/cibuildwheel/environment.py index a544852a2..23ea98770 100644 --- a/cibuildwheel/environment.py +++ b/cibuildwheel/environment.py @@ -4,6 +4,7 @@ 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 75e367c6d..5a03c6b8f 100644 --- a/cibuildwheel/options.py +++ b/cibuildwheel/options.py @@ -10,7 +10,7 @@ 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 .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 @@ class Override: class TableFmt(TypedDict): item: str sep: str + quote: NotRequired[Callable[[str], str]] class ConfigOptionError(KeyError): @@ -329,7 +330,7 @@ def get( 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 @@ def get( 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 @@ def build_options(self, identifier: str | None) -> BuildOptions: 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 a546d7bf3..3ffa5476b 100644 --- a/cibuildwheel/typing.py +++ b/cibuildwheel/typing.py @@ -10,6 +10,10 @@ 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 @@ "OrderedDict", "Union", "assert_never", + "NotRequired", ) diff --git a/pyproject.toml b/pyproject.toml index 6c78cdb09..92c086e58 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/setup.cfg b/setup.cfg index 4d9e9656d..8a9f4ef9a 100644 --- a/setup.cfg +++ b/setup.cfg @@ -37,7 +37,7 @@ install_requires = packaging>=20.9 platformdirs tomli;python_version < '3.11' - typing-extensions>=3.10.0.0;python_version < '3.8' + typing-extensions>=4.1.0;python_version < '3.11' python_requires = >=3.7 include_package_data = True zip_safe = False diff --git a/unit_test/options_test.py b/unit_test/options_test.py index f0fb5f63e..73d558f4e 100644 --- a/unit_test/options_test.py +++ b/unit_test/options_test.py @@ -1,11 +1,14 @@ from __future__ import annotations +import os import platform as platform_module import textwrap +from pathlib import Path import pytest from cibuildwheel.__main__ import get_build_identifiers +from cibuildwheel.bashlex_eval import local_environment_executor from cibuildwheel.environment import parse_environment from cibuildwheel.options import Options, _get_pinned_container_images @@ -59,7 +62,7 @@ def test_options_1(tmp_path, monkeypatch): 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"] @@ -119,30 +122,75 @@ 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): args = get_default_command_line_arguments() args.package_dir = tmp_path - with tmp_path.joinpath("pyproject.toml").open("w") as f: - f.write( - textwrap.dedent( - f"""\ - [tool.cibuildwheel.environment] - EXAMPLE='''{env_var_value}''' - """ - ) + tmp_path.joinpath("pyproject.toml").write_text( + textwrap.dedent( + f"""\ + [tool.cibuildwheel.environment] + EXAMPLE='''{env_var_value}''' + """ ) + ) options = Options(platform="linux", command_line_arguments=args) parsed_environment = options.build_options(identifier=None).environment assert parsed_environment.as_dictionary(prev_environment={}) == {"EXAMPLE": env_var_value} + + +@pytest.mark.parametrize( + "toml_assignment,result_value", + [ + ('TEST_VAR="simple_value"', "simple_value"), + # spaces + ('TEST_VAR="simple value"', "simple value"), + # env var + ('TEST_VAR="$PARAM"', "spam"), + ('TEST_VAR="$PARAM $PARAM"', "spam spam"), + # env var extension + ('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): + args = get_default_command_line_arguments() + args.package_dir = tmp_path + + tmp_path.joinpath("pyproject.toml").write_text( + textwrap.dedent( + f"""\ + [tool.cibuildwheel.environment] + {toml_assignment} + """ + ) + ) + + options = Options(platform="linux", command_line_arguments=args) + parsed_environment = options.build_options(identifier=None).environment + environment_values = parsed_environment.as_dictionary( + prev_environment={**os.environ, "PARAM": "spam"}, + executor=local_environment_executor, + ) + + assert environment_values["TEST_VAR"] == result_value