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 <modmuss50@gmail.com>
This commit is contained in:
embeddedt
2023-09-25 17:44:31 -04:00
committed by GitHub
parent bd09af1783
commit b7c80133ce
6 changed files with 68 additions and 69 deletions

View File

@@ -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.MappingProcessorContext;
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 @@ public final class MinecraftJarProcessorManager {
}
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 final class MinecraftJarProcessorManager {
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 final class MinecraftJarProcessorManager {
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) {

View File

@@ -32,7 +32,9 @@ import java.io.UncheckedIOException;
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.mappings.layered.MappingsNamespace;
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 abstract class ModJavadocProcessor implements MinecraftJarProcessor<ModJa
return null;
}
javadocs.sort(Comparator.comparing(ModJavadoc::modId));
return new Spec(Collections.unmodifiableList(javadocs));
}
@@ -104,7 +108,7 @@ public abstract class ModJavadocProcessor implements MinecraftJarProcessor<ModJa
};
}
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 abstract class ModJavadocProcessor implements MinecraftJarProcessor<ModJa
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 abstract class ModJavadocProcessor implements MinecraftJarProcessor<ModJa
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 @@ public abstract class ModJavadocProcessor implements MinecraftJarProcessor<ModJa
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);
}
}
}

View File

@@ -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.io.IOException;
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 @@ public abstract class ProcessedNamedMinecraftProvider<M extends MinecraftProvide
@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

View File

@@ -33,6 +33,7 @@ import java.nio.file.Path;
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 class Checksum {
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());

View File

@@ -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"
}
}

View File

@@ -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) {