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

Make hibernate-jpamodelgen easier to apply to Quarkus 3.7+ projects #37477

Closed
yrodiere opened this issue Dec 4, 2023 · 27 comments · Fixed by #38227
Closed

Make hibernate-jpamodelgen easier to apply to Quarkus 3.7+ projects #37477

yrodiere opened this issue Dec 4, 2023 · 27 comments · Fixed by #38227
Labels
area/hibernate-orm Hibernate ORM kind/enhancement New feature or request
Milestone

Comments

@yrodiere
Copy link
Member

yrodiere commented Dec 4, 2023

Description

In Quarkus 3.7, Hibernate ORM's "JPAModelGen" annotation processor can no longer simply be dropped into the compile path as a "provided" dependency, because it now has dependencies of its own.

See #37460 and in particular 9e072ed:

Properly apply annotation processors in Panache-Hibernate extensions

Unfortunately, HHH-17362 doesn't fix the problem,
and it's unlikely that it will ever be fixed.
See https://hibernate.atlassian.net/browse/HHH-17362?focusedCommentId=113465

We'll have to apply annotation processors the "right" way, as explained
in https://github.com/hibernate/hibernate-orm/blob/6.3/migration-guide.adoc

Long story short, the proper way to use this annotation processor (and, really, annotation processors in general) is to add the processor, not to the compile path, but to the annotation processor path:

            <plugin>
                <artifactId>maven-compiler-plugin</artifactId>
                <configuration>
                    <annotationProcessorPaths>
                        <annotationProcessorPath>
                            <groupId>org.hibernate.orm</groupId>
                            <artifactId>hibernate-jpamodelgen</artifactId>
                            <version>6.4.0.Final</version> <!-- This is required even with a BOM! See below -->
                        </annotationProcessorPath>
                    </annotationProcessorPaths>
                    <annotationProcessors>
                        <!-- Not sure this is strictly necessary; perhaps processors can be auto-detected -->
                        <annotationProcessor>org.hibernate.jpamodelgen.JPAMetaModelEntityProcessor</annotationProcessor>
                    </annotationProcessors>
                </configuration>
            </plugin>

This is fine and this is already what we recommend in the migration guide.

However, as documented in the migration guide, the BOM won't be applied to Maven's annotationProcessorPath entry and the version of hibernate-jpamodel will have to be specified explicitly by every application. That's inconvenient.

This should be solved in Maven Compiler Plugin version 3.12.0 (see https://issues.apache.org/jira/browse/MCOMPILER-391), but 3.11 was released back in February 2023 and the plugin hasn't seen a release since :/

Implementation ideas

No response

@yrodiere yrodiere added the kind/enhancement New feature or request label Dec 4, 2023
@yrodiere yrodiere added area/hibernate-orm Hibernate ORM and removed triage/needs-triage labels Dec 4, 2023
Copy link

quarkus-bot bot commented Dec 4, 2023

/cc @Sanne (hibernate-orm), @gsmet (hibernate-orm)

@yrodiere yrodiere changed the title Address Make hibernate-jpamodelgen easier to apply to Quarkus 3.7+ projects Dec 4, 2023
@yrodiere
Copy link
Member Author

yrodiere commented Dec 4, 2023

Hey @FroMage , would your current work on annotation processors in Quarkus help in any way, by chance?

@yrodiere
Copy link
Member Author

yrodiere commented Dec 4, 2023

Hey @gsmet and @geoand , I seem to remember you talking to people maintaining Maven plugins to resolve some problems some time ago... would you have contacts working on Maven Compiler Plugin, by chance?

A release of Maven Compiler Plugin 3.12.0 before the release of Quarkus 3.7 would make the issue largely irrelevant, but given how this project hasn't seen many releases this year, I don't know how likely this is.

@geoand
Copy link
Contributor

geoand commented Dec 4, 2023

I personally do not unfortunately

@gsmet
Copy link
Member

gsmet commented Dec 4, 2023

Hey @gnodet ,

Do you know if it would be possible to release a new Maven Compiler Plugin version or do you know someone who could trigger a release? We are eagerly waiting for https://issues.apache.org/jira/browse/MCOMPILER-391 to be released as it's really important for the Hibernate ORM annotation processor (see the description of the issue for the details).

Thanks!

@yrodiere yrodiere added the triage/needs-feedback We are waiting for feedback. label Dec 4, 2023
@FroMage
Copy link
Member

FroMage commented Dec 4, 2023

Well, yeah. My branch makes this sort of set up work in DEV mode, which it does not ATM. The PR is at #37044 but it's sorta stuck pending review and help because I don't know how to write tests for this, or how to make it work for Gradle.

@FroMage
Copy link
Member

FroMage commented Dec 4, 2023

Also note that there's #29068 to make this plugin run automatically, which is sorta also blocked pending help from the IDE folks.

@FroMage
Copy link
Member

FroMage commented Dec 4, 2023

To be cristal clear:

  • <annotationProcessors> is actually not required
  • This already works today, but not with DEV mode (the hot reload will not trigger APT), and my PR fixes that
  • The https://issues.apache.org/jira/browse/MCOMPILER-391 issue only fixes the isssue wrt the version being used automatically from the BOM (which would indeed be great)

@adampoplawski
Copy link

adampoplawski commented Dec 6, 2023

Hello
We are using Quarkus with Gradle and Kotlin and cannot make jpamodelgen work. Is plan for 3.7 support also including Kotlin+Gradle?

@yrodiere yrodiere removed the triage/needs-feedback We are waiting for feedback. label Dec 7, 2023
@yrodiere
Copy link
Member Author

yrodiere commented Dec 7, 2023

Hello.

This particular issue is not about making it possible to use jpamodelgen, but about making it easier. It already works for Java users, it's just that it will become inconvenient in Quarkus 3.7 if we don't do anything.

I'd suggest opening an issue with a clear description (version of Quarkus, stacktrace, ...) and reproducer. Hopefully someone with more Gradle/Kotlin knowledge than me will be able to look into this.

@yrodiere
Copy link
Member Author

Hey @gnodet ,

Do you know if it would be possible to release a new Maven Compiler Plugin version or do you know someone who could trigger a release? We are eagerly waiting for https://issues.apache.org/jira/browse/MCOMPILER-391 to be released as it's really important for the Hibernate ORM annotation processor (see the description of the issue for the details).

Thanks!

Hey @gnodet , gentle reminder of ^ :)

