Ensure that the new options system reports goals in the right order.

We sometimes rely on this, e.g., when specifying "clean-all compile".

Testing Done:
option unittests pass. CI passed when run with https://rbcommons.com/s/twitter/r/1309/

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

src/python/pants/option/BUILD

@@ -5,6 +5,7 @@ python_library(
   name = 'option',
   sources = globs('*.py') - ['migrate_config.py'],
   dependencies = [
+    '3rdparty/python/twitter/commons:twitter.common.collections',
     'src/python/pants/base:build_environment',
     'src/python/pants/goal'
   ]

src/python/pants/option/arg_splitter.py

@@ -5,7 +5,7 @@
 from __future__ import (nested_scopes, generators, division, absolute_import, with_statement,
                         print_function, unicode_literals)
 
-from collections import defaultdict, namedtuple
+from collections import namedtuple, OrderedDict
 import sys
 
 
@@ -19,8 +19,9 @@ class ArgSplitterError(Exception):
 class SplitArgs(namedtuple('SplitArgs', ['scope_to_flags', 'targets', 'passthru'])):
   """The result of splitting args.
 
-  scope_to_flags: A map from scope name to the list of flags belonging to that scope.
+  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.
   """
@@ -63,7 +64,13 @@ def split_args(self, args=None):
 
     Returns a SplitArgs tuple.
     """
-    scope_to_flags = defaultdict(list)
+    scope_to_flags = OrderedDict()
+
+    def add_scope(s):
+      # Force the scope to appear, even if empty.
+      if s not in scope_to_flags:
+        scope_to_flags[s] = []
+
     targets = []
     passthru = []
 
@@ -81,15 +88,17 @@ def split_args(self, args=None):
 
     def assign_flag_to_scope(flag, default_scope):
       flag_scope, descoped_flag = self._descope_flag(flag, default_scope=default_scope)
+      if flag_scope not in scope_to_flags:
+        scope_to_flags[flag_scope] = []
       scope_to_flags[flag_scope].append(descoped_flag)
 
     global_flags = self._consume_flags()
-    scope_to_flags[GLOBAL_SCOPE].extend([])  # Force the scope to appear, even if empty.
+    add_scope(GLOBAL_SCOPE)
     for flag in global_flags:
       assign_flag_to_scope(flag, GLOBAL_SCOPE)
     scope, flags = self._consume_scope()
     while scope:
-      scope_to_flags[scope].extend([])  # Force the scope to appear, even if empty.
+      add_scope(scope)
       for flag in flags:
         assign_flag_to_scope(flag, scope)
       scope, flags = self._consume_scope()

src/python/pants/option/options.py

@@ -8,6 +8,8 @@
 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
@@ -82,10 +84,8 @@ def target_specs(self):
 
   @property
   def goals(self):
-    """The requested goals."""
-    # TODO: Order them in some way? We don't know anything about the topological
-    # order here, but it would be nice to, e.g., display help in that order.
-    return set([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 OrderedSet([g.partition('.')[0] for g in self._scope_to_flags.keys() if g])
 
   @property
   def is_help(self):

tests/python/pants_test/option/test_arg_splitter.py

@@ -5,11 +5,11 @@
 from __future__ import (nested_scopes, generators, division, absolute_import, with_statement,
                         print_function, unicode_literals)
 
-import pytest
+from collections import OrderedDict
 import shlex
 import unittest2 as unittest
 
-from pants.option.arg_splitter import ArgSplitter, ArgSplitterError
+from pants.option.arg_splitter import ArgSplitter
 
 
 class ArgSplitterTest(unittest.TestCase):
@@ -34,17 +34,21 @@ def test_arg_splitting(self):
     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',
-                {'': ['-f'], 'compile': ['-g'], 'compile.java': ['--long-flag', '-x'],
-                 'test.junit': ['-i']},
+                OrderedDict([
+                  ('', ['-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',
-                {
-                  '': ['-farg', '--fff=arg'],
-                  'compile': ['--gg-gg=arg-arg', '-g'],
-                  'compile.java': ['--long-flag'],
-                  'test.junit': ['--iii']
-                },
+                OrderedDict([
+                  ('', ['-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.