Skip to content

Add explicit jboss-logging-processor reference to compiler plugin#576

Merged
dmlloyd merged 1 commit intosmallrye:mainfrom
xstefank:logging-annotation-processor
Jan 27, 2025
Merged

Add explicit jboss-logging-processor reference to compiler plugin#576
dmlloyd merged 1 commit intosmallrye:mainfrom
xstefank:logging-annotation-processor

Conversation

@xstefank
Copy link
Member

JDK 23 changed the default annotation processing policy - https://inside.java/2024/06/18/quality-heads-up/#:~:text=Starting%20with%20JDK%2023%2C%20at,the%20default%20on%20JDK%2023.

This breaks tests in sr-health on JDK 23, so we need to declare the annotation processor explicitly.

@xstefank xstefank requested a review from radcortez January 24, 2025 12:37
@xstefank xstefank force-pushed the logging-annotation-processor branch from 365f9b1 to d6b4821 Compare January 24, 2025 12:43
@dmlloyd
Copy link
Contributor

dmlloyd commented Jan 24, 2025

I agree with the change, but we should also remove it from dependencyManagement and possibly even ban it as dependency as well then.

@dmlloyd
Copy link
Contributor

dmlloyd commented Jan 24, 2025

We might have to do that in a follow up to avoid breaking dependents.

@dmlloyd dmlloyd merged commit 03f6cd5 into smallrye:main Jan 27, 2025
1 check passed
@github-actions github-actions bot added this to the 47 milestone Jan 27, 2025
@Ladicek
Copy link
Member

Ladicek commented Jan 28, 2025

FTR, this breaks Jandex build, because jboss-logging-processor is compiled for Java 11 and Jandex stays on Java 8.

In SmallRye Fault Tolerance, I declare all annotation processors explicitly in the modules that require them. I'm honestly not sure we have to specify the jboss-logging-processor as an annotation processor for all compilations. It should be harmless... until it isn't :-)

@dmlloyd
Copy link
Contributor

dmlloyd commented Jan 28, 2025

Jandex should use JDK 17 or later to build. Also if it consumes this project then its build should be in CI for this project as well.

@Ladicek
Copy link
Member

Ladicek commented Jan 28, 2025

It does. For testing, though, I also run the build with Java 8 (and 11, but that's not relevant here), and that's where it fails.

@dmlloyd
Copy link
Contributor

dmlloyd commented Jan 28, 2025

The build should only be run on one later JDK (or else you're going to start hitting some awkward incompatibilities). Testing is (currently) best done by creating extra surefire and/or failsafe executions which run test on the older JDK. This also improves CI time considerably. This parent POM provides a mechanism to do this.

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 this pull request may close these issues.

3 participants