Relativize classpath for non-ng java execution

Review Request #804 — Created July 30, 2014 and submitted

ddavydov
pants
pants-reviews
jsirois, stuhood, tejal
-Factored the classpath relativization logic into it's own module and reworked it slightly.
-Made jmake relativize classpath in addition to the nailgun executor

The jvm_utils BUILD file has an empty dependencies key to follow the convention I found in another file.
Ran a java/scala test target with nailgun enabled/disabled and made sure the classpath was minimized and the tests passed.
  • 1
  • 0
  • 1
  • 1
  • 3
Description From Last Updated
I think you wanted to add this to JvmCompile TE tejal
ST
  1. 
      
  2. src/python/pants/backend/jvm/jvm_utils.py (Diff revision 1)
     
     
     
    This will work for any fileset, not just classpaths, right? Might want to find a location more relevant for "path/file" manipulation.
    
    Also, the inner method should probably move out and become public (if it doesn't already exist in whichever other file you find that is doing file/dir ops)
    1. Good point, I will make these changes.
  3. Does this need to happen for scala compiles as well?
    1. No because scala_compile uses zinc tools which always use the nailgun executor (the nailgun executor "optionally" runs nailgun). I should have put this in the RB description, sorry.
  4. how efficient is `+` on a large list? might consider templating these in rather than concatenating before the join
    1. The maximum classpath size isn't that large, so I doubt this would be a concern. I'm not sure what you mean by templating these in.
  5. 
      
TE
  1. 
      
  2. I think you wanted to add this to JvmCompile
  3. 
      
DD
ST
  1. Ship It!
  2. 
      
TE
  1. 
      
  2. You added the dependency jvm_utils to 
    'src/python/pants/backend/jvm/tasks/jvm_compile:jvm_compile' target
    
    But added relative_utils functionality in src/python/pants/backend/jvm/tasks/jvm_compile/compile.py. 
    
    That led me to thinking you meant to add this to JvmCompile => src/python/pants/backend/jvm/tasks/jvm_compile/java_compile.py.
    
    However, after read your comments to Stu's remark, add this dependency to 
    'src/python/pants/backend/jvm/tasks/jvm_compile:java'
    1.     pants('src/python/pants/util:dirutil'),
      already exists in jvm_compile:java
    2. Ok took this offline. 
      
      In R2. "pants('src/python/pants/backend/jvm:jvm_utils')" is removed.
  3. 
      
TE
  1. Ship It!
  2. 
      
JS
  1. Please mark this review as submitted - look like it was here: https://github.com/pantsbuild/pants/commit/4a5ec2d54bd1f03df5210175658fec58742553fb
  2. 
      
DD
Review request changed

Status: Closed (submitted)

Loading...