Modernize Gradle buildscript #71

Merged
zml2008 merged 4 commits from feature/gradle-modernization into master 2020-11-10 22:11:54 +00:00
8 changed files with 79 additions and 43 deletions

41
.github/workflows/build.yml vendored Normal file
View File

@ -0,0 +1,41 @@
# Automatically build the project and run any configured tests for every push
# and submitted pull request. This can help catch issues that only occur on
# certain platforms or Java versions, and provides a first line of defence
# against bad commits.
name: build
on: [pull_request, push]
jobs:
build:
strategy:
matrix:
# Use these Java versions
java: [
1.8, # Minimum supported by Minecraft
11, # Current Java LTS
15 # Latest version
]
# and run on both Linux and Windows
os: [ubuntu-20.04, windows-latest]
runs-on: ${{ matrix.os }}
modmuss50 commented 2020-11-02 10:56:52 +00:00 (Migrated from github.com)
Review

I would just do ubuntu only, I dont think building on windows provides any benefit. Building on windows would double the amount of stuff to download, and increase the amount github has to build (I know its free, but dont want to wase it for the sake of it)

I would just do ubuntu only, I dont think building on windows provides any benefit. Building on windows would double the amount of stuff to download, and increase the amount github has to build (I know its free, but dont want to wase it for the sake of it)
zml2008 commented 2020-11-03 05:58:05 +00:00 (Migrated from github.com)
Review

I've run into platform-specific issues before, in my own projects and what I've seen in some other repos:

  • checkstyle doesn't normalize path separators in its suppressions file, so that can end up only working on one OS
  • some parts of path handling differ, between different line separators and handling of symlinks -- though these would only really be caught by unit tests
  • When developing on a case-insensitive filesystem (like windows has), renaming a class or folder in a way that only changes its case will not be captured by git.

These can be caught by running CI builds on multiple operating systems.

I've run into platform-specific issues before, in my own projects and what I've seen in some other repos: - checkstyle doesn't normalize path separators in its suppressions file, so that can end up only working on one OS - some parts of path handling differ, between different line separators and handling of symlinks -- though these would only really be caught by unit tests - When developing on a case-insensitive filesystem (like windows has), renaming a class or folder in a way that only changes its case will not be captured by `git`. These can be caught by running CI builds on multiple operating systems.
steps:
- name: checkout repository
uses: actions/checkout@v2
- name: validate gradle wrapper
uses: gradle/wrapper-validation-action@v1
- name: setup jdk ${{ matrix.java }}
uses: actions/setup-java@v1
with:
java-version: ${{ matrix.java }}
- name: make gradle wrapper executable
if: ${{ runner.os != 'Windows' }}
run: chmod +x ./gradlew
modmuss50 commented 2020-11-02 10:57:15 +00:00 (Migrated from github.com)
Review

This shouldnt be needed, it should be executable on the repo.

This shouldnt be needed, it should be executable on the repo.
JamiesWhiteShirt commented 2020-11-02 16:12:49 +00:00 (Migrated from github.com)
Review

Depending on the method used to replicate this repo, the file mode may be lost. I don't think it should be removed.

Depending on the method used to replicate this repo, the file mode may be lost. I don't think it should be removed.
zml2008 commented 2020-11-03 05:41:09 +00:00 (Migrated from github.com)
Review

Yeah, windows generally doesn't preserve file permissions -- I've had it happen a few times before when setting up gradle wrapper.

There's no harm to keeping it there -- it'll basically be a no-op if the repo is set up correctly, but removes one more thing that could go wrong.

Yeah, windows generally doesn't preserve file permissions -- I've had it happen a few times before when setting up gradle wrapper. There's no harm to keeping it there -- it'll basically be a no-op if the repo is set up correctly, but removes one more thing that could go wrong.
- name: build
run: ./gradlew build
modmuss50 commented 2020-11-02 10:59:18 +00:00 (Migrated from github.com)
Review

Saving the build artifacts after this might not be a bad idea. Make sure to only do it on one java version

Saving the build artifacts after this might not be a bad idea. Make sure to only do it on one java version
- name: capture build artifacts
if: ${{ runner.os == 'Linux' && matrix.java == '11' }} # Only upload artifacts built from LTS java on one OS
uses: actions/upload-artifact@v2
with:
name: Artifacts
path: build/libs/

View File

@ -3,15 +3,12 @@ plugins {
id 'maven-publish'
}
sourceCompatibility = JavaVersion.VERSION_1_8
targetCompatibility = JavaVersion.VERSION_1_8
archivesBaseName = project.archives_base_name
version = project.mod_version
group = project.maven_group
dependencies {
//to change the versions see the gradle.properties file
// To change the versions see the gradle.properties file
minecraft "com.mojang:minecraft:${project.minecraft_version}"
mappings "net.fabricmc:yarn:${project.yarn_mappings}:v2"
modImplementation "net.fabricmc:fabric-loader:${project.loader_version}"
@ -31,23 +28,36 @@ processResources {
}
}
// ensure that the encoding is set to UTF-8, no matter what the system default is
// this fixes some edge cases with special characters not displaying correctly
// see http://yodaconditions.net/blog/fix-for-java-file-encoding-problems-with-gradle.html
tasks.withType(JavaCompile) {
options.encoding = "UTF-8"
tasks.withType(JavaCompile).configureEach {
// ensure that the encoding is set to UTF-8, no matter what the system default is
// this fixes some edge cases with special characters not displaying correctly
// see http://yodaconditions.net/blog/fix-for-java-file-encoding-problems-with-gradle.html
// If Javadoc is generated, this must be specified in that task too.
it.options.encoding = "UTF-8"
// The Minecraft launcher currently installs Java 8 for users, so your mod probably wants to target Java 8 too
// JDK 9 introduced a new way of specifying this that will make sure no newer classes or methods are used.
// We'll use that if it's available, but otherwise we'll use the older option.
def targetVersion = 8
if (JavaVersion.current().isJava9Compatible()) {
it.options.release = targetVersion
} else {
it.sourceCompatibility = JavaVersion.toVersion(targetVersion)
it.targetCompatibility = JavaVersion.toVersion(targetVersion)
}
}
// Loom will automatically attach sourcesJar to a RemapSourcesJar task and to the "build" task
// if it is present.
// If you remove this task, sources will not be generated.
task sourcesJar(type: Jar, dependsOn: classes) {
classifier = "sources"
from sourceSets.main.allSource
java {
// Loom will automatically attach sourcesJar to a RemapSourcesJar task and to the "build" task
// if it is present.
// If you remove this line, sources will not be generated.
withSourcesJar()
}
jar {
from "LICENSE"
from("LICENSE") {
rename { "${it}_${project.archivesBaseName}"}
}
}
// configure the maven publication
@ -64,9 +74,9 @@ publishing {
}
modmuss50 commented 2020-11-02 10:02:02 +00:00 (Migrated from github.com)
Review

I dont think this is really needed, its another thing to confuse new develeops. Very few people publish to maven, and if so there are good docs online? Possbly just link to https://docs.gradle.org/current/userguide/publishing_maven.html

I dont think this is really needed, its another thing to confuse new develeops. Very few people publish to maven, and if so there are good docs online? Possbly just link to https://docs.gradle.org/current/userguide/publishing_maven.html
}
// select the repositories you want to publish to
// Select the repositories you want to publish to
// To publish to maven local, no extra repositories are necessary. Just use the task `publishToMavenLocal`.
repositories {
// uncomment to publish to the local maven
// mavenLocal()
// See https://docs.gradle.org/current/userguide/publishing_maven.html for information on how to set up publishing.
}
}
Juuxel commented 2020-11-02 17:02:55 +00:00 (Migrated from github.com)
Review

Does this work for sources in other source directories than java, like Kotlin sources?

