Use hash of cache value as the JAR name for processed Minecraft JARs (#…

…944) * Use hash of cache value as the JAR name for processed Minecraft JARs In basic testing, this cuts down the number of JARs to just one provided the same AWs are used on all subprojects * Fix checkstyle * Remove redundant code * Fix mod javadoc caching, and fixup tests. * Hopefully fix ModJavadocTest on windows. --------- Co-authored-by: modmuss <[email protected]>
FabricMC · Sep 25, 2023 · b7c8013 · b7c8013
1 parent bd09af1
commit b7c8013
Show file tree

Hide file tree

Showing 6 changed files with 68 additions and 69 deletions.
diff --git a/src/main/java/net/fabricmc/loom/configuration/processors/MinecraftJarProcessorManager.java b/src/main/java/net/fabricmc/loom/configuration/processors/MinecraftJarProcessorManager.java
@@ -1,7 +1,7 @@
 /*
  * This file is part of fabric-loom, licensed under the MIT License (MIT).
  *
- * Copyright (c) 2022 FabricMC
+ * Copyright (c) 2022-2023 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
@@ -25,7 +25,6 @@
 package net.fabricmc.loom.configuration.processors;
 
 import java.io.IOException;
-import java.io.UncheckedIOException;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
@@ -47,11 +46,10 @@
 import net.fabricmc.loom.api.processor.MinecraftJarProcessor;
 import net.fabricmc.loom.api.processor.ProcessorContext;
 import net.fabricmc.loom.api.processor.SpecContext;
-import net.fabricmc.loom.util.ZipUtils;
+import net.fabricmc.loom.util.Checksum;
 import net.fabricmc.mappingio.tree.MemoryMappingTree;
 
 public final class MinecraftJarProcessorManager {
-	private static final String CACHE_VALUE_FILE_PATH = "META-INF/Loom-Jar-Processor-Cache";
 	private static final Logger LOGGER = LoggerFactory.getLogger(MinecraftJarProcessorManager.class);
 
 	private final List<ProcessorEntry<?>> jarProcessors;
@@ -102,17 +100,22 @@ private String getCacheValue() {
 	}
 
 	private String getDebugString() {
-		final StringJoiner sj = new StringJoiner("\n");
+		final var sj = new StringJoiner("\n");
 
 		for (ProcessorEntry<?> jarProcessor : jarProcessors) {
 			sj.add(jarProcessor.name() + ":");
 			sj.add("\tHash: " + jarProcessor.hashCode());
-			sj.add("\tStr: " + jarProcessor.toString());
+			sj.add("\tStr: " + jarProcessor.cacheValue());
 		}
 
 		return sj.toString();
 	}
 
+	public String getJarHash() {
+		//fabric-loom:mod-javadoc:-1289977000
+		return Checksum.sha1Hex(getCacheValue().getBytes(StandardCharsets.UTF_8)).substring(0, 10);
+	}
+
 	public boolean requiresProcessingJar(Path jar) {
 		Objects.requireNonNull(jar);
 
@@ -121,32 +124,7 @@ public boolean requiresProcessingJar(Path jar) {
 			return true;
 		}
 
-		byte[] existingCache;
-
-		try {
-			existingCache = ZipUtils.unpackNullable(jar, CACHE_VALUE_FILE_PATH);
-		} catch (IOException e) {
-			throw new UncheckedIOException("Failed to unpack jar: " + jar, e);
-		}
-
-		if (existingCache == null) {
-			LOGGER.info("{} does not contain a processor cache value, regenerating", jar);
-			return true;
-		}
-
-		final String existingCacheValue = new String(existingCache, StandardCharsets.UTF_8);
-		final String expectedCacheValue = getCacheValue();
-		final boolean matches = existingCacheValue.equals(expectedCacheValue);
-
-		if (!matches) {
-			LOGGER.info("{} has an invalid cache, got {} expected {}", jar, existingCacheValue, expectedCacheValue);
-
-			if (LOGGER.isInfoEnabled()) {
-				LOGGER.info("Expected state: {}", getDebugString());
-			}
-		}
-
-		return !matches;
+		return false;
 	}
 
 	public void processJar(Path jar, ProcessorContext context) throws IOException {
@@ -157,8 +135,6 @@ public void processJar(Path jar, ProcessorContext context) throws IOException {
 				throw new IOException("Failed to process jar when running jar processor: %s".formatted(entry.name()), e);
 			}
 		}
-
-		ZipUtils.add(jar, CACHE_VALUE_FILE_PATH, getCacheValue());
 	}
 
 	public boolean processMappings(MemoryMappingTree mappings, MappingProcessorContext context) {

diff --git a/src/main/java/net/fabricmc/loom/configuration/processors/ModJavadocProcessor.java b/src/main/java/net/fabricmc/loom/configuration/processors/ModJavadocProcessor.java
@@ -32,7 +32,9 @@
 import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.Comparator;
 import java.util.List;
+import java.util.Objects;
 
 import javax.inject.Inject;
 
@@ -45,6 +47,7 @@
 import net.fabricmc.loom.api.processor.MinecraftJarProcessor;
 import net.fabricmc.loom.api.processor.ProcessorContext;
 import net.fabricmc.loom.api.processor.SpecContext;
+import net.fabricmc.loom.util.Checksum;
 import net.fabricmc.loom.util.Constants;
 import net.fabricmc.loom.util.fmj.FabricModJson;
 import net.fabricmc.mappingio.MappingReader;
@@ -82,6 +85,7 @@ public String getName() {
 			return null;
 		}
 
+		javadocs.sort(Comparator.comparing(ModJavadoc::modId));
 		return new Spec(Collections.unmodifiableList(javadocs));
 	}
 
@@ -104,7 +108,7 @@ public void processJar(Path jar, Spec spec, ProcessorContext context) {
 		};
 	}
 
-	public record ModJavadoc(String modId, MemoryMappingTree mappingTree) {
+	public record ModJavadoc(String modId, MemoryMappingTree mappingTree, String mappingsHash) {
 		@Nullable
 		public static ModJavadoc create(FabricModJson fabricModJson) {
 			final String modId = fabricModJson.getId();
@@ -116,9 +120,11 @@ public static ModJavadoc create(FabricModJson fabricModJson) {
 
 			final String javaDocPath = customElement.getAsString();
 			final MemoryMappingTree mappings = new MemoryMappingTree();
+			final String mappingsHash;
 
 			try {
 				final byte[] data = fabricModJson.getSource().read(javaDocPath);
+				mappingsHash = Checksum.sha1Hex(data);
 
 				try (Reader reader = new InputStreamReader(new ByteArrayInputStream(data))) {
 					MappingReader.read(reader, mappings);
@@ -135,7 +141,7 @@ public static ModJavadoc create(FabricModJson fabricModJson) {
 				throw new IllegalStateException("Javadoc provided by mod (%s) must not contain any dst names".formatted(modId));
 			}
 
-			return new ModJavadoc(modId, mappings);
+			return new ModJavadoc(modId, mappings, mappingsHash);
 		}
 
 		public void apply(MemoryMappingTree target) {
@@ -196,5 +202,16 @@ private <T extends MappingTree.ElementMapping> void applyComment(T source, T tar
 			targetComment += sourceComment;
 			target.setComment(targetComment);
 		}
+
+		// Must override as not to include MemoryMappingTree
+		@Override
+		public int hashCode() {
+			return Objects.hash(modId, mappingsHash);
+		}
+
+		@Override
+		public String toString() {
+			return "ModJavadoc{modId='%s', mappingsHash='%s'}".formatted(modId, mappingsHash);
+		}
 	}
 }
diff --git a/...bricmc/loom/configuration/providers/minecraft/mapped/ProcessedNamedMinecraftProvider.java b/...bricmc/loom/configuration/providers/minecraft/mapped/ProcessedNamedMinecraftProvider.java
@@ -1,7 +1,7 @@
 /*
  * This file is part of fabric-loom, licensed under the MIT License (MIT).
  *
- * Copyright (c) 2021-2022 FabricMC
+ * Copyright (c) 2021-2023 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
@@ -28,14 +28,11 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.List;
-import java.util.Locale;
 import java.util.Map;
 import java.util.Objects;
 import java.util.function.Function;
 import java.util.stream.Collectors;
 
-import org.gradle.api.Project;
-
 import net.fabricmc.loom.configuration.ConfigContext;
 import net.fabricmc.loom.configuration.mods.dependency.LocalMavenHelper;
 import net.fabricmc.loom.configuration.processors.MinecraftJarProcessorManager;
@@ -129,14 +126,8 @@ private void deleteSimilarJars(Path jar) throws IOException {
 
 	@Override
 	protected String getName(String name) {
-		final Project project = getProject();
-
-		if (project.getRootProject() == project) {
-			return "minecraft-%s-project-root".formatted(name).toLowerCase(Locale.ROOT);
-		}
-
-		final String projectPath = project.getPath().replace(':', '@');
-		return "minecraft-%s-project-%s".formatted(name, projectPath).toLowerCase(Locale.ROOT);
+		// Hash the cache value so that we don't have to process the same JAR multiple times for many projects
+		return "minecraft-%s-%s".formatted(name, jarProcessorManager.getJarHash());
 	}
 
 	@Override

diff --git a/src/main/java/net/fabricmc/loom/util/Checksum.java b/src/main/java/net/fabricmc/loom/util/Checksum.java
@@ -33,6 +33,7 @@
 import com.google.common.hash.HashCode;
 import com.google.common.hash.Hashing;
 import com.google.common.io.BaseEncoding;
+import com.google.common.io.ByteSource;
 import com.google.common.io.Files;
 import org.gradle.api.Project;
 import org.gradle.api.logging.Logger;
@@ -71,6 +72,15 @@ public static String sha1Hex(Path path) throws IOException {
 		return toHex(hash.asBytes());
 	}
 
+	public static String sha1Hex(byte[] input) {
+		try {
+			HashCode hash = ByteSource.wrap(input).hash(Hashing.sha1());
+			return toHex(hash.asBytes());
+		} catch (IOException e) {
+			throw new UncheckedIOException("Failed to hash", e);
+		}
+	}
+
 	public static String truncatedSha256(File file) {
 		try {
 			HashCode hash = Files.asByteSource(file).hash(Hashing.sha256());

diff --git a/...test/groovy/net/fabricmc/loom/test/unit/processor/MinecraftJarProcessorManagerTest.groovy b/...test/groovy/net/fabricmc/loom/test/unit/processor/MinecraftJarProcessorManagerTest.groovy
@@ -26,46 +26,37 @@ package net.fabricmc.loom.test.unit.processor
 
 import spock.lang.Specification
 
-import net.fabricmc.loom.api.processor.ProcessorContext
 import net.fabricmc.loom.api.processor.SpecContext
 import net.fabricmc.loom.configuration.processors.MinecraftJarProcessorManager
 import net.fabricmc.loom.test.util.processor.TestMinecraftJarProcessor
 
-import static net.fabricmc.loom.test.util.ZipTestUtils.createZip
-
 class MinecraftJarProcessorManagerTest extends Specification {
-	def "Does not require re-processing"() {
-		given:
+	def "Cache value matches"() {
+		when:
 		def specContext = Mock(SpecContext)
-		def processorContext = Mock(ProcessorContext)
 
 		def processor1 = new TestMinecraftJarProcessor(input: "Test1")
 		def processor2 = new TestMinecraftJarProcessor(input: "Test2")
-		def manager = MinecraftJarProcessorManager.create([processor1, processor2], specContext)
-
-		when:
-		def jar = createZip(["fabric.mod.json": "{}"])
-		manager.processJar(jar, processorContext)
+		def manager1 = MinecraftJarProcessorManager.create([processor1, processor2], specContext)
+		def manager2 = MinecraftJarProcessorManager.create([processor1, processor2], specContext)
 
 		then:
-		!manager.requiresProcessingJar(jar)
+		manager1.jarHash == manager2.jarHash
+		manager1.jarHash == "eb6faafa72"
 	}
 
-	def "Requires re-processing"() {
-		given:
+	def "Cache value does not match"() {
+		when:
 		def specContext = Mock(SpecContext)
-		def processorContext = Mock(ProcessorContext)
 
 		def processor1 = new TestMinecraftJarProcessor(input: "Test1")
 		def processor2 = new TestMinecraftJarProcessor(input: "Test2")
 		def manager1 = MinecraftJarProcessorManager.create([processor1], specContext)
 		def manager2 = MinecraftJarProcessorManager.create([processor1, processor2], specContext)
 
-		when:
-		def jar = createZip(["fabric.mod.json": "{}"])
-		manager1.processJar(jar, processorContext)
-
 		then:
-		manager2.requiresProcessingJar(jar)
+		manager1.jarHash != manager2.jarHash
+		manager1.jarHash == "a714eb2de6"
+		manager2.jarHash == "eb6faafa72"
 	}
 }
diff --git a/src/test/groovy/net/fabricmc/loom/test/util/GradleProjectTestTrait.groovy b/src/test/groovy/net/fabricmc/loom/test/util/GradleProjectTestTrait.groovy
@@ -24,6 +24,7 @@
 
 package net.fabricmc.loom.test.util
 
+import groovy.io.FileType
 import groovy.transform.Immutable
 import org.apache.commons.io.FileUtils
 import org.gradle.testkit.runner.BuildResult
@@ -241,7 +242,20 @@ trait GradleProjectTestTrait {
 		}
 
 		File getGeneratedLocalSources(String mappings) {
-			return new File(getProjectDir(), ".gradle/loom-cache/minecraftMaven/net/minecraft/minecraft-merged-project-root/${mappings}/minecraft-merged-project-root-${mappings}-sources.jar")
+			File file
+			getProjectDir().traverse(type: FileType.FILES) {
+				if (it.name.startsWith("minecraft-merged-")
+						&& it.name.contains(mappings)
+						&& it.name.endsWith("-sources.jar")) {
+					file = it
+				}
+			}
+
+			if (file == null) {
+				throw new FileNotFoundException()
+			}
+
+			return file
 		}
 
 		void buildSrc(String name) {