Skip to content

Commit

Permalink
Support options whose values are lists of dicts.
Browse files Browse the repository at this point in the history
Support list-typed options with dict, target and file members.

Testing Done:
CI passes: https://travis-ci.org/pantsbuild/pants/builds/116713915

Reviewed at https://rbcommons.com/s/twitter/r/3580/
  • Loading branch information
benjyw committed Mar 19, 2016
1 parent 3e6ce84 commit 58fd185
Show file tree
Hide file tree
Showing 12 changed files with 196 additions and 58 deletions.
1 change: 1 addition & 0 deletions contrib/go/tests/python/pants_test/contrib/go/tasks/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ python_tests(
'tests/python/pants_test:int-test',
],
tags={'integration'},
timeout=120,
)

python_tests(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,19 +24,20 @@ def test_go_thrift_gen_simple(self):
self.assert_success(pants_run)
with subsystem_instance(GoDistribution.Factory) as factory:
go_dist = factory.create()
goos = go_dist.create_go_cmd('env', args=['GOOS']).check_output().strip()
goarch = go_dist.create_go_cmd('env', args=['GOARCH']).check_output().strip()
expected_files = set([
'contrib.go.testprojects.src.thrift.thrifttest.fleem/a2a123c192ab/src/go/thrifttest/duck/constants.go',
'contrib.go.testprojects.src.thrift.thrifttest.fleem/a2a123c192ab/src/go/thrifttest/duck/ttypes.go',
])
go_dist.create_go_cmd('env', args=['GOOS']).check_output().strip()
go_dist.create_go_cmd('env', args=['GOARCH']).check_output().strip()
expected_files = {
'src/go/thrifttest/duck/constants.go',
'src/go/thrifttest/duck/ttypes.go',
}

# Fetch the hash for task impl version.
go_thrift_contents = os.listdir(os.path.join(workdir, 'gen', 'go-thrift'))
self.assertEqual(len(go_thrift_contents), 1)

root = os.path.join(workdir, 'gen', 'go-thrift', go_thrift_contents[0])
self.assertEquals(sorted(expected_files), sorted(exact_files(root, ignore_links=True)))
root = os.path.join(workdir, 'gen', 'go-thrift', go_thrift_contents[0],
'contrib.go.testprojects.src.thrift.thrifttest.fleem', 'current')
self.assertEquals(sorted(expected_files), sorted(exact_files(root)))

def test_go_thrift_gen_and_compile(self):
with self.temporary_workdir() as workdir:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import os
import shutil

from pants.option.custom_types import target_list_option
from pants.option.custom_types import list_option, target_option
from pants.subsystem.subsystem import Subsystem
from pants.util.dirutil import safe_mkdir

Expand All @@ -25,7 +25,8 @@ class ScalaJSPlatform(Subsystem, NodeResolverBase):
def register_options(cls, register):
super(ScalaJSPlatform, cls).register_options(register)
# TODO: revisit after https://rbcommons.com/s/twitter/r/3225/
register('--runtime', advanced=True, type=target_list_option, default=['//:scala-js-library'],
register('--runtime', advanced=True, type=list_option, member_type=target_option,
default=['//:scala-js-library'],
help='Target specs pointing to the scala-js runtime libraries.')

@classmethod
Expand Down
9 changes: 4 additions & 5 deletions src/python/pants/option/custom_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@
from __future__ import (absolute_import, division, generators, nested_scopes, print_function,
unicode_literals, with_statement)

import os

import six

from pants.option.errors import ParseError
Expand Down Expand Up @@ -128,8 +126,9 @@ def create(cls, value):
Note that we accept tuple literals, but the internal value is always a list.
:param The value to convert. Can be an instance of ListValueComponent, a list, a tuple,
a string representation (possibly prefixed by +) of a list or tuple, or a scalar.
:param value: The value to convert. Can be an instance of ListValueComponent, a list, a tuple,
a string representation (possibly prefixed by +) of a list or tuple, or any allowed
member_type.
:rtype: `ListValueComponent`
"""
if isinstance(value, cls): # Ensure idempotency.
Expand All @@ -146,7 +145,7 @@ def create(cls, value):
val = _convert(value[1:], (list, tuple))
elif isinstance(value, six.string_types):
action = cls.EXTEND
val = _convert('[r"{}"]'.format(value), list)
val = _convert('[r"""{}"""]'.format(value), list)
else:
action = cls.EXTEND
val = _convert('[{}]'.format(value), list)
Expand Down
6 changes: 3 additions & 3 deletions src/python/pants/option/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ def __init__(self, scope, option, **msg_format_args):
ImplicitValIsNone = mk_registration_error('Implicit value cannot be None.')
InvalidAction = mk_registration_error('Invalid action {action}.')
InvalidKwarg = mk_registration_error('Invalid registration kwarg {kwarg}.')
MemberTypeNotAllowed = mk_registration_error('member_type may only be specified if '
'type=list_option.')
NonScalarMemberType = mk_registration_error('member_type must be a scalar type.')
InvalidMemberType = mk_registration_error('member_type {member_type} not allowed.')
MemberTypeNotAllowed = mk_registration_error('member_type not allowed on option with type {type_}. '
'It may only be specified if type=list_option.')
NoOptionNames = mk_registration_error('No option names provided.')
OptionNameDash = mk_registration_error('Option name must begin with a dash.')
OptionNameDoubleDash = mk_registration_error('Long option name must begin with a double-dash.')
Expand Down
6 changes: 5 additions & 1 deletion src/python/pants/option/options.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import sys

from pants.option.arg_splitter import GLOBAL_SCOPE, ArgSplitter
from pants.option.custom_types import list_option
from pants.option.global_options import GlobalOptionsRegistrar
from pants.option.option_value_container import OptionValueContainer
from pants.option.parser_hierarchy import ParserHierarchy, enclosing_scope
Expand Down Expand Up @@ -302,7 +303,10 @@ def get_fingerprintable_for_scope(self, scope):
# scope, to get the right value for recursive options (and because this mirrors what
# option-using code does).
val = self.for_scope(scope)[kwargs['dest']]
val_type = kwargs.get('type', '')
val_type = kwargs.get('type', str)
# If we have a list then we delegate to the fingerprinting implementation of the members.
if val_type == list_option:
val_type = kwargs.get('member_type', str)
pairs.append((val_type, val))
registration_scope = (None if registration_scope == ''
else enclosing_scope(registration_scope))
Expand Down
29 changes: 17 additions & 12 deletions src/python/pants/option/options_fingerprinter.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import json
from hashlib import sha1

from pants.option.custom_types import file_option, target_list_option, target_option
from pants.option.custom_types import file_option, target_option


def stable_json_dumps(obj):
Expand Down Expand Up @@ -38,14 +38,17 @@ def fingerprint(self, option_type, option_val):
if option_val is None:
return None

if option_type == target_list_option:
# For simplicity, we always fingerprint a list. For non-list-valued options,
# this will be a singleton list.
if not isinstance(option_val, (list, tuple)):
option_val = [option_val]

if option_type == target_option:
return self._fingerprint_target_specs(option_val)
elif option_type == target_option:
return self._fingerprint_target_specs([option_val])
elif option_type == file_option:
return self._fingerprint_file(option_val)
return self._fingerprint_files(option_val)
else:
return self._fingerprint_primitive(option_val)
return self._fingerprint_primitives(option_val)

def _fingerprint_target_specs(self, specs):
"""Returns a fingerprint of the targets resolved from given target specs."""
Expand All @@ -58,13 +61,15 @@ def _fingerprint_target_specs(self, specs):
hasher.update(h)
return hasher.hexdigest()

def _fingerprint_file(self, filepath):
"""Returns a fingerprint of the given filepath and its contents."""
def _fingerprint_files(self, filepaths):
"""Returns a fingerprint of the given filepaths and their contents."""
hasher = sha1()
hasher.update(filepath)
with open(filepath, 'rb') as f:
hasher.update(f.read())
# Note that we don't sort the filepaths, as their order may have meaning.
for filepath in filepaths:
hasher.update(filepath)
with open(filepath, 'rb') as f:
hasher.update(f.read())
return hasher.hexdigest()

def _fingerprint_primitive(self, val):
def _fingerprint_primitives(self, val):
return stable_json_sha1(val)
33 changes: 23 additions & 10 deletions src/python/pants/option/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,14 @@
from pants.base.deprecated import check_deprecated_semver, deprecated_conditional
from pants.base.revision import Revision
from pants.option.arg_splitter import GLOBAL_SCOPE, GLOBAL_SCOPE_CONFIG_SECTION
from pants.option.custom_types import ListValueComponent, file_option, list_option
from pants.option.custom_types import (ListValueComponent, dict_option, file_option, list_option,
target_list_option, target_option)
from pants.option.errors import (BooleanOptionImplicitVal, BooleanOptionNameWithNo,
BooleanOptionType, DeprecatedOptionError, FrozenRegistration,
ImplicitValIsNone, InvalidAction, InvalidKwarg,
MemberTypeNotAllowed, NonScalarMemberType, NoOptionNames,
OptionNameDash, OptionNameDoubleDash, ParseError,
RecursiveSubsystemOption, Shadowing)
ImplicitValIsNone, InvalidAction, InvalidKwarg, InvalidMemberType,
MemberTypeNotAllowed, NoOptionNames, OptionNameDash,
OptionNameDoubleDash, ParseError, RecursiveSubsystemOption,
Shadowing)
from pants.option.option_util import is_boolean_option, is_list_option
from pants.option.ranked_value import RankedValue
from pants.option.scope import ScopeInfo
Expand Down Expand Up @@ -290,6 +291,18 @@ def register(self, *args, **kwargs):
kwargs['type'] = list_option
del kwargs['action']

# Temporary munging to effectively turn type='target_list_option' options into list options,
# with member type 'target_option', for uniform handling.
# TODO: Remove after target_list_option deprecation.
if kwargs.get('type') == target_list_option:
kwargs['type'] = list_option
kwargs['member_type'] = target_option
deprecated_conditional(lambda: True, '0.0.80',
'target_list_option is deprecated for option {} in scope {}. '
'Use type=list_option, member_type=target_option.'.format(
args[0], self.scope
))

# Record the args. We'll do the underlying parsing on-demand.
self._option_registrations.append((args, kwargs))
if self._parent_parser:
Expand Down Expand Up @@ -329,8 +342,8 @@ def _check_deprecated(self, dest, kwargs):
'store', 'store_true', 'store_false'
}

_scalar_types = {
str, int, float
_allowed_member_types = {
str, int, float, dict_option, file_option, target_option
}

def _validate(self, args, kwargs):
Expand Down Expand Up @@ -363,10 +376,10 @@ def error(exception_type, arg_name=None, **msg_kwargs):
error(ImplicitValIsNone)

if 'member_type' in kwargs and kwargs.get('type', str) != list_option:
error(MemberTypeNotAllowed)
error(MemberTypeNotAllowed, type_=kwargs.get('type', str))

if kwargs.get('member_type', str) not in self._scalar_types:
error(NonScalarMemberType)
if kwargs.get('member_type', str) not in self._allowed_member_types:
error(InvalidMemberType, member_type=kwargs.get('member_type', str))

for kwarg in kwargs:
if kwarg not in self._allowed_registration_kwargs:
Expand Down
1 change: 1 addition & 0 deletions tests/python/pants_test/base/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ python_tests(
'tests/python/pants_test:int-test',
],
tags = {'integration'},
timeout = 120,
)

python_tests(
Expand Down
Loading

0 comments on commit 58fd185

Please sign in to comment.