Split out a JvmCompileStrategy interface

Review Request #1885 - Created March 8, 2015 and submitted

Information
Stu Hood
pants
7e6391c...
Reviewers
pants-reviews
benjyw, jsirois, nhoward_tw, patricklaw

Second step in the implementation of https://docs.google.com/a/twitter.com/document/d/1E-eysqdKUz7IvL5snRNe7orW24A9AzcApt9q50UpwAc/edit?pli=1#

  • Split JvmCompileStrategy out of JvmCompile and give it all references to:
    -- the global classpath
    -- the global analysis
    -- "partitioning" within chunks
  • Add the existing JvmCompileStrategy as JvmCompileGlobalStrategy.
  • Propagate a classes dir into analysis parsers on a case-by-case basis.
  • Introduce compile_context, and begin passing it around in place of targets.
  • Use a dedicated directory for the global PROCESSOR_INFO_FILE in JavaCompile.
  • Assert that all classpath entries were contained within the working copy, to avoid the need to relativize to the ivy cache dir.

This strategy class isolates all behavior related to partitioning and the global classpath, and begins to pass targets with a CompileContext that indicates where their analysis and classes live.

I'm going to begin working on the isolated strategy, and a unit test harness to allow for running tests with both strategies.

Fixed the last failure: was related to annotation processors. Might have a proposal for improving that situation; will post it after the next review.
https://github.com/pantsbuild/pants/pull/1220

Issues

  • 0
  • 4
  • 0
  • 4
Description From Last Updated
Stu Hood
Stu Hood
Benjy Weinberger
John Sirois
Stu Hood
Stu Hood
Review request changed

Status: Closed (submitted)

Patrick Lawson

Very nice

Good idea

whitespace

src/python/pants/goal/products.py (Diff revision 2)
 
 

.format, and str is almost certainly not what you want (different semantics in py2 vs py2).

  1. err, py2 vs py3. Also, I didn't notice that you'd already submitted when I was doing this review--oh well. The suggestions were all trivial.

  2. Will get them in the next review. Thanks!

src/python/pants/goal/products.py (Diff revision 2)
 
 

Ditto .format/str

Mateo Rodriguez

This breaks Android support. Looks like Pants no longer allows classpath entries outside the buildroot. Android's compile phase adds the android.jar to the classpath, that file is located in the SDK on disk. So my guess is that I need to update Android support to be in-line with this change as opposed to the other way around, correct?

The exception you want to raise is not working, see below.

  1. Ah, yea... you'll want to symlink those artifacts into the working copy in a task-specific directory.

You raise TaskError in several places in this file but forgot to import it so that errors are raising

File "/Users/mateor/development/pants/src/python/pants/backend/jvm/tasks/jvm_compile/jvm_compile_strategy.py", line 159, in _validate_classpath
    raise TaskError('Classpath entry {f} is located outside the buildroot.'.format(f=f))

Exception message: global name 'TaskError' is not defined
  1. Is the same thing happening with defaultdict and OrderedSet()?

  2. Mmm. Will fix in https://rbcommons.com/s/twitter/r/1898/

  3. Actually, opened a smaller followup to fix those: https://rbcommons.com/s/twitter/r/1900/ Sorry!

John Sirois

   
This was broken with the yank of self._resources_dir from the JvmCompile base-class.  Discovered by upgrading twitter/commons which has a scalac_plugin target to 0.0.30.  I'll have a fix up for this in a bit.
  1. https://rbcommons.com/s/twitter/r/1945/
Loading...