Add support for incremental annotation processing in Gradle

[wrotte]

My understanding is that currently, projects that include annotation processors are unable to be incrementally compiled in Gradle; such projects always trigger a full rebuild. The forthcoming Gradle version 4.7 will contain improvements that will allow for incremental annotation processing.

https://docs.gradle.org/nightly/userguide/java_plugin.html#sec:incremental_annotation_processing

Presuming Dagger fits into one of the two categories of annotation processor described in the above link, adding the required meta-data to the META-INF directory would likely significantly improve build times.

[tbroyer]

I'm actually 99.9% certain Dagger does not fit. @stephanenicolas?

[TWiStErRob]

Even if not directly possible, would it be possible to split the processor into two parts? e.g. one that handles @Inject on constructors (to generate _Factory and other trivial generation, incremental) and another that handles the complex graph building (full regeneration). I might not have deep enough insight, but this should help a lot, if people have modules with constructor injection, and assemble the Dagger graph at top level (:app).

[stephanenicolas]

This is what we plan to do for toothpick, an alternative DI lib. We already have 2 different APs. To support Incap, we will have an AP that is incremental and another one that is not, but that can actually be optional. So we plan to only use it for release and not debug. Hence getting a fully incremental DI lib. Le mer. 18 avr. 2018 07 h 32, Róbert Papp (TWiStErRob) < notifications@github.com> a écrit :

…

Even if not directly possible, would it be possible to split the processor into two parts? e.g. one that handles @Inject on constructors (to generate _Factory, incremental) and another that handles the complex graph building (full regeneration). I might not have deep enough insight, but this should help a lot, if people have modules with constructor injection, and assemble the Dagger graph at top level (:app). — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1120 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABv33dS5c_G6esV1QEck_hqXvdCv2wA9ks5tp06OgaJpZM4S_Nif> .

[oehme]

Why would Dagger not fit, can you please elaborate on that?

[ronshapiro]

@TWiStErRob it would likely be very difficult to split the processors since they the different steps require ordering that would be hard to ensure if they're split.

@stephanenicolas Can I suggest that we add a flag or some other way to test incap without requiring every project in the wild to release a new artifact? I don't think this scales well, and it forces the effort on project maintainers. It would be great if someone could file a bug saying "Hey, I've been using Dagger with Incap and it works great, can you add default support so I don't need to keep this ugly flag in my repo?"

I don't love the idea of releasing an artifact with incap support without knowing that it works.

[oehme]

Can I suggest that we add a flag or some other way to test incap without requiring every project in the wild to release a new artifact? I don't think this scales well, and it forces the effort on project maintainers.

The effort has to be on the project maintainers, because you really have to think about what your processor does. You can't turn a switch in Gradle and say "Let's just pretend this is incremental". That would lead to broken builds and lost trust with users. Afaik @hungvietnguyen is already planning to work on Dagger after he's done with DataBinding.

You shouldn't rely on someone trying it out and saying "it works". There should be automated functional tests ensuring that it works. Functional testing can be done with the Gradle Tooling API.

I don't yet see why anything would have to be split. I'd like to understand why Dagger wouldn't fit the isolating or aggregating category.

[TWiStErRob]

different steps require ordering that would be hard to ensure if they're split.

Do you need the Blah_Factory class to be generated before you start using it? If there's an @Inject on a ctor that class will have a _Factory, right? Couldn't you just assume it'll be there next to Blah by the time the real compilation gets to it? You control both processors, so you can assume that it'll do these steps. I'm sure there's more to it than I see currently, just throwing this out there.

[stephanenicolas]

I agree with @oehme that testing is the way to go more than letting users experiments. The amount of complexity to rely on users. And only maintainers can have a good understanding of their AP to declare their behaviour to gradle.

I don't think a release for a flag is so much of an issue, but I believe there is much more fine grained understanding of incap necessary to flag the AP properly.

Though, about testing, I don't like so much the idea of integration tests, because they can slow down running tests and also use a non standard testing framework. A better solution would be to modify the google testing lib that most APs use for unit tessting to check that the right elements are passed to the filer when creating files. This seems to be doable and would provide very little change to existing tests to make sure incremental behaviors are working fine.

I am not too sure about the current status of this issue, but it could be a key to faster tests of incremental annotation processing: google/compile-testing#58

[oehme]

Not sure what you mean by "non-standard" testing framework, you can use JUnit just like you would for any other test and call Gradle through the Tooling API. End-to-end tests are key to ensure that this actually works. Unit tests are of course better to test all kinds of corner cases. But if I had to choose a place to start, end-to-end always get my recommendation.

[ronshapiro]

@oehme @stephanenicolas that's fine and your opinion is totally valid, but I think it's unlikely for project authors to write integration tests for a particular build system's compilation strategy. I don't think we'd ever trust a single voice saying that it's working, but if a larger group gives it a try and can all verify, that's a strong signal. There are many build systems available, and taking away the effort from project maintainers can help features take hold.

[oehme]

We can't take that effort away from the maintainer, they actually have to know what the processor is doing and ensure that complies with the constraints mentioned in the incremental annotation processing documentation. We put in a lot of work to make it as transparent as possible (i.e. no new APIs), but there are still lots of things a badly behaved processor could do to break it. Hence the opt in which only someone with deep knowledge like the maintainer can do.

[oehme]

This is the highest voted issue on both our and your issue tracker. For many Android users this is the only thing standing between them and fast incremental builds. How can I help move this forward?

[ronshapiro]

I have reread the docs over and over... and I still don't understand what would make an annotation processor aggregating. All I know is what it can't do.

What happens in this case?

class Foo implements Bar {
  @Inject Foo() {}
}

interface Bar extends RandomAccess {}

@Module
abstract class FooModule {
  @Binds abstract RandomAccess from(Foo foo);
}

@Component(modules = FooModule.class)
interface TestComponent {
  RandomAccess r();
}

If Bar is changed to no longer implement RandomAccess, does that cause every other file to be recompiled? Or no because RandomAccess has no methods and/or Bar has no annotations that Dagger declares?