From e3b51e9e97823fa278b90a875cb9cfbe62e1de06 Mon Sep 17 00:00:00 2001 From: shedaniel Date: Mon, 13 Nov 2023 14:52:02 +0800 Subject: [PATCH] Fix Field Migration (#168) * Fix #162 * Refactor some field migrator code --- .../FieldMigratedMappingConfiguration.java | 112 ++++++++---------- .../mappings/MappingConfiguration.java | 3 +- 2 files changed, 51 insertions(+), 64 deletions(-) diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/forge/FieldMigratedMappingConfiguration.java b/src/main/java/net/fabricmc/loom/configuration/providers/forge/FieldMigratedMappingConfiguration.java index 72304ffb..7a06ed06 100644 --- a/src/main/java/net/fabricmc/loom/configuration/providers/forge/FieldMigratedMappingConfiguration.java +++ b/src/main/java/net/fabricmc/loom/configuration/providers/forge/FieldMigratedMappingConfiguration.java @@ -26,8 +26,8 @@ package net.fabricmc.loom.configuration.providers.forge; import java.io.BufferedReader; import java.io.IOException; -import java.io.StringWriter; import java.io.UncheckedIOException; +import java.io.Writer; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardOpenOption; @@ -44,7 +44,6 @@ import com.google.common.collect.HashBasedTable; import com.google.common.collect.Table; import com.google.common.reflect.TypeToken; import com.google.gson.Gson; -import dev.architectury.refmapremapper.utils.DescriptorRemapper; import org.gradle.api.Project; import org.objectweb.asm.ClassReader; import org.objectweb.asm.ClassVisitor; @@ -58,7 +57,6 @@ import net.fabricmc.loom.configuration.providers.minecraft.MinecraftProvider; import net.fabricmc.loom.util.FileSystemUtil; import net.fabricmc.loom.util.ThreadingUtils; import net.fabricmc.loom.util.service.SharedServiceManager; -import net.fabricmc.loom.util.srg.SrgMerger; import net.fabricmc.mappingio.MappingReader; import net.fabricmc.mappingio.format.Tiny2Writer; import net.fabricmc.mappingio.tree.MappingTree; @@ -110,14 +108,6 @@ public final class FieldMigratedMappingConfiguration extends MappingConfiguratio this.rawTinyMappings = tinyMappings; this.rawTinyMappingsWithSrg = tinyMappingsWithSrg; - if (extension.shouldGenerateSrgTiny()) { - if (Files.notExists(rawTinyMappingsWithSrg) || extension.refreshDeps()) { - // Merge tiny mappings with srg - SrgMerger.ExtraMappings extraMappings = SrgMerger.ExtraMappings.ofMojmapTsrg(getMojmapSrgFileIfPossible(project)); - SrgMerger.mergeSrg(getRawSrgFile(project), rawTinyMappings, rawTinyMappingsWithSrg, extraMappings, true); - } - } - tinyMappings = mappingsWorkingDir().resolve("mappings-field-migrated.tiny"); tinyMappingsWithSrg = mappingsWorkingDir().resolve("mappings-srg-field-migrated.tiny"); @@ -132,50 +122,58 @@ public final class FieldMigratedMappingConfiguration extends MappingConfiguratio public void updateFieldMigration(Project project) throws IOException { if (!Files.exists(migratedFieldsCache)) { - generateNewFieldMigration(project); + migratedFields.clear(); + migratedFields.addAll(generateNewFieldMigration(project, MinecraftPatchedProvider.get(project).getMinecraftPatchedSrgJar(), MappingsNamespace.SRG.toString(), rawTinyMappingsWithSrg).entrySet()); Map map = new HashMap<>(); migratedFields.forEach(entry -> { map.put(entry.getKey().owner + "#" + entry.getKey().field, entry.getValue()); }); Files.writeString(migratedFieldsCache, new Gson().toJson(map)); Files.deleteIfExists(tinyMappings); + Files.deleteIfExists(tinyMappingsWithSrg); } - if (!Files.exists(tinyMappings)) { + if (Files.notExists(tinyMappings) || Files.notExists(tinyMappingsWithSrg)) { Table fieldDescriptorMap = HashBasedTable.create(); for (Map.Entry entry : migratedFields) { fieldDescriptorMap.put(entry.getKey().owner, entry.getKey().field, entry.getValue()); } - MemoryMappingTree mappings = new MemoryMappingTree(); - - try (BufferedReader reader = Files.newBufferedReader(rawTinyMappings)) { - MappingReader.read(reader, mappings); - - for (MappingTree.ClassMapping classDef : new ArrayList<>(mappings.getClasses())) { - Map row = fieldDescriptorMap.row(classDef.getName(MappingsNamespace.INTERMEDIARY.toString())); - - if (!row.isEmpty()) { - for (MappingTree.FieldMapping fieldDef : new ArrayList<>(classDef.getFields())) { - String newDescriptor = row.get(fieldDef.getName(MappingsNamespace.INTERMEDIARY.toString())); - - if (newDescriptor != null) { - fieldDef.setSrcDesc(mappings.mapDesc(newDescriptor, mappings.getNamespaceId(MappingsNamespace.INTERMEDIARY.toString()), MappingTreeView.SRC_NAMESPACE_ID)); - } - } - } - } - } - - StringWriter stringWriter = new StringWriter(); - Tiny2Writer tiny2Writer = new Tiny2Writer(stringWriter, false); - mappings.accept(tiny2Writer); - Files.writeString(tinyMappings, stringWriter.toString(), StandardOpenOption.CREATE); + injectMigration(project, fieldDescriptorMap, rawTinyMappings, tinyMappings); + injectMigration(project, fieldDescriptorMap, rawTinyMappingsWithSrg, tinyMappingsWithSrg); } } - private void generateNewFieldMigration(Project project) throws IOException { + private static void injectMigration(Project project, Table fieldDescriptorMap, Path source, Path out) throws IOException { + MemoryMappingTree mappings = new MemoryMappingTree(); + + try (BufferedReader reader = Files.newBufferedReader(source)) { + MappingReader.read(reader, mappings); + } + + for (MappingTree.ClassMapping classDef : new ArrayList<>(mappings.getClasses())) { + Map row = fieldDescriptorMap.row(classDef.getName(MappingsNamespace.INTERMEDIARY.toString())); + + if (!row.isEmpty()) { + for (MappingTree.FieldMapping fieldDef : new ArrayList<>(classDef.getFields())) { + String newDescriptor = row.get(fieldDef.getName(MappingsNamespace.INTERMEDIARY.toString())); + + if (newDescriptor != null) { + String prev = fieldDef.getDesc(MappingsNamespace.INTERMEDIARY.toString()); + fieldDef.setSrcDesc(mappings.mapDesc(newDescriptor, mappings.getNamespaceId(MappingsNamespace.INTERMEDIARY.toString()), MappingTreeView.SRC_NAMESPACE_ID)); + project.getLogger().info("Migrated field descriptor of field {}#{} from {} to {}", classDef.getName(MappingsNamespace.INTERMEDIARY.toString()), fieldDef.getName(MappingsNamespace.INTERMEDIARY.toString()), prev, newDescriptor); + } + } + } + } + + try (Writer writer = Files.newBufferedWriter(out, StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING)) { + mappings.accept(new Tiny2Writer(writer, false)); + } + } + + private static Map generateNewFieldMigration(Project project, Path patchedJar, String patchedJarNamespace, Path mappingsPath) throws IOException { Map fieldDescriptorMap = new ConcurrentHashMap<>(); LoomGradleExtension extension = LoomGradleExtension.get(project); ThreadingUtils.TaskCompleter completer = ThreadingUtils.taskCompleter(); @@ -200,8 +198,7 @@ public final class FieldMigratedMappingConfiguration extends MappingConfiguratio } Visitor visitor = new Visitor(Opcodes.ASM9); - Path patchedSrgJar = MinecraftPatchedProvider.get(project).getMinecraftPatchedSrgJar(); - FileSystemUtil.Delegate system = FileSystemUtil.getJarFileSystem(patchedSrgJar, false); + FileSystemUtil.Delegate system = FileSystemUtil.getJarFileSystem(patchedJar, false); completer.onComplete(value -> system.close()); for (Path fsPath : (Iterable) Files.walk(system.get().getPath("/"))::iterator) { @@ -216,40 +213,29 @@ public final class FieldMigratedMappingConfiguration extends MappingConfiguratio completer.complete(); Map migratedFields = new HashMap<>(); - try (BufferedReader reader = Files.newBufferedReader(rawTinyMappingsWithSrg)) { + try (BufferedReader reader = Files.newBufferedReader(mappingsPath)) { MemoryMappingTree mappings = new MemoryMappingTree(); MappingReader.read(reader, mappings); - Map srgToIntermediary = new HashMap<>(); - - for (MappingTree.ClassMapping aClass : mappings.getClasses()) { - srgToIntermediary.put(aClass.getName("srg"), aClass.getName("intermediary")); - } for (MappingTree.ClassMapping classDef : mappings.getClasses()) { - String ownerSrg = classDef.getName("srg"); - String ownerIntermediary = classDef.getName("intermediary"); - for (MappingTree.FieldMapping fieldDef : classDef.getFields()) { - String fieldSrg = fieldDef.getName("srg"); - String descriptorSrg = fieldDef.getDesc("srg"); + String newDescriptor = fieldDescriptorMap.get(new FieldMember(classDef.getName(patchedJarNamespace), fieldDef.getName(patchedJarNamespace))); + String existingDescriptor = fieldDef.getDesc(patchedJarNamespace); - FieldMember member = new FieldMember(ownerSrg, fieldSrg); - String newDescriptor = fieldDescriptorMap.get(member); - - if (newDescriptor != null && !newDescriptor.equals(descriptorSrg)) { - String fieldIntermediary = fieldDef.getName("intermediary"); - String descriptorIntermediary = fieldDef.getDesc("intermediary"); - String newDescriptorRemapped = DescriptorRemapper.remapDescriptor(newDescriptor, - clazz -> srgToIntermediary.getOrDefault(clazz, clazz)); - migratedFields.put(new FieldMember(ownerIntermediary, fieldIntermediary), newDescriptorRemapped); - project.getLogger().info(ownerIntermediary + "#" + fieldIntermediary + ": " + descriptorIntermediary + " -> " + newDescriptorRemapped); + if (newDescriptor != null && !newDescriptor.equals(existingDescriptor)) { + String ownerIntermediary = classDef.getName(MappingsNamespace.INTERMEDIARY.toString()); + String fieldIntermediary = fieldDef.getName(MappingsNamespace.INTERMEDIARY.toString()); + String descriptorIntermediary = fieldDef.getDesc(MappingsNamespace.INTERMEDIARY.toString()); + String newDescriptorIntermediary = mappings.mapDesc(newDescriptor, mappings.getNamespaceId(patchedJarNamespace), + mappings.getNamespaceId(MappingsNamespace.INTERMEDIARY.toString())); + migratedFields.put(new FieldMember(ownerIntermediary, fieldIntermediary), newDescriptorIntermediary); + project.getLogger().info("Found migration of " + ownerIntermediary + "#" + fieldIntermediary + ": " + descriptorIntermediary + " -> " + newDescriptorIntermediary); } } } } - this.migratedFields.clear(); - this.migratedFields.addAll(migratedFields.entrySet()); + return migratedFields; } public static class FieldMember { 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 a2716566..bef4fdbd 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 @@ -195,7 +195,6 @@ public class MappingConfiguration { public void setupPost(Project project) throws IOException { LoomGradleExtension extension = LoomGradleExtension.get(project); - manipulateMappings(project, tinyMappingsJar); if (extension.shouldGenerateSrgTiny()) { if (Files.notExists(tinyMappingsWithSrg) || extension.refreshDeps()) { @@ -206,6 +205,8 @@ public class MappingConfiguration { project.getLogger().info(":merged srg mappings in " + stopwatch.stop()); } } + + manipulateMappings(project, tinyMappingsJar); } public void applyToProject(Project project, DependencyInfo dependency) throws IOException {