[engine] Pass selectors to select nodes; Use selectors in error messages

Currently selectors are dropped when select nodes for task nodes are instantiated. This means that if a selector fails to match, the error message can't reproduce the selector.

This changes the select nodes so that they are passed the selector and can reuse it for producing error messages. It additionally overrides the `__repr__` implementations for selectors so they'll be displayed similarly to how they were declared.

Currently a select failure will look like this:

    Noop(msg=u"No source of SelectNode(subject=PathGlobs(dependencies=(PathDirWildcard(canonical_stat=Dir(path=u''), symbolic_path=u'', wildcard=u'fs_test', remainder=u'a/b/*'),)), product=<class 'pants_test.engine.test_isolated_process.Concatted'>, variants=None, variant_key=None).").

With the repr and message changes, it looks like this:

    Noop(msg=u"No source of SelectNode(subject=PathGlobs(dependencies=(PathDirWildcard(canonical_stat=Dir(path=u''), symbolic_path=u'', wildcard=u'fs_test', remainder=u'a/b/*'),)), variants=None, selector=Select(Concatted)).")

Testing Done:
CI away on PR.

Bugs closed: 3611

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

closes pantsbuild#3611

src/python/pants/engine/nodes.py

@@ -131,7 +131,7 @@ def step(self, step_context):
     """
 
 
-class SelectNode(datatype('SelectNode', ['subject', 'product', 'variants', 'variant_key']), Node):
+class SelectNode(datatype('SelectNode', ['subject', 'variants', 'selector']), Node):
   """A Node that selects a product for a subject.
 
   A Select can be satisfied by multiple sources, but fails if multiple sources produce a value. The
