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

Move to Java 17 as runtime JVM #983

Closed
akurtakov opened this issue Jun 1, 2022 · 28 comments · Fixed by #1365
Closed

Move to Java 17 as runtime JVM #983

akurtakov opened this issue Jun 1, 2022 · 28 comments · Fixed by #1365
Labels
Milestone

Comments

@akurtakov
Copy link
Member

Java 17 is the latest LTS and new major version of Tycho is in the works. It's the perfect time to do this switch too.

@akurtakov
Copy link
Member Author

Switch in Tycho will be delayed till 2022-06 release and happen right after that so we'll be taking this with priority after opening 4.25 stream. This would not affect produced artifacts as they are built compatible according to their BREE .

@merks
Copy link
Contributor

merks commented Jun 2, 2022

Your cross projects email and the referenced issue suggest this switch was necessary for some "latest fixes" and for "Java 19 to work". I'm curious which latest fixes are needed and why Java 19 specifically won't work without requiring Java 17 in Tycho.

@mickaelistria
Copy link
Contributor

Building with BREE JavaSE-19 will be available in Tycho 3.0. Tycho 3.0 will soon require Java 17 to run.

@akurtakov
Copy link
Member Author

Tycho ships EE profile for Java versions. Java 19 profile will be added in Tycho 3.0. There is agreement among Tycho developers that we want to require Java 17 for our main version as we don't plan having new major release for Java 17 soon after 3.0.

@akurtakov
Copy link
Member Author

For the record https://github.com/eclipse/tycho/blob/master/CONTRIBUTING.md#increasing-versions clearly describes the cases which are considered breaking changes and Tycho should bump its major version.

@merks
Copy link
Contributor

merks commented Jun 2, 2022

Ah, I see. So you expect to need/want Java 17 soon and switching to that would require a major version increment so you might as well do this as part of your 3.0 version switch. Thanks for the details and background.

@laeubi
Copy link
Member

laeubi commented Jun 28, 2022

Tycho ships EE profile for Java versions. Java 19 profile will be added in Tycho 3.0.

Just to mention that this is no longer a problem as tycho do no longer include any profiles.

@akurtakov
Copy link
Member Author

A (possible) blockers on JDT side seems to be eclipse-jdt/eclipse.jdt.core#147 and eclipse-jdt/eclipse.jdt.core#114 . This makes it risky to require Java 17 as JVM if there are so many bundles targetting Java 11 and thus risking to have wrong bytecode generated.

@laeubi
Copy link
Member

laeubi commented Jul 6, 2022

This is a blocker for Tycho: eclipse-jdt/eclipse.jdt.core#183

@akurtakov
Copy link
Member Author

All blockers coming from jdt.core are done now. I plan to move to Java 17 ASAP.

@laeubi
Copy link
Member

laeubi commented Sep 15, 2022

@laeubi
Copy link
Member

laeubi commented Sep 15, 2022

All blockers coming from jdt.core are done now. I plan to move to Java 17 ASAP.

I must confess it feels still a bit risky for me to require Java 17 just because we can without an immediate need (e.g. a library is only available for java 17) given the major impact on all users + build systems...

@akurtakov
Copy link
Member Author

By risk you mean users sticking to 2.7 and not moving to 3.0 or actual technical issues ?

@laeubi
Copy link
Member

laeubi commented Sep 15, 2022

By risk you mean users sticking to 2.7 and not moving to 3.0 or actual technical issues ?

Both while the later I'm more concerned about as once we moved forward its hard to go back, and given that in the last release some serve Java 17 issues are discovered in JDT it might be better give it some more time, I assume more and more projects are moving forward and thus we will maybe discover more issues in the 2022-12 cycle. And if platform itself moves to java 17 we must move Tycho also anyways... So given this might happen with 2022-12 we might then start with a fresh Tycho 4.x in the next year?

@akurtakov
Copy link
Member Author

In the past Tycho was one of the driving force to Java 11 adoption rather than waiting for others. I hoped to keep that state. Let's have a vote on that. I'll start in on the mailing list.

@laeubi
Copy link
Member

laeubi commented Sep 15, 2022

In the past Tycho was one of the driving force to Java 11 adoption rather than waiting for others.

I would also be fine with a Poll on the internal comiter group on github but maybe this would require a vote on the mailinglist first :-)

@HannesWell
Copy link
Member

While I think it is good to move forward and push instead of being pulled I also agree that we should be relatively sure that there are no sever technical issues, especially since Java 17 has no absolutely required feature (as far as I know, nevertheless it has many benefits!)
On the other hand if we now release Tycho 3 at Java 11 we maybe have Tycho 4 with Java 17 not too far in the future (when Eclipse JDT/Platform require Java 17) and then maybe not much later have Tycho 5 targeting Maven 4. No clue when Maven 4 will be finally available but I read that the Maven deps hope to have an Alpha-1 for Maven 4 by the end of this year. Maybe the GA release will need another year. Btw. for Maven 4 there is also a discussion to require Java-17 (just in case not everybody noticed):
https://www.mail-archive.com/dev@maven.apache.org/msg127434.html

However, I think we should not burn too many major versions and therefore think it would be good to have at least two of those changes in one major of Tycho. I guess that Maven 4 will take the longest and we cannot postpone requiring Java-17 until Maven 4 is available. So it would be good to move to Java-17 with Tycho 3.

But if there are still things to address before Tycho moves to Java-17, wouldn't it be possible to postpone the Tycho 3.0 release a bit further? Or is there a hard requirement to release it end of September?

@mickaelistria
Copy link
Contributor

#951 seems to be invalid as per ECJ behavior requiring to run a Java 17 or newer in order to target Java 17 (to be confirmed on the issue)
#809 is not a related to adopting Java 17, the issue seems reproducible with Java 11.

@laeubi laeubi added the Java 17 label Sep 18, 2022
@laeubi
Copy link
Member

laeubi commented Sep 18, 2022

Another (blocking) issue regarding Java 17 as it seems compiling with Java 11 fail under some circumstances when running with Java 17:

@akurtakov
Copy link
Member Author

We should come to an agreement what "blocking" means. This issue popped up from the patch trying to compile targetting Java-17 while running Java-11 which IMHO shouldn't be blocking.

@laeubi
Copy link
Member

laeubi commented Sep 18, 2022

No this is the other way round, the build is running with java 17 (what we will enforce when using Java 17 as runtime requirement), while platform try compiling code for Java 11 with useJDK=BREE it is just that previously useJDK=BREE obviously has never did what is claims and has used the running JVM instead...

@mickaelistria
Copy link
Contributor

I think we can revert former patch.
But this story is not at all a blocker for making Java 17 a requirement. Similar issue is/was probably happening when targetting Java 11 while running build on Java 12.

@laeubi
Copy link
Member

laeubi commented Sep 18, 2022

I think we can revert former patch.

I'm already working on a patch that fixes that so I thin no need to add more noise to the git-history...

But this story is not at all a blocker for making Java 17 a requirement.

Actually it revealed that useJDK=BREE is/was broken since 9+, what might not be noticed as in most cases people either use Java 8 (where it worked) or Java 11+, but if we really switch to Java 17 this should work first.

@mickaelistria
Copy link
Contributor

mickaelistria commented Sep 18, 2022 via email

@laeubi
Copy link
Member

laeubi commented Sep 18, 2022

@mickaelistria actually we have had multiple issues:

  1. jdk=BREE not working as expected (either X > Y or Y < X) and actually using the build vm with target
  2. fork not working
  3. when Tycho would move to Java 17, then 1+2 would be required if compiling to "older" (e.g. java 11) what will be a major use-case and should be supported

I think #1356 will fix 1+2 and then 3 might be not an issue anymore.

@mickaelistria
Copy link
Contributor

jdk=BREE not working as expected (either X > Y or Y < X) and actually using the build vm with target

That shouldn't be a big deal with ECJ, it is something ECJ should be capable of doing without using a different version/jdk, unlike javac.

@laeubi
Copy link
Member

laeubi commented Sep 18, 2022

Technically you can use a striped down version of the JVM, so if we never pass that JVM to ECJ how should it know? Hopefully Tycho will already catch this but at least for javac it seem to produce slight different bytecodes when using a "real" Java versus a --release java

https://www.morling.dev/blog/bytebuffer-and-the-dreaded-nosuchmethoderror/

akurtakov added a commit to akurtakov/tycho that referenced this issue Sep 19, 2022
akurtakov added a commit to akurtakov/tycho that referenced this issue Sep 19, 2022
akurtakov added a commit to akurtakov/tycho that referenced this issue Sep 19, 2022
akurtakov added a commit to akurtakov/tycho that referenced this issue Sep 19, 2022
akurtakov added a commit to akurtakov/tycho that referenced this issue Sep 19, 2022
akurtakov added a commit to akurtakov/tycho that referenced this issue Sep 19, 2022
@laeubi
Copy link
Member

laeubi commented Sep 20, 2022

Another (subtile) issue with Java 17 described here #1365 (comment) please help to investigate as this could cause a lot of headache if we are forcing users into Java 17 and suddenly their builds fail with strange errors.

akurtakov added a commit to akurtakov/tycho that referenced this issue Sep 20, 2022
akurtakov added a commit to akurtakov/tycho that referenced this issue Sep 20, 2022
akurtakov added a commit to akurtakov/tycho that referenced this issue Sep 21, 2022
mickaelistria pushed a commit to mickaelistria/tycho that referenced this issue Sep 21, 2022
@mickaelistria mickaelistria linked a pull request Sep 21, 2022 that will close this issue
mickaelistria pushed a commit that referenced this issue Sep 21, 2022
@laeubi laeubi added this to the 4.0 milestone Jun 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants