-
Notifications
You must be signed in to change notification settings - Fork 443
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
fixed Java version check in bash template #1111
Conversation
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.
Thanks a lot for the well done pull request 😍
The newly written test seems to fail due to the doc
task.
[info] [error] 'jvm-1.8' is not a valid choice for '-target'
[info] [error] bad option: '-target:jvm-1.8'
[info] [error] 'jvm-1.8' is not a valid choice for '-target'
[info] [error] bad option: '-target:jvm-1.8'
[info] [error] (jdk8/compile:doc) Scaladoc generation failed
[info] [error] (jdk8/compile:compileIncremental) Compilation failed
Otherwise everything looks super smooth :)
@@ -0,0 +1,31 @@ | |||
val basename = "jdk-versions" | |||
|
|||
scalacOptions in ThisBuild := Seq("-target:jvm-1.8") |
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.
it seems the scala doc compiler doesn't like this setting. I never actually fought
the docs compiler (only my colleagues), so sorry for not being a greater help here 😞
IIRC you can either turn the scaladoc generation off or do some research on how
to fix this 😎 I'm fine with either way 👍
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.
Thanks for pointing this out. Will push a fix. Normally limiting the scope should fix this.
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.
Awesome, thanks :)
Hi @jubecker, Thank you for your contribution! We really value the time you've taken to put this together. Before we proceed with reviewing this pull request, please sign the Lightbend Contributors License Agreement: |
Awesome :) |
It includes a fix for this bug: sbt/sbt-native-packager#1111
This brings in sbt/sbt-native-packager#1111 which fixes #443
Summary: Wesley hit the bug mentioned in sbt/sbt-native-packager#1111 while running on a Amazon Linux 2 image. Upgrading the packaging plugin sbt-native-packager to a later version fixes the issue Test Plan: Build with this change, deploy to an Amazon Linux 2 imaged c5.large, verify that yugabyted comes up normally Reviewers: ram, wesley Reviewed By: wesley Subscribers: jenkins-bot, yugaware Differential Revision: https://phabricator.dev.yugabyte.com/D7683
The current Java version check in 'bash-template' relies on a string comparison for numbers.
As far as I could investigate this check fails at least for bash 4.3.48 on a xenial/sid based docker images.
It fails for the latest JDK release 10, as "10" is not greater than "1.6" when compared as string in bash.
Later bash version seem to treat this check differently.
I changed the version check to compare numbers. For versions 1.x.y, the check extracts the minor version "x" and compares agains "6". If the major version is unequal to "1", the major version is compared against "6".
Using docker openjdk images I added scripted test.
Tested with bash 4.3.48 and 4.4.19, docker 17.12.0-ce.