Buck: Enabling proper Error Prone integration with Buck

[msridhar]

We wanted to have a discussion on the best way to "upstream" our technique for using using Error Prone with Buck, which would primarily consist of adding documentation.

For the most part, our integration uses the available javac_jar and compiler_class_name options, where the javac jar is the Error Prone Ant artifact. Error Prone is packaged with a version of the JDK 9 javac compiler, whereas we currently build on JDK 8. This makes integration slightly tricky, as the JDK 9 javac includes updated versions of certain javax.tools and javax.lang.model.type classes as compared to what is in the JDK 8 rt.jar. To work around this issue, we have extracted the relevant classes from the EP javac jar into a small new jar tiny_tools.jar that we place in Buck's bootclasspath via .buckjavaargs. (Placing the full EP Ant artifact in the bootclasspath is risky; see google/error-prone#934.) With this change, everything seems to work well, including plugin checks. One downside is that the EP Ant artifact exposes some third-party libraries like Guava to annotation processors; we opened google/error-prone#950 to relocate the classes most likely to cause conflicts.

An alternative integration point would be to use Error Prone as a Javac plugin, which is supported by Error Prone (see the docs). However, this integration requires Java 9, and we are still building on JDK 8; we have not yet tried it in the context of Buck builds.

In terms of upstreaming, one question is where the tiny_tools.jar file should live. We could create a separate Maven artifact for it, or add it to the Buck repo. It was created via a fairly manual process, and it could requiring updating when Error Prone uses a new javac version. We could also add some scripting around this to auto-generate the jar.

Other feedback / comments welcome. Thanks!

[kageiit]

cc @ttsugriy @styurin @jkeljo

[ttsugriy]

@msridhar, having error prone support would be awesome! The jar would have to live in Buck repo, but only if it can be generated in a fully automated fashion and accompanied with all relevant licensing information. In fact that's what other build systems that provide error-prone seem to be doing anyways. For example, Bazel bundles jdk9 as well -
https://source.bazel.build/bazel/+/master:third_party/java/jdk/langtools/ Please give it a try and if it works we could just steal their jar instead of maintaining our own :)

[msridhar]

Hi @ttsugriy so to clarify, you would need all jars relevant to Error Prone (including Error Prone itself) inside the buck repo?

I am going to follow up with EP folks regarding their Bazel integration. There are some aspects of it I don't understand. E.g., if they are sticking the entire javac jar in the bootclasspath, that can lead to weirdness in terms of getting the parent classloader relationship right for annotation processors. Will report back here once I have some answers.

[ttsugriy]

@msridhar, we already have an ant uberjar in buck repo that I'm trying to properly integrate with our ant build. You can find it here - https://github.com/facebook/buck/tree/master/third-party/java/errorprone It needs some cleanup, but the basic idea is the same - all error-prone jars and its dependencies should ultimately be checked in into buck repo.

[ttsugriy]

@msridhar, the class loader concerns you may be valid, which is why for now I've been mostly considering adding as a separate CI ant target where with ECJ compiler replaced by ErrorProne. This way we still make sure that all builds work with standard compilers, but also get useful static analysis suggestions.

[msridhar]

Hi @ttsugriy, an update here. I got some info on the Error Prone Bazel integration: mailing list message

It looks like they have a bootstrap_deploy.jar shipped with Bazel that they stick in the bootclasspath. I think for Buck integration doing the same thing would make sense.

As far as their handling of hiding EP-bundled classes from annotation processors, that requires some classloader trickery and I'm not too sure how to integrate this into Buck. AFAIK, with Bazel, Error Prone is the only version of javac that is supported. With Buck this will not be the case, and so I don't know if adding the classloader magic would make sense. We could just make it a documented issue with EP integration that annotation processors may have to watch out for conflicts with EP-bundled libraries. (FWIW, this has not really been much of an issue for us at Uber.)

To summarize, I think we could do the following:

1. Figure out the way to most easily package up Bazel's bootstrap_deploy.jar for use with Buck.
2. Document how to use Error Prone with Buck

Let me know your thoughts. I'll test out 1 on our internal builds

[msridhar]

Some further clarity on classloader stuff. My understanding of bootstrap_deploy.jar was way off in the above comment. What actually happens is:

1. Bazel places the full javac jar in the bootclasspath here
2. Separate code manages the bootclasspath used when running javac, through a mix of passing the -bootclasspath parameter and calling the file manager APIs, e.g., here and here. This makes sense, as for example if you're targeting 1.6, you want the bootclasspath seen by the compiler to match a 1.6 JVM.

Anyway, wanted to clarify that. Still thinking about the right integration for Buck; will post back when I have something.

[msridhar]

FYI, we have documented how we got Error Prone to work with Buck builds on JDK 8 at Uber:

https://github.com/uber/okbuck/wiki/Using-Error-Prone-with-Buck-and-OkBuck

Feedback welcome and I'm happy to help with upstreaming docs or scripts if desired