-
Notifications
You must be signed in to change notification settings - Fork 278
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support Gradle Incremental Processing #804
Comments
@elucash Hi, could you please just give some pointers how to move this forward? If it is in line with your plans, of course. I have put together some resources to understand what the issue is about: |
Thank you for the feature request! I think it would be really great to support this. I would definitely like such a feature if this would not require a lot of work. Looking at examples I barely understand what exactly has to be done here (with respect to annotation processing code). Is there a way to externally tell / configure Annotation processor to be run in one of these sandbox/isolation modes and see if processor works before actually augmenting annotation processor's metadata?
As a side note. I think it is unfair for Gradle to promote such a feature, so they try to artificially lock-in into their tool. The problem with incremental compilation is that no one trust it, so people are doing clean builds with Maven, Ant, Gradle, thus defeating the purpose of incremental compilation. Tools like Bazel (or Buck etc) have a proven approach to reproducible and reliable builds by using isolated modules (as opposing to separate custom compilation-unit/file change tracking inside module as Gradle apparently tries to do). Also, Gradle, by creating yet another wrapping around Java Lang Model elements make annotation processor authors life more complicated as we already try to "fix" or workaround compiler bugs by inspecting those elements, so now, with Gradle wrappers added to the mix, the things might get complicated i.e. certain features which benefit from fixes(workarounds) to compiler bugs will now fail again, because of Gradle's "efforts to improve". |
Thank you for the detailed response. I'm not really qualified to answer your question as I'm only a passerby who happens to use Immutables (love it!) and Gradle (it works for us, so 👍 too) I think that your question can be answered by @oehme (https://github.com/oehme). Hopefully, he'll notice the mention. :-) |
It's all explained here. There is nothing more to it. You need to know which category your processor falls into and declare that category in a single-line file. Since you say your processor works with JDT, it must be in the "isolating" category, since that is the only kind of processor that JDT handles correctly.
You are assuming we are doing something invasive, which we don't. The only thing we decorate are the Now to respond to the unconstructive negativity at the end. I don't know where that was coming from, given your otherwise positive comment:
What is unfair? We're providing a great new feature that requires a single line of code to opt in. How is this "lock in"? JetBrains will soon adopt it for the Kotlin compiler too and we'd be more than happy for others to pick up this metadata file too.
Incremental compilation is the default in Gradle. Our users have full trust in it and for good reason. We have a huge set of end to end tests ensuring its correctness in all kinds of interesting corner cases.
Maven and Ant yes. Gradle users don't use clean.
This is just FUD. There is no difference between how Gradle detects file changes between modules or inside a single module. Bazel simply decided not to expose the exact change information to the individual tasks, so they always rerun fully if anything has changed. That's fine, but it means bigger modules will always be slow to build on incremental changes. Gradle goes the extra mile and gives you fast and reliable incremental builds, no matter where you are on the modularization spectrum.
Again, this is based on wrong assumptions. We don't wrap the model elements. Also, blaming Eclipse (or Gradle or any other tool) is a bit weird. It is your decision to access compiler internals. By doing that you are breaking encapsulation. You only have yourself to blame for any resulting failure. I know you're doing it for good reasons, but don't blame others when it breaks.
Where is this negativity coming from? Did you have a bad day? |
@oehme I sympathize with your desire to defend the product and clear some FUD, and I stand corrected about Gradle not wrapping model object (just env and filer). Also apologies about apparent negativity (and yes, I'm not positive about Gradle as a product due do a lot of reasons, sorry, and this is not a place to argue about those). Please, don't go personal with "bad day" or assuming I blaming something, it is just an exposure to another information bubble where Gradle is not considered "cool".
Yes, and we workaround bugs that either stay unfixed for years or only available in newest compiler version. Immutables processor is competent to not break on those, just some fixes might disappear and. I and other project contributors spent countless hours tuning for many compilers environments including Eclipse (batch and incremental), Javac, ErrorProne's Javac, Kapt, running with some other rogue processors. Supporting yet another specialized compiler runtime is not something we can take too lightly, the position that "if you only use standard APIs everything will be ok" is incomplete at best and probably inaccurate. |
You should have very little trouble supporting the Gradle case since it's just straight Javac with a wrapper around the Filer and ProcessingEnvironment. Using your fixture project as a test case sounds like a good idea. You could even automate that using Gradle's tooling API if you'd like. |
@oehme thank you for the hints, this should be a way to go (thinking of adding ad-hoc Gradle build to |
If you really want, you could declare the processor as dynamic and use a processor option ( |
@tbroyer Thank you for the hint! I've thought about something like that and I guess I'll git it a try. |
Fwiw, Dagger is going to use something similar: google/dagger#1120 (comment) |
Interesting, actually ordering issue (of calling |
There shouldn't be any ordering issue. |
Apparently, more changes are needed for incremental processing to actually work: with
|
@tbroyer I wonder how origination is established |
In |
ok, thx, I've found it, these are varargs Element after file name. For this, a number of small changes across templates have to be made. The simplest would be to set/scope a "context" element for |
Note that for incremental annotation processing, there needs to be exactly one originating element, i.e. (IIUC) the |
Yep, I've got the idea |
fix for originating elements is released in 2.7.4, please, try it out, thank you! Ideally, we need to also add some notes to the documentation, but this will be, probably, folded into #882 |
Unfortunately, it looks like some more work would be needed (it at all possible):
(I'm sorry, I can't help much more than testing and reporting problems: immutables code base is way too complex; from quick searches into the code, it looks like the processor might be trying to read the source code for the compiled class to… infer its |
The restrictions are quite arbitrary, naive and are not documented. Maybe I'll do the last attempt on this or just leave as is, dunno. |
Hey looks like this restriction has been removed in Gradle 5.0 (and I was testing with 4.10): gradle/gradle@8ce7ea5 Will retest with 5.0 and let you know. |
@tbroyer I appreciate, thanks! |
So, I tried 2.7.4 with Gradle 5.0 and compile is incremental 🎉 Now, I have another problem though, but this is on Gradle and/or JDK side, not Immutables (when I change a source file that won't be processed by Immutables):
(if I change a file that Immutables will process, compilation succeeds) Edit: reported upstream: gradle/gradle#8128 |
Thanks for trying it out! so it's actually if we specify option |
In the repro case attached to gradle/gradle#8128, the first run (full compilation) is basically equivalent to:
when changing
IMO, the warning should either be a "lint" warning that could be disabled with |
my inflamed imagination can think of a way of remembering supported options returned by annotation processors the first time compilation runs so it would subsequently know which ones to pass depending on which annotation processors are skipped... Well I dunno, sounds like overkill |
But then what if the change to |
it is also possible to know in theory, or just resort to full recompilation, there's no bottom to complexity here.. unfortunately |
Since we're driving javac, we could probably suppress that warning, it doesn't play well with incremental processing as you rightly pointed out. Feel free to open an issue.
They're all documented in the incremental annotation processing chapter. I can't comment on "naive", since I don't know what that's supposed to mean. They are not arbitrary, we chose them consciously to keep the scope of this feature in check and because the processors highest on our list didn't need to read/write resource files. This may change in the future, if we encounter enough popular processors that need to read or write resources. Btw. what resources are you trying to read? Gradle uses an empty "sourcePath" by default, so |
this is the issue with evolving documentation, the section contains different information at different moments in time, the current link does not contain anything about reading, and digging into history reveals that it was mentioned (but removed now) but without specifying effect (inhibiting from read resources, issuing error or resorting to full recompilation).
Agree about "non-preciseness" of term "naive". The limitation to reading resources (as opposed to creating) is arbitrary and naive in my view because it was not proved to be needed (and later was changed gradle/gradle@8ce7ea5 Or and, ohboi, I[/we] was[/is] naive too: like checking for source information if it's available to overcome compiler bugs for some niche functionality and expect it will not backfire, sigh.. I think that the main problem is that products like Gradle or Immutables (oh, I don't put them on the same level of popularity or maturity, of course not) have one level of documentation, but where it comes to compiler specifications and interoperabiltity related technologies and dealing with bugs and quirks of existing compiler implementations, we're lacking here. |
Hi, how is the state of full incremental compilation of Immutables? I will get following message, when re-compiling: Gradle: gradle-5.4.1 Change any class will print following output:
Full recompilation is required because org.immutables.processor.ProxyProcessor is not incremental. Analysis took 0.01 secs. If I look into the value-processor jar, there is a incremental.annotation.processors file, which contains "org.immutables.processor.ProxyProcessor,dynamic". |
@SergejIsbrecht You need to opt in: tasks {
compileJava {
options.compilerArgs.add("-Aimmutables.gradle.incremental")
}
} |
Thank you. Looks good to me. |
Hi there, Is this working using the above compileJava command, or was this never released? I see warnings in Intellij after trying the above
|
@ymartin I'm facing a similar issue in my Android project. Were you able to enable it successfully? |
On a (Java 11) service that I'm working on:
|
Gradle 4.7+ has a new feature to let annotation processors run incrementally. In order to make that work, Immutables has to tell Gradle how its annotation processor behave during execution.
For details see:
https://docs.gradle.org/nightly/userguide/java_plugin.html#sec:incremental_annotation_processing
gradle/gradle#5277
Supporting this feature, would greatly reduce build times using Gradle.
The text was updated successfully, but these errors were encountered: