Fix local file mod dependencies (#430)

* ModProcessor: Add more descriptive error TR output error message

* Fix flatDir/files/fileTree mod dependencies

* Add clarifying comment

* Use hash as a placeholder version

* ProjectTestTrait: Copy instead of reading and writing text

This allows having jars and other binary data in tests.

* Add integration test for local file dependencies

* Use File.bytes instead of Files.copy

* Use truncated SHA256 instead of murmur3

src/main/java/net/fabricmc/loom/build/ModCompileRemapper.java

@@ -28,11 +28,14 @@ import java.io.File;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.function.Supplier;
 import java.util.zip.ZipFile;
 
+import com.google.common.io.Files;
 import org.gradle.api.Project;
 import org.gradle.api.artifacts.Configuration;
 import org.gradle.api.artifacts.Dependency;
+import org.gradle.api.artifacts.FileCollectionDependency;
 import org.gradle.api.artifacts.ModuleDependency;
 import org.gradle.api.artifacts.ResolvedArtifact;
 import org.gradle.api.artifacts.dsl.DependencyHandler;
@@ -40,9 +43,11 @@ import org.gradle.api.artifacts.query.ArtifactResolutionQuery;
 import org.gradle.api.artifacts.result.ArtifactResult;
 import org.gradle.api.artifacts.result.ComponentArtifactsResult;
 import org.gradle.api.artifacts.result.ResolvedArtifactResult;
+import org.gradle.api.file.FileCollection;
 import org.gradle.api.logging.Logger;
 import org.gradle.jvm.JvmLibrary;
 import org.gradle.language.base.artifact.SourcesArtifact;
+import org.jetbrains.annotations.Nullable;
 
 import net.fabricmc.loom.LoomGradleExtension;
 import net.fabricmc.loom.LoomGradlePlugin;
@@ -51,12 +56,21 @@ import net.fabricmc.loom.configuration.RemappedConfigurationEntry;
 import net.fabricmc.loom.configuration.mods.ModProcessor;
 import net.fabricmc.loom.configuration.processors.dependency.ModDependencyInfo;
 import net.fabricmc.loom.configuration.processors.dependency.RemapData;
+import net.fabricmc.loom.util.Checksum;
 import net.fabricmc.loom.util.Constants;
 import net.fabricmc.loom.util.OperatingSystem;
 import net.fabricmc.loom.util.SourceRemapper;
 
 @SuppressWarnings("UnstableApiUsage")
 public class ModCompileRemapper {
+	// This is a placeholder that is used when the actual group is missing (null or empty).
+	// This can happen when the dependency is a FileCollectionDependency or from a flatDir repository.
+	private static final String MISSING_GROUP = "unspecified";
+
+	private static String replaceIfNullOrEmpty(@Nullable String s, Supplier<String> fallback) {
+		return s == null || s.isEmpty() ? fallback.get() : s;
+	}
+
 	public static void remapDependencies(Project project, String mappingsSuffix, LoomGradleExtension extension, SourceRemapper sourceRemapper) {
 		Logger logger = project.getLogger();
 		DependencyHandler dependencies = project.getDependencies();
@@ -75,29 +89,18 @@ public class ModCompileRemapper {
 				List<ModDependencyInfo> modDependencies = new ArrayList<>();
 
 				for (ResolvedArtifact artifact : sourceConfig.getResolvedConfiguration().getResolvedArtifacts()) {
-					// TODO: This collection doesn't appear to include FileCollection dependencies
+					String group = replaceIfNullOrEmpty(artifact.getModuleVersion().getId().getGroup(), () -> MISSING_GROUP);
-					// Might have to go based on the dependencies, rather than their resolved form?
-					// File dependencies use SelfResolvingDependency, which appears to be handled differently
-					String group = artifact.getModuleVersion().getId().getGroup();
 					String name = artifact.getModuleVersion().getId().getName();
-					String version = artifact.getModuleVersion().getId().getVersion();
+					String version = replaceIfNullOrEmpty(artifact.getModuleVersion().getId().getVersion(), () -> Checksum.truncatedSha256(artifact.getFile()));
 
-					if (!isFabricMod(logger, artifact)) {
+					if (!isFabricMod(logger, artifact.getFile(), artifact.getId())) {
 						addToRegularCompile(project, regularConfig, artifact);
 						continue;
 					}
 
 					ModDependencyInfo info = new ModDependencyInfo(group, name, version, artifact.getClassifier(), artifact.getFile(), remappedConfig, remapData);
-
-					if (refreshDeps) {
-						info.forceRemap();
-					}
-
 					modDependencies.add(info);
 
-					String remappedLog = group + ":" + name + ":" + version + (artifact.getClassifier() == null ? "" : ":" + artifact.getClassifier()) + " (" + mappingsSuffix + ")";
-					project.getLogger().info(":providing " + remappedLog);
-
 					File remappedSources = info.getRemappedOutput("sources");
 
 					if ((!remappedSources.exists() || refreshDeps) && !OperatingSystem.isCIBuild()) {
@@ -109,6 +112,36 @@ public class ModCompileRemapper {
 					}
 				}
 
+				// FileCollectionDependency (files/fileTree) doesn't resolve properly,
+				// so we have to "resolve" it on our own. The naming is "abc.jar" => "unspecified:abc:unspecified".
+				for (FileCollectionDependency dependency : sourceConfig.getAllDependencies().withType(FileCollectionDependency.class)) {
+					String group = replaceIfNullOrEmpty(dependency.getGroup(), () -> MISSING_GROUP);
+					FileCollection files = dependency.getFiles();
+
+					// Create a mod dependency for each file in the file collection
+					for (File artifact : files) {
+						if (!isFabricMod(logger, artifact, artifact.getName())) {
+							dependencies.add(regularConfig.getName(), project.files(artifact));
+							continue;
+						}
+
+						String name = Files.getNameWithoutExtension(artifact.getAbsolutePath());
+						String version = replaceIfNullOrEmpty(dependency.getVersion(), () -> Checksum.truncatedSha256(artifact));
+
+						ModDependencyInfo info = new ModDependencyInfo(group, name, version, null, artifact, remappedConfig, remapData);
+						modDependencies.add(info);
+					}
+				}
+
+				for (ModDependencyInfo info : modDependencies) {
+					if (refreshDeps) {
+						info.forceRemap();
+					}
+
+					String remappedLog = info.getRemappedNotation() + " (" + mappingsSuffix + ")";
+					project.getLogger().info(":providing " + remappedLog);
+				}
+
 				try {
 					ModProcessor.processMods(project, modDependencies);
 				} catch (IOException e) {
@@ -128,12 +161,10 @@ public class ModCompileRemapper {
 	/**
 	 * Checks if an artifact is a fabric mod, according to the presence of a fabric.mod.json.
 	 */
-	private static boolean isFabricMod(Logger logger, ResolvedArtifact artifact) {
+	private static boolean isFabricMod(Logger logger, File artifact, Object id) {
-		File input = artifact.getFile();
+		try (ZipFile zipFile = new ZipFile(artifact)) {
-
-		try (ZipFile zipFile = new ZipFile(input)) {
 			if (zipFile.getEntry("fabric.mod.json") != null) {
-				logger.info("Found Fabric mod in modCompile: {}", artifact.getId());
+				logger.info("Found Fabric mod in modCompile: {}", id);
 				return true;
 			}
 

src/main/java/net/fabricmc/loom/configuration/mods/ModProcessor.java

@@ -175,7 +175,14 @@ public class ModProcessor {
 
 		// Apply this in a second loop as we need to ensure all the inputs are on the classpath before remapping.
 		for (ModDependencyInfo info : remapList) {
-			OutputConsumerPath outputConsumer = new OutputConsumerPath.Builder(info.getRemappedOutput().toPath()).build();
+			OutputConsumerPath outputConsumer;
+
+			try {
+				outputConsumer = new OutputConsumerPath.Builder(info.getRemappedOutput().toPath()).build();
+			} catch (Exception e) {
+				throw new IOException("Could not create output consumer for " + info.getRemappedOutput().getAbsolutePath());
+			}
+
 			outputConsumer.addNonClassFiles(info.getInputFile().toPath());
 			outputConsumerMap.put(info, outputConsumer);
 			String accessWidener = info.getAccessWidener();

src/main/java/net/fabricmc/loom/util/Checksum.java

@@ -26,6 +26,7 @@ package net.fabricmc.loom.util;
 
 import java.io.File;
 import java.io.IOException;
+import java.io.UncheckedIOException;
 
 import com.google.common.hash.HashCode;
 import com.google.common.hash.Hashing;
@@ -60,4 +61,13 @@ public class Checksum {
 			throw new RuntimeException("Failed to get file hash");
 		}
 	}
+
+	public static String truncatedSha256(File file) {
+		try {
+			HashCode hash = Files.asByteSource(file).hash(Hashing.sha256());
+			return hash.toString().substring(0, 12);
+		} catch (IOException e) {
+			throw new UncheckedIOException("Failed to get file hash of " + file, e);
+		}
+	}
 }

src/test/groovy/net/fabricmc/loom/test/integration/LocalFileDependencyTest.groovy

@@ -0,0 +1,50 @@
+/*
+ * This file is part of fabric-loom, licensed under the MIT License (MIT).
+ *
+ * Copyright (c) 2016, 2017, 2018 FabricMC
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in all
+ * copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+package net.fabricmc.loom.test.integration
+
+import net.fabricmc.loom.test.util.ProjectTestTrait
+import spock.lang.Specification
+import spock.lang.Unroll
+
+import static org.gradle.testkit.runner.TaskOutcome.SUCCESS
+
+class LocalFileDependencyTest extends Specification implements ProjectTestTrait {
+	@Override
+	String name() {
+		"localFileDependency"
+	}
+
+	@Unroll
+	def "build (gradle #gradle)"() {
+		when:
+			def result = create("build", gradle)
+		then:
+			result.task(":build").outcome == SUCCESS
+		where:
+			gradle              | _
+			DEFAULT_GRADLE      | _
+			PRE_RELEASE_GRADLE  | _
+	}
+}

src/test/groovy/net/fabricmc/loom/test/util/ProjectTestTrait.groovy

@@ -59,7 +59,7 @@ trait ProjectTestTrait {
 			}
 
 			tempFile.parentFile.mkdirs()
-			tempFile << file.text
+			tempFile.bytes = file.bytes
 		}
 
 		// Disable the CI checks to ensure nothing is skipped

src/test/resources/projects/localFileDependency/build.gradle

@@ -0,0 +1,100 @@
+plugins {
+	id 'fabric-loom'
+	id 'maven-publish'
+}
+
+sourceCompatibility = JavaVersion.VERSION_1_8
+targetCompatibility = JavaVersion.VERSION_1_8
+
+archivesBaseName = project.archives_base_name
+version = project.mod_version
+group = project.maven_group
+
+repositories {
+    // Add repositories to retrieve artifacts from in here.
+    // You should only use this when depending on other mods because
+    // Loom adds the essential maven repositories to download Minecraft and libraries from automatically.
+    // See https://docs.gradle.org/current/userguide/declaring_repositories.html
+    // for more information about repositories.
+	flatDir {
+		dirs "myFlatDir"
+	}
+}
+
+dependencies {
+	// To change the versions see the gradle.properties file
+	minecraft "com.mojang:minecraft:${project.minecraft_version}"
+	mappings "net.fabricmc:yarn:${project.yarn_mappings}:v2"
+	modImplementation "net.fabricmc:fabric-loader:${project.loader_version}"
+
+	// Fabric API. This is technically optional, but you probably want it anyway.
+	modImplementation "net.fabricmc.fabric-api:fabric-api:${project.fabric_version}"
+
+	// Local files
+	modImplementation files("test-data-a.jar", "test-data-b.jar") // multiple files in a bare FileCollection
+	modImplementation fileTree("myFileTree") // an entire file tree
+	modImplementation name: "test-data-e" // a flatDir dependency
+
+	// PSA: Some older mods, compiled on Loom 0.2.1, might have outdated Maven POMs.
+	// You may need to force-disable transitiveness on them.
+}
+
+processResources {
+	inputs.property "version", project.version
+
+	filesMatching("fabric.mod.json") {
+		expand "version": project.version
+	}
+}
+
+tasks.withType(JavaCompile).configureEach {
+	// ensure that the encoding is set to UTF-8, no matter what the system default is
+	// this fixes some edge cases with special characters not displaying correctly
+	// see http://yodaconditions.net/blog/fix-for-java-file-encoding-problems-with-gradle.html
+	// If Javadoc is generated, this must be specified in that task too.
+	it.options.encoding = "UTF-8"
+
+	// The Minecraft launcher currently installs Java 8 for users, so your mod probably wants to target Java 8 too
+	// JDK 9 introduced a new way of specifying this that will make sure no newer classes or methods are used.
+	// We'll use that if it's available, but otherwise we'll use the older option.
+	def targetVersion = 8
+	if (JavaVersion.current().isJava9Compatible()) {
+		 it.options.release = targetVersion
+	}
+}
+
+java {
+	// Loom will automatically attach sourcesJar to a RemapSourcesJar task and to the "build" task
+	// if it is present.
+	// If you remove this line, sources will not be generated.
+	withSourcesJar()
+}
+
+jar {
+	from("LICENSE") {
+		rename { "${it}_${project.archivesBaseName}"}
+	}
+}
+
+// configure the maven publication
+publishing {
+	publications {
+		mavenJava(MavenPublication) {
+			// add all the jars that should be included when publishing to maven
+			artifact(remapJar) {
+				builtBy remapJar
+			}
+			artifact(sourcesJar) {
+				builtBy remapSourcesJar
+			}
+		}
+	}
+    
+    // See https://docs.gradle.org/current/userguide/publishing_maven.html for information on how to set up publishing.
+    repositories {
+        // Add repositories to publish to here.
+        // Notice: This block does NOT have the same function as the block in the top level.
+        // The repositories here will be used for publishing your artifact, not for
+        // retrieving dependencies.
+    }
+}

src/test/resources/projects/localFileDependency/gradle.properties

@@ -0,0 +1,17 @@
+# Done to increase the memory available to gradle.
+org.gradle.jvmargs=-Xmx1G
+
+# Fabric Properties
+	# check these on https://fabricmc.net/use
+	minecraft_version=1.16.5
+	yarn_mappings=1.16.5+build.5
+	loader_version=0.11.2
+
+# Mod Properties
+	mod_version = 1.0.0
+	maven_group = com.example
+	archives_base_name = fabric-example-mod
+
+# Dependencies
+	# currently not on the main fabric site, check on the maven: https://maven.fabricmc.net/net/fabricmc/fabric-api/fabric-api
+	fabric_version=0.31.0+1.16

src/test/resources/projects/localFileDependency/settings.gradle

@@ -0,0 +1,2 @@
+rootProject.name = "fabric-example-mod"
+

src/test/resources/projects/localFileDependency/src/main/java/net/fabricmc/example/ExampleMod.java

@@ -0,0 +1,28 @@
+package net.fabricmc.example;
+
+import net.minecraft.block.Block;
+
+import net.fabricmc.api.ModInitializer;
+import net.fabricmc.loom.LoomTestDataA;
+import net.fabricmc.loom.LoomTestDataB;
+import net.fabricmc.loom.LoomTestDataC;
+import net.fabricmc.loom.LoomTestDataD;
+import net.fabricmc.loom.LoomTestDataE;
+
+public class ExampleMod implements ModInitializer {
+	@Override
+	public void onInitialize() {
+		// This code runs as soon as Minecraft is in a mod-load-ready state.
+		// However, some things (like resources) may still be uninitialized.
+		// Proceed with mild caution.
+
+		System.out.println("Hello Fabric world!");
+
+		// If this doesn't compile, remapping went wrong.
+		Block blockA = LoomTestDataA.referenceToMinecraft();
+		Block blockB = LoomTestDataB.referenceToMinecraft();
+		Block blockC = LoomTestDataC.referenceToMinecraft();
+		Block blockD = LoomTestDataD.referenceToMinecraft();
+		Block blockE = LoomTestDataE.referenceToMinecraft();
+	}
+}

src/test/resources/projects/localFileDependency/src/main/java/net/fabricmc/example/mixin/ExampleMixin.java

@@ -0,0 +1,15 @@
+package net.fabricmc.example.mixin;
+
+import net.minecraft.client.gui.screen.TitleScreen;
+import org.spongepowered.asm.mixin.Mixin;
+import org.spongepowered.asm.mixin.injection.At;
+import org.spongepowered.asm.mixin.injection.Inject;
+import org.spongepowered.asm.mixin.injection.callback.CallbackInfo;
+
+@Mixin(TitleScreen.class)
+public class ExampleMixin {
+	@Inject(at = @At("HEAD"), method = "init()V")
+	private void init(CallbackInfo info) {
+		System.out.println("This line is printed by an example mod mixin!");
+	}
+}

src/test/resources/projects/localFileDependency/src/main/resources/fabric.mod.json

@@ -0,0 +1,36 @@
+{
+  "schemaVersion": 1,
+  "id": "modid",
+  "version": "${version}",
+
+  "name": "Example Mod",
+  "description": "This is an example description! Tell everyone what your mod is about!",
+  "authors": [
+    "Me!"
+  ],
+  "contact": {
+    "homepage": "https://fabricmc.net/",
+    "sources": "https://github.com/FabricMC/fabric-example-mod"
+  },
+
+  "license": "CC0-1.0",
+
+  "environment": "*",
+  "entrypoints": {
+    "main": [
+      "net.fabricmc.example.ExampleMod"
+    ]
+  },
+  "mixins": [
+    "modid.mixins.json"
+  ],
+
+  "depends": {
+    "fabricloader": ">=0.7.4",
+    "fabric": "*",
+    "minecraft": "1.16.x"
+  },
+  "suggests": {
+    "another-mod": "*"
+  }
+}

src/test/resources/projects/localFileDependency/src/main/resources/modid.mixins.json

@@ -0,0 +1,14 @@
+{
+  "required": true,
+  "minVersion": "0.8",
+  "package": "net.fabricmc.example.mixin",
+  "compatibilityLevel": "JAVA_8",
+  "mixins": [
+  ],
+  "client": [
+    "ExampleMixin"
+  ],
+  "injectors": {
+    "defaultRequire": 1
+  }
+}