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

Add support for Hibernate 6.6.0.Final #523

Merged
merged 2 commits into from
Aug 21, 2024
Merged

Conversation

sdeleuze
Copy link
Collaborator

@sdeleuze sdeleuze commented Aug 19, 2024

Please wait before merging it we have understood another issue we see with Spring. Will post an update later.

@sdeleuze sdeleuze requested a review from a team as a code owner August 19, 2024 15:22
@sdeleuze sdeleuze requested a review from dnestoro August 19, 2024 15:22
sdeleuze added a commit to sdeleuze/spring-petclinic that referenced this pull request Aug 19, 2024
Changes:
- Metadata from oracle/graalvm-reachability-metadata#523
- Spring Boot 3.4.0-SNAPSHOT
- ext['hibernate.version'] = "6.6.0.Final"
@sdeleuze
Copy link
Collaborator Author

Some entries need to be excluded before this PR is merged, will push an update shortly.

@dnestoro
Copy link
Member

dnestoro commented Aug 20, 2024

Some entries need to be excluded before this PR is merged, will push an update shortly.

Sure, I will wait for your update, but before we merge this PR, can we remove gradle stuff added to this library testing? All other tests are using top level gradle infrastructure

@sdeleuze sdeleuze force-pushed the hibernate-6.6 branch 2 times, most recently from 63ac2cc to a0bd31a Compare August 20, 2024 11:34
@sdeleuze
Copy link
Collaborator Author

sdeleuze commented Aug 20, 2024

@dnestoro Done, I introduced it as it was needed by generate-metadata.sh but was able to refine the script to use the root one.

As the script seems to generate far more metadata, it chose to just add missing entries for [Lorg.hibernate.event.spi.PostUpsertEventListener; and org.hibernate.internal.log.ConnectionInfoLogger_$logger in order to limit the risk of regressions.

Please wait for @christophstrobl green light before merging it.

@christophstrobl
Copy link
Contributor

Should we add "default-for": "6\\.5\\..*" to the metadata-version targeting hibernate 6.5.0.Final line?

this one also showed up in the diff comparing metadata to 6.5:

{
  "condition" : {
    "typeReachable" : "org.hibernate.dialect.function.array.AbstractArrayContainsFunction"
  },
  "name" : "org.hibernate.internal.log.DeprecationLogger_$logger",
  "methods" : [ {
    "name" : "<init>",
    "parameterTypes" : [ "org.jboss.logging.Logger" ]
  } ]
}

might be covered by which is in place.

{
  "condition":{"typeReachable":"org.hibernate.internal.log.DeprecationLogger"},
  "name":"org.hibernate.internal.log.DeprecationLogger_$logger",
  "methods":[{"name":"<init>","parameterTypes":["org.jboss.logging.Logger"] }]
},

Add missing reflection entries for
[Lorg.hibernate.event.spi.PostUpsertEventListener; and
org.hibernate.internal.log.ConnectionInfoLogger_$logger
to the Hibernate 6.5.0.Final metadata to create the
6.6.0.Final one.
@sdeleuze
Copy link
Collaborator Author

@christophstrobl Good point for "default-for": "6\\.5\\..*", I specified in my PR the 3 released versions in tested-versions but you are right, default-for is better and consistent with what we did for other versions, so I modified the PR accordingly.

I think DeprecationLogger is indeed covered by the entry you mentioned, so I kept that as it is.

I tested those metadata with Spring Boot 3.3.2 (Hibernate 6.5) and 3.4.0-SNAPSHOT (Hibernate 6.6), seems to work fine.

@dnestoro The PR looks ready to be merged to me.

@dnestoro
Copy link
Member

LGTM, but since the PR is huge, please wait for a second review as well. cc @olpaw

@olpaw
Copy link
Member

olpaw commented Aug 21, 2024

I looked at the dirdiff for 6.5.0.Final vs. 6.6.0.Final in metadata and tests. The changes are minimal/trivial. No issues.

@dnestoro dnestoro merged commit 0d2fc55 into oracle:master Aug 21, 2024
23 checks passed
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.

4 participants