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

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

Open
SubordinalBlue opened this issue May 24, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@SubordinalBlue
Copy link

More of a question really, than a normal issue.

@sciwhiz12 sciwhiz12 added the enhancement New feature or request label May 24, 2024
@sciwhiz12
Copy link
Member

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.

@sciwhiz12 sciwhiz12 added bug Something isn't working and removed enhancement New feature or request labels May 24, 2024
@tmvkrpxl0
Copy link

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

@lukebemish
Copy link

lukebemish commented Jun 5, 2024

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 #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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants