Redo how the mod configurations in legacy MDG are remapped, fixing source downloads (#249)

Author: Technici4n

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

@@ -3,9 +3,7 @@
 import java.util.List;
 import java.util.Objects;
 import javax.inject.Inject;
-import net.neoforged.moddevgradle.legacyforge.internal.LegacyForgeModDevPlugin;
 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;
@@ -16,7 +14,6 @@
 import org.gradle.api.artifacts.ExternalModuleDependency;
 import org.gradle.api.artifacts.FileCollectionDependency;
 import org.gradle.api.artifacts.ProjectDependency;
-import org.gradle.api.artifacts.type.ArtifactTypeDefinition;
 import org.gradle.api.attributes.Attribute;
 import org.gradle.api.component.AdhocComponentWithVariants;
 import org.gradle.api.component.ConfigurationVariantDetails;
@@ -36,7 +33,6 @@ public abstract class ObfuscationExtension {
     private final FileCollection extraMixinMappings;
 
     private final MinecraftMappings namedMappings;
-    private final MinecraftMappings srgMappings;
 
     @Inject
     public ObfuscationExtension(Project project,
@@ -49,7 +45,6 @@ public ObfuscationExtension(Project project,
         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) {
@@ -139,9 +134,12 @@ public TaskProvider<RemapJar> reobfuscate(TaskProvider<? extends AbstractArchive
             reobfConfig.setDescription("The artifacts remapped to intermediate (SRG) Minecraft names for use in a production environment");
             reobfConfig.getArtifacts().clear(); // If this is called multiple times...
             for (var attribute : config.getAttributes().keySet()) {
-                copyAttribute(project, attribute, config, reobfConfig);
+                // Don't copy the mappings attribute because we don't want to leak it in the published metadata
+                // and there is no way to unset it later.
+                if (attribute != MinecraftMappings.ATTRIBUTE) {
+                    copyAttribute(project, attribute, config, reobfConfig);
+                }
             }
-            reobfConfig.getAttributes().attribute(MinecraftMappings.ATTRIBUTE, srgMappings);
             project.getArtifacts().add(reobfConfig.getName(), reobf);
 
             // Publish the reobf configuration instead of the original one to Maven
@@ -166,7 +164,7 @@ 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.setCanBeConsumed(false);
-            spec.setCanBeResolved(true);
+            spec.setCanBeResolved(false);
             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
@@ -175,19 +173,12 @@ 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, namedMappings));
+                        });
+                    });
                     externalModuleDependency.setTransitive(false);
-
-                    // artifact dependencies need to be forced to be from our custom artifact type to be able
-                    // to inherit the mapping attribute (and be remapped). Module metadata doesn't apply here.
-                    for (var artifact : externalModuleDependency.getArtifacts()) {
-                        artifact.setType(LegacyForgeModDevPlugin.ARTIFACT_TYPE_SRG_JAR);
-                    }
-
-                    // 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 -> {
@@ -204,16 +195,7 @@ public Configuration createRemappingConfiguration(Configuration parent) {
                 }
             }));
         });
-
-        var remappedDep = project.getDependencyFactory().create(
-                remappingConfig.getIncoming().artifactView(view -> {
-                    view.attributes(a -> a.attribute(MinecraftMappings.ATTRIBUTE, namedMappings));
-                    // Forcing resolution to JAR here allows our SRG_JAR artifact transform to work
-                    view.attributes(a -> a.attribute(ArtifactTypeDefinition.ARTIFACT_TYPE_ATTRIBUTE, ArtifactTypeDefinition.JAR_TYPE));
-                }).getFiles());
-        remappedDep.because("Remapped mods from " + remappingConfig.getName());
-
-        parent.getDependencies().add(remappedDep);
+        parent.extendsFrom(remappingConfig);
 
         return remappingConfig;
     }

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

