-
Notifications
You must be signed in to change notification settings - Fork 89
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
Point out that PR needs to remove GraalVM reachability metadata related to MBeans #135
Conversation
@@ -122,6 +122,19 @@ output if necessary: | |||
./gradlew check | |||
``` | |||
|
|||
All submitted GraalVM reachability metadata, if the MBean-related part is unnecessary, should actively remove the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should fix the root cause in Native Image. Could you be more precise about what is failing if we introduce MBean metadata?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- As an additional topic, can I bring the following reference links to this document, namely Remove MBean-related reflection entries from hibernate-validator #162 , Remove MBean-related reflection entries from undertow-core #161 and Remove MBean-related entries from hibernate-core's metadata #113 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-
I tried to describe this process in the latest commit. But since this is currently only happening on the Spring Boot side, I'm not too sure about the more specific behavior.
-
I haven't personally investigated AOT on Spring Boot 3.x yet. Because I'm blocked by other behavior at Add support for
com.baomidou:dynamic-datasource-spring-boot-starter:3.6.1
#154.
ff8f69e
to
3e7e698
Compare
32dfdd0
to
a27f568
Compare
a27f568
to
f123f76
Compare
8293f3d
to
914d00e
Compare
914d00e
to
c7825dc
Compare
We need to fix this on the GraalVM side: no extra metadata should change the correct functionality. It would be best to open an issue on the GraalVM repo with a small reproducer. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-
@vjovanov I never reproduced this issue reported by the Spring team, which seems to have to use Spring Boot to write a relatively complex Example.
-
@wilkinsona I wonder if there are some reproducible unit tests on the Spring side?
I don't think there's anything Spring or Spring Boot-specific about the problem. It will be harder to reproduce now after the corrections to the metadata. Please open the requested GraalVM issue and I'll help out there as time permits. IMO, linking to spring-projects/spring-boot#33210 would be a good start. |
I should also have said that, while I agree that this should be fixed on the Graal side, I don't think metadata contributions should include anything for types like |
We have a check now that makes sure that all conditions in the metadata are related to the library in question. This way, all unintentional metadata additions of this kind should be avoided. |
Great stuff! Thank you. |
|
What does this PR do?
CONTRIBUTING.md
point out that PR needs to remove GraalVM reachability metadata related to MBeans #133 .Checklist before merging