-
Notifications
You must be signed in to change notification settings - Fork 44
Use setup-gradle #147
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
base: main
Are you sure you want to change the base?
Use setup-gradle #147
Conversation
setup-gradle adds persistent gradle cache for faster CI builds, build summary and does wrapper validation as part of its lifecycle. https://github.com/gradle/actions?tab=readme-ov-file#the-wrapper-validation-action
| - name: setup gradle | ||
| uses: gradle/actions/setup-gradle@v5 |
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.
I dont see why this is needed, I dont think the caching it does by default is a great idea for us to include in the example mod. There are some serious secuirty considerations that need to be made when using any form of caching with github actions, this isnt a choice I want to make for others.
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.
From the gradle/actions repository's README at the very top: "The setup-gradle action can be used to configure Gradle for optimal execution on any platform supported by GitHub Actions."
In my opinion, the template mod should ship an optimal configuration by default.
Gradle already performs encryption on the relevant parts of the cache per my knowledge.
See https://github.com/gradle/actions/blob/main/docs/setup-gradle.md#why-use-the-setup-gradle-action for more information. This section lists advantages over using setup-java's cache: gradle option (which is also not used here), or custom configurations with actions/cache.
The current behaviour wastes resources by unnecessarily throwing away the already built caches and repeatedly downloading a gradle distribution, fetching dependencies, configuring and running tasks each workflow run, even if nothing has been changed. This not only wastes resources of the runner, but also the repositories serving the dependencies, and potentially delays development while you wait for the workflow to finish.
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.
actions/setup-java apparently also recommends setup-gradle now for caching. See actions/setup-java#588
The official GitHub document about "Building and testing Java with Gradle" also seems to recommend setup-gradle. See https://docs.github.com/en/actions/tutorials/build-and-test-code/java-with-gradle#using-a-gradle-workflow-template - step 8 explains the purpose of setup-gradle: "Sets up the Gradle environment. The gradle/actions/setup-gradle action takes care of caching state between workflow runs, and provides a detailed summary of all Gradle executions."
As someone that got into Fabric modding recently and did not fiddle with the default actions configuration much, discovering it didn't use setup-gradle was suprising. Once switched over to it, there was tremendous reductions in build times in actions (from 13 minutes to 2 minutes for a 3 version multi-version build), so I decided to make a PR to include it in the template. However, if others do not agree I'm willing to close this PR, but maybe it should at least have a note like "# If you get long actions build times, consider switching to setup-gradle" or something, as a reference point for when people's mods get bigger or build for multiple versions. It does not do any harm for single version or small mods per my testing though so I'd prefer if it was the default starting point.
The security concerns might be valid, but in that case I don't think setup-java or official GitHub documents would also recommend the action. It does not reside under GitHub maintained standard actions like actions/checkout or actions/setup-java but rather gradle/setup-gradle, but I don't think that's relevant as the current default workflow also uses gradle/wrapper-validation (wrapper-validation is included as part of setup-gradle implicitly, so it is not needed with it, to clarify again)
setup-gradle adds persistent gradle cache for faster CI builds, build summary and does wrapper validation as part of its lifecycle. https://github.com/gradle/actions?tab=readme-ov-file#the-wrapper-validation-action