Skip to content

Commit

Permalink
Fix shlexing of passthrough args. (cherrypick of #12547) (#12550)
Browse files Browse the repository at this point in the history
`./pants run example.py -- -flag "some args"` currently results in a different `sys.argv` for the process than `./example.py -flag "some args"` would. This was because we were applying shlexing after the source/rank of an option had been discarded, which was too late to determine whether to shlex (flag/config/env values should be shlexed: passthrough arguments should not).

Move `shell_str` `list` shlexing into `ListValueComponent` such that it is skipped when a `ListValueComponent` is manually constructed for passthrough args.

One downside of this change is that list-value addition and subtraction will now apply to the shlexed tokens of a string, rather than to the entire string (i.e `+["some args"],-["args"]` will result in a final set of `["some"]`). But I expect that list subtraction is rare for `shell_str` args.

[ci skip-rust]
[ci skip-build-wheels]
  • Loading branch information
stuhood authored Aug 12, 2021
1 parent 07535a9 commit 91b927a
Show file tree
Hide file tree
Showing 7 changed files with 54 additions and 44 deletions.
4 changes: 2 additions & 2 deletions src/python/pants/option/arg_splitter_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -218,11 +218,11 @@ def test_descoping_qualified_flags(self) -> None:

def test_passthru_args(self) -> None:
self.assert_valid_split(
"./pants test foo/bar -- -t arg",
"./pants test foo/bar -- -t 'this is the arg'",
expected_goals=["test"],
expected_scope_to_flags={"": [], "test": []},
expected_specs=["foo/bar"],
expected_passthru=["-t", "arg"],
expected_passthru=["-t", "this is the arg"],
)
self.assert_valid_split(
"./pants -farg --fff=arg compile --gg-gg=arg-arg -g test.junit --iii "
Expand Down
23 changes: 18 additions & 5 deletions src/python/pants/option/custom_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import inspect
import os
import re
import shlex
from enum import Enum
from typing import Dict, Iterable, List, Pattern, Sequence

Expand Down Expand Up @@ -121,6 +122,14 @@ def _convert_list(val, member_type, is_enum):
return [item if isinstance(item, member_type) else member_type(item) for item in converted]


def _flatten_shlexed_list(shlexed_args: Sequence[str]) -> List[str]:
"""Convert a list of shlexed args into a flattened list of individual args.
For example, ['arg1 arg2=foo', '--arg3'] would be converted to ['arg1', 'arg2=foo', '--arg3'].
"""
return [arg for shlexed_arg in shlexed_args for arg in shlex.split(shlexed_arg)]


class ListValueComponent:
"""A component of the value of a list-typed option.
Expand Down Expand Up @@ -208,6 +217,9 @@ def create(cls, value, member_type=str) -> ListValueComponent:
indicating modification instead of replacement), or any allowed member_type.
May also be a comma-separated sequence of modifications.
"""
if isinstance(value, cls): # Ensure idempotency.
return value

if isinstance(value, bytes):
value = value.decode()

Expand All @@ -220,11 +232,7 @@ def create(cls, value, member_type=str) -> ListValueComponent:
appends: Sequence[str] = []
filters: Sequence[str] = []
is_enum = inspect.isclass(member_type) and issubclass(member_type, Enum)
if isinstance(value, cls): # Ensure idempotency.
action = value._action
appends = value._appends
filters = value._filters
elif isinstance(value, (list, tuple)): # Ensure we can handle list-typed default values.
if isinstance(value, (list, tuple)): # Ensure we can handle list-typed default values.
action = cls.REPLACE
appends = value
elif value.startswith("[") or value.startswith("("):
Expand All @@ -240,6 +248,11 @@ def create(cls, value, member_type=str) -> ListValueComponent:
appends = [value]
else:
appends = _convert(f"[{value}]", list)

if member_type == shell_str:
appends = _flatten_shlexed_list(appends)
filters = _flatten_shlexed_list(filters)

return cls(action, list(appends), list(filters))

def __repr__(self) -> str:
Expand Down
17 changes: 16 additions & 1 deletion src/python/pants/option/custom_types_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,29 @@
from textwrap import dedent
from typing import Dict, List, Union

from pants.option.custom_types import DictValueComponent, ListValueComponent, UnsetBool
from pants.option.custom_types import (
DictValueComponent,
ListValueComponent,
UnsetBool,
_flatten_shlexed_list,
)
from pants.option.errors import ParseError

ValidPrimitives = Union[int, str]
ParsedList = List[ValidPrimitives]
ParsedDict = Dict[str, Union[ValidPrimitives, ParsedList]]


def test_flatten_shlexed_list() -> None:
assert _flatten_shlexed_list(["arg1", "arg2"]) == ["arg1", "arg2"]
assert _flatten_shlexed_list(["arg1 arg2"]) == ["arg1", "arg2"]
assert _flatten_shlexed_list(["arg1 arg2=foo", "--arg3"]) == ["arg1", "arg2=foo", "--arg3"]
assert _flatten_shlexed_list(["arg1='foo bar'", "arg2='baz'"]) == [
"arg1=foo bar",
"arg2=baz",
]


class CustomTypesTest(unittest.TestCase):
def assert_list_parsed(self, s: str, *, expected: ParsedList) -> None:
assert expected == ListValueComponent.create(s).val
Expand Down
11 changes: 1 addition & 10 deletions src/python/pants/option/option_util.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
# Copyright 2014 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

import shlex
from typing import List, Sequence, cast
from typing import cast

from pants.option.custom_types import dict_with_files_option

Expand All @@ -13,11 +12,3 @@ def is_list_option(kwargs) -> bool:

def is_dict_option(kwargs) -> bool:
return kwargs.get("type") in (dict, dict_with_files_option)


def flatten_shlexed_list(shlexed_args: Sequence[str]) -> List[str]:
"""Convert a list of shlexed args into a flattened list of individual args.
For example, ['arg1 arg2=foo', '--arg3'] would be converted to ['arg1', 'arg2=foo', '--arg3'].
"""
return [arg for shlexed_arg in shlexed_args for arg in shlex.split(shlexed_arg)]
17 changes: 0 additions & 17 deletions src/python/pants/option/option_util_test.py

This file was deleted.

19 changes: 15 additions & 4 deletions src/python/pants/option/options_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -1222,20 +1222,31 @@ def test_passthru_args_not_interpreted(self):
# Test that passthrough args are not interpreted.
options = Options.create(
env={},
config=self._create_config(),
config=self._create_config(
{"consumer": {"shlexed": ["from config"], "string": ["from config"]}}
),
known_scope_infos=[global_scope(), task("test"), subsystem("consumer")],
args=["./pants", "--consumer-shlexed=a", "--consumer-string=b", "test", "--", "[bar]"],
args=[
"./pants",
"--consumer-shlexed=a",
"--consumer-string=b",
"test",
"--",
"[bar]",
"multi token from passthrough",
],
)
options.register(
"consumer", "--shlexed", passthrough=True, type=list, member_type=shell_str
)
options.register("consumer", "--string", passthrough=True, type=list, member_type=str)

self.assertEqual(
["a", "[bar]"],
["from", "config", "a", "[bar]", "multi token from passthrough"],
options.for_scope("consumer").shlexed,
)
self.assertEqual(
["b", "[bar]"],
["from config", "b", "[bar]", "multi token from passthrough"],
options.for_scope("consumer").string,
)

Expand Down
7 changes: 2 additions & 5 deletions src/python/pants/option/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
Shadowing,
UnknownFlagsError,
)
from pants.option.option_util import flatten_shlexed_list, is_dict_option, is_list_option
from pants.option.option_util import is_dict_option, is_list_option
from pants.option.option_value_container import OptionValueContainer, OptionValueContainerBuilder
from pants.option.ranked_value import Rank, RankedValue
from pants.option.scope import GLOBAL_SCOPE, GLOBAL_SCOPE_CONFIG_SECTION, ScopeInfo
Expand Down Expand Up @@ -745,10 +745,7 @@ def group(value_component_type, process_val_func) -> List[RankedValue]:
if is_list_option(kwargs):

def process_list(lst):
lst = [self._convert_member_type(member_type, val) for val in lst]
if member_type == shell_str:
lst = flatten_shlexed_list(lst)
return lst
return [self._convert_member_type(member_type, val) for val in lst]

historic_ranked_vals = group(ListValueComponent, process_list)
elif is_dict_option(kwargs):
Expand Down

0 comments on commit 91b927a

Please sign in to comment.