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

add GraalVM native image configuration files #21

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

remkop
Copy link

@remkop remkop commented Oct 23, 2019

This PR adds GraalVM native image configuration files. These configuration files allow applications that use JANSI to be compiled to native images.

This PR partly addresses fusesource/jansi#162 ; I raised another PR in the hawtjni project to address the Library issue.

For reference, see

The resource configuration allows the windows64/jansi.dll file to be included in the native image.

The intention is that these files end up in the jansi-$VERSION.jar file in the following locations:

/META-INF/native-image/jansi-native/jni-config.json
/META-INF/native-image/jansi-native/resource-config.json

@remkop
Copy link
Author

remkop commented Oct 24, 2019

The resource-config.json in the current PR only includes the win64 jansi.dll in the native image. It may be better to include the 64-bit libraries for all supported OS-es, to allow more general usage of the Jansi native code in GraalVM native images for Linux and MacOS as well as Windows. (Since GraalVM doesn’t support 32-bit, there’s no need to include the 32-bit libraries as resources in native images.)

I’ll update the PR.

@remkop
Copy link
Author

remkop commented Oct 24, 2019

On second thought, the current resource-config.json is the minimal configuration that solves the immediate problem. Applications can provide additional config files to include more native libraries if desired. Removing undesired resources, on the other hand, is more tricky. It may be better to leave the config as is, and only include the windows64/jansi.dll.

@hboutemy
Copy link
Collaborator

why support only Windows by default, and not other OS?
Linux or Mac are also very popular
I don't know what's the impact of having too much resources included, but perhaps the solution is to have no resource by default but provide a good documentation on how to add the right resource based on users native platform, isn't it?

@hboutemy
Copy link
Collaborator

on jni-config.json, is there a way to check that the content matches the library? to avoid issues in the future if the library is updated

@remkop
Copy link
Author

remkop commented Jun 19, 2020

@hboutemy Thanks for getting back!

I don't know what's the impact of having too much resources included, but perhaps the solution is to have no resource by default but provide a good documentation on how to add the right resource based on users native platform

Yes, that sounds reasonable.

why support only Windows by default, and not other OS? Linux or Mac are also very popular.

Interesting! What is the use case for Jansi on Linux and Mac? I thought on those OS's ANSI colors are supported out of the box.

on jni-config.json, is there a way to check that the content matches the library? to avoid issues in the future if the library is updated

Good point. Yes, every time the library is updated, the jni-config.json should be updated too, perhaps as part of the build. One idea is to invoke picocli.codegen.aot.graalvm.JniConfigGenerator that is included in picocli-codegen as part of the build. The below command (copied from the fusesource/jansi#162 ticket) shows how this can be done in a windows batch script, please adjust the syntax as needed for your build tool:

REM (on windows)

set PICOCLI_VERSION=4.3.2
set JANSI_VERSION=1.18

java -cp picocli-%PICOCLI_VERSION%.jar;jansi-%JANSI_VERSION%.jar;picocli-codegen-%PICOCLI_VERSION%.jar ^
  picocli.codegen.aot.graalvm.JniConfigGenerator ^
  org.fusesource.jansi.internal.CLibrary ^
  org.fusesource.jansi.internal.Kernel32 ^
  -o=.\jni-config.json

Then, ensure the file is included in the jansi JAR in the following location (any unique location under /META-INF/native-image is fine):

/META-INF/native-image/fusesource/jansi-native/jni-config.json

@gnodet
Copy link
Member

gnodet commented Oct 21, 2020

@hboutemy Thanks for getting back!

I don't know what's the impact of having too much resources included, but perhaps the solution is to have no resource by default but provide a good documentation on how to add the right resource based on users native platform

Yes, that sounds reasonable.

why support only Windows by default, and not other OS? Linux or Mac are also very popular.

Interesting! What is the use case for Jansi on Linux and Mac? I thought on those OS's ANSI colors are supported out of the box.

Jansi provides access to several low level functions that are useful when a java application needs to access the console. Those are the ones in the CLibrary and Kernel32 libraries exposed by jansi-native.

on jni-config.json, is there a way to check that the content matches the library? to avoid issues in the future if the library is updated

Good point. Yes, every time the library is updated, the jni-config.json should be updated too, perhaps as part of the build. One idea is to invoke picocli.codegen.aot.graalvm.JniConfigGenerator that is included in picocli-codegen as part of the build. The below command (copied from the fusesource/jansi#162 ticket) shows how this can be done in a windows batch script, please adjust the syntax as needed for your build tool:

REM (on windows)

set PICOCLI_VERSION=4.3.2
set JANSI_VERSION=1.18

java -cp picocli-%PICOCLI_VERSION%.jar;jansi-%JANSI_VERSION%.jar;picocli-codegen-%PICOCLI_VERSION%.jar ^
  picocli.codegen.aot.graalvm.JniConfigGenerator ^
  org.fusesource.jansi.internal.CLibrary ^
  org.fusesource.jansi.internal.Kernel32 ^
  -o=.\jni-config.json

Then, ensure the file is included in the jansi JAR in the following location (any unique location under /META-INF/native-image is fine):

/META-INF/native-image/fusesource/jansi-native/jni-config.json

I'd like to release a new version of Jansi. Can we leave that aside or do we need to consider fixing the problems before ?

@remkop
Copy link
Author

remkop commented Oct 23, 2020

@gnodet Thanks for getting back!

I'd like to release a new version of Jansi. Can we leave that aside or do we need to consider fixing the problems before ?

A new release, great! Sounds like an opportunity to fix the problem! :-)

jni-config

This is straightforward. You can either include the file from this PR, or, you can regenerate the jni-config.json file with the command shown in my previous comment. Regenerating is only needed if the CLibrary or Kernel32 API has changed since jansi-1.18.

Include the jni-config.json file in this location in the new jansi jar:

/META-INF/native-image/fusesource/jansi-native/jni-config.json

resource-config

Given your feedback, the simplest idea is to create a resource-config.json file that lists all 64-bit native libraries that are included in the jansi JAR. The contents of this file looks like this:

{
  "resources": [
    {"pattern": "META-INF/native/freebsd64/libjansi.so"},
    {"pattern": "META-INF/native/linux64/libjansi.so"},
    {"pattern": "META-INF/native/osx/libjansi.jnilib"},
    {"pattern": "META-INF/native/windows64/jansi.dll"}
  ]
}

Include this resource-config.json file in the following location in the new jansi jar:

/META-INF/native-image/fusesource/jansi-native/resource-config.json

What this does

The above two steps will allow applications to use the jansi functionality in GraalVM native images.

Including all native Jansi libs in the GraalVM native image is a bit overkill (ideally a MacOS native image would only include osx/libjansi.jnilib, and a Windows native image would only include windows64/jansi.dll, etc.), but these libs are not that big, and applications can customize the GraalVM native image with a separate exclusion config if they really really want to. Meanwhile, this works out of the box (nice for users developing apps with Jansi) and is easy to maintain (nice for the jansi maintainers).

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

Successfully merging this pull request may close these issues.

None yet

3 participants