[engine] Move subselectors to selector properties

Review Request #4235 — Created Sept. 13, 2016 and submitted

nhoward_tw
pants
3867
pants-reviews
kwlzn, stuhood, yujiec

Select/Dependency/ProjectionNodes cause sub-selectors to be generated in order to select different products. This moves those selectors onto the selectors instead of having the nodes construct them directly.

Working on the new validation / graph creation, I find myself needing those sub-selectors in a few other places, so I wanted to give them consistent names.

Ran engine tests locally. CI away in PR.

YU
  1. I think this change sacrifices code cleanliness a little bit. Maybe we can modify Select.\_\_new\_\_ to be memoized?
    1. Part of my reason for wanting this is so that analysis code can grab the subselectors for the various selector types. I could add @property based declarations instead as a starting point. Then later when we do a profiling pass, we can decide whether to do something like this.

      How's that sound?

    2. I think that sounds good! Thanks!

  2. 
      
NH
ST
  1. Ship It!
  2. 
      
KW
  1. lgtm, one non-blocking thought.

  2. src/python/pants/engine/nodes.py (Diff revision 2)
     
     

    any particular reason to not move this to SelectDependencies and access similarly to the others (e.g. self.selector.variant_selector)?

    1. Hm. I think if it went anywhere, it'd be to Select. That could make sense. But, the model I'm working on doesn't cover variants yet, so I wanted to avoid abstracting over them too much.

      I'm going to leave it as is--keeping it close to the point of use.

  3. 
      
NH
Review request changed

Status: Closed (submitted)

Change Summary:

Submitted as https://github.com/pantsbuild/pants/commit/1d096907cae1f729ff3abcff3b144df43df65af9
Loading...