A ScalaPlatform subsystem.

Review Request #2176 — Created May 7, 2015 and submitted

benjyw
pants
1497
273ea58...
pants-reviews
jsirois, zundel

This replaces scala/TargetPlatform, and its config dependencies.
It allows us to remove several config hacks in tests.

This involved some refactoring of ZincUtils, notably:
- Moving the code that turns jvm tool specs into targets
(for fingerprinting/invalidation) into JvmToolTaskBase.
There's nothing zinc-specific about that logic anyway.
- Moving scalac plugin-related logic into ScalaCompile.

We've been meaning to slim down (and maybe kill) ZincUtils,
so this was a good thing to do anyway.

Also adds support for subsystems in group tasks, which was overlooked
previously.

Tests pass locally. CI passes: https://travis-ci.org/pantsbuild/pants/builds/61810735

ST
  1. Thanks!

  2. Would it delineate these methods more clearly to have them in an inner Plugins class?

    1. Possibly, although there's no real benefit to it being nested (<pedantry>Python has nested classes, not inner classes in the Java sense - an instance of the nested class is not bound to an instance of the outer class.</pedantry>). However I'd like to punt on this to a future change where we move all scalac-specific stuff (plugins, but also the -S options) to ScalaZincCompile (and add -C options to JavaZincCompile?)

  3. This could bear more explanation.

    1. I've peppered a few of these around in tests too with similar comments. What's going on here is that the subsystem options aren't registered in the options namespace until the call to self.context(). If your test never calls this method, you won't get some of the options set that you need and you'll get 'no such attribute' error on a call in your task to get_options().foo when foo is supposed to be already registered.

    2. What Eric said... I modified the comment to be a little more explanatory.

  4. tests/python/pants_test/base_test.py (Diff revision 1)
     
     

    ws

  5. 
      
ZU
  1. 
      
  2. Neat idea. Maybe we would allow an alternative format that we can distinguish easily from a spec, like a string surrounded in parens:

    (<org>:<name>:<rev>)
    
    1. That's a good idea. One of my main goals in all of this work in the past few months (options system, subsystems, removing direct config references) has been to get us to a point where pants works out of the box, with no pants.ini or BUILD.tools twiddling. So this is one way to achieve that. Another, possibly better, way is to embed a standard BUILD.tools in the pants distro and use that if the repo has no explicit one.

  3. Are these methods just moved from zinc_utils.py or do they deserve a closer look?

    1. They're just moved.

  4. 
      
BE
BE
ZU
  1. Ship It!
  2. 
      
BE
BE
BE
Review request changed

Status: Closed (submitted)

Change Summary:

Submitted as 77be0ba1fdd58e01dc08e1832fd3768db878cced.

BE
  1. Thanks all. Submitted as 77be0ba1fdd58e01dc08e1832fd3768db878cced.

  2. 
      
Loading...