@@ -143,16 +143,23 @@ class SelectNode(datatype('SelectNode', ['subject', 'product', 'variants', 'vari
   is_cacheable = False
   is_inlineable = True
 
-  def _variants_node(self):
-    if type(self.subject) is Address and self.product is not Variants:
-      return SelectNode(self.subject, Variants, self.variants, None)
-    return None
+  @property
+  def variant_key(self):
+    if isinstance(self.selector, SelectVariant):
+      return self.selector.variant_key
+    else:
+      return None
+
+  @property
+  def product(self):
+    return self.selector.product
 
   def _select_literal(self, candidate, variant_value):
     """Looks for has-a or is-a relationships between the given value and the requested product.
 
     Returns the resulting product value, or None if no match was made.
     """
+
     def items():
       # Check whether the subject is-a instance of the product.
       yield candidate
@@ -175,8 +182,8 @@ def step(self, step_context):
     # Request default Variants for the subject, so that if there are any we can propagate
     # them to task nodes.
     variants = self.variants
-    variants_node = self._variants_node()
-    if variants_node:
+    if type(self.subject) is Address and self.product is not Variants:
+      variants_node = step_context.select_node(Select(Variants), self.subject, self.variants)
       dep_state = step_context.get(variants_node)
       if type(dep_state) is Waiting:
         return dep_state
@@ -234,7 +241,7 @@ def step(self, step_context):
       return Return(matches[0][1])
 
 
-class DependenciesNode(datatype('DependenciesNode', ['subject', 'product', 'variants', 'dep_product', 'field']), Node):
+class DependenciesNode(datatype('DependenciesNode', ['subject', 'variants', 'selector']), Node):
   """A Node that selects the given Product for each of the items in `field` on `dep_product`.
 
   Begins by selecting the `dep_product` for the subject, and then selects a product for each
@@ -246,8 +253,21 @@ class DependenciesNode(datatype('DependenciesNode', ['subject', 'product', 'vari
   is_cacheable = False
   is_inlineable = True
 
-  def _dep_product_node(self):
-    return SelectNode(self.subject, self.dep_product, self.variants, None)
+  def __new__(cls, subject, variants, selector):
+    return super(DependenciesNode, cls).__new__(cls, subject, variants,
+                                                selector)
+
+  @property
+  def dep_product(self):
+    return self.selector.dep_product
+
+  @property
+  def product(self):
+    return self.selector.product
+
+  @property
+  def field(self):
+    return self.selector.field
 
   def _dependency_nodes(self, step_context, dep_product):
     for dependency in getattr(dep_product, self.field or 'dependencies'):
@@ -256,11 +276,11 @@ def _dependency_nodes(self, step_context, dep_product):
         # If a subject has literal variants for particular dependencies, they win over all else.
         dependency, literal_variants = parse_variants(dependency)
         variants = Variants.merge(variants, literal_variants)
-      yield SelectNode(dependency, self.product, variants, None)
+      yield step_context.select_node(Select(self.product), subject=dependency, variants=variants)
 
   def step(self, step_context):
     # Request the product we need in order to request dependencies.
-    dep_product_node = self._dep_product_node()
+    dep_product_node = step_context.select_node(Select(self.dep_product), self.subject, self.variants)
     dep_product_state = step_context.get(dep_product_node)
     if type(dep_product_state) in (Throw, Waiting):
       return dep_product_state
@@ -290,7 +310,7 @@ def step(self, step_context):
     return Return(dep_values)
 
 
-class ProjectionNode(datatype('ProjectionNode', ['subject', 'product', 'variants', 'projected_subject', 'fields', 'input_product']), Node):
+class ProjectionNode(datatype('ProjectionNode', ['subject', 'variants', 'selector']), Node):
   """A Node that selects the given input Product for the Subject, and then selects for a new subject.
 
   TODO: This is semantically very similar to DependenciesNode (which might be considered to be a
@@ -299,15 +319,25 @@ class ProjectionNode(datatype('ProjectionNode', ['subject', 'product', 'variants
   is_cacheable = False
   is_inlineable = True
 
-  def _input_node(self):
-    return SelectNode(self.subject, self.input_product, self.variants, None)
+  @property
+  def product(self):
+    return self.selector.product
 
-  def _output_node(self, step_context, projected_subject):
-    return SelectNode(projected_subject, self.product, self.variants, None)
+  @property
+  def projected_subject(self):
+    return self.selector.projected_subject
+
+  @property
+  def fields(self):
+    return self.selector.fields
+
+  @property
+  def input_product(self):
+    return self.selector.input_product
 
   def step(self, step_context):
     # Request the product we need to compute the subject.
-    input_node = self._input_node()
+    input_node = step_context.select_node(Select(self.input_product), self.subject, self.variants)
     input_state = step_context.get(input_node)
     if type(input_state) in (Throw, Waiting):
       return input_state
@@ -330,15 +360,15 @@ def step(self, step_context):
         projected_subject = self.projected_subject(*values)
     except Exception as e:
       return Throw(ValueError('Fields {} of {} could not be projected as {}: {}'.format(
-        self.fields, input_product, self.projected_subject, e)))
-    output_node = self._output_node(step_context, projected_subject)
+                   self.fields, input_product, self.projected_subject, e)))
 
     # When the output node is available, return its result.
+    output_node = step_context.select_node(Select(self.product), projected_subject, self.variants)
     output_state = step_context.get(output_node)
     if type(output_state) in (Return, Throw, Waiting):
       return output_state
     elif type(output_state) is Noop:
-      return Throw(ValueError('No source of projected dependency {}'.format(output_node)))
+      return Throw(ValueError('No source of projected dependency {}'.format(output_node.selector)))
     else:
       raise State.raise_unrecognized(output_state)
 
@@ -369,8 +399,9 @@ def step(self, step_context):
         if selector.optional:
           dep_values.append(None)
         else:
-          return Noop('Was missing (at least) input {}.', dep_node)
+          return Noop('Was missing (at least) input for {}.', selector)
       elif type(dep_state) is Throw:
+        # NB: propagate thrown exception directly.
         return dep_state
       else:
         State.raise_unrecognized(dep_state)
@@ -484,16 +515,14 @@ def select_node(self, selector, subject, variants):
     need a dependency on the `nodes` package.
     """
     selector_type = type(selector)
-    if selector_type is Select:
-      return SelectNode(subject, selector.product, variants, None)
-    elif selector_type is SelectVariant:
-      return SelectNode(subject, selector.product, variants, selector.variant_key)
-    elif selector_type is SelectDependencies:
-      return DependenciesNode(subject, selector.product, variants, selector.deps_product, selector.field)
-    elif selector_type is SelectProjection:
-      return ProjectionNode(subject, selector.product, variants, selector.projected_subject, selector.fields, selector.input_product)
+    if selector_type in [Select, SelectVariant]:
+      return SelectNode(subject, variants, selector)
     elif selector_type is SelectLiteral:
       # NB: Intentionally ignores subject parameter to provide a literal subject.
-      return SelectNode(selector.subject, selector.product, variants, None)
+      return SelectNode(selector.subject, variants, selector)
+    elif selector_type is SelectDependencies:
+      return DependenciesNode(subject, variants, selector)
+    elif selector_type is SelectProjection:
+      return ProjectionNode(subject, variants, selector)
     else:
       raise ValueError('Unrecognized Selector type "{}" for: {}'.format(selector_type, selector))

src/python/pants/engine/scheduler.py

@@ -19,6 +19,7 @@
 from pants.engine.nodes import (DependenciesNode, FilesystemNode, Node, Noop, Return, SelectNode,
                                 State, StepContext, TaskNode, Throw, Waiting)
 from pants.engine.objects import Closable
+from pants.engine.selectors import Select, SelectDependencies
 from pants.util.objects import datatype
 
 
@@ -645,12 +646,10 @@ def execution_request(self, products, subjects):
     def roots():
       for subject in subjects:
         for product in products:
-          if type(subject) is Address:
-            yield SelectNode(subject, product, None, None)
+          if type(subject) in [Address, PathGlobs]:
+            yield SelectNode(subject, None, Select(product))
           elif type(subject) in [SingleAddress, SiblingAddresses, DescendantAddresses]:
-            yield DependenciesNode(subject, product, None, Addresses, None)
-          elif type(subject) is PathGlobs:
-            yield SelectNode(subject, product, None, None)
+            yield DependenciesNode(subject, None, SelectDependencies(product, Addresses))
           else:
             raise ValueError('Unsupported root subject type: {}'.format(subject))
 

src/python/pants/engine/selectors.py

@@ -19,7 +19,7 @@ def optional(self):
     """Return true if this Selector is optional. It may result in a `None` match."""
 
 
-class Select(datatype('Subject', ['product', 'optional']), Selector):
+class Select(datatype('Select', ['product', 'optional']), Selector):
   """Selects the given Product for the Subject provided to the constructor.
 
   If optional=True and no matching product can be produced, will return None.
@@ -28,6 +28,11 @@ class Select(datatype('Subject', ['product', 'optional']), Selector):
   def __new__(cls, product, optional=False):
     return super(Select, cls).__new__(cls, product, optional)
 
+  def __repr__(self):
+    return '{}({}{})'.format(type(self).__name__,
+                             self.product.__name__,
+                             ', optional=True' if self.optional else '')
+
 
 class SelectVariant(datatype('Variant', ['product', 'variant_key']), Selector):
   """Selects the matching Product and variant name for the Subject provided to the constructor.
@@ -38,20 +43,31 @@ class SelectVariant(datatype('Variant', ['product', 'variant_key']), Selector):
   """
   optional = False
 
+  def __repr__(self):
+    return '{}({}, {})'.format(type(self).__name__,
+                             self.product.__name__,
+                             repr(self.variant_key))
+
 
-class SelectDependencies(datatype('Dependencies', ['product', 'deps_product', 'field']), Selector):
+class SelectDependencies(datatype('Dependencies', ['product', 'dep_product', 'field']), Selector):
   """Selects a product for each of the dependencies of a product for the Subject.
 
-  The dependencies declared on `deps_product` (in the optional `field` parameter, which defaults
+  The dependencies declared on `dep_product` (in the optional `field` parameter, which defaults
   to 'dependencies' when not specified) will be provided to the requesting task in the
   order they were declared.
   """
 
-  def __new__(cls, product, deps_product, field=None):
-    return super(SelectDependencies, cls).__new__(cls, product, deps_product, field)
+  def __new__(cls, product, dep_product, field=None):
+    return super(SelectDependencies, cls).__new__(cls, product, dep_product, field)
 
   optional = False
 
+  def __repr__(self):
+    return '{}({}, {}{})'.format(type(self).__name__,
+                             self.product.__name__,
+                             self.dep_product.__name__,
+                             ', {}'.format(repr(self.field)) if self.field else '')
+
 
 class SelectProjection(datatype('Projection', ['product', 'projected_subject', 'fields', 'input_product']), Selector):
   """Selects a field of the given Subject to produce a Subject, Product dependency from.
@@ -64,11 +80,21 @@ class SelectProjection(datatype('Projection', ['product', 'projected_subject', '
   """
   optional = False
 
+  def __repr__(self):
+    return '{}({}, {}, {}, {})'.format(type(self).__name__,
+                             self.product.__name__,
+                             self.projected_subject.__name__,
+                             repr(self.fields),
+                             self.input_product.__name__)
+
 
 class SelectLiteral(datatype('Literal', ['subject', 'product']), Selector):
   """Selects a literal Subject (other than the one applied to the selector)."""
   optional = False
 
+  def __repr__(self):
+    return '{}({}, {})'.format(type(self).__name__, repr(self.subject), self.product.__name__)
+
 
 class Collection(object):
   """

tests/python/pants_test/engine/BUILD

@@ -76,6 +76,14 @@ python_tests(
   ]
 )
 
+python_tests(
+  name='selectors',
+  sources=['test_selectors.py'],
+  dependencies=[
+    'src/python/pants/engine:selectors',
+  ]
+)
+
 python_tests(
   name='struct',
   sources=['test_struct.py'],

tests/python/pants_test/engine/test_engine.py

@@ -17,6 +17,7 @@
                                  ThreadHybridEngine)
 from pants.engine.nodes import FilesystemNode, Return, SelectNode
 from pants.engine.scheduler import Promise
+from pants.engine.selectors import Select
 from pants.engine.storage import Cache, Storage
 from pants_test.engine.examples.planners import Classpath, setup_json_scheduler
 
@@ -34,7 +35,7 @@ def request(self, goals, *addresses):
 
   def assert_engine(self, engine):
     result = engine.execute(self.request(['compile'], self.java))
-    self.assertEqual({SelectNode(self.java, Classpath, None, None):
+    self.assertEqual({SelectNode(self.java, None, Select(Classpath)):
                       Return(Classpath(creator='javac'))},
                      result.root_products)
     self.assertIsNone(result.error)