@famod
Copy link
Member

famod commented Dec 16, 2023

FYI, release vote was started yesterday evening. So I think we can expect maven-compiler-plugin 3.12.0 very soon.

@yrodiere
Copy link
Member Author

maven-compiler-plugin 3.12.0 and I've got reports that dependency management indeed works fine for annotationProcessorPath now.

We'll need to upgrade in Quarkus, and maybe add recipes to quarkus upgrade so that the Maven compiler plugin gets updated automatically in existing apps.

@yrodiere
Copy link
Member Author

I updated the migration guide.

Still:

We'll need to upgrade in Quarkus, and maybe add recipes to quarkus upgrade so that the Maven compiler plugin gets updated automatically in existing apps.

@yrodiere
Copy link
Member Author

yrodiere commented Jan 8, 2024

FYI, this depMgmt version lookup does not seem to support relocations: apache/maven-compiler-plugin#180 (comment)

Edit: I probably should have posted this in #37477 instead. cc @yrodiere

Thanks @famod ; from what I understand this could easily be solved on our end by adding dependency management for org.hibernate:hibernate-jpamodelgen? Is that right?

@famod
Copy link
Member

famod commented Jan 8, 2024

@yrodiere

from what I understand this could easily be solved on our end by adding dependency management for org.hibernate:hibernate-jpamodelgen? Is that right?

Yes, that should work. But I don't think it should be there forever, should be dropped for 4.x at the latest (whenever that will be).

@gsmet
Copy link
Member

gsmet commented Jan 8, 2024

from what I understand this could easily be solved on our end by adding dependency management for org.hibernate:hibernate-jpamodelgen? Is that right?

Yeah, I had the same thought the other day. Let's do it and we can drop it at some point.

@yrodiere
Copy link
Member Author

yrodiere commented Jan 8, 2024

Ok, so to sum up:

  1. We need to add dependency management for org.hibernate:hibernate-jpamodelgen
  2. We might want to add something to quarkus update to migrate people away from the provided dependencies to hibernate-jpamodelgen and use <annotationProcessorPaths> instead (and equivalent for Gradle)... though I'm not sure this is easy.
  3. If we do 2, we probably want to backport it to Quarkus 3.2 as well, because of 3.0 Update process does not update hibernate-jpamodelgen annotation processor #32615.

@gsmet
Copy link
Member

gsmet commented Jan 8, 2024

We could probably handle 2. by having a look at https://docs.openrewrite.org/recipes/maven/changepluginconfiguration and create a custom recipe based on this code.

