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

Dependency fixes for Hibernate extensions #37460

Merged
merged 2 commits into from Dec 2, 2023

Conversation

yrodiere
Copy link
Member

@yrodiere yrodiere commented Dec 1, 2023

The first item will likely require that we open an issue, because critically, it means Quarkus users can't rely on the BOM to apply, since the BOM doesn't manage Maven's <annotationProcessorPaths> and hibernate-orm.version doesn't get propagated to POMs using our BOM.

Not sure about changes in the second commit, but let's CI what CI has to say.

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

  2. Fix some wonky dependency exclusions in Hibernate ORM/Reactive/Search extensions

    1. It makes no sense to exclude a transitive dependency if its version
      is managed and we re-declare that dependency ourselves two lines
      below.
    2. jakarta.xml.bind-api and jakarta.activation-api are not banned at
      all, are actually transitive dependencies of hibernate-core and
      hibernate-jpamodelgen, and have their version managed in our BOM.
      So there's no need to exclude these transitive dependencies
      or to declare them when we don't use them directly.
    3. jakarta.activation-api is a dependency of jakarta.xml.bind-api,
      so there's no point excluding jakarta.activation-api if the same
      module is going to depend on jakarta.xml.bind-api.
    4. org.jboss:jandex (old artifact) is no longer a dependency of Hibernate ORM,
      so we don't need to exclude it.
    5. io.smallrye:jandex (new artifact) is never required as a runtime dependency
      of Hibernate ORM, so it should be excluded directly in the BOM.

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
… extensions

1. It makes no sense to exclude a transitive dependency if its version
   is managed and we re-declare that dependency ourselves two lines
   below.
2. jakarta.xml.bind-api and jakarta.activation-api are not banned at
   all, are actually transitive dependencies of hibernate-core and
   hibernate-jpamodelgen, and have their version managed in our BOM.
   So there's no need to exclude these transitive dependencies
   or to declare them when we don't use them directly.
3. jakarta.activation-api is a dependency of jakarta.xml.bind-api,
   so there's no point excluding jakarta.activation-api if the same
   module is going to depend on jakarta.xml.bind-api.
4. org.jboss:jandex (old artifact) is no longer a dependency of Hibernate ORM,
   so we don't need to exclude it.
5. io.smallrye:jandex (new artifact) is never required as a runtime dependency
   of Hibernate ORM, so it should be excluded directly in the BOM.
@quarkus-bot quarkus-bot bot added area/dependencies Pull requests that update a dependency file area/hibernate-orm Hibernate ORM area/hibernate-search Hibernate Search / Elasticsearch area/panache area/persistence labels Dec 1, 2023
@quarkus-bot
Copy link

quarkus-bot bot commented Dec 1, 2023

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

@quarkus-bot
Copy link

quarkus-bot bot commented Dec 1, 2023

✔️ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

<groupId>org.eclipse.angus</groupId>
<artifactId>angus-activation</artifactId>
<scope>provided</scope>
</dependency>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah these were leftovers of when we weren't using these artifacts.

@gsmet gsmet merged commit 9b6f5f0 into quarkusio:main Dec 2, 2023
50 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.7 - main milestone Dec 2, 2023
@yrodiere yrodiere changed the title Dependency fixes Dependency fixes for Hibernate extensions Dec 4, 2023
@yrodiere yrodiere deleted the dependency-fixes branch January 29, 2024 11:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/dependencies Pull requests that update a dependency file area/hibernate-orm Hibernate ORM area/hibernate-search Hibernate Search / Elasticsearch area/panache area/persistence
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants