-
Notifications
You must be signed in to change notification settings - Fork 93
Use AdoptOpenJDK JDK 8 and 11 #334
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
Conversation
f20da4a to
6842ed2
Compare
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.
Could you explain this line? Thanks!
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.
JAVA_HOME is set by Travis CI's base image or language: scala to be whatevet the JDK, like JDK 11 for xenial. since I'm installing a new JDK I don't want the java on PATH and JAVA_HOME to be not inconsistent. This normally only matters when there's forking or Java only project. Alternatively I could try to do which java and set JAVA_HOME to wherever SDKMAN installs AdoptOpenJDK JDK.
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.
OK, so sdk install adds the installed version to the path, but doesn't set JAVA_HOME. I'm fine with unsetting if we don't need it. For the record, jabba use sets both the PATH and JAVA_HOME.
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.
lgtm, can be squashed.
529d32c to
d90955e
Compare
Squashed.
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.
Hmm, looking at the validation of this PR, I see that travis is not actually enabled on this repo. It seems to use circle CI...
@lrytz lol. good catch. I saw those too but somehow I didn't put 2 and 2 together.
I guess I'd use adoptopenjdk/openjdk8 or eed3si9n/sbt:jdk8-alpine Docker image.
@ashawley I am not familiar with CircleCI, but I am guessing that creating a custom image of a drop-in replacement of the current image using AdoptOpenJDK is going to be some work, so what do you think about switching back to Travis CI for now?
I haven't touched TravisCI. It should still be enabled for the repo. Is there an issue with Travis? Is there a syntax error in your yaml file? The travis UI changed recently and you have to find those errors from the requests page from under the hamburger menu.
Interesting. We noticed it's no longer running, but maybe you're right about it potentially being an YAML error etc. The last known build on Travis CI was from this PR yesterday - https://travis-ci.org/scala/scala-xml/builds/556385195/config
d90955e to
2998e29
Compare
@ashawley you were totally right about YAML syntax.
Looks like it ran on Travis CI fine (in addition to Circle CI), so I am going to self-merge.
Yeah, I've ever been bit by both the YAML syntax problem, and the hidden error problem in Travis.
👍 funny episode
No description provided.