Skip to content

[GR-48144] Fix native-image build with --no-jlinking #7205

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

Merged
merged 3 commits into from
Oct 27, 2023
Merged

[GR-48144] Fix native-image build with --no-jlinking #7205

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c6bdbb6
Add new dependencies after graal-sdk split
jerboaa Aug 11, 2023
261dd42
Drop polyglot.jar and truffle-api.jar from native-image launcher
jerboaa Aug 14, 2023
0c518c7
Add polyglot.jar only if truffle-runtime-svm is there as well
jerboaa Oct 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 2 additions & 6 deletions sdk/mx.sdk/mx_sdk_vm_impl.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2232,13 +2232,9 @@ def _get_extra_jvm_args():
extra_jvm_args = mx.list_to_cmd_line(image_config.extra_jvm_args)
if not _jlink_libraries():
if mx.is_windows():
extra_jvm_args = ' '.join([extra_jvm_args, r'--upgrade-module-path "%location%\..\..\jvmci\graal.jar"',
r'--add-modules org.graalvm.polyglot',
r'--module-path "%location%\..\..\truffle\truffle-api.jar:%location%\..\..\jvmci\polyglot.jar"'])
extra_jvm_args = ' '.join([extra_jvm_args, r'--upgrade-module-path "%location%\..\..\jvmci\graal.jar"'])
else:
extra_jvm_args = ' '.join([extra_jvm_args, '--upgrade-module-path "${location}/../../jvmci/graal.jar"',
'--add-modules org.graalvm.polyglot',
'--module-path "${location}/../../truffle/truffle-api.jar:${location}/../../jvmci/polyglot.jar"'])
extra_jvm_args = ' '.join([extra_jvm_args, '--upgrade-module-path "${location}/../../jvmci/graal.jar"'])
return extra_jvm_args

def _get_option_vars():
Expand Down
32 changes: 26 additions & 6 deletions substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/NativeImage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -561,10 +561,12 @@ public List<String> getBuilderJavaArgs() {
*/
public List<Path> getBuilderModulePath() {
List<Path> result = new ArrayList<>();
// Non-jlinked JDKs need truffle and graal-sdk on the module path since they
// don't have those modules as part of the JDK.
// Non-jlinked JDKs need truffle and word, collections, nativeimage on the
// module path since they don't have those modules as part of the JDK. Note
// that graal-sdk is now obsolete after the split in GR-43819 (#7171)
if (libJvmciDir != null) {
result.addAll(getJars(libJvmciDir, "graal-sdk", "enterprise-graal"));
result.addAll(getJars(libJvmciDir, "enterprise-graal"));
result.addAll(getJars(libJvmciDir, "word", "collections", "nativeimage"));
}
if (modulePathBuild) {
result.addAll(createTruffleBuilderModulePath());
Expand All @@ -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()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chumer As @zakkak said, we can drop those changes if you are happy for us to drop them. Or we can clean it up in a subsequent patch? Either way, those changes aren't important for us. The key piece is to have the native-image component build working in the --no-jlinking config.

Copy link
Member

@chumer chumer Oct 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thank you. Will do that, then.

/*
* If Truffle is installed as part of the JDK we always add the builder modules of
* Truffle to the builder module path. This is legacy support and should in the
* future no longer be needed.
*/
jars.addAll(getJars(rootDir.resolve(Paths.get("lib", "truffle")), "truffle-compiler"));
jars.addAll(getJars(libTruffleDir, "truffle-compiler"));
Path builderPath = rootDir.resolve(Paths.get("lib", "truffle", "builder"));
if (Files.exists(builderPath)) {
jars.addAll(getJars(builderPath, "truffle-runtime-svm", "truffle-enterprise-svm"));
List<Path> truffleRuntimeSVMJars = getJars(builderPath, "truffle-runtime-svm", "truffle-enterprise-svm");
jars.addAll(truffleRuntimeSVMJars);
if (libJvmciDir != null && !truffleRuntimeSVMJars.isEmpty()) {
// truffle-runtime-svm depends on polyglot, which is not part of non-jlinked
// JDKs
jars.addAll(getJars(libJvmciDir, "polyglot"));
}
}
if (libJvmciDir != null) {
// truffle-runtime depends on polyglot, which is not part of non-jlinked JDKs
jars.addAll(getJars(libTruffleDir, "jniutils"));
}
}
/*
* Non-Jlinked JDKs don't have truffle-compiler as part of the JDK, however the native
* image builder still needs it
*/
if (libJvmciDir != null) {
jars.addAll(getJars(libTruffleDir, "truffle-compiler"));
}

return jars;
}
Expand Down