Split out a JvmCompileStrategy interface

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

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.

  • 0
  • 0
  • 4
  • 0
  • 4
Description From Last Updated
  2. no longer true: stale comment.

  1. I have a few potential comments, but I will actually hold off on them until this takes its final shape. So far this is so much better than the previous source-file-of-death, that I'd rather rejoice in the fact there there are now abstractions, instead of niggling over their exact shape.

  1. I'll be out climbing again tomorrow and Benjy's shipits on this series of changes is most important so I'll bow out.
Review request changed

Status: Closed (submitted)

  1. Very nice

  2. Good idea

  3. 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!

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

    Ditto .format/str

  1. 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.

  2. 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!

  2. 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/