Merge pull request #1621 from pypa/parse-kvs-colons

fix: improve parse_key_value_string to allow colons in values
pypa · Oct 3, 2023 · 77d3a5f · 77d3a5f
2 parents 9535d16 + 6879810
commit 77d3a5f
Show file tree

Hide file tree

Showing 2 changed files with 90 additions and 5 deletions.
diff --git a/cibuildwheel/util.py b/cibuildwheel/util.py
@@ -731,23 +731,26 @@ def parse_key_value_string(
 
     all_field_names = [*positional_arg_names, *kw_arg_names]
 
-    shlexer = shlex.shlex(key_value_string, posix=True, punctuation_chars=";:")
+    shlexer = shlex.shlex(key_value_string, posix=True, punctuation_chars=";")
     shlexer.commenters = ""
+    shlexer.whitespace_split = True
     parts = list(shlexer)
     # parts now looks like
-    # ['docker', ';', 'create_args',':', '--some-option=value', 'another-option']
+    # ['docker', ';', 'create_args:', '--some-option=value', 'another-option']
 
     # split by semicolon
     fields = [list(group) for k, group in itertools.groupby(parts, lambda x: x == ";") if not k]
 
     result: dict[str, list[str]] = defaultdict(list)
     for field_i, field in enumerate(fields):
-        if len(field) > 1 and field[1] == ":":
-            field_name = field[0]
-            values = field[2:]
+        # check to see if the option name is specified
+        field_name, sep, first_value = field[0].partition(":")
+        if sep:
             if field_name not in all_field_names:
                 msg = f"Failed to parse {key_value_string!r}. Unknown field name {field_name!r}"
                 raise ValueError(msg)
+
+            values = ([first_value] if first_value else []) + field[1:]
         else:
             try:
                 field_name = positional_arg_names[field_i]

diff --git a/unit_test/utils_test.py b/unit_test/utils_test.py
@@ -9,6 +9,7 @@
     find_compatible_wheel,
     fix_ansi_codes_for_github_actions,
     format_safe,
+    parse_key_value_string,
     prepare_command,
 )
 
@@ -124,3 +125,84 @@ def test_fix_ansi_codes_for_github_actions():
     output = fix_ansi_codes_for_github_actions(input)
 
     assert output == expected
+
+
+def test_parse_key_value_string():
+    assert parse_key_value_string("bar", positional_arg_names=["foo"]) == {"foo": ["bar"]}
+    assert parse_key_value_string("foo:bar", kw_arg_names=["foo"]) == {"foo": ["bar"]}
+    with pytest.raises(ValueError, match="Too many positional arguments"):
+        parse_key_value_string("bar")
+    with pytest.raises(ValueError, match="Unknown field name"):
+        parse_key_value_string("foo:bar")
+    assert parse_key_value_string("foo:bar", kw_arg_names=["foo"]) == {"foo": ["bar"]}
+    assert parse_key_value_string("foo:bar", positional_arg_names=["foo"]) == {"foo": ["bar"]}
+    assert parse_key_value_string("foo: bar", kw_arg_names=["foo"]) == {"foo": ["bar"]}
+    assert parse_key_value_string("foo: bar", kw_arg_names=["foo"]) == {"foo": ["bar"]}
+    assert parse_key_value_string("foo: bar; baz: qux", kw_arg_names=["foo", "baz"]) == {
+        "foo": ["bar"],
+        "baz": ["qux"],
+    }
+
+    # some common options
+    assert parse_key_value_string(
+        "docker; create_args: --some-option --another-option=foo",
+        positional_arg_names=["name"],
+        kw_arg_names=["create_args"],
+    ) == {
+        "name": ["docker"],
+        "create_args": ["--some-option", "--another-option=foo"],
+    }
+    # semicolon in value
+    assert parse_key_value_string(
+        "docker; create_args: --some-option='this; that'",
+        positional_arg_names=["name"],
+        kw_arg_names=["create_args"],
+    ) == {
+        "name": ["docker"],
+        "create_args": ["--some-option=this; that"],
+    }
+    # colon in value
+    assert parse_key_value_string(
+        "docker; create_args: --mount a:b",
+        positional_arg_names=["name"],
+        kw_arg_names=["create_args"],
+    ) == {
+        "name": ["docker"],
+        "create_args": ["--mount", "a:b"],
+    }
+    assert parse_key_value_string(
+        "docker;create_args:--mount a:b",
+        positional_arg_names=["name"],
+        kw_arg_names=["create_args"],
+    ) == {
+        "name": ["docker"],
+        "create_args": ["--mount", "a:b"],
+    }
+    # quoted value with spaces
+    assert parse_key_value_string(
+        "docker;create_args:'some string with spaces'",
+        positional_arg_names=["name"],
+        kw_arg_names=["create_args"],
+    ) == {
+        "name": ["docker"],
+        "create_args": ["some string with spaces"],
+    }
+
+    # colon in positional value
+    assert parse_key_value_string(
+        "docker; --mount a:b",
+        positional_arg_names=["name", "create_args"],
+    ) == {
+        "name": ["docker"],
+        "create_args": ["--mount", "a:b"],
+    }
+
+    # empty option gives empty array
+    assert parse_key_value_string(
+        "docker;create_args:",
+        positional_arg_names=["name"],
+        kw_arg_names=["create_args"],
+    ) == {
+        "name": ["docker"],
+        "create_args": [],
+    }