[engine] Calculate legacy target sources using the engine

Review Request #3592 — Created March 21, 2016 and submitted

stuhood
pants
3058, 3076
pants-reviews
dturner-tw, gmalmquist, kwlzn, peiyu

This review replaces sources fields on targets reified in the ExpGraph with a helper object that creates a FilesetWithSpec that uses the engine to compute the Paths and FileContent. This means that once construction of the Graph switches over, all BUILD files and source hashes will be cached.

This is in support of #3058.

  • Replace glob implementations with capturing implementations and create FilesetWithSpecs.
  • Add filemap legacy command to demonstrate that we are able to compute paths.
  • Mutate a single ParseContext in LegacyParser rather than recreating for each path.
  • Normalize SourcesField to always consume a FilesetWithSpec object.
  • Move the implementation of file-content fetching to FilesetWithSpec, to allow our alternate implementation to use the engine to get FileContent.
  • Finished removing num_chunking_units (after r/3474), which was forcing sources in cases where we otherwise didn't need to.

https://travis-ci.org/pantsbuild/pants/builds/118376108

  • 0
  • 0
  • 1
  • 0
  • 1
Description From Last Updated
  1. lgtm, afaict!

  2. not sure what a Lobs is. maybe LegacyFilesetWrapper?

  3. src/python/pants/engine/exp/legacy/globs.py (Diff revision 1)
     
     
     
     
     
     
     
     

    consider replacing the passs here with simple docstrings

  4. 
      
  1. 
      
  2. GlobBase or LegacyGlobBase?

  3. 
      
  1. Re the globs with the missing files, it seems like in the long run we might want support for reading these from an alternate source (e.g. git). So that's one option. The other is just to create the files, of course.

  2. 
      
  1. 
      
  2. src/python/pants/source/wrapped_globs.py (Diff revision 5)
     
     
     

    This will not throw a value error if the user calls this with a kwarg that is not root but only one. I would expect kwargs.keys() to be ['root'] or []

    Files._literal_files(foo='bar')

    1. seems to me like this is guaranteed by the root = kwargs['root'] above, which would throw a KeyError if the one key in the dict is not 'root'.

    2. Clarified this with:

      if kwargs.keys() != {'root'}:
        ..
      
  3. 
      
  1. lgtm!

  2. src/python/pants/engine/exp/legacy/graph.py (Diff revisions 2 - 5)
     
     
     
     
     
     
     

    complete driveby, so take it with a grain of salt - but isn't the expected return type here FilesetWithSpec vs Addresses?

    or are the two interchangeable?

    1. Addresses is for the case where from_target is in use. Will update the comment.

  3. 
      
Review request changed

Status: Closed (submitted)

Change Summary:

Merged as 1c7de069b54c4fde261b0cf7d099cbd0060f73ad

Loading...