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

fix APIUtil.apiGetManifestValue() for GraalVM native image build #644

Closed
wants to merge 1 commit into from
Closed

fix APIUtil.apiGetManifestValue() for GraalVM native image build #644

wants to merge 1 commit into from

Conversation

chirontt
Copy link

The APIUtil.apiGetManifestValue() method doesn't work in a native image
generated by GraalVM, as ClassLoader.getResource() doesn't return a URL
starting with "jar:" as expected when running in standard JVM
environment. Instead, in native image which is run by an embedded
SubstrateVM, ClassLoader.getResource() returns a URL starting with
"resource:", for all resources in the classpath.

This fix would make APIUtil.apiGetManifestValue() work in both standard
JVM as well as in native image generated by GraalVM.

The APIUtil.apiGetManifestValue() method doesn't work in a native image
generated by GraalVM, as ClassLoader.getResource() doesn't return a URL
starting with "jar:" as expected when running in standard JVM
environment. Instead, in native image which is run by an embedded
SubstrateVM, ClassLoader.getResource() returns a URL starting with
"resource:", for all resources in the classpath.

This fix would make APIUtil.apiGetManifestValue() work in both standard
JVM as well as in native image generated by GraalVM.
@Spasi Spasi added the Type: Bug label Sep 4, 2021
@Spasi Spasi closed this in f825d58 Sep 4, 2021
@Spasi
Copy link
Member

Spasi commented Sep 4, 2021

Hey @chirontt, thanks for the comprehensive code!

I don't remember why we've been manually loading the manifest, instead of simply using Package::getImplementationVersion, but since apiGetManifestValue isn't used for something else anymore, I've decided to remove it. Your implementation will certainly be useful if we ever need more though.

@chirontt chirontt deleted the fix_apiGetManifestValue_for_graal_native_image branch September 7, 2021 23:02
Spasi pushed a commit to sormuras/lwjgl3 that referenced this pull request Aug 24, 2022
- Package::getImplementationVersion is unspecified when loading a JAR
  file from the module path. The new multi-release code uses the module
  descriptor to retrieve the implementation version.
- Restored code that retrieves the implementation version from the JAR
  manifest, as a fallback. Also added the suggested fix from LWJGL#644, when
  running on a GraalVM native image.
- Changed the LWJGL version string to match the Java version format.

Close LWJGL#770

Co-authored-by: Leon Linhart <themrmilchmann@gmail.com>
Spasi pushed a commit to sormuras/lwjgl3 that referenced this pull request Aug 24, 2022
- Package::getImplementationVersion is unspecified when loading a JAR
  file from the module path. The new multi-release code uses the module
  descriptor to retrieve the implementation version.
- Restored code that retrieves the implementation version from the JAR
  manifest, as a fallback. Also added the suggested fix from LWJGL#644, when
  running on a GraalVM native image.
- Changed the LWJGL version string to match the Java version format.

Close LWJGL#770

Co-authored-by: Leon Linhart <themrmilchmann@gmail.com>
Spasi pushed a commit that referenced this pull request Aug 24, 2022
- Package::getImplementationVersion is unspecified when loading a JAR
  file from the module path. The new multi-release code uses the module
  descriptor to retrieve the implementation version.
- Restored code that retrieves the implementation version from the JAR
  manifest, as a fallback. Also added the suggested fix from #644, when
  running on a GraalVM native image.
- Changed the LWJGL version string to match the Java version format.

Close #770

Co-authored-by: Leon Linhart <themrmilchmann@gmail.com>
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.

None yet

2 participants