From 21cdebe8c7b92982c2c76268479f3239cbd6b2c9 Mon Sep 17 00:00:00 2001 From: modmuss50 Date: Thu, 8 Aug 2024 21:00:39 +0100 Subject: [PATCH 1/4] Configuration cache support for SourceSetHelper.findFileInResource --- .../loom/util/gradle/GradleUtils.java | 10 +++ .../loom/util/gradle/SourceSetHelper.java | 10 ++- .../integration/ConfigurationCacheTest.groovy | 72 +++++++++++++++++++ 3 files changed, 90 insertions(+), 2 deletions(-) diff --git a/src/main/java/net/fabricmc/loom/util/gradle/GradleUtils.java b/src/main/java/net/fabricmc/loom/util/gradle/GradleUtils.java index dbe914427..1166bc217 100644 --- a/src/main/java/net/fabricmc/loom/util/gradle/GradleUtils.java +++ b/src/main/java/net/fabricmc/loom/util/gradle/GradleUtils.java @@ -24,9 +24,11 @@ package net.fabricmc.loom.util.gradle; +import java.io.File; import java.util.function.Consumer; import org.gradle.api.Project; +import org.gradle.api.file.RegularFileProperty; import org.gradle.api.invocation.Gradle; import org.gradle.api.provider.Provider; @@ -78,4 +80,12 @@ public static Provider getBooleanPropertyProvider(Project project, Stri public static boolean getBooleanProperty(Project project, String key) { return getBooleanPropertyProvider(project, key).getOrElse(false); } + + // A hack to include the given file in the configuration cache input + // this ensures that configuration cache is invalidated when the file changes + public static File configurationInputFile(Project project, File file) { + final RegularFileProperty property = project.getObjects().fileProperty(); + property.set(file); + return property.getAsFile().get(); + } } diff --git a/src/main/java/net/fabricmc/loom/util/gradle/SourceSetHelper.java b/src/main/java/net/fabricmc/loom/util/gradle/SourceSetHelper.java index b693a8a0d..d56c550a8 100644 --- a/src/main/java/net/fabricmc/loom/util/gradle/SourceSetHelper.java +++ b/src/main/java/net/fabricmc/loom/util/gradle/SourceSetHelper.java @@ -241,8 +241,14 @@ public static File findFileInResource(SourceSet sourceSet, String path) { final LoomGradleExtension extension = LoomGradleExtension.get(project); if (extension.isConfigurationCacheActive()) { - // TODO config cache, figure this out - project.getLogger().warn("Unable to find resource ({}) in source set ({}) when configuration cache is active", path, sourceSet.getName()); + for (File rootDir: sourceSet.getResources().getSrcDirs()) { + final File file = new File(rootDir, path); + + if (file.exists()) { + return GradleUtils.configurationInputFile(project, file); + } + } + return null; } diff --git a/src/test/groovy/net/fabricmc/loom/test/integration/ConfigurationCacheTest.groovy b/src/test/groovy/net/fabricmc/loom/test/integration/ConfigurationCacheTest.groovy index a6af3e2d9..656c5e9c5 100644 --- a/src/test/groovy/net/fabricmc/loom/test/integration/ConfigurationCacheTest.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/integration/ConfigurationCacheTest.groovy @@ -30,6 +30,7 @@ import spock.lang.Unroll import net.fabricmc.loom.test.util.GradleProjectTestTrait import static net.fabricmc.loom.test.LoomTestConstants.PRE_RELEASE_GRADLE +import static net.fabricmc.loom.test.LoomTestConstants.STANDARD_TEST_VERSIONS import static org.gradle.testkit.runner.TaskOutcome.FAILED class ConfigurationCacheTest extends Specification implements GradleProjectTestTrait { @@ -60,4 +61,75 @@ class ConfigurationCacheTest extends Specification implements GradleProjectTestT "jar" | _ "check" | _ } + + // Test GradleUtils.configurationInputFile invalidates the cache when the file changes + @Unroll + def "File input (#version)"() { + setup: + def gradle = gradleProject(project: "minimalBase", version: version) + gradle.buildGradle << """ + dependencies { + minecraft 'com.mojang:minecraft:1.20.4' + mappings 'net.fabricmc:yarn:1.20.4+build.3:v2' + modImplementation 'net.fabricmc:fabric-loader:0.15.6' + } + + abstract class TestTask extends DefaultTask { + @Input + abstract Property getModVersion() + + @TaskAction + void run() { + println "Version: " + modVersion.get() + } + } + + tasks.register('testTask', TestTask) { + modVersion.set(loom.getModVersion()) // loom.getModVersion() returns a String + } + """.stripIndent() + + def fabricModJson = new File(gradle.projectDir, "src/main/resources/fabric.mod.json") + fabricModJson.parentFile.mkdirs() + fabricModJson.text = fmj("1.0.0") + + // Without these the 2nd run complains about the cache being invalid, TODO look into why + new File(gradle.projectDir, ".gradle/loom-cache/remapped_mods").mkdirs() + new File(gradle.projectDir, "build/loom-cache").mkdirs() + + when: + def result = gradle.run(task: "testTask", configurationCache: true) + def result2 = gradle.run(task: "testTask", configurationCache: true) + fabricModJson.text = fmj("2.0.0") + def result3 = gradle.run(task: "testTask", configurationCache: true) + + then: + // Test that the cache is created + result.task(":testTask").outcome != FAILED + result.output.contains("Calculating task graph as no cached configuration is available for tasks: testTask") + result.output.contains("Version: 1.0.0") + + // Test that the cache is reused when nothing has changed + result2.task(":testTask").outcome != FAILED + !result2.output.contains("Calculating task graph") + result2.output.contains("Version: 1.0.0") + + // Test that the cache is invalidated when the file changes + result3.task(":testTask").outcome != FAILED + result3.output.contains("Calculating task graph as configuration cache cannot be reused because file 'src/main/resources/fabric.mod.json' has changed.") + result3.output.contains("Version: 2.0.0") + + where: + version << STANDARD_TEST_VERSIONS + } + + static def fmj(String version) { + return """ + { + "schemaVersion": 1, + "id": "test", + "version": "${version}" + } + """ + } } From 7c8961a17ac3be2d68d65e1764a9afe6462a1c8b Mon Sep 17 00:00:00 2001 From: modmuss50 Date: Thu, 8 Aug 2024 21:04:31 +0100 Subject: [PATCH 2/4] Prove that config cache is working --- .../loom/test/integration/ConfigurationCacheTest.groovy | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/test/groovy/net/fabricmc/loom/test/integration/ConfigurationCacheTest.groovy b/src/test/groovy/net/fabricmc/loom/test/integration/ConfigurationCacheTest.groovy index 656c5e9c5..586d402cb 100644 --- a/src/test/groovy/net/fabricmc/loom/test/integration/ConfigurationCacheTest.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/integration/ConfigurationCacheTest.groovy @@ -84,6 +84,7 @@ class ConfigurationCacheTest extends Specification implements GradleProjectTestT } } + println "Configuring task testTask" tasks.register('testTask', TestTask) { modVersion.set(loom.getModVersion()) // loom.getModVersion() returns a String } @@ -107,16 +108,19 @@ class ConfigurationCacheTest extends Specification implements GradleProjectTestT // Test that the cache is created result.task(":testTask").outcome != FAILED result.output.contains("Calculating task graph as no cached configuration is available for tasks: testTask") + result.output.contains("Configuring task testTask") result.output.contains("Version: 1.0.0") // Test that the cache is reused when nothing has changed result2.task(":testTask").outcome != FAILED !result2.output.contains("Calculating task graph") + !result2.output.contains("Configuring task testTask") result2.output.contains("Version: 1.0.0") // Test that the cache is invalidated when the file changes result3.task(":testTask").outcome != FAILED result3.output.contains("Calculating task graph as configuration cache cannot be reused because file 'src/main/resources/fabric.mod.json' has changed.") + result3.output.contains("Configuring task testTask") result3.output.contains("Version: 2.0.0") where: From 9703632bc32f10bf6646ec6000b44edcbc365e12 Mon Sep 17 00:00:00 2001 From: modmuss50 Date: Fri, 9 Aug 2024 08:56:15 +0100 Subject: [PATCH 3/4] Misc improvements --- .../java/net/fabricmc/loom/extension/LoomFilesBaseImpl.java | 6 +----- .../java/net/fabricmc/loom/util/gradle/SourceSetHelper.java | 4 ++-- .../loom/test/integration/ConfigurationCacheTest.groovy | 4 ---- 3 files changed, 3 insertions(+), 11 deletions(-) diff --git a/src/main/java/net/fabricmc/loom/extension/LoomFilesBaseImpl.java b/src/main/java/net/fabricmc/loom/extension/LoomFilesBaseImpl.java index 8b23e4f97..7dd74d5c1 100644 --- a/src/main/java/net/fabricmc/loom/extension/LoomFilesBaseImpl.java +++ b/src/main/java/net/fabricmc/loom/extension/LoomFilesBaseImpl.java @@ -40,11 +40,7 @@ public LoomFilesBaseImpl() { } private static File createFile(File parent, String child) { File file = new File(parent, child); - - if (!file.exists()) { - file.mkdirs(); - } - + file.mkdirs(); return file; } diff --git a/src/main/java/net/fabricmc/loom/util/gradle/SourceSetHelper.java b/src/main/java/net/fabricmc/loom/util/gradle/SourceSetHelper.java index d56c550a8..d17b3cbca 100644 --- a/src/main/java/net/fabricmc/loom/util/gradle/SourceSetHelper.java +++ b/src/main/java/net/fabricmc/loom/util/gradle/SourceSetHelper.java @@ -242,10 +242,10 @@ public static File findFileInResource(SourceSet sourceSet, String path) { if (extension.isConfigurationCacheActive()) { for (File rootDir: sourceSet.getResources().getSrcDirs()) { - final File file = new File(rootDir, path); + final File file = GradleUtils.configurationInputFile(project, new File(rootDir, path)); if (file.exists()) { - return GradleUtils.configurationInputFile(project, file); + return file; } } diff --git a/src/test/groovy/net/fabricmc/loom/test/integration/ConfigurationCacheTest.groovy b/src/test/groovy/net/fabricmc/loom/test/integration/ConfigurationCacheTest.groovy index 586d402cb..096447247 100644 --- a/src/test/groovy/net/fabricmc/loom/test/integration/ConfigurationCacheTest.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/integration/ConfigurationCacheTest.groovy @@ -94,10 +94,6 @@ class ConfigurationCacheTest extends Specification implements GradleProjectTestT fabricModJson.parentFile.mkdirs() fabricModJson.text = fmj("1.0.0") - // Without these the 2nd run complains about the cache being invalid, TODO look into why - new File(gradle.projectDir, ".gradle/loom-cache/remapped_mods").mkdirs() - new File(gradle.projectDir, "build/loom-cache").mkdirs() - when: def result = gradle.run(task: "testTask", configurationCache: true) def result2 = gradle.run(task: "testTask", configurationCache: true) From 549c6ad2e92e1f37a8f15e60dace2ffaf1803d7e Mon Sep 17 00:00:00 2001 From: modmuss50 Date: Fri, 9 Aug 2024 17:33:27 +0100 Subject: [PATCH 4/4] Fix tests on windows --- .../loom/test/integration/ConfigurationCacheTest.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/groovy/net/fabricmc/loom/test/integration/ConfigurationCacheTest.groovy b/src/test/groovy/net/fabricmc/loom/test/integration/ConfigurationCacheTest.groovy index 096447247..bcd9ea88d 100644 --- a/src/test/groovy/net/fabricmc/loom/test/integration/ConfigurationCacheTest.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/integration/ConfigurationCacheTest.groovy @@ -115,7 +115,7 @@ class ConfigurationCacheTest extends Specification implements GradleProjectTestT // Test that the cache is invalidated when the file changes result3.task(":testTask").outcome != FAILED - result3.output.contains("Calculating task graph as configuration cache cannot be reused because file 'src/main/resources/fabric.mod.json' has changed.") + result3.output.contains("Calculating task graph as configuration cache cannot be reused because file 'src/main/resources/fabric.mod.json' has changed.".replace("/", File.separator)) result3.output.contains("Configuring task testTask") result3.output.contains("Version: 2.0.0")