[engine] Fix filedeps goal in v2 engine

Review Request #4137 — Created Aug. 5, 2016 and updated

ity
pants
3752, 3753
pants-reviews
jsirois, kwlzn, nhoward_tw, stuhood
  • In the v1 engine an Address is resolved to a BuildFileAddress, in the v2 engine thats not the case. Hence, invocation of goals such as filedeps causes an exception.

  • Use address_mapper to correctly resolve an Address to a BuildFileAddress

$ ./build-support/bin/ci.sh succeeded locally

travis ci running - https://travis-ci.org/pantsbuild/pants/builds/149976208

  • 0
  • 0
  • 0
  • 1
  • 1
Description From Last Updated
NH
  1. 
      
  2. src/python/pants/engine/graph.py (Diff revision 1)
     
     

    I think this will cause the BUILD file to be parsed a second time because the address mapper doesn't use the engine's cache.

    Had you considered splitting the task that parses into address families?

    With that, I think we could have a task that converts a single address into a BuildFileAddress by selecting the associated AddressMap and pulling the build file off of it.

    Alternatively we could not erase the BUILD file path when constructing AddressFamilys, which would also make constructing BuildFileAddresses easier. https://github.com/pantsbuild/pants/blob/master/src/python/pants/engine/mapper.py#L122-L125

    1. Turns out this does not re-parse all BUILD files just the top level -- once with old engine and once with new. Looked at timing stats and its comparable to other goals in v2 engine. Since Nick has redesigned the AddressMapper in https://rbcommons.com/s/twitter/r/4114/ will send a follow up review to accommodate for that. As it is, this fixes filedeps and some other internal goals run with v2 engine

      ```time ./pants --enable-v2-engine filedeps abc::
      real 0m15.156s
      user 0m8.822s
      sys 0m0.884s

      time ./pants filedeps abc::
      real 0m7.600s
      user 0m1.868s
      sys 0m0.493s

      time ./pants list abc::
      real 0m5.263s
      user 0m0.142s
      sys 0m0.124s

      time ./pants --enable-v2-engine list abc::
      real 0m15.100s
      user 0m8.543s
      sys 0m0.803s```

  3. 
      
JS
  1. I'm going to bow out of this review in favor of the Tweeps who all have their head in the v2 engine right now.
  2. 
      
NH
  1. Ship It!
  2. 
      
YU
  1. Ship It!
  2. 
      
KW
  1. Ship It!
  2. 
      
Loading...