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

Process property upgrades in each subproject separately #25990

Closed
lptr opened this issue Aug 1, 2023 · 2 comments · Fixed by #26165
Closed

Process property upgrades in each subproject separately #25990

lptr opened this issue Aug 1, 2023 · 2 comments · Fixed by #26165
Milestone

Comments

@lptr
Copy link
Member

lptr commented Aug 1, 2023

We should have an annotation processor execute on each subproject individually to process @UpgradedProperty definitions. This is to avoid the need for introducing dependencies from the instrumentation-declarations subproject.


cc: @gradle/bt-execution

@asodja
Copy link
Member

asodja commented Aug 11, 2023

I found one issue with annotation processor: currently we do full recompilation of Groovy tests, see:
#26085

So we need to fix that.

bot-gradle added a commit that referenced this issue Aug 22, 2023
…erceptor from plugins classloader via SPI

And add Property upgrades instrumentation to plugins classpath.

That way we don't have classloading issues in adapter logic, where we reference plugin types.

Main changes:
1. With annotation processor we now generate also `META-INF/services/org.gradle.internal.classpath.intercept.CallInterceptor` and `META-INF/services/org.gradle.internal.instrumentation.api.jvmbytecode.JvmBytecodeCallInterceptor$Factory`.
2. In `DefaultClassLoaderRegistry` we load interceptors from plugins class loader as: `CallInterceptorRegistry.loadCallInterceptors(pluginsClassLoader);`
3. `instrumentation-declarations` are now a `pluginsRuntimeOnly` dependency of `:distribution-core` and not anymore a `runtimeOnly` dependency of `:launcher`

Fixes #25893
Fixes #25894

This change will also make it simpler to implement #25990.

### Reviewing cheatsheet

Before merging the PR, comments starting with
- ❌ ❓**must** be fixed
- 🤔 💅 **should** be fixed
- 💭 **may** be fixed
- 🎉 celebrate happy things

Co-authored-by: Anže Sodja <asodja@gradle.com>
@asodja
Copy link
Member

asodja commented Aug 23, 2023

It seems issue #26085 is specific just for :core where we generate test interceptors.
Although that shows that incremental annotation processing is a problem for projects that have main source set with mix Groovy/Java joint compilation (e.g. :code-quality).

For such projects we can probably keep instrumentation-declarations or maybe we separate Groovy code to a different SourceSet.

For reference, it seems we disabled incremental annotation processing for Groovy/Java joint compilation in #9871, because there were too many corner cases and supporting it was expensive.

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

Successfully merging a pull request may close this issue.

4 participants