From e34325f7bcbe70f1a3047b39e2c19ebbd97cd082 Mon Sep 17 00:00:00 2001 From: modmuss Date: Wed, 16 Apr 2025 22:08:49 +0100 Subject: [PATCH] Add option to drop non root methods when using Mojang mappings. (#1258) * Attempt to fix #1209 * More layered mappings tests * Fix build * Make opt in * Rename --- .../api/mappings/layered/MappingContext.java | 6 +- .../mappings/GradleMappingContext.java | 13 ++- .../mappings/mojmap/MojangMappingLayer.java | 42 ++++++++-- .../mappings/mojmap/MojangMappingsSpec.java | 5 +- .../net/fabricmc/loom/util/Constants.java | 4 + ...Test.groovy => LayeredMappingsTest.groovy} | 83 ++++++++++++++++++- .../test/integration/ParchmentTest.groovy | 50 ----------- .../LayeredMappingsSpecification.groovy | 33 +++++++- .../MojangMappingLayerTest.groovy | 44 +++++++++- 9 files changed, 214 insertions(+), 66 deletions(-) rename src/test/groovy/net/fabricmc/loom/test/integration/{MojangMappingsProjectTest.groovy => LayeredMappingsTest.groovy} (63%) delete mode 100644 src/test/groovy/net/fabricmc/loom/test/integration/ParchmentTest.groovy diff --git a/src/main/java/net/fabricmc/loom/api/mappings/layered/MappingContext.java b/src/main/java/net/fabricmc/loom/api/mappings/layered/MappingContext.java index ebb1c394..5dd9a47c 100644 --- a/src/main/java/net/fabricmc/loom/api/mappings/layered/MappingContext.java +++ b/src/main/java/net/fabricmc/loom/api/mappings/layered/MappingContext.java @@ -1,7 +1,7 @@ /* * This file is part of fabric-loom, licensed under the MIT License (MIT). * - * Copyright (c) 2016-2021 FabricMC + * Copyright (c) 2016-2025 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 @@ -46,6 +46,8 @@ public interface MappingContext { Supplier intermediaryTree(); + boolean isUsingIntermediateMappings(); + MinecraftProvider minecraftProvider(); default String minecraftVersion() { @@ -62,4 +64,6 @@ public interface MappingContext { DownloadBuilder download(String url); boolean refreshDeps(); + + boolean hasProperty(String property); } diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/GradleMappingContext.java b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/GradleMappingContext.java index 3db4043c..2e5595a2 100644 --- a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/GradleMappingContext.java +++ b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/GradleMappingContext.java @@ -1,7 +1,7 @@ /* * This file is part of fabric-loom, licensed under the MIT License (MIT). * - * Copyright (c) 2018-2021 FabricMC + * Copyright (c) 2018-2025 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 @@ -41,6 +41,7 @@ import net.fabricmc.loom.LoomGradleExtension; import net.fabricmc.loom.api.mappings.layered.MappingContext; import net.fabricmc.loom.configuration.providers.minecraft.MinecraftProvider; import net.fabricmc.loom.util.download.DownloadBuilder; +import net.fabricmc.loom.util.gradle.GradleUtils; import net.fabricmc.loom.util.service.ScopedServiceFactory; import net.fabricmc.mappingio.tree.MemoryMappingTree; @@ -85,6 +86,11 @@ public class GradleMappingContext implements MappingContext { }; } + @Override + public boolean isUsingIntermediateMappings() { + return !(extension.getIntermediateMappingsProvider() instanceof NoOpIntermediateMappingsProvider); + } + @Override public MinecraftProvider minecraftProvider() { return extension.getMinecraftProvider(); @@ -110,6 +116,11 @@ public class GradleMappingContext implements MappingContext { return extension.refreshDeps(); } + @Override + public boolean hasProperty(String property) { + return GradleUtils.getBooleanProperty(project, property); + } + public Project getProject() { return project; } diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/mojmap/MojangMappingLayer.java b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/mojmap/MojangMappingLayer.java index cd37bc01..ffe586c8 100644 --- a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/mojmap/MojangMappingLayer.java +++ b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/mojmap/MojangMappingLayer.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-2025 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 @@ -30,9 +30,11 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.util.List; +import java.util.function.Supplier; import java.util.regex.Pattern; import org.gradle.api.logging.Logger; +import org.jetbrains.annotations.Nullable; import net.fabricmc.loom.api.mappings.layered.MappingLayer; import net.fabricmc.loom.api.mappings.layered.MappingsNamespace; @@ -41,21 +43,51 @@ import net.fabricmc.loom.configuration.providers.mappings.utils.DstNameFilterMap import net.fabricmc.mappingio.MappingVisitor; import net.fabricmc.mappingio.adapter.MappingSourceNsSwitch; import net.fabricmc.mappingio.format.proguard.ProGuardFileReader; +import net.fabricmc.mappingio.tree.MemoryMappingTree; -public record MojangMappingLayer(Path clientMappings, Path serverMappings, boolean nameSyntheticMembers, - Logger logger) implements MappingLayer { +public record MojangMappingLayer(Path clientMappings, Path serverMappings, boolean nameSyntheticMembers, boolean dropNoneIntermediaryRoots, + @Nullable Supplier intermediarySupplier, Logger logger) implements MappingLayer { private static final Pattern SYNTHETIC_NAME_PATTERN = Pattern.compile("^(access|this|val\\$this|lambda\\$.*)\\$[0-9]+$"); @Override public void visit(MappingVisitor mappingVisitor) throws IOException { printMappingsLicense(clientMappings); + if (!dropNoneIntermediaryRoots) { + logger().debug("Not attempting to drop none intermediary roots"); + + readMappings(mappingVisitor); + return; + } + + logger().info("Attempting to drop none intermediary roots"); + + if (intermediarySupplier == null) { + // Using no-op intermediary mappings + readMappings(mappingVisitor); + return; + } + + // Create a mapping tree with src: official dst: named, intermediary + MemoryMappingTree mappingTree = new MemoryMappingTree(); + intermediarySupplier.get().accept(mappingTree); + readMappings(mappingTree); + + // The following code first switches the src namespace to intermediary dropping any entries that don't have an intermediary name + // This removes any none root methods before switching it back to official + var officialSwitch = new MappingSourceNsSwitch(mappingVisitor, getSourceNamespace().toString(), false); + var intermediarySwitch = new MappingSourceNsSwitch(officialSwitch, MappingsNamespace.INTERMEDIARY.toString(), true); + mappingTree.accept(intermediarySwitch); + } + + private void readMappings(MappingVisitor mappingVisitor) throws IOException { // Filter out field names matching the pattern - DstNameFilterMappingVisitor nameFilter = new DstNameFilterMappingVisitor(mappingVisitor, SYNTHETIC_NAME_PATTERN); + var nameFilter = new DstNameFilterMappingVisitor(mappingVisitor, SYNTHETIC_NAME_PATTERN); // Make official the source namespace - MappingSourceNsSwitch nsSwitch = new MappingSourceNsSwitch(nameSyntheticMembers() ? mappingVisitor : nameFilter, MappingsNamespace.OFFICIAL.toString()); + var nsSwitch = new MappingSourceNsSwitch(nameSyntheticMembers() ? mappingVisitor : nameFilter, MappingsNamespace.OFFICIAL.toString()); + // Read both server and client mappings try (BufferedReader clientBufferedReader = Files.newBufferedReader(clientMappings, StandardCharsets.UTF_8); BufferedReader serverBufferedReader = Files.newBufferedReader(serverMappings, StandardCharsets.UTF_8)) { ProGuardFileReader.read(clientBufferedReader, MappingsNamespace.NAMED.toString(), MappingsNamespace.OFFICIAL.toString(), nsSwitch); diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/mojmap/MojangMappingsSpec.java b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/mojmap/MojangMappingsSpec.java index 5fbeaeeb..0162c5ce 100644 --- a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/mojmap/MojangMappingsSpec.java +++ b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/mojmap/MojangMappingsSpec.java @@ -1,7 +1,7 @@ /* * This file is part of fabric-loom, licensed under the MIT License (MIT). * - * Copyright (c) 2016-2021 FabricMC + * Copyright (c) 2016-2025 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 @@ -30,6 +30,7 @@ import java.nio.file.Path; import net.fabricmc.loom.api.mappings.layered.MappingContext; import net.fabricmc.loom.api.mappings.layered.spec.MappingsSpec; import net.fabricmc.loom.configuration.providers.minecraft.MinecraftVersionMeta; +import net.fabricmc.loom.util.Constants; import net.fabricmc.loom.util.download.DownloadException; public record MojangMappingsSpec(boolean nameSyntheticMembers) implements MappingsSpec { @@ -66,6 +67,8 @@ public record MojangMappingsSpec(boolean nameSyntheticMembers) implements Mappin clientMappings, serverMappings, nameSyntheticMembers(), + context.hasProperty(Constants.Properties.DROP_NON_INTERMEDIATE_ROOT_METHODS), + context.isUsingIntermediateMappings() ? context.intermediaryTree() : null, context.getLogger() ); } diff --git a/src/main/java/net/fabricmc/loom/util/Constants.java b/src/main/java/net/fabricmc/loom/util/Constants.java index 4ca0a1f6..38055f7f 100644 --- a/src/main/java/net/fabricmc/loom/util/Constants.java +++ b/src/main/java/net/fabricmc/loom/util/Constants.java @@ -153,6 +153,10 @@ public class Constants { * Skip the signature verification of the Minecraft jar after downloading it. */ public static final String DISABLE_MINECRAFT_VERIFICATION = "fabric.loom.disableMinecraftVerification"; + /** + * When using the MojangMappingLayer this will remove names for non root methods by using the intermediary mappings. + */ + public static final String DROP_NON_INTERMEDIATE_ROOT_METHODS = "fabric.loom.dropNonIntermediateRootMethods"; } public static final class Manifest { diff --git a/src/test/groovy/net/fabricmc/loom/test/integration/MojangMappingsProjectTest.groovy b/src/test/groovy/net/fabricmc/loom/test/integration/LayeredMappingsTest.groovy similarity index 63% rename from src/test/groovy/net/fabricmc/loom/test/integration/MojangMappingsProjectTest.groovy rename to src/test/groovy/net/fabricmc/loom/test/integration/LayeredMappingsTest.groovy index 4669f362..c98a9f24 100644 --- a/src/test/groovy/net/fabricmc/loom/test/integration/MojangMappingsProjectTest.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/integration/LayeredMappingsTest.groovy @@ -1,7 +1,7 @@ /* * This file is part of fabric-loom, licensed under the MIT License (MIT). * - * Copyright (c) 2018-2022 FabricMC + * Copyright (c) 2018-2025 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 @@ -28,11 +28,73 @@ import spock.lang.Specification import spock.lang.Unroll import net.fabricmc.loom.test.util.GradleProjectTestTrait +import net.fabricmc.loom.util.Constants -import static net.fabricmc.loom.test.LoomTestConstants.* +import static net.fabricmc.loom.test.LoomTestConstants.DEFAULT_GRADLE +import static net.fabricmc.loom.test.LoomTestConstants.STANDARD_TEST_VERSIONS import static org.gradle.testkit.runner.TaskOutcome.SUCCESS -class MojangMappingsProjectTest extends Specification implements GradleProjectTestTrait { +class LayeredMappingsTest extends Specification implements GradleProjectTestTrait { + def "build #layer"() { + setup: + def gradle = gradleProject(project: "minimalBase", version: DEFAULT_GRADLE) + gradle.buildGradle << """ + repositories { + maven { + name = 'ParchmentMC' + url = 'https://maven.parchmentmc.org' + } + } + dependencies { + minecraft "com.mojang:minecraft:1.21.4" + mappings loom.layered { + $layer + } + } + """ + + if (layer.contains("// Drop none roots")) { + new File(gradle.projectDir, "gradle.properties").text = "${Constants.Properties.DROP_NON_INTERMEDIATE_ROOT_METHODS}=true" + } + + when: + def result = gradle.run(task: "build") + + then: + result.task(":build").outcome == SUCCESS + + where: + layer << [ + // Only mojang mappings + """ + officialMojangMappings() + """, + // Yarn on top of Mojmap + """ + // Drop none roots + officialMojangMappings() + mappings("net.fabricmc:yarn:1.21.4+build.8:v2") + """, + // Mojmap on top of yarn + """ + mappings("net.fabricmc:yarn:1.21.4+build.8:v2") + officialMojangMappings() + """, + // Mojmap with parchment + """ + officialMojangMappings() + parchment("org.parchmentmc.data:parchment-1.21.4:2025.01.19@zip") + """, + // Yarn on top of Mojmap with parchment + """ + // Drop none roots + officialMojangMappings() + parchment("org.parchmentmc.data:parchment-1.21.4:2025.01.19@zip") + mappings("net.fabricmc:yarn:1.21.4+build.8:v2") + """, + ] + } + @Unroll def "build (gradle #version)"() { setup: @@ -146,4 +208,19 @@ class MojangMappingsProjectTest extends Specification implements GradleProjectTe where: version << STANDARD_TEST_VERSIONS } + + @Unroll + def "parchment #version"() { + setup: + def gradle = gradleProject(project: "parchment", version: version) + + when: + def result = gradle.run(task: "build") + + then: + result.task(":build").outcome == SUCCESS + + where: + version << STANDARD_TEST_VERSIONS + } } diff --git a/src/test/groovy/net/fabricmc/loom/test/integration/ParchmentTest.groovy b/src/test/groovy/net/fabricmc/loom/test/integration/ParchmentTest.groovy deleted file mode 100644 index 95714280..00000000 --- a/src/test/groovy/net/fabricmc/loom/test/integration/ParchmentTest.groovy +++ /dev/null @@ -1,50 +0,0 @@ -/* - * This file is part of fabric-loom, licensed under the MIT License (MIT). - * - * Copyright (c) 2018-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 spock.lang.Specification -import spock.lang.Unroll - -import net.fabricmc.loom.test.util.GradleProjectTestTrait - -import static net.fabricmc.loom.test.LoomTestConstants.STANDARD_TEST_VERSIONS -import static org.gradle.testkit.runner.TaskOutcome.SUCCESS - -class ParchmentTest extends Specification implements GradleProjectTestTrait { - @Unroll - def "parchment #version"() { - setup: - def gradle = gradleProject(project: "parchment", version: version) - - when: - def result = gradle.run(task: "build") - - then: - result.task(":build").outcome == SUCCESS - - where: - version << STANDARD_TEST_VERSIONS - } -} diff --git a/src/test/groovy/net/fabricmc/loom/test/unit/layeredmappings/LayeredMappingsSpecification.groovy b/src/test/groovy/net/fabricmc/loom/test/unit/layeredmappings/LayeredMappingsSpecification.groovy index a8985796..f7d4cb71 100644 --- a/src/test/groovy/net/fabricmc/loom/test/unit/layeredmappings/LayeredMappingsSpecification.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/unit/layeredmappings/LayeredMappingsSpecification.groovy @@ -48,6 +48,7 @@ import net.fabricmc.loom.configuration.providers.mappings.utils.AddConstructorMa import net.fabricmc.loom.configuration.providers.minecraft.MinecraftProvider import net.fabricmc.loom.test.LoomTestConstants import net.fabricmc.loom.test.unit.LoomMocks +import net.fabricmc.loom.util.Constants import net.fabricmc.loom.util.download.Download import net.fabricmc.loom.util.download.DownloadBuilder import net.fabricmc.mappingio.MappingReader @@ -60,7 +61,6 @@ abstract class LayeredMappingsSpecification extends Specification implements Lay Logger mockLogger = Mock(Logger) MinecraftProvider mockMinecraftProvider = Mock(MinecraftProvider) String intermediaryUrl - MappingContext mappingContext = new TestMappingContext() File tempDir = new File(LoomTestConstants.TEST_DIR, "layered/${getClass().name}") @@ -92,18 +92,25 @@ abstract class LayeredMappingsSpecification extends Specification implements Lay MemoryMappingTree getSingleMapping(MappingsSpec spec) { MemoryMappingTree mappingTree = new MemoryMappingTree() - spec.createLayer(mappingContext).visit(mappingTree) + spec.createLayer(new TestMappingContext([spec])).visit(mappingTree) return mappingTree } MemoryMappingTree getLayeredMappings(MappingsSpec... specs) { LayeredMappingsProcessor processor = createLayeredMappingsProcessor(specs) - return processor.getMappings(processor.resolveLayers(mappingContext)) + return processor.getMappings(processor.resolveLayers(new TestMappingContext(specs.toList()))) + } + + MemoryMappingTree getLayeredMappingsDropNoneIntermediaryRoots(MappingsSpec... specs) { + LayeredMappingsProcessor processor = createLayeredMappingsProcessor(specs) + return processor.getMappings(processor.resolveLayers(new TestMappingContext(specs.toList(), [ + Constants.Properties.DROP_NON_INTERMEDIATE_ROOT_METHODS + ]))) } UnpickLayer.UnpickData getUnpickData(MappingsSpec... specs) { LayeredMappingsProcessor processor = createLayeredMappingsProcessor(specs) - return processor.getUnpickData(processor.resolveLayers(mappingContext)) + return processor.getUnpickData(processor.resolveLayers(new TestMappingContext(specs.toList()))) } private static LayeredMappingsProcessor createLayeredMappingsProcessor(MappingsSpec... specs) { @@ -134,6 +141,14 @@ abstract class LayeredMappingsSpecification extends Specification implements Lay } class TestMappingContext implements MappingContext { + private final List> specs + private final List enabledProperties + + TestMappingContext(List> specs, List enabledProperties = []) { + this.specs = specs + this.enabledProperties = enabledProperties + } + @Override Path resolveDependency(Dependency dependency) { throw new UnsupportedOperationException("TODO") @@ -165,6 +180,11 @@ abstract class LayeredMappingsSpecification extends Specification implements Lay } } + @Override + boolean isUsingIntermediateMappings() { + return !specs.any { it instanceof NoIntermediateMappingsSpec } + } + @Override MinecraftProvider minecraftProvider() { return mockMinecraftProvider @@ -189,6 +209,11 @@ abstract class LayeredMappingsSpecification extends Specification implements Lay boolean refreshDeps() { return false } + + @Override + boolean hasProperty(String property) { + return enabledProperties.contains(property) + } } @EqualsAndHashCode diff --git a/src/test/groovy/net/fabricmc/loom/test/unit/layeredmappings/MojangMappingLayerTest.groovy b/src/test/groovy/net/fabricmc/loom/test/unit/layeredmappings/MojangMappingLayerTest.groovy index 4d860ff0..60fb338e 100644 --- a/src/test/groovy/net/fabricmc/loom/test/unit/layeredmappings/MojangMappingLayerTest.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/unit/layeredmappings/MojangMappingLayerTest.groovy @@ -1,7 +1,7 @@ /* * This file is part of fabric-loom, licensed under the MIT License (MIT). * - * Copyright (c) 2016-2021 FabricMC + * Copyright (c) 2016-2025 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 @@ -70,6 +70,48 @@ class MojangMappingLayerTest extends LayeredMappingsSpecification { !tiny.contains('this$0') } + def "Read mojang mappings with synthetic field names drop roots" () { + setup: + intermediaryUrl = INTERMEDIARY_1_17_URL + mockMinecraftProvider.getVersionInfo() >> VERSION_META_1_17 + mockMinecraftProvider.minecraftVersion() >> "1.17" + when: + def mappings = getLayeredMappingsDropNoneIntermediaryRoots( + new IntermediaryMappingsSpec(), + buildMojangMappingsSpec(true) + ) + def tiny = getTiny(mappings) + then: + mappings.srcNamespace == "named" + mappings.dstNamespaces == ["intermediary", "official"] + mappings.classes.size() == 6107 + mappings.classes[0].srcName.hashCode() == 1869546970 // MojMap name, just check the hash + mappings.classes[0].getDstName(0) == "net/minecraft/class_2354" + mappings.classes[0].methods[0].args.size() == 0 // No Args + tiny.contains('this$0') + } + + def "Read mojang mappings without synthetic field names drop roots" () { + setup: + intermediaryUrl = INTERMEDIARY_1_17_URL + mockMinecraftProvider.getVersionInfo() >> VERSION_META_1_17 + mockMinecraftProvider.minecraftVersion() >> "1.17" + when: + def mappings = getLayeredMappingsDropNoneIntermediaryRoots( + new IntermediaryMappingsSpec(), + buildMojangMappingsSpec(false) + ) + def tiny = getTiny(mappings) + then: + mappings.srcNamespace == "named" + mappings.dstNamespaces == ["intermediary", "official"] + mappings.classes.size() == 6107 + mappings.classes[0].srcName.hashCode() == 1869546970 // MojMap name, just check the hash + mappings.classes[0].getDstName(0) == "net/minecraft/class_2354" + mappings.classes[0].methods[0].args.size() == 0 // No Args + !tiny.contains('this$0') + } + def "Read mojang mappings with no intermediary" () { setup: intermediaryUrl = INTERMEDIARY_1_17_URL