-
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
Drop "deployment" dependency on org.graalvm.nativeimage.impl package #28084
Conversation
Resolves errors like: ``` [error]: Build step io.quarkus.deployment.steps.NativeImageFeatureStep#generateFeature threw an exception: java.lang.IllegalAccessError: class io.quarkus.deployment.steps.NativeImageFeatureStep (in unnamed module @0x53fa18a5) cannot access class org.graalvm.nativeimage.impl.ConfigurationCondition (in module org.graalvm.sdk) because module org.graalvm.sdk does not export org.graalvm.nativeimage.impl to unnamed module @0x53fa18a5 ``` Note that this is a temporary work-around until the reliance on `org.graalvm.nativeimage.impl` is completely dropped in favour of using public APIs only.
@jerboaa can you please review this? |
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. Looking forward to get rid of those impl
classes from GraalVM.
It's WIP (see #28093), we still need a couple of changes upstream to make it possible (see oracle/graal#5013) |
Excellent, thank you! |
Failing Jobs - Building e9133ed
Full information is available in the Build summary check run. Failures⚙️ JVM Tests - JDK 11 Windows #- Failing: extensions/smallrye-reactive-messaging-amqp/deployment
! Skipped: integration-tests/reactive-messaging-amqp 📦 extensions/smallrye-reactive-messaging-amqp/deployment✖
✖
✖
✖
✖
|
Resolves errors (appearing after #27728) like:
Note that this is a temporary work-around until the reliance on
org.graalvm.nativeimage.impl
is completely dropped in favor of using public APIs only as discussed in #27728 (comment).Fixes graalvm/mandrel#417