search

beehive-lab / TornadoVM

TornadoVM: A practical and efficient heterogeneous programming framework for managed languages
https://www.tornadovm.org
Apache License 2.0
1.18k stars 113 forks source link

Fix Typo in Installer Script #248

Closed fkellner closed 1 year ago

fkellner commented 1 year ago

Description

Fixes a typo in a condition in the installer script preventing the installation with a custom OpenJDK whose path does not start with "graal"

Problem description

Running ./scripts/tornadovm-installer --help states:

If you want to select another version of OpenJDK, you can use --javaHome="<pathToJavaHome>" and install as follows:
      $ ./scripts/tornadovm-installer --backend <BACKEND> --javaHome=<pathToJavaHome>

Enter a <pathToJavaHome> that does not start with "graal" and the script tries to access args.jdk.startswith which throws an error because it is None.

Backend/s tested

Does not apply, just a line in a python script

How to test the new patch?

/scripts/tornadovm-installer --backend <BACKEND> --javaHome=<pathToJavaHome>

with a <pathToJavaHome> that does not start with "graal".

fkellner commented 1 year ago

Also, find is probably a better choice than startswith because users could enter an absolute path for the javaHome parameter

jjfumero commented 1 year ago

Hi @fkellner . Good catch. Thank you for the fix. Currently, the University of Manchester is creating a contributors agreement for all its open-source projects for collaborators outside the University. Thus, we will need until this agreement is in place in order to process new PRs. Apologies for this inconvenience, but unfortunately it is out of our (TornadoVM team) hands.

If you agree, we can apply the patch ourselves in the meantime.

jjfumero commented 1 year ago

BTW, the patch looks good to me.

jjfumero commented 1 year ago

This has been fixed: https://github.com/beehive-lab/TornadoVM/commit/5a93a7ceb6003a0f6aa719d8000e6f6fd5e8a5a4