Add support for constructing Optionables (including Subsystems) via @…

…rules, and consume a ListOptions Subsystem in order to implement the rest of `list`.

src/python/pants/build_graph/build_configuration.py

@@ -8,11 +8,14 @@
 from builtins import object, str
 from collections import Iterable, namedtuple
 
+from twitter.common.collections import OrderedSet
+
 from pants.base.deprecated import deprecated
 from pants.base.parse_context import ParseContext
 from pants.build_graph.addressable import AddressableCallProxy
 from pants.build_graph.build_file_aliases import BuildFileAliases
 from pants.build_graph.target_addressable import TargetAddressable
+from pants.engine.rules import RuleIndex
 from pants.option.optionable import Optionable
 from pants.subsystem.subsystem import Subsystem
 from pants.util.memo import memoized_method
@@ -34,8 +37,8 @@ def __init__(self):
     self._target_macro_factory_by_alias = {}
     self._exposed_object_by_alias = {}
     self._exposed_context_aware_object_factory_by_alias = {}
-    self._optionables = set()
-    self._rules = []
+    self._optionables = OrderedSet()
+    self._rules = OrderedSet()
 
   def registered_aliases(self):
     """Return the registered aliases exposed in BUILD files.
@@ -154,21 +157,30 @@ def subsystems(self):
   def register_rules(self, rules):
     """Registers the given rules.
 
-    :param rules: The rules to register.
+    param rules: The rules to register.
     :type rules: :class:`collections.Iterable` containing
                  :class:`pants.engine.rules.Rule` instances.
     """
-
     if not isinstance(rules, Iterable):
       raise TypeError('The rules must be an iterable, given {!r}'.format(rules))
-    self._rules.extend(rules)
+
+    # "Index" the rules to normalize them and expand their dependencies.
+    indexed_rules = RuleIndex.create(rules).normalized_rules()
+
+    # Store the rules and record their dependency Optionables.
+    self._rules.update(indexed_rules)
+    dependency_optionables = {do
+                              for rule in indexed_rules
+                              for do in rule.dependency_optionables
+                              if rule.dependency_optionables}
+    self.register_optionables(dependency_optionables)
 
   def rules(self):
     """Returns the registered rules.
 
     :rtype list
     """
-    return self._rules
+    return list(self._rules)
 
   @memoized_method
   def _get_addressable_factory(self, target_type, alias):

src/python/pants/engine/rules.py

@@ -18,6 +18,7 @@
 from twitter.common.collections import OrderedSet
 
 from pants.engine.selectors import Get, type_or_constraint_repr
+from pants.util.memo import memoized
 from pants.util.meta import AbstractClass
 from pants.util.objects import Exactly, datatype
 
@@ -72,6 +73,19 @@ def _terminated(generator, terminator):
     yield terminator
 
 
+@memoized
+def optionable_rule(optionable_factory):
+  """Returns a TaskRule that constructs an instance of the Optionable for the given OptionableFactory.
+
+  TODO: This API is slightly awkward for two reasons:
+    1) We should consider whether Subsystems/Optionables should be constructed explicitly using
+      `@rule`s, which would allow them to have non-option dependencies that would be explicit in
+      their constructors (which would avoid the need for the `Subsystem.Factory` pattern).
+    2) Optionable depending on TaskRule would create a cycle in the Python package graph.
+  """
+  return TaskRule(**optionable_factory.signature())
+
+
 def _make_rule(output_type, input_selectors, for_goal=None, cacheable=True):
   """A @decorator that declares that a particular static function may be used as a TaskRule.
 
@@ -123,9 +137,14 @@ def goal_and_return(*args, **kwargs):
     else:
       wrapped_func = func
 
-    wrapped_func._rule = TaskRule(output_type, tuple(input_selectors), wrapped_func, input_gets=tuple(gets), cacheable=cacheable)
-    wrapped_func.output_type = output_type
-    wrapped_func.goal = for_goal
+    wrapped_func.rule = TaskRule(
+        output_type,
+        tuple(input_selectors),
+        wrapped_func,
+        input_gets=tuple(gets),
+        goal=for_goal,
+        cacheable=cacheable
+      )
 
     return wrapped_func
   return wrapper
@@ -151,17 +170,35 @@ class Rule(AbstractClass):
   def output_constraint(self):
     """An output Constraint type for the rule."""
 
+  @abstractproperty
+  def dependency_optionables(self):
+    """A tuple of Optionable classes that are known to be necessary to run this rule."""
+
 
 class TaskRule(datatype([
   'output_constraint',
   ('input_selectors', tuple),
   ('input_gets', tuple),
   'func',
+  'goal',
+  ('dependency_optionables', tuple),
   ('cacheable', bool),
 ]), Rule):
-  """A Rule that runs a task function when all of its input selectors are satisfied."""
+  """A Rule that runs a task function when all of its input selectors are satisfied.
 
-  def __new__(cls, output_type, input_selectors, func, input_gets, cacheable=True):
+  NB: This API is experimental, and not meant for direct consumption. To create a `TaskRule` you
+  should always prefer the `@rule` constructor, and in cases where that is too constraining
+  (likely due to #4535) please bump or open a ticket to explain the usecase.
+  """
+
+  def __new__(cls,
+              output_type,
+              input_selectors,
+              func,
+              input_gets,
+              goal=None,
+              dependency_optionables=None,
+              cacheable=True):
     # Validate result type.
     if isinstance(output_type, Exactly):
       constraint = output_type
@@ -171,8 +208,16 @@ def __new__(cls, output_type, input_selectors, func, input_gets, cacheable=True)
       raise TypeError("Expected an output_type for rule `{}`, got: {}".format(
         func.__name__, output_type))
 
-    # Create.
-    return super(TaskRule, cls).__new__(cls, constraint, input_selectors, input_gets, func, cacheable)
+    return super(TaskRule, cls).__new__(
+        cls,
+        constraint,
+        input_selectors,
+        input_gets,
+        func,
+        goal,
+        dependency_optionables or tuple(),
+        cacheable,
+      )
 
   def __str__(self):
     return '({}, {!r}, {})'.format(type_or_constraint_repr(self.output_constraint),
@@ -199,6 +244,10 @@ def __new__(cls, output_type, value):
     # Create.
     return super(SingletonRule, cls).__new__(cls, constraint, value)
 
+  @property
+  def dependency_optionables(self):
+    return tuple()
+
   def __repr__(self):
     return '{}({}, {})'.format(type(self).__name__, type_or_constraint_repr(self.output_constraint), self.value)
 
@@ -211,16 +260,19 @@ class RootRule(datatype(['output_constraint']), Rule):
   of an execution.
   """
 
+  @property
+  def dependency_optionables(self):
+    return tuple()
+
 
 class RuleIndex(datatype(['rules', 'roots'])):
-  """Holds an index of Tasks and Singletons used to instantiate Nodes."""
+  """Holds a normalized index of Rules used to instantiate Nodes."""
 
   @classmethod
   def create(cls, rule_entries):
     """Creates a RuleIndex with tasks indexed by their output type."""
-    # NB make tasks ordered so that gen ordering is deterministic.
     serializable_rules = OrderedDict()
-    serializable_roots = set()
+    serializable_roots = OrderedSet()
 
     def add_task(product_type, rule):
       if product_type not in serializable_rules:
@@ -229,7 +281,7 @@ def add_task(product_type, rule):
 
     def add_rule(rule):
       if isinstance(rule, RootRule):
-        serializable_roots.add(rule.output_constraint)
+        serializable_roots.add(rule)
         return
       # TODO: Ensure that interior types work by indexing on the list of types in
       # the constraint. This heterogenity has some confusing implications:
@@ -242,7 +294,7 @@ def add_rule(rule):
       if isinstance(entry, Rule):
         add_rule(entry)
       elif hasattr(entry, '__call__'):
-        rule = getattr(entry, '_rule', None)
+        rule = getattr(entry, 'rule', None)
         if rule is None:
           raise TypeError("Expected callable {} to be decorated with @rule.".format(entry))
         add_rule(rule)
@@ -252,3 +304,10 @@ def add_rule(rule):
                         "decorated with @rule.".format(type(entry)))
 
     return cls(serializable_rules, serializable_roots)
+
+  def normalized_rules(self):
+    rules = OrderedSet(rule
+                       for ruleset in self.rules.values()
+                       for rule in ruleset)
+    rules.update(self.roots)
+    return rules

src/python/pants/engine/scheduler.py

@@ -80,7 +80,7 @@ def __init__(
     self._visualize_to_dir = visualize_to_dir
     # Validate and register all provided and intrinsic tasks.
     rule_index = RuleIndex.create(list(rules))
-    self._root_subject_types = sorted(rule_index.roots, key=repr)
+    self._root_subject_types = [r.output_constraint for r in rule_index.roots]
 
     # Create the native Scheduler and Session.
     # TODO: This `_tasks` reference could be a local variable, since it is not used
@@ -130,7 +130,7 @@ def __init__(
       self._assert_ruleset_valid()
 
   def _root_type_ids(self):
-    return self._to_ids_buf(sorted(self._root_subject_types, key=repr))
+    return self._to_ids_buf(self._root_subject_types)
 
   def graph_trace(self, execution_request):
     with temporary_file_path() as path:

src/python/pants/help/scope_info_iterator.py

@@ -67,7 +67,7 @@ def _expand_subsystems(self, scope_infos):
     def subsys_deps(subsystem_client_cls):
       for dep in subsystem_client_cls.subsystem_dependencies_iter():
         if dep.scope != GLOBAL_SCOPE:
-          yield self._scope_to_info[dep.options_scope()]
+          yield self._scope_to_info[dep.options_scope]
           for x in subsys_deps(dep.subsystem_cls):
             yield x
 

src/python/pants/init/engine_initializer.py

@@ -39,7 +39,6 @@
 from pants.init.options_initializer import BuildConfigInitializer
 from pants.option.global_options import (DEFAULT_EXECUTION_OPTIONS, ExecutionOptions,
                                          GlobMatchErrorBehavior)
-from pants.rules.core.register import create_core_rules
 from pants.util.objects import datatype
 
 
@@ -248,7 +247,7 @@ def _make_goal_map_from_rules(rules):
           'could not map goal `{}` to rule `{}`: already claimed by product `{}`'
           .format(goal, rule, goal_map[goal])
         )
-      goal_map[goal] = rule.output_type
+      goal_map[goal] = rule.output_constraint
     return goal_map
 
   @staticmethod
@@ -356,7 +355,6 @@ def setup_legacy_graph_extended(
       create_process_rules() +
       create_graph_rules(address_mapper) +
       create_options_parsing_rules() +
-      create_core_rules() +
       # TODO: This should happen automatically, but most tests (e.g. tests/python/pants_test/auth) fail if it's not here:
       [run_python_test] +
       rules

src/python/pants/option/global_options.py

@@ -134,6 +134,8 @@ def register_bootstrap_options(cls, register):
                       'pants.backend.python',
                       'pants.backend.jvm',
                       'pants.backend.native',
+                      # TODO: Move into the graph_info backend.
+                      'pants.rules.core',
                       'pants.backend.codegen.antlr.java',
                       'pants.backend.codegen.antlr.python',
                       'pants.backend.codegen.jaxb',

src/python/pants/option/optionable.py

@@ -4,16 +4,59 @@
 
 from __future__ import absolute_import, division, print_function, unicode_literals
 
+import functools
 import re
+from abc import abstractproperty
+from builtins import str
 
 from six import string_types
 
+from pants.engine.selectors import Get
 from pants.option.errors import OptionsError
-from pants.option.scope import ScopeInfo
-from pants.util.meta import AbstractClass
+from pants.option.scope import Scope, ScopedOptions, ScopeInfo
+from pants.util.meta import AbstractClass, classproperty
 
 
-class Optionable(AbstractClass):
+def _construct_optionable(optionable_factory):
+  scope = optionable_factory.options_scope
+  scoped_options = yield Get(ScopedOptions, Scope(str(scope)))
+  yield optionable_factory.optionable_cls(scope, scoped_options.options)
+
+
+class OptionableFactory(AbstractClass):
+  """A mixin that provides a method that returns an @rule to construct subclasses of Optionable.
+
+  Optionable subclasses constructed in this manner must have a particular constructor shape, which is
+  loosely defined by `_construct_optionable` and `OptionableFactory.signature`.
+  """
+
+  @abstractproperty
+  def optionable_cls(self):
+    """The Optionable class that is constructed by this OptionableFactory."""
+
+  @abstractproperty
+  def options_scope(self):
+    """The scope from which the ScopedOptions for the target Optionable will be parsed."""
+
+  @classmethod
+  def signature(cls):
+    """Returns kwargs to construct a `TaskRule` that will construct the target Optionable.
+
+    TODO: This indirection avoids a cycle between this module and the `rules` module.
+    """
+    snake_scope = cls.options_scope.replace('-', '_')
+    partial_construct_optionable = functools.partial(_construct_optionable, cls)
+    partial_construct_optionable.__name__ = 'construct_scope_{}'.format(snake_scope)
+    return dict(
+        output_type=cls.optionable_cls,
+        input_selectors=tuple(),
+        func=partial_construct_optionable,
+        input_gets=(Get(ScopedOptions, Scope),),
+        dependency_optionables=(cls.optionable_cls,),
+      )
+
+
+class Optionable(OptionableFactory, AbstractClass):
   """A mixin for classes that can register options on some scope."""
 
   # Subclasses must override.
@@ -29,6 +72,11 @@ class Optionable(AbstractClass):
 
   _scope_name_component_re = re.compile(r'^(?:[a-z0-9])+(?:-(?:[a-z0-9])+)*$')
 
+  @classproperty
+  def optionable_cls(cls):
+    # Fills the `OptionableFactory` contract.
+    return cls
+
   @classmethod
   def is_valid_scope_name_component(cls, s):
     return cls._scope_name_component_re.match(s) is not None