From 02af089e5745b8ea3cbbe53259755e192702f237 Mon Sep 17 00:00:00 2001 From: modmuss Date: Sun, 14 Apr 2024 15:14:44 +0100 Subject: [PATCH 1/5] Lock disowning +write linemap txt file for debugging (#1094) * Write line map file (#1090) * Lock disowning (#1093) * Lock disowning * Comment fix * Typo * Use a much lower lock timeout on CI --- .../configuration/CompileConfiguration.java | 68 +++++++++++++++---- .../loom/decompilers/ClassLineNumbers.java | 5 ++ .../decompilers/cache/CachedJarProcessor.java | 12 +++- .../loom/task/GenerateSourcesTask.java | 9 +++ 4 files changed, 79 insertions(+), 15 deletions(-) diff --git a/src/main/java/net/fabricmc/loom/configuration/CompileConfiguration.java b/src/main/java/net/fabricmc/loom/configuration/CompileConfiguration.java index fff381f6..72ee99eb 100644 --- a/src/main/java/net/fabricmc/loom/configuration/CompileConfiguration.java +++ b/src/main/java/net/fabricmc/loom/configuration/CompileConfiguration.java @@ -109,15 +109,16 @@ public abstract class CompileConfiguration implements Runnable { try { setupMinecraft(configContext); + + LoomDependencyManager dependencyManager = new LoomDependencyManager(); + extension.setDependencyManager(dependencyManager); + dependencyManager.handleDependencies(getProject(), serviceManager); } catch (Exception e) { ExceptionUtil.printFileLocks(e, getProject()); + disownLock(); throw ExceptionUtil.createDescriptiveWrapper(RuntimeException::new, "Failed to setup Minecraft", e); } - LoomDependencyManager dependencyManager = new LoomDependencyManager(); - extension.setDependencyManager(dependencyManager); - dependencyManager.handleDependencies(getProject(), serviceManager); - releaseLock(); extension.setRefreshDeps(previousRefreshDeps); @@ -274,12 +275,14 @@ public abstract class CompileConfiguration implements Runnable { // already owned by current pid ACQUIRED_ALREADY_OWNED, // acquired due to current owner not existing - ACQUIRED_PREVIOUS_OWNER_MISSING + ACQUIRED_PREVIOUS_OWNER_MISSING, + // acquired due to previous owner disowning the lock + ACQUIRED_PREVIOUS_OWNER_DISOWNED } private LockResult acquireProcessLockWaiting(LockFile lockFile) { // one hour - return this.acquireProcessLockWaiting(lockFile, Duration.ofHours(1)); + return this.acquireProcessLockWaiting(lockFile, getDefaultTimeout()); } private LockResult acquireProcessLockWaiting(LockFile lockFile, Duration timeout) { @@ -297,25 +300,34 @@ public abstract class CompileConfiguration implements Runnable { final Logger logger = Logging.getLogger("loom_acquireProcessLockWaiting"); final long currentPid = ProcessHandle.current().pid(); boolean abrupt = false; + boolean disowned = false; if (Files.exists(lockFile.file)) { - long lockingProcessId; + long lockingProcessId = -1; try { - lockingProcessId = Long.parseLong(Files.readString(lockFile.file)); - } catch (final Exception e) { - lockingProcessId = -1; + String lockValue = Files.readString(lockFile.file); + + if ("disowned".equals(lockValue)) { + disowned = true; + } else { + lockingProcessId = Long.parseLong(lockValue); + logger.lifecycle("\"{}\" is currently held by pid '{}'.", lockFile, lockingProcessId); + } + } catch (final Exception ignored) { + // ignored } if (lockingProcessId == currentPid) { return LockResult.ACQUIRED_ALREADY_OWNED; } - logger.lifecycle("\"{}\" is currently held by pid '{}'.", lockFile, lockingProcessId); - Optional handle = ProcessHandle.of(lockingProcessId); - if (handle.isEmpty()) { + if (disowned) { + logger.lifecycle("Previous process has disowned the lock due to abrupt termination."); + Files.deleteIfExists(lockFile.file); + } else if (handle.isEmpty()) { logger.lifecycle("Locking process does not exist, assuming abrupt termination and deleting lock file."); Files.deleteIfExists(lockFile.file); abrupt = true; @@ -356,7 +368,35 @@ public abstract class CompileConfiguration implements Runnable { } Files.writeString(lockFile.file, String.valueOf(currentPid)); - return abrupt ? LockResult.ACQUIRED_PREVIOUS_OWNER_MISSING : LockResult.ACQUIRED_CLEAN; + + if (disowned) { + return LockResult.ACQUIRED_PREVIOUS_OWNER_DISOWNED; + } else if (abrupt) { + return LockResult.ACQUIRED_PREVIOUS_OWNER_MISSING; + } + + return LockResult.ACQUIRED_CLEAN; + } + + private static Duration getDefaultTimeout() { + if (System.getenv("CI") != null) { + // Set a small timeout on CI, as it's unlikely going to unlock. + return Duration.ofMinutes(1); + } + + return Duration.ofHours(1); + } + + // When we fail to configure, write "disowned" to the lock file to release it from this process + // This allows the next run to rebuild without waiting for this process to exit + private void disownLock() { + final Path lock = getLockFile().file; + + try { + Files.writeString(lock, "disowned"); + } catch (IOException e) { + throw new RuntimeException(e); + } } private void releaseLock() { diff --git a/src/main/java/net/fabricmc/loom/decompilers/ClassLineNumbers.java b/src/main/java/net/fabricmc/loom/decompilers/ClassLineNumbers.java index 2c872406..120972fa 100644 --- a/src/main/java/net/fabricmc/loom/decompilers/ClassLineNumbers.java +++ b/src/main/java/net/fabricmc/loom/decompilers/ClassLineNumbers.java @@ -46,6 +46,11 @@ public record ClassLineNumbers(Map lineMap) { if (lineMap.isEmpty()) { throw new IllegalArgumentException("lineMap is empty"); } + + for (Map.Entry entry : lineMap.entrySet()) { + Objects.requireNonNull(entry.getKey(), "lineMap key"); + Objects.requireNonNull(entry.getValue(), "lineMap value"); + } } public static ClassLineNumbers readMappings(Path lineMappingsPath) { diff --git a/src/main/java/net/fabricmc/loom/decompilers/cache/CachedJarProcessor.java b/src/main/java/net/fabricmc/loom/decompilers/cache/CachedJarProcessor.java index 82952210..e89d035e 100644 --- a/src/main/java/net/fabricmc/loom/decompilers/cache/CachedJarProcessor.java +++ b/src/main/java/net/fabricmc/loom/decompilers/cache/CachedJarProcessor.java @@ -85,7 +85,13 @@ public record CachedJarProcessor(CachedFileStore fileStore, String b final Path outputPath = existingFs.getPath(outputFileName); Files.createDirectories(outputPath.getParent()); Files.writeString(outputPath, entryData.sources()); - lineNumbersMap.put(entryData.className(), entryData.lineNumbers()); + + if (entryData.lineNumbers() != null) { + lineNumbersMap.put(entryData.className(), entryData.lineNumbers()); + } else { + LOGGER.info("Cached entry ({}) does not have line numbers", outputFileName); + } + hasSomeExisting = true; LOGGER.debug("Cached entry ({}) found: {}", fullHash, outputFileName); @@ -166,6 +172,10 @@ public record CachedJarProcessor(CachedFileStore fileStore, String b lineMapEntry = lineNumbers.lineMap().get(className); } + if (lineMapEntry == null) { + LOGGER.info("No line numbers generated for class: {}", className); + } + final var cachedData = new CachedData(className, sources, lineMapEntry); fileStore.putEntry(hash, cachedData); diff --git a/src/main/java/net/fabricmc/loom/task/GenerateSourcesTask.java b/src/main/java/net/fabricmc/loom/task/GenerateSourcesTask.java index acf71a2e..44c20751 100644 --- a/src/main/java/net/fabricmc/loom/task/GenerateSourcesTask.java +++ b/src/main/java/net/fabricmc/loom/task/GenerateSourcesTask.java @@ -25,6 +25,7 @@ package net.fabricmc.loom.task; import java.io.BufferedReader; +import java.io.BufferedWriter; import java.io.File; import java.io.IOException; import java.io.InputStream; @@ -489,6 +490,14 @@ public abstract class GenerateSourcesTask extends AbstractLoomTask { Objects.requireNonNull(lineNumbers, "lineNumbers"); final var remapper = new LineNumberRemapper(lineNumbers); remapper.process(inputJar, outputJar); + + final Path lineMap = inputJar.resolveSibling(inputJar.getFileName() + ".linemap.txt"); + + try (BufferedWriter writer = Files.newBufferedWriter(lineMap)) { + lineNumbers.write(writer); + } + + LOGGER.info("Wrote linemap to {}", lineMap); } private void doWork(@Nullable IPCServer ipcServer, Path inputJar, Path outputJar, Path linemapFile, @Nullable Path existingJar) { From 1b1168d7e26118942188a83252f5a5c13429cf00 Mon Sep 17 00:00:00 2001 From: modmuss Date: Mon, 15 Apr 2024 15:11:57 +0100 Subject: [PATCH 2/5] Fix and test line number remapper (#1097) --- .../loom/decompilers/LineNumberRemapper.java | 11 +- .../integration/DebugLineNumbersTest.groovy | 2 + .../test/unit/LineNumberRemapperTests.groovy | 108 ++++++++++++++++++ .../loom/test/util/ZipTestUtils.groovy | 11 +- .../loom/test/unit/LineNumberSource.java | 31 +++++ 5 files changed, 158 insertions(+), 5 deletions(-) create mode 100644 src/test/groovy/net/fabricmc/loom/test/unit/LineNumberRemapperTests.groovy create mode 100644 src/test/java/net/fabricmc/loom/test/unit/LineNumberSource.java diff --git a/src/main/java/net/fabricmc/loom/decompilers/LineNumberRemapper.java b/src/main/java/net/fabricmc/loom/decompilers/LineNumberRemapper.java index 9fefae44..9b3ab78f 100644 --- a/src/main/java/net/fabricmc/loom/decompilers/LineNumberRemapper.java +++ b/src/main/java/net/fabricmc/loom/decompilers/LineNumberRemapper.java @@ -60,12 +60,11 @@ public record LineNumberRemapper(ClassLineNumbers lineNumbers) { } } - String fileName = file.getFileName().toString(); + String fileName = file.toAbsolutePath().toString(); if (fileName.endsWith(".class")) { - String idx = fileName.substring(0, fileName.length() - 6); - - LOGGER.debug("Remapping line numbers for class: " + idx); + // Strip the leading slash and the .class extension + String idx = fileName.substring(1, fileName.length() - 6); int dollarPos = idx.indexOf('$'); //This makes the assumption that only Java classes are to be remapped. @@ -74,6 +73,8 @@ public record LineNumberRemapper(ClassLineNumbers lineNumbers) { } if (lineNumbers.lineMap().containsKey(idx)) { + LOGGER.debug("Remapping line numbers for class: {}", idx); + try (InputStream is = Files.newInputStream(file)) { ClassReader reader = new ClassReader(is); ClassWriter writer = new ClassWriter(0); @@ -82,6 +83,8 @@ public record LineNumberRemapper(ClassLineNumbers lineNumbers) { Files.write(dst, writer.toByteArray()); return; } + } else { + LOGGER.debug("No linemap found for: {}", idx); } } diff --git a/src/test/groovy/net/fabricmc/loom/test/integration/DebugLineNumbersTest.groovy b/src/test/groovy/net/fabricmc/loom/test/integration/DebugLineNumbersTest.groovy index e7eda02b..ee791785 100644 --- a/src/test/groovy/net/fabricmc/loom/test/integration/DebugLineNumbersTest.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/integration/DebugLineNumbersTest.groovy @@ -122,6 +122,8 @@ class DebugLineNumbersTest extends Specification implements GradleProjectTestTra def result = it.get() println("Breakpoint triggered: ${result.location()}") } + + println("All breakpoints triggered") } finally { // Close the debugger and target process debugger.close() diff --git a/src/test/groovy/net/fabricmc/loom/test/unit/LineNumberRemapperTests.groovy b/src/test/groovy/net/fabricmc/loom/test/unit/LineNumberRemapperTests.groovy new file mode 100644 index 00000000..e07bfc70 --- /dev/null +++ b/src/test/groovy/net/fabricmc/loom/test/unit/LineNumberRemapperTests.groovy @@ -0,0 +1,108 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2024 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.unit + +import java.nio.file.Files + +import org.objectweb.asm.ClassReader +import org.objectweb.asm.ClassVisitor +import org.objectweb.asm.Label +import org.objectweb.asm.MethodVisitor +import spock.lang.Specification + +import net.fabricmc.loom.decompilers.ClassLineNumbers +import net.fabricmc.loom.decompilers.LineNumberRemapper +import net.fabricmc.loom.test.util.ZipTestUtils +import net.fabricmc.loom.util.Constants +import net.fabricmc.loom.util.ZipUtils + +class LineNumberRemapperTests extends Specification { + def "remapLinenumbers"() { + given: + def className = LineNumberSource.class.name.replace('.', '/') + def input = ZipTestUtils.createZipFromBytes([(className + ".class"): getClassBytes(LineNumberSource.class)]) + + // + 10 to each line number + def entry = new ClassLineNumbers.Entry(className, 30, 40, [ + 27: 37, + 29: 39, + 30: 40 + ]) + def lineNumbers = new ClassLineNumbers([(className): entry]) + + def outputJar = Files.createTempDirectory("loom").resolve("output.jar") + + when: + def remapper = new LineNumberRemapper(lineNumbers) + remapper.process(input, outputJar) + + def unpacked = ZipUtils.unpack(outputJar, className + ".class") + + then: + readLineNumbers(getClassBytes(LineNumberSource.class)) == [27, 29, 30] + readLineNumbers(unpacked) == [37, 39, 40] + } + + static byte[] getClassBytes(Class clazz) { + return clazz.classLoader.getResourceAsStream(clazz.name.replace('.', '/') + ".class").withCloseable { + it.bytes + } + } + + static List readLineNumbers(byte[] clazzBytes) { + def reader = new ClassReader(clazzBytes) + def visitor = new LineNumberCollectorClassVisitor() + reader.accept(visitor, 0) + return visitor.lineNumbers + } + + private static class LineNumberCollectorClassVisitor extends ClassVisitor { + final List lineNumbers = [] + + protected LineNumberCollectorClassVisitor() { + super(Constants.ASM_VERSION) + } + + @Override + MethodVisitor visitMethod(int access, String name, String descriptor, String signature, String[] exceptions) { + return new LineNumberCollectorMethodVisitor(super.visitMethod(access, name, descriptor, signature, exceptions), lineNumbers) + } + } + + private static class LineNumberCollectorMethodVisitor extends MethodVisitor { + final List lineNumbers + + protected LineNumberCollectorMethodVisitor(MethodVisitor methodVisitor, List lineNumbers) { + super(Constants.ASM_VERSION, methodVisitor) + this.lineNumbers = lineNumbers + } + + @Override + void visitLineNumber(int line, Label start) { + super.visitLineNumber(line, start) + lineNumbers.add(line) + } + } +} diff --git a/src/test/groovy/net/fabricmc/loom/test/util/ZipTestUtils.groovy b/src/test/groovy/net/fabricmc/loom/test/util/ZipTestUtils.groovy index fd324198..75add96d 100644 --- a/src/test/groovy/net/fabricmc/loom/test/util/ZipTestUtils.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/util/ZipTestUtils.groovy @@ -34,6 +34,15 @@ import net.fabricmc.loom.util.FileSystemUtil class ZipTestUtils { static Path createZip(Map entries) { + return createZipFromBytes(entries.collectEntries { k, v -> + [ + k, + v.getBytes(StandardCharsets.UTF_8) + ] + }) + } + + static Path createZipFromBytes(Map entries) { def file = Files.createTempFile("loom-test", ".zip") Files.delete(file) @@ -42,7 +51,7 @@ class ZipTestUtils { def fsPath = zip.getPath(path) def fsPathParent = fsPath.getParent() if (fsPathParent != null) Files.createDirectories(fsPathParent) - Files.writeString(fsPath, value, StandardCharsets.UTF_8) + Files.write(fsPath, value) } } diff --git a/src/test/java/net/fabricmc/loom/test/unit/LineNumberSource.java b/src/test/java/net/fabricmc/loom/test/unit/LineNumberSource.java new file mode 100644 index 00000000..190b25c0 --- /dev/null +++ b/src/test/java/net/fabricmc/loom/test/unit/LineNumberSource.java @@ -0,0 +1,31 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2024 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.unit; + +public class LineNumberSource { + public static void main(String[] args) { + System.out.println("Hello World!"); + } +} From 7010ad633be376bce94b70a27b5e4fa6ee2e7566 Mon Sep 17 00:00:00 2001 From: modmuss Date: Mon, 15 Apr 2024 15:12:10 +0100 Subject: [PATCH 3/5] Fix sources publishing (#1098) * Fix sources publishing * Revert "Fix sources publishing" This reverts commit a3fec653dc5255b9bc939dd3305e5df8529da729. * A better fix --- .../loom/task/RemapTaskConfiguration.java | 59 ++++++++++--------- 1 file changed, 32 insertions(+), 27 deletions(-) diff --git a/src/main/java/net/fabricmc/loom/task/RemapTaskConfiguration.java b/src/main/java/net/fabricmc/loom/task/RemapTaskConfiguration.java index f38fed6d..a6c57e10 100644 --- a/src/main/java/net/fabricmc/loom/task/RemapTaskConfiguration.java +++ b/src/main/java/net/fabricmc/loom/task/RemapTaskConfiguration.java @@ -120,45 +120,50 @@ public abstract class RemapTaskConfiguration implements Runnable { private void trySetupSourceRemapping() { final LoomGradleExtension extension = LoomGradleExtension.get(getProject()); - final String sourcesJarTaskName = SourceSetHelper.getMainSourceSet(getProject()).getSourcesJarTaskName(); TaskProvider remapSourcesTask = getTasks().register(REMAP_SOURCES_JAR_TASK_NAME, RemapSourcesJarTask.class, task -> { task.setDescription("Remaps the default sources jar to intermediary mappings."); task.setGroup(Constants.TaskGroup.FABRIC); - - final Task sourcesTask = getTasks().findByName(sourcesJarTaskName); - - if (sourcesTask == null) { - getProject().getLogger().info("{} task was not found, not remapping sources", sourcesJarTaskName); - task.setEnabled(false); - return; - } - - if (!(sourcesTask instanceof Jar sourcesJarTask)) { - getProject().getLogger().info("{} task is not a Jar task, not remapping sources", sourcesJarTaskName); - task.setEnabled(false); - return; - } - - sourcesJarTask.getArchiveClassifier().convention("dev-sources"); - sourcesJarTask.getDestinationDirectory().set(getProject().getLayout().getBuildDirectory().map(directory -> directory.dir("devlibs"))); - task.getArchiveClassifier().convention("sources"); - - task.dependsOn(sourcesJarTask); - task.getInputFile().convention(sourcesJarTask.getArchiveFile()); task.getIncludesClientOnlyClasses().set(getProject().provider(extension::areEnvironmentSourceSetsSplit)); }); getTasks().named(BasePlugin.ASSEMBLE_TASK_NAME).configure(task -> task.dependsOn(remapSourcesTask)); - if (GradleUtils.getBooleanProperty(getProject(), "fabric.loom.disableRemappedVariants")) { - return; - } - GradleUtils.afterSuccessfulEvaluation(getProject(), () -> { + final String sourcesJarTaskName = SourceSetHelper.getMainSourceSet(getProject()).getSourcesJarTaskName(); final Task sourcesTask = getTasks().findByName(sourcesJarTaskName); - if (!(sourcesTask instanceof Jar sourcesJarTask)) { + boolean canRemap = true; + + if (sourcesTask == null) { + getProject().getLogger().info("{} task was not found, not remapping sources", sourcesJarTaskName); + canRemap = false; + } + + if (canRemap && !(sourcesTask instanceof Jar)) { + getProject().getLogger().info("{} task is not a Jar task, not remapping sources", sourcesJarTaskName); + canRemap = false; + } + + boolean finalCanRemap = canRemap; + + remapSourcesTask.configure(task -> { + if (!finalCanRemap) { + task.setEnabled(false); + return; + } + + final Jar sourcesJarTask = (Jar) sourcesTask; + + sourcesJarTask.getArchiveClassifier().convention("dev-sources"); + sourcesJarTask.getDestinationDirectory().set(getProject().getLayout().getBuildDirectory().map(directory -> directory.dir("devlibs"))); + task.getArchiveClassifier().convention("sources"); + + task.dependsOn(sourcesJarTask); + task.getInputFile().convention(sourcesJarTask.getArchiveFile()); + }); + + if (GradleUtils.getBooleanProperty(getProject(), "fabric.loom.disableRemappedVariants")) { return; } From e142cb8d0c610be4c0e9f84269df3a1d93a4449e Mon Sep 17 00:00:00 2001 From: modmuss Date: Wed, 17 Apr 2024 20:41:29 +0100 Subject: [PATCH 4/5] Print file locks in more cases during decompile (#1099) --- .../java/net/fabricmc/loom/task/GenerateSourcesTask.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/main/java/net/fabricmc/loom/task/GenerateSourcesTask.java b/src/main/java/net/fabricmc/loom/task/GenerateSourcesTask.java index 44c20751..842eeee4 100644 --- a/src/main/java/net/fabricmc/loom/task/GenerateSourcesTask.java +++ b/src/main/java/net/fabricmc/loom/task/GenerateSourcesTask.java @@ -201,6 +201,9 @@ public abstract class GenerateSourcesTask extends AbstractLoomTask { if (!getUseCache().get()) { try (var timer = new Timer("Decompiled sources")) { runWithoutCache(); + } catch (Exception e) { + ExceptionUtil.printFileLocks(e, getProject()); + throw ExceptionUtil.createDescriptiveWrapper(RuntimeException::new, "Failed to decompile", e); } return; @@ -218,6 +221,9 @@ public abstract class GenerateSourcesTask extends AbstractLoomTask { try (FileSystemUtil.Delegate fs = FileSystemUtil.getJarFileSystem(cacheFile, true)) { runWithCache(fs.getRoot()); } + } catch (Exception e) { + ExceptionUtil.printFileLocks(e, getProject()); + throw ExceptionUtil.createDescriptiveWrapper(RuntimeException::new, "Failed to decompile", e); } } @@ -415,7 +421,6 @@ public abstract class GenerateSourcesTask extends AbstractLoomTask { final var provideContext = new AbstractMappedMinecraftProvider.ProvideContext(false, true, configContext); minecraftJars = getExtension().getNamedMinecraftProvider().provide(provideContext); } catch (Exception e) { - ExceptionUtil.printFileLocks(e, getProject()); throw ExceptionUtil.createDescriptiveWrapper(RuntimeException::new, "Failed to rebuild input jars", e); } From bd009515cbaded588ae734a4edb97ea1da103cce Mon Sep 17 00:00:00 2001 From: modmuss Date: Sat, 20 Apr 2024 22:49:21 +0100 Subject: [PATCH 5/5] Update loom native, with better error handling. (#1102) --- gradle/libs.versions.toml | 2 +- .../java/net/fabricmc/loom/util/ExceptionUtil.java | 14 +++++++++++++- .../java/net/fabricmc/loom/util/ProcessUtil.java | 13 ++++++++++++- 3 files changed, 26 insertions(+), 3 deletions(-) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index b1923d2d..9a14cffe 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -12,7 +12,7 @@ mapping-io = "0.5.1" lorenz-tiny = "4.0.2" mercury = "0.4.1" kotlinx-metadata = "0.9.0" -loom-native = "0.1.1" +loom-native = "0.2.0" # Plugins spotless = "6.25.0" diff --git a/src/main/java/net/fabricmc/loom/util/ExceptionUtil.java b/src/main/java/net/fabricmc/loom/util/ExceptionUtil.java index 8f942260..2f6d91ea 100644 --- a/src/main/java/net/fabricmc/loom/util/ExceptionUtil.java +++ b/src/main/java/net/fabricmc/loom/util/ExceptionUtil.java @@ -32,10 +32,15 @@ import java.util.List; import java.util.function.BiFunction; import org.gradle.api.Project; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import net.fabricmc.loom.nativeplatform.LoomNativePlatform; +import net.fabricmc.loom.nativeplatform.LoomNativePlatformException; public final class ExceptionUtil { + private static final Logger LOGGER = LoggerFactory.getLogger(ExceptionUtil.class); + /** * Creates a descriptive user-facing wrapper exception for an underlying cause. * @@ -74,7 +79,14 @@ public final class ExceptionUtil { return; } - final List processes = LoomNativePlatform.getProcessesWithLockOn(path); + final List processes; + + try { + processes = LoomNativePlatform.getProcessesWithLockOn(path); + } catch (LoomNativePlatformException e) { + LOGGER.error("{}, Failed to query processes holding a lock on {}", e.getMessage(), path); + return; + } if (processes.isEmpty()) { return; diff --git a/src/main/java/net/fabricmc/loom/util/ProcessUtil.java b/src/main/java/net/fabricmc/loom/util/ProcessUtil.java index 93f8e1e3..8b36e549 100644 --- a/src/main/java/net/fabricmc/loom/util/ProcessUtil.java +++ b/src/main/java/net/fabricmc/loom/util/ProcessUtil.java @@ -31,11 +31,15 @@ import java.util.StringJoiner; import org.gradle.api.Project; import org.gradle.api.logging.LogLevel; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import net.fabricmc.loom.nativeplatform.LoomNativePlatform; +import net.fabricmc.loom.nativeplatform.LoomNativePlatformException; public record ProcessUtil(LogLevel logLevel) { private static final String EXPLORER_COMMAND = "C:\\Windows\\explorer.exe"; + private static final Logger LOGGER = LoggerFactory.getLogger(ProcessUtil.class); public static ProcessUtil create(Project project) { return new ProcessUtil(project.getGradle().getStartParameter().getLogLevel()); @@ -92,7 +96,14 @@ public record ProcessUtil(LogLevel logLevel) { return Optional.empty(); } - List titles = LoomNativePlatform.getWindowTitlesForPid(processHandle.pid()); + List titles; + + try { + titles = LoomNativePlatform.getWindowTitlesForPid(processHandle.pid()); + } catch (LoomNativePlatformException e) { + LOGGER.error("{}, Failed to query window title for pid {}", e.getMessage(), processHandle.pid()); + return Optional.empty(); + } if (titles.isEmpty()) { return Optional.empty();