Support for Java 21 (new LTS)?

xKiv

xKiv

Active member
MCroft said:
I hadn't tried Java 21 (I'll need to upgrade Gradle), but I know we haven't touched the menus in ages.
Click to expand...

Warnings for building with 21:
1) current-er gradle requires that tasks always have defined order of execution. Older gradle already gave warning about this, 8.5 upgrades them to errors. I added the following:

Code: 
startScripts.mustRunAfter shadowJar
distTar.mustRunAfter shadowJar
startShadowScripts.mustRunAfter jar

2) java 21 (at least the adoptium version?) removed some method that's use by google-java-format in the version used by com.diffplug.spotless 6.12.0 (which is what cleanly checked out mafia build.gradle has) -> breaks build when calling googleJavaFormat

3) after upgrading java to 21, gradle to 8.5, and spotless to 6.23.3: calling spotless finds several new violations in build.gradle (I just followed the patches that it spat out at me - it was just line formatting), and once I fixed those it found so many new violations in java files that I gave up and just commeted out the line that calls googleJavaFormat
 
Based on the findings of @xKiv above, it's non-trivial to switch our dev environment to Java 21, although it seems as if users can use java 21 (adoptium`s version, at least).

I don't know of any compelling reasons to switch, but if someone does have a good case for doing so, there's apparently some work to do.
 
I upgraded Gradle to 8.8, and Spotless no longer finds any violations with openjdk21.

However, I'm seeing several test failures due to Mockito being unable to mock certain standard library classes. It seems that for some strange reason, mockito-inline in the central maven repo is stuck on 5.2.0 (seems like it was always meant to be a temporary package), when other mockito libraries have been updated to 5.12.

When I update the dependency to 'org.mockito:mockito-core:5.12.0', I get different test failures that will probably need more time to figure out what's wrong.

Those tests pass when using openjdk17, though, but RabbitHoleManagerTest.canAutomateChessPuzzleFromRelayBrowser() then fails (which then succeeds again when I drop the version of mockito-core to 5.2.0, or when I upgrade back up to 5.8.0). I'll look at this some more later.
 
heeheehee said:
Those tests pass when using openjdk17, though, but RabbitHoleManagerTest.canAutomateChessPuzzleFromRelayBrowser() then fails (which then succeeds again when I drop the version of mockito-core to 5.2.0, or when I upgrade back up to 5.8.0). I'll look at this some more later.
Click to expand...

The relay automation tests are the source of frequent intermittent failures exposed by https://github.com/kolmafia/kolmafia/pull/1889 although they have been observed elsewhere as well. Anecdotally they are more likely to fail on Windows.
 
Bumping this thread -- as far as I'm aware, we are now able to build with Java 21. Spotless is now on 6.23.3, the task errors are now resolved, we're on Gradle 8.8. Most warnings are gone (there's still a warning for lib/darrylbu/util/MenuScroller.java since Object.finalize is marked for removal).

Any objections to opening a ~1 month window for "we are going to upgrade Mafia to require Java 21, please update your version of Java"?

I'd like to remove MenuScroller at some point, but I don't think it should block this migration.

(The next LTS, Java 25, is slated to arrive in September 2025. I'd certainly like us to adopt Java 21 before then, especially if there are any features we'd like.)
 
I guess I should upgrade too.

Once upon a time Java was pretty aggressive about telling me there was an update. That hasn't happened for a couple of years. I wonder if I disabled that somehow or it is a difference in Java providers?
 
I upgraded this morning and got weird behavior on the first load. A background frame, that appeared to be the item manager, appeared behind the login frame. When I dismissed the background frame I found that my saved character credentials were not available.

Closed KoLmafia, tried again, and everything worked as expected.
 
I'm okay with requiring an upgrade.

I didn't want to force it because I can't see anything in 21 I really want -- 17 brought records, and I think those were worth it.
 
21 brings new preview features in the form of string templates and unnamed patterns / variables, and finalizes record patterns and virtual threads (both introduced in Java 19).

By upgrading from 17, we also get better Unicode handling, and a couple of other minor library changes that would allow us to use simdjson-java if we so desired.

(Apparently latest minor versions of Java 24 include non-broken GOAWAY handling... which should be backported to Java 21 at some point. See https://bugs.openjdk.org/browse/JDK-8335181)
 
heeheehee said:
Most warnings are gone (there's still a warning for lib/darrylbu/util/MenuScroller.java since Object.finalize is marked for removal).

I'd like to remove MenuScroller at some point, but I don't think it should block this migration.
Click to expand...

Any reason why we don't just remove the override for finalize? Perhaps naively it seems to me that if we don't override we are just letting the garbage collector do its job. If for some reason instances of MenuScroller are not garbage collected is there any impact other than increased memory usage? If we don't finalize will there be a measurable change in memory usage and/or run time?

This may be too much of a veer but source code in the lib subtree is a legacy from a time when third party intellectual property was available as source but not in a jar file. The third party source was isolated and untouched as much as possible since an "update" involved diffs and merges and once upon a time there were no good tools to do that. But in 2024 I think most of the source there is no longer maintained by the author or a community. So, with proper acknowledgement, we could just refactor and move that source so we could maintain it.
 
fronobulax said:
Any reason why we don't just remove the override for finalize?
Click to expand...

It is (via calling .dispose()) removing its listeners from the associated JPopupMenu. Those would still be referenced from that JPopupMenu even after the MenuScroller is completely gone and collected.
That's already screaming bad design - finalize isn't even guaranteed to fire, much less fire "soon" after the last reference disappears. Whatever is responsible for "forgetting" the MenuScroller should also be responsible for calling .dispose() on it.
The most pushed solution seems to be "implement AutoCloseable and use try-with-resources", but that's not really possible if the lifecycle of the object isn't contained within one lexical scope. There are other ways (a Cleaner?) that have problems similar to finalize - you could make it guaranteed to fire even during application termination, but still can't guarantee "timely" execution.
It's the same problem as with any IO activity (you didn't close the stream? well maybe it flushed, and maybe it didn't ...).
 
You must log in or register to reply here.
Back
Top