Fixes for Mojang mappings handling (#264)

* Only resolve SelfResolvingDependencies once

* mojmap: Improve styling, move to a better group + artifact id

3 files changed, 26 insertions, 13 deletions

diff --git a/src/main/java/net/fabricmc/loom/task/MigrateMappingsTask.java b/src/main/java/net/fabricmc/loom/task/MigrateMappingsTask.java
index 4ec85e15..ad23b398 100644
--- a/src/main/java/net/fabricmc/loom/task/MigrateMappingsTask.java
+++ b/src/main/java/net/fabricmc/loom/task/MigrateMappingsTask.java
@@ -118,7 +118,7 @@ public class MigrateMappingsTask extends AbstractLoomTask {
 		Set<File> files;
 
 		try {
-			if (mappings.startsWith("net.mojang.minecraft:mappings:")) {
+			if (mappings.startsWith(MojangMappingsDependency.GROUP + ':' + MojangMappingsDependency.MODULE + ':')) {
 				if (!mappings.endsWith(":" + project.getExtensions().getByType(LoomGradleExtension.class).getMinecraftProvider().getMinecraftVersion())) {
 					throw new UnsupportedOperationException("Migrating Mojang mappings is currently only supported for the specified minecraft version");
 				}
diff --git a/src/main/java/net/fabricmc/loom/util/DependencyProvider.java b/src/main/java/net/fabricmc/loom/util/DependencyProvider.java
index 9711669c..d1b19e75 100644
--- a/src/main/java/net/fabricmc/loom/util/DependencyProvider.java
+++ b/src/main/java/net/fabricmc/loom/util/DependencyProvider.java
@@ -175,12 +175,14 @@ public abstract class DependencyProvider {
 
 	public static class FileDependencyInfo extends DependencyInfo {
 		protected final Map<String, File> classifierToFile = new HashMap<>();
+		protected final Set<File> resolvedFiles;
 		protected final String group, name, version;
 
 		FileDependencyInfo(Project project, SelfResolvingDependency dependency, Configuration configuration) {
 			super(project, dependency, configuration);
 
 			Set<File> files = dependency.resolve();
+			this.resolvedFiles = files;
 			switch (files.size()) {
 			case 0: //Don't think Gradle would ever let you do this
 				throw new IllegalStateException("Empty dependency?");
@@ -263,5 +265,10 @@ public abstract class DependencyProvider {
 		public String getResolvedDepString() {
 			return getDepString();
 		}
+
+		@Override
+		public Set<File> resolve() {
+			return this.resolvedFiles;
+		}
 	}
 }
diff --git a/src/main/java/net/fabricmc/loom/util/mappings/MojangMappingsDependency.java b/src/main/java/net/fabricmc/loom/util/mappings/MojangMappingsDependency.java
index 6be883b1..b84d8d2b 100644
--- a/src/main/java/net/fabricmc/loom/util/mappings/MojangMappingsDependency.java
+++ b/src/main/java/net/fabricmc/loom/util/mappings/MojangMappingsDependency.java
@@ -60,6 +60,12 @@ import net.fabricmc.lorenztiny.TinyMappingsReader;
 import net.fabricmc.mapping.tree.TinyMappingFactory;
 
 public class MojangMappingsDependency implements SelfResolvingDependency {
+	public static final String GROUP = "net.minecraft";
+	public static final String MODULE = "mappings";
+	// Keys in dependency manifest
+	private static final String MANIFEST_CLIENT_MAPPINGS = "client_mappings";
+	private static final String MANIFEST_SERVER_MAPPINGS = "server_mappings";
+
 	private final Project project;
 	private final LoomGradleExtension extension;
 
@@ -71,9 +77,9 @@ public class MojangMappingsDependency implements SelfResolvingDependency {
 	@Override
 	public Set<File> resolve() {
 		Path mappingsDir = extension.getMappingsProvider().getMappingsDir();
-		Path mappingsFile = mappingsDir.resolve(String.format("net.mojang.minecraft-mappings-%s.tiny", getVersion()));
-		Path clientMappings = mappingsDir.resolve(String.format("net.mojang.minecraft.mappings-%s-client.map", getVersion()));
-		Path serverMappings = mappingsDir.resolve(String.format("net.mojang.minecraft.mappings-%s-server.map", getVersion()));
+		Path mappingsFile = mappingsDir.resolve(String.format("%s.%s-%s.tiny", GROUP, MODULE, getVersion()));
+		Path clientMappings = mappingsDir.resolve(String.format("%s.%s-%s-client.map", GROUP, MODULE, getVersion()));
+		Path serverMappings = mappingsDir.resolve(String.format("%s.%s-%s-server.map", GROUP, MODULE, getVersion()));
 
 		if (!Files.exists(mappingsFile) || project.getGradle().getStartParameter().isRefreshDependencies()) {
 			MappingSet mappingSet;
@@ -95,17 +101,17 @@ public class MojangMappingsDependency implements SelfResolvingDependency {
 		}
 
 		try (BufferedReader clientBufferedReader = Files.newBufferedReader(clientMappings, StandardCharsets.UTF_8)) {
-			project.getLogger().warn("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~");
+			project.getLogger().warn("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~");
 			project.getLogger().warn("Using of the official minecraft mappings is at your own risk!");
 			project.getLogger().warn("Please make sure to read and understand the following license:");
-			project.getLogger().warn("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~");
+			project.getLogger().warn("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~");
 			String line;
 
 			while ((line = clientBufferedReader.readLine()).startsWith("#")) {
 				project.getLogger().warn(line);
 			}
 
-			project.getLogger().warn("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~");
+			project.getLogger().warn("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~");
 		} catch (IOException e) {
 			throw new RuntimeException("Failed to read client mappings", e);
 		}
@@ -116,12 +122,12 @@ public class MojangMappingsDependency implements SelfResolvingDependency {
 	private MappingSet getMappingsSet(Path clientMappings, Path serverMappings) throws IOException {
 		MinecraftVersionInfo versionInfo = extension.getMinecraftProvider().getVersionInfo();
 
-		if (versionInfo.downloads.get("client_mappings") == null) {
+		if (versionInfo.downloads.get(MANIFEST_CLIENT_MAPPINGS) == null) {
 			throw new RuntimeException("Failed to find official mojang mappings for " + getVersion());
 		}
 
-		String clientMappingsUrl = versionInfo.downloads.get("client_mappings").url;
-		String serverMappingsUrl = versionInfo.downloads.get("server_mappings").url;
+		String clientMappingsUrl = versionInfo.downloads.get(MANIFEST_CLIENT_MAPPINGS).url;
+		String serverMappingsUrl = versionInfo.downloads.get(MANIFEST_SERVER_MAPPINGS).url;
 
 		DownloadUtil.downloadIfChanged(new URL(clientMappingsUrl), clientMappings.toFile(), project.getLogger());
 		DownloadUtil.downloadIfChanged(new URL(serverMappingsUrl), serverMappings.toFile(), project.getLogger());
@@ -150,7 +156,7 @@ public class MojangMappingsDependency implements SelfResolvingDependency {
 		iterateClasses(intermediaryToOfficial, inputMappings -> {
 			officialToNamed.getClassMapping(inputMappings.getFullDeobfuscatedName())
 					.ifPresent(namedClass -> {
-						ClassMapping<?, ?> mojangClassMapping = intermediaryToMojang.getOrCreateClassMapping(inputMappings .getFullObfuscatedName())
+						ClassMapping<?, ?> mojangClassMapping = intermediaryToMojang.getOrCreateClassMapping(inputMappings.getFullObfuscatedName())
 								.setDeobfuscatedName(namedClass.getFullDeobfuscatedName());
 
 						for (FieldMapping fieldMapping : inputMappings .getFieldMappings()) {
@@ -186,12 +192,12 @@ public class MojangMappingsDependency implements SelfResolvingDependency {
 
 	@Override
 	public String getGroup() {
-		return "net.mojang.minecraft";
+		return GROUP;
 	}
 
 	@Override
 	public String getName() {
-		return "mappings";
+		return MODULE;
 	}
 
 	@Override