Repository navigation
Add support for incremental annotation processing in Gradle #1120
Description
Activity
I'm actually 99.9% certain Dagger does not fit. @stephanenicolas?
Reacted by Dawid Hyży, Alan Evans and Matthew PageEven if not directly possible, would it be possible to split the processor into two parts? e.g. one that handles
@Injecton constructors (to generate_Factoryand 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).Reacted by Ben Sandee, Maciej Czekański, Dawid Hyży, Artem Hluhovskyi, Andrew Munn, David Medenjak, Chaitanya Pramod, Curtis Kroetsch, burningfireplace, Stojan Anastasov and 1 more- 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> .
Why would Dagger not fit, can you please elaborate on that?
@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.
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.
Reacted by Andrey Mischenko, TechYourChance and Andrew Reitzdifferent steps require ordering that would be hard to ensure if they're split.
Do you need the
Blah_Factoryclass to be generated before you start using it? If there's an@Injecton a ctor that class will have a_Factory, right? Couldn't you just assume it'll be there next toBlahby 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.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
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.
Reacted by Said Tahsin Dane, Andrew Reitz and Arunkumar@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.
Reacted by Stanislav DimitrovWe 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.
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?
Reacted by Stojan Anastasov, Milos Marinkovic and Andrew ReitzI 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
Baris changed to no longer implementRandomAccess, does that cause every other file to be recompiled? Or no becauseRandomAccesshas no methods and/orBarhas no annotations that Dagger declares?Or, what about this:
@Retention(SOURCE) @interface MaybeScoped {} @MaybeScoped class Blue { @Inject Blue {} } @Component @MaybeScoped interface BlueComponent { Blue blue(); }
If I add the
@javax.inject.Scopeannotation to@MaybeScoped, do the other files get recompiled?JSR 330 states that all scopes must be
RUNTIMEretention, but that's not enforced by dagger (we operate on whatever we can see). We don't declarejavax.inject.Scopeas an annotation that we process over because it's a meta-annotation.Would this functionality still be considered in "aggregating"?
I wonder if these question can be answered by setting up a Gradle functional test suite?
12 remaining items
It should be fine to read the options before you say which you support. The latter is only there so javac can warn when an option was not used by anyone.
Apart from that the PR looks 👍
- added 4 commits that reference this issue
on Sep 12, 2018 @ronshapiro just wondering when we might expect a release with this feature?
Reacted by Juan Diana, Ivan Dyatlov, Łukasz Wasylkowski and Eugene KrivobokovThanks for the reminder. Just released 2.18.
Reacted by Łukasz Wasylkowski, Giovanni Longatto Nazario Marques, Tony Robalik, Rooz Mohazzabi, Paul Merlin, Juan Diana, Yenchi Lin, Donát Csikós, Artur Artikov, Jeremy Tecson and 13 moreIs this the correct way to pass this to Dagger from Gradle?:
android { defaultConfig { javaCompileOptions { annotationProcessorOptions { arguments ["-Adagger.gradle.incremental", "-Adagger.formatGeneratedSource=disabled"] } } }@wbervoets to support both Kotlin and Java I have it configured (with other options) as:
// Enables Dagger fastInit mode, which reduces component building latency by loading less classes, // see: https://google.github.io/dagger/compiler-options.html#fastinit-mode def optionFastInitEnabled = '-Adagger.fastInit=enabled' // Disables Dagger code formatting to speed builds, // see: https://google.github.io/dagger/compiler-options.html#turning-off-code-formatting def optionFormattingDisabled = '-Adagger.formatGeneratedSource=disabled' // Enabled incremental annotation processing support, since Kotlin 1.3.30 // See: https://kotlinlang.org/docs/reference/kapt.html#incremental-annotation-processing-since-1330 def argumentIncremental = 'dagger.gradle.incremental' subprojects { pluginManager.withPlugin('kotlin-kapt') { kapt { javacOptions { option(optionFastInitEnabled) option(optionFormattingDisabled) } arguments { arg(argumentIncremental, 'true') } } } afterEvaluate { tasks.withType(JavaCompile.class) { options.compilerArgs << optionFastInitEnabled << optionFormattingDisabled } if (pluginManager.hasPlugin('com.android.library') || pluginManager.hasPlugin('com.android.application')) { android { defaultConfig { javaCompileOptions { annotationProcessorOptions { arguments[argumentIncremental] = 'true' } } } } } } }
Reacted by Tran Huu Tin, Miłosz Lewandowski, Pavel Shchahelski, CW and dijgas@eleventigers Is the aforementioned code block necessary for dagger version, 2.23.1? Currently, when scanning for dependencies that do not support incremental annotation processing without the code block, I do not get any warnings.
Reacted by cfl777, Luis Cortes, Luca Spinazzola and Yaroslav Berezanskyi@eleventigers Is the aforementioned code block necessary for dagger version, 2.23.1? Currently, when scanning for dependencies that do not support incremental annotation processing without the code block, I do not get any warnings.
It's enabled by default since Dagger 2.24.
Reacted by Csaba Kozák
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.