[engine] Fix filedeps goal in v2 engine

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

Information
ity
pants
3752, 3753
Reviewers
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
nhoward_tw 
  1. 
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
      src/python/pants/engine/graph.py
      


       (Diff revision 1)


      
     
    
    
       
   
    
     
    

    
     
    

       
  
    
 
    

   
    
    
    



    
 
    
  
    
   
   
    
    
     
     
     
     
     
    
    Show all issues
   
  
    
 
    

    



    
    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```
      
 
      

      2. 


 
    


    

 
    

    3. 




  3. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 




  

 
















 





 

  

   





 JS





   

   





  

 



 

  


   

   

    
 
jsirois


    
   


  

  

  



    




  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. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 





 

  

   





 NH





   

   







  

 



 

  


   

   

    
 
nhoward_tw


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Ship It!
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 





 

  

   





 YU





   

   







  

 



 

  


   

   

    
 
yujiec


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Ship It!
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 





 

  

   





 KW





   

   







  

 



 

  


   

   

    
 
kwlzn


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Ship It!
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 









     

    

   



  

  

   
   Loading...