Fix named attributes in legacy failing when reading back data from Mo…

…dule Metadata (#225)

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

@@ -4,6 +4,7 @@
 import java.util.Objects;
 import javax.inject.Inject;
 import net.neoforged.moddevgradle.legacyforge.internal.MinecraftMappings;
+import net.neoforged.moddevgradle.legacyforge.internal.SrgMappingsRule;
 import net.neoforged.moddevgradle.legacyforge.tasks.RemapJar;
 import net.neoforged.moddevgradle.legacyforge.tasks.RemapOperation;
 import org.apache.commons.lang3.StringUtils;
@@ -32,6 +33,9 @@ public abstract class ObfuscationExtension {
     private final Configuration installerToolsRuntime;
     private final FileCollection extraMixinMappings;
 
+    private final MinecraftMappings namedMappings;
+    private final MinecraftMappings srgMappings;
+
     @Inject
     public ObfuscationExtension(Project project,
             Configuration autoRenamingToolRuntime,
@@ -41,6 +45,9 @@ public ObfuscationExtension(Project project,
         this.autoRenamingToolRuntime = autoRenamingToolRuntime;
         this.installerToolsRuntime = installerToolsRuntime;
         this.extraMixinMappings = extraMixinMappings;
+
+        this.namedMappings = project.getObjects().named(MinecraftMappings.class, MinecraftMappings.NAMED);
+        this.srgMappings = project.getObjects().named(MinecraftMappings.class, MinecraftMappings.SRG);
     }
 
     private <T> Provider<T> assertConfigured(Provider<T> provider) {
@@ -123,7 +130,7 @@ public TaskProvider<RemapJar> reobfuscate(TaskProvider<? extends AbstractArchive
             var config = configurations.getByName(configurationName);
             // Mark the original configuration as NAMED to be able to disambiguate between it and the reobfuscated jar,
             // this is used for example by the JarJar configuration.
-            config.getAttributes().attribute(MinecraftMappings.ATTRIBUTE, MinecraftMappings.NAMED);
+            config.getAttributes().attribute(MinecraftMappings.ATTRIBUTE, namedMappings);
 
             // Now create a reobf configuration
             var reobfConfig = configurations.maybeCreate("reobf" + StringUtils.capitalize(configurationName));
@@ -132,7 +139,7 @@ public TaskProvider<RemapJar> reobfuscate(TaskProvider<? extends AbstractArchive
             for (var attribute : config.getAttributes().keySet()) {
                 copyAttribute(project, attribute, config, reobfConfig);
             }
-            reobfConfig.getAttributes().attribute(MinecraftMappings.ATTRIBUTE, MinecraftMappings.SRG);
+            reobfConfig.getAttributes().attribute(MinecraftMappings.ATTRIBUTE, srgMappings);
             project.getArtifacts().add(reobfConfig.getName(), reobf);
 
             // Publish the reobf configuration instead of the original one to Maven
@@ -156,11 +163,8 @@ private static <T> void copyAttribute(Project project, Attribute<T> attribute, C
     public Configuration createRemappingConfiguration(Configuration parent) {
         var remappingConfig = project.getConfigurations().create("mod" + StringUtils.capitalize(parent.getName()), spec -> {
             spec.setDescription("Configuration for dependencies of " + parent.getName() + " that needs to be remapped");
-            spec.attributes(attributeContainer -> {
-                attributeContainer.attribute(MinecraftMappings.ATTRIBUTE, MinecraftMappings.SRG);
-            });
             spec.setCanBeConsumed(false);
-            spec.setCanBeResolved(false);
+            spec.setCanBeResolved(true);
             spec.setTransitive(false);
 
             // Unfortunately, if we simply try to make the parent extend this config, transformations will not run because the parent doesn't request remapped deps
@@ -169,29 +173,39 @@ public Configuration createRemappingConfiguration(Configuration parent) {
             // Additionally, we force dependencies to be non-transitive since we cannot apply the attribute hack to transitive dependencies.
             spec.withDependencies(dependencies -> dependencies.forEach(dep -> {
                 if (dep instanceof ExternalModuleDependency externalModuleDependency) {
-                    project.getDependencies().constraints(constraints -> {
-                        constraints.add(parent.getName(), externalModuleDependency.getGroup() + ":" + externalModuleDependency.getName() + ":" + externalModuleDependency.getVersion(), c -> {
-                            c.attributes(a -> a.attribute(MinecraftMappings.ATTRIBUTE, MinecraftMappings.SRG));
-                        });
-                    });
                     externalModuleDependency.setTransitive(false);
+
+                    // This rule ensures that this external module will be enriched with the attribute MAPPINGS=SRG
+                    project.getDependencies().getComponents().withModule(
+                            dep.getGroup() + ":" + dep.getName(), SrgMappingsRule.class, cfg -> {
+                                cfg.params(srgMappings);
+                            });
                 } else if (dep instanceof FileCollectionDependency fileCollectionDependency) {
                     project.getDependencies().constraints(constraints -> {
                         constraints.add(parent.getName(), fileCollectionDependency.getFiles(), c -> {
-                            c.attributes(a -> a.attribute(MinecraftMappings.ATTRIBUTE, MinecraftMappings.SRG));
+                            c.attributes(a -> a.attribute(MinecraftMappings.ATTRIBUTE, namedMappings));
                         });
                     });
                 } else if (dep instanceof ProjectDependency projectDependency) {
                     project.getDependencies().constraints(constraints -> {
                         constraints.add(parent.getName(), projectDependency.getDependencyProject(), c -> {
-                            c.attributes(a -> a.attribute(MinecraftMappings.ATTRIBUTE, MinecraftMappings.SRG));
+                            c.attributes(a -> a.attribute(MinecraftMappings.ATTRIBUTE, namedMappings));
                         });
                     });
                     projectDependency.setTransitive(false);
                 }
             }));
         });
-        parent.extendsFrom(remappingConfig);
+
+        var remappedDep = project.getDependencyFactory().create(
+                remappingConfig.getIncoming().artifactView(view -> {
+                    view.attributes(a -> a.attribute(MinecraftMappings.ATTRIBUTE, namedMappings));
+                }).getFiles());
+        remappedDep.because("Remapped mods from " + remappingConfig.getName());
+
+        parent.getDependencies().add(
+                remappedDep);
+
         return remappingConfig;
     }
 }

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

@@ -2,6 +2,7 @@
 
 import java.net.URI;
 import java.util.stream.Stream;
+import javax.inject.Inject;
 import net.neoforged.minecraftdependencies.MinecraftDependenciesPlugin;
 import net.neoforged.moddevgradle.internal.ArtifactNamingStrategy;
 import net.neoforged.moddevgradle.internal.Branding;
@@ -23,6 +24,7 @@
 import org.gradle.api.Project;
 import org.gradle.api.artifacts.ProjectDependency;
 import org.gradle.api.artifacts.type.ArtifactTypeDefinition;
+import org.gradle.api.model.ObjectFactory;
 import org.gradle.api.plugins.JavaLibraryPlugin;
 import org.gradle.api.plugins.JavaPlugin;
 import org.gradle.api.tasks.SourceSet;
@@ -43,6 +45,15 @@ public class LegacyForgeModDevPlugin implements Plugin<Project> {
     public static final String CONFIGURATION_TOOL_ART = "autoRenamingToolRuntime";
     public static final String CONFIGURATION_TOOL_INSTALLERTOOLS = "installerToolsRuntime";
 
+    private final MinecraftMappings namedMappings;
+    private final MinecraftMappings srgMappings;
+
+    @Inject
+    public LegacyForgeModDevPlugin(ObjectFactory objectFactory) {
+        namedMappings = objectFactory.named(MinecraftMappings.class, MinecraftMappings.NAMED);
+        srgMappings = objectFactory.named(MinecraftMappings.class, MinecraftMappings.SRG);
+    }
+
     @Override
     public void apply(Project project) {
         project.getPlugins().apply(JavaLibraryPlugin.class);
@@ -203,36 +214,37 @@ public void enable(Project project, LegacyForgeModdingSettings settings, LegacyF
                 parameters.getMinecraftDependencies().from(remapDeps);
             });
             params.getFrom()
-                    .attribute(MinecraftMappings.ATTRIBUTE, MinecraftMappings.NAMED)
+                    .attribute(MinecraftMappings.ATTRIBUTE, srgMappings)
                     .attribute(ArtifactTypeDefinition.ARTIFACT_TYPE_ATTRIBUTE, ArtifactTypeDefinition.JAR_TYPE);
             params.getTo()
-                    .attribute(MinecraftMappings.ATTRIBUTE, MinecraftMappings.SRG)
+                    .attribute(MinecraftMappings.ATTRIBUTE, namedMappings)
                     .attribute(ArtifactTypeDefinition.ARTIFACT_TYPE_ATTRIBUTE, ArtifactTypeDefinition.JAR_TYPE);
         });
     }
 
-    private static void configureDependencyRemapping(Project project, ObfuscationExtension obf) {
+    private void configureDependencyRemapping(Project project, ObfuscationExtension obf) {
         // JarJar cross-project dependencies are packaged into the final jar and should be remapped
         // We must however do this without affecting external dependencies since those are usually already in the
         // right namespace.
         var sourceSets = ExtensionUtils.getSourceSets(project);
         sourceSets.all(sourceSet -> {
             var configurationName = sourceSet.getTaskName(null, "jarJar");
-            project.getConfigurations().getByName("jarJar").withDependencies(dependencies -> {
+            project.getConfigurations().getByName(configurationName).withDependencies(dependencies -> {
                 dependencies.forEach(dep -> {
                     if (dep instanceof ProjectDependency projectDependency) {
                         projectDependency.attributes(a -> {
-                            a.attribute(MinecraftMappings.ATTRIBUTE, MinecraftMappings.SRG);
+                            a.attribute(MinecraftMappings.ATTRIBUTE, srgMappings);
                         });
                     }
                 });
             });
         });
 
-        project.getDependencies().attributesSchema(schema -> schema.attribute(MinecraftMappings.ATTRIBUTE).getDisambiguationRules().add(MappingsDisambiguationRule.class));
-        project.getDependencies().getArtifactTypes().named("jar", a -> {
-            // By default all produced artifacts are NAMED
-            a.getAttributes().attribute(MinecraftMappings.ATTRIBUTE, MinecraftMappings.NAMED);
+        project.getDependencies().attributesSchema(schema -> {
+            var attr = schema.attribute(MinecraftMappings.ATTRIBUTE);
+            attr.getDisambiguationRules().add(MappingsDisambiguationRule.class, actionConfiguration -> {
+                actionConfiguration.params(namedMappings);
+            });
         });
 
         obf.createRemappingConfiguration(project.getConfigurations().getByName(JavaPlugin.IMPLEMENTATION_CONFIGURATION_NAME));

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

@@ -1,18 +1,26 @@
 package net.neoforged.moddevgradle.legacyforge.internal;
 
+import javax.inject.Inject;
 import org.gradle.api.attributes.AttributeDisambiguationRule;
 import org.gradle.api.attributes.MultipleCandidatesDetails;
 
 /**
  * This disambiguation rule will prefer NAMED over SRG when both are present.
  */
 class MappingsDisambiguationRule implements AttributeDisambiguationRule<MinecraftMappings> {
+    private final MinecraftMappings named;
+
+    @Inject
+    MappingsDisambiguationRule(MinecraftMappings named) {
+        this.named = named;
+    }
+
     @Override
     public void execute(MultipleCandidatesDetails<MinecraftMappings> details) {
         var consumerValue = details.getConsumerValue();
         if (consumerValue == null) {
-            if (details.getCandidateValues().contains(MinecraftMappings.NAMED)) {
-                details.closestMatch(MinecraftMappings.NAMED);
+            if (details.getCandidateValues().contains(named)) {
+                details.closestMatch(named);
             }
         }
     }

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

@@ -1,19 +1,13 @@
 package net.neoforged.moddevgradle.legacyforge.internal;
 
-import java.util.Locale;
 import org.gradle.api.Named;
 import org.gradle.api.attributes.Attribute;
 import org.jetbrains.annotations.ApiStatus;
 
 @ApiStatus.Internal
-public enum MinecraftMappings implements Named {
-    NAMED,
-    SRG;
+public interface MinecraftMappings extends Named {
+    String NAMED = "named";
+    String SRG = "srg";
 
-    public static final Attribute<MinecraftMappings> ATTRIBUTE = Attribute.of("net.neoforged.moddevgradle.legacy.minecraft_mappings", MinecraftMappings.class);
-
-    @Override
-    public String getName() {
-        return name().toLowerCase(Locale.ROOT);
-    }
+    Attribute<MinecraftMappings> ATTRIBUTE = Attribute.of("net.neoforged.moddevgradle.legacy.minecraft_mappings", MinecraftMappings.class);
 }

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

@@ -3,7 +3,6 @@
 import java.io.IOException;
 import javax.inject.Inject;
 import net.neoforged.moddevgradle.legacyforge.tasks.RemapOperation;
-import org.gradle.api.artifacts.transform.CacheableTransform;
 import org.gradle.api.artifacts.transform.InputArtifact;
 import org.gradle.api.artifacts.transform.InputArtifactDependencies;
 import org.gradle.api.artifacts.transform.TransformAction;
@@ -20,7 +19,6 @@
 import org.gradle.api.tasks.PathSensitivity;
 import org.gradle.process.ExecOperations;
 
-@CacheableTransform
 abstract class RemappingTransform implements TransformAction<RemappingTransform.Parameters> {
     @InputArtifact
     @PathSensitive(PathSensitivity.NONE)

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

@@ -0,0 +1,25 @@
+package net.neoforged.moddevgradle.legacyforge.internal;
+
+import javax.inject.Inject;
+import org.gradle.api.artifacts.ComponentMetadataContext;
+import org.gradle.api.artifacts.ComponentMetadataRule;
+
+public class SrgMappingsRule implements ComponentMetadataRule {
+    private final MinecraftMappings srgMappings;
+
+    @Inject
+    public SrgMappingsRule(MinecraftMappings srgMappings) {
+        this.srgMappings = srgMappings;
+    }
+
+    @Override
+    public void execute(ComponentMetadataContext context) {
+        context.getDetails().allVariants(variant -> {
+            if (variant.getAttributes().contains(MinecraftMappings.ATTRIBUTE)) {
+                return;
+            }
+
+            variant.getAttributes().attribute(MinecraftMappings.ATTRIBUTE, srgMappings);
+        });
+    }
+}

src/test/java/net/neoforged/moddevgradle/AbstractProjectBuilderTest.java

@@ -22,19 +22,25 @@ protected final AbstractListAssert<?, List<? extends String>, String, ObjectAsse
     }
 
     protected final String describeDependency(Dependency dependency) {
+        String result;
         if (dependency instanceof FileCollectionDependency fileCollectionDependency) {
-            return fileCollectionDependency.getFiles().getFiles()
+            result = fileCollectionDependency.getFiles().getFiles()
                     .stream()
                     .map(f -> project.getProjectDir().toPath().relativize(f.toPath()).toString().replace('\\', '/'))
                     .collect(Collectors.joining(";"));
         } else if (dependency instanceof ExternalModuleDependency moduleDependency) {
-            return moduleDependency.getGroup()
+            result = moduleDependency.getGroup()
                     + ":" + moduleDependency.getName()
                     + ":" + moduleDependency.getVersion()
                     + formatCapabilities(moduleDependency);
         } else {
-            return dependency.toString();
+            result = dependency.toString();
         }
+
+        if (dependency.getReason() != null) {
+            result += " (" + dependency.getReason() + ")";
+        }
+        return result;
     }
 
     protected final String formatCapabilities(ExternalModuleDependency moduleDependency) {