Add support for Incremental compilation

[stephanenicolas]

Solves issue #126.

The idea is that we only needed to preserve the same file manager, and make it able to retrieve sources and classes that were already compiled.

There are 2 other things we could do starting from this PR, but I would like to get some feedback first:

1. locally we have used a custom ClassLoader that is able to load classes that were compiled using the lib. We thnk this would be a useful addition to compile testing, but it was rejected in issue: Compile generated code and execute it #70. Though the code is really small... Just a class loader that uses bytes we already have..
2. if this PR is accepted, you might want to introduce or modify source adapters to get a whole Truth chain using the new IncrementalCompileTester. Let us know if we can help, it looks like an interesting thing to code.

Please note that this PR is a by-product of the research we have been doing on incremental annotation processing in the Incap project.

[tbroyer]

Why use source path? Nobody should ever use source path in any "smart" build tool (and Gradle, for instance but not only, uses an empty source path on purpose; see https://blog.ltgt.net/most-build-tools-misuse-javac/ for the rationale).

Incremental compilation is "as easy as" putting the output location in the classpath (so the compiled classes from earlier compilation tasks are available) and passing fewer source files to compile. If you use the source path and don't include previously-compiled classes to the classpath, what you're doing is "implicit compilation" of the "missing" classes, which is entirely different, and something almost nobody does nowadays (possibly Maven under some circumstances, but I'd argue that this is either a broken build configuration or a Maven bug anyway).

I don't have time to dig into how to do that with compile-testing, and/or whether it's possible already, but IMO your approach in this PR is wrong.

[stephanenicolas]

You can use either source path or classpath, it's a normal option that is supported by javac.
See the -Xprefer option in javac documentation. But you might be totally right about implicit compilation not being a common thing, I got no idea about that.

But I am really not sure of the real impact of this as actually even if the compilation is implicitly being done again, as we are talking of source/classes that didn't change.

If you want we can limit this PR to search in the classpath.

The PR, again, actually supports both implicit compilation or using classes that were compiled in previous builds. Let us know if you want to change this.

Do you mind to turn your next comment into something positive that you ask us to do to get this merged ? And if you could answer the 2 open questions in the original PR description, it would be nice too.

[stephanenicolas]

@tbroyer Did you see my comment above ?

[stephanenicolas]

@tbroyer is that closer to what you asked for ? The new commit only uses classpath to provide the previously generated classes. It's more a proof of concept, the PR would need some cleanup.

[tbroyer]

If you want my opinion (I'm not a project member, so I have no power on whether this PR will or won't be merged):

I, for one, don't like incremental compilation to begin with, so I'll refrain from commenting further on whether this PR (and what you want to achieve) is a good idea or not (at the risk of not sounding much more positive this time again 😉).
Incremental compilation is more than tricky to get right (at the build system level), particularly when used in combination with annotation processors (IIRC, Gradle disables incremental compilation when it detects annotation processors; IDEs might possibly manage things better, though I personally tend to delegate to the build system when possible for correctness/reproducibility)

BTW, I think you can achieve the same API using withClasspathFrom and copying compiled classes (from generatedFile) to a temporary directory and a URLClassLoader, with no need to touch the InMemoryJavaFileManager; which means this could be added around compile-testing, and not necessarily within compile-testing (while copying to a temporary folder is not ideal, it should probably not be a showstopper either; make the IncrementalCompilerTester a @Rule to properly clean the temporary folder, possibly delegating to org.junit.rules.TemporaryFolder). An alternative could be to use a custom JavaCompiler whose getStandardFileManager would implement the changes you're proposing here.
This could possibly make a good companion library for people interested in incremental compilation with annotation processors (and with enough traction could eventually be merged upstream).

Another approach of course, though not applicable to all use-cases, is to have the classes as part of your code (src/test/java) so you don't have to use the outputs from a previous "full compilation" to actually test an "incremental compilation", so you can use withClasspathFrom the current ClassLoader (that might actually already be the default behavior) and only change the list of sources passed to the compile() method.

[stephanenicolas]

@tbroyer it seems to me that using the file folder is not much better than using the InMemoryJavaFileManager. And if we would apply it for incremental compilation, we could also do it for any other thing in compile testing. So, I think this approach is mainly a rewrite of the lib to use temp folders.

Also, I tried using compiled classes for incremental testing and using the withClasspathFrom method but it turns out to be more complicated as you want your tests to look similar for full builds and incremental builds and in this case, to avoid conflicts on full builds with the pre-compiled classes, you have to write new tests or load classes from somewhere else. It's not easier to say the least.

And regarding incremental annotation processing, we are actually working on that and the PR here comes from this work.

[hohserg1]

Any news?