Skip to content
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

Merged
merged 8 commits into from
May 9, 2016

Conversation

eregon
Copy link
Member

@eregon eregon commented Apr 29, 2016

[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

@eregon eregon added the truffle label Apr 29, 2016
@eregon eregon added this to the truffle-dev milestone Apr 29, 2016
@eregon
Copy link
Member Author

eregon commented Apr 29, 2016

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) {
Copy link
Member

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 ?

Copy link
Member Author

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?

Copy link
Member Author

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.

Copy link
Member Author

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!

@eregon
Copy link
Member Author

eregon commented May 4, 2016

@headius @enebo @kares @mkristian Would you be OK with this?
This would make the truffle module (in Maven terms) Java 8+.
One can easily exclude it if they want to build/test with Java 7 (as the CI does).

For distribution, I do not know what is the current process. Is a Java 7 javac used? If that's the case then everything but truffle should be compiled that way and then switch to a Java 8 javac to compile truffle.

@eregon
Copy link
Member Author

eregon commented May 4, 2016

I added another commit which enables the truffle profile and module only when using a JDK 8 or newer. This allows a user with a default jdk 7 to still be able to build core JRuby without any change.
For distribution, we of course want the truffle module to be included, so -Ptruffle should be specified to guarantee it's built (or it will error out).

@eregon eregon force-pushed the truffle_java8 branch 4 times, most recently from a1e63c0 to f65fe5e Compare May 4, 2016 15:30
@enebo
Copy link
Member

enebo commented May 4, 2016

Thanks. I would like to keep building core with 7 since then we catch 8
APIs slipping in.
On May 4, 2016 9:00 AM, "Benoit Daloze" notifications@github.com wrote:

I added another commit which enables the truffle profile and module only
when using a JDK 8 or newer. This allows the user with a default jdk 7
still be able to build core JRuby without any change.
For distribution, we of course want the truffle module to be included, so
-Ptruffle should be specified to guarantee it's built (or it will error
out).


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#3836 (comment)

@eregon
Copy link
Member Author

eregon commented May 4, 2016

@enebo Tavis keeps building with 7 as before for most of the tasks.

Do you mean for the distribution?
It can be done although it's easier with a Java 8 and Travis should already catch missing APIs. Something like this seems to work quite nicely (package should be replaced by the appropriate phase of course):

switch to java7
./mvnw package
switch to java8
./mvnw package -pl truffle

@enebo
Copy link
Member

enebo commented May 4, 2016

I am ok with using 8 for dist so long as Travis and I can routinely build
core with 7. Sounds like you got it covered.
On May 4, 2016 2:51 PM, "Benoit Daloze" notifications@github.com wrote:

@enebo https://github.com/enebo Tavis keeps building with 7 as before
for most of the tasks.

Do you mean for the distribution?
It can be done although it's easier with a Java 8 and Travis should
already catch missing APIs. Something like this seems to work quite nicely (
package should be replaced by the appropriate phase of course):

switch to java7
./mvnw package
switch to java8
./mvnw package -pl truffle


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#3836 (comment)

@eregon
Copy link
Member Author

eregon commented May 4, 2016

@enebo Yep, that should work transparently since the truffle module is only considered if there is a Java 8 😃

@eregon
Copy link
Member Author

eregon commented May 9, 2016

@headius @enebo I plan to merge this today. Please raise your voice if there is any objection!

@eregon
Copy link
Member Author

eregon commented May 9, 2016

(the build is green on Travis BTW, just failing on Windows as it already was)

@eregon eregon merged commit 85abff7 into jruby:master May 9, 2016
@eregon eregon deleted the truffle_java8 branch May 9, 2016 15:49
@enebo enebo added this to the Non-Release milestone Dec 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants