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 package Script for Graal #1950

Closed
wants to merge 2 commits into from

Conversation

madoar
Copy link
Collaborator

@madoar madoar commented May 25, 2019

This PR tries to fix the packaging script to work with Graal.js.
When executing the script I currently receive the following error message:

Illegal argument [--java-options -XX:+UnlockExperimentalVMOptions --java-options -XX:+EnableJVMCI --java-options -classpath /home/marc/git/POL-POM-5/phoenicis-dist/src/scripts/../../target/lib --java-options --upgrade-module-path=/home/marc/git/POL-POM-5/phoenicis-dist/src/scripts/../../target/lib/compiler.jar]
WARNING: argument [linux-bundle-name] is not supported for current configuration.

@qparis @plata any ideas?

@plata
Copy link
Collaborator

plata commented May 25, 2019

No idea currently (except for searching on the Internet but that's obvious...).

@plata
Copy link
Collaborator

plata commented May 31, 2019

Just tried in Gitpod. I get:

./phoenicis-create-package.sh: line 102: fakeroot: command not found

@plata
Copy link
Collaborator

plata commented May 31, 2019

Maybe this article is helpful: https://medium.com/@adam_carroll/java-packager-with-jdk11-31b3d620f4a8. It links to a script which runs the packager: https://github.com/Santulator/Santulator/blob/1.1.0/package/bin/build-package.sh.
Especially relevant seem to be the options --module-path and --jvm-args. Note that the script is using create-installer instead of create-image.

As an alternative, we could might consider GraalVM Native Image as described here.

@madoar
Copy link
Collaborator Author

madoar commented Jun 1, 2019

Maybe this article is helpful: https://medium.com/@adam_carroll/java-packager-with-jdk11-31b3d620f4a8. It links to a script which runs the packager: https://github.com/Santulator/Santulator/blob/1.1.0/package/bin/build-package.sh.

The explained script in the article doesn't require runtime arguments like we do with Graal. It also does not require --upgrade-module-path. The article does not explain how to set either of these options.

Especially relevant seem to be the options --module-path and --jvm-args. Note that the script is using create-installer instead of create-image.

I have no clue what --jvm-args does... According to the corresponding JEP 343 this argument doesn't exist...

As an alternative, we could might consider GraalVM Native Image as described here.

GraalVM would be an alternative that should definitively work with Graal.js. Sadly GraalVM doesn't support JavaFX yet (see also here), which removes it from the list currently.

@madoar
Copy link
Collaborator Author

madoar commented Jun 1, 2019

I'm currently not sure which direction we should take for creating an installer for Phoenicis. The issue is that the whole Java ecosystem seems to be broker currently in regards to creating system dependent installers. Starting with the introduction of the module system in Java 9 and the removal from JavaFX from the main repository it became kind of unclear how to create an installer. The current choices I see (each with their own disadvantages) are:

  • "the old way": i.e. using a maven plugin for each installer type. We did that previously. I think that this solution will most likely stop to work in one of the next Java versions with the availability to build your own JRE and the coming of GraalVM.
  • javapackager: javapackager (jpackage) has the big disadvantage that it still has not been released it seems to be scheduled to be released with Java 13, which will still take around half a year until it has been released. For this reason it is also hard to find more extensive examples how to use the tool.
  • GraalVM native images: similar to javapackager this has the disadvantage that it has not been released as of now. I would even say that it is further from a release than javapackager. In addition I don't know whether the GraalVM native image approach supports loading scripts at runtime, which we absolutely require. Again we have nearly zero documentation and examples for this solution.

All solutions have an in my opinion huge risk attached. The most stable and tested solution is the first one, while the other two are kind of new solution.

I know this comment is more of a rant from my part but currently I can't really recommend building any kind of installer for/with Java because the current technologies just don't seem mature enough.

@qparis
Copy link
Member

qparis commented Jun 1, 2019

@madoar See pull request #1678

@qparis
Copy link
Member

qparis commented Jun 1, 2019

At the end, I think that the package script should more look like this:

PHOENICIS_MODULES="jdk.crypto.ec,java.base,javafx.base,javafx.web,javafx.media,javafx.graphics,javafx.controls,java.naming,java.sql,java.scripting,jdk.scripting.nashorn,jdk.internal.vm.ci"
PHOENICIS_RUNTIME_OPTIONS="-XX:+UnlockExperimentalVMOptions -XX:+EnableJVMCI --upgrade-module-path=../Java/compiler.jar"
PHOENICIS_JPACKAGER_ARGUMENTS=("-i" "$PHOENICIS_TARGET/lib" "--main-jar" "phoenicis-javafx-$VERSION.jar" "-n" "$PHOENICIS_APPTITLE" "--output" "$PHOENICIS_TARGET/packages/" "--add-modules" "$PHOENICIS_MODULES" "-p" "$PHOENICIS_TARGET/lib/" "--version" "$VERSION" "--jvm-args" "$PHOENICIS_RUNTIME_OPTIONS")

@qparis
Copy link
Member

qparis commented Jun 1, 2019

Also, you may need to exclude jackson from org.mock-server dependency (phoenicis-tools/pom.xml)

<exclusion>
                    <!-- Jackson is already included by Phoenicis -->
                    <artifactId>jackson-core</artifactId>
                    <groupId>com.fasterxml.jackson.core</groupId>
                </exclusion>

@plata
Copy link
Collaborator

plata commented Jun 1, 2019

@qparis can you try if your proposed solution works?

@qparis
Copy link
Member

qparis commented Jun 1, 2019

GraalVM merge request needs to be merged first

@plata
Copy link
Collaborator

plata commented Jun 1, 2019

I don't think we can merge the other PR if the packaging does not work (which is also why this PR is vs the graalvm branch).

@qparis
Copy link
Member

qparis commented Jun 1, 2019

I have tried to use the branch, but I don't understand how it works. It seems that we have some old packaging code, and at the end, I end up with the following error:

Advice to fix: Please specify a applicationClass or ensure that the jar RelativeFileSet{basedir:/Users/qparis/Documents/Developement/git/phoenicis/phoenicis-dist/src/scripts/../../target/lib, files:[phoenicis-javafx-5.0-SNAPSHOT.jar]} specifies one in the manifest.

@plata
Copy link
Collaborator

plata commented Jun 1, 2019

I've just merged master again and the PR does not show any differences in the packaging script. So the packaging code should really be the same as on master (for the graalvm branch).

@qparis
Copy link
Member

qparis commented Jun 1, 2019

Strange, because a Ubuntu deb file is generated when I run package

@plata
Copy link
Collaborator

plata commented Jun 1, 2019

Does Phoenicis work if you try to run it from that deb (especially scripts)?

@qparis
Copy link
Member

qparis commented Jun 1, 2019

I'm not testing on Linux, but this deb is not standalone

@plata
Copy link
Collaborator

plata commented Jun 1, 2019

I thought the deb is only build on Debian/Ubuntu?

@qparis
Copy link
Member

qparis commented Jun 1, 2019

With jpackager, yes. Not with maven package plugin. This is why I am suspecting that we have old code in the branch

@plata
Copy link
Collaborator

plata commented Jun 1, 2019

That's strange. I cannot find anything like that in phoenicis-dist/pom.xml.

@qparis
Copy link
Member

qparis commented Jun 1, 2019

Hmm it was my mistake. My local branch was not fully up-to-date

@qparis
Copy link
Member

qparis commented Jun 1, 2019

image

So it almost works

@qparis
Copy link
Member

qparis commented Jun 1, 2019

Seems that I made it work fairly easily. I'm opening a merge request.

Good job

@plata
Copy link
Collaborator

plata commented Jun 2, 2019

Superseded by #1957.

@plata plata closed this Jun 2, 2019
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.

3 participants