From 2752b61cb78a68933fc46714de0077036c3565cd Mon Sep 17 00:00:00 2001 From: modmuss50 Date: Wed, 29 Dec 2021 19:27:24 +0000 Subject: [PATCH] Cleanup signature fixer code + add basic integration test. Fixes #546 --- .../minecraft/MinecraftMappedProvider.java | 61 +------------ .../minecraft/SignatureFixerApplyVisitor.java | 85 +++++++++++++++++++ .../integration/SignatureFixesTest.groovy | 57 +++++++++++++ 3 files changed, 145 insertions(+), 58 deletions(-) create mode 100644 src/main/java/net/fabricmc/loom/configuration/providers/minecraft/SignatureFixerApplyVisitor.java create mode 100644 src/test/groovy/net/fabricmc/loom/test/integration/SignatureFixesTest.groovy diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/MinecraftMappedProvider.java b/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/MinecraftMappedProvider.java index 0025e91..d1ecf29 100644 --- a/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/MinecraftMappedProvider.java +++ b/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/MinecraftMappedProvider.java @@ -29,15 +29,10 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; import java.util.Arrays; -import java.util.HashMap; import java.util.Map; -import java.util.Objects; -import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; import org.gradle.api.Project; -import org.objectweb.asm.ClassVisitor; -import org.objectweb.asm.commons.Remapper; import net.fabricmc.loom.api.mappings.layered.MappingsNamespace; import net.fabricmc.loom.configuration.DependencyProvider; @@ -47,7 +42,6 @@ import net.fabricmc.loom.util.Constants; import net.fabricmc.loom.util.TinyRemapperHelper; import net.fabricmc.tinyremapper.OutputConsumerPath; import net.fabricmc.tinyremapper.TinyRemapper; -import net.fabricmc.tinyremapper.api.TrClass; public class MinecraftMappedProvider extends DependencyProvider { private File minecraftMappedJar; @@ -110,66 +104,17 @@ public class MinecraftMappedProvider extends DependencyProvider { for (String toM : Arrays.asList(MappingsNamespace.NAMED.toString(), MappingsNamespace.INTERMEDIARY.toString())) { final boolean toNamed = MappingsNamespace.NAMED.toString().equals(toM); final boolean toIntermediary = MappingsNamespace.INTERMEDIARY.toString().equals(toM); - final boolean fixSignatures = mappingsProvider.getSignatureFixes() != null; final Path output = toNamed ? outputMapped : outputIntermediary; - getProject().getLogger().lifecycle(":remapping minecraft (TinyRemapper, " + fromM + " -> " + toM + ")"); + getProject().getLogger().info(":remapping minecraft (TinyRemapper, " + fromM + " -> " + toM + ")"); Files.deleteIfExists(output); - // Bit ugly but whatever, the whole issue is a bit ugly :) - AtomicReference> remappedSignatures = new AtomicReference<>(); + final Map remappedSignatures = SignatureFixerApplyVisitor.getRemappedSignatures(toIntermediary, mappingsProvider, getProject(), toM); TinyRemapper remapper = TinyRemapperHelper.getTinyRemapper(getProject(), fromM, toM, true, (builder) -> { - if (!fixSignatures) { - return; - } - - builder.extraPostApplyVisitor(new TinyRemapper.ApplyVisitorProvider() { - @Override - public ClassVisitor insertApplyVisitor(TrClass cls, ClassVisitor next) { - return new ClassVisitor(Constants.ASM_VERSION, next) { - @Override - public void visit(int version, int access, String name, String signature, String superName, String[] interfaces) { - Map signatureFixes = Objects.requireNonNull(remappedSignatures.get(), "Could not get remapped signatures"); - - if (signature == null) { - signature = signatureFixes.getOrDefault(name, null); - - if (signature != null) { - getProject().getLogger().info("Replaced signature for {} with {}", name, signature); - } - } - - super.visit(version, access, name, signature, superName, interfaces); - } - }; - } - }); + builder.extraPostApplyVisitor(new SignatureFixerApplyVisitor(remappedSignatures)); }); - if (fixSignatures) { - if (toIntermediary) { - // No need to remap, as these are already intermediary - remappedSignatures.set(mappingsProvider.getSignatureFixes()); - } else { - // Remap the sig fixes from intermediary to the target namespace - final Map remapped = new HashMap<>(); - final TinyRemapper sigTinyRemapper = TinyRemapperHelper.getTinyRemapper(getProject(), MappingsNamespace.INTERMEDIARY.toString(), toM); - final Remapper sigAsmRemapper = sigTinyRemapper.getEnvironment().getRemapper(); - - // Remap the class names and the signatures using a new tiny remapper instance. - for (Map.Entry entry : mappingsProvider.getSignatureFixes().entrySet()) { - remapped.put( - sigAsmRemapper.map(entry.getKey()), - sigAsmRemapper.mapSignature(entry.getValue(), false) - ); - } - - sigTinyRemapper.finish(); - remappedSignatures.set(remapped); - } - } - try (OutputConsumerPath outputConsumer = new OutputConsumerPath.Builder(output).build()) { outputConsumer.addNonClassFiles(input); remapper.readClassPath(TinyRemapperHelper.getMinecraftDependencies(getProject())); diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/SignatureFixerApplyVisitor.java b/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/SignatureFixerApplyVisitor.java new file mode 100644 index 0000000..0ebff34 --- /dev/null +++ b/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/SignatureFixerApplyVisitor.java @@ -0,0 +1,85 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2021 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.configuration.providers.minecraft; + +import java.io.IOException; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; + +import org.gradle.api.Project; +import org.objectweb.asm.ClassVisitor; +import org.objectweb.asm.commons.Remapper; + +import net.fabricmc.loom.api.mappings.layered.MappingsNamespace; +import net.fabricmc.loom.configuration.providers.mappings.MappingsProviderImpl; +import net.fabricmc.loom.util.Constants; +import net.fabricmc.loom.util.TinyRemapperHelper; +import net.fabricmc.tinyremapper.TinyRemapper; +import net.fabricmc.tinyremapper.api.TrClass; + +public record SignatureFixerApplyVisitor(Map signatureFixes) implements TinyRemapper.ApplyVisitorProvider { + @Override + public ClassVisitor insertApplyVisitor(TrClass cls, ClassVisitor next) { + return new ClassVisitor(Constants.ASM_VERSION, next) { + @Override + public void visit(int version, int access, String name, String signature, String superName, String[] interfaces) { + if (signature == null) { + signature = signatureFixes.getOrDefault(name, null); + } + + super.visit(version, access, name, signature, superName, interfaces); + } + }; + } + + public static Map getRemappedSignatures(boolean toIntermediary, MappingsProviderImpl mappingsProvider, Project project, String targetNamespace) throws IOException { + if (mappingsProvider.getSignatureFixes() == null) { + // No fixes + return Collections.emptyMap(); + } + + if (toIntermediary) { + // No need to remap, as these are already intermediary + return mappingsProvider.getSignatureFixes(); + } + + // Remap the sig fixes from intermediary to the target namespace + final Map remapped = new HashMap<>(); + final TinyRemapper sigTinyRemapper = TinyRemapperHelper.getTinyRemapper(project, MappingsNamespace.INTERMEDIARY.toString(), targetNamespace); + final Remapper sigAsmRemapper = sigTinyRemapper.getEnvironment().getRemapper(); + + // Remap the class names and the signatures using a new tiny remapper instance. + for (Map.Entry entry : mappingsProvider.getSignatureFixes().entrySet()) { + remapped.put( + sigAsmRemapper.map(entry.getKey()), + sigAsmRemapper.mapSignature(entry.getValue(), false) + ); + } + + sigTinyRemapper.finish(); + return remapped; + } +} diff --git a/src/test/groovy/net/fabricmc/loom/test/integration/SignatureFixesTest.groovy b/src/test/groovy/net/fabricmc/loom/test/integration/SignatureFixesTest.groovy new file mode 100644 index 0000000..e91b7c1 --- /dev/null +++ b/src/test/groovy/net/fabricmc/loom/test/integration/SignatureFixesTest.groovy @@ -0,0 +1,57 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2021 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.integration + +import net.fabricmc.loom.test.util.GradleProjectTestTrait +import spock.lang.Specification +import spock.lang.Unroll + +import static net.fabricmc.loom.test.LoomTestConstants.STANDARD_TEST_VERSIONS +import static org.gradle.testkit.runner.TaskOutcome.SUCCESS + +// Basic test to just ensure it builds. +class SignatureFixesTest extends Specification implements GradleProjectTestTrait { + @Unroll + def "build (gradle #version)"() { + setup: + def gradle = gradleProject(project: "minimalBase", version: version) + + gradle.buildGradle << ''' + dependencies { + minecraft "com.mojang:minecraft:1.18-pre1" + mappings "net.fabricmc:yarn:1.18-pre1+build.14:v2" + } + ''' + + when: + def result = gradle.run(task: "build") + + then: + result.task(":build").outcome == SUCCESS + + where: + version << STANDARD_TEST_VERSIONS + } +}