From babbc555869c0a9cba90cbcac7dcba241ee62099 Mon Sep 17 00:00:00 2001 From: modmuss50 Date: Sat, 20 Nov 2021 21:46:33 +0000 Subject: Add an option (enabled by default) to map synthetic field and method names from the official mojang mappings. (#538) --- .../integration/MojangMappingsProjectTest.groovy | 26 +++++++++++++++++ .../LayeredMappingSpecBuilderTest.groovy | 8 +++--- .../layeredmappings/MojangMappingLayerTest.groovy | 33 ++++++++++++++++++++-- .../ParchmentMappingLayerTest.groovy | 4 +-- 4 files changed, 62 insertions(+), 9 deletions(-) (limited to 'src/test/groovy') diff --git a/src/test/groovy/net/fabricmc/loom/test/integration/MojangMappingsProjectTest.groovy b/src/test/groovy/net/fabricmc/loom/test/integration/MojangMappingsProjectTest.groovy index f1a46a82..6afba527 100644 --- a/src/test/groovy/net/fabricmc/loom/test/integration/MojangMappingsProjectTest.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/integration/MojangMappingsProjectTest.groovy @@ -48,4 +48,30 @@ class MojangMappingsProjectTest extends Specification implements GradleProjectTe where: version << STANDARD_TEST_VERSIONS } + + @Unroll + def "mojang mappings without synthetic field names (gradle #version)"() { + setup: + def gradle = gradleProject(project: "minimalBase", version: version) + + gradle.buildGradle << ''' + dependencies { + minecraft "com.mojang:minecraft:1.18-pre5" + mappings loom.layered { + officialMojangMappings { + nameSyntheticMembers = false + } + } + } + ''' + + 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/LayeredMappingSpecBuilderTest.groovy b/src/test/groovy/net/fabricmc/loom/test/unit/layeredmappings/LayeredMappingSpecBuilderTest.groovy index 743ddc5a..57850515 100644 --- a/src/test/groovy/net/fabricmc/loom/test/unit/layeredmappings/LayeredMappingSpecBuilderTest.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/unit/layeredmappings/LayeredMappingSpecBuilderTest.groovy @@ -42,7 +42,7 @@ class LayeredMappingSpecBuilderTest extends Specification { } def layers = spec.layers() then: - spec.version == "layered+hash.961" + spec.version == "layered+hash.2192" layers.size() == 2 layers[0].class == IntermediaryMappingsSpec layers[1].class == MojangMappingsSpec @@ -57,7 +57,7 @@ class LayeredMappingSpecBuilderTest extends Specification { def layers = spec.layers() def parchment = layers[2] as ParchmentMappingsSpec then: - spec.version == "layered+hash.863714404" + spec.version == "layered+hash.863752565" layers.size() == 3 layers[0].class == IntermediaryMappingsSpec layers[1].class == MojangMappingsSpec @@ -77,7 +77,7 @@ class LayeredMappingSpecBuilderTest extends Specification { def layers = spec.layers() def parchment = layers[2] as ParchmentMappingsSpec then: - spec.version == "layered+hash.863714410" + spec.version == "layered+hash.863752571" layers.size() == 3 layers[0].class == IntermediaryMappingsSpec layers[1].class == MojangMappingsSpec @@ -97,7 +97,7 @@ class LayeredMappingSpecBuilderTest extends Specification { def layers = spec.layers() def parchment = layers[2] as ParchmentMappingsSpec then: - spec.version == "layered+hash.1144465487" + spec.version == "layered+hash.1144427326" layers.size() == 3 layers[0].class == IntermediaryMappingsSpec layers[1].class == MojangMappingsSpec 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 87eca55c..aad3661a 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 @@ -25,17 +25,17 @@ package net.fabricmc.loom.test.unit.layeredmappings import net.fabricmc.loom.configuration.providers.mappings.intermediary.IntermediaryMappingsSpec -import net.fabricmc.loom.configuration.providers.mappings.mojmap.MojangMappingsSpec +import net.fabricmc.loom.configuration.providers.mappings.mojmap.MojangMappingsSpecBuilderImpl class MojangMappingLayerTest extends LayeredMappingsSpecification { - def "Read mojang mappings" () { + def "Read mojang mappings with synthetic field names" () { setup: mockMappingsProvider.intermediaryTinyFile() >> extractFileFromZip(downloadFile(INTERMEDIARY_1_17_URL, "intermediary.jar"), "mappings/mappings.tiny") mockMinecraftProvider.getVersionInfo() >> VERSION_META_1_17 when: def mappings = getLayeredMappings( new IntermediaryMappingsSpec(), - new MojangMappingsSpec() + buildMojangMappingsSpec(true) ) def tiny = getTiny(mappings) then: @@ -45,5 +45,32 @@ class MojangMappingLayerTest extends LayeredMappingsSpecification { 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" () { + setup: + mockMappingsProvider.intermediaryTinyFile() >> extractFileFromZip(downloadFile(INTERMEDIARY_1_17_URL, "intermediary.jar"), "mappings/mappings.tiny") + mockMinecraftProvider.getVersionInfo() >> VERSION_META_1_17 + when: + def mappings = getLayeredMappings( + new IntermediaryMappingsSpec(), + buildMojangMappingsSpec(false) + ) + def tiny = getTiny(mappings) + then: + mappings.srcNamespace == "named" + mappings.dstNamespaces == ["intermediary", "official"] + mappings.classes.size() == 6113 + 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') + } + + static def buildMojangMappingsSpec(boolean nameSyntheticFields) { + def builder = MojangMappingsSpecBuilderImpl.builder() + builder.setNameSyntheticMembers(nameSyntheticFields) + return builder.build() } } diff --git a/src/test/groovy/net/fabricmc/loom/test/unit/layeredmappings/ParchmentMappingLayerTest.groovy b/src/test/groovy/net/fabricmc/loom/test/unit/layeredmappings/ParchmentMappingLayerTest.groovy index 73e2a259..2c3dc3dc 100644 --- a/src/test/groovy/net/fabricmc/loom/test/unit/layeredmappings/ParchmentMappingLayerTest.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/unit/layeredmappings/ParchmentMappingLayerTest.groovy @@ -38,7 +38,7 @@ class ParchmentMappingLayerTest extends LayeredMappingsSpecification { withMavenFile(PARCHMENT_NOTATION, downloadFile(PARCHMENT_URL, "parchment.zip")) def mappings = getLayeredMappings( new IntermediaryMappingsSpec(), - new MojangMappingsSpec(), + new MojangMappingsSpec(true), new ParchmentMappingsSpec(FileSpec.create(PARCHMENT_NOTATION), false) ) def tiny = getTiny(mappings) @@ -61,7 +61,7 @@ class ParchmentMappingLayerTest extends LayeredMappingsSpecification { withMavenFile(PARCHMENT_NOTATION, downloadFile(PARCHMENT_URL, "parchment.zip")) def mappings = getLayeredMappings( new IntermediaryMappingsSpec(), - new MojangMappingsSpec(), + new MojangMappingsSpec(true), new ParchmentMappingsSpec(FileSpec.create(PARCHMENT_NOTATION), true) ) def tiny = getTiny(mappings) -- cgit From 7b28d61fab89c2a7c89e93f8b36f25b1fb6fc291 Mon Sep 17 00:00:00 2001 From: modmuss50 Date: Thu, 25 Nov 2021 14:49:17 +0000 Subject: More improvements/fixes to shared caches maven publications + test improvements. --- build.gradle | 2 +- .../loom/configuration/RemapConfiguration.java | 30 +++++++---- .../fabricmc/loom/test/LoomTestConstants.groovy | 2 +- .../loom/test/integration/FabricAPITest.groovy | 11 ++-- .../loom/test/integration/MavenProjectTest.groovy | 1 + .../loom/test/util/GradleProjectTestTrait.groovy | 6 ++- .../fabricmc/loom/test/util/ServerRunner.groovy | 2 +- src/test/resources/patches/fabric_api.patch | 62 ++++++++++++++++++++++ 8 files changed, 97 insertions(+), 19 deletions(-) create mode 100644 src/test/resources/patches/fabric_api.patch (limited to 'src/test/groovy') diff --git a/build.gradle b/build.gradle index e70dcca1..99e3f876 100644 --- a/build.gradle +++ b/build.gradle @@ -101,7 +101,7 @@ dependencies { exclude module: 'groovy-all' } testImplementation 'io.javalin:javalin:3.13.11' - testImplementation 'net.fabricmc:fabric-installer:0.7.4' + testImplementation 'net.fabricmc:fabric-installer:0.9.0' compileOnly 'org.jetbrains:annotations:22.0.0' } diff --git a/src/main/java/net/fabricmc/loom/configuration/RemapConfiguration.java b/src/main/java/net/fabricmc/loom/configuration/RemapConfiguration.java index 1a9ac21c..efbf5a1d 100644 --- a/src/main/java/net/fabricmc/loom/configuration/RemapConfiguration.java +++ b/src/main/java/net/fabricmc/loom/configuration/RemapConfiguration.java @@ -31,6 +31,7 @@ import org.gradle.api.Action; import org.gradle.api.Project; import org.gradle.api.Task; import org.gradle.api.UnknownTaskException; +import org.gradle.api.artifacts.ConfigurablePublishArtifact; import org.gradle.api.artifacts.Configuration; import org.gradle.api.artifacts.PublishArtifact; import org.gradle.api.artifacts.dsl.ArtifactHandler; @@ -64,24 +65,17 @@ public class RemapConfiguration { if (extension.getSetupRemappedVariants().get()) { ArtifactHandler artifacts = project.getArtifacts(); project.getTasks().named(DEFAULT_REMAP_JAR_TASK_NAME, task -> { - artifacts.add(JavaPlugin.API_ELEMENTS_CONFIGURATION_NAME, task); - artifacts.add(JavaPlugin.RUNTIME_ELEMENTS_CONFIGURATION_NAME, task); + artifacts.add(JavaPlugin.API_ELEMENTS_CONFIGURATION_NAME, task, artifactConfigurationAction(task, DEFAULT_REMAP_JAR_TASK_NAME, project)); + artifacts.add(JavaPlugin.RUNTIME_ELEMENTS_CONFIGURATION_NAME, task, artifactConfigurationAction(task, DEFAULT_REMAP_JAR_TASK_NAME, project)); }); project.getTasks().named(DEFAULT_REMAP_SOURCES_JAR_TASK_NAME, RemapSourcesJarTask.class, task -> { if (!project.getConfigurations().getNames().contains(JavaPlugin.SOURCES_ELEMENTS_CONFIGURATION_NAME)) { // Sources jar may not have been created with withSourcesJar + project.getLogger().info("Not publishing sources jar as it was not found. Use java.withSourcesJar() to fix."); return; } - PublishArtifact artifact = artifacts.add(JavaPlugin.SOURCES_ELEMENTS_CONFIGURATION_NAME, task.getOutput(), configurablePublishArtifact -> { - Task remapJarTask = task; - - if (extension.getShareRemapCaches().get()) { - remapJarTask = project.getRootProject().getTasks().getByName(DEFAULT_REMAP_ALL_JARS_TASK_NAME); - } - - configurablePublishArtifact.builtBy(remapJarTask); - }); + PublishArtifact artifact = artifacts.add(JavaPlugin.SOURCES_ELEMENTS_CONFIGURATION_NAME, task.getOutput(), artifactConfigurationAction(task, DEFAULT_REMAP_ALL_SOURCES_TASK_NAME, project)); // Remove the existing artifact that does not run remapSourcesJar. // It doesn't seem to hurt, but I'm not sure if the file-level duplicates cause issues. @@ -209,4 +203,18 @@ public class RemapConfiguration { // pass } } + + private static Action artifactConfigurationAction(Task standardTask, String sharedTaskName, Project project) { + LoomGradleExtension extension = LoomGradleExtension.get(project); + + return artifact -> { + Task remapTask = standardTask; + + if (extension.getShareRemapCaches().get()) { + remapTask = project.getRootProject().getTasks().getByName(sharedTaskName); + } + + artifact.builtBy(remapTask); + }; + } } diff --git a/src/test/groovy/net/fabricmc/loom/test/LoomTestConstants.groovy b/src/test/groovy/net/fabricmc/loom/test/LoomTestConstants.groovy index 8156fce6..0c4fc294 100644 --- a/src/test/groovy/net/fabricmc/loom/test/LoomTestConstants.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/LoomTestConstants.groovy @@ -28,7 +28,7 @@ import org.gradle.util.GradleVersion class LoomTestConstants { public final static String DEFAULT_GRADLE = GradleVersion.current().getVersion() - public final static String PRE_RELEASE_GRADLE = "7.4-20211110232442+0000" + public final static String PRE_RELEASE_GRADLE = "7.4-20211124232407+0000" public final static String[] STANDARD_TEST_VERSIONS = [DEFAULT_GRADLE, PRE_RELEASE_GRADLE] } 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 49db69b9..b7300249 100644 --- a/src/test/groovy/net/fabricmc/loom/test/integration/FabricAPITest.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/integration/FabricAPITest.groovy @@ -43,10 +43,10 @@ class FabricAPITest extends Specification implements GradleProjectTestTrait { def "build and run (gradle #version)"() { setup: def gradle = gradleProject( - repo: "https://github.com/modmuss50/fabric.git", - commit: "e954edb6069e36139fd70428cfe4cddb5826c498", + repo: "https://github.com/FabricMC/fabric.git", + commit: "ce6198f63bbe0e17ba631420e9186fb72cc8b2af", version: version, -// patch: "fabric_api" + patch: "fabric_api" ) // Set the version to something constant @@ -62,8 +62,11 @@ class FabricAPITest extends Specification implements GradleProjectTestTrait { then: result.task(":build").outcome == SUCCESS + new File(gradle.mavenLocalDir, "net/fabricmc/fabric-api/fabric-biome-api-v1/3.2.2/fabric-biome-api-v1-3.2.2.jar").exists() + new File(gradle.mavenLocalDir, "net/fabricmc/fabric-api/fabric-biome-api-v1/3.2.2/fabric-biome-api-v1-3.2.2-sources.jar").exists() + serverResult.successful() - serverResult.output.contains("fabric@$API_VERSION") + serverResult.output.contains("- fabric $API_VERSION") where: version << STANDARD_TEST_VERSIONS } diff --git a/src/test/groovy/net/fabricmc/loom/test/integration/MavenProjectTest.groovy b/src/test/groovy/net/fabricmc/loom/test/integration/MavenProjectTest.groovy index f7aba96f..cda61b1b 100644 --- a/src/test/groovy/net/fabricmc/loom/test/integration/MavenProjectTest.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/integration/MavenProjectTest.groovy @@ -53,6 +53,7 @@ class MavenProjectTest extends Specification implements MockMavenServerTrait, Gr then: result.task(":publish").outcome == SUCCESS gradle.hasOutputZipEntry("fabric-example-lib-${version}.jar", "net/fabricmc/example/ExampleLib.class") + gradle.hasOutputZipEntry("fabric-example-lib-${version}-sources.jar", "net/fabricmc/example/ExampleLib.java") where: version | gradleVersion diff --git a/src/test/groovy/net/fabricmc/loom/test/util/GradleProjectTestTrait.groovy b/src/test/groovy/net/fabricmc/loom/test/util/GradleProjectTestTrait.groovy index 6380992e..fa08793d 100644 --- a/src/test/groovy/net/fabricmc/loom/test/util/GradleProjectTestTrait.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/util/GradleProjectTestTrait.groovy @@ -138,7 +138,7 @@ trait GradleProjectTestTrait { // And override the CI check to ensure that everything is ran System.setProperty("fabric.loom.test", "true") System.setProperty("fabric.loom.ci", "false") - System.setProperty("maven.repo.local", new File(getGradleHomeDir(), "m2").absolutePath) + System.setProperty("maven.repo.local", mavenLocalDir.absolutePath) def runner = this.runner def args = [] @@ -180,6 +180,10 @@ trait GradleProjectTestTrait { return new File(getProjectDir(), "build/libs/$filename") } + File getMavenLocalDir() { + return new File(gradleHomeDir, "m2") + } + void printOutputFiles() { new File(getProjectDir(), "build/libs/").listFiles().each { println(it.name) diff --git a/src/test/groovy/net/fabricmc/loom/test/util/ServerRunner.groovy b/src/test/groovy/net/fabricmc/loom/test/util/ServerRunner.groovy index d3a68768..82b519d9 100644 --- a/src/test/groovy/net/fabricmc/loom/test/util/ServerRunner.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/util/ServerRunner.groovy @@ -29,7 +29,7 @@ import groovy.transform.Immutable import java.util.concurrent.TimeUnit class ServerRunner { - static final String LOADER_VERSION = "0.11.6" + static final String LOADER_VERSION = "0.12.5" static final Map FABRIC_API_URLS = [ "1.16.5": "https://github.com/FabricMC/fabric/releases/download/0.37.1%2B1.16/fabric-api-0.37.1+1.16.jar", "1.17.1": "https://github.com/FabricMC/fabric/releases/download/0.37.1%2B1.17/fabric-api-0.37.1+1.17.jar" diff --git a/src/test/resources/patches/fabric_api.patch b/src/test/resources/patches/fabric_api.patch new file mode 100644 index 00000000..b9f578a3 --- /dev/null +++ b/src/test/resources/patches/fabric_api.patch @@ -0,0 +1,62 @@ +diff --git a/build.gradle b/build.gradle +--- a/build.gradle (revision ce6198f63bbe0e17ba631420e9186fb72cc8b2af) ++++ b/build.gradle (date 1637848132986) +@@ -31,17 +31,7 @@ + throw new NullPointerException("Could not find version for " + project.name) + } + +- if (grgit == null) { +- return version + "+nogit" +- } +- +- def latestCommits = grgit.log(paths: [project.name], maxCommits: 1) +- +- if (latestCommits.isEmpty()) { +- return version + "+uncommited" +- } +- +- return version + "+" + latestCommits.get(0).id.substring(0, 8) + DigestUtils.sha256Hex(project.rootProject.minecraft_version).substring(0, 2) ++ return version + } + + def getBranch() { +@@ -132,9 +122,8 @@ + include "**/*.java" + } + +- task sourcesJar(type: Jar, dependsOn: classes) { +- archiveClassifier = "sources" +- from sourceSets.main.allSource ++ java { ++ withSourcesJar() + } + + checkstyle { +@@ -229,12 +218,16 @@ + publications { + mavenJava(MavenPublication) { + from components.java ++ ++ artifact javadocJar + } + } + + setupRepositories(repositories) + } + ++ loom.disableDeprecatedPomGeneration(publishing.publications.mavenJava) ++ + javadoc.enabled = false + + afterEvaluate { +@@ -242,10 +235,6 @@ + genSourcesWithFernFlower.enabled = false + genSourcesWithCfr.enabled = false + unpickJar.enabled = false +- +- // Work around a loom bug causing empty jars to be pushed to maven local. +- publishMavenJavaPublicationToMavenLocal.dependsOn rootProject.tasks.getByName("remapAllJars") +- publishMavenJavaPublicationToMavenLocal.dependsOn rootProject.tasks.getByName("remapAllSources") + } + } + -- cgit