Skip to content

Conversation

thyming
Copy link

@thyming thyming commented Apr 18, 2019

Gradle 5.4 removes the limitation that incremental annotation processors cannot create resources, so these can now be marked as incremental annotation processors.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Apr 18, 2019
@philwebb
Copy link
Member

@thyming If we make this change do you know what the impact will be for earlier versions of Gradle? I'm not sure we're quite ready to make 5.4 the minimum supported version.

@philwebb philwebb added the status: waiting-for-feedback We need additional information before we can continue label Apr 19, 2019
@thyming
Copy link
Author

thyming commented Apr 19, 2019

Per @oehme (gradle/gradle#4702 (comment)) the fallback behavior is just as it is now.

@wilkinsona wilkinsona changed the title Declare configuration annotation processors as incremental for gradle Declare configuration annotation processors as incremental for Gradle Apr 23, 2019
@wilkinsona wilkinsona added this to the 2.2.x milestone Apr 23, 2019
@wilkinsona wilkinsona added type: enhancement A general enhancement and removed status: waiting-for-feedback We need additional information before we can continue status: waiting-for-triage An issue we've not yet triaged labels Apr 23, 2019
@wilkinsona
Copy link
Member

Thanks for the PR. I am not sure that we can safely mark spring-boot-configuration-processor as incremental. The Gradle documentation says the following:

They must not depend on compiler-specific APIs like com.sun.source.util.Trees. Gradle wraps the processing APIs, so attempts to cast to compiler-specific types will fail. If your processor does this, it cannot be incremental, unless you have some fallback mechanism.

The Trees API is used to determine the default values of a configuration property field for inclusion in the generated metadata. While we use reflection, I expect the wrapping that's done by Gradle to stop this from working.

Have you tried your proposed changes with Gradle 5.4 and verified that the annotation processor works correctly?

@wilkinsona wilkinsona added the status: waiting-for-feedback We need additional information before we can continue label Apr 24, 2019
@snicoll
Copy link
Member

snicoll commented May 21, 2019

@thyming gentle ping. Can you please get back to us? See #16603 (comment)

@thyming
Copy link
Author

thyming commented May 21, 2019

Apologies - this slipped through the cracks.
@wilkinsona I did just build the processor locally and your reading was correct - while everything builds without errors and the incremental compilation behavior is correct, it fails to include the default values in the resultant metadata file.

@thyming thyming closed this May 21, 2019
@wilkinsona
Copy link
Member

Thanks, @thyming.

@wilkinsona wilkinsona removed this from the 2.2.x milestone May 21, 2019
@wilkinsona wilkinsona added status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-feedback We need additional information before we can continue labels May 21, 2019
@oehme
Copy link

oehme commented May 21, 2019

Gradle wraps the processing APIs, so attempts to cast to compiler-specific types will fail. If your processor does this, it cannot be incremental, unless you have some fallback mechanism.

A few processors (e.g. Lombok) unwrap the Gradle decorator and we keep the name of the delegate field stable for that reason. If your usage of the Trees API is in line with the incremental processing contract, you could do that as well. I totally understand if that's too much hackery though :)

Accessing default values of a field would only be okay if you do that for the fields of the currently processed class. If the generated code depends on field initializers of other classes (e.g. superclasses), it won't work since field initializers are considered implementation details and don't trigger recompilation of dependent classes.

@mattdkerr
Copy link

Is this possible now that Gradle 6.0 offers more options? Would a plugin for Gradle aid in solving this problem?

@wilkinsona
Copy link
Member

AFAIK, nothing has changed in Gradle 6 in this area. The 6.0 docs still describes the same limitation as I mentioned above.

@LifeIsStrange
Copy link

LifeIsStrange commented Jun 26, 2020

Wouldn't investigating @oehme idea (doing the same technique as lombok) be a great improvement idea for Spring boot 2.4?

Micronaut 2 has released with incremental annotation processing support! The competition is increasingly eating the deceloppers mindshare because they achieve two pain points objectively better than current spring boot:

  1. Catching more errors at compile time.
  2. And here: faster startup time.

Resolving this issue would be a great progress towards being competitive regarding 2)
Therefore I believe that Pivotal should prioritize it.

@wilkinsona
Copy link
Member

We can take another look at this but it won’t make any difference to startup time. Incremental annotation processing only has the potential to reduce compilation time. Once compilation is complete, there’s no difference to startup time as the end result of the compilation is identical, irrespective of whether or not it was done incrementally.

@wilkinsona wilkinsona added the for: team-attention An issue we'd like other members of the team to review label Jun 26, 2020
@philwebb philwebb reopened this Jun 29, 2020
@philwebb philwebb removed for: team-attention An issue we'd like other members of the team to review status: declined A suggestion or change that we don't feel we should currently apply labels Jun 29, 2020
@philwebb
Copy link
Member

We're going to take another look at this, but there's slightly more work to do than the changes in this PR. Please follow #22150 for updates.

@philwebb philwebb closed this Jun 29, 2020
@philwebb philwebb added status: superseded An issue that has been superseded by another and removed for: merge-with-amendments Needs some changes when we merge type: enhancement A general enhancement labels Jun 29, 2020
@thyming thyming deleted the gradle-incremental-annotation-processors branch June 29, 2020 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: superseded An issue that has been superseded by another
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants