Review comments

neoforged · Nov 30, 2024 · 1e1752a · 1e1752a
1 parent 84a82bc
commit 1e1752a
Show file tree

Hide file tree

Showing 5 changed files with 46 additions and 17 deletions.
diff --git a/legacytest/forge/build.gradle b/legacytest/forge/build.gradle
@@ -1,5 +1,6 @@
 plugins {
     id 'net.neoforged.moddev.legacyforge'
+    id 'maven-publish'
 }
 
 repositories {
@@ -50,3 +51,16 @@ neoForge {
         }
     }
 }
+
+publishing {
+    publications {
+        maven(MavenPublication) {
+            from components.java
+        }
+    }
+    repositories {
+        maven {
+            url rootProject.file('repo')
+        }
+    }
+}
diff --git a/src/legacy/java/net/neoforged/moddevgradle/internal/LegacyForgeFacade.java b/src/legacy/java/net/neoforged/moddevgradle/internal/LegacyForgeFacade.java
@@ -5,6 +5,7 @@
 
 public class LegacyForgeFacade {
     public static void configureRun(Project project, RunModel run) {
+        // This will explicitly be replaced in RunUtils to make this work for IDEs
         run.getEnvironment().put("MOD_CLASSES", RunUtils.getGradleModFoldersProvider(project, run.getLoadedMods(), null).getClassesArgument());
     }
 }
diff --git a/src/legacy/java/net/neoforged/moddevgradle/legacyforge/internal/LegacyForgeModDevPlugin.java b/src/legacy/java/net/neoforged/moddevgradle/legacyforge/internal/LegacyForgeModDevPlugin.java
@@ -85,7 +85,6 @@ public void apply(Project project) {
                 project.getTasks().named(JavaPlugin.JAR_TASK_NAME, Jar.class),
                 project.getExtensions().getByType(SourceSetContainer.class).getByName(SourceSet.MAIN_SOURCE_SET_NAME),
                 task -> {
-                    task.getArchiveClassifier().set("");
                     task.getParameters().getMappings().from(extraMixinMappings);
                 }
         );
