From 20ea426a7a0341294378e05f9b4aa6e42a0469e8 Mon Sep 17 00:00:00 2001 From: modmuss Date: Tue, 2 Jan 2024 22:35:22 +0000 Subject: [PATCH 1/7] Ensure that modImplementation is processed first, so any installer.json on that configuration takes priority. (#1015) --- .../configuration/mods/ModConfigurationRemapper.java | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/main/java/net/fabricmc/loom/configuration/mods/ModConfigurationRemapper.java b/src/main/java/net/fabricmc/loom/configuration/mods/ModConfigurationRemapper.java index 85f2c28b..1564e8bb 100644 --- a/src/main/java/net/fabricmc/loom/configuration/mods/ModConfigurationRemapper.java +++ b/src/main/java/net/fabricmc/loom/configuration/mods/ModConfigurationRemapper.java @@ -30,6 +30,7 @@ import java.io.UncheckedIOException; import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayList; +import java.util.Comparator; import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; @@ -83,7 +84,16 @@ public class ModConfigurationRemapper { // Client remapped dep collectors for split source sets. Same keys and values. final Map clientConfigsToRemap = new HashMap<>(); - for (RemapConfigurationSettings entry : extension.getRemapConfigurations()) { + /* + * Hack fix/improvement for https://github.com/FabricMC/fabric-loom/issues/1012 + * Ensure that modImplementation is processed first, so any installer.json on that configuration takes priority. + */ + final List remapConfigurationSettings = extension.getRemapConfigurations() + .stream() + .sorted(Comparator.comparing(setting -> !setting.getName().equals("modImplementation"))) + .toList(); + + for (RemapConfigurationSettings entry : remapConfigurationSettings) { // key: true if runtime, false if compile final Map envToEnabled = ImmutableMap.of( false, entry.getOnCompileClasspath().get(), From c5d73548e7b160337e0ab57d8baf273d3de62e1c Mon Sep 17 00:00:00 2001 From: modmuss Date: Tue, 2 Jan 2024 22:35:37 +0000 Subject: [PATCH 2/7] Remove jackson (#1014) * Remove jackson * Fix unit tests --- build.gradle | 1 - gradle/libs.versions.toml | 2 -- src/main/java/net/fabricmc/loom/LoomGradlePlugin.java | 3 --- .../providers/mappings/LayeredMappingsDependency.java | 2 +- .../providers/mappings/MappingConfiguration.java | 2 +- .../providers/mappings/extras/unpick/UnpickLayer.java | 6 +++--- .../providers/minecraft/MinecraftMetadataProvider.java | 4 ++-- .../providers/minecraft/assets/AssetIndex.java | 4 ++-- .../net/fabricmc/loom/task/DownloadAssetsTask.java | 2 +- .../net/fabricmc/loom/util/LibraryLocationLogger.java | 2 -- src/main/java/net/fabricmc/loom/util/ZipUtils.java | 2 +- .../loom/test/benchmark/SimpleBenchmark.groovy | 3 --- .../loom/test/unit/library/LibraryContextTest.groovy | 2 +- .../fabricmc/loom/test/util/MinecraftTestUtils.groovy | 10 +++++----- 14 files changed, 17 insertions(+), 28 deletions(-) diff --git a/build.gradle b/build.gradle index d7cf3719..de84c29f 100644 --- a/build.gradle +++ b/build.gradle @@ -119,7 +119,6 @@ dependencies { // libraries implementation libs.commons.io implementation libs.gson - implementation libs.jackson implementation libs.guava implementation libs.bundles.asm diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 30416679..4f3c55eb 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -3,7 +3,6 @@ kotlin = "1.9.0" asm = "9.6" commons-io = "2.15.0" gson = "2.10.1" -jackson = "2.16.0" guava = "33.0.0-jre" stitch = "0.6.2" @@ -31,7 +30,6 @@ asm-util = { module = "org.ow2.asm:asm-util", version.ref = "asm" } commons-io = { module = "commons-io:commons-io", version.ref = "commons-io" } gson = { module = "com.google.code.gson:gson", version.ref = "gson" } -jackson = { module = "com.fasterxml.jackson.core:jackson-databind", version.ref = "jackson" } guava = { module = "com.google.guava:guava", version.ref = "guava" } fabric-stitch = { module = "net.fabricmc:stitch", version.ref = "stitch" } diff --git a/src/main/java/net/fabricmc/loom/LoomGradlePlugin.java b/src/main/java/net/fabricmc/loom/LoomGradlePlugin.java index 8231476d..71248d2d 100644 --- a/src/main/java/net/fabricmc/loom/LoomGradlePlugin.java +++ b/src/main/java/net/fabricmc/loom/LoomGradlePlugin.java @@ -27,8 +27,6 @@ package net.fabricmc.loom; import java.util.List; import java.util.Objects; -import com.fasterxml.jackson.databind.DeserializationFeature; -import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.collect.ImmutableMap; import com.google.gson.Gson; import com.google.gson.GsonBuilder; @@ -52,7 +50,6 @@ import net.fabricmc.loom.util.LibraryLocationLogger; public class LoomGradlePlugin implements BootstrappedPlugin { public static final Gson GSON = new GsonBuilder().setPrettyPrinting().create(); - public static final ObjectMapper OBJECT_MAPPER = new ObjectMapper().configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false); public static final String LOOM_VERSION = Objects.requireNonNullElse(LoomGradlePlugin.class.getPackage().getImplementationVersion(), "0.0.0+unknown"); /** diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/LayeredMappingsDependency.java b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/LayeredMappingsDependency.java index c912bbe8..e99f4551 100644 --- a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/LayeredMappingsDependency.java +++ b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/LayeredMappingsDependency.java @@ -118,7 +118,7 @@ public class LayeredMappingsDependency implements SelfResolvingDependency, FileC return; } - byte[] data = LoomGradlePlugin.OBJECT_MAPPER.writeValueAsString(signatureFixes).getBytes(StandardCharsets.UTF_8); + byte[] data = LoomGradlePlugin.GSON.toJson(signatureFixes).getBytes(StandardCharsets.UTF_8); ZipUtils.add(mappingsFile, "extras/record_signatures.json", data); } diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/MappingConfiguration.java b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/MappingConfiguration.java index a32883da..bce9d5d0 100644 --- a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/MappingConfiguration.java +++ b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/MappingConfiguration.java @@ -235,7 +235,7 @@ public class MappingConfiguration { try (Reader reader = Files.newBufferedReader(recordSignaturesJsonPath, StandardCharsets.UTF_8)) { //noinspection unchecked - signatureFixes = LoomGradlePlugin.OBJECT_MAPPER.readValue(reader, Map.class); + signatureFixes = LoomGradlePlugin.GSON.fromJson(reader, Map.class); } } diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/extras/unpick/UnpickLayer.java b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/extras/unpick/UnpickLayer.java index ce53e64b..8b3879ab 100644 --- a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/extras/unpick/UnpickLayer.java +++ b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/extras/unpick/UnpickLayer.java @@ -46,15 +46,15 @@ public interface UnpickLayer { final Metadata metadata; try (Reader reader = Files.newBufferedReader(metadataPath, StandardCharsets.UTF_8)) { - metadata = LoomGradlePlugin.OBJECT_MAPPER.readValue(reader, Metadata.class); + metadata = LoomGradlePlugin.GSON.fromJson(reader, Metadata.class); } return new UnpickData(metadata, definitions); } public record Metadata(int version, String unpickGroup, String unpickVersion) { - public String asJson() throws IOException { - return LoomGradlePlugin.OBJECT_MAPPER.writeValueAsString(this); + public String asJson() { + return LoomGradlePlugin.GSON.toJson(this); } } } diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/MinecraftMetadataProvider.java b/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/MinecraftMetadataProvider.java index 396202ce..840207f6 100644 --- a/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/MinecraftMetadataProvider.java +++ b/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/MinecraftMetadataProvider.java @@ -115,7 +115,7 @@ public final class MinecraftMetadataProvider { } final String versionManifest = builder.downloadString(cacheFile); - return LoomGradlePlugin.OBJECT_MAPPER.readValue(versionManifest, ManifestVersion.class); + return LoomGradlePlugin.GSON.fromJson(versionManifest, ManifestVersion.class); } private MinecraftVersionMeta readVersionMeta() throws IOException { @@ -128,7 +128,7 @@ public final class MinecraftMetadataProvider { } final String json = builder.downloadString(options.minecraftMetadataPath()); - return LoomGradlePlugin.OBJECT_MAPPER.readValue(json, MinecraftVersionMeta.class); + return LoomGradlePlugin.GSON.fromJson(json, MinecraftVersionMeta.class); } public record Options(String minecraftVersion, diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/assets/AssetIndex.java b/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/assets/AssetIndex.java index 3dd6a3c5..aa7feb48 100644 --- a/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/assets/AssetIndex.java +++ b/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/assets/AssetIndex.java @@ -28,10 +28,10 @@ import java.util.Collection; import java.util.LinkedHashMap; import java.util.Map; -import com.fasterxml.jackson.annotation.JsonProperty; +import com.google.gson.annotations.SerializedName; @SuppressWarnings("unused") -public record AssetIndex(Map objects, boolean virtual, @JsonProperty("map_to_resources") boolean mapToResources) { +public record AssetIndex(Map objects, boolean virtual, @SerializedName("map_to_resources") boolean mapToResources) { public AssetIndex() { this(new LinkedHashMap<>(), false, false); } diff --git a/src/main/java/net/fabricmc/loom/task/DownloadAssetsTask.java b/src/main/java/net/fabricmc/loom/task/DownloadAssetsTask.java index b8abca18..c31b683b 100644 --- a/src/main/java/net/fabricmc/loom/task/DownloadAssetsTask.java +++ b/src/main/java/net/fabricmc/loom/task/DownloadAssetsTask.java @@ -122,7 +122,7 @@ public abstract class DownloadAssetsTask extends AbstractLoomTask { .sha1(assetIndex.sha1()) .downloadString(indexFile.toPath()); - return LoomGradlePlugin.OBJECT_MAPPER.readValue(json, AssetIndex.class); + return LoomGradlePlugin.GSON.fromJson(json, AssetIndex.class); } private Path getAssetsPath(AssetIndex.Object object, AssetIndex index) { diff --git a/src/main/java/net/fabricmc/loom/util/LibraryLocationLogger.java b/src/main/java/net/fabricmc/loom/util/LibraryLocationLogger.java index 2d27427f..ce193de4 100644 --- a/src/main/java/net/fabricmc/loom/util/LibraryLocationLogger.java +++ b/src/main/java/net/fabricmc/loom/util/LibraryLocationLogger.java @@ -26,7 +26,6 @@ package net.fabricmc.loom.util; import java.util.List; -import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.base.Preconditions; import com.google.gson.Gson; import kotlinx.metadata.jvm.KotlinClassMetadata; @@ -53,7 +52,6 @@ public final class LibraryLocationLogger { ClassRemapper.class, ClassNode.class, ASMifier.class, - ObjectMapper.class, Gson.class, Preconditions.class, FileUtils.class diff --git a/src/main/java/net/fabricmc/loom/util/ZipUtils.java b/src/main/java/net/fabricmc/loom/util/ZipUtils.java index f6214aed..9b387ad9 100644 --- a/src/main/java/net/fabricmc/loom/util/ZipUtils.java +++ b/src/main/java/net/fabricmc/loom/util/ZipUtils.java @@ -123,7 +123,7 @@ public class ZipUtils { public static T unpackJackson(Path zip, String path, Class clazz) throws IOException { final byte[] bytes = unpack(zip, path); - return LoomGradlePlugin.OBJECT_MAPPER.readValue(new String(bytes, StandardCharsets.UTF_8), clazz); + return LoomGradlePlugin.GSON.fromJson(new String(bytes, StandardCharsets.UTF_8), clazz); } public static void pack(Path from, Path zip) throws IOException { diff --git a/src/test/groovy/net/fabricmc/loom/test/benchmark/SimpleBenchmark.groovy b/src/test/groovy/net/fabricmc/loom/test/benchmark/SimpleBenchmark.groovy index dfb82df4..ca2b7a9b 100644 --- a/src/test/groovy/net/fabricmc/loom/test/benchmark/SimpleBenchmark.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/benchmark/SimpleBenchmark.groovy @@ -39,9 +39,6 @@ import static org.gradle.testkit.runner.TaskOutcome.SUCCESS @Singleton class SimpleBenchmark implements GradleProjectTestTrait { def run(File dir) { - // Forces loom to refresh files - System.setProperty("loom.refresh", "true") - def gradle = gradleProject( project: "minimalBase", version: LoomTestConstants.PRE_RELEASE_GRADLE, diff --git a/src/test/groovy/net/fabricmc/loom/test/unit/library/LibraryContextTest.groovy b/src/test/groovy/net/fabricmc/loom/test/unit/library/LibraryContextTest.groovy index 3432c387..256a21c4 100644 --- a/src/test/groovy/net/fabricmc/loom/test/unit/library/LibraryContextTest.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/unit/library/LibraryContextTest.groovy @@ -131,7 +131,7 @@ class LibraryContextTest extends Specification { } ] }""" - def context = new LibraryContext(MinecraftTestUtils.OBJECT_MAPPER.readValue(metaJson, MinecraftVersionMeta.class), JavaVersion.VERSION_17) + def context = new LibraryContext(MinecraftTestUtils.GSON.fromJson(metaJson, MinecraftVersionMeta.class), JavaVersion.VERSION_17) then: context.supportsJava19OrLater() == supportsJava19OrLater diff --git a/src/test/groovy/net/fabricmc/loom/test/util/MinecraftTestUtils.groovy b/src/test/groovy/net/fabricmc/loom/test/util/MinecraftTestUtils.groovy index f1d45bfd..8fb35383 100644 --- a/src/test/groovy/net/fabricmc/loom/test/util/MinecraftTestUtils.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/util/MinecraftTestUtils.groovy @@ -26,8 +26,8 @@ package net.fabricmc.loom.test.util import java.time.Duration -import com.fasterxml.jackson.databind.DeserializationFeature -import com.fasterxml.jackson.databind.ObjectMapper +import com.google.gson.Gson +import com.google.gson.GsonBuilder import net.fabricmc.loom.configuration.providers.minecraft.ManifestVersion import net.fabricmc.loom.configuration.providers.minecraft.MinecraftVersionMeta @@ -37,15 +37,15 @@ import net.fabricmc.loom.util.download.Download class MinecraftTestUtils { private static final File TEST_DIR = new File(LoomTestConstants.TEST_DIR, "minecraft") - public static final ObjectMapper OBJECT_MAPPER = new ObjectMapper().configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false) + public static final Gson GSON = new GsonBuilder().create() static MinecraftVersionMeta getVersionMeta(String id) { def versionManifest = download(Constants.VERSION_MANIFESTS, "version_manifest.json") - def manifest = OBJECT_MAPPER.readValue(versionManifest, ManifestVersion.class) + def manifest = GSON.fromJson(versionManifest, ManifestVersion.class) def version = manifest.versions().find { it.id == id } def metaJson = download(version.url, "${id}.json") - OBJECT_MAPPER.readValue(metaJson, MinecraftVersionMeta.class) + GSON.fromJson(metaJson, MinecraftVersionMeta.class) } static String download(String url, String name) { From 872d12ace0c3b4c3258fcefcbb5b280061d58238 Mon Sep 17 00:00:00 2001 From: modmuss Date: Thu, 4 Jan 2024 00:39:36 +0000 Subject: [PATCH 3/7] Update tiny-remapper, misc perf improvements, test fixes. (#1009) * Only mixin remap/analyse classpath jars that use static mixin remappings. * More of a mess * Less of a mess? * Nope? * Exclude the none root MC jars from the remap classpath when using MPO * Improve test a little * Update TR * Checkstyle * Fix DLN test * Fix possible crash when closing build services --- gradle/libs.versions.toml | 2 +- gradle/test.libs.versions.toml | 2 +- .../loom/configuration/mods/ModProcessor.java | 9 +--- .../net/fabricmc/loom/task/RemapJarTask.java | 12 ++--- .../loom/task/RemapTaskConfiguration.java | 15 ++++-- .../task/service/TinyRemapperService.java | 52 ++++++++++++++++--- .../loom/util/TinyRemapperHelper.java | 2 +- .../integration/DebugLineNumbersTest.groovy | 2 +- .../test/integration/FabricAPITest.groovy | 10 ++-- .../test/integration/SimpleProjectTest.groovy | 22 +++++++- 10 files changed, 94 insertions(+), 34 deletions(-) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 4f3c55eb..4ebcdb38 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -6,7 +6,7 @@ gson = "2.10.1" guava = "33.0.0-jre" stitch = "0.6.2" -tiny-remapper = "0.9.0" +tiny-remapper = "0.10.0" access-widener = "2.1.0" mapping-io = "0.5.1" lorenz-tiny = "4.0.2" diff --git a/gradle/test.libs.versions.toml b/gradle/test.libs.versions.toml index f01ff02c..ef27d3a3 100644 --- a/gradle/test.libs.versions.toml +++ b/gradle/test.libs.versions.toml @@ -7,7 +7,7 @@ java-debug = "0.49.0" mixin = "0.11.4+mixin.0.8.5" gradle-nightly = "8.6-20231219002119+0000" -fabric-loader = "0.14.24" +fabric-loader = "0.15.3" fabric-installer = "0.11.1" [libraries] diff --git a/src/main/java/net/fabricmc/loom/configuration/mods/ModProcessor.java b/src/main/java/net/fabricmc/loom/configuration/mods/ModProcessor.java index b961df48..83c4bc7e 100644 --- a/src/main/java/net/fabricmc/loom/configuration/mods/ModProcessor.java +++ b/src/main/java/net/fabricmc/loom/configuration/mods/ModProcessor.java @@ -132,8 +132,6 @@ public class ModProcessor { private void remapJars(List remapList) throws IOException { final LoomGradleExtension extension = LoomGradleExtension.get(project); final MappingConfiguration mappingConfiguration = extension.getMappingConfiguration(); - Path[] mcDeps = project.getConfigurations().getByName(Constants.Configurations.MINECRAFT_COMPILE_LIBRARIES).getFiles() - .stream().map(File::toPath).toArray(Path[]::new); TinyRemapper.Builder builder = TinyRemapper.newRemapper() .withKnownIndyBsm(extension.getKnownIndyBsms().get()) @@ -164,11 +162,7 @@ public class ModProcessor { final TinyRemapper remapper = builder.build(); - for (Path minecraftJar : extension.getMinecraftJars(MappingsNamespace.INTERMEDIARY)) { - remapper.readClassPathAsync(minecraftJar); - } - - remapper.readClassPathAsync(mcDeps); + remapper.readClassPath(extension.getMinecraftJars(MappingsNamespace.INTERMEDIARY).toArray(Path[]::new)); final Map tagMap = new HashMap<>(); final Map outputConsumerMap = new HashMap<>(); @@ -178,7 +172,6 @@ public class ModProcessor { for (File inputFile : entry.getSourceConfiguration().get().getFiles()) { if (remapList.stream().noneMatch(info -> info.getInputFile().toFile().equals(inputFile))) { project.getLogger().debug("Adding " + inputFile + " onto the remap classpath"); - remapper.readClassPathAsync(inputFile.toPath()); } } diff --git a/src/main/java/net/fabricmc/loom/task/RemapJarTask.java b/src/main/java/net/fabricmc/loom/task/RemapJarTask.java index ca405b28..cb302af0 100644 --- a/src/main/java/net/fabricmc/loom/task/RemapJarTask.java +++ b/src/main/java/net/fabricmc/loom/task/RemapJarTask.java @@ -41,6 +41,7 @@ import javax.inject.Inject; import com.google.gson.JsonObject; import org.gradle.api.artifacts.Configuration; +import org.gradle.api.artifacts.ConfigurationContainer; import org.gradle.api.file.ConfigurableFileCollection; import org.gradle.api.file.FileCollection; import org.gradle.api.plugins.JavaPlugin; @@ -96,16 +97,11 @@ public abstract class RemapJarTask extends AbstractRemapJarTask { public RemapJarTask() { super(); serviceManagerProvider = BuildSharedServiceManager.createForTask(this, getBuildEventsListenerRegistry()); - - final Configuration compileClasspath = getProject().getConfigurations().getByName(JavaPlugin.COMPILE_CLASSPATH_CONFIGURATION_NAME); - final Configuration minecraftCompileLibraries = getProject().getConfigurations().getByName(Constants.Configurations.MINECRAFT_COMPILE_LIBRARIES); - // Filter out minecraft libraries from the classpath as we are 100% sure they don't play a part in the obfuscation - final FileCollection remapClasspath = compileClasspath.filter(file -> !minecraftCompileLibraries.getFiles().contains(file)); - - getClasspath().from(remapClasspath); + final ConfigurationContainer configurations = getProject().getConfigurations(); + getClasspath().from(configurations.getByName(JavaPlugin.COMPILE_CLASSPATH_CONFIGURATION_NAME)); getAddNestedDependencies().convention(true).finalizeValueOnRead(); - Configuration includeConfiguration = getProject().getConfigurations().getByName(Constants.Configurations.INCLUDE); + Configuration includeConfiguration = configurations.getByName(Constants.Configurations.INCLUDE); getNestedJars().from(new IncludedJarFactory(getProject()).getNestedJars(includeConfiguration)); getUseMixinAP().set(LoomGradleExtension.get(getProject()).getMixin().getUseLegacyMixinAp()); diff --git a/src/main/java/net/fabricmc/loom/task/RemapTaskConfiguration.java b/src/main/java/net/fabricmc/loom/task/RemapTaskConfiguration.java index a6303454..01eb92ac 100644 --- a/src/main/java/net/fabricmc/loom/task/RemapTaskConfiguration.java +++ b/src/main/java/net/fabricmc/loom/task/RemapTaskConfiguration.java @@ -26,6 +26,7 @@ package net.fabricmc.loom.task; import javax.inject.Inject; +import org.gradle.api.Action; import org.gradle.api.Project; import org.gradle.api.Task; import org.gradle.api.artifacts.Configuration; @@ -70,8 +71,7 @@ public abstract class RemapTaskConfiguration implements Runnable { return; } - // Register the default remap jar task - must not be lazy to ensure that the prepare tasks get setup for other projects to depend on. - RemapJarTask remapJarTask = getTasks().create(REMAP_JAR_TASK_NAME, RemapJarTask.class, task -> { + Action remapJarTaskAction = task -> { final AbstractArchiveTask jarTask = getTasks().named(JavaPlugin.JAR_TASK_NAME, AbstractArchiveTask.class).get(); // Basic task setup @@ -85,7 +85,14 @@ public abstract class RemapTaskConfiguration implements Runnable { task.getInputFile().convention(jarTask.getArchiveFile()); task.dependsOn(getTasks().named(JavaPlugin.JAR_TASK_NAME)); task.getIncludesClientOnlyClasses().set(getProject().provider(extension::areEnvironmentSourceSetsSplit)); - }); + }; + + if (extension.multiProjectOptimisation()) { + // must not be lazy to ensure that the prepare tasks get setup for other projects to depend on. + getTasks().create(REMAP_JAR_TASK_NAME, RemapJarTask.class, remapJarTaskAction); + } else { + getTasks().register(REMAP_JAR_TASK_NAME, RemapJarTask.class, remapJarTaskAction); + } // Configure the default jar task getTasks().named(JavaPlugin.JAR_TASK_NAME, AbstractArchiveTask.class).configure(task -> { @@ -93,7 +100,7 @@ public abstract class RemapTaskConfiguration implements Runnable { task.getDestinationDirectory().set(getProject().getLayout().getBuildDirectory().map(directory -> directory.dir("devlibs"))); }); - getTasks().named(BasePlugin.ASSEMBLE_TASK_NAME).configure(task -> task.dependsOn(remapJarTask)); + getTasks().named(BasePlugin.ASSEMBLE_TASK_NAME).configure(task -> task.dependsOn(getTasks().named(REMAP_JAR_TASK_NAME))); trySetupSourceRemapping(); diff --git a/src/main/java/net/fabricmc/loom/task/service/TinyRemapperService.java b/src/main/java/net/fabricmc/loom/task/service/TinyRemapperService.java index d576e07b..4d2c4098 100644 --- a/src/main/java/net/fabricmc/loom/task/service/TinyRemapperService.java +++ b/src/main/java/net/fabricmc/loom/task/service/TinyRemapperService.java @@ -38,15 +38,19 @@ import java.util.Set; import java.util.StringJoiner; import org.gradle.api.Project; +import org.gradle.api.artifacts.ConfigurationContainer; +import org.gradle.api.file.ConfigurableFileCollection; import org.gradle.api.invocation.Gradle; import org.gradle.api.model.ObjectFactory; import org.gradle.api.tasks.SourceSet; import org.jetbrains.annotations.Nullable; import net.fabricmc.loom.LoomGradleExtension; +import net.fabricmc.loom.api.mappings.layered.MappingsNamespace; import net.fabricmc.loom.build.mixin.AnnotationProcessorInvoker; import net.fabricmc.loom.extension.RemapperExtensionHolder; import net.fabricmc.loom.task.AbstractRemapJarTask; +import net.fabricmc.loom.util.Constants; import net.fabricmc.loom.util.gradle.GradleUtils; import net.fabricmc.loom.util.gradle.SourceSetHelper; import net.fabricmc.loom.util.kotlin.KotlinClasspath; @@ -66,6 +70,7 @@ public class TinyRemapperService implements SharedService { final LoomGradleExtension extension = LoomGradleExtension.get(project); final boolean legacyMixin = extension.getMixin().getUseLegacyMixinAp().get(); final @Nullable KotlinClasspathService kotlinClasspathService = KotlinClasspathService.getOrCreateIfRequired(serviceManager, project); + boolean multiProjectOptimisation = extension.multiProjectOptimisation(); // Generates an id that is used to share the remapper across projects. This tasks in the remap jar task name to handle custom remap jar tasks separately. final var joiner = new StringJoiner(":"); @@ -76,7 +81,7 @@ public class TinyRemapperService implements SharedService { joiner.add("kotlin-" + kotlinClasspathService.version()); } - if (remapJarTask.getRemapperIsolation().get() || !extension.multiProjectOptimisation()) { + if (remapJarTask.getRemapperIsolation().get() || !multiProjectOptimisation) { joiner.add(project.getPath()); } @@ -95,8 +100,34 @@ public class TinyRemapperService implements SharedService { return new TinyRemapperService(mappings, !legacyMixin, kotlinClasspathService, extension.getKnownIndyBsms().get(), extension.getRemapperExtensions().get(), from, to, project.getObjects()); }); - service.readClasspath(remapJarTask.getClasspath().getFiles().stream().map(File::toPath).filter(Files::exists).toList()); + final ConfigurationContainer configurations = project.getConfigurations(); + ConfigurableFileCollection excludedMinecraftJars = project.files(); + // Exclude none root minecraft jars. + if (multiProjectOptimisation && !extension.isRootProject()) { + MappingsNamespace mappingsNamespace = MappingsNamespace.of(from); + + if (mappingsNamespace != null) { + for (Path minecraftJar : extension.getMinecraftJars(mappingsNamespace)) { + excludedMinecraftJars.from(minecraftJar.toFile()); + } + } else { + // None fatal as this is a performance optimisation. + project.getLogger().warn("Unable to find minecraft jar for namespace {}", from); + } + } + + List classPath = remapJarTask.getClasspath() + .minus(configurations.getByName(Constants.Configurations.MINECRAFT_COMPILE_LIBRARIES)) + .minus(configurations.getByName(Constants.Configurations.MINECRAFT_RUNTIME_LIBRARIES)) + .minus(excludedMinecraftJars) + .getFiles() + .stream() + .map(File::toPath) + .filter(Files::exists) + .toList(); + + service.readClasspath(classPath); return service; } @@ -183,11 +214,21 @@ public class TinyRemapperService implements SharedService { } void readClasspath(List paths) { - List toRead; + List toRead = new ArrayList<>(); synchronized (classpath) { - toRead = paths.stream().filter(path -> !classpath.contains(path)).toList(); - classpath.addAll(paths); + for (Path path: paths) { + if (classpath.contains(path)) { + continue; + } + + toRead.add(path); + classpath.add(path); + } + } + + if (toRead.isEmpty()) { + return; } tinyRemapper.readClassPath(toRead.toArray(Path[]::new)); @@ -196,7 +237,6 @@ public class TinyRemapperService implements SharedService { @Override public void close() throws IOException { if (tinyRemapper != null) { - tinyRemapper.getEnvironment(); tinyRemapper.finish(); tinyRemapper = null; } diff --git a/src/main/java/net/fabricmc/loom/util/TinyRemapperHelper.java b/src/main/java/net/fabricmc/loom/util/TinyRemapperHelper.java index 0381a268..202e4646 100644 --- a/src/main/java/net/fabricmc/loom/util/TinyRemapperHelper.java +++ b/src/main/java/net/fabricmc/loom/util/TinyRemapperHelper.java @@ -1,7 +1,7 @@ /* * This file is part of fabric-loom, licensed under the MIT License (MIT). * - * Copyright (c) 2021 FabricMC + * Copyright (c) 2021-2023 FabricMC * * Permission is hereby granted, free of charge, to any person obtaining a copy * of this software and associated documentation files (the "Software"), to deal diff --git a/src/test/groovy/net/fabricmc/loom/test/integration/DebugLineNumbersTest.groovy b/src/test/groovy/net/fabricmc/loom/test/integration/DebugLineNumbersTest.groovy index b239301b..dd220320 100644 --- a/src/test/groovy/net/fabricmc/loom/test/integration/DebugLineNumbersTest.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/integration/DebugLineNumbersTest.groovy @@ -139,7 +139,7 @@ class DebugLineNumbersTest extends Specification implements GradleProjectTestTra } private static String getClassSource(GradleProject gradle, String classname, String mappings = MAPPINGS) { - File sourcesJar = gradle.getGeneratedSources(mappings, MinecraftJar.Server.NAME) + File sourcesJar = gradle.getGeneratedSources(mappings, MinecraftJar.Type.SERVER.toString()) return new String(ZipUtils.unpack(sourcesJar.toPath(), classname), StandardCharsets.UTF_8) } diff --git a/src/test/groovy/net/fabricmc/loom/test/integration/FabricAPITest.groovy b/src/test/groovy/net/fabricmc/loom/test/integration/FabricAPITest.groovy index bc0f3809..6107dc64 100644 --- a/src/test/groovy/net/fabricmc/loom/test/integration/FabricAPITest.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/integration/FabricAPITest.groovy @@ -44,7 +44,7 @@ class FabricAPITest extends Specification implements GradleProjectTestTrait { setup: def gradle = gradleProject( repo: "https://github.com/FabricMC/fabric.git", - commit: "23e8616e7457d7d4a65119b93952d134607ffc5c", + commit: "efa5891941a32589207dc58c2e77183d599465b8", version: version, patch: "fabric_api" ) @@ -60,7 +60,7 @@ class FabricAPITest extends Specification implements GradleProjectTestTrait { """.stripIndent() } - def minecraftVersion = "23w45a" + def minecraftVersion = "23w51b" def server = ServerRunner.create(gradle.projectDir, minecraftVersion) .withMod(gradle.getOutputFile("fabric-api-999.0.0.jar")) @@ -106,8 +106,8 @@ class FabricAPITest extends Specification implements GradleProjectTestTrait { result.task(":prepareRemapJar").outcome == SUCCESS def biomeApiJar = new File(gradle.mavenLocalDir, "net/fabricmc/fabric-api/fabric-biome-api-v1/999.0.0/fabric-biome-api-v1-999.0.0.jar") - def manifest = ZipUtils.unpack(biomeApiJar.toPath(), "META-INF/MANIFEST.MF").toString() new File(gradle.mavenLocalDir, "net/fabricmc/fabric-api/fabric-biome-api-v1/999.0.0/fabric-biome-api-v1-999.0.0-sources.jar").exists() + def manifest = ZipUtils.unpack(biomeApiJar.toPath(), "META-INF/MANIFEST.MF").toString() if (disableMixinAp) { manifest.contains("Fabric-Loom-Mixin-Remap-Type=static") @@ -115,6 +115,10 @@ class FabricAPITest extends Specification implements GradleProjectTestTrait { manifest.contains("Fabric-Loom-Mixin-Remap-Type=mixin") } + // Check that a client mixin exists + def blockViewApiJar = new File(gradle.mavenLocalDir, "net/fabricmc/fabric-api/fabric-block-view-api-v2/999.0.0/fabric-block-view-api-v2-999.0.0.jar") + ZipUtils.contains(blockViewApiJar.toPath(), "net/fabricmc/fabric/mixin/blockview/client/ChunkRendererRegionBuilderMixin.class") + serverResult.successful() serverResult.output.contains("- fabric-api 999.0.0") diff --git a/src/test/groovy/net/fabricmc/loom/test/integration/SimpleProjectTest.groovy b/src/test/groovy/net/fabricmc/loom/test/integration/SimpleProjectTest.groovy index ff7bda77..1d2e1425 100644 --- a/src/test/groovy/net/fabricmc/loom/test/integration/SimpleProjectTest.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/integration/SimpleProjectTest.groovy @@ -33,7 +33,8 @@ import spock.lang.Unroll import net.fabricmc.loom.test.util.GradleProjectTestTrait import net.fabricmc.loom.test.util.ServerRunner -import static net.fabricmc.loom.test.LoomTestConstants.* +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.SUCCESS @Timeout(value = 20, unit = TimeUnit.MINUTES) @@ -80,4 +81,23 @@ class SimpleProjectTest extends Specification implements GradleProjectTestTrait 'eclipse' | _ 'vscode' | _ } + + @Unroll + def "remap mixins with tiny-remapper"() { + setup: + def gradle = gradleProject(project: "simple", version: PRE_RELEASE_GRADLE) + gradle.buildGradle << """ + allprojects { + loom.mixin.useLegacyMixinAp = false + } + """.stripIndent() + + when: + def result = gradle.run(task: "build") + + then: + result.task(":build").outcome == SUCCESS + !result.output.contains("[WARN] [MIXIN]") // Assert that tiny remapper didnt not have any warnings when remapping + gradle.getOutputZipEntry("fabric-example-mod-1.0.0.jar", "META-INF/MANIFEST.MF").contains("Fabric-Loom-Version: 0.0.0+unknown") + } } From 793388cbfb333d08bc274abbb9afadabcecc69b9 Mon Sep 17 00:00:00 2001 From: modmuss50 Date: Thu, 4 Jan 2024 14:07:12 +0000 Subject: [PATCH 4/7] Update libs --- gradle/libs.versions.toml | 4 ++-- gradle/runtime.libs.versions.toml | 2 +- gradle/test.libs.versions.toml | 8 ++++---- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 4ebcdb38..ee984081 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -1,7 +1,7 @@ [versions] kotlin = "1.9.0" asm = "9.6" -commons-io = "2.15.0" +commons-io = "2.15.1" gson = "2.10.1" guava = "33.0.0-jre" @@ -16,7 +16,7 @@ kotlinx-metadata = "0.8.0" # Plugins spotless = "6.20.0" test-retry = "1.5.6" -checkstyle = "10.12.5" +checkstyle = "10.12.7" codenarc = "3.3.0" jacoco = "0.8.11" diff --git a/gradle/runtime.libs.versions.toml b/gradle/runtime.libs.versions.toml index 84f7acc3..8b6872bf 100644 --- a/gradle/runtime.libs.versions.toml +++ b/gradle/runtime.libs.versions.toml @@ -7,7 +7,7 @@ vineflower = "1.9.3" # Runtime depedencies mixin-compile-extensions = "0.6.0" dev-launch-injector = "0.2.1+build.8" -terminal-console-appender = "1.2.0" +terminal-console-appender = "1.3.0" jetbrains-annotations = "24.1.0" native-support = "1.0.1" diff --git a/gradle/test.libs.versions.toml b/gradle/test.libs.versions.toml index ef27d3a3..08fa5b91 100644 --- a/gradle/test.libs.versions.toml +++ b/gradle/test.libs.versions.toml @@ -2,13 +2,13 @@ spock = "2.3-groovy-3.0" junit = "5.10.1" javalin = "5.6.3" -mockito = "5.7.0" -java-debug = "0.49.0" +mockito = "5.8.0" +java-debug = "0.50.0" mixin = "0.11.4+mixin.0.8.5" -gradle-nightly = "8.6-20231219002119+0000" +gradle-nightly = "8.7-20240104001326+0000" fabric-loader = "0.15.3" -fabric-installer = "0.11.1" +fabric-installer = "1.0.0" [libraries] spock = { module = "org.spockframework:spock-core", version.ref = "spock" } From c52f868af43d2ea14bd9d981c65950b28f4bf0bf Mon Sep 17 00:00:00 2001 From: modmuss50 Date: Thu, 4 Jan 2024 14:43:50 +0000 Subject: [PATCH 5/7] Revert checkstyle update for now. --- gradle/libs.versions.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index ee984081..0b2075ca 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -16,7 +16,7 @@ kotlinx-metadata = "0.8.0" # Plugins spotless = "6.20.0" test-retry = "1.5.6" -checkstyle = "10.12.7" +checkstyle = "10.12.5" codenarc = "3.3.0" jacoco = "0.8.11" From 8df229313e3dd143af95903021da861d4eb9e638 Mon Sep 17 00:00:00 2001 From: Jason Penilla <11360596+jpenilla@users.noreply.github.com> Date: Fri, 5 Jan 2024 10:35:39 -0800 Subject: [PATCH 6/7] Cache FMJs and ArtifactMetadata when reading during configuration time (#1018) We have to read every jar on remapped configs to see if they are a mod or should otherwise be remapped. By caching we can avoid re-reading jars that are java-runtime & java-api or are on multiple remapped configs. The cache scope could probably be widened to help more with multi-project builds, but I am leaving that for future work as I think it would also require some sort of invalidation. --- .../mods/ModConfigurationRemapper.java | 13 +++--- .../processors/SpecContextImpl.java | 45 +++++++++++++------ 2 files changed, 39 insertions(+), 19 deletions(-) diff --git a/src/main/java/net/fabricmc/loom/configuration/mods/ModConfigurationRemapper.java b/src/main/java/net/fabricmc/loom/configuration/mods/ModConfigurationRemapper.java index 1564e8bb..61a4d844 100644 --- a/src/main/java/net/fabricmc/loom/configuration/mods/ModConfigurationRemapper.java +++ b/src/main/java/net/fabricmc/loom/configuration/mods/ModConfigurationRemapper.java @@ -137,6 +137,7 @@ public class ModConfigurationRemapper { // the installer data. The installer data has to be added before // any mods are remapped since remapping needs the dependencies provided by that data. final Map> dependenciesBySourceConfig = new HashMap<>(); + final Map metaCache = new HashMap<>(); configsToRemap.forEach((sourceConfig, remappedConfig) -> { /* sourceConfig - The source configuration where the intermediary named artifacts come from. i.e "modApi" @@ -148,11 +149,13 @@ public class ModConfigurationRemapper { for (ArtifactRef artifact : resolveArtifacts(project, sourceConfig)) { final ArtifactMetadata artifactMetadata; - try { - artifactMetadata = ArtifactMetadata.create(artifact, LoomGradlePlugin.LOOM_VERSION); - } catch (IOException e) { - throw ExceptionUtil.createDescriptiveWrapper(UncheckedIOException::new, "Failed to read metadata from " + artifact.path(), e); - } + artifactMetadata = metaCache.computeIfAbsent(artifact, a -> { + try { + return ArtifactMetadata.create(a, LoomGradlePlugin.LOOM_VERSION); + } catch (IOException e) { + throw ExceptionUtil.createDescriptiveWrapper(UncheckedIOException::new, "Failed to read metadata from " + a.path(), e); + } + }); if (artifactMetadata.installerData() != null) { if (extension.getInstallerData() != null) { diff --git a/src/main/java/net/fabricmc/loom/configuration/processors/SpecContextImpl.java b/src/main/java/net/fabricmc/loom/configuration/processors/SpecContextImpl.java index 622a0a1c..97e96a47 100644 --- a/src/main/java/net/fabricmc/loom/configuration/processors/SpecContextImpl.java +++ b/src/main/java/net/fabricmc/loom/configuration/processors/SpecContextImpl.java @@ -29,8 +29,10 @@ import java.nio.file.Path; import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; +import java.util.HashMap; import java.util.List; -import java.util.Optional; +import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.function.Function; import java.util.stream.Stream; @@ -57,11 +59,12 @@ import net.fabricmc.loom.util.gradle.GradleUtils; */ public record SpecContextImpl(List modDependencies, List localMods, List compileRuntimeMods) implements SpecContext { public static SpecContextImpl create(Project project) { - return new SpecContextImpl(getDependentMods(project), FabricModJsonHelpers.getModsInProject(project), getCompileRuntimeMods(project)); + final Map> fmjCache = new HashMap<>(); + return new SpecContextImpl(getDependentMods(project, fmjCache), FabricModJsonHelpers.getModsInProject(project), getCompileRuntimeMods(project, fmjCache)); } // Reruns a list of mods found on both the compile and/or runtime classpaths - private static List getDependentMods(Project project) { + private static List getDependentMods(Project project, Map> fmjCache) { final LoomGradleExtension extension = LoomGradleExtension.get(project); var mods = new ArrayList(); @@ -69,10 +72,14 @@ public record SpecContextImpl(List modDependencies, List artifacts = entry.getSourceConfiguration().get().resolve(); for (File artifact : artifacts) { - final FabricModJson fabricModJson = FabricModJsonFactory.createFromZipNullable(artifact.toPath()); + final List fabricModJson = fmjCache.computeIfAbsent(artifact.toPath().toAbsolutePath().toString(), $ -> { + return FabricModJsonFactory.createFromZipOptional(artifact.toPath()) + .map(List::of) + .orElseGet(List::of); + }); - if (fabricModJson != null) { - mods.add(fabricModJson); + if (!fabricModJson.isEmpty()) { + mods.add(fabricModJson.get(0)); } } } @@ -80,7 +87,9 @@ public record SpecContextImpl(List modDependencies, List { + return FabricModJsonHelpers.getModsInProject(dependentProject); + })); } } @@ -96,18 +105,20 @@ public record SpecContextImpl(List modDependencies, List getCompileRuntimeMods(Project project) { - var mods = new ArrayList<>(getCompileRuntimeModsFromRemapConfigs(project).toList()); + private static List getCompileRuntimeMods(Project project, Map> fmjCache) { + var mods = new ArrayList<>(getCompileRuntimeModsFromRemapConfigs(project, fmjCache).toList()); for (Project dependentProject : getCompileRuntimeProjectDependencies(project).toList()) { - mods.addAll(FabricModJsonHelpers.getModsInProject(dependentProject)); + mods.addAll(fmjCache.computeIfAbsent(dependentProject.getPath(), $ -> { + return FabricModJsonHelpers.getModsInProject(dependentProject); + })); } return Collections.unmodifiableList(mods); } // Returns a list of jar mods that are found on the compile and runtime remapping configurations - private static Stream getCompileRuntimeModsFromRemapConfigs(Project project) { + private static Stream getCompileRuntimeModsFromRemapConfigs(Project project, Map> fmjCache) { final LoomGradleExtension extension = LoomGradleExtension.get(project); final List runtimeEntries = extension.getRuntimeRemapConfigurations().stream() .filter(settings -> settings.getApplyDependencyTransforms().get()) @@ -118,9 +129,15 @@ public record SpecContextImpl(List modDependencies, List settings.getApplyDependencyTransforms().get()) .flatMap(resolveArtifacts(project, false)) .filter(runtimeEntries::contains) // Use the intersection of the two configurations. - .map(FabricModJsonFactory::createFromZipOptional) - .filter(Optional::isPresent) - .map(Optional::get) + .map(zipPath -> { + final List list = fmjCache.computeIfAbsent(zipPath.toAbsolutePath().toString(), $ -> { + return FabricModJsonFactory.createFromZipOptional(zipPath) + .map(List::of) + .orElseGet(List::of); + }); + return list.isEmpty() ? null : list.get(0); + }) + .filter(Objects::nonNull) .sorted(Comparator.comparing(FabricModJson::getId)); } From 480dd5e393c29cad65229743098f3d79f56449ac Mon Sep 17 00:00:00 2001 From: modmuss Date: Sat, 6 Jan 2024 17:17:26 +0000 Subject: [PATCH 7/7] Some minor peformance improvements (#1019) --- .../net/fabricmc/loom/task/RemapJarTask.java | 11 ++--- .../fabricmc/loom/util/download/Download.java | 21 ++++---- .../test/benchmark/FabricAPIBenchmark.groovy | 12 ++++- .../unit/download/DownloadFileTest.groovy | 48 ++++++++++++++++++- 4 files changed, 72 insertions(+), 20 deletions(-) diff --git a/src/main/java/net/fabricmc/loom/task/RemapJarTask.java b/src/main/java/net/fabricmc/loom/task/RemapJarTask.java index cb302af0..0948b353 100644 --- a/src/main/java/net/fabricmc/loom/task/RemapJarTask.java +++ b/src/main/java/net/fabricmc/loom/task/RemapJarTask.java @@ -1,7 +1,7 @@ /* * This file is part of fabric-loom, licensed under the MIT License (MIT). * - * Copyright (c) 2021-2022 FabricMC + * Copyright (c) 2021-2024 FabricMC * * Permission is hereby granted, free of charge, to any person obtaining a copy * of this software and associated documentation files (the "Software"), to deal @@ -124,12 +124,9 @@ public abstract class RemapJarTask extends AbstractRemapJarTask { mustRunAfter(prepareJarTask); getProject().getGradle().allprojects(project -> { - project.getTasks().configureEach(task -> { - if (task instanceof PrepareJarRemapTask otherTask) { - // Ensure that all remap jars run after all prepare tasks - mustRunAfter(otherTask); - } - }); + project.getTasks() + .withType(PrepareJarRemapTask.class) + .configureEach(this::mustRunAfter); }); } diff --git a/src/main/java/net/fabricmc/loom/util/download/Download.java b/src/main/java/net/fabricmc/loom/util/download/Download.java index 79c200cc..bdd9b04b 100644 --- a/src/main/java/net/fabricmc/loom/util/download/Download.java +++ b/src/main/java/net/fabricmc/loom/util/download/Download.java @@ -1,7 +1,7 @@ /* * This file is part of fabric-loom, licensed under the MIT License (MIT). * - * Copyright (c) 2022 FabricMC + * Copyright (c) 2022-2024 FabricMC * * Permission is hereby granted, free of charge, to any person obtaining a copy * of this software and associated documentation files (the "Software"), to deal @@ -43,7 +43,6 @@ import java.nio.file.FileSystems; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardOpenOption; -import java.nio.file.attribute.BasicFileAttributeView; import java.nio.file.attribute.FileTime; import java.time.Duration; import java.time.Instant; @@ -191,6 +190,13 @@ public final class Download { boolean success = statusCode == HttpURLConnection.HTTP_NOT_MODIFIED || (statusCode >= 200 && statusCode < 300); if (statusCode == HttpURLConnection.HTTP_NOT_MODIFIED) { + try { + // Update the last modified time so we don't retry the request until the max age has passed again. + Files.setLastModifiedTime(output, FileTime.from(Instant.now())); + } catch (IOException e) { + throw error(e, "Failed to update last modified time"); + } + // Success, etag matched. return; } @@ -365,9 +371,9 @@ public final class Download { private boolean isOutdated(Path path) throws DownloadException { try { - final FileTime lastModified = getLastModified(path); - return lastModified.toInstant().plus(maxAge) - .isBefore(Instant.now()); + final FileTime lastModified = Files.getLastModifiedTime(path); + return lastModified.toInstant() + .isBefore(Instant.now().minus(maxAge)); } catch (IOException e) { throw error(e, "Failed to check if (%s) is outdated", path); } @@ -430,11 +436,6 @@ public final class Download { return path.getFileSystem() == FileSystems.getDefault() ? path.toFile().exists() : Files.exists(path); } - private FileTime getLastModified(Path path) throws IOException { - final BasicFileAttributeView basicView = Files.getFileAttributeView(path, BasicFileAttributeView.class); - return basicView.readAttributes().lastModifiedTime(); - } - private Path getLockFile(Path output) { return output.resolveSibling(output.getFileName() + ".lock"); } diff --git a/src/test/groovy/net/fabricmc/loom/test/benchmark/FabricAPIBenchmark.groovy b/src/test/groovy/net/fabricmc/loom/test/benchmark/FabricAPIBenchmark.groovy index cad874f3..6857224d 100644 --- a/src/test/groovy/net/fabricmc/loom/test/benchmark/FabricAPIBenchmark.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/benchmark/FabricAPIBenchmark.groovy @@ -1,7 +1,7 @@ /* * This file is part of fabric-loom, licensed under the MIT License (MIT). * - * Copyright (c) 2022 FabricMC + * Copyright (c) 2022-2024 FabricMC * * Permission is hereby granted, free of charge, to any person obtaining a copy * of this software and associated documentation files (the "Software"), to deal @@ -44,12 +44,20 @@ class FabricAPIBenchmark implements GradleProjectTestTrait { allowExistingRepo: true, repo: "https://github.com/FabricMC/fabric.git", - commit: "2facd446984085376bd23245410ebf2dc0881b02", + commit: "efa5891941a32589207dc58c2e77183d599465b8", patch: "fabric_api" ) gradle.enableMultiProjectOptimisation() + if (!gradle.buildGradle.text.contains("loom.mixin.useLegacyMixinAp")) { + gradle.buildGradle << """ + allprojects { + loom.mixin.useLegacyMixinAp = false + } + """.stripIndent() + } + def timeStart = new Date() def result = gradle.run(tasks: [ diff --git a/src/test/groovy/net/fabricmc/loom/test/unit/download/DownloadFileTest.groovy b/src/test/groovy/net/fabricmc/loom/test/unit/download/DownloadFileTest.groovy index 1d977203..cb3edfc3 100644 --- a/src/test/groovy/net/fabricmc/loom/test/unit/download/DownloadFileTest.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/unit/download/DownloadFileTest.groovy @@ -1,7 +1,7 @@ /* * This file is part of fabric-loom, licensed under the MIT License (MIT). * - * Copyright (c) 2022 FabricMC + * Copyright (c) 2022-2024 FabricMC * * Permission is hereby granted, free of charge, to any person obtaining a copy * of this software and associated documentation files (the "Software"), to deal @@ -234,6 +234,52 @@ class DownloadFileTest extends DownloadTest { requestCount == 1 } + def "ETag with max age"() { + setup: + // The count of requests, can return early if the ETag matches. + int requestCount = 0 + // The count of full downloads + int downloadCount = 0 + + server.get("/etag") { + def clientEtag = it.req.getHeader("If-None-Match") + + def result = "Hello world" + def etag = result.hashCode().toString() + it.header("ETag", etag) + + requestCount ++ + + if (clientEtag == etag) { + // Etag matches, no need to send the data. + it.status(HttpStatus.NOT_MODIFIED) + return + } + + it.result(result) + downloadCount ++ + } + + def output = new File(File.createTempDir(), "etag.txt").toPath() + + when: + for (i in 0..<3) { + if (i == 2) { + // On the third request, set the file to be 2 days old, forcing the etag to be checked. + Files.setLastModifiedTime(output, FileTime.from(Instant.now() - Duration.ofDays(2))) + } + + Download.create("$PATH/etag") + .etag(true) + .maxAge(Duration.ofDays(1)) + .downloadPath(output) + } + + then: + requestCount == 2 + downloadCount == 1 + } + def "Progress: File"() { setup: server.get("/progressFile") {