Fix Mixin Remapping for reobf tasks added by users (#197)

Author: shartte

LEGACY.md

@@ -44,25 +44,22 @@ neoForge {
 ## Reobfuscating artifacts
 Forge used SRG mappings as intermediary mappings in 1.20.1 and below. While your mod is developed against the mappings provided
 by Mojang (known as official mappings), you need to reobfuscate it to SRG mappings for it to work in production.  
-Reobfuscation will automatically be configured for the `jar` task; the non-obfuscated jar will have a `-dev` classifier
+Reobfuscation will automatically be configured for the `jar` task; the non-obfuscated jar will be moved to `build/devlibs`
 and will not be published in favour of the reobfuscated variant. You should upload the `reobfJar` task's output when using a
-task to upload to a mod hosting platform, or otherwise the jar without a `-dev` classifier if you're uploading it manually.  
+task to upload to a mod hosting platform.
+
+You may reobfuscate other jar tasks using `obfuscation.reobfuscate(TaskProvider<AbstractArchiveTask>, SourceSet, Action<RemapJarTask>)`.
 
-You may reobfuscate other jar tasks using `obfuscation.reobfuscate(TaskProvider<AbstractArchiveTask>, SourceSet, Action<RemapJarTask>)`.  
 For instance, if you want to reobfuscate a `shadowJar` task:
 ```groovy
 shadowJar {
-    // Change the classifier of the shadow jar to be -dev-all as it's not mapped in intermediary and not usable for production
-    archiveClassifier = 'dev-all'
+    archiveClassifier = 'all'
 }
 
 obfuscation {
     // Reobfuscate the shadowJar task, using the classpath of the main sourceset for properly remapping inherited members
-    reobfuscate(tasks.named('shadowJar'), sourceSets.main) {
-        // Make the reobfuscated shadowJar have the all classifier
-        // You could also change it to an empty string if you want it to not have a classifier (in that case, you will also need to change the classifier of the slim `reobfJar` task
-        archiveClassifier = 'all'
-    }
+    // This will place the original shadow jar in build/devlibs while putting this reobfuscated version in build/libs
+    reobfuscate(tasks.named('shadowJar'), sourceSets.main)
 }
 ```
 

build.gradle

@@ -71,6 +71,10 @@ sourceSets {
         runtimeClasspath += java8.output
     }
     legacy
+    test {
+        compileClasspath += legacy.output
+        runtimeClasspath += legacy.output
+    }
 }
 
 configurations {

src/legacy/java/net/neoforged/moddevgradle/legacyforge/dsl/Obfuscation.java

@@ -11,6 +11,7 @@
 import org.gradle.api.artifacts.FileCollectionDependency;
 import org.gradle.api.artifacts.ProjectDependency;
 import org.gradle.api.component.AdhocComponentWithVariants;
+import org.gradle.api.file.FileCollection;
 import org.gradle.api.file.RegularFile;
 import org.gradle.api.plugins.JavaPlugin;
 import org.gradle.api.provider.Provider;
@@ -28,18 +29,21 @@ public abstract class Obfuscation {
     private final Provider<RegularFile> mappingsCsv;
     private final Configuration autoRenamingToolRuntime;
     private final Configuration installerToolsRuntime;
+    private final FileCollection extraMixinMappings;
 
     @Inject
     public Obfuscation(Project project,
                        Provider<RegularFile> officialToSrg,
                        Provider<RegularFile> mappingsCsv,
                        Configuration autoRenamingToolRuntime,
-                       Configuration installerToolsRuntime) {
+                       Configuration installerToolsRuntime,
+                       FileCollection extraMixinMappings) {
         this.project = project;
         this.officialToSrg = officialToSrg;
         this.mappingsCsv = mappingsCsv;
         this.autoRenamingToolRuntime = autoRenamingToolRuntime;
         this.installerToolsRuntime = installerToolsRuntime;
+        this.extraMixinMappings = extraMixinMappings;
     }
 
     @ApiStatus.Internal
@@ -56,6 +60,17 @@ public void configureSrgToNamedOperation(RemapOperation operation) {
         operation.getMappings().from(mappingsCsv);
     }
 
+    /**
+     * Create a reobfuscation task.
+     *
+     * @param jar           the jar task to reobfuscate
+     * @param sourceSet     the source set whose classpath to use when remapping inherited methods
+     * @return a provider of the created task
+     */
+    public TaskProvider<RemapJar> reobfuscate(TaskProvider<? extends AbstractArchiveTask> jar, SourceSet sourceSet) {
+        return reobfuscate(jar, sourceSet, ignored -> {});
+    }
+
     /**
      * Create and configure a reobfuscation task.
      *
@@ -77,6 +92,7 @@ public TaskProvider<RemapJar> reobfuscate(TaskProvider<? extends AbstractArchive
             task.getArchiveAppendix().convention(jar.flatMap(AbstractArchiveTask::getArchiveAppendix));
             task.getLibraries().from(sourceSet.getCompileClasspath());
             configureNamedToSrgOperation(task.getRemapOperation());
+            task.getRemapOperation().getMappings().from(extraMixinMappings);
             configuration.execute(task);
         });
 

src/legacy/java/net/neoforged/moddevgradle/legacyforge/internal/LegacyForgeModDevPlugin.java

@@ -66,9 +66,10 @@ public void apply(Project project) {
         var intermediateToNamed = modDevBuildDir.map(d -> d.file("intermediateToNamed.srg"));
         var mappingsCsv = modDevBuildDir.map(d -> d.file("intermediateToNamed.zip"));
 
-        var obf = project.getExtensions().create("obfuscation", Obfuscation.class, project, namedToIntermediate, mappingsCsv, autoRenamingToolRuntime, installerToolsRuntime);
-
+        // This collection is used to share the files added by mixin with the obfuscation extension
         var extraMixinMappings = project.files();
+
+        var obf = project.getExtensions().create("obfuscation", Obfuscation.class, project, namedToIntermediate, mappingsCsv, autoRenamingToolRuntime, installerToolsRuntime, extraMixinMappings);
         var mixin = project.getExtensions().create("mixin", MixinExtension.class, project, namedToIntermediate, extraMixinMappings);
 
         project.getExtensions().configure(NeoForgeExtension.class, extension -> {
@@ -92,10 +93,7 @@ public void apply(Project project) {
 
         var reobfJar = obf.reobfuscate(
                 project.getTasks().named(JavaPlugin.JAR_TASK_NAME, Jar.class),
-                project.getExtensions().getByType(SourceSetContainer.class).getByName(SourceSet.MAIN_SOURCE_SET_NAME),
-                task -> {
-                    task.getRemapOperation().getMappings().from(extraMixinMappings);
-                }
+                project.getExtensions().getByType(SourceSetContainer.class).getByName(SourceSet.MAIN_SOURCE_SET_NAME)
         );
 
         project.getTasks().named("assemble", assemble -> assemble.dependsOn(reobfJar));

src/test/java/net/neoforged/moddevgradle/legacyforge/dsl/MixinMappingTest.java

@@ -0,0 +1,42 @@
+package net.neoforged.moddevgradle.legacyforge.dsl;
+
+import net.neoforged.moddevgradle.internal.utils.ExtensionUtils;
+import net.neoforged.moddevgradle.legacyforge.internal.LegacyForgeModDevPlugin;
+import net.neoforged.moddevgradle.legacyforge.tasks.RemapJar;
+import org.gradle.jvm.tasks.Jar;
+import org.gradle.testfixtures.ProjectBuilder;
+import org.junit.jupiter.api.Test;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+class MixinMappingTest {
+    /**
+     * Tests that we expect the Mixin AP to create for each refmap is added to the reobfuscation tasks
+     * (both the default reobfJar and custom ones).
+     */
+    @Test
+    public void testMixinMappingsArePropagatedToObfuscationTasks() {
+        var project = ProjectBuilder.builder().build();
+        project.getPlugins().apply(LegacyForgeModDevPlugin.class);
+
+        var obfuscation = ExtensionUtils.getExtension(project, "obfuscation", Obfuscation.class);
+        var sourceSets = ExtensionUtils.getSourceSets(project);
+        var mixinExtension = ExtensionUtils.getExtension(project, "mixin", MixinExtension.class);
+        var mainSourceSet = sourceSets.getByName("main");
+        mixinExtension.add(mainSourceSet, "testmod.refmap.json");
+
+        var someJarTask = project.getTasks().register("someJar", Jar.class);
+        var customRemapJarTask = obfuscation.reobfuscate(someJarTask, mainSourceSet).get();
+        var remapJarTask = (RemapJar) project.getTasks().getByName("reobfJar");
+
+        // The main named->intermediary mappings for the game
+        var namedToIntermediary = project.getLayout().getBuildDirectory().file("moddev/namedToIntermediate.tsrg").get().getAsFile();
+        var mixinApMappings = project.getLayout().getBuildDirectory().file("mixin/testmod.refmap.json.mappings.tsrg").get().getAsFile();
+
+        // The mapping file produced by the Mixin AP should be added as an input to both Jar tasks.
+        var otherMappings = customRemapJarTask.getRemapOperation().getMappings().getFiles();
+        assertThat(otherMappings).containsOnly(namedToIntermediary, mixinApMappings);
+        var mappings = remapJarTask.getRemapOperation().getMappings().getFiles();
+        assertThat(mappings).containsOnly(namedToIntermediary, mixinApMappings);
+    }
+}