Does this work for sources in other source directories than `java`, like Kotlin sources?
zml2008 commented 2020-11-03 05:21:39 +00:00 (Migrated from github.com)
Review
It does. `withSourcesJar()` [collects its source from `SourceSet.getAllSource()`](https://github.com/gradle/gradle/blob/a24197ee1f665acd9ed93f345eb5b7f788385151/subprojects/plugins/src/main/java/org/gradle/api/plugins/internal/DefaultJavaPluginExtension.java#L125)

View File

@ -4,8 +4,8 @@ org.gradle.jvmargs=-Xmx1G
# Fabric Properties
# check these on https://fabricmc.net/use
minecraft_version=1.16.3
yarn_mappings=1.16.3+build.17
loader_version=0.10.0+build.208
yarn_mappings=1.16.3+build.47
loader_version=0.10.6+build.214
# Mod Properties
mod_version = 1.0.0
@ -14,4 +14,4 @@ org.gradle.jvmargs=-Xmx1G
# Dependencies
# currently not on the main fabric site, check on the maven: https://maven.fabricmc.net/net/fabricmc/fabric-api/fabric-api
fabric_version=0.22.0+build.408-1.16
fabric_version=0.25.0+build.415-1.16

Binary file not shown.

View File

@ -1,5 +1,5 @@
distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
distributionUrl=https\://services.gradle.org/distributions/gradle-6.6.1-bin.zip
distributionUrl=https\://services.gradle.org/distributions/gradle-6.7-bin.zip
zipStoreBase=GRADLE_USER_HOME
zipStorePath=wrapper/dists

21
gradlew.bat vendored
View File

@ -40,7 +40,7 @@ if defined JAVA_HOME goto findJavaFromJavaHome
set JAVA_EXE=java.exe
%JAVA_EXE% -version >NUL 2>&1
if "%ERRORLEVEL%" == "0" goto init
if "%ERRORLEVEL%" == "0" goto execute
echo.
echo ERROR: JAVA_HOME is not set and no 'java' command could be found in your PATH.
@ -54,7 +54,7 @@ goto fail
set JAVA_HOME=%JAVA_HOME:"=%
set JAVA_EXE=%JAVA_HOME%/bin/java.exe
if exist "%JAVA_EXE%" goto init
if exist "%JAVA_EXE%" goto execute
echo.
echo ERROR: JAVA_HOME is set to an invalid directory: %JAVA_HOME%
@ -64,21 +64,6 @@ echo location of your Java installation.
goto fail
:init
@rem Get command-line arguments, handling Windows variants
if not "%OS%" == "Windows_NT" goto win9xME_args
:win9xME_args
@rem Slurp the command line arguments.
set CMD_LINE_ARGS=
set _SKIP=2
:win9xME_args_slurp
if "x%~1" == "x" goto execute
set CMD_LINE_ARGS=%*
:execute
@rem Setup the command line
@ -86,7 +71,7 @@ set CLASSPATH=%APP_HOME%\gradle\wrapper\gradle-wrapper.jar
@rem Execute Gradle
"%JAVA_EXE%" %DEFAULT_JVM_OPTS% %JAVA_OPTS% %GRADLE_OPTS% "-Dorg.gradle.appname=%APP_BASE_NAME%" -classpath "%CLASSPATH%" org.gradle.wrapper.GradleWrapperMain %CMD_LINE_ARGS%
"%JAVA_EXE%" %DEFAULT_JVM_OPTS% %JAVA_OPTS% %GRADLE_OPTS% "-Dorg.gradle.appname=%APP_BASE_NAME%" -classpath "%CLASSPATH%" org.gradle.wrapper.GradleWrapperMain %*
:end
@rem End local scope for the variables with windows NT shell

View File

@ -32,6 +32,6 @@
"minecraft": "1.16.x"
},
"suggests": {
"flamingo": "*"
"another-mod": "*"
}
}
modmuss50 commented 2020-11-02 09:59:24 +00:00 (Migrated from github.com)
Review

I would either keep this as flamingo or remote it all togeather.

I dont think many people use the suggests block, and there are 3 good examples on how to use the depends block so removing it seems fine. As good as flamingo is, its a bit of an in joke for the people that know about it, others it may just confuse.

I would either keep this as flamingo or remote it all togeather. I dont think many people use the suggests block, and there are 3 good examples on how to use the depends block so removing it seems fine. As good as flamingo is, its a bit of an in joke for the people that know about it, others it may just confuse.
zml2008 commented 2020-11-03 06:08:21 +00:00 (Migrated from github.com)
Review

will remove... if only JSON allowed comments, would be a good way to explain some things inline

will remove... if only JSON allowed comments, would be a good way to explain some things inline
altrisi commented 2020-11-15 11:58:19 +00:00 (Migrated from github.com)
Review

This ended up being merged with another-mod instead of flamingo or nothing...

This ended up being merged with `another-mod` instead of `flamingo` or nothing...