Refactor ExtraJarDependencyManager and enhance error handling in file downloads

Author: marchermans

common/src/functionalTest/groovy/net/neoforged/gradle/common/FunctionalTests.groovy

@@ -0,0 +1,40 @@
+package net.neoforged.gradle.common
+
+
+import net.neoforged.trainingwheels.gradle.functional.BuilderBasedTestSpecification
+import org.gradle.testkit.runner.TaskOutcome
+
+class FunctionalTests extends BuilderBasedTestSpecification {
+
+    @Override
+    protected void configurePluginUnderTest() {
+        pluginUnderTest = "net.neoforged.gradle.common";
+        injectIntoAllProject = true;
+    }
+
+    def "a mod with common as dependency can run build with the client-extra dependencies"() {
+        given:
+        def project = create "common-can-run-clean-build", {
+            it.build("""
+            java.toolchain.languageVersion = JavaLanguageVersion.of(25)
+            
+            dependencies {
+                implementation "net.minecraft:client:+:client-extra"
+            }
+            """)
+            it.withToolchains()
+            it.withGlobalCacheDirectory(tempDir)
+        }
+
+        when:
+        def run = project.run {
+            it.tasks(':clean', ':build')
+            it.stacktrace()
+            it.debug()
+        }
+
+        then:
+        run.task(':clean').outcome == TaskOutcome.SUCCESS || run.task(':clean').outcome == TaskOutcome.UP_TO_DATE
+        run.task(':build').outcome == TaskOutcome.SUCCESS
+    }
+}

common/src/main/java/net/neoforged/gradle/common/dependency/ExtraJarDependencyManager.java

@@ -3,10 +3,10 @@
 import com.google.common.collect.Maps;
 import net.neoforged.gradle.common.extensions.problems.IProblemReporter;
 import net.neoforged.gradle.common.runtime.tasks.GenerateExtraJar;
+import net.neoforged.gradle.common.util.ConfigurationUtils;
 import net.neoforged.gradle.dsl.common.extensions.MinecraftArtifactCache;
 import net.neoforged.gradle.dsl.common.extensions.dependency.replacement.DependencyReplacement;
 import net.neoforged.gradle.dsl.common.extensions.dependency.replacement.ReplacementResult;
-import net.neoforged.gradle.common.util.ConfigurationUtils;
 import net.neoforged.gradle.dsl.common.tasks.WithOutput;
 import net.neoforged.gradle.dsl.common.util.DistributionType;
 import net.neoforged.gradle.dsl.common.util.GameArtifact;
@@ -18,72 +18,92 @@
 import org.gradle.api.tasks.TaskProvider;
 
 import javax.inject.Inject;
-import java.util.*;
+import java.util.Collections;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
 
