-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Adapts AWT extension to the new dynamic .so loading [JDK 17, 19, 20] #32432
Conversation
core/deployment/src/main/java/io/quarkus/deployment/steps/NativeImageJNIConfigStep.java
Outdated
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
@zakkak JDK 20 is fine, but I need to add version conditions to maintain compatibility with older 22.x Mandrel/GraalVM.... |
core/deployment/src/main/java/io/quarkus/deployment/pkg/steps/NativeImageBuildStep.java
Show resolved
Hide resolved
@zakkak I've got the version condition in. I'll adapt your PR about removing raw JSON and test again. THX 👍 |
I am testing it now again with the latest changes purging raw JSON and there are some backward compatibility issues atm...
|
Much better now
|
extensions/awt/runtime/src/main/java/io/quarkus/awt/runtime/JDKSubstitutions.java
Outdated
Show resolved
Hide resolved
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.
LGTM, but please double check the color profile resources aren't included by default already.
extensions/awt/deployment/src/main/java/io/quarkus/awt/deployment/AwtProcessor.java
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
I am content with it now. It's IMHO ready for a review @geoand @gastaldi @zakkak |
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.
LGTM, it would be nice to have a Javadoc on the new BuildItems
...rc/main/java/io/quarkus/deployment/builditem/nativeimage/JniRuntimeAccessFieldBuildItem.java
Show resolved
Hide resolved
...c/main/java/io/quarkus/deployment/builditem/nativeimage/JniRuntimeAccessMethodBuildItem.java
Show resolved
Hide resolved
Awesome! Please squash them all before we merge it. Good job! |
Failing Jobs - Building a354621
Full information is available in the Build summary check run. Failures⚙️ Gradle Tests - JDK 11 Windows #📦 integration-tests/gradle✖
✖
✖
✖
✖
⚙️ JVM Tests - JDK 11 #- Failing: integration-tests/opentelemetry-jdbc-instrumentation
📦 integration-tests/opentelemetry-jdbc-instrumentation✖
|
I don't think the OpenTelemetry JDBC instrumentation would be related... |
Yes, that's a flaky test |
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.
LGTM.
Fixes #31596 (JDK 17)