search

neoforged / MDK

The Mod Developer Kit - this is where you start if you want to develop a new mod
https://github.com/NeoForgeMDKs
183 stars 60 forks source link

.github/workflows/build.yaml ... shouldn't that be calling for Java21? #66

Open SubordinalBlue opened 1 month ago

SubordinalBlue commented 1 month ago

More of a question really, than a normal issue.

sciwhiz12 commented 1 month ago

Yep, that should be installing JDK 21. It seems it was neglected to be updated when the MDK was updated to 1.20.5 and above.

However, it's relatively harmless, since that Java version is used to run the Gradle Daemon, which supports Java 17. For setting up the NeoForge environment, Gradle will automatically provision a JVM 21 toolchain (since the foojay-resolver plugin is installed via settings.gradle) and use that.

tmvkrpxl0 commented 1 month ago

Although it will install 2 java versions. so it's better to change it to 21.

lukebemish commented 1 month ago

The setup-java action should actually probably be removed entirely -- it substantially slows down CI runs and is not actually necessary in this case. ubuntu-latest already has both java 17 and 21 present; for actual runs, the gradle toolchains feature will locate the right one, and for running gradle (which should be done with a run now instead of an argument in the gradle action step -- see the deprecated features of the gradle action, that action should only be used for cache stuff, and then the actual tasks ran with ./gradlew), this can be specified either with JAVA_HOME or by using the new daemon toolchain feature (see https://github.com/neoforged/MDK/pull/67)

All of that said -- there's no particular reason that should be setting up java 21 instead of 17. It sets up the java version needed to run gradle -- not the one needed to run MC, which can be handled by toolchains. The one for gradle could be 17 or 21 and it'll work just fine, as NG only needs 17.