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.

Sign up for GitHub

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

Jump to bottom

Introduce new JPMS exports to allow running native-image on module-path #25986

Merged
merged 3 commits into from
Jun 16, 2022
Merged

Introduce new JPMS exports to allow running native-image on module-path #25986

merged 3 commits into from
Jun 16, 2022

Conversation

zakkak
Copy link
Contributor

@zakkak zakkak commented Jun 7, 2022

Once oracle/graal#4468 gets merged, native-image will run on module-path requiring some additional exports to be passed to it in order to access internal APIs.

Marking as Draft till I finish testing it with oracle/graal#4468

@quarkus-bot quarkus-bot bot added the area/core label Jun 7, 2022
@jerboaa
Copy link
Contributor

jerboaa commented Jun 10, 2022

@zakkak Could we move this out of draft now, please? It looks like our nightly mandrel builds are failing miserably because of oracle/graal#4468

See for example:
https://github.com/graalvm/mandrel/runs/6824595648

@jerboaa jerboaa mentioned this pull request Jun 10, 2022
Closed
@zakkak
Copy link
Contributor Author

zakkak commented Jun 10, 2022

@jerboaa this is not ready to be merged though.
We should only export these modules if graalvm is newer than 22.1. Additionally there is still an issue with the awt test which @olpaw is investigating. Hopefully we will have these ready next week.

In the meantime for Mandrel's CI we can set USE_NATIVE_IMAGE_JAVA_MODULE_SYSTEM to false for now.

@jerboaa
Copy link
Contributor

jerboaa commented Jun 10, 2022

In the meantime for Mandrel's CI we can set USE_NATIVE_IMAGE_JAVA_MODULE_SYSTEM to false for now.

Fair enough. As long as we keep a tracker with remaining issues and flip that once everything is fixed.

@jerboaa jerboaa mentioned this pull request Jun 10, 2022
Closed
1 task
@zakkak
Introduce new JPMS exports to allow running native-image on module-path
6ddd964 
Once oracle/graal#4468 gets merged, native-image
will run on module-path requiring some additional exports to be passed
to it in order to access internal APIs.
@zakkak zakkak force-pushed the maodule-path-patch branch from acd19ea to 2e3fcf7 Compare June 15, 2022 11:50
@zakkak zakkak force-pushed the maodule-path-patch branch from 2e3fcf7 to 2e80961 Compare June 15, 2022 13:35
@zakkak zakkak marked this pull request as ready for review June 15, 2022 14:07
@zakkak zakkak requested review from geoand and galderz June 15, 2022 14:07
@zakkak
Copy link
Contributor Author

zakkak commented Jun 15, 2022 •
edited
Loading

We should only export these modules if graalvm is newer than 22.1. Additionally there is still an issue with the awt test which @olpaw is investigating. Hopefully we will have these ready next week.

Both issues are now tackled and this is now ready for review.

@quarkus-bot

This comment has been minimized.

Sign in to view
@geoand geoand merged commit 749f26a into quarkusio:main Jun 16, 2022
@quarkus-bot quarkus-bot bot added this to the 2.11 - main milestone Jun 16, 2022
@zakkak zakkak deleted the maodule-path-patch branch June 16, 2022 06:40
zakkak added a commit to zakkak/mandrel that referenced this pull request Jun 16, 2022
@zakkak
CI: Re-enable running image-builder on module path
0ae9f06 
quarkusio/quarkus#25986 allows Quarkus native to
work with the image-builder running on module path.

Reverts: oracle#388, oracle#390, oracle#391

Closes: oracle#389
@zakkak zakkak mentioned this pull request Jun 16, 2022
Merged
zakkak added a commit to graalvm/mandrel that referenced this pull request Jun 16, 2022
@zakkak
CI: Re-enable running image-builder on module path
0c53a5b 
quarkusio/quarkus#25986 allows Quarkus native to
work with the image-builder running on module path.

Reverts: #388, #390, #391

Closes: #389
zakkak added a commit to zakkak/quarkus that referenced this pull request Jun 29, 2022
@zakkak
Fix JPMS exports
8b8ed07 
PR quarkusio#25986 was mistakenly tested only with GraalVM set as
`JAVA_HOME`. This resulted in Quarkus being able to properly figure the
GraalVM version using `org.graalvm.home.Version#getCurrent`. This is not
the case though when `JAVA_HOME` is set to point to a non-GraalVM JDK,
since `org.graalvm.home.Version#getCurrent` always returns `snapshot` in
that case which results in the JPMS exports being always passed to the
`native-image` command. This results in warnings like the following when
building with GraalVM 22.1 or earlier:

```
WARNING: Unknown module: org.graalvm.nativeimage.builder specified to --add-exports
WARNING: Unknown module: org.graalvm.nativeimage.base specified to --add-exports
WARNING: Unknown module: org.graalvm.nativeimage.builder specified to --add-exports
```
@zakkak zakkak mentioned this pull request Jun 29, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@geoand geoand geoand approved these changes

@galderz galderz Awaiting requested review from galderz

Assignees
No one assigned
Labels
area/core area/graphics
Projects
None yet
Milestone
2.11.0.CR1
Development

Successfully merging this pull request may close these issues.
3 participants
@zakkak @jerboaa @geoand