[GR-48144] Fix native-image build with --no-jlinking #7205
Conversation
oracle-contributor-agreement
bot
added
the
OCA Verified
substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/NativeImage.java
Outdated
Show resolved
Hide resolved
substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/NativeImage.java
Outdated
Show resolved
Hide resolved
substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/NativeImage.java
Show resolved
Hide resolved
jerboaa
force-pushed
the
no-jlinking-fixes-7085
branch
from
3259d65 to
accb9ef
Compare
There was a problem hiding this comment.
@jerboaa I was able to get rid of the polyglot.jar and truffle-api.jar in the launcher with zakkak@85088b1 (on top of your branch)
To test it I used:
~/code/mx/mx --primary-suite vm --no-jlinking --env ce build
which is supposed to build GraalVM CE (including polyglot and truffle):
❯ ~/code/mx/mx --primary-suite vm --no-jlinking --env ce graalvm-show
GraalVM distribution: GRAALVM_COMMUNITY_JAVA21
Version: 23.1.0-dev
Config name: community
Components:
- GraalVM compiler ('cmp', /graal, experimental)
- GraalVM Coverage ('cov', /coverage, experimental)
- GraalVM Debug Protocol Server ('dap', /dap, experimental)
- Component installer ('gu', /installer, experimental)
- GraalVM license files ('gvm', /., experimental)
- GraalVM Chrome Inspector ('ins', /chromeinspector, experimental)
- Insight ('insight', /insight, experimental)
- Insight Heap ('insightheap', /insightheap, experimental)
- LibGraal ('lg', /False, experimental)
- GraalVM Language Server ('lsp', /lsp, experimental)
- Truffle NFI LIBFFI ('nfi-libffi', /nfi, experimental)
- Native Image ('ni', /svm, experimental)
- Native Image Configure Tool ('nic', /svm, experimental)
- Native Image licence files ('nil', /svm, experimental)
- Polyglot Launcher ('poly', /polyglot, experimental)
- Polyglot Native API ('polynative', /polyglot, experimental)
- GraalVM Profiler ('pro', /profiler, experimental)
- Graal SDK ('sdk', /graalvm, experimental)
- GraalVM Launcher Common ('sdkl', /graalvm, experimental)
- SubstrateVM ('svm', /svm, experimental)
- Truffle Runtime SVM ('svmt', /truffle, experimental)
- SVM Truffle NFI Support ('svmnfi', /nfi, experimental)
- SubstrateVM Static Libraries ('svmsl', /False, experimental)
- Truffle ('tfl', /truffle, experimental)
- Truffle API ('tfla', /truffle, experimental)
- Truffle Compiler ('tflc', /truffle, experimental)
- Truffle Macro ('tflm', /truffle, experimental)
- Truffle JSON Library ('truffle-json', /truffle-json, experimental)
- Graal SDK Compiler ('sdkc', /graalvm, experimental)
- Truffle NFI ('nfi', /nfi, experimental)
- SubstrateVM Foreign API Preview Feature ('svmforeign', /svm-preview, experimental)
- Graal SDK Native Image ('sdkni', /graalvm, experimental)
- Polyglot Library ('libpoly', /polyglot, experimental)
Launchers:
- gu (bash, rebuildable)
- native-image (native, rebuildable)
- native-image-configure (bash, rebuildable)
- polyglot (bash, rebuildable)
Libraries:
- libjvmcicompiler.so (native, non-rebuildable)
- libnative-image-agent.so (native, rebuildable)
- libnative-image-diagnostics-agent.so (native, rebuildable)
- libpolyglot.so (skipped, rebuildable)
Installables:
- NATIVE_IMAGE_INSTALLABLE_SVM_JAVA21
No standalone
Please have a look.
|
Fantastic! Let me merge that in and I'll test myself as well. |
jerboaa
force-pushed
the
no-jlinking-fixes-7085
branch
from
accb9ef to
70bb6a9
Compare
|
Thanks @zakkak for that update. It's now included. @olpaw @chumer @fniephaus Could you please help get this reviewed, as this is needed to get Mandrel 23.1 builds working again (broken for 2 weeks now). Thank you! |
jerboaa
force-pushed
the
no-jlinking-fixes-7085
branch
from
70bb6a9 to
4af2a84
Compare
substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/NativeImage.java
Outdated
Show resolved
Hide resolved
substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/NativeImage.java
Outdated
Show resolved
Hide resolved
substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/NativeImage.java
Outdated
Show resolved
Hide resolved
substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/NativeImage.java
Outdated
Show resolved
Hide resolved
jerboaa
force-pushed
the
no-jlinking-fixes-7085
branch
from
4af2a84 to
0eb2ca4
Compare
|
Thanks for the review, @olpaw! Please take another look. |
@olpaw The gate failures seem to be caused by passing in When I add This didn't fail with the earlier patch as not all options got passed whole-sale to the
Which one should it be? |
Please go for IMO, |
|
OK, glad we came to the same conclusion. Testing that update right now. |
olpaw
changed the title
|
Running in internal CI ... ⏳ |
|
Thanks for the review! Let me know if there is anything more that you need from my end. |
This found issues with the current approach. We are regressing on On master this works fine but on this PR we get: The reason is that the But luckily we do have our to see what I mean (and compare it to the output of |
|
Thanks for the update. I'll look into it. |
substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/NativeImage.java
Show resolved
Hide resolved
|
@olpaw Gentle ping on as to how to move forward with this for 24.0. |
I'm playing with using |
jerboaa
force-pushed
the
no-jlinking-fixes-7085
branch
from
ece06b7 to
261dd42
Compare
|
Thanks! I will take a look in the next days. |
|
All our internal CI gates passed with this change. I'm now just waiting for @chumer to respond to #7205 (comment) |
Thanks for the update! |
|
@chumer Could you please take another look? Does the latest update address your concerns? Thanks in advance! |
|
Is there something missing for this PR to get merged? If so, please let me know. Thanks! |
Still waiting for @chumer to respond to #7205 (comment) If there is no further feedback by the end of the week I will merge the PR as is. |
Thank you! |
| @@ -574,19 +576,37 @@ public List<Path> getBuilderModulePath() { | |||
| } | |||
|
|
|||
| private List<Path> createTruffleBuilderModulePath() { | |||
| List<Path> jars = getJars(rootDir.resolve(Paths.get("lib", "truffle")), "truffle-api", "truffle-runtime", "truffle-enterprise"); | |||
| Path libTruffleDir = rootDir.resolve(Paths.get("lib", "truffle")); | |||
| List<Path> jars = getJars(libTruffleDir, "truffle-api", "truffle-runtime", "truffle-enterprise"); | |||
| if (!jars.isEmpty()) { | |||
There was a problem hiding this comment.
This branch should never be triggered >= 23.1. This is only here to support builds where truffle is still living in a GraalVM JDK. How come this triggers for you? What component configuration are you using?
After 23.1 you should use a configuration like this: https://github.com/oracle/graal/blob/master/vm/mx.vm/ce#L2
There was a problem hiding this comment.
@chumer these changes were introduced by me in zakkak@85088b1
As I explain in #7205 (review) (back then) these changes allowed me to build GraalVM CE with --no-jlinking using:
~/code/mx/mx --primary-suite vm --no-jlinking --env ce build
Looking at the CE configuration that was used while testing the above I see more components being included which probably explains the need for these changes back then: https://github.com/zakkak/mandrel/blob/85088b19f8062246d5f0a95c83ace120ee034c25/vm/mx.vm/ce#L2
As @jerboaa said we are more than happy to drop these if they are no longer relevant.
@jerboaa let me know if you need any help with this.
There was a problem hiding this comment.
It doesn't (usually) trigger for us. I think this code is pre-existing and we wanted to make sure we don't break anything when this path is being used. This code seems to account for the fact that certain (modular) jar files have dependencies. If this path is taken and a truffle-runtime-svm is in lib/truffle/builder, polyglot.jar and jniutils.jar needs to be added when the JDK isn't jlinked (and, therefore, doesn't include those modules):
$ jar -d -f sdk/mxbuild/linux-amd64/GRAALVM_COMMUNITY_JAVA22/graalvm-community-openjdk-22+17.1/lib/truffle/builder/truffle-runtime-svm.jar | grep -E 'polyglot|truffle\.runtime'
org.graalvm.truffle.runtime.svm jar:file:///disk/graal/upstream-sources/graal/sdk/mxbuild/linux-amd64/GRAALVM_COMMUNITY_JAVA22/graalvm-community-openjdk-22+17.1/lib/truffle/builder/truffle-runtime-svm.jar!/module-info.class
requires org.graalvm.polyglot transitive
requires org.graalvm.truffle.runtime static
qualified exports com.oracle.svm.truffle to com.oracle.truffle.enterprise.svm org.graalvm.truffle.runtime.svm
qualified exports com.oracle.svm.truffle.api to com.oracle.truffle.enterprise.svm org.graalvm.truffle org.graalvm.truffle.runtime.svm
qualified exports com.oracle.svm.truffle.isolated to com.oracle.truffle.enterprise.svm org.graalvm.truffle.runtime.svm
qualified exports com.oracle.svm.truffle.nfi to com.oracle.truffle.enterprise.svm org.graalvm.truffle.runtime.svm
qualified exports com.oracle.svm.truffle.nfi.libffi to com.oracle.truffle.enterprise.svm org.graalvm.truffle.runtime.svm
qualified exports com.oracle.svm.truffle.nfi.posix to com.oracle.truffle.enterprise.svm org.graalvm.truffle.runtime.svm
qualified exports com.oracle.svm.truffle.nfi.windows to com.oracle.truffle.enterprise.svm org.graalvm.truffle.runtime.svm
$ jar -d -f ./truffle/mxbuild/jdk22/dists/jdk17/truffle-runtime.jar | grep jniutils
requires org.graalvm.jniutils transitive
$ jar -d -f sdk/mxbuild/jdk22/dists/jdk17/jniutils.jar | head -n1
org.graalvm.jniutils jar:file:///disk/graal/upstream-sources/graal/sdk/mxbuild/jdk22/dists/jdk17/jniutils.jar!/module-info.class
Does that sound OK?
There was a problem hiding this comment.
There was a problem hiding this comment.
@jerboaa @zakkak Your CE env config explains why this still triggers for you. As you know, we have gone unchained and many of your CE env configuration components are about to be removed on master. I'd recommend switching to a config similar to this: https://github.com/oracle/graal/blob/master/vm/mx.vm/ce#L2
We do change these component lists whenever we need to, so I'd recommend undoing the custom changes to your env config.
Does that sound OK?
You should never need to put truffle-runtime-svm on the module-path manually in the driver after >= 23.1, as you mentioned it has further module dependencies so can only activate automatically if certain jars are put on the image module/class-path by the user. The driver no longer decides this >= 23.1, which is why I mentioned that the condition should not trigger above. If the truffle-runtime.jar gets put on the image module-path by the user the truffle-svm macro gets activated which then activates truffle-runtime-svm. truffle-runtime.jar has all the necessary dependencies and already expects them on the image module-path, so when truffle-runtime-svm gets activated through the macro all dependencies are satisfied, including polyglot.
Ultimately @olpaw is in charge of this code and needs to decide what goes in and what not. I do not know enough about --no-jlinking to judge whether these changes are justified. I know that many of your changes probably stem from an outdated/wrong environment config, and I think these changes will mostly no longer be necessary after you update the env. @olpaw should decide whether it is fine to merge this as an intermediate step top unblock you.
There was a problem hiding this comment.
Thanks for your input. So the the comment inside the if block is accurate: This is legacy support and should in the future no longer be needed. Question is when we remove this. @olpaw Opinions?
There was a problem hiding this comment.
@jerboaa your PR is already in our merge queue. Feel free to provide a cleanup PR after this one got merged that reduces the changes needed for your usecase to a minimum.
There was a problem hiding this comment.
Great, thank you. Will do that, then.
|
Thank you, @chumer for the review and approval! |
|
@olpaw Hi! Just checking, is this still in the merge queue? |
Yes, it is... it unfortunately failed due to some infra issue, but it's currently on the top again. If all goes well, it should be merged within the next couple of hours. |
OK, thanks Fabio. |
|
Sorry, but it failed again...I hope we can resolve the infra issues very soon. |
In #7171
graal-sdkgot split into several parts. This patch accounts for that and addsthe new dependencies to the module path when the JDKs
jimageis being queried for built-in modules.--add-modules org.graalvm.polyglothas been dropped from the native-image launcher. Itgets added to the module path on demand if needed.
Closes #7085