From 92eed264ab07f240b31159bea2819886908ae6ab Mon Sep 17 00:00:00 2001 From: modmuss50 Date: Tue, 2 May 2023 09:21:51 +0100 Subject: [PATCH 1/3] Downloader improvements (#880) * Fixes Integration tests locally often fail with download errors #878 * Hopefully fixes Loom is very slow to download files, often hanging for 10+ minutes #877 * "Forcing downloading" error should show much less often. * Progress loggers get closed when download fails. * Download assets uses a maxmium of 10 threads. * Update tests to run against 8.3 nightlys. * Fix windows arm tests as Mojang back-ported this change to all 1.19 versions. --- .../loom/task/DownloadAssetsTask.java | 2 +- .../fabricmc/loom/util/download/Download.java | 39 ++++++++----------- .../loom/test/LoomTestConstants.groovy | 2 +- .../unit/library/LibraryContextTest.groovy | 2 +- .../ArmNativesLibraryProcessorTest.groovy | 19 +-------- 5 files changed, 21 insertions(+), 43 deletions(-) diff --git a/src/main/java/net/fabricmc/loom/task/DownloadAssetsTask.java b/src/main/java/net/fabricmc/loom/task/DownloadAssetsTask.java index e19cce09..b8abca18 100644 --- a/src/main/java/net/fabricmc/loom/task/DownloadAssetsTask.java +++ b/src/main/java/net/fabricmc/loom/task/DownloadAssetsTask.java @@ -71,7 +71,7 @@ public abstract class DownloadAssetsTask extends AbstractLoomTask { getAssetsDirectory().set(assetsDir); getAssetsHash().set(versionInfo.assetIndex().sha1()); - getDownloadThreads().convention(Runtime.getRuntime().availableProcessors()); + getDownloadThreads().convention(Math.min(Runtime.getRuntime().availableProcessors(), 10)); getMinecraftVersion().set(versionInfo.id()); getMinecraftVersion().finalizeValue(); diff --git a/src/main/java/net/fabricmc/loom/util/download/Download.java b/src/main/java/net/fabricmc/loom/util/download/Download.java index efbde99e..a21d4ce4 100644 --- a/src/main/java/net/fabricmc/loom/util/download/Download.java +++ b/src/main/java/net/fabricmc/loom/util/download/Download.java @@ -62,6 +62,10 @@ import net.fabricmc.loom.util.Checksum; public final class Download { private static final String E_TAG = "ETag"; private static final Logger LOGGER = LoggerFactory.getLogger(Download.class); + private static final HttpClient HTTP_CLIENT = HttpClient.newBuilder() + .followRedirects(HttpClient.Redirect.ALWAYS) + .proxy(ProxySelector.getDefault()) + .build(); public static DownloadBuilder create(String url) throws URISyntaxException { return DownloadBuilder.create(url); @@ -89,17 +93,6 @@ public final class Download { this.downloadAttempt = downloadAttempt; } - private HttpClient getHttpClient() throws DownloadException { - if (offline) { - throw error("Unable to download %s in offline mode", this.url); - } - - return HttpClient.newBuilder() - .followRedirects(HttpClient.Redirect.ALWAYS) - .proxy(ProxySelector.getDefault()) - .build(); - } - private HttpRequest getRequest() { return HttpRequest.newBuilder(url) .version(httpVersion) @@ -116,10 +109,14 @@ public final class Download { } private HttpResponse send(HttpRequest httpRequest, HttpResponse.BodyHandler bodyHandler) throws DownloadException { + if (offline) { + throw error("Unable to download %s in offline mode", this.url); + } + progressListener.onStart(); try { - return getHttpClient().send(httpRequest, bodyHandler); + return HTTP_CLIENT.send(httpRequest, bodyHandler); } catch (IOException | InterruptedException e) { throw error(e, "Failed to download (%s)", url); } @@ -131,6 +128,7 @@ public final class Download { final boolean successful = statusCode >= 200 && statusCode < 300; if (!successful) { + progressListener.onEnd(); throw error("HTTP request to (%s) returned unsuccessful status (%d)", url, statusCode); } @@ -148,6 +146,7 @@ public final class Download { if (!downloadRequired) { // Does not require download, we are done here. + progressListener.onEnd(); return; } @@ -282,16 +281,18 @@ public final class Download { } private boolean requiresDownload(Path output) throws DownloadException { - if (getAndResetLock(output) & downloadAttempt == 1) { - LOGGER.warn("Forcing downloading {} as existing lock file was found. This may happen if the gradle build was forcefully canceled.", output); - return true; - } + final boolean locked = getAndResetLock(output); if (forceDownload || !exists(output)) { // File does not exist, or we are forced to download again. return true; } + if (locked && downloadAttempt == 1) { + LOGGER.warn("Forcing downloading {} as existing lock file was found. This may happen if the gradle build was forcefully canceled.", output); + return true; + } + if (offline) { // We know the file exists, nothing more we can do. return false; @@ -311,12 +312,6 @@ public final class Download { return false; } - if (System.getProperty("fabric.loom.test") != null) { - // This should never happen in an ideal world. - // It means that something has altered a file that should be cached. - throw error("Download file (%s) may have been modified", output); - } - LOGGER.info("Found existing file ({}) to download with unexpected hash.", output); } diff --git a/src/test/groovy/net/fabricmc/loom/test/LoomTestConstants.groovy b/src/test/groovy/net/fabricmc/loom/test/LoomTestConstants.groovy index 1db061a2..1adaaaf2 100644 --- a/src/test/groovy/net/fabricmc/loom/test/LoomTestConstants.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/LoomTestConstants.groovy @@ -27,7 +27,7 @@ package net.fabricmc.loom.test import org.gradle.util.GradleVersion class LoomTestConstants { - private final static String NIGHTLY_VERSION = "8.2-20230415221329+0000" + private final static String NIGHTLY_VERSION = "8.3-20230430222526+0000" private final static boolean NIGHTLY_EXISTS = nightlyExists(NIGHTLY_VERSION) // Test against the version of Gradle being used to build loom diff --git a/src/test/groovy/net/fabricmc/loom/test/unit/library/LibraryContextTest.groovy b/src/test/groovy/net/fabricmc/loom/test/unit/library/LibraryContextTest.groovy index 871d318d..3432c387 100644 --- a/src/test/groovy/net/fabricmc/loom/test/unit/library/LibraryContextTest.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/unit/library/LibraryContextTest.groovy @@ -58,7 +58,7 @@ class LibraryContextTest extends Specification { where: id || supported "23w16a" || true - "1.19.4" || false + "1.19.4" || true "1.18.2" || false "1.16.5" || false "1.4.7" || false diff --git a/src/test/groovy/net/fabricmc/loom/test/unit/library/processors/ArmNativesLibraryProcessorTest.groovy b/src/test/groovy/net/fabricmc/loom/test/unit/library/processors/ArmNativesLibraryProcessorTest.groovy index 7a7af5c9..393f3eb5 100644 --- a/src/test/groovy/net/fabricmc/loom/test/unit/library/processors/ArmNativesLibraryProcessorTest.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/unit/library/processors/ArmNativesLibraryProcessorTest.groovy @@ -58,7 +58,7 @@ class ArmNativesLibraryProcessorTest extends LibraryProcessorTest { where: id || result "23w16a" || LibraryProcessor.ApplicationResult.DONT_APPLY // Supports ARM64 Windows - "1.19.4" || LibraryProcessor.ApplicationResult.MUST_APPLY + "1.19.4" || LibraryProcessor.ApplicationResult.DONT_APPLY "1.18.2" || LibraryProcessor.ApplicationResult.DONT_APPLY // Only support versions with classpath natives. "1.12.2" || LibraryProcessor.ApplicationResult.DONT_APPLY // Not LWJGL 3 } @@ -118,23 +118,6 @@ class ArmNativesLibraryProcessorTest extends LibraryProcessorTest { "1.14.4" | _ } - def "Add windows arm64 natives"() { - when: - def (original, context) = getLibs("1.19.4", PlatformTestUtils.WINDOWS_ARM64) - def processor = new ArmNativesLibraryProcessor(PlatformTestUtils.WINDOWS_ARM64, context) - def processed = mockLibraryProcessorManager().processLibraries([processor], original) - - then: - // Test that the arm64 natives are added alongside the existing ones - def originalNatives = original.findAll { it.is("org.lwjgl") && it.target() == Library.Target.NATIVES } - originalNatives.count { it.classifier() == "natives-windows-arm64" } == 0 - originalNatives.count { it.classifier() == "natives-windows" } > 0 - - def processedNatives = processed.findAll { it.is("org.lwjgl") && it.target() == Library.Target.NATIVES } - processedNatives.count { it.classifier() == "natives-windows-arm64" } > 0 - processedNatives.count { it.classifier() == "natives-windows" } > 0 - } - def "Add linux arm64 natives"() { when: def (original, context) = getLibs("1.19.4", PlatformTestUtils.LINUX_ARM64) From 35e827566ea8eaf67142123c55fd72efd268dc50 Mon Sep 17 00:00:00 2001 From: modmuss50 Date: Fri, 5 May 2023 13:26:06 +0100 Subject: [PATCH 2/3] Move jar merger to loom. (#882) * Move jar merger to loom. * Fix copyright years --- .../minecraft/MergedMinecraftProvider.java | 3 +- .../minecraft/MinecraftClassMerger.java | 280 ++++++++++++++++++ .../minecraft/MinecraftJarMerger.java | 248 ++++++++++++++++ .../loom/util/SnowmanClassVisitor.java | 83 ++++++ .../util/SyntheticParameterClassVisitor.java | 113 +++++++ 5 files changed, 725 insertions(+), 2 deletions(-) create mode 100644 src/main/java/net/fabricmc/loom/configuration/providers/minecraft/MinecraftClassMerger.java create mode 100644 src/main/java/net/fabricmc/loom/configuration/providers/minecraft/MinecraftJarMerger.java create mode 100644 src/main/java/net/fabricmc/loom/util/SnowmanClassVisitor.java create mode 100644 src/main/java/net/fabricmc/loom/util/SyntheticParameterClassVisitor.java diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/MergedMinecraftProvider.java b/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/MergedMinecraftProvider.java index 50ca208f..40206306 100644 --- a/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/MergedMinecraftProvider.java +++ b/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/MergedMinecraftProvider.java @@ -32,7 +32,6 @@ import java.util.List; import java.util.Objects; import net.fabricmc.loom.configuration.ConfigContext; -import net.fabricmc.stitch.merge.JarMerger; public final class MergedMinecraftProvider extends MinecraftProvider { private Path minecraftMergedJar; @@ -86,7 +85,7 @@ public final class MergedMinecraftProvider extends MinecraftProvider { Objects.requireNonNull(jarToMerge, "Cannot merge null input jar?"); - try (JarMerger jarMerger = new JarMerger(getMinecraftClientJar(), jarToMerge, minecraftMergedJar.toFile())) { + try (var jarMerger = new MinecraftJarMerger(getMinecraftClientJar(), jarToMerge, minecraftMergedJar.toFile())) { jarMerger.enableSyntheticParamsOffset(); jarMerger.merge(); } diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/MinecraftClassMerger.java b/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/MinecraftClassMerger.java new file mode 100644 index 00000000..7d38da2e --- /dev/null +++ b/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/MinecraftClassMerger.java @@ -0,0 +1,280 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2016-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 + * 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.configuration.providers.minecraft; + +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Locale; +import java.util.Map; + +import org.objectweb.asm.AnnotationVisitor; +import org.objectweb.asm.ClassReader; +import org.objectweb.asm.ClassVisitor; +import org.objectweb.asm.ClassWriter; +import org.objectweb.asm.Type; +import org.objectweb.asm.tree.ClassNode; +import org.objectweb.asm.tree.FieldNode; +import org.objectweb.asm.tree.InnerClassNode; +import org.objectweb.asm.tree.MethodNode; + +import net.fabricmc.loom.util.Constants; + +public class MinecraftClassMerger { + private static final String SIDE_DESCRIPTOR = "Lnet/fabricmc/api/EnvType;"; + private static final String ITF_DESCRIPTOR = "Lnet/fabricmc/api/EnvironmentInterface;"; + private static final String ITF_LIST_DESCRIPTOR = "Lnet/fabricmc/api/EnvironmentInterfaces;"; + private static final String SIDED_DESCRIPTOR = "Lnet/fabricmc/api/Environment;"; + + private abstract static class Merger { + private final Map entriesClient, entriesServer; + private final List entryNames; + + Merger(List entriesClient, List entriesServer) { + this.entriesClient = new LinkedHashMap<>(); + this.entriesServer = new LinkedHashMap<>(); + + List listClient = toMap(entriesClient, this.entriesClient); + List listServer = toMap(entriesServer, this.entriesServer); + + this.entryNames = mergePreserveOrder(listClient, listServer); + } + + public abstract String getName(T entry); + + public abstract void applySide(T entry, String side); + + private List toMap(List entries, Map map) { + List list = new ArrayList<>(entries.size()); + + for (T entry : entries) { + String name = getName(entry); + map.put(name, entry); + list.add(name); + } + + return list; + } + + public void merge(List list) { + for (String s : entryNames) { + T entryClient = entriesClient.get(s); + T entryServer = entriesServer.get(s); + + if (entryClient != null && entryServer != null) { + list.add(entryClient); + } else if (entryClient != null) { + applySide(entryClient, "CLIENT"); + list.add(entryClient); + } else { + applySide(entryServer, "SERVER"); + list.add(entryServer); + } + } + } + } + + private static void visitSideAnnotation(AnnotationVisitor av, String side) { + av.visitEnum("value", SIDE_DESCRIPTOR, side.toUpperCase(Locale.ROOT)); + av.visitEnd(); + } + + private static void visitItfAnnotation(AnnotationVisitor av, String side, List itfDescriptors) { + for (String itf : itfDescriptors) { + AnnotationVisitor avItf = av.visitAnnotation(null, ITF_DESCRIPTOR); + avItf.visitEnum("value", SIDE_DESCRIPTOR, side.toUpperCase(Locale.ROOT)); + avItf.visit("itf", Type.getType("L" + itf + ";")); + avItf.visitEnd(); + } + } + + public static class SidedClassVisitor extends ClassVisitor { + private final String side; + + public SidedClassVisitor(int api, ClassVisitor cv, String side) { + super(api, cv); + this.side = side; + } + + @Override + public void visitEnd() { + AnnotationVisitor av = cv.visitAnnotation(SIDED_DESCRIPTOR, true); + visitSideAnnotation(av, side); + super.visitEnd(); + } + } + + public MinecraftClassMerger() { + } + + public byte[] merge(byte[] classClient, byte[] classServer) { + ClassReader readerC = new ClassReader(classClient); + ClassReader readerS = new ClassReader(classServer); + ClassWriter writer = new ClassWriter(0); + + ClassNode nodeC = new ClassNode(Constants.ASM_VERSION); + readerC.accept(nodeC, 0); + + ClassNode nodeS = new ClassNode(Constants.ASM_VERSION); + readerS.accept(nodeS, 0); + + ClassNode nodeOut = new ClassNode(Constants.ASM_VERSION); + nodeOut.version = nodeC.version; + nodeOut.access = nodeC.access; + nodeOut.name = nodeC.name; + nodeOut.signature = nodeC.signature; + nodeOut.superName = nodeC.superName; + nodeOut.sourceFile = nodeC.sourceFile; + nodeOut.sourceDebug = nodeC.sourceDebug; + nodeOut.outerClass = nodeC.outerClass; + nodeOut.outerMethod = nodeC.outerMethod; + nodeOut.outerMethodDesc = nodeC.outerMethodDesc; + nodeOut.module = nodeC.module; + nodeOut.nestHostClass = nodeC.nestHostClass; + nodeOut.nestMembers = nodeC.nestMembers; + nodeOut.attrs = nodeC.attrs; + + if (nodeC.invisibleAnnotations != null) { + nodeOut.invisibleAnnotations = new ArrayList<>(); + nodeOut.invisibleAnnotations.addAll(nodeC.invisibleAnnotations); + } + + if (nodeC.invisibleTypeAnnotations != null) { + nodeOut.invisibleTypeAnnotations = new ArrayList<>(); + nodeOut.invisibleTypeAnnotations.addAll(nodeC.invisibleTypeAnnotations); + } + + if (nodeC.visibleAnnotations != null) { + nodeOut.visibleAnnotations = new ArrayList<>(); + nodeOut.visibleAnnotations.addAll(nodeC.visibleAnnotations); + } + + if (nodeC.visibleTypeAnnotations != null) { + nodeOut.visibleTypeAnnotations = new ArrayList<>(); + nodeOut.visibleTypeAnnotations.addAll(nodeC.visibleTypeAnnotations); + } + + List itfs = mergePreserveOrder(nodeC.interfaces, nodeS.interfaces); + nodeOut.interfaces = new ArrayList<>(); + + List clientItfs = new ArrayList<>(); + List serverItfs = new ArrayList<>(); + + for (String s : itfs) { + boolean nc = nodeC.interfaces.contains(s); + boolean ns = nodeS.interfaces.contains(s); + nodeOut.interfaces.add(s); + + if (nc && !ns) { + clientItfs.add(s); + } else if (ns && !nc) { + serverItfs.add(s); + } + } + + if (!clientItfs.isEmpty() || !serverItfs.isEmpty()) { + AnnotationVisitor envInterfaces = nodeOut.visitAnnotation(ITF_LIST_DESCRIPTOR, false); + AnnotationVisitor eiArray = envInterfaces.visitArray("value"); + + if (!clientItfs.isEmpty()) { + visitItfAnnotation(eiArray, "CLIENT", clientItfs); + } + + if (!serverItfs.isEmpty()) { + visitItfAnnotation(eiArray, "SERVER", serverItfs); + } + + eiArray.visitEnd(); + envInterfaces.visitEnd(); + } + + new Merger<>(nodeC.innerClasses, nodeS.innerClasses) { + @Override + public String getName(InnerClassNode entry) { + return entry.name; + } + + @Override + public void applySide(InnerClassNode entry, String side) { + } + }.merge(nodeOut.innerClasses); + + new Merger<>(nodeC.fields, nodeS.fields) { + @Override + public String getName(FieldNode entry) { + return entry.name + ";;" + entry.desc; + } + + @Override + public void applySide(FieldNode entry, String side) { + AnnotationVisitor av = entry.visitAnnotation(SIDED_DESCRIPTOR, false); + visitSideAnnotation(av, side); + } + }.merge(nodeOut.fields); + + new Merger<>(nodeC.methods, nodeS.methods) { + @Override + public String getName(MethodNode entry) { + return entry.name + entry.desc; + } + + @Override + public void applySide(MethodNode entry, String side) { + AnnotationVisitor av = entry.visitAnnotation(SIDED_DESCRIPTOR, false); + visitSideAnnotation(av, side); + } + }.merge(nodeOut.methods); + + nodeOut.accept(writer); + return writer.toByteArray(); + } + + private static List mergePreserveOrder(List first, List second) { + List out = new ArrayList<>(); + int i = 0; + int j = 0; + + while (i < first.size() || j < second.size()) { + while (i < first.size() && j < second.size() + && first.get(i).equals(second.get(j))) { + out.add(first.get(i)); + i++; + j++; + } + + while (i < first.size() && !second.contains(first.get(i))) { + out.add(first.get(i)); + i++; + } + + while (j < second.size() && !first.contains(second.get(j))) { + out.add(second.get(j)); + j++; + } + } + + return out; + } +} diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/MinecraftJarMerger.java b/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/MinecraftJarMerger.java new file mode 100644 index 00000000..5b0f9d9c --- /dev/null +++ b/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/MinecraftJarMerger.java @@ -0,0 +1,248 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2016-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 + * 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.configuration.providers.minecraft; + +import java.io.File; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.FileVisitResult; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.SimpleFileVisitor; +import java.nio.file.StandardOpenOption; +import java.nio.file.attribute.BasicFileAttributeView; +import java.nio.file.attribute.BasicFileAttributes; +import java.util.Arrays; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Set; +import java.util.TreeSet; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; + +import org.objectweb.asm.ClassReader; +import org.objectweb.asm.ClassVisitor; +import org.objectweb.asm.ClassWriter; + +import net.fabricmc.loom.util.Constants; +import net.fabricmc.loom.util.FileSystemUtil; +import net.fabricmc.loom.util.SnowmanClassVisitor; +import net.fabricmc.loom.util.SyntheticParameterClassVisitor; + +public class MinecraftJarMerger implements AutoCloseable { + public static class Entry { + public final Path path; + public final BasicFileAttributes metadata; + public final byte[] data; + + public Entry(Path path, BasicFileAttributes metadata, byte[] data) { + this.path = path; + this.metadata = metadata; + this.data = data; + } + } + + private static final MinecraftClassMerger CLASS_MERGER = new MinecraftClassMerger(); + private final FileSystemUtil.Delegate inputClientFs, inputServerFs, outputFs; + private final Path inputClient, inputServer; + private final Map entriesClient, entriesServer; + private final Set entriesAll; + private boolean removeSnowmen = false; + private boolean offsetSyntheticsParams = false; + + public MinecraftJarMerger(File inputClient, File inputServer, File output) throws IOException { + if (output.exists()) { + if (!output.delete()) { + throw new IOException("Could not delete " + output.getName()); + } + } + + this.inputClient = (inputClientFs = FileSystemUtil.getJarFileSystem(inputClient, false)).get().getPath("/"); + this.inputServer = (inputServerFs = FileSystemUtil.getJarFileSystem(inputServer, false)).get().getPath("/"); + this.outputFs = FileSystemUtil.getJarFileSystem(output, true); + + this.entriesClient = new HashMap<>(); + this.entriesServer = new HashMap<>(); + this.entriesAll = new TreeSet<>(); + } + + public void enableSnowmanRemoval() { + removeSnowmen = true; + } + + public void enableSyntheticParamsOffset() { + offsetSyntheticsParams = true; + } + + @Override + public void close() throws IOException { + inputClientFs.close(); + inputServerFs.close(); + outputFs.close(); + } + + private void readToMap(Map map, Path input) { + try { + Files.walkFileTree(input, new SimpleFileVisitor<>() { + @Override + public FileVisitResult visitFile(Path path, BasicFileAttributes attr) throws IOException { + if (attr.isDirectory()) { + return FileVisitResult.CONTINUE; + } + + if (!path.getFileName().toString().endsWith(".class")) { + if (path.toString().equals("/META-INF/MANIFEST.MF")) { + map.put("META-INF/MANIFEST.MF", new Entry(path, attr, + "Manifest-Version: 1.0\nMain-Class: net.minecraft.client.Main\n".getBytes(StandardCharsets.UTF_8))); + } else { + if (path.toString().startsWith("/META-INF/")) { + if (path.toString().endsWith(".SF") || path.toString().endsWith(".RSA")) { + return FileVisitResult.CONTINUE; + } + } + + map.put(path.toString().substring(1), new Entry(path, attr, null)); + } + + return FileVisitResult.CONTINUE; + } + + byte[] output = Files.readAllBytes(path); + map.put(path.toString().substring(1), new Entry(path, attr, output)); + return FileVisitResult.CONTINUE; + } + }); + } catch (IOException e) { + e.printStackTrace(); + } + } + + private void add(Entry entry) throws IOException { + Path outPath = outputFs.get().getPath(entry.path.toString()); + + if (outPath.getParent() != null) { + Files.createDirectories(outPath.getParent()); + } + + if (entry.data != null) { + Files.write(outPath, entry.data, StandardOpenOption.CREATE_NEW); + } else { + Files.copy(entry.path, outPath); + } + + Files.getFileAttributeView(outPath, BasicFileAttributeView.class) + .setTimes( + entry.metadata.creationTime(), + entry.metadata.lastAccessTime(), + entry.metadata.lastModifiedTime() + ); + } + + public void merge() throws IOException { + ExecutorService service = Executors.newFixedThreadPool(2); + service.submit(() -> readToMap(entriesClient, inputClient)); + service.submit(() -> readToMap(entriesServer, inputServer)); + service.shutdown(); + + try { + service.awaitTermination(1, TimeUnit.HOURS); + } catch (InterruptedException e) { + e.printStackTrace(); + } + + entriesAll.addAll(entriesClient.keySet()); + entriesAll.addAll(entriesServer.keySet()); + + List entries = entriesAll.parallelStream().map((entry) -> { + boolean isClass = entry.endsWith(".class"); + boolean isMinecraft = entriesClient.containsKey(entry) || entry.startsWith("net/minecraft") || !entry.contains("/"); + Entry result; + String side = null; + + Entry entry1 = entriesClient.get(entry); + Entry entry2 = entriesServer.get(entry); + + if (entry1 != null && entry2 != null) { + if (Arrays.equals(entry1.data, entry2.data)) { + result = entry1; + } else { + if (isClass) { + result = new Entry(entry1.path, entry1.metadata, CLASS_MERGER.merge(entry1.data, entry2.data)); + } else { + // FIXME: More heuristics? + result = entry1; + } + } + } else if ((result = entry1) != null) { + side = "CLIENT"; + } else if ((result = entry2) != null) { + side = "SERVER"; + } + + if (isClass && !isMinecraft && "SERVER".equals(side)) { + // Server bundles libraries, client doesn't - skip them + return null; + } + + if (result != null) { + if (isMinecraft && isClass) { + byte[] data = result.data; + ClassReader reader = new ClassReader(data); + ClassWriter writer = new ClassWriter(0); + ClassVisitor visitor = writer; + + if (side != null) { + visitor = new MinecraftClassMerger.SidedClassVisitor(Constants.ASM_VERSION, visitor, side); + } + + if (removeSnowmen) { + visitor = new SnowmanClassVisitor(Constants.ASM_VERSION, visitor); + } + + if (offsetSyntheticsParams) { + visitor = new SyntheticParameterClassVisitor(Constants.ASM_VERSION, visitor); + } + + if (visitor != writer) { + reader.accept(visitor, 0); + data = writer.toByteArray(); + result = new Entry(result.path, result.metadata, data); + } + } + + return result; + } else { + return null; + } + }).filter(Objects::nonNull).toList(); + + for (Entry e : entries) { + add(e); + } + } +} diff --git a/src/main/java/net/fabricmc/loom/util/SnowmanClassVisitor.java b/src/main/java/net/fabricmc/loom/util/SnowmanClassVisitor.java new file mode 100644 index 00000000..0cc59c68 --- /dev/null +++ b/src/main/java/net/fabricmc/loom/util/SnowmanClassVisitor.java @@ -0,0 +1,83 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2016-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 + * 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.util; + +import org.objectweb.asm.ClassVisitor; +import org.objectweb.asm.Label; +import org.objectweb.asm.MethodVisitor; + +public class SnowmanClassVisitor extends ClassVisitor { + public static class SnowmanMethodVisitor extends MethodVisitor { + public SnowmanMethodVisitor(int api, MethodVisitor methodVisitor) { + super(api, methodVisitor); + } + + @Override + public void visitParameter(final String name, final int access) { + if (name != null && name.startsWith("\u2603")) { + super.visitParameter(null, access); + } else { + super.visitParameter(name, access); + } + } + + @Override + public void visitLocalVariable( + final String name, + final String descriptor, + final String signature, + final Label start, + final Label end, + final int index) { + String newName = name; + + if (name != null && name.startsWith("\u2603")) { + newName = "lvt" + index; + } + + super.visitLocalVariable(newName, descriptor, signature, start, end, index); + } + } + + public SnowmanClassVisitor(int api, ClassVisitor cv) { + super(api, cv); + } + + @Override + public void visitSource(final String source, final String debug) { + // Don't trust the obfuscation on this. + super.visitSource(null, null); + } + + @Override + public MethodVisitor visitMethod( + final int access, + final String name, + final String descriptor, + final String signature, + final String[] exceptions) { + return new SnowmanMethodVisitor(api, super.visitMethod(access, name, descriptor, signature, exceptions)); + } +} diff --git a/src/main/java/net/fabricmc/loom/util/SyntheticParameterClassVisitor.java b/src/main/java/net/fabricmc/loom/util/SyntheticParameterClassVisitor.java new file mode 100644 index 00000000..585b50c0 --- /dev/null +++ b/src/main/java/net/fabricmc/loom/util/SyntheticParameterClassVisitor.java @@ -0,0 +1,113 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2016-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 + * 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.util; + +import org.objectweb.asm.AnnotationVisitor; +import org.objectweb.asm.ClassVisitor; +import org.objectweb.asm.MethodVisitor; +import org.objectweb.asm.Opcodes; + +/** + * ProGuard has a bug where parameter annotations are applied incorrectly in the presence of + * synthetic arguments. This causes javac to balk when trying to load affected classes. + * + *

We use several heuristics to guess what the synthetic arguments may be for a particular + * constructor. We then check if the constructor matches our guess, and if so, offset all + * parameter annotations. + */ +public class SyntheticParameterClassVisitor extends ClassVisitor { + private static class SyntheticMethodVisitor extends MethodVisitor { + private final int offset; + + SyntheticMethodVisitor(int api, int offset, MethodVisitor methodVisitor) { + super(api, methodVisitor); + this.offset = offset; + } + + @Override + public AnnotationVisitor visitParameterAnnotation(int parameter, String descriptor, boolean visible) { + return super.visitParameterAnnotation(parameter - offset, descriptor, visible); + } + + @Override + public void visitAnnotableParameterCount(int parameterCount, boolean visible) { + super.visitAnnotableParameterCount(parameterCount - offset, visible); + } + } + + private String className; + private int synthetic; + private String syntheticArgs; + private boolean backoff = false; + + public SyntheticParameterClassVisitor(int api, ClassVisitor cv) { + super(api, cv); + } + + @Override + public void visit(int version, int access, String name, String signature, String superName, String[] interfaces) { + super.visit(version, access, name, signature, superName, interfaces); + + this.className = name; + + // Enums will always have a string name and then the ordinal + if ((access & Opcodes.ACC_ENUM) != 0) { + synthetic = 2; + syntheticArgs = "(Ljava/lang/String;I"; + } + + if (version >= 55) { + // Backoff on java 11 or newer due to nest mates being used. + backoff = true; + } + } + + @Override + public void visitInnerClass(String name, String outerName, String innerName, int access) { + super.visitInnerClass(name, outerName, innerName, access); + + // If we're a non-static, non-anonymous inner class then we can assume the first argument + // is the parent class. + // See https://docs.oracle.com/javase/specs/jls/se11/html/jls-8.html#jls-8.8.1 + if (synthetic == 0 && name.equals(this.className) && innerName != null && outerName != null && (access & Opcodes.ACC_STATIC) == 0) { + this.synthetic = 1; + this.syntheticArgs = "(L" + outerName + ";"; + } + } + + @Override + public MethodVisitor visitMethod( + final int access, + final String name, + final String descriptor, + final String signature, + final String[] exceptions) { + MethodVisitor mv = super.visitMethod(access, name, descriptor, signature, exceptions); + + return mv != null && synthetic != 0 && name.equals("") && descriptor.startsWith(syntheticArgs) && !backoff + ? new SyntheticMethodVisitor(api, synthetic, mv) + : mv; + } +} From d1f35d42218d0710ca640e6f8bef31b7232a9304 Mon Sep 17 00:00:00 2001 From: modmuss50 Date: Fri, 5 May 2023 13:30:54 +0100 Subject: [PATCH 3/3] Fix publish --- .github/workflows/publish.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 8828b3b4..4ad1b555 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -17,10 +17,10 @@ jobs: # Generate the build number based on tags to allow per branch build numbers, not something github provides by default. - name: Generate build number id: buildnumber - uses: einaregilsson/build-number@v3 + uses: onyxmueller/build-tag-number@v1 with: token: ${{ secrets.github_token }} - prefix: ${{ github.ref }} + prefix: "build/${{ github.ref }}" - run: ./gradlew build publish -x test --stacktrace env: