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
Allow to install Presto RPM with Java 11 #2057
Conversation
With #2064 we could easily test this using a different Docker image that contains Java 11 |
@@ -22,7 +22,11 @@ check_if_correct_java_version() { | |||
# candidate for JAVA_HOME). | |||
JAVA_VERSION=$(java_version "$1") | |||
JAVA_UPDATE=$(echo $JAVA_VERSION | cut -d'_' -f2) | |||
if [[ ("$JAVA_VERSION" > "1.8") && ($JAVA_UPDATE -ge 161) ]]; then | |||
JAVA_MAJOR=$(echo $JAVA_VERSION | cut -d'.' -f1) | |||
if [[ ("$JAVA_MAJOR" -ge "11") ]]; then |
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.
what about other versions? Can we make the mechanism generic?
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 quite generic for example 12 > 11
. However then env variable JAVA11_HOME
has unfortunate name.
if [[ ("$JAVA_MAJOR" -ge "11") ]]; then | ||
echo "JAVA11_HOME=$1" > /tmp/presto_env.sh | ||
return 0 | ||
elif [[ ("$JAVA_VERSION" > "1.8") && ($JAVA_UPDATE -ge 161) ]]; then | ||
echo "JAVA8_HOME=$1" > /tmp/presto_env.sh |
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.
shouldn't we make this generic instead?
#2064 is merged, so please try adding a test for this in |
d72397a
to
af0931e
Compare
Locally:
|
Depends on trinodb/docker-images#41 |
@@ -40,6 +44,7 @@ if ! check_if_correct_java_version "$JAVA8_HOME" && ! check_if_correct_java_vers | |||
/usr/java/jdk1.8* \ | |||
/usr/java/jre1.8* \ | |||
/usr/jdk64/jdk1.8* \ | |||
/usr/lib/jvm/java-11-* \ |
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.
Should we put this first in the list so that it's found first?
af0931e
to
52e347b
Compare
Allow to install Presto RPM with Java 11