From dbebbdb9442f2557c492799b4185788b5254ee87 Mon Sep 17 00:00:00 2001 From: modmuss Date: Tue, 12 Mar 2024 19:11:26 +0000 Subject: [PATCH] Add RemapJarTask.getOptimizeFabricModJson() (#1068) * Optimise fabric.mod.json files * Fixes * Make opt-in * Revert * Fix --- .../signatures/SignatureFixesLayerImpl.java | 2 +- .../parchment/ParchmentMappingLayer.java | 2 +- .../net/fabricmc/loom/task/RemapJarTask.java | 25 ++++ .../java/net/fabricmc/loom/util/ZipUtils.java | 10 +- .../loom/util/fmj/FabricModJsonFactory.java | 2 +- .../loom/util/fmj/FabricModJsonUtils.java | 27 +++- .../loom/test/unit/ZipUtilsTest.groovy | 25 ++++ .../unit/fmj/FabricModJsonUtilsTest.groovy | 128 ++++++++++++++++++ 8 files changed, 216 insertions(+), 5 deletions(-) create mode 100644 src/test/groovy/net/fabricmc/loom/test/unit/fmj/FabricModJsonUtilsTest.groovy diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/extras/signatures/SignatureFixesLayerImpl.java b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/extras/signatures/SignatureFixesLayerImpl.java index 4328311b..b4ea05c8 100644 --- a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/extras/signatures/SignatureFixesLayerImpl.java +++ b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/extras/signatures/SignatureFixesLayerImpl.java @@ -47,7 +47,7 @@ public record SignatureFixesLayerImpl(Path mappingsFile) implements MappingLayer public Map getSignatureFixes() { try { //noinspection unchecked - return ZipUtils.unpackJackson(mappingsFile(), SIGNATURE_FIXES_PATH, Map.class); + return ZipUtils.unpackJson(mappingsFile(), SIGNATURE_FIXES_PATH, Map.class); } catch (IOException e) { throw new RuntimeException("Failed to extract signature fixes", e); } diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/parchment/ParchmentMappingLayer.java b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/parchment/ParchmentMappingLayer.java index 0e2f68c6..16af9d5c 100644 --- a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/parchment/ParchmentMappingLayer.java +++ b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/parchment/ParchmentMappingLayer.java @@ -47,6 +47,6 @@ public record ParchmentMappingLayer(Path parchmentFile, boolean removePrefix) im } private ParchmentTreeV1 getParchmentData() throws IOException { - return ZipUtils.unpackJackson(parchmentFile, PARCHMENT_DATA_FILE_NAME, ParchmentTreeV1.class); + return ZipUtils.unpackJson(parchmentFile, PARCHMENT_DATA_FILE_NAME, ParchmentTreeV1.class); } } diff --git a/src/main/java/net/fabricmc/loom/task/RemapJarTask.java b/src/main/java/net/fabricmc/loom/task/RemapJarTask.java index 0948b353..54dd46bb 100644 --- a/src/main/java/net/fabricmc/loom/task/RemapJarTask.java +++ b/src/main/java/net/fabricmc/loom/task/RemapJarTask.java @@ -75,6 +75,7 @@ import net.fabricmc.loom.util.SidedClassVisitor; import net.fabricmc.loom.util.ZipUtils; import net.fabricmc.loom.util.fmj.FabricModJson; import net.fabricmc.loom.util.fmj.FabricModJsonFactory; +import net.fabricmc.loom.util.fmj.FabricModJsonUtils; import net.fabricmc.loom.util.service.BuildSharedServiceManager; import net.fabricmc.loom.util.service.UnsafeWorkQueueHelper; import net.fabricmc.tinyremapper.OutputConsumerPath; @@ -87,6 +88,14 @@ public abstract class RemapJarTask extends AbstractRemapJarTask { @Input public abstract Property getAddNestedDependencies(); + /** + * Whether to optimize the fabric.mod.json file, by default this is false. + * + *

The schemaVersion entry will be placed first in the json file + */ + @Input + public abstract Property getOptimizeFabricModJson(); + @Input @ApiStatus.Internal public abstract Property getUseMixinAP(); @@ -100,6 +109,7 @@ public abstract class RemapJarTask extends AbstractRemapJarTask { final ConfigurationContainer configurations = getProject().getConfigurations(); getClasspath().from(configurations.getByName(JavaPlugin.COMPILE_CLASSPATH_CONFIGURATION_NAME)); getAddNestedDependencies().convention(true).finalizeValueOnRead(); + getOptimizeFabricModJson().convention(false).finalizeValueOnRead(); Configuration includeConfiguration = configurations.getByName(Constants.Configurations.INCLUDE); getNestedJars().from(new IncludedJarFactory(getProject()).getNestedJars(includeConfiguration)); @@ -156,6 +166,8 @@ public abstract class RemapJarTask extends AbstractRemapJarTask { final var refmapRemapType = mixinAp ? ArtifactMetadata.MixinRemapType.MIXIN : ArtifactMetadata.MixinRemapType.STATIC; params.getManifestAttributes().put(Constants.Manifest.MIXIN_REMAP_TYPE, refmapRemapType.manifestValue()); } + + params.getOptimizeFmj().set(getOptimizeFabricModJson().get()); }); } @@ -197,6 +209,7 @@ public abstract class RemapJarTask extends AbstractRemapJarTask { Property getUseMixinExtension(); Property getMultiProjectOptimisation(); + Property getOptimizeFmj(); record RefmapData(List mixinConfigs, String refmapName) implements Serializable { } ListProperty getMixinData(); @@ -243,6 +256,10 @@ public abstract class RemapJarTask extends AbstractRemapJarTask { modifyJarManifest(); rewriteJar(); + if (getParameters().getOptimizeFmj().get()) { + optimizeFMJ(); + } + if (tinyRemapperService != null && !getParameters().getMultiProjectOptimisation().get()) { tinyRemapperService.close(); } @@ -349,6 +366,14 @@ public abstract class RemapJarTask extends AbstractRemapJarTask { } } } + + private void optimizeFMJ() throws IOException { + if (!ZipUtils.contains(outputFile, FabricModJsonFactory.FABRIC_MOD_JSON)) { + return; + } + + ZipUtils.transformJson(JsonObject.class, outputFile, FabricModJsonFactory.FABRIC_MOD_JSON, FabricModJsonUtils::optimizeFmj); + } } @Override diff --git a/src/main/java/net/fabricmc/loom/util/ZipUtils.java b/src/main/java/net/fabricmc/loom/util/ZipUtils.java index 9b387ad9..8fdf1fd6 100644 --- a/src/main/java/net/fabricmc/loom/util/ZipUtils.java +++ b/src/main/java/net/fabricmc/loom/util/ZipUtils.java @@ -121,7 +121,7 @@ public class ZipUtils { } } - public static T unpackJackson(Path zip, String path, Class clazz) throws IOException { + public static T unpackJson(Path zip, String path, Class clazz) throws IOException { final byte[] bytes = unpack(zip, path); return LoomGradlePlugin.GSON.fromJson(new String(bytes, StandardCharsets.UTF_8), clazz); } @@ -209,6 +209,14 @@ public class ZipUtils { s -> LoomGradlePlugin.GSON.toJson(s, typeOfT).getBytes(StandardCharsets.UTF_8)); } + public static void transformJson(Class typeOfT, Path zip, String path, UnsafeUnaryOperator transformer) throws IOException { + int transformed = transformJson(typeOfT, zip, Map.of(path, transformer)); + + if (transformed != 1) { + throw new IOException("Failed to transform " + path + " in " + zip); + } + } + public static int transform(Path zip, Collection>> transforms) throws IOException { return transform(zip, transforms.stream()); } diff --git a/src/main/java/net/fabricmc/loom/util/fmj/FabricModJsonFactory.java b/src/main/java/net/fabricmc/loom/util/fmj/FabricModJsonFactory.java index bbd22916..4bdeb403 100644 --- a/src/main/java/net/fabricmc/loom/util/fmj/FabricModJsonFactory.java +++ b/src/main/java/net/fabricmc/loom/util/fmj/FabricModJsonFactory.java @@ -46,7 +46,7 @@ import net.fabricmc.loom.util.ZipUtils; import net.fabricmc.loom.util.gradle.SourceSetHelper; public final class FabricModJsonFactory { - private static final String FABRIC_MOD_JSON = "fabric.mod.json"; + public static final String FABRIC_MOD_JSON = "fabric.mod.json"; private FabricModJsonFactory() { } diff --git a/src/main/java/net/fabricmc/loom/util/fmj/FabricModJsonUtils.java b/src/main/java/net/fabricmc/loom/util/fmj/FabricModJsonUtils.java index 78b94032..5559551b 100644 --- a/src/main/java/net/fabricmc/loom/util/fmj/FabricModJsonUtils.java +++ b/src/main/java/net/fabricmc/loom/util/fmj/FabricModJsonUtils.java @@ -25,13 +25,14 @@ package net.fabricmc.loom.util.fmj; import java.util.Locale; +import java.util.Map; import java.util.function.Predicate; import com.google.gson.JsonElement; import com.google.gson.JsonObject; import com.google.gson.JsonPrimitive; -final class FabricModJsonUtils { +public final class FabricModJsonUtils { private FabricModJsonUtils() { } @@ -49,6 +50,30 @@ final class FabricModJsonUtils { return element.getAsInt(); } + // Ensure that the schemaVersion json entry, is first in the json file + // This exercises an optimisation here: https://github.com/FabricMC/fabric-loader/blob/d69cb72d26497e3f387cf46f9b24340b402a4644/src/main/java/net/fabricmc/loader/impl/metadata/ModMetadataParser.java#L62 + public static JsonObject optimizeFmj(JsonObject json) { + if (!json.has("schemaVersion")) { + // No schemaVersion, something will explode later?! + return json; + } + + // Create a new json object with the schemaVersion first + var out = new JsonObject(); + out.add("schemaVersion", json.get("schemaVersion")); + + for (Map.Entry entry : json.entrySet()) { + if (entry.getKey().equals("schemaVersion")) { + continue; + } + + // Add all other entries + out.add(entry.getKey(), entry.getValue()); + } + + return out; + } + private static JsonElement getElement(JsonObject jsonObject, String key) { final JsonElement element = jsonObject.get(key); diff --git a/src/test/groovy/net/fabricmc/loom/test/unit/ZipUtilsTest.groovy b/src/test/groovy/net/fabricmc/loom/test/unit/ZipUtilsTest.groovy index e967638e..7798a5c0 100644 --- a/src/test/groovy/net/fabricmc/loom/test/unit/ZipUtilsTest.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/unit/ZipUtilsTest.groovy @@ -28,6 +28,7 @@ import java.nio.charset.StandardCharsets import java.nio.file.Files import java.time.ZoneId +import com.google.gson.JsonObject import spock.lang.Specification import net.fabricmc.loom.util.Checksum @@ -188,4 +189,28 @@ class ZipUtilsTest extends Specification { "Etc/GMT-6" | _ "Etc/GMT+9" | _ } + + def "transform json"() { + given: + def dir = File.createTempDir() + def zip = File.createTempFile("loom-zip-test", ".zip").toPath() + new File(dir, "test.json").text = """ + { + "test": "This is a test of transforming" + } + """ + ZipUtils.pack(dir.toPath(), zip) + + when: + ZipUtils.transformJson(JsonObject.class, zip, "test.json") { json -> + def test = json.get("test").getAsString() + json.addProperty("test", test.toUpperCase()) + json + } + + def transformed = ZipUtils.unpackJson(zip, "test.json", JsonObject.class) + + then: + transformed.get("test").asString == "THIS IS A TEST OF TRANSFORMING" + } } diff --git a/src/test/groovy/net/fabricmc/loom/test/unit/fmj/FabricModJsonUtilsTest.groovy b/src/test/groovy/net/fabricmc/loom/test/unit/fmj/FabricModJsonUtilsTest.groovy new file mode 100644 index 00000000..c0c42284 --- /dev/null +++ b/src/test/groovy/net/fabricmc/loom/test/unit/fmj/FabricModJsonUtilsTest.groovy @@ -0,0 +1,128 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2022 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 + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +package net.fabricmc.loom.test.unit.fmj + +import com.google.gson.GsonBuilder +import com.google.gson.JsonObject +import org.intellij.lang.annotations.Language +import spock.lang.Specification + +import net.fabricmc.loom.util.fmj.FabricModJsonUtils + +class FabricModJsonUtilsTest extends Specification { + // Test that the schemaVersion is moved to the first position + def "optimize FMJ"() { + given: + // Matches LoomGradlePlugin + def gson = new GsonBuilder().setPrettyPrinting().create() + def json = gson.fromJson(INPUT_FMJ, JsonObject.class) + when: + def outputJson = FabricModJsonUtils.optimizeFmj(json) + def output = gson.toJson(outputJson) + then: + output == OUTPUT_FMJ + true + } + + // schemaVersion is not first + @Language("json") + static String INPUT_FMJ = """ +{ + "id": "modid", + "version": "1.0.0", + "name": "Example mod", + "description": "This is an example description! Tell everyone what your mod is about!", + "license": "CC0-1.0", + "icon": "assets/modid/icon.png", + "environment": "*", + "entrypoints": { + "main": [ + "com.example.ExampleMod" + ], + "client": [ + "com.example.ExampleModClient" + ] + }, + "schemaVersion": 1, + "mixins": [ + "modid.mixins.json", + { + "config": "modid.client.mixins.json", + "environment": "client" + } + ], + "depends": { + "fabricloader": "\\u003e\\u003d0.15.0", + "minecraft": "~1.20.4", + "java": "\\u003e\\u003d17", + "fabric-api": "*" + }, + "suggests": { + "another-mod": "*" + } +} + +""".trim() + + // schemaVersion is first, everything else is unchanged + @Language("json") + static String OUTPUT_FMJ = """ +{ + "schemaVersion": 1, + "id": "modid", + "version": "1.0.0", + "name": "Example mod", + "description": "This is an example description! Tell everyone what your mod is about!", + "license": "CC0-1.0", + "icon": "assets/modid/icon.png", + "environment": "*", + "entrypoints": { + "main": [ + "com.example.ExampleMod" + ], + "client": [ + "com.example.ExampleModClient" + ] + }, + "mixins": [ + "modid.mixins.json", + { + "config": "modid.client.mixins.json", + "environment": "client" + } + ], + "depends": { + "fabricloader": "\\u003e\\u003d0.15.0", + "minecraft": "~1.20.4", + "java": "\\u003e\\u003d17", + "fabric-api": "*" + }, + "suggests": { + "another-mod": "*" + } +} + +""".trim() +}