-
-
Notifications
You must be signed in to change notification settings - Fork 925
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
[Truffle] Experiment: try to make the truffle module Java 8+ #3836
Conversation
This also shaves 1h-2h from the total Travis time due to building truffle only for truffle tests and less maven work. |
@@ -912,6 +912,13 @@ public JRubyTruffleInterface getTruffleContext() { | |||
private JRubyTruffleInterface loadTruffle() { | |||
Main.printTruffleTimeMetric("before-load-context"); | |||
|
|||
String javaVersion = System.getProperty("java.version"); | |||
String[] parts = javaVersion.split("\\."); | |||
if (Integer.valueOf(parts[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.
wasn't Java 9 switching from '1.8' to '9.0' java.version format ?
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.
@kares Yeah I was wondering as well, do you have any link?
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.
http://openjdk.java.net/jeps/223 seems to be the proposed change.
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.
jjs> java.lang.System.getProperty("java.version")
9-ea
Indeed, thanks for the catch!
@headius @enebo @kares @mkristian Would you be OK with this? For distribution, I do not know what is the current process. Is a Java 7 |
I added another commit which enables the |
a1e63c0
to
f65fe5e
Compare
Thanks. I would like to keep building core with 7 since then we catch 8
|
@enebo Tavis keeps building with 7 as before for most of the tasks. Do you mean for the distribution? switch to java7
./mvnw package
switch to java8
./mvnw package -pl truffle |
I am ok with using 8 for dist so long as Travis and I can routinely build
|
@enebo Yep, that should work transparently since the truffle module is only considered if there is a Java 8 😃 |
(the build is green on Travis BTW, just failing on Windows as it already was) |
…t's already implied by using PHASE.
…en will complain ...
[Work in progress, do not merge]
This PR intends to evaluate the effect of having Java compatibility
for JRuby at 1.7 and for JRuby+Truffle at 1.8.
cc @jruby/truffle