-
Notifications
You must be signed in to change notification settings - Fork 445
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
Build native images using GraalVM #1165
Conversation
I'd like feedback on the naming: the plugin name in particular - is it better for it to be GraalVM, or GraalVMNativeImage, or any other alternative? |
8870f16
to
97dc4c7
Compare
Will be there also a possibility to use GraalVM bundling, but inside a docker container (also using |
That is a great comment @dborisenko. I thought about it as a separate PR as it may need some abstraction of the Docker plugin. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is pretty awesome 😎
Is there any option to test this on Travis?
* GraalVM settings | ||
*/ | ||
trait GraalVMKeys { | ||
val nativeImageOptions = SettingKey[Seq[String]]("native-image-options", "native-image options") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a graal only setting, right? For package specific settings we simply prefix them with the package name. In this case graalVmNativeImageOptions
and don't scope them. This way they are clearly marked as format specific settings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed a native-image
specific setting.
override lazy val projectSettings = Seq( | ||
target in GraalVM := target.value / "graalvm", | ||
sourceDirectory in GraalVM := sourceDirectory.value, | ||
name in GraalVM := name.value, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The name setting is not normalized. Are there any restrictions for the image name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure what you mean by "normalized".
|
||
override lazy val projectSettings = Seq( | ||
target in GraalVM := target.value / "graalvm", | ||
sourceDirectory in GraalVM := sourceDirectory.value, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is the source directory required?
packageBin in GraalVM := { | ||
(target in GraalVM).value.mkdirs() | ||
val className = (mainClass in Compile).value.getOrElse(sys.error("Could not find a main class.")) | ||
val classpath = Seq((packageBin in Compile).value) ++ (dependencyClasspath in Compile).value.map(_.data) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Depending on packageBin and dependencyClasspath in compile is probably fine. If users want to build graal images in test scope we'll make this more flexible.
@muuki88 great review, thank you! For Travis: yes, it is possible. This would require a download of the GraalVM distribution. I can make that happen. |
You are welcome 🤗 Running this on Travis would awesome, because I'll be able to merge PR directly from my phone ( as I am doing now ) without worrying that things are being terribly broken. If you need any help on the Travis file, don't hesitate to ask. I spent quite some time figuring this out 😂 |
a285430
to
a242081
Compare
@muuki88 for some reason Travis is not even starting to run the tests any more (is it because I modified the travis yml?). Can you take a look at the code again. I believe I improved the naming. I made the name of the plug-in longer, so we don't have conflict with any potential future GraalVM plugin (graalvm is much more than native-image). |
I wrote the Travis change to the best of my experience, but can't be sure it'll work until it runs ... :-) |
Let's give Travis some time. I found that sbt-native- packager often hits some rate limits. I'll check as soon as I'm on my laptop of I find anything suspicious |
79d0568
to
e968271
Compare
Meh. Really seems if there's a small error in the Travis file. It looks rather correct to me though 🤔 |
a50c59f
to
6da8701
Compare
Somehow, pushing it again reactivated Travis :-) |
Looks like it fails. Does not seem to pick up the command. |
.travis.yml
Outdated
fi; | ||
mkdir -p ~/.bin; | ||
ln -f -s ~/graalvm-ce-1.0.0-rc7/bin/native-image ~/.bin/native-image; | ||
export PATH=~/.bin:$PATH; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIRC Travis has an env variable for the current path. The path should absolute. We had the exact same issue on our company Jenkins yesterday
Once again Travis needs some help 😣 |
b65a931
to
0d0590c
Compare
.travis.yml
Outdated
# Make GraalVM `native-image` available and nothing else | ||
- if [[ ! -d graalvm-ce-1.0.0-rc7 ]]; then | ||
cd $HOME; | ||
wget -O graalvm-ce-1.0.0-rc7-linux-amd64.tar.gz https://github.com/oracle/graal/releases/download/vm-1.0.0-rc7/graalvm-ce-1.0.0-rc7-linux-amd64.tar.gz; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about Oracle’s licensing. Are we permitted to do this, or should Travis’s infra provide it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a very good question @huntc 🤔 thanks for bringing this up 🤗😀
I have no source to ask for legal advice like this. I'll try to do some research. Any hint is appreciated here 😘
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The community edition
is being used, which is at least free of fees.
There is a community edition (CE) and an enterprise edition (EE) of GraalVM.
The community edition is distributed under an open source license. It is free
to use in production and comes with no strings
attached, but also no guarantees or support
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems that license acceptance is required: https://www.oracle.com/technetwork/oracle-labs/program-languages/downloads/index.html
I’d consult the Travis team and/or request specific permission from Oracle.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ScalaWilliams having technical and legals blockers here I vote for postponing the Travis integration into a separate pull request.
Sorry for the back and forth 😣
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's for Enterprise Edition, @huntc. Notice the "ee" in the .gz
download name after you accept. Of course it's healthy to be wary of Oracle.
Community Edition is as follows:
https://github.com/oracle/graal#license
b6e1121
to
e16074e
Compare
I strengthened the test and removed Travis bits @muuki88. I've put the Travis bits in a separate branch here: https://github.com/ScalaWilliam/sbt-native-packager/tree/travis-graal |
Anything else before the merge? :-) |
Nope 😀 Was busy getting 30 today 🤣 I'll comment with a bit more details on how we could move forward regarding comments on twitter that a docker Integration would be nice. Will do so on Tuesday |
Regarding Docker integration: actually, this would apply to RPM as well as Universal as well as Debian. In all these cases, we can package up the native binary and distribute it. RPM/Debian + native:
Docker + native:
|
In the meanwhile, I'm wondering when you'd intend to release the next version of sbt-native-packager? It's good to get feedback from the community regarding the use cases above, especially with them trying out the native image functionality. |
& a big thanks for your excellent review & help in merging this. Look forward to contributing more where appropriate. |
Basically yes to all you just wrote 😍👍 all in one archetype plugin. Can I interest you in commit rights for native-packager 🤗 |
I'll try to release this tomorrow |
Release is on its way |
@muuki88 commit rights sound excellent! Thank you :-) |
Sweet ❤️ Welcome on board. You can release simply by pushing a signed that with the version you want to release, e.g. |
As per #1123.
This allows us to use GraalVM's ecosystem and create native binaries straight from SBT. High performance native binaries, native interop with many programming languages (JavaScript, Python, Ruby, R), .... Especially R and Python, super important for data science.