[go: nahoru, domu]

Skip to content
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

Open
khaidinh opened this issue Jul 23, 2018 · 37 comments
Open

Support Gradle Incremental Processing #804

khaidinh opened this issue Jul 23, 2018 · 37 comments

Comments

@khaidinh
Copy link

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.

@MartyIX
Copy link
MartyIX commented Aug 13, 2018

@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:

@elucash
Copy link
Member
elucash commented Aug 13, 2018

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?

  • Immutables is already incremental capable, like when working inside of Eclipse on ECJ with auto-build on, so in theory we should be ok if incrementality enabled
  • Some features will not work or work slightly differently if put in Gradle isolation sandbox, depending how properly Gradle wrappers are coded (as both Javac and ECJ dev screwed up implementing those elements properly, little hope that Gradle will get it right early on)
  • PRs are welcome. The main problem here is not implementing any change, but ensuring things work properly and I definitely need help here. One of the testing strategies might be copying value-fixture content under Gradle build and see how much of edge cases compile.

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".

@MartyIX
Copy link
MartyIX commented Aug 13, 2018

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. :-)

@oehme
Copy link
oehme commented Aug 14, 2018

Looking at examples I barely understand what exactly has to be done here

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.

as both Javac and ECJ dev screwed up implementing those elements properly, little hope that Gradle will get it right early on

You are assuming we are doing something invasive, which we don't. The only thing we decorate are the ProcessingEnvironment and the Filer. Everything will still work like in plain javac, unless you try to cast these two interfaces to a concrete implementation (which would be a breakage of the processing contract).


Now to respond to the unconstructive negativity at the end. I don't know where that was coming from, given your otherwise positive comment:

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.

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.

The problem with incremental compilation is that no one trust it

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.

so people are doing clean builds with Maven, Ant, Gradle

Maven and Ant yes. Gradle users don't use clean.

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).

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.

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

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.

because of Gradle's "efforts to improve".

Where is this negativity coming from? Did you have a bad day?

@elucash
Copy link
Member
elucash commented Aug 14, 2018

@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".

It is your decision to access compiler internals.

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.

@oehme
Copy link
oehme commented Aug 14, 2018

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.

@elucash
Copy link
Member
elucash commented Aug 14, 2018

@oehme thank you for the hints, this should be a way to go (thinking of adding ad-hoc Gradle build to fixture module for a test drive)

@tbroyer
Copy link
tbroyer commented Aug 29, 2018

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?

If you really want, you could declare the processor as dynamic and use a processor option (-Aimmutables.gradle.incremental=ISOLATING) to activate each mode.

@elucash
Copy link
Member
elucash commented Aug 29, 2018

@tbroyer Thank you for the hint! I've thought about something like that and I guess I'll git it a try.

@tbroyer
Copy link
tbroyer commented Aug 29, 2018

Fwiw, Dagger is going to use something similar: google/dagger#1120 (comment)

@elucash
Copy link
Member
elucash commented Aug 29, 2018

Interesting, actually ordering issue (of calling getSupportedOptions vs init) is something I've considered to be a problem, and indeed it might be, so system props or -A might just work, need to try... still reading the thread

@oehme
Copy link
oehme commented Aug 29, 2018

There shouldn't be any ordering issue. init is called before getSupportedOptions.

@tbroyer
Copy link
tbroyer commented Nov 14, 2018

Apparently, more changes are needed for incremental processing to actually work: with ./gradlew compileJava --info on my project, I got:

Full recompilation is required because the generated type 'xxx.xxx.ImmutableXxx' must have exactly one originating element, but had 0. Analysis took 0.017 secs.

@elucash
Copy link
Member
elucash commented Nov 14, 2018

@tbroyer I wonder how origination is established

@tbroyer
Copy link
tbroyer commented Nov 14, 2018

In Filer's createSourceFile.

@elucash
Copy link
Member
elucash commented Nov 14, 2018

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 [output.java] directives.

@tbroyer
Copy link
tbroyer commented Nov 14, 2018

Note that for incremental annotation processing, there needs to be exactly one originating element, i.e. (IIUC) the @Value.Enclosing when it exists, and not its inner @Value.Immutable types.

@elucash
Copy link
Member
elucash commented Nov 14, 2018

Yep, I've got the idea

@elucash
Copy link
Member
elucash commented Dec 27, 2018

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

@tbroyer
Copy link
tbroyer commented Dec 27, 2018

Unfortunately, it looks like some more work would be needed (it at all possible):

Full recompilation is required because incremental annotation processors are not allowed to read resources. Analysis took 0.039 secs.

(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 imports? or possibly the culprit is the loading of extensions?)

@elucash
Copy link
Member
elucash commented Dec 27, 2018

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.

@tbroyer
Copy link
tbroyer commented Dec 27, 2018

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.

@elucash
Copy link
Member
elucash commented Dec 27, 2018

@tbroyer I appreciate, thanks!

@tbroyer
Copy link
tbroyer commented Dec 29, 2018

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):

