Build: Update to EGT 0.7, KGP 2.3, Gradle 9, UC 453#174
Build: Update to EGT 0.7, KGP 2.3, Gradle 9, UC 453#174RedEpicness wants to merge 6 commits intomasterfrom
Conversation
This also requires increasing the minimum required Kotlin version to 1.9, because targeting 1.6 is no longer supported by Kotlin Gradle Plugin 2.3. This also adopts modern "jvm-default" compiler option and changes corresponding annotations accordingly to minimise binary changes.
| public final class gg/essential/elementa/UIComponent$sam$i$java_util_function_Predicate$0 : java/util/function/Predicate { | ||
| public fun <init> (Lkotlin/jvm/functions/Function1;)V | ||
| public final synthetic fun test (Ljava/lang/Object;)Z | ||
| } |
There was a problem hiding this comment.
This is a side effect of changing how the jvm-default option behaves. Given that it's a generated class I cannot add an annotation to ensure the generation of this, but looking at both the class itself and it's compiled version, it seems like the only use of this class is internal and not applicable or useful to any external user of the UIComponent class. Unless another solution is found for the handling of jvm-default, I want someone else to verify this removal is ok.
There was a problem hiding this comment.
I'm not sure this is a side effect of anything to do with jvm-default. It didn't happen in the other PR.
It definitely looks like an internal class though, that probably shouldn't have been public to begin with.
I reckon it's either the bump to the Kotlin compiler which made it private, or the bump to binary-compatibility-validator which now excludes such internal (but technically public) classes.
Do you know which lambda in the source code this class corresponds to?
There was a problem hiding this comment.
Looking into the decompiled UIComponent , the only use of the Predicate class is in the removeEffect() function (the no-args one). This was the basis of my assumption that it's only ever meant to be used internally and thus is useless to expose. The same can be seen in the compiled class before the update so I assume it could be the compatibility validator ignoring this now. In either case I think it's safe to safe to remove.
There was a problem hiding this comment.
issue: It seems like this is the wrapper jar from Gradle 8.7-8.8.
Note that you first need to bump the Gradle version, and only then run the wrapper task (or run the wrapper task twice), otherwise you'll still be using the old Gradle version to generate the wrapper.
| 8 | ||
| 16 | ||
| 17 | ||
| 21 |
There was a problem hiding this comment.
suggestion: We can probably drop 16 and 17. Given we compile a single Elementa library for all MC versions, we only really need 8 for that (and 21 to run Gradle).
gradle/libs.versions.toml
Outdated
| kotlinx-coroutines = "1.5.2" | ||
| jetbrains-annotations = "23.0.0" | ||
| universalcraft = "406" | ||
| universalcraft = "453" |
There was a problem hiding this comment.
question: Do we need to bump UC for this?
| compileOnly("org.lwjgl:lwjgl-opengl:3.3.1") | ||
| } | ||
| tasks.compileKotlin.setJvmDefault("all") | ||
| kotlin.compilerOptions.jvmDefault.set(JvmDefaultMode.NO_COMPATIBILITY) |
There was a problem hiding this comment.
The change from
alltono-compatibilityin layoutdsl/build.gradle.kts and statev2/build.gradle.kts was smooth and only required the removal of@JvmDefaultWithoutCompatibilityin state.kt
note: This isn't actually a change. The equivalent option to -Xjvm-default=all is -jvm-default=no-compatibility.
The usage of JvmDefaultWithoutCompatibility was incorrect to begin with, because -Xjvm-default=all doesn't generate the compatibility classes to begin with (that's what -Xjvm-default=all-compatibility is for).
This is because we import the code for the unstable/ projects from Essential (except for the build.gradle.kts files) and Essential's build.gradle.kts does use -Xjvm-default=all-compatibility.
I'll open a PR on Essential to rectify this, and once that's merged, import that change (and any other changes which haven't been imported yet) into Elementa's repo.
Edit: Done. The JvmDefaultWithoutCompatibility is gone now.
| import java.awt.Color | ||
|
|
||
| @JvmDefaultWithCompatibility | ||
| interface ImageProvider { |
There was a problem hiding this comment.
note: I've pulled out this change of "generate compatibility classes by default" to "only for old interfaces" into a separate PR because it can be done independently of the EGT/KGP/Gradle bump, and so simplifies the changes in this PR.
2 participants
I need this as a prerequisite for a change in UniversalCraft.
While most of the update is self-explanatory, the changes to the now deprecated
-Xjvm-defaultcompiler options caused a lot of unnecessary changes to the API, alongside build errors since@JvmDefaultWithoutCompatibilityin state.kt requires the option to beenabled. Thus, I decided to migrate over to the new-jvm-defaultoption.Attempting to migrate according to the migration guidelines does not immediately fix the issue.
The change from
alltono-compatibilityin layoutdsl/build.gradle.kts and statev2/build.gradle.kts was smooth and only required the removal of@JvmDefaultWithoutCompatibilityin state.ktBut, the change from
all-compatibilitytoenablein build.gradle.kts did not remove the unnecessary api changes. I wasn't able to manually remove them with annotations so i gave up and after a lot of trial and error I ended up with migrating tono-compatibilitylike the other two modules, which removed a much much smaller list of the generated compatibility functions which I re-added with@JvmDefaultWithCompatibilityannotations where needed. There was one api removal that I couldn't avoid, but to my knowledge it was not necessary and should be ok to remove. I left a separate comment on the PR about this.If anyone has any idea on how to tackle this by keeping the option set to
enabled, let me know.