One other thing I was wondering: should we provide a codestart for Hibernate ORM to contribute the annotation processor config to the POM? Or do we want it to be kept optional?

@yrodiere
Copy link
Member Author

yrodiere commented Jan 8, 2024

One other thing I was wondering: should we provide a codestart for Hibernate ORM to contribute the annotation processor config to the POM? Or do we want it to be kept optional?

Personally I'd wait and see how #37044 turns out, because right now we know such annotation processors won't be run again when restarting the app in dev mode, which leads to a poor dev experience. That's true regardless of how annotation processors are defined (provided or annotationProcessorPaths).

@FroMage
Copy link
Member

FroMage commented Jan 11, 2024

I verified in #37044 that the latest plugin uses managed versions. But I'm stuck in DevMojo trying to find it for DEV mode :(

FroMage added a commit to FroMage/quarkus that referenced this issue Jan 12, 2024
FroMage added a commit to FroMage/quarkus that referenced this issue Jan 12, 2024
@FroMage
Copy link
Member

FroMage commented Jan 12, 2024

Works now.

@gnodet
Copy link
Contributor

gnodet commented Jan 12, 2024

Hey @gnodet ,

Do you know if it would be possible to release a new Maven Compiler Plugin version or do you know someone who could trigger a release? We are eagerly waiting for https://issues.apache.org/jira/browse/MCOMPILER-391 to be released as it's really important for the Hibernate ORM annotation processor (see the description of the issue for the details).

Thanks!

Sorry guys, I just realized I was ping'ed on this issue. If you need anything, let me know...

FroMage added a commit to FroMage/quarkus that referenced this issue Jan 12, 2024
FroMage added a commit to FroMage/quarkus that referenced this issue Jan 12, 2024
FroMage added a commit to FroMage/quarkus that referenced this issue Jan 15, 2024
@FroMage
Copy link
Member

FroMage commented Jan 16, 2024

@yrodiere I've merged #37044 (for Maven, not Gradle).

@yrodiere
Copy link
Member Author

Thanks @FroMage! That'll definitely make Quarkus easier to use.

I created #38227 to address the last remaining problem we mentioned further up.

@yrodiere
Copy link
Member Author

Hey @gnodet ,
Do you know if it would be possible to release a new Maven Compiler Plugin version or do you know someone who could trigger a release? We are eagerly waiting for https://issues.apache.org/jira/browse/MCOMPILER-391 to be released as it's really important for the Hibernate ORM annotation processor (see the description of the issue for the details).
Thanks!

Sorry guys, I just realized I was ping'ed on this issue. If you need anything, let me know...

Thanks :) I think we're good now, as Maven Compiler Plugin 3.12.1 behaves pretty much like we need it to :)

@quarkus-bot quarkus-bot bot added this to the 3.7 - main milestone Jan 17, 2024
bpasson pushed a commit to bpasson/quarkus that referenced this issue Jan 18, 2024
@gsmet gsmet modified the milestones: 3.7.0.CR1, 3.6.7 Jan 18, 2024
@yrodiere
Copy link
Member Author

yrodiere commented Feb 2, 2024

FWIW kotlin-maven-plugin latest (1.9.22) also doesn't handle dependency management for annotationProcessorPaths. It only handles copying the version from an existing dependency in the same project (why would that even be useful?).

See https://youtrack.jetbrains.com/issue/KT-59521, JetBrains/kotlin@a3583ea

See also #38558 (comment)

yrodiere added a commit to yrodiere/quarkus that referenced this issue Feb 2, 2024
Removes workaround for HHH-17683 in Panache.

We need to keep specifying the version for Kotlin extensions due to
kotlin-maven-plugin not taking dependency management into account;
see
quarkusio#37477 (comment).
However, we now use the same version as the one we expect people to use
in applications (6.4.3), so it's no longer so much of a hack.

This reverts commit f4b489b.
This reverts commit e745fe3.
gsmet pushed a commit to gsmet/quarkus that referenced this issue Feb 6, 2024
Removes workaround for HHH-17683 in Panache.

We need to keep specifying the version for Kotlin extensions due to
kotlin-maven-plugin not taking dependency management into account;
see
quarkusio#37477 (comment).
However, we now use the same version as the one we expect people to use
in applications (6.4.3), so it's no longer so much of a hack.

This reverts commit f4b489b.
This reverts commit e745fe3.

(cherry picked from commit c62dc92)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/hibernate-orm Hibernate ORM kind/enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants