Skip to content

Commit

Permalink
Adds custom list and dict option types.
Browse files Browse the repository at this point in the history
Values (in cmd-line args, env vars and config files) must be JSON lists/objects.

Currently our config reader 'parses' list and dict values using python 'eval', so we
change all of those in our pants.ini to be valid JSON. Fortunately they remain valid
eval-able python too. Eventually we'll get rid of that eval-ing code, but for now
we need to support both, during the transition.

Adds checks in migrate_config.py for list/dict values that aren't valid JSON.

Note that the list type is different from action='append'. The latter
appends cmd-line arg values to the default. The former replaces the
default.

Testing Done:
CI passes.

Reviewed at https://rbcommons.com/s/twitter/r/1389/
  • Loading branch information
benjyw authored and Benjy committed Nov 24, 2014
1 parent d59ea8e commit 8c3b5b3
Show file tree
Hide file tree
Showing 10 changed files with 253 additions and 90 deletions.
148 changes: 73 additions & 75 deletions pants.ini
Original file line number Diff line number Diff line change
Expand Up @@ -6,62 +6,60 @@
# pants_supportdir: pants support files for this repo go here; for example: ivysettings.xml
# pants_distdir: user visible artifacts for this repo go here
# pants_workdir: the scratch space used to for live builds in this repo
#
# NOTE: Values of list and dict type must be valid JSON.

[DEFAULT]
pants_support_baseurls = [
'https://dl.bintray.com/pantsbuild/bin/build-support',
]

checkstyle_suppression_files = [
'%(pants_supportdir)s/commons/checkstyle/checkstyle_suppressions.xml'
"https://dl.bintray.com/pantsbuild/bin/build-support"
]

outdir: %(pants_distdir)s

jvm_options: ['-Xmx1g', '-XX:MaxPermSize=256m']
jvm_options: ["-Xmx1g", "-XX:MaxPermSize=256m"]


[goals]
# This will pick up the whole top level BUILD file family, including BUILD.commons
bootstrap_buildfiles: [
# This will pick up the whole top level BUILD file family, including BUILD.commons
'%(buildroot)s/BUILD',
"%(buildroot)s/BUILD"
]

[ivy]
ivy_settings: %(pants_supportdir)s/ivy/ivysettings.xml


