Merge pull request #1273 from pypa/toml-environment-quoting

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:

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)
+            qv = quote_function(inner_v)
-            yield table_item.format(k=k, v=qv)
+            yield table["item"].format(k=k, v=qv)
     else:
-        qv = shlex.quote(v)
+        qv = quote_function(v)
-        yield table_item.format(k=k, v=qv)
+        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")

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",
 )
 
 

pyproject.toml

@@ -66,6 +66,7 @@ module = [
     "setuptools",
     "pytest", # ignored in pre-commit to speed up check
     "bashlex",
+    "bashlex.*",
     "importlib_resources",
     "ghapi.*",
 ]

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

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 @@ 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"]
@@ -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"',
+        pytest.param('"value wrapped in quotes"', marks=[xfail_env_parse]),
-        'an unclosed double-quote: "',
+        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:
+    tmp_path.joinpath("pyproject.toml").write_text(
-        f.write(
+        textwrap.dedent(
-            textwrap.dedent(
+            f"""\
-                f"""\
+            [tool.cibuildwheel.environment]
-        [tool.cibuildwheel.environment]
+            EXAMPLE='''{env_var_value}'''
-        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