Skip to content
This repository has been archived by the owner on Dec 10, 2020. It is now read-only.

Commit

Permalink
Make most compile-related options advanced. #docfixit
Browse files Browse the repository at this point in the history
I may have been overzealous here. Please check if I've included
any flags that your end users typically use/need.

Testing Done:
CI passed here: https://travis-ci.org/pantsbuild/pants/builds/75500449.

Reviewed at https://rbcommons.com/s/twitter/r/2617/
  • Loading branch information
benjyw authored and Benjy committed Aug 13, 2015
1 parent db2263e commit b82a99b
Show file tree
Hide file tree
Showing 13 changed files with 89 additions and 69 deletions.
8 changes: 4 additions & 4 deletions contrib/cpp/src/python/pants/contrib/cpp/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def build_file_aliases():
)

def register_goals():
task(name='cpp-compile', action=CppCompile).install('compile')
task(name='cpp-library', action=CppLibraryCreate).install('binary')
task(name='cpp-binary', action=CppBinaryCreate).install('binary')
task(name='cpp-run', action=CppRun).install('run')
task(name='cpp', action=CppCompile).install('compile')
task(name='cpplib', action=CppLibraryCreate).install('binary')
task(name='cpp', action=CppBinaryCreate).install('binary')
task(name='cpp', action=CppRun).install('run')
17 changes: 8 additions & 9 deletions contrib/cpp/src/python/pants/contrib/cpp/tasks/cpp_compile.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,17 @@


class CppCompile(CppTask):
"""Compiles object files from C++ sources."""
"""Compile C++ sources into object files."""

@classmethod
def register_options(cls, register):
super(CppCompile, cls).register_options(register)
register('--cc-options',
register('--cc-options', advanced=True, action='append', default=[], fingerprint=True,
help='Append these options to the compiler command line.')
register('--cc-extensions',
default=['cc', 'cxx', 'cpp'],
help=('The list of extensions (without the .) to consider when '
'determining if a file is a C++ source file.'))
register('--cc-extensions', advanced=True, action='append', fingerprint=True,
default=['.cc', '.cxx', '.cpp'],
help=('The list of extensions to consider when determining if a file is a '
'C++ source file.'))

@classmethod
def product_types(cls):
Expand All @@ -39,7 +39,7 @@ def execute(self):

def is_cc(source):
_, ext = os.path.splitext(source)
return ext[1:] in self.get_options().cc_extensions
return ext in self.get_options().cc_extensions

targets = self.context.targets(self.is_cpp)

Expand Down Expand Up @@ -84,8 +84,7 @@ def _compile(self, target, results_dir, source):
cmd.extend(['-c'])
cmd.extend(('-I{0}'.format(i) for i in include_dirs))
cmd.extend(['-o' + obj, abs_source])
if self.get_options().cc_options != None:
cmd.extend([self.get_options().cc_options])
cmd.extend(self.get_options().cc_options)

# TODO: submit_async_work with self.run_command, [(cmd)] as a Work object.
with self.context.new_workunit(name='cpp-compile', labels=[WorkUnit.COMPILER]) as workunit:
Expand Down
2 changes: 1 addition & 1 deletion contrib/cpp/src/python/pants/contrib/cpp/tasks/cpp_task.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def is_binary(target):
@classmethod
def register_options(cls, register):
super(CppTask, cls).register_options(register)
register('--compiler',
register('--compiler', advanced=True, fingerprint=True,
help='Set a specific compiler to use (eg, g++-4.8, clang++)')

def execute(self):
Expand Down
8 changes: 8 additions & 0 deletions migrations/options/src/python/migrate_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,9 @@
('compile.zinc-java', 'enabled'): ('compile.java', 'use-jmake'),

('compile.scala', 'args'): ('compile.zinc', 'args'),

('compile.cpp-compile', 'cc_options'): ('compile.cpp', 'cc_options'),
('compile.cpp-compile', 'cc_extensions'): ('compile.cpp', 'cc_extensions'),
}

ng_daemons_note = ('The global "ng_daemons" option has been replaced by a "use_nailgun" option '
Expand Down Expand Up @@ -352,6 +355,11 @@
'to "disable jmake for java", more precisely, instead of '
'--compile-zinc-java-enabled, use --no-compile-java-use-jmake',
('compile.scala', 'args'): 'ALL `compile.scala` options have moved to `compile.zinc`.',

('compile.cpp-compile', 'cc_options'): 'Value used to be a string, is now a list.',
('compile.cpp-compile', 'cc_extensions'): 'Value used to be a string (but default was a list), '
'is now a list. Values also now include the dot, e.g.,'
'it\'s now .cpp, not cpp.',
}


Expand Down
12 changes: 6 additions & 6 deletions src/python/pants/backend/jvm/tasks/checkstyle.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@


class Checkstyle(NailgunTask):
"""Check Java code for style violations."""

_CHECKSTYLE_MAIN = 'com.puppycrawl.tools.checkstyle.Main'

Expand All @@ -30,14 +31,13 @@ def register_options(cls, register):
super(Checkstyle, cls).register_options(register)
register('--skip', action='store_true', fingerprint=True,
help='Skip checkstyle.')
register('--configuration', type=file_option, fingerprint=True,
register('--configuration', advanced=True, type=file_option, fingerprint=True,
help='Path to the checkstyle configuration file.')
register('--properties', type=dict_option, default={}, fingerprint=True,
register('--properties', advanced=True, type=dict_option, default={}, fingerprint=True,
help='Dictionary of property mappings to use for checkstyle.properties.')
register('--confs', default=['default'],
help='One or more ivy configurations to resolve for this target. This parameter is '
'not intended for general use. ')
register('--jvm-options', action='append', metavar='<option>...', advanced=True,
register('--confs', advanced=True, default=['default'],
help='One or more ivy configurations to resolve for this target.')
register('--jvm-options', advanced=True, action='append', metavar='<option>...',
help='Run checkstyle with these extra jvm options.')
cls.register_jvm_tool(register,
'checkstyle',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@


class AptCompile(JmakeCompile):
"""Compile Java annotation processors."""
# Well known metadata file to auto-register annotation processors with a java 1.6+ compiler
_PROCESSOR_INFO_FILE = 'META-INF/services/javax.annotation.processing.Processor'

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@


class JmakeCompile(JvmCompile):
"""Compile Java code with JMake."""
"""Compile Java code using JMake."""
_name = 'java'
_file_suffix = '.java'
_supports_concurrent_execution = False
Expand Down Expand Up @@ -72,7 +72,8 @@ def get_no_warning_args_default(cls):
@classmethod
def register_options(cls, register):
super(JmakeCompile, cls).register_options(register)
register('--use-jmake', action='store_true', default=True, help='Use jmake to compile Java targets')
register('--use-jmake', advanced=True, action='store_true', default=True,
help='Use jmake to compile Java targets')
register('--source', advanced=True, fingerprint=True,
help='Provide source compatibility with this release. Overrides the jvm platform '
'source.',
Expand Down
25 changes: 13 additions & 12 deletions src/python/pants/backend/jvm/tasks/jvm_compile/jvm_compile.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,44 +32,45 @@ class JvmCompile(NailgunTaskBase, GroupMember):
@classmethod
def register_options(cls, register):
super(JvmCompile, cls).register_options(register)
register('--partition-size-hint', type=int, default=sys.maxint, metavar='<# source files>',
register('--partition-size-hint', advanced=True, type=int, default=sys.maxint,
metavar='<# source files>',
help='Roughly how many source files to attempt to compile together. Set to a large '
'number to compile all sources together. Set to 0 to compile target-by-target.')

register('--jvm-options', type=list_option, default=[],
register('--jvm-options', advanced=True, type=list_option, default=[],
help='Run the compiler with these JVM options.')

register('--args', action='append',
register('--args', advanced=True, action='append',
default=list(cls.get_args_default(register.bootstrap)), fingerprint=True,
help='Pass these args to the compiler.')

register('--confs', type=list_option, default=['default'],
register('--confs', advanced=True, type=list_option, default=['default'],
help='Compile for these Ivy confs.')

# TODO: Stale analysis should be automatically ignored via Task identities:
# https://github.com/pantsbuild/pants/issues/1351
register('--clear-invalid-analysis', default=False, action='store_true',
advanced=True,
register('--clear-invalid-analysis', advanced=True, default=False, action='store_true',
help='When set, any invalid/incompatible analysis files will be deleted '
'automatically. When unset, an error is raised instead.')

register('--warnings', default=True, action='store_true',
help='Compile with all configured warnings enabled.')

register('--warning-args', action='append', default=list(cls.get_warning_args_default()),
advanced=True,
register('--warning-args', advanced=True, action='append',
default=list(cls.get_warning_args_default()),
help='Extra compiler args to use when warnings are enabled.')

register('--no-warning-args', action='append', default=list(cls.get_no_warning_args_default()),
advanced=True,
register('--no-warning-args', advanced=True, action='append',
default=list(cls.get_no_warning_args_default()),
help='Extra compiler args to use when warnings are disabled.')

register('--strategy', choices=['global', 'isolated'], default='global', fingerprint=True,
register('--strategy', advanced=True, choices=['global', 'isolated'], default='global',
fingerprint=True,
help='Selects the compilation strategy to use. The "global" strategy uses a shared '
'global classpath for all compiled classes, and the "isolated" strategy uses '
'per-target classpaths.')

register('--delete-scratch', default=True, action='store_true',
register('--delete-scratch', advanced=True, default=True, action='store_true',
help='Leave intermediate scratch files around, for debugging build problems.')

JvmCompileGlobalStrategy.register_options(register, cls._name, cls._supports_concurrent_execution)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,33 +35,37 @@ class InternalTargetPartitioningError(Exception):

@classmethod
def register_options(cls, register, compile_task_name, supports_concurrent_execution):
register('--missing-deps', choices=['off', 'warn', 'fatal'], default='warn',
help='Check for missing dependencies in code compiled with {0}. Reports actual dependencies A -> B '
'where there is no transitive BUILD file dependency path from A to B. If fatal, '
'missing deps are treated as a build error.'.format(compile_task_name))

register('--missing-direct-deps', choices=['off', 'warn', 'fatal'], default='off',
help='Check for missing direct dependencies in code compiled with {0}. Reports actual dependencies '
'A -> B where there is no direct BUILD file dependency path from A to B. This is '
'a very strict check; In practice it is common to rely on transitive, indirect '
'dependencies, e.g., due to type inference or when the main target in a BUILD '
'file is modified to depend on other targets in the same BUILD file, as an '
'implementation detail. However it may still be useful to use this on '
'occasion. '.format(compile_task_name))

register('--missing-deps-whitelist', type=list_option,
register('--missing-deps', advanced=True, choices=['off', 'warn', 'fatal'], default='warn',
help='Check for missing dependencies in code compiled with {0}. Reports actual '
'dependencies A -> B where there is no transitive BUILD file dependency path '
'from A to B. If fatal, missing deps are treated as a build error.'.format(
compile_task_name))

register('--missing-direct-deps', advanced=True, choices=['off', 'warn', 'fatal'],
default='off',
help='Check for missing direct dependencies in code compiled with {0}. Reports actual '
'dependencies A -> B where there is no direct BUILD file dependency path from '
'A to B. This is a very strict check; In practice it is common to rely on '
'transitive, indirect dependencies, e.g., due to type inference or when the main '
'target in a BUILD file is modified to depend on other targets in the same BUILD '
'file, as an implementation detail. However it may still be useful to use this '
'on occasion. '.format(compile_task_name))

register('--missing-deps-whitelist', advanced=True, type=list_option,
help="Don't report these targets even if they have missing deps.")

register('--unnecessary-deps', choices=['off', 'warn', 'fatal'], default='off',
help='Check for declared dependencies in code compiled with {0} that are not needed. This is a very '
'strict check. For example, generated code will often legitimately have BUILD '
'dependencies that are unused in practice.'.format(compile_task_name))
register('--unnecessary-deps', advanced=True, choices=['off', 'warn', 'fatal'], default='off',
help='Check for declared dependencies in code compiled with {0} that are not needed. '
'This is a very strict check. For example, generated code will often '
'legitimately have BUILD dependencies that are unused in practice.'.format(
compile_task_name))

register('--changed-targets-heuristic-limit', type=int, default=0,
register('--changed-targets-heuristic-limit', advanced=True, type=int, default=0,
help='If non-zero, and we have fewer than this number of locally-changed targets, '
'partition them separately, to preserve stability when compiling repeatedly.')

def __init__(self, context, options, workdir, analysis_tools, compile_task_name, sources_predicate):
def __init__(self, context, options, workdir, analysis_tools, compile_task_name,
sources_predicate):
super(JvmCompileGlobalStrategy, self).__init__(context, options, workdir, analysis_tools,
compile_task_name, sources_predicate)

Expand Down Expand Up @@ -144,7 +148,8 @@ def pre_compile(self):
self.validate_analysis(f)

def prepare_compile(self, cache_manager, all_targets, relevant_targets):
super(JvmCompileGlobalStrategy, self).prepare_compile(cache_manager, all_targets, relevant_targets)
super(JvmCompileGlobalStrategy, self).prepare_compile(cache_manager, all_targets,
relevant_targets)

# Update the classpath for us and for downstream tasks.
compile_classpaths = self.context.products.get_data('compile_classpath')
Expand Down Expand Up @@ -623,7 +628,7 @@ def _record_previous_sources_by_target(self, target, sources):
with open(os.path.join(self._target_sources_dir, target.identifier), 'w') as outfile:
for src in sources:
outfile.write(os.path.join(get_buildroot(), src))
outfile.write('\n')
outfile.write(b'\n')

def _compute_deleted_sources(self):
"""Computes the list of sources present in the last analysis that have since been deleted.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,14 @@ class JvmCompileIsolatedStrategy(JvmCompileStrategy):
@classmethod
def register_options(cls, register, compile_task_name, supports_concurrent_execution):
if supports_concurrent_execution:
register('--worker-count', type=int, default=1, advanced=True,
help='The number of concurrent workers to use compiling with {task} with the isolated'
' strategy.'.format(task=compile_task_name))
register('--capture-log', action='store_true', default=False, advanced=True,
register('--worker-count', advanced=True, type=int, default=1,
help='The number of concurrent workers to use compiling with {task} with the '
'isolated strategy.'.format(task=compile_task_name))
register('--capture-log', advanced=True, action='store_true', default=False,
help='Capture compilation output to per-target logs.')

def __init__(self, context, options, workdir, analysis_tools, compile_task_name, sources_predicate):
def __init__(self, context, options, workdir, analysis_tools, compile_task_name,
sources_predicate):
super(JvmCompileIsolatedStrategy, self).__init__(context, options, workdir, analysis_tools,
compile_task_name, sources_predicate)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,8 @@ def register_options(cls, register, compile_task_name, supports_concurrent_execu
The abstract base class does not register any options itself: those are left to JvmCompile.
"""

def __init__(self, context, options, workdir, analysis_tools, compile_task_name, sources_predicate):
def __init__(self, context, options, workdir, analysis_tools, compile_task_name,
sources_predicate):
self._compile_task_name = compile_task_name
self.context = context
self._analysis_tools = analysis_tools
Expand Down Expand Up @@ -114,8 +115,7 @@ def compile_chunk(self,
extra_compile_time_classpath_elements,
compile_vts,
register_vts,
update_artifact_cache_vts_work,
settings):
update_artifact_cache_vts_work):
"""Executes compilations for that invalid targets contained in a single language chunk."""

@abstractmethod
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@


class ZincCompile(JvmCompile):
"""Compile Scala and Java code using Zinc."""

_ZINC_MAIN = 'org.pantsbuild.zinc.Main'

_name = 'zinc'
Expand Down Expand Up @@ -69,11 +71,11 @@ def get_no_warning_args_default(cls):
@classmethod
def register_options(cls, register):
super(ZincCompile, cls).register_options(register)
register('--plugins', action='append', fingerprint=True,
register('--plugins', advanced=True, action='append', fingerprint=True,
help='Use these scalac plugins.')
register('--plugin-args', advanced=True, type=dict_option, default={}, fingerprint=True,
help='Map from plugin name to list of arguments for that plugin.')
register('--name-hashing', action='store_true', default=False, fingerprint=True,
register('--name-hashing', advanced=True, action='store_true', default=False, fingerprint=True,
help='Use zinc name hashing.')

cls.register_jvm_tool(register,
Expand Down
6 changes: 4 additions & 2 deletions src/python/pants/backend/jvm/tasks/nailgun_task.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,10 @@ def register_options(cls, register):
cls.register_jvm_tool(register, 'nailgun-server')
register('--use-nailgun', action='store_true', default=True,
help='Use nailgun to make repeated invocations of this task quicker.')
register('--nailgun-timeout-seconds', default=10, help='Timeout (secs) for nailgun startup.')
register('--nailgun-connect-attempts', default=5, help='Max attempts for nailgun connects.')
register('--nailgun-timeout-seconds', advanced=True, default=10,
help='Timeout (secs) for nailgun startup.')
register('--nailgun-connect-attempts', advanced=True, default=5,
help='Max attempts for nailgun connects.')

def __init__(self, *args, **kwargs):
super(NailgunTaskBase, self).__init__(*args, **kwargs)
Expand Down

0 comments on commit b82a99b

Please sign in to comment.