[scrooge-gen]
bootstrap-tools: ['//:scrooge-gen']
bootstrap-tools: ["//:scrooge-gen"]
supportdir: %(pants_supportdir)s/bin/scrooge
strict: False
verbose: True
java: {
'gen': 'java',
'deps': {
'service': [
'3rdparty:commons-lang',
'3rdparty:finagle-thrift',
'3rdparty:slf4j-api',
'3rdparty:util-core',
"gen": "java",
"deps": {
"service": [
"3rdparty:commons-lang",
"3rdparty:finagle-thrift",
"3rdparty:slf4j-api",
"3rdparty:util-core"
],
'structs': [
'3rdparty:commons-lang',
'3rdparty:thrift',
"structs": [
"3rdparty:commons-lang",
"3rdparty:thrift"
]
}
}
scala: {
'gen': 'scala',
'deps': {
'service': [
'3rdparty:scrooge-core',
'3rdparty:finagle-thrift',
'3rdparty:util-core',
"gen": "scala",
"deps": {
"service": [
"3rdparty:scrooge-core",
"3rdparty:finagle-thrift",
"3rdparty:util-core"
],
'structs': [
'3rdparty:scrooge-core',
'3rdparty:thrift'
"structs": [
"3rdparty:scrooge-core",
"3rdparty:thrift"
]
}
}
Expand All @@ -73,28 +71,28 @@ strict: False
verbose: False
version: 0.5.0-finagle
java: {
'gen': 'java:hashcode',
'deps': {
'service': ['3rdparty:thrift-0.5.0-finagle'],
'structs': ['3rdparty:thrift-0.5.0']
"gen": "java:hashcode",
"deps": {
"service": ["3rdparty:thrift-0.5.0-finagle"],
"structs": ["3rdparty:thrift-0.5.0"]
}
}
python: {
'gen': 'py:newstyle',
'deps': {
'service': ['3rdparty/python:thrift'],
'structs': ['3rdparty/python:thrift']
"gen": "py:newstyle",
"deps": {
"service": ["3rdparty/python:thrift"],
"structs": ["3rdparty/python:thrift"]
}
}


[antlr-gen]
version: 3.4
javadeps: ['//:antlr-%(version)s']
javadeps: ["//:antlr-%(version)s"]

[antlr4-gen]
version: 4.0
javadeps: ['//:antlr-4']
javadeps: ["//:antlr-4"]


[ragel-gen]
Expand All @@ -103,11 +101,9 @@ version: 6.8


[checkstyle]
bootstrap-tools: ['//:twitter-checkstyle']
bootstrap-tools: ["//:twitter-checkstyle"]
configuration: %(pants_supportdir)s/checkstyle/coding_style.xml
properties: {
'checkstyle.suppression.files': ','.join(%(checkstyle_suppression_files)s)
}
suppression_files = [ "%(pants_supportdir)s/commons/checkstyle/checkstyle_suppressions.xml" ]


[scalastyle]
Expand All @@ -116,25 +112,25 @@ excludes: %(buildroot)s/build-support/scalastyle/excludes.txt


[java-compile]
jmake-bootstrap-tools: ['//:jmake']
jmake-bootstrap-tools: ["//:jmake"]
partition_size_hint: 1000000000

jvm_args: ['-Xmx2G']
jvm_args: ["-Xmx2G"]

source: '6'
target: '6'
source: "6"
target: "6"

compiler-bootstrap-tools: ['//:java-compiler']
compiler-bootstrap-tools: ["//:java-compiler"]


[scala-compile]
compile-bootstrap-tools: ['//:scala-compiler-2.9.3']
zinc-bootstrap-tools: ['//:zinc']
compile-bootstrap-tools: ["//:scala-compiler-2.9.3"]
zinc-bootstrap-tools: ["//:zinc"]

runtime-deps: ['//:scala-library-2.9.3']
runtime-deps: ["//:scala-library-2.9.3"]


jvm_args: ['-Xmx2g', '-XX:MaxPermSize=256m', '-Dzinc.analysis.cache.limit=0']
jvm_args: ["-Xmx2g", "-XX:MaxPermSize=256m", "-Dzinc.analysis.cache.limit=0"]


[jvm]
Expand All @@ -144,48 +140,49 @@ parallel_test_paths: True


[run.jvm]
jvm_options: ['-Xmx1g', '-XX:MaxPermSize=256m']
jvm_options: ["-Xmx1g", "-XX:MaxPermSize=256m"]


[repl.scala]
jvm_options: ['-Xmx1g', '-XX:MaxPermSize=256m', '-Dscala.usejavacp=true' ]
jvm_options: ["-Xmx1g", "-XX:MaxPermSize=256m", "-Dscala.usejavacp=true" ]

[scala-repl]
bootstrap-tools: ['//:scala-repl-2.9.3']
bootstrap-tools: ["//:scala-repl-2.9.3"]
main: scala.tools.nsc.MainGenericRunner

[test.junit]
# -XX:-UseSplitVerifier is needed for emma instrumenter to work against classfiles generated by
# java 7 compilers
jvm_options: [
'-Djava.awt.headless=true', '-Xmx1g', '-XX:MaxPermSize=256m',
# Needed for emma instrumenter to work against classfiles generated by java 7 compilers
'-XX:-UseSplitVerifier'
"-Djava.awt.headless=true", "-Xmx1g", "-XX:MaxPermSize=256m",
"-XX:-UseSplitVerifier"
]

[junit-run]
junit-bootstrap-tools: ['//:junit']
emma-bootstrap-tools: ['//:emma']
cobertura-bootstrap-tools: ['//:cobertura']
junit-bootstrap-tools: ["//:junit"]
emma-bootstrap-tools: ["//:emma"]
cobertura-bootstrap-tools: ["//:cobertura"]


[run.specs]
jvm_options: ['-Xmx1g', '-XX:MaxPermSize=256m']
jvm_options: ["-Xmx1g", "-XX:MaxPermSize=256m"]

[specs-run]
bootstrap-tools: ['//:scala-specs-2.9.3']
bootstrap-tools: ["//:scala-specs-2.9.3"]


[bench]
jvm_options: ['-Xmx1g', '-XX:MaxPermSize=256m']
jvm_options: ["-Xmx1g", "-XX:MaxPermSize=256m"]

[benchmark-run]
bootstrap-tools: ['//:benchmark-caliper-0.5']
agent-bootstrap-tools: ['//:benchmark-java-allocation-instrumenter-2.1']
bootstrap-tools: ["//:benchmark-caliper-0.5"]
agent-bootstrap-tools: ["//:benchmark-java-allocation-instrumenter-2.1"]


[ide]
python_source_paths: ['src/python']
python_test_paths: ['tests/python']
python_lib_paths: ['3rdparty/python']
python_source_paths: ["src/python"]
python_test_paths: ["tests/python"]
python_lib_paths: ["3rdparty/python"]


[idea]
Expand All @@ -202,20 +199,21 @@ interpreter_requirement: CPython>=2.7,<3

[python-repos]
repos: [
'https://pantsbuild.github.io/cheeseshop/third_party/python/dist/index.html',
'https://pantsbuild.github.io/cheeseshop/third_party/python/index.html']
"https://pantsbuild.github.io/cheeseshop/third_party/python/dist/index.html",
"https://pantsbuild.github.io/cheeseshop/third_party/python/index.html"
]

indices: ['https://pypi.python.org/simple/']
indices: ["https://pypi.python.org/simple/"]


[python-ipython]
entry_point: IPython:start_ipython
requirements: ['ipython==1.0.0']
requirements: ["ipython==1.0.0"]


[backends]
packages: [
'internal_backend.optional',
'internal_backend.repositories',
'internal_backend.sitegen',
"internal_backend.optional",
"internal_backend.repositories",
"internal_backend.sitegen"
]
2 changes: 2 additions & 0 deletions src/python/pants/backend/jvm/tasks/checkstyle.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,9 @@ def __init__(self, *args, **kwargs):

self._configuration_file = self.context.config.get(self._CONFIG_SECTION, 'configuration')

suppression_files = self.context.config.getlist(self._CONFIG_SECTION, 'suppression_files', [])
self._properties = self.context.config.getdict(self._CONFIG_SECTION, 'properties', {})
self._properties['checkstyle.suppression.files'] = ','.join(suppression_files)
self._confs = self.context.config.getlist(self._CONFIG_SECTION, 'confs', default=['default'])

@property
Expand Down
5 changes: 2 additions & 3 deletions src/python/pants/backend/jvm/tasks/ide_gen.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,8 @@ def __init__(self, *args, **kwargs):

self.intransitive = self.get_options().intransitive

self.checkstyle_suppression_files = self.context.config.getdefault(
'checkstyle_suppression_files', type=list, default=[]
)
self.checkstyle_suppression_files = self.context.config.get('checkstyle',
'suppression_files', type=list, default=[])
# Everywhere else, debug_port is specified in the 'jvm' section. Use that as a default if none
# is specified in the 'ide' section.
jvm_config_debug_port = JvmDebugConfig.debug_port(self.context.config)
Expand Down
51 changes: 51 additions & 0 deletions src/python/pants/option/custom_types.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
# coding=utf-8
# Copyright 2014 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from __future__ import (nested_scopes, generators, division, absolute_import, with_statement,
print_function, unicode_literals)

import json

from pants.option.errors import ParseError


def _parse_error(s, msg):
"""Return a ParseError with a usefully formatted message, for the caller to throw.
:param s: The option value we're parsing.
:param msg: An extra message to add to the ParseError.
"""
return ParseError('Error while parsing option value {0}: {1}'.format(s, msg))


def dict_type(s):
"""An option of type 'dict'.
The value (on the command-line, in an env var or in the config file) must be a JSON object.
"""
if isinstance(s, dict):
return s
try:
ret = json.loads(s)
except ValueError as e:
raise _parse_error(s, e.message)
if not isinstance(ret, dict):
raise _parse_error(s, 'Value is not dict')
return ret


def list_type(s):
"""An option of type 'list'.
The value (on the command-line, in an env var or in the config file) must be a JSON list.
"""
if isinstance(s, (list, tuple)):
return s
try:
ret = json.loads(s)
except ValueError as e:
raise _parse_error(s, e.message)
if not isinstance(ret, list):
raise _parse_error(s, 'Value is not list')
return ret
16 changes: 16 additions & 0 deletions src/python/pants/option/errors.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# coding=utf-8
# Copyright 2014 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from __future__ import (nested_scopes, generators, division, absolute_import, with_statement,
print_function, unicode_literals)


class RegistrationError(Exception):
"""An error at option registration time."""
pass


class ParseError(Exception):
"""An error at flag parsing time."""
pass
Loading

0 comments on commit 8c3b5b3

Please sign in to comment.