Compiling with JDK Java compiler API.
warning: The following options were not recognized by any processor: '[immutables.gradle.incremental]'
error: warnings found and -Werror specified
1 error
1 warning
Incremental compilation of 1 classes completed in 0.233 secs.

(if I change a file that Immutables will process, compilation succeeds)

Edit: reported upstream: gradle/gradle#8128

@elucash
Copy link
Member
elucash commented Dec 29, 2018

Thanks for trying it out! so it's actually if we specify option -A warning tells us that no processor consumes it because we skipped immutables processor on that incremental run when no immutables affected class changed.
A possible solution might be either disabling this particular warning, or trying to suppress such warning in incremental compilation run, or always launch annotation processors but with no classes in the round environment (if at all possible). Looks like all of these options are outside the annotation processor, so let's see what would be the advice from Gradle team

@tbroyer
Copy link
tbroyer commented Dec 29, 2018

In the repro case attached to gradle/gradle#8128, the first run (full compilation) is basically equivalent to:

javac -cp value-annotations-2.7.4.jar -processorpath value-2.7.4.jar -d build/classes/java/main -Aimmutables.gradle.incremental src/main/java/reprocase/Main.java src/main/java/reprocase/SomeValue.java

when changing Main.java, the second run is equivalent to:

javac -cp value-annotations-2.7.4.jar:build/classes/java/main -processorpath value-2.7.4.jar -d build/classes/java/main -Aimmutables.gradle.incremental src/main/java/reprocase/Main.java

SomeValue is loaded as a class from build/classes/java/main in the classpath, compiled from the previous run, and Main.java doesn't trigger the processor, so javac complains. AFAIK, there's no way to turn the warning off (using javac flags at least, haven't tried with javax.tools.JavaCompiler), and it turns into an error when using -Werror.

IMO, the warning should either be a "lint" warning that could be disabled with -Xlint:-processing, or shouldn't be turned into an error by -Werror; but as it is Gradle that calls javac with different arguments each time, maybe there's something that can be done at their level.

@elucash
Copy link
Member
elucash commented Dec 29, 2018

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

@tbroyer
Copy link
tbroyer commented Dec 29, 2018

But then what if the change to Main.java is to add @Value.Immutable? 😉

@elucash
Copy link
Member
elucash commented Dec 29, 2018

it is also possible to know in theory, or just resort to full recompilation, there's no bottom to complexity here.. unfortunately

@oehme
Copy link
oehme commented Dec 31, 2018

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.

The restrictions are quite arbitrary, naive and are not documented.

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 Filer#getResource would never be successful as far as I can tell, independently of incremental processing.

@elucash
Copy link
Member
elucash commented Dec 31, 2018

They're all documented in the incremental annotation processing chapter

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).

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...

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
) and "statistical" approach is "working" but cannot be claimed to be free of "arbitrary and naive".

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.

@SergejIsbrecht
Copy link
SergejIsbrecht commented Apr 28, 2019

Hi,

how is the state of full incremental compilation of Immutables? I will get following message, when re-compiling:

Gradle: gradle-5.4.1
dependencies { annotationProcessor("org.immutables:value-processor:2.7.4") compileOnly("org.immutables:value-annotations:2.7.4") }

Change any class will print following output:
`

Task :compileJava
Caching disabled for task ':compileJava' because:
Build cache is disabled
Task ':compileJava' is not up-to-date because:
Input property 'source' file /home/sergej/Development/IdeaProjects/playground/src/main/java/Wurst.java has changed.
Created classpath snapshot for incremental compilation in 0.004 secs.
Class dependency analysis for incremental compilation took 0.004 secs.
Full recompilation is required because org.immutables.processor.ProxyProcessor is not incremental. Analysis took 0.01 secs.
Compiling with JDK Java compiler API.
:compileJava (Thread[Execution worker for ':' Thread 2,5,main]) completed. Took 0.847 secs.

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".

@tbroyer
Copy link
tbroyer commented Apr 28, 2019

@SergejIsbrecht You need to opt in:

tasks {
    compileJava {
        options.compilerArgs.add("-Aimmutables.gradle.incremental")
    }
}

@SergejIsbrecht
Copy link

@SergejIsbrecht You need to opt in:

tasks {
    compileJava {
        options.compilerArgs.add("-Aimmutables.gradle.incremental")
    }
}

Thank you. Looks good to me.

@ymartin
Copy link
ymartin commented Apr 24, 2020

Hi there,

Is this working using the above compileJava command, or was this never released?

I see warnings in Intellij after trying the above Warning:java: The following options were not recognized by any processor: '[immutables.gradle.incremental]'

@SergejIsbrecht You need to opt in:

tasks {
    compileJava {
        options.compilerArgs.add("-Aimmutables.gradle.incremental")
    }
}

@dijgas
Copy link
dijgas commented Aug 31, 2021

@ymartin I'm facing a similar issue in my Android project. Were you able to enable it successfully?

@zanella
Copy link
zanella commented Sep 30, 2022

On a (Java 11) service that I'm working on:

  • Bumped Gradle to 7.5 (Immutables version = 2.8.1)
  • Added the "-Aimmutables.gradle.incremental"
  • Incremental compilation works 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants