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.

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

[GR-31840] [GR-31681] Allow native-image shared libs to be built on module-path and fix resource bundle lookup. #3446

Merged
merged 25 commits into from
Jun 29, 2021

Conversation

olpaw
Copy link
Member

@olpaw olpaw commented Jun 2, 2021

  • Removes the undocumented native-image.properties property ImageIncludeBuiltinModules. It was previously needed for building truffle-based images. Cleaning up class-init for Java 11+ make the existence of ImageIncludeBuiltinModules obsolete. (I.e. if there is truffle, it's classes are subject to class-init)
  • Fix implementation of ResourceBundle access. Packages that contain ResourceBundles are opened up in their respective modules prior to accessing them from the image-builder. This also fixes JDK bundles are not discovered and added to native image with JDK16 #3404
  • Add support for building shared library images where the --module-path is used.
  • Build native-image-agent and native-image-diagnostics-agent via --module-path and test the shared objects in mx hellomodule

@olpaw olpaw linked an issue Jun 9, 2021 that may be closed by this pull request
@olpaw olpaw force-pushed the paw/GR-31840 branch 3 times, most recently from 1c29a4c to ea04d3c Compare June 21, 2021 14:52
@olpaw olpaw changed the title [GR-31840] Allow native-image shared libs to be built on module-path. [GR-31840] [GR-31681] Allow native-image shared libs to be built on module-path. Jun 21, 2021
@olpaw olpaw changed the title [GR-31840] [GR-31681] Allow native-image shared libs to be built on module-path. [GR-31840] [GR-31681] Allow native-image shared libs to be built on module-path and fix resource bundle lookup. Jun 21, 2021
@olpaw
Copy link
Member Author

olpaw commented Jun 21, 2021

@gilles-duboscq please have a look at the changes in sdk/mx.sdk/mx_sdk_vm_impl.py and sdk/mx.sdk/mx_sdk_vm.py if that makes sense to you.

sdk/mx.sdk/mx_sdk_vm.py Outdated Show resolved Hide resolved
sdk/mx.sdk/mx_sdk_vm_impl.py Outdated Show resolved Hide resolved
sdk/mx.sdk/mx_sdk_vm_impl.py Show resolved Hide resolved
@olpaw
Copy link
Member Author

olpaw commented Jun 23, 2021

Now that a461aa0 is on master we need 4580142 cc @gradinac

@olpaw olpaw force-pushed the paw/GR-31840 branch 2 times, most recently from 909cc97 to d130946 Compare June 24, 2021 09:29
@olpaw
Copy link
Member Author

olpaw commented Jun 25, 2021

7302e45 ... how silly of me, forgetting that not everything is in a java module ... yet 🤡

@graalvmbot graalvmbot merged commit d8d9146 into oracle:master Jun 29, 2021
zakkak added a commit to zakkak/mandrel-packaging that referenced this pull request Jun 30, 2021
oracle/graal@1e639c9
from oracle/graal#3446 introduces some more
dependencies on Truffle that Mandrel doesn't really depend on
zakkak added a commit to zakkak/mandrel-packaging that referenced this pull request Jun 30, 2021
…ation

oracle/graal@1e639c9
introduces some more dependencies on Truffle that Mandrel doesn't really
depend on

oracle/graal@d81403a
change the location of the jvmti-agent-base.jar

Both changes where part of oracle/graal#3446
zakkak added a commit to zakkak/mandrel-packaging that referenced this pull request Jun 30, 2021
…ation

oracle/graal@1e639c9
introduces some more dependencies on Truffle that Mandrel doesn't really
depend on

oracle/graal@d81403a
change the location of the jvmti-agent-base.jar

Both changes were part of oracle/graal#3446
zakkak added a commit to zakkak/mandrel-packaging that referenced this pull request Jun 30, 2021
…ation

oracle/graal@1e639c9
introduces some more dependencies on Truffle that Mandrel doesn't really
depend on

oracle/graal@d81403a
change the location of the jvmti-agent-base.jar

Both changes were part of oracle/graal#3446
zakkak added a commit to graalvm/mandrel-packaging that referenced this pull request Jun 30, 2021
…ation

oracle/graal@1e639c9
introduces some more dependencies on Truffle that Mandrel doesn't really
depend on

oracle/graal@d81403a
change the location of the jvmti-agent-base.jar

Both changes were part of oracle/graal#3446
}
String packageName = packageName(bundleName);
if (packageName == null) {
throw new MissingResourceException("ResourceBundle does not seem to be a fully qualified class name.", bundleName, locale.toLanguageTag());
Copy link
Collaborator

@zakkak zakkak Jul 1, 2021

Choose a reason for hiding this comment

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

Hi @olpaw ,

With this check in place jar files including resource bundles in .properties file format at their root are not supported. Such an example is org.eclipse.yasson (seen failing in quarkusio/quarkus#18305).

Going through ResourceBundle's Javadoc I was not able to find any mention/guidance on avoiding adding ResourceBundles to the root of jar files, so I am wondering if this is something we should relax on GraalVM.

Thanks

Copy link
Member Author

@olpaw olpaw Jul 1, 2021

Choose a reason for hiding this comment

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

@zakkak Yes we should probably add fallback code for that to support that. Please create an issue for me.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm I forgot to mention the issue here.
FWIW the created issue was #3535 and it got fixed by #3540

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JDK bundles are not discovered and added to native image with JDK16
6 participants