-public abstract class ExtraJarDependencyManager {
+public abstract class ExtraJarDependencyManager
+{
 
     private final Map<String, ReplacementResult> replacements = Maps.newHashMap();
 
-    public static String generateClientCoordinateFor(final String version) {
+    public static String generateClientCoordinateFor(final String version)
+    {
         return "net.minecraft:client:" + version + ":client-extra";
     }
-    
-    public static String generateServerCoordinateFor(final String version) {
+
+    public static String generateServerCoordinateFor(final String version)
+    {
         return "net.minecraft:client:" + version + ":client-extra";
     }
-    
-    public static String generateCoordinateFor(final DistributionType type, final String version) {
+
+    public static String generateCoordinateFor(final DistributionType type, final String version)
+    {
         return String.format("net.minecraft:%s:%s:%s-extra", type.getName().toLowerCase(Locale.ROOT), version, type.getName().toLowerCase(Locale.ROOT));
     }
 
     @Inject
-    public ExtraJarDependencyManager(final Project project) {
+    public ExtraJarDependencyManager(final Project project)
+    {
         final DependencyReplacement dependencyReplacementsExtension = project.getExtensions().getByType(DependencyReplacement.class);
 
         dependencyReplacementsExtension.getReplacementHandlers().register("extraJar", dependencyReplacementHandler -> dependencyReplacementHandler.getReplacer().set(context -> {
-                if (isNotAMatchingDependency(context.getDependency()))
-                    return Optional.empty();
+            if (isNotAMatchingDependency(context.getDependency()))
+            {
+                return Optional.empty();
+            }
 
-                return Optional.of(generateReplacement(project, context.getDependency()));
-            }));
+            return Optional.of(generateReplacement(project, context.getDependency()));
+        }));
     }
 
-    private boolean isNotAMatchingDependency(final Dependency dependencyToCheck) {
-        if (dependencyToCheck instanceof ExternalModuleDependency) {
+    private boolean isNotAMatchingDependency(final Dependency dependencyToCheck)
+    {
+        if (dependencyToCheck instanceof ExternalModuleDependency)
+        {
             final ExternalModuleDependency externalModuleDependency = (ExternalModuleDependency) dependencyToCheck;
-            return externalModuleDependency.getGroup() == null || !externalModuleDependency.getGroup().equals("net.minecraft") || !isSupportedSide(dependencyToCheck) || !hasMatchingArtifact(externalModuleDependency);
+            return externalModuleDependency.getGroup() == null || !externalModuleDependency.getGroup().equals("net.minecraft") || !isSupportedSide(dependencyToCheck)
+                || !hasMatchingArtifact(externalModuleDependency);
         }
 
         return true;
     }
 
-    private boolean isSupportedSide(final Dependency dependency) {
+    private boolean isSupportedSide(final Dependency dependency)
+    {
         return dependency.getName().equals("client") || dependency.getName().equals("server");
     }
 
-    private boolean hasMatchingArtifact(ExternalModuleDependency externalModuleDependency) {
-        if (externalModuleDependency.getVersion() == null) {
+    private boolean hasMatchingArtifact(ExternalModuleDependency externalModuleDependency)
+    {
+        if (externalModuleDependency.getVersion() == null)
+        {
             return false;
         }
 
-        if (externalModuleDependency.getArtifacts().size() != 1) {
+        if (externalModuleDependency.getArtifacts().size() != 1)
+        {
             return false;
         }
 
         final DependencyArtifact artifact = externalModuleDependency.getArtifacts().iterator().next();
         return Objects.equals(artifact.getClassifier(), "client-extra") || Objects.equals(artifact.getClassifier(), "server-extra");
     }
 
-    private ReplacementResult generateReplacement(final Project project, final Dependency dependency) {
+    private ReplacementResult generateReplacement(final Project project, final Dependency dependency)
+    {
         final String minecraftVersion = dependency.getVersion();
-        if (minecraftVersion == null) {
+        if (minecraftVersion == null)
+        {
             final IProblemReporter problemReporter = project.getExtensions().getByType(IProblemReporter.class);
             throw problemReporter.throwing(spec -> {
-                        spec.id("dependencies.extra-jar", "missingVersion")
-                        .contextualLabel("Client-Extra Jar: Missing Version")
-                        .details("The dependency %s does not have a version specified".formatted(dependency))
-                        .solution("Specify a version for the dependency")
-                        .section("common-dep-management-extra-jar");
+                spec.id("dependencies.extra-jar", "missingVersion")
+                    .contextualLabel("Client-Extra Jar: Missing Version")
+                    .details("The dependency %s does not have a version specified".formatted(dependency))
+                    .solution("Specify a version for the dependency")
+                    .section("common-dep-management-extra-jar");
             });
         }
 
@@ -92,22 +112,27 @@ private ReplacementResult generateReplacement(final Project project, final Depen
 
             Map<GameArtifact, TaskProvider<? extends WithOutput>> tasks = minecraftArtifactCacheExtension.cacheGameVersionTasks(project, minecraftVersion, DistributionType.JOINED);
 
-            final TaskProvider<GenerateExtraJar> extraJarTaskProvider = project.getTasks().register("create" + minecraftVersion + StringUtils.capitalize(dependency.getName()) + "ExtraJar", GenerateExtraJar.class, task -> {
-                task.getOriginalJar().set(tasks.get(GameArtifact.CLIENT_JAR).flatMap(WithOutput::getOutput));
-                task.getServerJar().set(tasks.get(GameArtifact.EXTRACTED_SERVER_JAR).flatMap(WithOutput::getOutput));
-                task.getMappings().set(tasks.get(GameArtifact.CLIENT_MAPPINGS).flatMap(WithOutput::getOutput));
-                task.getOutput().set(project.getLayout().getBuildDirectory().dir("jars/extra/" + dependency.getName()).map(cacheDir -> cacheDir.dir(Objects.requireNonNull(minecraftVersion)).file( dependency.getName() + "-extra.jar")));
-            });
+            final TaskProvider<GenerateExtraJar> extraJarTaskProvider =
+                project.getTasks().register("create" + minecraftVersion + StringUtils.capitalize(dependency.getName()) + "ExtraJar", GenerateExtraJar.class, task -> {
+                    task.getOriginalJar().set(tasks.get(GameArtifact.CLIENT_JAR).flatMap(WithOutput::getOutput));
+                    task.getServerJar().set(tasks.get(GameArtifact.EXTRACTED_SERVER_JAR).flatMap(WithOutput::getOutput));
+                    task.getMappings().set(tasks.get(GameArtifact.CLIENT_MAPPINGS).flatMap(WithOutput::getOutput));
+                    task.getOutput()
+                        .set(project.getLayout()
+                            .getBuildDirectory()
+                            .dir("jars/extra/" + dependency.getName())
+                            .map(cacheDir -> cacheDir.dir(Objects.requireNonNull(minecraftVersion)).file(dependency.getName() + "-extra.jar")));
+                });
 
             return new ReplacementResult(
-                    project,
-                    extraJarTaskProvider,
-                    project.getConfigurations().detachedConfiguration(),
-                    ConfigurationUtils.temporaryUnhandledConfiguration(
-                            project.getConfigurations(),
-                            "EmptyExtraJarConfigurationFor" + minecraftVersion.replace(".", "_")
-                    ),
-                    Collections.emptySet()
+                project,
+                extraJarTaskProvider,
+                project.getConfigurations().detachedConfiguration(),
+                ConfigurationUtils.temporaryUnhandledConfiguration(
+                    project.getConfigurations(),
+                    "EmptyExtraJarConfigurationFor" + minecraftVersion.replace(".", "_")
+                ),
+                Collections.emptySet()
             );
         });
     }

common/src/main/java/net/neoforged/gradle/common/runtime/tasks/GenerateExtraJar.java

@@ -1,5 +1,6 @@
 package net.neoforged.gradle.common.runtime.tasks;
 
+import net.minecraftforge.srgutils.IMappingBuilder;
 import net.minecraftforge.srgutils.IMappingFile;
 import net.neoforged.gradle.common.services.caching.CachedExecutionService;
 import net.neoforged.gradle.common.services.caching.jobs.ICacheableJob;
@@ -14,13 +15,15 @@
 import org.gradle.api.services.ServiceReference;
 import org.gradle.api.tasks.CacheableTask;
 import org.gradle.api.tasks.InputFile;
+import org.gradle.api.tasks.Optional;
 import org.gradle.api.tasks.PathSensitive;
 import org.gradle.api.tasks.PathSensitivity;
 import org.gradle.api.tasks.TaskAction;
 
 import java.io.BufferedOutputStream;
 import java.io.File;
 import java.io.FileOutputStream;
+import java.io.IOException;
 import java.util.HashSet;
 import java.util.Set;
 import java.util.jar.Attributes;
@@ -55,7 +58,7 @@ private void doRun() throws Exception {
         final File outputJar = ensureFileWorkspaceReady(getOutput());
 
         // Official mappings are Named -> Obf and need to be reversed
-        var mappings = IMappingFile.load(getMappings().getAsFile().get()).reverse();
+        var mappings = getMappings().isPresent() ? getReverse() : null;
         try (var clientZip = new JarFile(getOriginalJar().getAsFile().get());
              var serverZip = new JarFile(getServerJar().getAsFile().get())) {
             var clientFiles = getFileIndex(clientZip);
@@ -67,8 +70,10 @@ private void doRun() throws Exception {
             manifest.getMainAttributes().put(Attributes.Name.MANIFEST_VERSION, "1.0");
             manifest.getMainAttributes().putValue("Minecraft-Dists", "server client");
 
-            addSourceDistEntries(clientFiles, serverFiles, "client", mappings, manifest);
-            addSourceDistEntries(serverFiles, clientFiles, "server", mappings, manifest);
+            if (mappings != null) {
+                addSourceDistEntries(clientFiles, serverFiles, "client", mappings, manifest);
+                addSourceDistEntries(serverFiles, clientFiles, "server", mappings, manifest);
+            }
 
             try (var zos = new ZipOutputStream(new BufferedOutputStream(new FileOutputStream(outputJar)))) {
                 zos.putNextEntry(FileUtils.getStableEntry(JarFile.MANIFEST_NAME));
@@ -91,6 +96,15 @@ private void doRun() throws Exception {
         }
     }
 
+    private IMappingFile getReverse() throws IOException
+    {
+        final File mappingsFile = getMappings().getAsFile().get();
+        if (mappingsFile.length() > 0)
+            return IMappingFile.load(getMappings().getAsFile().get()).reverse();
+
+        return IMappingBuilder.create("left", "right").build().getMap("left", "right");
+    }
+
     private static void addSourceDistEntries(Set<String> distFiles,
                                              Set<String> otherDistFiles,
                                              String dist,

common/src/main/java/net/neoforged/gradle/common/tasks/FileCacheProviding.java

@@ -52,6 +52,22 @@ protected void downloadJsonTo(String url) {
     protected File doDownloadVersionDownloadToCache(final String artifact, final String potentialError, File versionManifest) {
         JsonObject json = SerializationUtils.fromJson(versionManifest, JsonObject.class);
 
+        final JsonObject downloads = json.getAsJsonObject("downloads");
+        if (!downloads.has(artifact)) {
+            final File output = getOutput().get().getAsFile();
+            if (output.exists())
+                output.delete();
+
+            try
+            {
+                output.createNewFile();
+                return output;
+            }
+            catch (IOException e)
+            {
+                throw new RuntimeException(e);
+            }
+        }
         final JsonObject artifactInfo = json.getAsJsonObject("downloads").getAsJsonObject(artifact);
         final String url = artifactInfo.get("url").getAsString();
         final String hash = artifactInfo.get("sha1").getAsString();