-
Notifications
You must be signed in to change notification settings - Fork 187
Update BREE to Java21 for SWT #2824
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
|
This is expected to fail with missing version bumps, but as we are around branch point I will correct that once the master is open for 2026-03 dev. |
This comment was marked as resolved.
This comment was marked as resolved.
I'm just wondering, when the test needs java 21 why do we need to bump the bundles/fragments? |
|
Seemed like a good time to make the update. We are only testing on Java 21 anyways on CI, so let's make it official. |
One should consider that SWT can be used "standalone" as well so bumping the java version for no good reason can have a negative impact on a lot of unknown consumers and does not bring any benefits. Esepcially if you say we are already testing on Java 21 all whats needed is to use the higher version in the test bundle to make your use-case work what has a much smaller footprint, easier to review and so on. |
|
I think maybe the point is that nothing tests that it actually runs on the older JVMs. |
We don't do that on other places as well ... I also don't see why we should assume it is not the case, and if we do we need to recompile each and every library we use in eclipse because also there we never check it actually runs with the EE declared there.... not talking about that no one ever test we can run with any older version we currently allow in version ranges. |
Indeed with significant parts of equinox being an example. |
|
As for the June release the plan is to make use Java FFM (final in Java 22) thus SWT will bump to Java 25 it seems correct to have one release that moves to Java 21 (even if not strictly depending on things in it) to allow gradual progress for consumers. |
|
Fully agree that we should move to Java 21 sooner than later so that we have at least one further relase until we move to Java 25. Regarding moving to Java 25 in Eclipse/SWT, I would like to mention that it might be that consumers suffer from https://bugs.openjdk.org/browse/JDK-8336862. At least we do and we are still investigating solutions but have not found any to apply with reasonable effort yet. That behavior change was reverted for the more revent Java 21 release but is present in Java 22 onwards. |
If we're planning to raise the requirement to Java 25 in the June release (as mentioned in the discussion), I don't see a strong benefit in raising it to Java 21 now. This intermediate step would:
Alternative proposal: Stay at Java 17 for now and make a single, larger jump directly to Java 25 in the June release, skipping Java 21 altogether. This would:
What are the compelling reasons for the intermediate Java 21 step that would outweigh these concerns? Are there specific Java 21 features that SWT urgently needs before June? |
|
We seem to have a totally opposite view of what is a benefit and what a concern. Everything you describe as churn and etc. is what I see as benefit - send a message that no one can pretend to have missed that things are moving up and make people prepare for even bigger changes. |
|
Don't know this was a official "vote" from PL and assumed it was more an open discussion. As it seems not the case and my concerns are actually overruled I will simply abstain from this then as a consequence then, no need for any addition disagreement process here as I raised my concerns. |
|
I appreciate seeing arguments to consider being raised. No matter if they change any decision or not, it's valuable to have them on the table to consider. Probably no one of us would be in favor of increasing requirements / BREEs without any relevant need. In this case, I particularly see the need in the upcoming bump to Java 25. Skipping a complete Java LTS release could raise even more concerns. |
From my experience, "affected" users never read any announcements and never prepare upfront to anything. So announcing now we would switch to java 25 in two releases would not force anyone to try and fix problems locally, and because of that there would be no reduction of the burden, the opposite will happen: they will get all changes in one "big bang" event with Java 25, so they will have to deal with Java 21 and 25 changes at same time. Also from my experience as platform owner for our application, switching via one "big bang" event always caused lot of troubles and took much more time for me in the past. After we've switched to "build & test every week on master", almost all problems were "easy" to identify & fix (compared to "build once next target release is out & pray it works"). Switching to Java 21 now is fine IMO, earlier is better, also better to sort out problems specific to Java 21 and to 25. |
|
Actually I would expect a minor version bump in all bundles when increasing the BREE even though API tooling would be fine with the micro version bump already performed for most of the bundles/fragments. |
|
I agree with the minor version bump. It's better for setting bounds for those folks who might want to set bounds. |
|
Minor version bump is actually a requirement as per the guidelines https://github.com/eclipse-platform/eclipse.platform.releng.aggregator/wiki/Version-Numbering#when-to-change-the-minor-segment |
and:
I'll update minor versions and push new version in a bit. |
I know you actually mean 2026-03. 😁 |
Yup 🤣 (I corrected previous comments now) |
I agree. Smaller steps are usually the way to go for transitioning larger projects. Even if you do them in fast succession. It adds a safety net to fallback to. |
|
Doh! I assume there was an easier way to do this than how I did it (manually - but forgetting about pom.xmls). |
e06c763 to
785e5da
Compare
|
This will be ready for review when https://ci.eclipse.org/releng/job/eclipse.platform.swt/view/change-requests/job/PR-2824/8/ is complete. Note that this was a rebuild so that the forceNativeBuilds is on to make sure that part of the Jenkinsfile change is exercised before merge as much as possible (see parameters). There are a number of deprecation warnings that need to be fixed with the update to Java 21 - I will be providing them as follow-on PRs to make it easier to review. |
|
Because of eclipse-platform/eclipse.platform.releng.aggregator#3315 Jenkins timed out to rebuild the macos aarch natives. All the other platforms the natives built fine. I restarted the Jenkins build, but only building the natives for linux and win32 as run 11 So run 8 shows all the natives (except macos aarch) building fine and run 11 completed successfully, testing the GTK port against the Jenkins rebuild natives. Both those runs were against the same commit: 785e5da IIUC run 11 is marked as a failing check, but I will fix those warnings in subsequent commits/PRs. Any objections to merging? I would like to get it in before I end up with a bunch of conflicts as others submit their PRs. |
|
Other than some minor things in org.eclipse.swt/.classpath this is good to go. |
HeikoKlare
left a comment
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.
Except for the mentioned points, the changes also look fine to me.
Just one note on the BREE bump to the snippets project: the classpath files for that project are templates and automatically copied when using Oomph. When we merge this change, the copied classpath files will (probably) not be updated. I am not sure if this is something that we could add to the Oomph setup (if it's not already there and I am not aware of it). Otherwise we must be aware that every workspace with the snippets project will require a manual update.
Thanks for pointing this out - there is a warning (we could turn to an error?) if this is mismatched:
Also, it is ok for now the mismatch because we don't yet use any Java21 in the snippets. I'll make an Announcement when this is merged.
I don't think that is a good idea because that would presumably overwrite other changes in the file, such as snippet exclusions. Maybe instead of a copy it could be a link for that file to the correct platform one? But that would be a change for another day I think. New patchset on its way very soon. |
Includes: - bumping all minor versions - adding API filters for above because PDE is not considering BREE when reporting issues
Yes, better to do whatever we did for SWT itself because it previously had such a copy task too. |
|
As previous Jenkins build finished with warnings (new deprecations in Java 21) I'm merging this one to get the ball rolling without waiting on the current one that is just stuck with native machines still doing today's I-build tests as the nightly didn't run due to yet another EF infrastructructure issue eclipse-platform/eclipse.platform.releng.aggregator#3533 . |
Done - #2840 - however not in Announcements because it isn't actually enabled??? |
Finalization is deprecated for removal in Java, as of now there is no actual removal date planned so simply suppressing the warning seems most suitable. Follow-up to remove warnings in SWT workspace with update to Java 21 in eclipse-platform#2824
Finalization is deprecated for removal in Java, as of now there is no actual removal date planned so simply suppressing the warning seems most suitable. Follow-up to remove warnings in SWT workspace with update to Java 21 in eclipse-platform#2824
Finalization is deprecated for removal in Java, as of now there is no actual removal date planned so simply suppressing the warning seems most suitable. Follow-up to remove warnings in SWT workspace with update to Java 21 in #2824
Includes adding a handful of new tests for the code that this modifies to cover all the special cases. The new tests are Linux only, but they could be adapted to test these error conditions on other platforms. Needed to resolve new warnings due to update to Java21 eclipse-platform#2824
These calls are covered by existing Browser test cases. Needed to resolve new warnings due to update to Java21 eclipse-platform#2824
Includes adding a handful of new tests for the code that this modifies to cover all the special cases. The new tests are Linux only, but they could be adapted to test these error conditions on other platforms. Needed to resolve new warnings due to update to Java21 #2824
These calls are covered by existing Browser test cases. Needed to resolve new warnings due to update to Java21 #2824


To be able to do #2561 (comment) I need Java 21.