Support passthru args in tasks.

Tasks must declare that they take them.

The last task explicitly mentioned on the cmd-line gets them.

Implemented this for pytest.

Testing Done:
Verified that this does what you expect:

./pants goal test.pytest compile tests/python/pants_test/option/ -- -s

CI passes.

Reviewed at https://rbcommons.com/s/twitter/r/1320/

src/python/pants/backend/core/tasks/task.py

@@ -20,7 +20,6 @@
 from pants.base.config import Config
 from pants.base.exceptions import TaskError
 from pants.base.worker_pool import Work
-from pants.base.workunit import WorkUnit
 from pants.cache.artifact_cache import call_insert, call_use_cached_files
 from pants.cache.cache_setup import create_artifact_cache
 from pants.cache.read_write_artifact_cache import ReadWriteArtifactCache
@@ -74,6 +73,11 @@ def register_options(cls, register):
     to register options.
     """
 
+  @classmethod
+  def supports_passthru_args(cls):
+    """Subclasses may override to indicate that they can use passthru args."""
+    return False
+
   def __init__(self, context, workdir):
     """Subclass __init__ methods, if defined, *must* follow this idiom:
 
@@ -104,6 +108,12 @@ def get_options(self):
     """Returns the option values for this task's scope."""
     return self.context.new_options.for_scope(self.options_scope)
 
+  def get_passthru_args(self):
+    if not self.supports_passthru_args():
+      raise TaskError('{0} Does not support passthru args.'.format(self.__class__.__name__))
+    else:
+      return self.context.new_options.passthru_args_for_scope(self.options_scope)
+
   def prepare(self, round_manager):
     """Prepares a task for execution.
 

src/python/pants/backend/python/tasks/pytest_run.py

@@ -27,6 +27,10 @@ def register_options(cls, register):
     register('--options', action='append', legacy='pytest_run_options',
              help='Pass these options to pytest.')
 
+  @classmethod
+  def supports_passthru_args(cls):
+    return True
+
   def execute(self):
     def is_python_test(target):
       # Note that we ignore PythonTestSuite, because we'll see the PythonTests targets
@@ -42,9 +46,8 @@ def is_python_test(target):
       debug = self.get_options().level == 'debug'
 
       args = [] if self.get_options().no_colors else ['--color', 'yes']
-      if self.get_options().options:
-        for options in self.get_options().options:
-          args.extend(safe_shlex_split(options))
+      for options in self.get_options().options + self.get_passthru_args():
+        args.extend(safe_shlex_split(options))
       test_builder = PythonTestBuilder(targets=test_targets,
                                        args=args,
                                        interpreter=self.interpreter,

src/python/pants/option/arg_splitter.py

@@ -5,9 +5,11 @@
 from __future__ import (nested_scopes, generators, division, absolute_import, with_statement,
                         print_function, unicode_literals)
 
-from collections import namedtuple, OrderedDict
+from collections import namedtuple
 import sys
 
+from twitter.common.collections import OrderedSet
+
 
 GLOBAL_SCOPE = ''
 
@@ -16,14 +18,17 @@ class ArgSplitterError(Exception):
   pass
 
 
-class SplitArgs(namedtuple('SplitArgs', ['scope_to_flags', 'targets', 'passthru'])):
+class SplitArgs(namedtuple('SplitArgs',
+                           ['goals', 'scope_to_flags', 'targets', 'passthru', 'passthru_owner'])):
   """The result of splitting args.
 
+  goals: A list of explicitly specified goals.
   scope_to_flags: An ordered map from scope name to the list of flags belonging to that scope.
                   The global scope is specified as an empty string.
                   Keys are in the order encountered in the args.
   targets: A list of target specs.
   passthru: Any remaining args specified after a -- separator.
+  passthru_owner: The scope specified last on the command line, if any. None otherwise.
   """
   pass
 
@@ -64,7 +69,8 @@ def split_args(self, args=None):
 
     Returns a SplitArgs tuple.
     """
-    scope_to_flags = OrderedDict()
+    goals = OrderedSet()
+    scope_to_flags = {}
 
     def add_scope(s):
       # Force the scope to appear, even if empty.
@@ -73,6 +79,7 @@ def add_scope(s):
 
     targets = []
     passthru = []
+    passthru_owner = None
 
     self._unconsumed_args = list(reversed(sys.argv if args is None else args))
     # In regular use the first token is the binary name, so skip it. However tests may
@@ -99,6 +106,8 @@ def assign_flag_to_scope(flag, default_scope):
     scope, flags = self._consume_scope()
     while scope:
       add_scope(scope)
+      goals.add(scope.partition('.')[0])
+      passthru_owner = scope
       for flag in flags:
         assign_flag_to_scope(flag, scope)
       scope, flags = self._consume_scope()
@@ -118,21 +127,19 @@ def assign_flag_to_scope(flag, default_scope):
 
     # We parse the word 'help' as a scope, but it's not a real one, so ignore it.
     scope_to_flags.pop('help', None)
-    return SplitArgs(scope_to_flags, targets, passthru)
+    return SplitArgs(goals, scope_to_flags, targets, passthru, passthru_owner if passthru else None)
 
   def _consume_scope(self):
     """Returns a pair (scope, list of flags encountered in that scope).
 
-    Each entry in the list is a pair (scope, flag) in case the flag was explicitly
-    scoped in the old style, and therefore may not actually belong to this scope.
+    Note that the flag may be explicitly scoped, and therefore not actually belong to this scope.
 
     For example, in:
 
     ./pants --compile-java-partition-size-hint=100 compile <target>
 
     --compile-java-partition-size-hint should be treated as if it were --partition-size-hint=100
-    in the compile.java scope.  This will make migration from old- to new-style flags much easier.
-    In fact, we may want to allow this even post-migration, for users that prefer it.
+    in the compile.java scope.
     """
     if not self._at_scope():
       return None, []

src/python/pants/option/options.py

@@ -8,8 +8,6 @@
 import copy
 import sys
 
-from twitter.common.collections import OrderedSet
-
 from pants.base.build_environment import pants_release
 from pants.goal.goal import Goal
 from pants.option.arg_splitter import ArgSplitter, GLOBAL_SCOPE
@@ -70,7 +68,8 @@ def __init__(self, env, config, known_scopes, args=sys.argv, legacy_parser=None)
            the old-style flags during migration.
     """
     splitter = ArgSplitter(known_scopes)
-    self._scope_to_flags, self._target_specs, self._passthru = splitter.split_args(args)
+    self._goals, self._scope_to_flags, self._target_specs, self._passthru, self._passthru_owner = \
+      splitter.split_args(args)
     self._is_help = splitter.is_help
     self._parser_hierarchy = ParserHierarchy(env, config, known_scopes, legacy_parser)
     self._legacy_parser = legacy_parser  # Old-style options, used temporarily during transition.
@@ -84,8 +83,15 @@ def target_specs(self):
 
   @property
   def goals(self):
-    """The requested goals, in the order specified on the cmd-line."""
-    return OrderedSet([g.partition('.')[0] for g in self._scope_to_flags.keys() if g])
+    """The requested goals, in the order specified on the cmd line."""
+    return self._goals
+
+  def passthru_args_for_scope(self, scope):
+    # Passthru args "belong" to the last scope mentioned on the command-line.
+    if scope == self._passthru_owner:
+      return self._passthru
+    else:
+      return []
 
   @property
   def is_help(self):

tests/python/pants_test/option/test_arg_splitter.py

@@ -5,7 +5,6 @@
 from __future__ import (nested_scopes, generators, division, absolute_import, with_statement,
                         print_function, unicode_literals)
 
-from collections import OrderedDict
 import shlex
 import unittest2 as unittest
 
@@ -15,77 +14,100 @@
 class ArgSplitterTest(unittest.TestCase):
   _known_scopes = ['compile', 'compile.java', 'compile.scala', 'test', 'test.junit']
 
-  def _split(self, args_str, expected_scope_to_flags, expected_target_specs,
-             expected_passthru=None, expected_help=False):
+  def _split(self, args_str, expected_goals, expected_scope_to_flags, expected_target_specs,
+             expected_passthru=None, expected_passthru_owner=None, expected_is_help=False):
     expected_passthru = expected_passthru or []
     splitter = ArgSplitter(ArgSplitterTest._known_scopes)
     args = shlex.split(str(args_str))
-    scope_to_flags, target_specs, passthru = splitter.split_args(args)
+    goals, scope_to_flags, target_specs, passthru, passthru_owner = splitter.split_args(args)
+    self.assertEquals(expected_goals, goals)
     self.assertEquals(expected_scope_to_flags, scope_to_flags)
     self.assertEquals(expected_target_specs, target_specs)
     self.assertEquals(expected_passthru, passthru)
-    self.assertEquals(expected_help, splitter.is_help)
+    self.assertEquals(expected_passthru_owner, passthru_owner)
+    self.assertEquals(expected_is_help, splitter.is_help)
 
   def test_arg_splitting(self):
     # Various flag combos.
-    self._split('./pants', {'': []}, [])
-    self._split('./pants goal', {'': []}, [])
-    self._split('./pants -f', {'': ['-f']}, [])
-    self._split('./pants goal -f', {'': ['-f']}, [])
+    self._split('./pants', [], {'': []}, [])
+    self._split('./pants goal', [], {'': []}, [])
+    self._split('./pants -f', [], {'': ['-f']}, [])
+    self._split('./pants goal -f', [], {'': ['-f']}, [])
     self._split('./pants --compile-java-long-flag -f compile -g compile.java -x test.junit -i '
                 'src/java/com/pants/foo src/java/com/pants/bar:baz',
-                OrderedDict([
-                  ('', ['-f']),
-                  ('compile.java', ['--long-flag', '-x']),
-                  ('compile', ['-g']),
-                  ('test.junit', ['-i'])
-                ]),
+                ['compile', 'test'],
+                {
+                  '': ['-f'],
+                  'compile.java': ['--long-flag', '-x'],
+                  'compile': ['-g'],
+                  'test.junit': ['-i']
+                },
                 ['src/java/com/pants/foo', 'src/java/com/pants/bar:baz'])
     self._split('./pants -farg --fff=arg compile --gg-gg=arg-arg -g test.junit --iii '
                 '--compile-java-long-flag src/java/com/pants/foo src/java/com/pants/bar:baz',
-                OrderedDict([
-                  ('', ['-farg', '--fff=arg']),
-                  ('compile', ['--gg-gg=arg-arg', '-g']),
-                  ('test.junit', ['--iii']),
-                  ('compile.java', ['--long-flag']),
-                ]),
+                ['compile', 'test'],
+                {
+                  '': ['-farg', '--fff=arg'],
+                  'compile': ['--gg-gg=arg-arg', '-g'],
+                  'test.junit': ['--iii'],
+                  'compile.java': ['--long-flag'],
+                },
                 ['src/java/com/pants/foo', 'src/java/com/pants/bar:baz'])
 
     # Distinguishing goals and target specs.
-    self._split('./pants compile test foo::', {'': [], 'compile': [], 'test': []}, ['foo::'])
-    self._split('./pants compile test foo::', {'': [], 'compile': [], 'test': []}, ['foo::'])
-    self._split('./pants compile test:test', {'': [], 'compile': []}, ['test:test'])
-    self._split('./pants test test:test', {'': [], 'test': []}, ['test:test'])
-    self._split('./pants test ./test', {'': [], 'test': []}, ['./test'])
-    self._split('./pants test //test', {'': [], 'test': []}, ['//test'])
+    self._split('./pants compile test foo::', ['compile', 'test'],
+                {'': [], 'compile': [], 'test': []}, ['foo::'])
+    self._split('./pants compile test foo::', ['compile', 'test'],
+                {'': [], 'compile': [], 'test': []}, ['foo::'])
+    self._split('./pants compile test:test', ['compile'], {'': [], 'compile': []}, ['test:test'])
+    self._split('./pants test test:test', ['test'], {'': [], 'test': []}, ['test:test'])
+    self._split('./pants test ./test', ['test'], {'': [], 'test': []}, ['./test'])
+    self._split('./pants test //test', ['test'], {'': [], 'test': []}, ['//test'])
 
     # De-scoping old-style flags correctly.
     self._split('./pants compile test --compile-java-bar --no-test-junit-baz foo',
-                {'': [], 'compile': [], 'compile.java': ['--bar'],
-                 'test': [], 'test.junit': ['--no-baz']}, ['foo'])
+                ['compile', 'test'],
+                {'': [], 'compile': [], 'compile.java': ['--bar'], 'test': [],
+                 'test.junit': ['--no-baz']}, ['foo'])
+
+    # Old-style flags don't count as explicit goals.
+    self._split('./pants compile --test-junit-bar foo',
+                ['compile'],
+                {'': [], 'compile': [], 'test.junit': ['--bar']}, ['foo'])
 
     # Passthru args.
-    self._split('./pants test foo -- -t arg', {'': [], 'test': []}, ['foo'], ['-t', 'arg'])
+    self._split('./pants test foo -- -t arg',
+                ['test'],
+                {'': [], 'test': []},
+                ['foo'],
+                expected_passthru=['-t', 'arg'],
+                expected_passthru_owner='test')
     self._split('./pants -farg --fff=arg compile --gg-gg=arg-arg -g test.junit --iii '
                 '--compile-java-long-flag src/java/com/pants/foo src/java/com/pants/bar:baz '
                 '-- passthru1 passthru2',
+                ['compile', 'test'],
                 {
                   '': ['-farg', '--fff=arg'],
                   'compile': ['--gg-gg=arg-arg', '-g'],
                   'compile.java': ['--long-flag'],
                   'test.junit': ['--iii']
                 },
                 ['src/java/com/pants/foo', 'src/java/com/pants/bar:baz'],
-                ['passthru1', 'passthru2'])
+                expected_passthru=['passthru1', 'passthru2'],
+                expected_passthru_owner='test.junit')
 
   def test_help_detection(self):
-    self._split('./pants help', {'': []}, [], [], True)
-    self._split('./pants goal help', {'': []}, [], [], True)
-    self._split('./pants -h', {'': []}, [], [], True)
-    self._split('./pants goal -h', {'': []}, [], [], True)
-    self._split('./pants --help', {'': []}, [], [], True)
-    self._split('./pants goal --help', {'': []}, [], [], True)
-    self._split('./pants help compile -x', {'': [], 'compile': ['-x']}, [], [], True)
-    self._split('./pants help compile -x', {'': [], 'compile': ['-x']}, [], [], True)
-    self._split('./pants compile -h', {'': [], 'compile': []}, [], [], True)
-    self._split('./pants compile --help test', {'': [], 'compile': [], 'test': []}, [], [], True)
+    self._split('./pants help', ['help'], {'': []}, [], [], expected_is_help=True)
+    self._split('./pants goal help', ['help'], {'': []}, [], [], expected_is_help=True)
+    self._split('./pants -h', [], {'': []}, [], [], expected_is_help=True)
+    self._split('./pants goal -h', [], {'': []}, [], [], expected_is_help=True)
+    self._split('./pants --help', [], {'': []}, [], [], expected_is_help=True)
+    self._split('./pants goal --help', [], {'': []}, [], [], expected_is_help=True)
+    self._split('./pants help compile -x', ['help', 'compile'],
+                {'': [], 'compile': ['-x']}, [], [], expected_is_help=True)
+    self._split('./pants help compile -x', ['help', 'compile'],
+                {'': [], 'compile': ['-x']}, [], [], expected_is_help=True)
+    self._split('./pants compile -h', ['compile'],
+                {'': [], 'compile': []}, [], [], expected_is_help=True)
+    self._split('./pants compile --help test', ['compile', 'test'],
+                {'': [], 'compile': [], 'test': []}, [], [], expected_is_help=True)