@@ -44,7 +44,6 @@ public class LegacyForgeModDevPlugin implements Plugin<Project> {
 
     public static final String CONFIGURATION_TOOL_ART = "autoRenamingToolRuntime";
     public static final String CONFIGURATION_TOOL_INSTALLERTOOLS = "installerToolsRuntime";
-    public static final String ARTIFACT_TYPE_SRG_JAR = "srgJar";
 
     private final MinecraftMappings namedMappings;
     private final MinecraftMappings srgMappings;
@@ -221,26 +220,14 @@ public void enable(Project project, LegacyForgeModdingSettings settings, LegacyF
                     .attribute(MinecraftMappings.ATTRIBUTE, namedMappings)
                     .attribute(ArtifactTypeDefinition.ARTIFACT_TYPE_ATTRIBUTE, ArtifactTypeDefinition.JAR_TYPE);
         });
-        // This second copy of the transform is used for remapping artifact dependencies (i.e. on classifiers)
-        // that circumvent the variant system.
-        project.getDependencies().registerTransform(RemappingTransform.class, params -> {
-            params.parameters(parameters -> {
-                obf.configureSrgToNamedOperation(parameters.getRemapOperation());
-                parameters.getMinecraftDependencies().from(remapDeps);
-            });
-            params.getFrom()
-                    .attribute(MinecraftMappings.ATTRIBUTE, srgMappings)
-                    .attribute(ArtifactTypeDefinition.ARTIFACT_TYPE_ATTRIBUTE, ARTIFACT_TYPE_SRG_JAR);
-            params.getTo()
-                    .attribute(MinecraftMappings.ATTRIBUTE, namedMappings)
-                    .attribute(ArtifactTypeDefinition.ARTIFACT_TYPE_ATTRIBUTE, ArtifactTypeDefinition.JAR_TYPE);
-        });
     }
 
     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.
+        // For cross-project dependencies, both the named (with the named attribute) and obfuscated (with no attribute)
+        // variants are available. Requesting the srg attribute seemingly excludes the named variant from the selection.
         var sourceSets = ExtensionUtils.getSourceSets(project);
         sourceSets.all(sourceSet -> {
             var configurationName = sourceSet.getTaskName(null, "jarJar");
@@ -257,15 +244,18 @@ private void configureDependencyRemapping(Project project, ObfuscationExtension
 
         project.getDependencies().attributesSchema(schema -> {
             var attr = schema.attribute(MinecraftMappings.ATTRIBUTE);
-            attr.getDisambiguationRules().add(MappingsDisambiguationRule.class, actionConfiguration -> {
-                actionConfiguration.params(namedMappings);
+            // Add a disambiguation rule that will prefer named variants.
+            // This is needed for cross-project dependencies: in that setting both the named (with the named attribute)
+            // and obfuscated (with no attribute) variants are available, and we want to choose named by default.
+            attr.getDisambiguationRules().add(MappingsDisambiguationRule.class, config -> {
+                config.params(namedMappings);
             });
         });
-
-        // custom artifact type used to force remapping of artifact dependencies (which circumvent variant selection)
-        project.getDependencies().getArtifactTypes().create(ARTIFACT_TYPE_SRG_JAR, artifactType -> {
-            artifactType.getAttributes().attribute(MinecraftMappings.ATTRIBUTE, srgMappings);
+        // Give every single jar the srg mappings attribute so it can be force-remapped by requesting named
+        project.getDependencies().getArtifactTypes().named(ArtifactTypeDefinition.JAR_TYPE, type -> {
+            type.getAttributes().attribute(MinecraftMappings.ATTRIBUTE, srgMappings);
         });
+
         obf.createRemappingConfiguration(project.getConfigurations().getByName(JavaPlugin.IMPLEMENTATION_CONFIGURATION_NAME));
         obf.createRemappingConfiguration(project.getConfigurations().getByName(JavaPlugin.RUNTIME_ONLY_CONFIGURATION_NAME));
         obf.createRemappingConfiguration(project.getConfigurations().getByName(JavaPlugin.COMPILE_ONLY_CONFIGURATION_NAME));

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

@@ -9,5 +9,5 @@ public interface MinecraftMappings extends Named {
     String NAMED = "named";
     String SRG = "srg";
 
-    Attribute<MinecraftMappings> ATTRIBUTE = Attribute.of("net.neoforged.moddevgradle.legacy.minecraft_mappings", MinecraftMappings.class);
+    Attribute<MinecraftMappings> ATTRIBUTE = Attribute.of("net.neoforged.moddevgradle.legacy.minecraft_mappings.v2", MinecraftMappings.class);
 }