@@ -98,6 +97,8 @@ public void apply(Project project) {
                 .getDependencies().add(project.getDependencyFactory().create(project.files(mappingsCsv)));
 
         // Forge expects to find the Forge and client-extra jar on the legacy classpath
+        // Newer FML versions also search for it on the java.class.path.
+        // MDG already adds cilent-extra, but the forge jar is missing.
         project.getConfigurations().getByName("additionalRuntimeClasspath")
                 .extendsFrom(project.getConfigurations().getByName(ModDevPlugin.CONFIGURATION_RUNTIME_DEPENDENCIES))
                 .exclude(Map.of("group", "net.neoforged", "module", "DevLaunch"));

diff --git a/src/legacy/java/net/neoforged/moddevgradle/legacyforge/internal/Obfuscation.java b/src/legacy/java/net/neoforged/moddevgradle/legacyforge/internal/Obfuscation.java
@@ -16,6 +16,7 @@
 import org.gradle.api.tasks.bundling.AbstractArchiveTask;
 
 import javax.inject.Inject;
+import java.io.File;
 import java.util.List;
 
 abstract class Obfuscation {
@@ -41,16 +42,25 @@ public Obfuscation(Project project, Provider<RegularFile> officialToSrg, Provide
      * @param configuration an action used to configure the rebfuscation task
      * @return a provider of the created task
      */
-    public TaskProvider<RemapJarTask> reobfuscate(TaskProvider<? extends AbstractArchiveTask> jar,
-                                                  SourceSet sourceSet,
-                                                  Action<RemapJarTask> configuration) {
-        var reobf = project.getTasks().register("reobf" + StringUtils.capitalize(jar.getName()), RemapJarTask.class, task -> {
-            task.getInput().set(jar.flatMap(AbstractArchiveTask::getArchiveFile));
-            task.getDestinationDirectory().convention(jar.flatMap(AbstractArchiveTask::getDestinationDirectory));
-            task.getArchiveBaseName().convention(jar.flatMap(AbstractArchiveTask::getArchiveBaseName));
-            task.getArchiveVersion().convention(jar.flatMap(AbstractArchiveTask::getArchiveVersion));
-            task.getArchiveClassifier().convention(jar.flatMap(AbstractArchiveTask::getArchiveClassifier).map(c -> c + "-reobf"));
-            task.getArchiveAppendix().convention(jar.flatMap(AbstractArchiveTask::getArchiveAppendix));
+    public TaskProvider<RemapJar> reobfuscate(TaskProvider<? extends AbstractArchiveTask> jar,
+                                              SourceSet sourceSet,
+                                              Action<RemapJar> configuration) {
+        var reobf = project.getTasks().register("reobf" + StringUtils.capitalize(jar.getName()), RemapJar.class, task -> {
+            var jarFile = jar.flatMap(AbstractArchiveTask::getArchiveFile);
+            task.getInput().set(jarFile);
+            task.getOutput().convention(jarFile.flatMap(regularFile -> {
+                var f = regularFile.getAsFile();
+                String newFilename;
+                var extSep = f.getName().lastIndexOf('.');
+                if (extSep != -1) {
+                    newFilename = f.getName().substring(0, extSep)
+                            + "-reobf" + f.getName().substring(extSep);
+                } else {
+                    newFilename = f.getName() + "-reobf";
+                }
+                return jar.flatMap(AbstractArchiveTask::getDestinationDirectory)
+                        .map(dir -> dir.file(newFilename));
+            }));
             task.getLibraries().from(sourceSet.getCompileClasspath());
             task.getParameters().from(this, RemapParameters.ToolType.ART);
             configuration.execute(task);
@@ -60,8 +70,8 @@ public TaskProvider<RemapJarTask> reobfuscate(TaskProvider<? extends AbstractArc
 
         var java = (AdhocComponentWithVariants) project.getComponents().getByName("java");
         for (var configurationNames : List.of(JavaPlugin.RUNTIME_ELEMENTS_CONFIGURATION_NAME, JavaPlugin.API_ELEMENTS_CONFIGURATION_NAME)) {
+            // Ensure that the published jar is the obfuscated jar, not the plain one
             project.getArtifacts().add(configurationNames, reobf);
-
             java.withVariantsFromConfiguration(project.getConfigurations().getByName(configurationNames), variant -> {
                 variant.getConfigurationVariant().getArtifacts().removeIf(artifact -> artifact.getFile().equals(jar.get().getArchiveFile().get().getAsFile()));
             });

diff --git a/...le/legacyforge/internal/RemapJarTask.java → ...gradle/legacyforge/internal/RemapJar.java b/...le/legacyforge/internal/RemapJarTask.java → ...gradle/legacyforge/internal/RemapJar.java
@@ -1,13 +1,14 @@
 package net.neoforged.moddevgradle.legacyforge.internal;
 
+import org.gradle.api.DefaultTask;
 import org.gradle.api.file.ConfigurableFileCollection;
 import org.gradle.api.file.RegularFileProperty;
 import org.gradle.api.tasks.InputFile;
 import org.gradle.api.tasks.InputFiles;
 import org.gradle.api.tasks.Nested;
 import org.gradle.api.tasks.Optional;
+import org.gradle.api.tasks.OutputFile;
 import org.gradle.api.tasks.TaskAction;
-import org.gradle.jvm.tasks.Jar;
 import org.gradle.process.ExecOperations;
 
 import javax.inject.Inject;
@@ -16,14 +17,13 @@
 /**
  * Task used to remap a jar using AutoRenamingTool.
  */
-abstract class RemapJarTask extends Jar {
+abstract class RemapJar extends DefaultTask {
 
     @Nested
     protected abstract RemapParameters getParameters();
 
     @Inject
-    public RemapJarTask() {
-        super();
+    public RemapJar() {
     }
 
     /**
@@ -36,11 +36,14 @@ public RemapJarTask() {
     @InputFile
     public abstract RegularFileProperty getInput();
 
+    @OutputFile
+    public abstract RegularFileProperty getOutput();
+
     @Inject
     protected abstract ExecOperations getExecOperations();
 
     @TaskAction
     public void remap() throws IOException {
-        getParameters().execute(getExecOperations(), getInput().getAsFile().get(), getArchiveFile().get().getAsFile(), getLibraries());
+        getParameters().execute(getExecOperations(), getInput().getAsFile().get(), getOutput().get().getAsFile(), getLibraries());
     }
 }