Support for javac plugins.

Review Request #3839 — Created May 6, 2016 and submitted

benjyw
pants
pants-reviews
nhoward_tw, stuhood, zundel

- Introduces a JavacPlugin target.
- Logic to compile plugins and generate their metadata file.
- Logic to compile with in-repo (or external, published) plugins
enabled. Allows two modes:
+ Targets can explicitly depend on a JavacPlugin, and that
plugin will be run when those targets are compiled. The plugin
will itself be compiled first if needed, naturally, so there's
no need to publish it first.
+ "Global" plugins can be specified via a JavacPluginSetup
subsystem. Any plugins configured there will be injected as deps
on every jvm target, and therefore compiled and used in all javac
compilations (except those of the plugin itself).
This allows you to apply "global" plugins (e.g., a linter)
without having to add dependencies everyhwere.
- Ensures that tools.jar is on the classpath when compiling
plugins.
- Provides a hack to allow tests to pass without needing to set up
the JavacPluginSetup subsystem, since we have hundreds of tests
that use JvmTasks in various ways.
- Removes the truly gratuitous jvm backend dependency from
test_wrapped_globs.py.

Note that the plugin API we support was introduced in Java 8, so
we have to temporarily exclude the example plugin from being compiled
in our tests, until we're entirely on Java 8 in our own CIs.

CI passes: http://jenkins.pantsbuild.org/job/pantsbuild/job/pants/branch/PR-3316/3/

BE
ZU
  1. What kinds of things would you do with this plugin? One thing that we've wanted is the ability to write a code generator in Java and have it plug into pants. Would this unlock that usecase?

    1. Can you clarify what you mean by "write a code generator in Java"? We already run code generators written in Java, no? E.g., ANTLR. They're just JVM tools. If you want to build the code generator and then run it directly from the repo, without a publishing step, then no, this won't unlock that.

      A Javac plugin gives you access to the AST, and other compiler internals, at various stages of compilation. So I think you can affect javac output, and you can generate other output files, based on the .java file as input, that a downstream task can consume.

    2. What I meant by writing a code geneator in Java is exposing a Java API to pants plugin developers. Right now we have to mediate everything through a command line interface.

    3. I take it this is to support this kind of plugin: https://docs.oracle.com/javase/8/docs/jdk/api/javac/tree/com/sun/source/util/Plugin.html

    4. Yes, exactly.

      A Java codegen API would be great, but is orthogonal to this.

  2. 
      
ST
  1. Thanks Benjy.

  2. Worth renaming this to javac_annotation_processor, and deprecating the old name?

    1. Possibly, but in a separate change? This one is already bogged down enough... :)

  3. src/python/pants/backend/jvm/subsystems/javac_plugin_setup.py (Diff revision 1)
     
     
     
     
     
     
     
     

    I feel that the options namespace is already pretty overloaded ("jvm-platform", "jvm-distribution", "java", "javac-plugin-setup") and would love to see some consolidtion into useful groups.

    Can these options live on the 'Java' subsystem instead?

    https://github.com/pantsbuild/pants/blob/master/src/python/pants/backend/jvm/subsystems/java.py

    1. I hear you regarding the namespace confusion. I'm of two minds on this. The docstring for the Java subsystem says "A subsystem to encapsulate Java language features." Which plugins to use when compiling is not a language feature, it's a build flag, essentially.

      However, Java's existing options (via the ZincLanguageMixin) are also build flags: --strict-deps and --fatal-warnings. Arguably, the javac plugin setup is "the same kind of thing" as those flags.

      So if we do this we need to change that docstring... The rest of it is pretty confusing too: "Platform specific options (ie, JDK specific) are captured by the JvmPlatform subsystem." I have no idea what that means, and looking at JvmPlatform, I figure its options could also move here. What do you think?

  4. IIRC, target_option already causes these to be parsed as addresses? Should be able to drop the re-parse below.

    1. There's a TODO on target_option to do that: https://github.com/pantsbuild/pants/blob/master/src/python/pants/option/custom_types.py#L41

      Right now it just returns a spec.

  5. This comment should link to a github issue probably.

    (It could maybe be fixed by transitively loading all subsystems required by the Targets in BuildFileAliases in TaskTestBase?)

    1. Good idea: https://github.com/pantsbuild/pants/issues/3409

  6. Is this used somewhere in plugin loading? Would be good to add a bit more info on why/how it is used.

    1. Added a sentence on this. These are the names passed to javac's -Xplugin flag.

  7. Can this be executed generically inside compile for all compiler_plugin_types?

    1. We can't do it inside compile because compile has no access to vts._targets. As for it being generic, each plugin type is enabled differently, and therefore would be excluded differently, so not really. We could model plugins better, but that would be a separate refactoring change.
  8. Is it worth extracting these three types (repeated in compiler_plugin_types) into implementations of a simple CompilerPlugin interface?

    1. Yes, see above. We could do some refactoring here. But I don't think this change is the right place for that. It's already heavyweight...

  9. src/python/pants/subsystem/subsystem.py (Diff revision 1)
     
     
     
     

    Is the intent of this something more like def is_initialized?

    1. That's a better name. Changed.

  10. Since boolean, maybe is_missing_jvm

    1. That was the prior name, but done.

  11. 
      
BE
BE
ST
  1. Ship It!
  2. 
      
BE
BE
Review request changed

Status: Closed (submitted)

Change Summary:

ef3c4f54d0a80e78f958bf5905313a41df6f196d

BE
  1. Submitted. Thanks Stu!

  2. 
      
Loading...