From b987b4e71156a6046f671f1dcc4c016480535699 Mon Sep 17 00:00:00 2001 From: modmuss Date: Thu, 23 Nov 2023 22:36:57 +0000 Subject: [PATCH 01/17] Always use unix line endings on windows (#986) * Always use unix line endings on windows * Also run checks on windows. * Fix binary files :) --- .gitattributes | 40 +------------------ .github/workflows/test-push.yml | 12 ++++++ build.gradle | 2 + .../integration/ReproducibleBuildTest.groovy | 12 ++---- 4 files changed, 19 insertions(+), 47 deletions(-) diff --git a/.gitattributes b/.gitattributes index 36452a5b..0f09d326 100644 --- a/.gitattributes +++ b/.gitattributes @@ -1,38 +1,2 @@ -# Handle line endings automatically for files detected as text -# and leave all files detected as binary untouched. -* text=auto - -*.patch text eol=lf - -# -# The above will handle all files NOT found below -# -# These files are text and should be normalized (Convert crlf => lf) -*.css text -*.df text -*.htm text -*.html text -*.java text -*.js text -*.json text -*.jsp text -*.jspf text -*.properties text -*.sh text -*.tld text -*.txt text -*.xml text - -# These files are binary and should be left untouched -# (binary is a macro for -text -diff) -*.class binary -*.dll binary -*.ear binary -*.gif binary -*.ico binary -*.jar binary -*.jpg binary -*.jpeg binary -*.png binary -*.so binary -*.war binary +* text=auto eol=lf +*.bat text eol=crlf \ No newline at end of file diff --git a/.github/workflows/test-push.yml b/.github/workflows/test-push.yml index 91956561..fad299cd 100644 --- a/.github/workflows/test-push.yml +++ b/.github/workflows/test-push.yml @@ -20,6 +20,18 @@ jobs: - uses: gradle/wrapper-validation-action@v1 - run: gradle build check -x test --stacktrace --warning-mode fail + build_windows: + runs-on: windows-2022 + steps: + - uses: actions/checkout@v3 + - name: setup jdk + uses: actions/setup-java@v3 + with: + java-version: 17 + distribution: 'temurin' + - uses: gradle/wrapper-validation-action@v1 + - run: ./gradlew build check -x test --stacktrace --warning-mode fail + # This job is used to feed the test matrix of next job to allow the tests to run in parallel prepare_test_matrix: # Lets wait to ensure it builds before going running tests diff --git a/build.gradle b/build.gradle index 54a7c7d3..d7cf3719 100644 --- a/build.gradle +++ b/build.gradle @@ -215,6 +215,8 @@ java { } spotless { + lineEndings = com.diffplug.spotless.LineEnding.UNIX + java { licenseHeaderFile(rootProject.file("HEADER")).yearSeparator("-") targetExclude("**/loom/util/DownloadUtil.java") diff --git a/src/test/groovy/net/fabricmc/loom/test/integration/ReproducibleBuildTest.groovy b/src/test/groovy/net/fabricmc/loom/test/integration/ReproducibleBuildTest.groovy index 4c51170d..2ebf3730 100644 --- a/src/test/groovy/net/fabricmc/loom/test/integration/ReproducibleBuildTest.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/integration/ReproducibleBuildTest.groovy @@ -51,18 +51,12 @@ class ReproducibleBuildTest extends Specification implements GradleProjectTestTr then: result.task(":build").outcome == SUCCESS generateMD5(gradle.getOutputFile("fabric-example-mod-1.0.0.jar")) == modHash - generateMD5(gradle.getOutputFile("fabric-example-mod-1.0.0-sources.jar")) in sourceHash // Done for different line endings. + generateMD5(gradle.getOutputFile("fabric-example-mod-1.0.0-sources.jar")) == sourceHash where: version | modHash | sourceHash - DEFAULT_GRADLE | "207bd75aa34fc996a97e962dd98b61d5" | [ - "8e8fac2a5e32fc872e6cf0f9ccc55cfd", - "ed331b6fae5677797a0104eba014e255" - ] - PRE_RELEASE_GRADLE | "207bd75aa34fc996a97e962dd98b61d5" | [ - "8e8fac2a5e32fc872e6cf0f9ccc55cfd", - "ed331b6fae5677797a0104eba014e255" - ] + DEFAULT_GRADLE | "207bd75aa34fc996a97e962dd98b61d5" | "8e8fac2a5e32fc872e6cf0f9ccc55cfd" + PRE_RELEASE_GRADLE | "207bd75aa34fc996a97e962dd98b61d5" | "8e8fac2a5e32fc872e6cf0f9ccc55cfd" } String generateMD5(File file) { From 229b3b38005823e00549826cbd532f21ee9174c4 Mon Sep 17 00:00:00 2001 From: Jason Penilla <11360596+jpenilla@users.noreply.github.com> Date: Mon, 4 Dec 2023 03:52:11 -0800 Subject: [PATCH 02/17] Don't resolve source artifacts in CI (#994) --- .../loom/configuration/mods/ModConfigurationRemapper.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/main/java/net/fabricmc/loom/configuration/mods/ModConfigurationRemapper.java b/src/main/java/net/fabricmc/loom/configuration/mods/ModConfigurationRemapper.java index 166bf59d..85f2c28b 100644 --- a/src/main/java/net/fabricmc/loom/configuration/mods/ModConfigurationRemapper.java +++ b/src/main/java/net/fabricmc/loom/configuration/mods/ModConfigurationRemapper.java @@ -259,6 +259,10 @@ public class ModConfigurationRemapper { @Nullable public static Path findSources(Project project, ResolvedArtifact artifact) { + if (isCIBuild()) { + return null; + } + final DependencyHandler dependencies = project.getDependencies(); @SuppressWarnings("unchecked") ArtifactResolutionQuery query = dependencies.createArtifactResolutionQuery() From 53112fb0b56dc86177b71db3ff4f9454279a2ff7 Mon Sep 17 00:00:00 2001 From: "J.T. McQuigg" Date: Fri, 8 Dec 2023 04:00:34 -0500 Subject: [PATCH 03/17] Replace Deprecated .getBuildDir with .getLayout().getBuildDirectory() and make Lazy (#978) * replace Deprecated .getBuildDir with .getLayout().getBuildDirectory().getAsFile().get() Signed-off-by: Joseph T. McQuigg * Make lazy Co-authored-by: modmuss * MORE Signed-off-by: Joseph T. McQuigg * remove file import Signed-off-by: Joseph T. McQuigg --------- Signed-off-by: Joseph T. McQuigg Co-authored-by: modmuss --- .../net/fabricmc/loom/extension/LoomFilesProjectImpl.java | 2 +- .../java/net/fabricmc/loom/task/RemapTaskConfiguration.java | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/main/java/net/fabricmc/loom/extension/LoomFilesProjectImpl.java b/src/main/java/net/fabricmc/loom/extension/LoomFilesProjectImpl.java index 3a5897e5..4c8811b7 100644 --- a/src/main/java/net/fabricmc/loom/extension/LoomFilesProjectImpl.java +++ b/src/main/java/net/fabricmc/loom/extension/LoomFilesProjectImpl.java @@ -53,6 +53,6 @@ public final class LoomFilesProjectImpl extends LoomFilesBaseImpl { @Override protected File getBuildDir() { - return project.getBuildDir(); + return project.getLayout().getBuildDirectory().getAsFile().get(); } } diff --git a/src/main/java/net/fabricmc/loom/task/RemapTaskConfiguration.java b/src/main/java/net/fabricmc/loom/task/RemapTaskConfiguration.java index cbd3630a..a6303454 100644 --- a/src/main/java/net/fabricmc/loom/task/RemapTaskConfiguration.java +++ b/src/main/java/net/fabricmc/loom/task/RemapTaskConfiguration.java @@ -24,8 +24,6 @@ package net.fabricmc.loom.task; -import java.io.File; - import javax.inject.Inject; import org.gradle.api.Project; @@ -92,7 +90,7 @@ public abstract class RemapTaskConfiguration implements Runnable { // Configure the default jar task getTasks().named(JavaPlugin.JAR_TASK_NAME, AbstractArchiveTask.class).configure(task -> { task.getArchiveClassifier().convention("dev"); - task.getDestinationDirectory().set(new File(getProject().getBuildDir(), "devlibs")); + task.getDestinationDirectory().set(getProject().getLayout().getBuildDirectory().map(directory -> directory.dir("devlibs"))); }); getTasks().named(BasePlugin.ASSEMBLE_TASK_NAME).configure(task -> task.dependsOn(remapJarTask)); @@ -139,7 +137,7 @@ public abstract class RemapTaskConfiguration implements Runnable { } sourcesJarTask.getArchiveClassifier().convention("dev-sources"); - sourcesJarTask.getDestinationDirectory().set(new File(getProject().getBuildDir(), "devlibs")); + sourcesJarTask.getDestinationDirectory().set(getProject().getLayout().getBuildDirectory().map(directory -> directory.dir("devlibs"))); task.getArchiveClassifier().convention("sources"); task.dependsOn(sourcesJarTask); From 7dfe800768323fc0cbcbf1454405fb65a184c8fa Mon Sep 17 00:00:00 2001 From: dicedpixels <121529979+dicedpixels@users.noreply.github.com> Date: Sat, 9 Dec 2023 19:33:02 +0530 Subject: [PATCH 04/17] Filter out realms connection error message from debug log (#991) * feat: filter out realms connection error message from debug log * fix: multiple regex filters --- src/main/resources/log4j2.fabric.xml | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/main/resources/log4j2.fabric.xml b/src/main/resources/log4j2.fabric.xml index cb8c84e2..9ebf46d5 100644 --- a/src/main/resources/log4j2.fabric.xml +++ b/src/main/resources/log4j2.fabric.xml @@ -4,8 +4,12 @@ - - + + + + + + From 3df66d612a3d10aeadb9f529e63894a17771c28f Mon Sep 17 00:00:00 2001 From: "J.T. McQuigg" Date: Sat, 9 Dec 2023 17:14:33 -0500 Subject: [PATCH 05/17] Update Github Actions (#997) --- .github/workflows/publish.yml | 2 +- .github/workflows/test-push.yml | 18 +++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 4ad1b555..df447afb 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -11,7 +11,7 @@ jobs: image: eclipse-temurin:17-jdk options: --user root steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - uses: gradle/wrapper-validation-action@v1 # Generate the build number based on tags to allow per branch build numbers, not something github provides by default. diff --git a/.github/workflows/test-push.yml b/.github/workflows/test-push.yml index fad299cd..54e2b1b0 100644 --- a/.github/workflows/test-push.yml +++ b/.github/workflows/test-push.yml @@ -16,16 +16,16 @@ jobs: image: gradle:${{ matrix.version }} options: --user root steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - uses: gradle/wrapper-validation-action@v1 - run: gradle build check -x test --stacktrace --warning-mode fail build_windows: runs-on: windows-2022 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: setup jdk - uses: actions/setup-java@v3 + uses: actions/setup-java@v4 with: java-version: 17 distribution: 'temurin' @@ -43,7 +43,7 @@ jobs: options: --user root steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - run: gradle writeActionsTestMatrix --stacktrace --warning-mode fail - id: set-matrix @@ -67,7 +67,7 @@ jobs: options: --user root steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - run: gradle printActionsTestName --name="${{ matrix.test }}" test --tests ${{ matrix.test }} --stacktrace --warning-mode fail env: TEST_WARNING_MODE: fail @@ -96,9 +96,9 @@ jobs: runs-on: windows-2022 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: setup jdk ${{ matrix.java }} - uses: actions/setup-java@v3 + uses: actions/setup-java@v4 with: java-version: ${{ matrix.java }} distribution: 'temurin' @@ -130,8 +130,8 @@ jobs: runs-on: ${{ matrix.os }} steps: - - uses: actions/checkout@v3 - - uses: actions/setup-java@v3 + - uses: actions/checkout@v4 + - uses: actions/setup-java@v4 with: java-version: ${{ matrix.java }} distribution: 'temurin' From cf8cbb245b806eb101279f4236b053ddf1439bff Mon Sep 17 00:00:00 2001 From: modmuss50 Date: Sat, 9 Dec 2023 22:27:06 +0000 Subject: [PATCH 06/17] Fix data gen folder not being added to resources. Closes https://github.com/FabricMC/fabricmc.net/issues/69 --- .../fabricmc/loom/configuration/FabricApiExtension.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/main/java/net/fabricmc/loom/configuration/FabricApiExtension.java b/src/main/java/net/fabricmc/loom/configuration/FabricApiExtension.java index 702a535b..5a82ffbc 100644 --- a/src/main/java/net/fabricmc/loom/configuration/FabricApiExtension.java +++ b/src/main/java/net/fabricmc/loom/configuration/FabricApiExtension.java @@ -28,7 +28,9 @@ import java.io.File; import java.io.UncheckedIOException; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.Map; +import java.util.Set; import javax.inject.Inject; import javax.xml.parsers.DocumentBuilder; @@ -104,7 +106,6 @@ public abstract class FabricApiExtension { DataGenerationSettings settings = getProject().getObjects().newInstance(DataGenerationSettings.class); settings.getOutputDirectory().set(getProject().file("src/main/generated")); settings.getCreateRunConfiguration().convention(true); - settings.getCreateSourceSet().convention(true); settings.getCreateSourceSet().convention(false); settings.getStrictValidation().convention(false); settings.getAddToResources().convention(true); @@ -117,7 +118,9 @@ public abstract class FabricApiExtension { if (settings.getAddToResources().get()) { mainSourceSet.resources(files -> { // Add the src/main/generated to the main sourceset's resources. - files.getSrcDirs().add(outputDirectory); + Set srcDirs = new HashSet<>(files.getSrcDirs()); + srcDirs.add(outputDirectory); + files.setSrcDirs(srcDirs); }); } From 0e9663b7a4a272f7cfd8a06942fe2029ac101833 Mon Sep 17 00:00:00 2001 From: Juuz <6596629+Juuxel@users.noreply.github.com> Date: Fri, 15 Dec 2023 11:46:53 +0200 Subject: [PATCH 07/17] Add missing Minecraft version check for mappings, fix typos (#1002) * Fix data gen folder not being added to resources. Closes https://github.com/FabricMC/fabricmc.net/issues/69 * Add missing Minecraft version check for mappings, fix typos Fixes #1001. * Add test for TinyJarInfo --------- Co-authored-by: modmuss50 --- .../mappings/MappingConfiguration.java | 4 +- .../providers/mappings/tiny/TinyJarInfo.java | 36 +++++-- .../unit/providers/TinyJarInfoTest.groovy | 97 +++++++++++++++++++ 3 files changed, 127 insertions(+), 10 deletions(-) create mode 100644 src/test/groovy/net/fabricmc/loom/test/unit/providers/TinyJarInfoTest.groovy diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/MappingConfiguration.java b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/MappingConfiguration.java index 317d4d36..a32883da 100644 --- a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/MappingConfiguration.java +++ b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/MappingConfiguration.java @@ -1,7 +1,7 @@ /* * This file is part of fabric-loom, licensed under the MIT License (MIT). * - * Copyright (c) 2016-2022 FabricMC + * 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 @@ -97,7 +97,7 @@ public class MappingConfiguration { final TinyJarInfo jarInfo = TinyJarInfo.get(inputJar); jarInfo.minecraftVersionId().ifPresent(id -> { if (!minecraftProvider.minecraftVersion().equals(id)) { - LOGGER.warn("The mappings (%s) were not build for minecraft version (%s) produce with caution.".formatted(dependency.getDepString(), minecraftProvider.minecraftVersion())); + LOGGER.warn("The mappings (%s) were not built for Minecraft version %s, proceed with caution.".formatted(dependency.getDepString(), minecraftProvider.minecraftVersion())); } }); diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/tiny/TinyJarInfo.java b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/tiny/TinyJarInfo.java index 8ea1efc1..121b444a 100644 --- a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/tiny/TinyJarInfo.java +++ b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/tiny/TinyJarInfo.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 @@ -26,29 +26,49 @@ package net.fabricmc.loom.configuration.providers.mappings.tiny; import java.io.BufferedReader; import java.io.IOException; +import java.io.InputStream; import java.io.UncheckedIOException; import java.nio.file.Files; import java.nio.file.Path; import java.util.Optional; +import java.util.jar.Manifest; import net.fabricmc.loom.util.FileSystemUtil; import net.fabricmc.mappingio.MappingReader; import net.fabricmc.mappingio.format.MappingFormat; public record TinyJarInfo(boolean v2, Optional minecraftVersionId) { + private static final String MANIFEST_PATH = "META-INF/MANIFEST.MF"; + private static final String MANIFEST_VERSION_ID_ATTRIBUTE = "Minecraft-Version-Id"; + public static TinyJarInfo get(Path jar) { - try { - return new TinyJarInfo(doesJarContainV2Mappings(jar), Optional.empty()); + try (FileSystemUtil.Delegate delegate = FileSystemUtil.getJarFileSystem(jar)) { + return new TinyJarInfo(doesJarContainV2Mappings(delegate), getMinecraftVersionId(delegate)); } catch (IOException e) { throw new UncheckedIOException("Failed to read tiny jar info", e); } } - private static boolean doesJarContainV2Mappings(Path path) throws IOException { - try (FileSystemUtil.Delegate delegate = FileSystemUtil.getJarFileSystem(path)) { - try (BufferedReader reader = Files.newBufferedReader(delegate.fs().getPath("mappings", "mappings.tiny"))) { - return MappingReader.detectFormat(reader) == MappingFormat.TINY_2_FILE; - } + private static boolean doesJarContainV2Mappings(FileSystemUtil.Delegate fs) throws IOException { + try (BufferedReader reader = Files.newBufferedReader(fs.getPath("mappings", "mappings.tiny"))) { + return MappingReader.detectFormat(reader) == MappingFormat.TINY_2_FILE; } } + + private static Optional getMinecraftVersionId(FileSystemUtil.Delegate fs) throws IOException { + final Path manifestPath = fs.getPath(MANIFEST_PATH); + + if (Files.exists(manifestPath)) { + final var manifest = new Manifest(); + + try (InputStream in = Files.newInputStream(manifestPath)) { + manifest.read(in); + } + + final String minecraftVersionId = manifest.getMainAttributes().getValue(MANIFEST_VERSION_ID_ATTRIBUTE); + return Optional.ofNullable(minecraftVersionId); + } + + return Optional.empty(); + } } diff --git a/src/test/groovy/net/fabricmc/loom/test/unit/providers/TinyJarInfoTest.groovy b/src/test/groovy/net/fabricmc/loom/test/unit/providers/TinyJarInfoTest.groovy new file mode 100644 index 00000000..df3b3c3a --- /dev/null +++ b/src/test/groovy/net/fabricmc/loom/test/unit/providers/TinyJarInfoTest.groovy @@ -0,0 +1,97 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 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.test.unit.providers + +import java.nio.file.Path +import java.util.jar.Attributes +import java.util.jar.Manifest + +import spock.lang.Specification +import spock.lang.TempDir + +import net.fabricmc.loom.configuration.providers.mappings.tiny.TinyJarInfo +import net.fabricmc.loom.util.ZipUtils + +class TinyJarInfoTest extends Specification { + @TempDir + Path tempDir + Path v1MappingsJar + Path v2MappingsJar + + def setup() { + v1MappingsJar = tempDir.resolve('mappings-v1.jar') + v2MappingsJar = tempDir.resolve('mappings-v2.jar') + ZipUtils.add(v1MappingsJar, 'mappings/mappings.tiny', 'v1\tintermediary\tnamed\n') + ZipUtils.add(v2MappingsJar, 'mappings/mappings.tiny', 'tiny\t2\t0\tintermediary\tnamed\n') + } + + def "v1 without minecraft version"() { + when: + def jarInfo = TinyJarInfo.get(v1MappingsJar) + + then: + jarInfo == new TinyJarInfo(false, Optional.empty()) + } + + def "v2 without minecraft version"() { + when: + def jarInfo = TinyJarInfo.get(v2MappingsJar) + + then: + jarInfo == new TinyJarInfo(true, Optional.empty()) + } + + def "v1 with minecraft version"() { + setup: + def manifest = new Manifest() + manifest.mainAttributes.put(Attributes.Name.MANIFEST_VERSION, '1.0') + manifest.mainAttributes.putValue('Minecraft-Version-Id', '18w50a') + def out = new ByteArrayOutputStream() + manifest.write(out) + ZipUtils.add(v1MappingsJar, 'META-INF/MANIFEST.MF', out.toByteArray()) + + when: + def jarInfo = TinyJarInfo.get(v1MappingsJar) + + then: + jarInfo == new TinyJarInfo(false, Optional.of('18w50a')) + } + + def "v2 with minecraft version"() { + setup: + def manifest = new Manifest() + manifest.mainAttributes.put(Attributes.Name.MANIFEST_VERSION, '1.0') + manifest.mainAttributes.putValue('Minecraft-Version-Id', '18w50a') + def out = new ByteArrayOutputStream() + manifest.write(out) + ZipUtils.add(v2MappingsJar, 'META-INF/MANIFEST.MF', out.toByteArray()) + + when: + def jarInfo = TinyJarInfo.get(v2MappingsJar) + + then: + jarInfo == new TinyJarInfo(true, Optional.of('18w50a')) + } +} From 014a6fce2b788fca59dc43c7757dcd81dadebcbd Mon Sep 17 00:00:00 2001 From: modmuss Date: Fri, 15 Dec 2023 09:47:26 +0000 Subject: [PATCH 08/17] Remapper extensions (#984) * Remapper extension API * Fix build * More work * Fixes, thanks Gradle. * Build fix * Cleanup --- .../fabricmc/loom/LoomGradleExtension.java | 3 + .../loom/api/LoomGradleExtensionAPI.java | 4 + .../loom/api/remapping/RemapperContext.java | 47 +++++++ .../loom/api/remapping/RemapperExtension.java | 53 ++++++++ .../api/remapping/RemapperParameters.java | 42 ++++++ .../api/remapping/TinyRemapperExtension.java | 68 ++++++++++ .../loom/configuration/mods/ModProcessor.java | 5 + .../extension/LoomGradleExtensionApiImpl.java | 24 ++++ .../extension/LoomGradleExtensionImpl.java | 5 + .../extension/RemapperExtensionHolder.java | 128 ++++++++++++++++++ .../task/service/TinyRemapperService.java | 10 +- .../test/integration/SimpleProjectTest.groovy | 2 + .../StringReplacementClassVisitor.groovy | 61 +++++++++ .../buildSrc/remapext/TestPlugin.groovy | 43 ++++++ .../remapext/TestRemapperExtension.groovy | 54 ++++++++ .../remapext/TestTinyRemapperExtension.groovy | 44 ++++++ .../java/net/fabricmc/example/ExampleMod.java | 6 +- 17 files changed, 595 insertions(+), 4 deletions(-) create mode 100644 src/main/java/net/fabricmc/loom/api/remapping/RemapperContext.java create mode 100644 src/main/java/net/fabricmc/loom/api/remapping/RemapperExtension.java create mode 100644 src/main/java/net/fabricmc/loom/api/remapping/RemapperParameters.java create mode 100644 src/main/java/net/fabricmc/loom/api/remapping/TinyRemapperExtension.java create mode 100644 src/main/java/net/fabricmc/loom/extension/RemapperExtensionHolder.java create mode 100644 src/test/groovy/net/fabricmc/loom/test/integration/buildSrc/remapext/StringReplacementClassVisitor.groovy create mode 100644 src/test/groovy/net/fabricmc/loom/test/integration/buildSrc/remapext/TestPlugin.groovy create mode 100644 src/test/groovy/net/fabricmc/loom/test/integration/buildSrc/remapext/TestRemapperExtension.groovy create mode 100644 src/test/groovy/net/fabricmc/loom/test/integration/buildSrc/remapext/TestTinyRemapperExtension.groovy diff --git a/src/main/java/net/fabricmc/loom/LoomGradleExtension.java b/src/main/java/net/fabricmc/loom/LoomGradleExtension.java index b985bd82..836138ba 100644 --- a/src/main/java/net/fabricmc/loom/LoomGradleExtension.java +++ b/src/main/java/net/fabricmc/loom/LoomGradleExtension.java @@ -45,6 +45,7 @@ import net.fabricmc.loom.configuration.providers.minecraft.mapped.IntermediaryMi import net.fabricmc.loom.configuration.providers.minecraft.mapped.NamedMinecraftProvider; import net.fabricmc.loom.extension.LoomFiles; import net.fabricmc.loom.extension.MixinExtension; +import net.fabricmc.loom.extension.RemapperExtensionHolder; import net.fabricmc.loom.util.download.DownloadBuilder; @ApiStatus.Internal @@ -115,4 +116,6 @@ public interface LoomGradleExtension extends LoomGradleExtensionAPI { boolean multiProjectOptimisation(); ListProperty getLibraryProcessors(); + + ListProperty getRemapperExtensions(); } diff --git a/src/main/java/net/fabricmc/loom/api/LoomGradleExtensionAPI.java b/src/main/java/net/fabricmc/loom/api/LoomGradleExtensionAPI.java index 78584e11..4fee840a 100644 --- a/src/main/java/net/fabricmc/loom/api/LoomGradleExtensionAPI.java +++ b/src/main/java/net/fabricmc/loom/api/LoomGradleExtensionAPI.java @@ -44,6 +44,8 @@ import net.fabricmc.loom.api.decompilers.DecompilerOptions; import net.fabricmc.loom.api.mappings.intermediate.IntermediateMappingsProvider; import net.fabricmc.loom.api.mappings.layered.spec.LayeredMappingSpecBuilder; import net.fabricmc.loom.api.processor.MinecraftJarProcessor; +import net.fabricmc.loom.api.remapping.RemapperExtension; +import net.fabricmc.loom.api.remapping.RemapperParameters; import net.fabricmc.loom.configuration.ide.RunConfigSettings; import net.fabricmc.loom.configuration.processors.JarProcessor; import net.fabricmc.loom.configuration.providers.mappings.NoOpIntermediateMappingsProvider; @@ -221,4 +223,6 @@ public interface LoomGradleExtensionAPI { Property getRuntimeOnlyLog4j(); Property getSplitModDependencies(); + + void addRemapperExtension(Class> remapperExtensionClass, Class parametersClass, Action parameterAction); } diff --git a/src/main/java/net/fabricmc/loom/api/remapping/RemapperContext.java b/src/main/java/net/fabricmc/loom/api/remapping/RemapperContext.java new file mode 100644 index 00000000..6f050263 --- /dev/null +++ b/src/main/java/net/fabricmc/loom/api/remapping/RemapperContext.java @@ -0,0 +1,47 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 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.api.remapping; + +import org.objectweb.asm.commons.Remapper; + +/** + * Context for a {@link RemapperExtension}. + */ +public interface RemapperContext { + /** + * @return The {@link Remapper} instance + */ + Remapper remapper(); + + /** + * @return the source namespace + */ + String sourceNamespace(); + + /** + * @return the target namespace + */ + String targetNamespace(); +} diff --git a/src/main/java/net/fabricmc/loom/api/remapping/RemapperExtension.java b/src/main/java/net/fabricmc/loom/api/remapping/RemapperExtension.java new file mode 100644 index 00000000..43a006e0 --- /dev/null +++ b/src/main/java/net/fabricmc/loom/api/remapping/RemapperExtension.java @@ -0,0 +1,53 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 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.api.remapping; + +import javax.inject.Inject; + +import org.gradle.api.Action; +import org.objectweb.asm.ClassVisitor; + +/** + * A remapper extension can be used to add extra processing to the remapping process. + * + *

Implementations of RemapperExtension's must have the following: + * A single constructor annotated with {@link Inject}, and taking a single argument of the parameters. + * Or a single constructor annotated with {@link Inject} taking no arguments, when the extension does not have any parameters. + * + *

Use {@link net.fabricmc.loom.api.LoomGradleExtensionAPI#addRemapperExtension(Class, Class, Action)} to register a remapper extension. + * + * @param Parameter type for the extension. Should be {@link RemapperParameters.None} if the action does not have parameters. + */ +public interface RemapperExtension { + /** + * Return a {@link ClassVisitor} that will be used when remapping the given class. + * + * @param className The name of the class being remapped + * @param remapperContext The remapper context + * @param classVisitor The parent class visitor + * @return A {@link ClassVisitor} that will be used when remapping the given class, or the given {@code classVisitor} if no extra processing is required for this class. + */ + ClassVisitor insertVisitor(String className, RemapperContext remapperContext, ClassVisitor classVisitor); +} diff --git a/src/main/java/net/fabricmc/loom/api/remapping/RemapperParameters.java b/src/main/java/net/fabricmc/loom/api/remapping/RemapperParameters.java new file mode 100644 index 00000000..32f806a7 --- /dev/null +++ b/src/main/java/net/fabricmc/loom/api/remapping/RemapperParameters.java @@ -0,0 +1,42 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 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.api.remapping; + +import org.jetbrains.annotations.ApiStatus; + +/** + * Marker interface for parameter objects to {@link RemapperExtension}s. + * + *

Design based off of Gradle's {@link org.gradle.workers.WorkParameters}. + */ +public interface RemapperParameters { + final class None implements RemapperParameters { + @ApiStatus.Internal + public static None INSTANCE = new None(); + + private None() { + } + } +} diff --git a/src/main/java/net/fabricmc/loom/api/remapping/TinyRemapperExtension.java b/src/main/java/net/fabricmc/loom/api/remapping/TinyRemapperExtension.java new file mode 100644 index 00000000..417b77f6 --- /dev/null +++ b/src/main/java/net/fabricmc/loom/api/remapping/TinyRemapperExtension.java @@ -0,0 +1,68 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 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.api.remapping; + +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.Nullable; + +import net.fabricmc.tinyremapper.TinyRemapper; + +/** + * A remapper extension, that has direct access to the TinyRemapper APIs. + * + *

This API is not stable and may change without notice. + */ +@ApiStatus.Experimental +public interface TinyRemapperExtension { + /** + * See: {@link TinyRemapper.Builder#extraAnalyzeVisitor(TinyRemapper.AnalyzeVisitorProvider)}. + * + * @return A {@link TinyRemapper.AnalyzeVisitorProvider} or {@code null}. + */ + @Nullable + default TinyRemapper.AnalyzeVisitorProvider getAnalyzeVisitorProvider() { + return null; + } + + /** + * See: {@link TinyRemapper.Builder#extraPreApplyVisitor(TinyRemapper.ApplyVisitorProvider)}. + * + * @return A {@link TinyRemapper.ApplyVisitorProvider} or {@code null}. + */ + @Nullable + default TinyRemapper.ApplyVisitorProvider getPreApplyVisitor() { + return null; + } + + /** + * See: {@link TinyRemapper.Builder#extraPostApplyVisitor(TinyRemapper.ApplyVisitorProvider)}. + * + * @return A {@link TinyRemapper.ApplyVisitorProvider} or {@code null}. + */ + @Nullable + default TinyRemapper.ApplyVisitorProvider getPostApplyVisitor() { + return null; + } +} diff --git a/src/main/java/net/fabricmc/loom/configuration/mods/ModProcessor.java b/src/main/java/net/fabricmc/loom/configuration/mods/ModProcessor.java index a4cf4503..b961df48 100644 --- a/src/main/java/net/fabricmc/loom/configuration/mods/ModProcessor.java +++ b/src/main/java/net/fabricmc/loom/configuration/mods/ModProcessor.java @@ -51,6 +51,7 @@ import net.fabricmc.loom.api.RemapConfigurationSettings; import net.fabricmc.loom.api.mappings.layered.MappingsNamespace; import net.fabricmc.loom.configuration.mods.dependency.ModDependency; import net.fabricmc.loom.configuration.providers.mappings.MappingConfiguration; +import net.fabricmc.loom.extension.RemapperExtensionHolder; import net.fabricmc.loom.util.Constants; import net.fabricmc.loom.util.Pair; import net.fabricmc.loom.util.TinyRemapperHelper; @@ -157,6 +158,10 @@ public class ModProcessor { builder.extension(new MixinExtension(remapMixins::contains)); } + for (RemapperExtensionHolder holder : extension.getRemapperExtensions().get()) { + holder.apply(builder, fromM, toM, project.getObjects()); + } + final TinyRemapper remapper = builder.build(); for (Path minecraftJar : extension.getMinecraftJars(MappingsNamespace.INTERMEDIARY)) { diff --git a/src/main/java/net/fabricmc/loom/extension/LoomGradleExtensionApiImpl.java b/src/main/java/net/fabricmc/loom/extension/LoomGradleExtensionApiImpl.java index f8d492ed..0bf6e98e 100644 --- a/src/main/java/net/fabricmc/loom/extension/LoomGradleExtensionApiImpl.java +++ b/src/main/java/net/fabricmc/loom/extension/LoomGradleExtensionApiImpl.java @@ -34,6 +34,7 @@ import org.gradle.api.Project; import org.gradle.api.artifacts.Dependency; import org.gradle.api.file.ConfigurableFileCollection; import org.gradle.api.file.RegularFileProperty; +import org.gradle.api.model.ObjectFactory; import org.gradle.api.provider.ListProperty; import org.gradle.api.provider.Property; import org.gradle.api.provider.SetProperty; @@ -50,6 +51,8 @@ import net.fabricmc.loom.api.decompilers.DecompilerOptions; import net.fabricmc.loom.api.mappings.intermediate.IntermediateMappingsProvider; import net.fabricmc.loom.api.mappings.layered.spec.LayeredMappingSpecBuilder; import net.fabricmc.loom.api.processor.MinecraftJarProcessor; +import net.fabricmc.loom.api.remapping.RemapperExtension; +import net.fabricmc.loom.api.remapping.RemapperParameters; import net.fabricmc.loom.configuration.RemapConfigurations; import net.fabricmc.loom.configuration.ide.RunConfigSettings; import net.fabricmc.loom.configuration.processors.JarProcessor; @@ -91,6 +94,7 @@ public abstract class LoomGradleExtensionApiImpl implements LoomGradleExtensionA private final NamedDomainObjectContainer mods; private final NamedDomainObjectList remapConfigurations; private final ListProperty> minecraftJarProcessors; + protected final ListProperty remapperExtensions; // A common mistake with layered mappings is to call the wrong `officialMojangMappings` method, use this to keep track of when we are building a layered mapping spec. protected final ThreadLocal layeredSpecBuilderScope = ThreadLocal.withInitial(() -> false); @@ -150,6 +154,9 @@ public abstract class LoomGradleExtensionApiImpl implements LoomGradleExtensionA this.splitEnvironmentalSourceSet = project.getObjects().property(Boolean.class).convention(false); this.splitEnvironmentalSourceSet.finalizeValueOnRead(); + remapperExtensions = project.getObjects().listProperty(RemapperExtensionHolder.class); + remapperExtensions.finalizeValueOnRead(); + // Enable dep iface injection by default interfaceInjection(interfaceInjection -> { interfaceInjection.getEnableDependencyInterfaceInjection().convention(true).finalizeValueOnRead(); @@ -382,6 +389,23 @@ public abstract class LoomGradleExtensionApiImpl implements LoomGradleExtensionA RemapConfigurations.setupForSourceSet(getProject(), sourceSet); } + @Override + public void addRemapperExtension(Class> remapperExtensionClass, Class parametersClass, Action parameterAction) { + final ObjectFactory objectFactory = getProject().getObjects(); + final RemapperExtensionHolder holder; + + if (parametersClass != RemapperParameters.None.class) { + T parameters = objectFactory.newInstance(parametersClass); + parameterAction.execute(parameters); + holder = objectFactory.newInstance(RemapperExtensionHolder.class, parameters); + } else { + holder = objectFactory.newInstance(RemapperExtensionHolder.class, RemapperParameters.None.INSTANCE); + } + + holder.getRemapperExtensionClassName().set(remapperExtensionClass.getName()); + remapperExtensions.add(holder); + } + // This is here to ensure that LoomGradleExtensionApiImpl compiles without any unimplemented methods private final class EnsureCompile extends LoomGradleExtensionApiImpl { private EnsureCompile() { diff --git a/src/main/java/net/fabricmc/loom/extension/LoomGradleExtensionImpl.java b/src/main/java/net/fabricmc/loom/extension/LoomGradleExtensionImpl.java index 81c5c793..7fe809fc 100644 --- a/src/main/java/net/fabricmc/loom/extension/LoomGradleExtensionImpl.java +++ b/src/main/java/net/fabricmc/loom/extension/LoomGradleExtensionImpl.java @@ -247,6 +247,11 @@ public class LoomGradleExtensionImpl extends LoomGradleExtensionApiImpl implemen return libraryProcessorFactories; } + @Override + public ListProperty getRemapperExtensions() { + return remapperExtensions; + } + @Override protected void configureIntermediateMappingsProviderInternal(T provider) { provider.getMinecraftVersion().set(getProject().provider(() -> getMinecraftProvider().minecraftVersion())); diff --git a/src/main/java/net/fabricmc/loom/extension/RemapperExtensionHolder.java b/src/main/java/net/fabricmc/loom/extension/RemapperExtensionHolder.java new file mode 100644 index 00000000..454e75ec --- /dev/null +++ b/src/main/java/net/fabricmc/loom/extension/RemapperExtensionHolder.java @@ -0,0 +1,128 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 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.extension; + +import javax.inject.Inject; + +import org.gradle.api.model.ObjectFactory; +import org.gradle.api.provider.Property; +import org.gradle.api.tasks.Input; +import org.gradle.api.tasks.Nested; +import org.gradle.api.tasks.Optional; +import org.jetbrains.annotations.Nullable; +import org.objectweb.asm.ClassVisitor; +import org.objectweb.asm.commons.Remapper; + +import net.fabricmc.loom.api.remapping.RemapperContext; +import net.fabricmc.loom.api.remapping.RemapperExtension; +import net.fabricmc.loom.api.remapping.RemapperParameters; +import net.fabricmc.loom.api.remapping.TinyRemapperExtension; +import net.fabricmc.tinyremapper.TinyRemapper; +import net.fabricmc.tinyremapper.api.TrClass; + +public abstract class RemapperExtensionHolder { + // Null when RemapperParameters.None.class + private final RemapperParameters remapperParameters; + + @Inject + public RemapperExtensionHolder(RemapperParameters remapperParameters) { + this.remapperParameters = remapperParameters; + } + + @Input + public abstract Property getRemapperExtensionClassName(); + + @Nested + @Optional + public RemapperParameters getRemapperParameters() { + return remapperParameters; + } + + public void apply(TinyRemapper.Builder tinyRemapperBuilder, String sourceNamespace, String targetNamespace, ObjectFactory objectFactory) { + final RemapperExtension remapperExtension = newInstance(objectFactory); + + tinyRemapperBuilder.extraPostApplyVisitor(new RemapperExtensionImpl(remapperExtension, sourceNamespace, targetNamespace)); + + if (remapperExtension instanceof TinyRemapperExtension tinyRemapperExtension) { + final TinyRemapper.AnalyzeVisitorProvider analyzeVisitorProvider = tinyRemapperExtension.getAnalyzeVisitorProvider(); + final TinyRemapper.ApplyVisitorProvider preApplyVisitorProvider = tinyRemapperExtension.getPreApplyVisitor(); + final TinyRemapper.ApplyVisitorProvider postApplyVisitorProvider = tinyRemapperExtension.getPostApplyVisitor(); + + if (analyzeVisitorProvider != null) { + tinyRemapperBuilder.extraAnalyzeVisitor(analyzeVisitorProvider); + } + + if (preApplyVisitorProvider != null) { + tinyRemapperBuilder.extraPreApplyVisitor(preApplyVisitorProvider); + } + + if (postApplyVisitorProvider != null) { + tinyRemapperBuilder.extraPostApplyVisitor(postApplyVisitorProvider); + } + } + } + + private RemapperExtension newInstance(ObjectFactory objectFactory) { + try { + Class remapperExtensionClass = Class.forName(getRemapperExtensionClassName().get()) + .asSubclass(RemapperExtension.class); + + if (remapperParameters == RemapperParameters.None.INSTANCE) { + return objectFactory.newInstance(remapperExtensionClass); + } + + return objectFactory.newInstance(remapperExtensionClass, remapperParameters); + } catch (Exception e) { + throw new RuntimeException("Failed to create remapper extension", e); + } + } + + private static final class RemapperExtensionImpl implements TinyRemapper.ApplyVisitorProvider { + private final RemapperExtension remapperExtension; + private final String sourceNamespace; + private final String targetNamespace; + + @Nullable + private RemapperContext context; + + private RemapperExtensionImpl(RemapperExtension remapperExtension, String sourceNamespace, String targetNamespace) { + this.remapperExtension = remapperExtension; + this.sourceNamespace = sourceNamespace; + this.targetNamespace = targetNamespace; + } + + @Override + public ClassVisitor insertApplyVisitor(TrClass cls, ClassVisitor next) { + if (context == null) { + context = new RemapperContextImpl(cls.getEnvironment().getRemapper(), sourceNamespace, targetNamespace); + } + + return remapperExtension.insertVisitor(cls.getName(), context, next); + } + } + + private record RemapperContextImpl(Remapper remapper, String sourceNamespace, String targetNamespace) implements RemapperContext { + } +} diff --git a/src/main/java/net/fabricmc/loom/task/service/TinyRemapperService.java b/src/main/java/net/fabricmc/loom/task/service/TinyRemapperService.java index 3e1eee87..d576e07b 100644 --- a/src/main/java/net/fabricmc/loom/task/service/TinyRemapperService.java +++ b/src/main/java/net/fabricmc/loom/task/service/TinyRemapperService.java @@ -39,11 +39,13 @@ import java.util.StringJoiner; import org.gradle.api.Project; import org.gradle.api.invocation.Gradle; +import org.gradle.api.model.ObjectFactory; import org.gradle.api.tasks.SourceSet; import org.jetbrains.annotations.Nullable; import net.fabricmc.loom.LoomGradleExtension; import net.fabricmc.loom.build.mixin.AnnotationProcessorInvoker; +import net.fabricmc.loom.extension.RemapperExtensionHolder; import net.fabricmc.loom.task.AbstractRemapJarTask; import net.fabricmc.loom.util.gradle.GradleUtils; import net.fabricmc.loom.util.gradle.SourceSetHelper; @@ -90,7 +92,7 @@ public class TinyRemapperService implements SharedService { mappings.add(gradleMixinMappingProvider(serviceManager, project.getGradle(), extension.getMappingConfiguration().mappingsIdentifier, from, to)); } - return new TinyRemapperService(mappings, !legacyMixin, kotlinClasspathService, extension.getKnownIndyBsms().get()); + return new TinyRemapperService(mappings, !legacyMixin, kotlinClasspathService, extension.getKnownIndyBsms().get(), extension.getRemapperExtensions().get(), from, to, project.getObjects()); }); service.readClasspath(remapJarTask.getClasspath().getFiles().stream().map(File::toPath).filter(Files::exists).toList()); @@ -129,7 +131,7 @@ public class TinyRemapperService implements SharedService { // Set to true once remapping has started, once set no inputs can be read. private boolean isRemapping = false; - public TinyRemapperService(List mappings, boolean useMixinExtension, @Nullable KotlinClasspath kotlinClasspath, Set knownIndyBsms) { + private TinyRemapperService(List mappings, boolean useMixinExtension, @Nullable KotlinClasspath kotlinClasspath, Set knownIndyBsms, List remapperExtensions, String sourceNamespace, String targetNamespace, ObjectFactory objectFactory) { TinyRemapper.Builder builder = TinyRemapper.newRemapper().withKnownIndyBsm(knownIndyBsms); for (IMappingProvider provider : mappings) { @@ -145,6 +147,10 @@ public class TinyRemapperService implements SharedService { builder.extension(kotlinRemapperClassloader.getTinyRemapperExtension()); } + for (RemapperExtensionHolder holder : remapperExtensions) { + holder.apply(builder, sourceNamespace, targetNamespace, objectFactory); + } + tinyRemapper = builder.build(); } diff --git a/src/test/groovy/net/fabricmc/loom/test/integration/SimpleProjectTest.groovy b/src/test/groovy/net/fabricmc/loom/test/integration/SimpleProjectTest.groovy index 3fc12edd..ff7bda77 100644 --- a/src/test/groovy/net/fabricmc/loom/test/integration/SimpleProjectTest.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/integration/SimpleProjectTest.groovy @@ -42,6 +42,7 @@ class SimpleProjectTest extends Specification implements GradleProjectTestTrait def "build and run (gradle #version)"() { setup: def gradle = gradleProject(project: "simple", version: version) + gradle.buildSrc("remapext") // apply the remap extension plugin def server = ServerRunner.create(gradle.projectDir, "1.16.5") .withMod(gradle.getOutputFile("fabric-example-mod-1.0.0.jar")) @@ -60,6 +61,7 @@ class SimpleProjectTest extends Specification implements GradleProjectTestTrait serverResult.successful() serverResult.output.contains("Hello simple Fabric mod") // A check to ensure our mod init was actually called + serverResult.output.contains("Hello Loom!") // Check that the remapper extension worked where: version << STANDARD_TEST_VERSIONS } diff --git a/src/test/groovy/net/fabricmc/loom/test/integration/buildSrc/remapext/StringReplacementClassVisitor.groovy b/src/test/groovy/net/fabricmc/loom/test/integration/buildSrc/remapext/StringReplacementClassVisitor.groovy new file mode 100644 index 00000000..b84501e9 --- /dev/null +++ b/src/test/groovy/net/fabricmc/loom/test/integration/buildSrc/remapext/StringReplacementClassVisitor.groovy @@ -0,0 +1,61 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 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.test.integration.buildSrc.remapext + +import org.objectweb.asm.ClassVisitor +import org.objectweb.asm.MethodVisitor + +class StringReplacementClassVisitor extends ClassVisitor { + final Map replacements + + StringReplacementClassVisitor(int api, ClassVisitor classVisitor, Map replacements) { + super(api, classVisitor) + this.replacements = replacements + } + + @Override + MethodVisitor visitMethod(int access, String name, String descriptor, String signature, String[] exceptions) { + def methodVisitor = super.visitMethod(access, name, descriptor, signature, exceptions) + return new StringReplacementMethodVisitor(api, methodVisitor) + } + + class StringReplacementMethodVisitor extends MethodVisitor { + StringReplacementMethodVisitor(int api, MethodVisitor methodVisitor) { + super(api, methodVisitor) + } + + @Override + void visitLdcInsn(Object value) { + if (value instanceof String) { + String replacement = replacements.get(value) + if (replacement != null) { + value = replacement + } + } + + super.visitLdcInsn(value) + } + } +} diff --git a/src/test/groovy/net/fabricmc/loom/test/integration/buildSrc/remapext/TestPlugin.groovy b/src/test/groovy/net/fabricmc/loom/test/integration/buildSrc/remapext/TestPlugin.groovy new file mode 100644 index 00000000..c3084c91 --- /dev/null +++ b/src/test/groovy/net/fabricmc/loom/test/integration/buildSrc/remapext/TestPlugin.groovy @@ -0,0 +1,43 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 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.test.integration.buildSrc.remapext + +import org.gradle.api.Plugin +import org.gradle.api.Project + +import net.fabricmc.loom.LoomGradleExtension +import net.fabricmc.loom.api.remapping.RemapperParameters + +class TestPlugin implements Plugin { + @Override + void apply(Project project) { + def extension = LoomGradleExtension.get(project) + extension.addRemapperExtension(TestRemapperExtension.class, TestRemapperExtension.Params.class) { TestRemapperExtension.Params p -> + p.replacements.put("Hello World!", "Hello Loom!") + } + extension.addRemapperExtension(TestTinyRemapperExtension.class, RemapperParameters.None.class) { RemapperParameters.None p -> + } + } +} diff --git a/src/test/groovy/net/fabricmc/loom/test/integration/buildSrc/remapext/TestRemapperExtension.groovy b/src/test/groovy/net/fabricmc/loom/test/integration/buildSrc/remapext/TestRemapperExtension.groovy new file mode 100644 index 00000000..7f71841d --- /dev/null +++ b/src/test/groovy/net/fabricmc/loom/test/integration/buildSrc/remapext/TestRemapperExtension.groovy @@ -0,0 +1,54 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 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.test.integration.buildSrc.remapext + +import javax.inject.Inject + +import org.gradle.api.provider.MapProperty +import org.objectweb.asm.ClassVisitor + +import net.fabricmc.loom.api.remapping.RemapperContext +import net.fabricmc.loom.api.remapping.RemapperExtension +import net.fabricmc.loom.api.remapping.RemapperParameters +import net.fabricmc.loom.util.Constants + +abstract class TestRemapperExtension implements RemapperExtension { + final Params parameters + + @Inject + TestRemapperExtension(Params parameters) { + this.parameters = parameters + } + + @Override + ClassVisitor insertVisitor(String className, RemapperContext remapperContext, ClassVisitor classVisitor) { + def replacements = parameters.replacements.get() + return new StringReplacementClassVisitor(Constants.ASM_VERSION, classVisitor, replacements) + } + + interface Params extends RemapperParameters { + MapProperty getReplacements() + } +} diff --git a/src/test/groovy/net/fabricmc/loom/test/integration/buildSrc/remapext/TestTinyRemapperExtension.groovy b/src/test/groovy/net/fabricmc/loom/test/integration/buildSrc/remapext/TestTinyRemapperExtension.groovy new file mode 100644 index 00000000..412c4d1a --- /dev/null +++ b/src/test/groovy/net/fabricmc/loom/test/integration/buildSrc/remapext/TestTinyRemapperExtension.groovy @@ -0,0 +1,44 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 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.test.integration.buildSrc.remapext + +import org.objectweb.asm.ClassVisitor + +import net.fabricmc.loom.api.remapping.RemapperContext +import net.fabricmc.loom.api.remapping.RemapperExtension +import net.fabricmc.loom.api.remapping.RemapperParameters +import net.fabricmc.loom.api.remapping.TinyRemapperExtension +import net.fabricmc.tinyremapper.TinyRemapper + +class TestTinyRemapperExtension implements RemapperExtension, TinyRemapperExtension { + @Override + ClassVisitor insertVisitor(String className, RemapperContext remapperContext, ClassVisitor classVisitor) { + return classVisitor + } + + TinyRemapper.AnalyzeVisitorProvider analyzeVisitorProvider = null + TinyRemapper.ApplyVisitorProvider preApplyVisitor = null + TinyRemapper.ApplyVisitorProvider PostApplyVisitor = null +} diff --git a/src/test/resources/projects/simple/src/main/java/net/fabricmc/example/ExampleMod.java b/src/test/resources/projects/simple/src/main/java/net/fabricmc/example/ExampleMod.java index 5330d119..ab456d38 100644 --- a/src/test/resources/projects/simple/src/main/java/net/fabricmc/example/ExampleMod.java +++ b/src/test/resources/projects/simple/src/main/java/net/fabricmc/example/ExampleMod.java @@ -1,9 +1,10 @@ package net.fabricmc.example; -import net.fabricmc.api.ModInitializer; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import net.fabricmc.api.ModInitializer; + public class ExampleMod implements ModInitializer { public static final Logger LOGGER = LogManager.getLogger("modid"); @@ -13,5 +14,6 @@ public class ExampleMod implements ModInitializer { @Override public void onInitialize() { LOGGER.info("Hello simple Fabric mod!"); + LOGGER.info("Hello World!"); } -} \ No newline at end of file +} From f2e8ff2cef55d1191f836a98415fe5fd34bb3199 Mon Sep 17 00:00:00 2001 From: Jamalam Date: Fri, 15 Dec 2023 09:50:25 +0000 Subject: [PATCH 09/17] Check refmaps exist before adding them to mixin configs (#971) --- .../java/net/fabricmc/loom/task/RemapJarTask.java | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/main/java/net/fabricmc/loom/task/RemapJarTask.java b/src/main/java/net/fabricmc/loom/task/RemapJarTask.java index fbc6e720..ca405b28 100644 --- a/src/main/java/net/fabricmc/loom/task/RemapJarTask.java +++ b/src/main/java/net/fabricmc/loom/task/RemapJarTask.java @@ -345,13 +345,15 @@ public abstract class RemapJarTask extends AbstractRemapJarTask { } for (RemapParams.RefmapData refmapData : getParameters().getMixinData().get()) { - int transformed = ZipUtils.transformJson(JsonObject.class, outputFile, refmapData.mixinConfigs().stream().collect(Collectors.toMap(s -> s, s -> json -> { - if (!json.has("refmap")) { - json.addProperty("refmap", refmapData.refmapName()); - } + if (ZipUtils.contains(outputFile, refmapData.refmapName())) { + int transformed = ZipUtils.transformJson(JsonObject.class, outputFile, refmapData.mixinConfigs().stream().collect(Collectors.toMap(s -> s, s -> json -> { + if (!json.has("refmap")) { + json.addProperty("refmap", refmapData.refmapName()); + } - return json; - }))); + return json; + }))); + } } } } From 6f38d5f2e80c26c839c0de2e2c846c6a02fdf5b0 Mon Sep 17 00:00:00 2001 From: Jason Penilla <11360596+jpenilla@users.noreply.github.com> Date: Tue, 19 Dec 2023 05:46:35 -0800 Subject: [PATCH 10/17] Change how `include` disables transitive dependencies to allow platform dependencies to work (#838) * Change how `include` disables transitive dependencies to allow platform dependencies to work * style fix * Cleanup and add test * spotlessApply --------- Co-authored-by: modmuss50 --- .../configuration/LoomConfigurations.java | 3 +- .../test/integration/IncludedJarsTest.groovy | 58 +++++++++++++++++++ .../test/integration/MultiProjectTest.groovy | 2 +- .../projects/includedJars/build.gradle | 21 +++++++ .../projects/includedJars/settings.gradle | 1 + .../src/main/resources/fabric.mod.json | 9 +++ 6 files changed, 92 insertions(+), 2 deletions(-) create mode 100644 src/test/groovy/net/fabricmc/loom/test/integration/IncludedJarsTest.groovy create mode 100644 src/test/resources/projects/includedJars/build.gradle create mode 100644 src/test/resources/projects/includedJars/settings.gradle create mode 100644 src/test/resources/projects/includedJars/src/main/resources/fabric.mod.json diff --git a/src/main/java/net/fabricmc/loom/configuration/LoomConfigurations.java b/src/main/java/net/fabricmc/loom/configuration/LoomConfigurations.java index 9d7008f3..16d98b73 100644 --- a/src/main/java/net/fabricmc/loom/configuration/LoomConfigurations.java +++ b/src/main/java/net/fabricmc/loom/configuration/LoomConfigurations.java @@ -81,7 +81,8 @@ public abstract class LoomConfigurations implements Runnable { registerNonTransitive(Constants.Configurations.LOADER_DEPENDENCIES, Role.RESOLVABLE); registerNonTransitive(Constants.Configurations.MINECRAFT, Role.NONE); - registerNonTransitive(Constants.Configurations.INCLUDE, Role.RESOLVABLE); + // We don't need to make this non-transitive due to the way we resolve it. Also, doing so would break platform dependencies. + register(Constants.Configurations.INCLUDE, Role.RESOLVABLE); registerNonTransitive(Constants.Configurations.MAPPING_CONSTANTS, Role.RESOLVABLE); register(Constants.Configurations.NAMED_ELEMENTS, Role.CONSUMABLE).configure(configuration -> { diff --git a/src/test/groovy/net/fabricmc/loom/test/integration/IncludedJarsTest.groovy b/src/test/groovy/net/fabricmc/loom/test/integration/IncludedJarsTest.groovy new file mode 100644 index 00000000..457b61f8 --- /dev/null +++ b/src/test/groovy/net/fabricmc/loom/test/integration/IncludedJarsTest.groovy @@ -0,0 +1,58 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 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.test.integration + +import spock.lang.Specification +import spock.lang.Unroll + +import net.fabricmc.loom.test.util.GradleProjectTestTrait + +import static net.fabricmc.loom.test.LoomTestConstants.STANDARD_TEST_VERSIONS +import static org.gradle.testkit.runner.TaskOutcome.SUCCESS + +class IncludedJarsTest extends Specification implements GradleProjectTestTrait { + @Unroll + def "included jars (gradle #version)"() { + setup: + def gradle = gradleProject(project: "includedJars", version: version) + + when: + def result = gradle.run(tasks: ["remapJar"]) + + then: + result.task(":remapJar").outcome == SUCCESS + + // Assert directly declared dependencies are present + gradle.hasOutputZipEntry("includedJars.jar", "META-INF/jars/log4j-core-2.22.0.jar") + gradle.hasOutputZipEntry("includedJars.jar", "META-INF/jars/adventure-text-serializer-gson-4.14.0.jar") + + // But not transitives. + !gradle.hasOutputZipEntry("includedJars.jar", "META-INF/jars/log4j-api-2.22.0.jar") + !gradle.hasOutputZipEntry("includedJars.jar", "META-INF/jars/adventure-api-4.14.0.jar") + + where: + version << STANDARD_TEST_VERSIONS + } +} diff --git a/src/test/groovy/net/fabricmc/loom/test/integration/MultiProjectTest.groovy b/src/test/groovy/net/fabricmc/loom/test/integration/MultiProjectTest.groovy index da866213..c5669df0 100644 --- a/src/test/groovy/net/fabricmc/loom/test/integration/MultiProjectTest.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/integration/MultiProjectTest.groovy @@ -53,7 +53,7 @@ class MultiProjectTest extends Specification implements GradleProjectTestTrait { gradle.hasOutputZipEntry("multiproject-1.0.0.jar", "META-INF/jars/example-1.0.0.jar") gradle.hasOutputZipEntry("multiproject-1.0.0.jar", "META-INF/jars/core-1.0.0.jar") - gradle.hasOutputZipEntry("multiproject-1.0.0.jar", "META-INF/jars/fabric-api-base-0.2.1+9354966b7d.jar") + gradle.hasOutputZipEntry("multiproject-1.0.0.jar", "META-INF/jars/fabric-api-base-0.3.0+f74f7c7d7d.jar") where: version << STANDARD_TEST_VERSIONS diff --git a/src/test/resources/projects/includedJars/build.gradle b/src/test/resources/projects/includedJars/build.gradle new file mode 100644 index 00000000..5808e9a5 --- /dev/null +++ b/src/test/resources/projects/includedJars/build.gradle @@ -0,0 +1,21 @@ +plugins { + id 'fabric-loom' +} + +repositories { + mavenCentral() +} + +dependencies { + minecraft 'com.mojang:minecraft:1.18.2' + mappings 'net.fabricmc:yarn:1.18.2+build.1:v2' + modImplementation 'net.fabricmc:fabric-loader:0.13.3' + + include 'org.apache.logging.log4j:log4j-core:2.22.0' + + // Test bom/platform dependencies + include platform('net.kyori:adventure-bom:4.14.0') + + // bom provides version + include 'net.kyori:adventure-text-serializer-gson' +} diff --git a/src/test/resources/projects/includedJars/settings.gradle b/src/test/resources/projects/includedJars/settings.gradle new file mode 100644 index 00000000..7fdfcf73 --- /dev/null +++ b/src/test/resources/projects/includedJars/settings.gradle @@ -0,0 +1 @@ +rootProject.name = 'includedJars' diff --git a/src/test/resources/projects/includedJars/src/main/resources/fabric.mod.json b/src/test/resources/projects/includedJars/src/main/resources/fabric.mod.json new file mode 100644 index 00000000..33bad8c2 --- /dev/null +++ b/src/test/resources/projects/includedJars/src/main/resources/fabric.mod.json @@ -0,0 +1,9 @@ +{ + "schemaVersion": 1, + "id": "modid", + "version": "0.0.0", + + "name": "Example Mod", + "description": "This is an example description! Tell everyone what your mod is about!", + "environment": "*" +} From bbf7f96b410ad3d7c0db559958eb2147fe75ea97 Mon Sep 17 00:00:00 2001 From: Matt Sturgeon Date: Tue, 19 Dec 2023 16:25:19 +0000 Subject: [PATCH 11/17] Allow disabling RunConfig appending project path (#1005) * Fix data gen folder not being added to resources. Closes https://github.com/FabricMC/fabricmc.net/issues/69 * Allow disabling RunConfig appending project path Add a `appendConfigNameWithPath` property to `RunConfigSettings` controlling whether to append the path for non-root projects. Default behaviour is unchanged. --------- Co-authored-by: modmuss50 --- .../loom/configuration/ide/RunConfig.java | 10 +++++++--- .../configuration/ide/RunConfigSettings.java | 20 +++++++++++++++++++ 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/src/main/java/net/fabricmc/loom/configuration/ide/RunConfig.java b/src/main/java/net/fabricmc/loom/configuration/ide/RunConfig.java index 1783e903..233f00a3 100644 --- a/src/main/java/net/fabricmc/loom/configuration/ide/RunConfig.java +++ b/src/main/java/net/fabricmc/loom/configuration/ide/RunConfig.java @@ -111,8 +111,11 @@ public class RunConfig { return e; } - private static void populate(Project project, LoomGradleExtension extension, RunConfig runConfig, String environment) { - runConfig.configName += extension.isRootProject() ? "" : " (" + project.getPath() + ")"; + private static void populate(Project project, LoomGradleExtension extension, RunConfig runConfig, String environment, boolean appendProjectPath) { + if (appendProjectPath && !extension.isRootProject()) { + runConfig.configName += " (" + project.getPath() + ")"; + } + runConfig.eclipseProjectName = project.getExtensions().getByType(EclipseModel.class).getProject().getName(); runConfig.mainClass = "net.fabricmc.devlaunchinjector.Main"; @@ -167,9 +170,10 @@ public class RunConfig { runDir = "run"; } + boolean appendProjectPath = settings.getAppendProjectPathToConfigName(); RunConfig runConfig = new RunConfig(); runConfig.configName = configName; - populate(project, extension, runConfig, environment); + populate(project, extension, runConfig, environment, appendProjectPath); runConfig.ideaModuleName = IdeaUtils.getIdeaModuleName(new SourceSetReference(sourceSet, project)); runConfig.runDirIdeaUrl = "file://$PROJECT_DIR$/" + runDir; runConfig.runDir = runDir; diff --git a/src/main/java/net/fabricmc/loom/configuration/ide/RunConfigSettings.java b/src/main/java/net/fabricmc/loom/configuration/ide/RunConfigSettings.java index 9b8bdf02..3357c741 100644 --- a/src/main/java/net/fabricmc/loom/configuration/ide/RunConfigSettings.java +++ b/src/main/java/net/fabricmc/loom/configuration/ide/RunConfigSettings.java @@ -67,9 +67,20 @@ public class RunConfigSettings implements Named { * The full name of the run configuration, i.e. 'Minecraft Client'. * *

By default this is determined from the base name. + * + *

Note: unless the project is the root project (or {@link #appendProjectPathToConfigName} is disabled), + * the project path will be appended automatically, e.g. 'Minecraft Client (:some:project)'. */ private String configName; + /** + * Whether to append the project path to the {@link #configName} when {@code project} isn't the root project. + * + *

Warning: could produce ambiguous run config names if disabled, unless used carefully in conjunction with + * {@link #configName}. + */ + private boolean appendProjectPathToConfigName; + /** * The default main class of the run configuration. * @@ -117,6 +128,7 @@ public class RunConfigSettings implements Named { public RunConfigSettings(Project project, String name) { this.name = name; this.project = project; + this.appendProjectPathToConfigName = true; this.extension = LoomGradleExtension.get(project); this.ideConfigGenerated = extension.isRootProject(); this.mainClass = project.getObjects().property(String.class).convention(project.provider(() -> { @@ -174,6 +186,14 @@ public class RunConfigSettings implements Named { this.configName = name; } + public boolean getAppendProjectPathToConfigName() { + return appendProjectPathToConfigName; + } + + public void setAppendProjectPathToConfigName(boolean append) { + appendProjectPathToConfigName = append; + } + public String getDefaultMainClass() { return defaultMainClass; } From a6547244e96c1e1cbffa6a47719dc69eae66a576 Mon Sep 17 00:00:00 2001 From: modmuss50 Date: Tue, 19 Dec 2023 16:31:35 +0000 Subject: [PATCH 12/17] Add loom.getMinecraftVersion() Closes #982 --- .../java/net/fabricmc/loom/api/LoomGradleExtensionAPI.java | 6 ++++++ .../fabricmc/loom/extension/LoomGradleExtensionApiImpl.java | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/src/main/java/net/fabricmc/loom/api/LoomGradleExtensionAPI.java b/src/main/java/net/fabricmc/loom/api/LoomGradleExtensionAPI.java index 4fee840a..ef647b9b 100644 --- a/src/main/java/net/fabricmc/loom/api/LoomGradleExtensionAPI.java +++ b/src/main/java/net/fabricmc/loom/api/LoomGradleExtensionAPI.java @@ -35,6 +35,7 @@ import org.gradle.api.file.ConfigurableFileCollection; import org.gradle.api.file.RegularFileProperty; import org.gradle.api.provider.ListProperty; import org.gradle.api.provider.Property; +import org.gradle.api.provider.Provider; import org.gradle.api.provider.SetProperty; import org.gradle.api.publish.maven.MavenPublication; import org.gradle.api.tasks.SourceSet; @@ -225,4 +226,9 @@ public interface LoomGradleExtensionAPI { Property getSplitModDependencies(); void addRemapperExtension(Class> remapperExtensionClass, Class parametersClass, Action parameterAction); + + /** + * @return The minecraft version, as a {@link Provider}. + */ + Provider getMinecraftVersion(); } diff --git a/src/main/java/net/fabricmc/loom/extension/LoomGradleExtensionApiImpl.java b/src/main/java/net/fabricmc/loom/extension/LoomGradleExtensionApiImpl.java index 0bf6e98e..87875f31 100644 --- a/src/main/java/net/fabricmc/loom/extension/LoomGradleExtensionApiImpl.java +++ b/src/main/java/net/fabricmc/loom/extension/LoomGradleExtensionApiImpl.java @@ -37,6 +37,7 @@ import org.gradle.api.file.RegularFileProperty; import org.gradle.api.model.ObjectFactory; import org.gradle.api.provider.ListProperty; import org.gradle.api.provider.Property; +import org.gradle.api.provider.Provider; import org.gradle.api.provider.SetProperty; import org.gradle.api.publish.maven.MavenPublication; import org.gradle.api.tasks.SourceSet; @@ -406,6 +407,11 @@ public abstract class LoomGradleExtensionApiImpl implements LoomGradleExtensionA remapperExtensions.add(holder); } + @Override + public Provider getMinecraftVersion() { + return getProject().provider(() -> LoomGradleExtension.get(getProject()).getMinecraftProvider().minecraftVersion()); + } + // This is here to ensure that LoomGradleExtensionApiImpl compiles without any unimplemented methods private final class EnsureCompile extends LoomGradleExtensionApiImpl { private EnsureCompile() { From e980ee60ade2a89877f1ed9f76ea3a6cd957fae2 Mon Sep 17 00:00:00 2001 From: modmuss50 Date: Tue, 19 Dec 2023 16:49:27 +0000 Subject: [PATCH 13/17] Use a Property for appendProjectPathToConfigName --- .../net/fabricmc/loom/configuration/ide/RunConfig.java | 2 +- .../loom/configuration/ide/RunConfigSettings.java | 10 +++------- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/main/java/net/fabricmc/loom/configuration/ide/RunConfig.java b/src/main/java/net/fabricmc/loom/configuration/ide/RunConfig.java index 233f00a3..242558e1 100644 --- a/src/main/java/net/fabricmc/loom/configuration/ide/RunConfig.java +++ b/src/main/java/net/fabricmc/loom/configuration/ide/RunConfig.java @@ -170,7 +170,7 @@ public class RunConfig { runDir = "run"; } - boolean appendProjectPath = settings.getAppendProjectPathToConfigName(); + boolean appendProjectPath = settings.getAppendProjectPathToConfigName().get(); RunConfig runConfig = new RunConfig(); runConfig.configName = configName; populate(project, extension, runConfig, environment, appendProjectPath); diff --git a/src/main/java/net/fabricmc/loom/configuration/ide/RunConfigSettings.java b/src/main/java/net/fabricmc/loom/configuration/ide/RunConfigSettings.java index 3357c741..73ea94cb 100644 --- a/src/main/java/net/fabricmc/loom/configuration/ide/RunConfigSettings.java +++ b/src/main/java/net/fabricmc/loom/configuration/ide/RunConfigSettings.java @@ -79,7 +79,7 @@ public class RunConfigSettings implements Named { *

Warning: could produce ambiguous run config names if disabled, unless used carefully in conjunction with * {@link #configName}. */ - private boolean appendProjectPathToConfigName; + private final Property appendProjectPathToConfigName; /** * The default main class of the run configuration. @@ -128,7 +128,7 @@ public class RunConfigSettings implements Named { public RunConfigSettings(Project project, String name) { this.name = name; this.project = project; - this.appendProjectPathToConfigName = true; + this.appendProjectPathToConfigName = project.getObjects().property(Boolean.class).convention(true); this.extension = LoomGradleExtension.get(project); this.ideConfigGenerated = extension.isRootProject(); this.mainClass = project.getObjects().property(String.class).convention(project.provider(() -> { @@ -186,14 +186,10 @@ public class RunConfigSettings implements Named { this.configName = name; } - public boolean getAppendProjectPathToConfigName() { + public Property getAppendProjectPathToConfigName() { return appendProjectPathToConfigName; } - public void setAppendProjectPathToConfigName(boolean append) { - appendProjectPathToConfigName = append; - } - public String getDefaultMainClass() { return defaultMainClass; } From 51e1da73301d77a395deb6a7a531e547b39e749d Mon Sep 17 00:00:00 2001 From: modmuss Date: Wed, 20 Dec 2023 10:14:26 +0000 Subject: [PATCH 14/17] Update deps (#1007) * Update deps * Fix KotlinRemapperClassloaderTest --- gradle/libs.versions.toml | 8 ++-- gradle/test.libs.versions.toml | 2 +- .../remapping/KotlinClassMetadataWrapper.java | 37 ------------------- ...ClassMetadataRemappingAnnotationVisitor.kt | 11 +++--- .../KotlinRemapperClassloaderTest.groovy | 3 +- 5 files changed, 12 insertions(+), 49 deletions(-) delete mode 100644 src/main/java/net/fabricmc/loom/kotlin/remapping/KotlinClassMetadataWrapper.java diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 1a6692f3..30416679 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -4,15 +4,15 @@ asm = "9.6" commons-io = "2.15.0" gson = "2.10.1" jackson = "2.16.0" -guava = "32.1.3-jre" +guava = "33.0.0-jre" stitch = "0.6.2" -tiny-remapper = "0.8.11" +tiny-remapper = "0.9.0" access-widener = "2.1.0" -mapping-io = "0.5.0" +mapping-io = "0.5.1" lorenz-tiny = "4.0.2" mercury = "0.4.1" -kotlinx-metadata = "0.7.0" +kotlinx-metadata = "0.8.0" # Plugins spotless = "6.20.0" diff --git a/gradle/test.libs.versions.toml b/gradle/test.libs.versions.toml index 3bca2dd8..f01ff02c 100644 --- a/gradle/test.libs.versions.toml +++ b/gradle/test.libs.versions.toml @@ -6,7 +6,7 @@ mockito = "5.7.0" java-debug = "0.49.0" mixin = "0.11.4+mixin.0.8.5" -gradle-nightly = "8.6-20231118001259+0000" +gradle-nightly = "8.6-20231219002119+0000" fabric-loader = "0.14.24" fabric-installer = "0.11.1" diff --git a/src/main/java/net/fabricmc/loom/kotlin/remapping/KotlinClassMetadataWrapper.java b/src/main/java/net/fabricmc/loom/kotlin/remapping/KotlinClassMetadataWrapper.java deleted file mode 100644 index 2a6238a9..00000000 --- a/src/main/java/net/fabricmc/loom/kotlin/remapping/KotlinClassMetadataWrapper.java +++ /dev/null @@ -1,37 +0,0 @@ -/* - * This file is part of fabric-loom, licensed under the MIT License (MIT). - * - * Copyright (c) 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.kotlin.remapping; - -import kotlin.Metadata; -import kotlinx.metadata.jvm.KotlinClassMetadata; - -/** - * Similar story to JvmExtensionWrapper, lets abuse the fact that Java can call "internal" Kotlin APIs without reflection :). - */ -public record KotlinClassMetadataWrapper(KotlinClassMetadata metadata) { - public Metadata getAnnotationData() { - return metadata.getAnnotationData$kotlinx_metadata_jvm(); - } -} diff --git a/src/main/kotlin/net/fabricmc/loom/kotlin/remapping/KotlinClassMetadataRemappingAnnotationVisitor.kt b/src/main/kotlin/net/fabricmc/loom/kotlin/remapping/KotlinClassMetadataRemappingAnnotationVisitor.kt index 56ce489e..8b529604 100644 --- a/src/main/kotlin/net/fabricmc/loom/kotlin/remapping/KotlinClassMetadataRemappingAnnotationVisitor.kt +++ b/src/main/kotlin/net/fabricmc/loom/kotlin/remapping/KotlinClassMetadataRemappingAnnotationVisitor.kt @@ -56,11 +56,11 @@ class KotlinClassMetadataRemappingAnnotationVisitor(private val remapper: Remapp logger.info("Kotlin metadata for class ($className) as it was built using a different major Kotlin version (${header.metadataVersion[0]}.${header.metadataVersion[1]}.x) while the remapper is using (${KotlinVersion.CURRENT}).") } - when (val metadata = KotlinClassMetadata.read(header)) { + when (val metadata = KotlinClassMetadata.readLenient(header)) { is KotlinClassMetadata.Class -> { var klass = metadata.kmClass klass = KotlinClassRemapper(remapper).remap(klass) - val remapped = KotlinClassMetadata.writeClass(klass, header.metadataVersion, header.extraInt) + val remapped = KotlinClassMetadata.Class(klass, metadata.version, metadata.flags).write() writeClassHeader(remapped) validateKotlinClassHeader(remapped, header) } @@ -69,7 +69,7 @@ class KotlinClassMetadataRemappingAnnotationVisitor(private val remapper: Remapp if (klambda != null) { klambda = KotlinClassRemapper(remapper).remap(klambda) - val remapped = KotlinClassMetadata.writeLambda(klambda, header.metadataVersion, header.extraInt) + val remapped = KotlinClassMetadata.SyntheticClass(klambda, metadata.version, metadata.flags).write() writeClassHeader(remapped) validateKotlinClassHeader(remapped, header) } else { @@ -79,15 +79,14 @@ class KotlinClassMetadataRemappingAnnotationVisitor(private val remapper: Remapp is KotlinClassMetadata.FileFacade -> { var kpackage = metadata.kmPackage kpackage = KotlinClassRemapper(remapper).remap(kpackage) - val remapped = KotlinClassMetadata.writeFileFacade(kpackage, header.metadataVersion, header.extraInt) + val remapped = KotlinClassMetadata.FileFacade(kpackage, metadata.version, metadata.flags).write() writeClassHeader(remapped) validateKotlinClassHeader(remapped, header) } is KotlinClassMetadata.MultiFileClassPart -> { var kpackage = metadata.kmPackage kpackage = KotlinClassRemapper(remapper).remap(kpackage) - val wrapper = KotlinClassMetadataWrapper(metadata) - val remapped = KotlinClassMetadata.writeMultiFileClassPart(kpackage, metadata.facadeClassName, wrapper.annotationData.metadataVersion, wrapper.annotationData.extraInt) + val remapped = KotlinClassMetadata.MultiFileClassPart(kpackage, metadata.facadeClassName, metadata.version, metadata.flags).write() writeClassHeader(remapped) validateKotlinClassHeader(remapped, header) } diff --git a/src/test/groovy/net/fabricmc/loom/test/unit/kotlin/KotlinRemapperClassloaderTest.groovy b/src/test/groovy/net/fabricmc/loom/test/unit/kotlin/KotlinRemapperClassloaderTest.groovy index f832df5e..98b4042a 100644 --- a/src/test/groovy/net/fabricmc/loom/test/unit/kotlin/KotlinRemapperClassloaderTest.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/unit/kotlin/KotlinRemapperClassloaderTest.groovy @@ -24,6 +24,7 @@ package net.fabricmc.loom.test.unit.kotlin +import kotlin.KotlinVersion import org.objectweb.asm.ClassReader import org.objectweb.asm.tree.ClassNode import spock.lang.Specification @@ -36,7 +37,7 @@ import net.fabricmc.tinyremapper.api.TrEnvironment import net.fabricmc.tinyremapper.api.TrRemapper class KotlinRemapperClassloaderTest extends Specification { - private static String KOTLIN_VERSION = "1.6.10" + private static String KOTLIN_VERSION = KotlinVersion.CURRENT.toString() private static String KOTLIN_METADATA_VERSION = KotlinPluginUtils.kotlinMetadataVersion private static String KOTLIN_URL = "https://repo1.maven.org/maven2/org/jetbrains/kotlin/kotlin-stdlib/${KOTLIN_VERSION}/kotlin-stdlib-${KOTLIN_VERSION}.jar" private static String KOTLIN_METADATA_URL = "https://repo1.maven.org/maven2/org/jetbrains/kotlinx/kotlinx-metadata-jvm/${KOTLIN_METADATA_VERSION}/kotlinx-metadata-jvm-${KOTLIN_METADATA_VERSION}.jar" From ecc7e730e9570256ea56c735fa13192125a6ac36 Mon Sep 17 00:00:00 2001 From: modmuss Date: Wed, 20 Dec 2023 15:52:13 +0000 Subject: [PATCH 15/17] Read data generation modid from FMJ by default. And code cleanup. (#1008) * Read data generation modid from FMJ by default. And code cleanup. Closes #999 * Fix #1000 --- .../configuration/FabricApiExtension.java | 25 ++++-- .../extension/LoomGradleExtensionApiImpl.java | 20 +++-- .../loom/extension/ModVersionParser.java | 76 ------------------- .../fabricmc/loom/util/fmj/FabricModJson.java | 4 + .../test/unit/fmj/FabricModJsonV0Test.groovy | 1 + .../test/unit/fmj/FabricModJsonV1Test.groovy | 1 + .../test/unit/fmj/FabricModJsonV2Test.groovy | 1 + 7 files changed, 41 insertions(+), 87 deletions(-) delete mode 100644 src/main/java/net/fabricmc/loom/extension/ModVersionParser.java diff --git a/src/main/java/net/fabricmc/loom/configuration/FabricApiExtension.java b/src/main/java/net/fabricmc/loom/configuration/FabricApiExtension.java index 5a82ffbc..4fde07fd 100644 --- a/src/main/java/net/fabricmc/loom/configuration/FabricApiExtension.java +++ b/src/main/java/net/fabricmc/loom/configuration/FabricApiExtension.java @@ -25,6 +25,7 @@ package net.fabricmc.loom.configuration; import java.io.File; +import java.io.IOException; import java.io.UncheckedIOException; import java.util.Collections; import java.util.HashMap; @@ -55,6 +56,8 @@ import org.w3c.dom.NodeList; import net.fabricmc.loom.LoomGradleExtension; import net.fabricmc.loom.util.download.DownloadException; +import net.fabricmc.loom.util.fmj.FabricModJson; +import net.fabricmc.loom.util.fmj.FabricModJsonFactory; import net.fabricmc.loom.util.gradle.SourceSetHelper; public abstract class FabricApiExtension { @@ -136,14 +139,10 @@ public abstract class FabricApiExtension { }); if (settings.getCreateSourceSet().get()) { - if (!settings.getModId().isPresent()) { - throw new IllegalStateException("DataGenerationSettings.getModId() must be set when using split sources."); - } - SourceSetContainer sourceSets = SourceSetHelper.getSourceSets(getProject()); // Create the new datagen sourceset, depend on the main sourceset. - sourceSets.create(DATAGEN_SOURCESET_NAME, sourceSet -> { + SourceSet dataGenSourceSet = sourceSets.create(DATAGEN_SOURCESET_NAME, sourceSet -> { sourceSet.setCompileClasspath( sourceSet.getCompileClasspath() .plus(mainSourceSet.getOutput()) @@ -158,6 +157,20 @@ public abstract class FabricApiExtension { extendsFrom(getProject(), sourceSet.getRuntimeClasspathConfigurationName(), mainSourceSet.getRuntimeClasspathConfigurationName()); }); + settings.getModId().convention(getProject().provider(() -> { + try { + final FabricModJson fabricModJson = FabricModJsonFactory.createFromSourceSetsNullable(dataGenSourceSet); + + if (fabricModJson == null) { + throw new RuntimeException("Could not find a fabric.mod.json file in the data source set or a value for DataGenerationSettings.getModId()"); + } + + return fabricModJson.getId(); + } catch (IOException e) { + throw new org.gradle.api.UncheckedIOException("Failed to read mod id from the datagen source set.", e); + } + })); + extension.getMods().create(settings.getModId().get(), mod -> { // Create a classpath group for this mod. Assume that the main sourceset is already in a group. mod.sourceSet(DATAGEN_SOURCESET_NAME); @@ -168,7 +181,7 @@ public abstract class FabricApiExtension { if (settings.getCreateRunConfiguration().get()) { extension.getRunConfigs().create("datagen", run -> { - run.name("Data Generation"); + run.setConfigName("Data Generation"); run.inherit(extension.getRunConfigs().getByName("server")); run.property("fabric-api.datagen"); diff --git a/src/main/java/net/fabricmc/loom/extension/LoomGradleExtensionApiImpl.java b/src/main/java/net/fabricmc/loom/extension/LoomGradleExtensionApiImpl.java index 87875f31..a48042b0 100644 --- a/src/main/java/net/fabricmc/loom/extension/LoomGradleExtensionApiImpl.java +++ b/src/main/java/net/fabricmc/loom/extension/LoomGradleExtensionApiImpl.java @@ -25,12 +25,14 @@ package net.fabricmc.loom.extension; import java.io.File; +import java.io.IOException; import java.util.Set; import org.gradle.api.Action; import org.gradle.api.NamedDomainObjectContainer; import org.gradle.api.NamedDomainObjectList; import org.gradle.api.Project; +import org.gradle.api.UncheckedIOException; import org.gradle.api.artifacts.Dependency; import org.gradle.api.file.ConfigurableFileCollection; import org.gradle.api.file.RegularFileProperty; @@ -65,6 +67,8 @@ import net.fabricmc.loom.configuration.providers.minecraft.MinecraftJarConfigura import net.fabricmc.loom.configuration.providers.minecraft.MinecraftSourceSets; import net.fabricmc.loom.task.GenerateSourcesTask; import net.fabricmc.loom.util.DeprecationHelper; +import net.fabricmc.loom.util.fmj.FabricModJson; +import net.fabricmc.loom.util.fmj.FabricModJsonFactory; import net.fabricmc.loom.util.gradle.SourceSetHelper; /** @@ -88,8 +92,6 @@ public abstract class LoomGradleExtensionApiImpl implements LoomGradleExtensionA private final Property splitEnvironmentalSourceSet; private final InterfaceInjectionExtensionAPI interfaceInjectionExtension; - private final ModVersionParser versionParser; - private final NamedDomainObjectContainer runConfigs; private final NamedDomainObjectContainer decompilers; private final NamedDomainObjectContainer mods; @@ -124,8 +126,6 @@ public abstract class LoomGradleExtensionApiImpl implements LoomGradleExtensionA this.intermediateMappingsProvider = project.getObjects().property(IntermediateMappingsProvider.class); this.intermediateMappingsProvider.finalizeValueOnRead(); - this.versionParser = new ModVersionParser(project); - this.deprecationHelper = new DeprecationHelper.ProjectBased(project); this.runConfigs = project.container(RunConfigSettings.class, @@ -252,7 +252,17 @@ public abstract class LoomGradleExtensionApiImpl implements LoomGradleExtensionA @Override public String getModVersion() { - return versionParser.getModVersion(); + try { + final FabricModJson fabricModJson = FabricModJsonFactory.createFromSourceSetsNullable(SourceSetHelper.getMainSourceSet(getProject())); + + if (fabricModJson == null) { + throw new RuntimeException("Could not find a fabric.mod.json file in the main sourceset"); + } + + return fabricModJson.getModVersion(); + } catch (IOException e) { + throw new UncheckedIOException("Failed to read mod version from main sourceset.", e); + } } @Override diff --git a/src/main/java/net/fabricmc/loom/extension/ModVersionParser.java b/src/main/java/net/fabricmc/loom/extension/ModVersionParser.java deleted file mode 100644 index 0ded4629..00000000 --- a/src/main/java/net/fabricmc/loom/extension/ModVersionParser.java +++ /dev/null @@ -1,76 +0,0 @@ -/* - * This file is part of fabric-loom, licensed under the MIT License (MIT). - * - * Copyright (c) 2021 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.extension; - -import java.io.File; -import java.io.FileReader; -import java.io.IOException; - -import com.google.gson.JsonObject; -import org.gradle.api.Project; -import org.gradle.api.plugins.JavaPluginExtension; - -import net.fabricmc.loom.LoomGradlePlugin; - -public class ModVersionParser { - private final Project project; - - private String version = null; - - public ModVersionParser(Project project) { - this.project = project; - } - - public String getModVersion() { - if (version != null) { - return version; - } - - File json = locateModJsonFile(); - JsonObject jsonObject; - - try (var reader = new FileReader(json)) { - jsonObject = LoomGradlePlugin.GSON.fromJson(reader, JsonObject.class); - } catch (IOException e) { - throw new RuntimeException("Failed to read fabric.mod.json file"); - } - - if (!jsonObject.has("version") || !jsonObject.get("version").isJsonPrimitive()) { - throw new UnsupportedOperationException("Could not find valid version in the fabric.mod.json file"); - } - - version = jsonObject.get("version").getAsString(); - - return version; - } - - private File locateModJsonFile() { - return project.getExtensions().getByType(JavaPluginExtension.class).getSourceSets() - .getByName("main") - .getResources() - .matching(patternFilterable -> patternFilterable.include("fabric.mod.json")) - .getSingleFile(); - } -} diff --git a/src/main/java/net/fabricmc/loom/util/fmj/FabricModJson.java b/src/main/java/net/fabricmc/loom/util/fmj/FabricModJson.java index 8356bfa7..617019cb 100644 --- a/src/main/java/net/fabricmc/loom/util/fmj/FabricModJson.java +++ b/src/main/java/net/fabricmc/loom/util/fmj/FabricModJson.java @@ -50,6 +50,10 @@ public abstract sealed class FabricModJson permits FabricModJsonV0, FabricModJso return readString(jsonObject, "id"); } + public String getModVersion() { + return readString(jsonObject, "version"); + } + @Nullable public abstract JsonElement getCustom(String key); diff --git a/src/test/groovy/net/fabricmc/loom/test/unit/fmj/FabricModJsonV0Test.groovy b/src/test/groovy/net/fabricmc/loom/test/unit/fmj/FabricModJsonV0Test.groovy index 3854bb38..384c3cf3 100644 --- a/src/test/groovy/net/fabricmc/loom/test/unit/fmj/FabricModJsonV0Test.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/unit/fmj/FabricModJsonV0Test.groovy @@ -62,6 +62,7 @@ class FabricModJsonV0Test extends Specification { def fmj = FabricModJsonFactory.create(JSON_OBJECT, mockSource) then: fmj.version == 0 + fmj.modVersion == "1.0.0" } def "id"() { diff --git a/src/test/groovy/net/fabricmc/loom/test/unit/fmj/FabricModJsonV1Test.groovy b/src/test/groovy/net/fabricmc/loom/test/unit/fmj/FabricModJsonV1Test.groovy index 3a79e965..9053d14f 100644 --- a/src/test/groovy/net/fabricmc/loom/test/unit/fmj/FabricModJsonV1Test.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/unit/fmj/FabricModJsonV1Test.groovy @@ -69,6 +69,7 @@ class FabricModJsonV1Test extends Specification { def fmj = FabricModJsonFactory.create(JSON_OBJECT, mockSource) then: fmj.version == 1 + fmj.modVersion == "1.0.0" } def "id"() { diff --git a/src/test/groovy/net/fabricmc/loom/test/unit/fmj/FabricModJsonV2Test.groovy b/src/test/groovy/net/fabricmc/loom/test/unit/fmj/FabricModJsonV2Test.groovy index 1cea56c9..212de2ac 100644 --- a/src/test/groovy/net/fabricmc/loom/test/unit/fmj/FabricModJsonV2Test.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/unit/fmj/FabricModJsonV2Test.groovy @@ -83,6 +83,7 @@ class FabricModJsonV2Test extends Specification { def fmj = FabricModJsonFactory.create(JSON_OBJECT, mockSource) then: fmj.version == 2 + fmj.modVersion == "1.0.0" } def "id"() { From 85101bca01812adf3dc7bbc46961759e642f287b Mon Sep 17 00:00:00 2001 From: modmuss Date: Wed, 20 Dec 2023 16:49:42 +0000 Subject: [PATCH 16/17] Fix Intelij download sources hook. (#1006) * Fix Intelij download sources hook. * Cleanup and performance improvements --- .../configuration/CompileConfiguration.java | 6 +- .../decompile/DecompileConfiguration.java | 10 +- .../SingleJarDecompileConfiguration.java | 17 +- .../SplitDecompileConfiguration.java | 15 +- .../ide/idea/DownloadSourcesHook.java | 145 ++++++++++++++++++ .../ide/idea/IdeaConfiguration.java | 96 +++--------- .../mods/dependency/LocalMavenHelper.java | 21 +-- .../providers/minecraft/MinecraftJar.java | 56 ++++--- .../minecraft/MinecraftJarConfiguration.java | 8 +- .../minecraft/MinecraftSourceSets.java | 20 +-- .../providers/minecraft/SingleJarEnvType.java | 14 +- .../AbstractMappedMinecraftProvider.java | 31 ++-- .../mapped/MappedMinecraftProvider.java | 19 +-- .../mapped/NamedMinecraftProvider.java | 13 +- .../ProcessedNamedMinecraftProvider.java | 17 +- 15 files changed, 307 insertions(+), 181 deletions(-) create mode 100644 src/main/java/net/fabricmc/loom/configuration/ide/idea/DownloadSourcesHook.java diff --git a/src/main/java/net/fabricmc/loom/configuration/CompileConfiguration.java b/src/main/java/net/fabricmc/loom/configuration/CompileConfiguration.java index 8930e596..041bc42d 100644 --- a/src/main/java/net/fabricmc/loom/configuration/CompileConfiguration.java +++ b/src/main/java/net/fabricmc/loom/configuration/CompileConfiguration.java @@ -222,8 +222,10 @@ public abstract class CompileConfiguration implements Runnable { private void configureDecompileTasks(ConfigContext configContext) { final LoomGradleExtension extension = configContext.extension(); - extension.getMinecraftJarConfiguration().get().getDecompileConfigurationBiFunction() - .apply(configContext, extension.getNamedMinecraftProvider()).afterEvaluation(); + extension.getMinecraftJarConfiguration().get() + .getDecompileConfigurationBiFunction() + .apply(configContext.project(), extension.getNamedMinecraftProvider()) + .afterEvaluation(); } private Path getLockFile() { diff --git a/src/main/java/net/fabricmc/loom/configuration/decompile/DecompileConfiguration.java b/src/main/java/net/fabricmc/loom/configuration/decompile/DecompileConfiguration.java index 0a6a5948..7c36d726 100644 --- a/src/main/java/net/fabricmc/loom/configuration/decompile/DecompileConfiguration.java +++ b/src/main/java/net/fabricmc/loom/configuration/decompile/DecompileConfiguration.java @@ -30,8 +30,8 @@ import org.gradle.api.Project; import org.gradle.api.artifacts.ConfigurationContainer; import net.fabricmc.loom.LoomGradleExtension; -import net.fabricmc.loom.configuration.ConfigContext; import net.fabricmc.loom.configuration.providers.mappings.MappingConfiguration; +import net.fabricmc.loom.configuration.providers.minecraft.MinecraftJar; import net.fabricmc.loom.configuration.providers.minecraft.mapped.MappedMinecraftProvider; import net.fabricmc.loom.task.GenerateSourcesTask; import net.fabricmc.loom.util.Constants; @@ -42,13 +42,15 @@ public abstract class DecompileConfiguration protected final LoomGradleExtension extension; protected final MappingConfiguration mappingConfiguration; - public DecompileConfiguration(ConfigContext configContext, T minecraftProvider) { - this.project = configContext.project(); + public DecompileConfiguration(Project project, T minecraftProvider) { + this.project = project; this.minecraftProvider = minecraftProvider; - this.extension = configContext.extension(); + this.extension = LoomGradleExtension.get(project); this.mappingConfiguration = extension.getMappingConfiguration(); } + public abstract String getTaskName(MinecraftJar.Type type); + public abstract void afterEvaluation(); protected final void configureUnpick(GenerateSourcesTask task, File unpickOutputJar) { diff --git a/src/main/java/net/fabricmc/loom/configuration/decompile/SingleJarDecompileConfiguration.java b/src/main/java/net/fabricmc/loom/configuration/decompile/SingleJarDecompileConfiguration.java index 538c3018..6965a0bf 100644 --- a/src/main/java/net/fabricmc/loom/configuration/decompile/SingleJarDecompileConfiguration.java +++ b/src/main/java/net/fabricmc/loom/configuration/decompile/SingleJarDecompileConfiguration.java @@ -27,16 +27,22 @@ package net.fabricmc.loom.configuration.decompile; import java.io.File; import java.util.List; +import org.gradle.api.Project; + import net.fabricmc.loom.LoomGradleExtension; -import net.fabricmc.loom.configuration.ConfigContext; import net.fabricmc.loom.configuration.providers.minecraft.MinecraftJar; import net.fabricmc.loom.configuration.providers.minecraft.mapped.MappedMinecraftProvider; import net.fabricmc.loom.task.GenerateSourcesTask; import net.fabricmc.loom.util.Constants; public class SingleJarDecompileConfiguration extends DecompileConfiguration { - public SingleJarDecompileConfiguration(ConfigContext configContext, MappedMinecraftProvider minecraftProvider) { - super(configContext, minecraftProvider); + public SingleJarDecompileConfiguration(Project project, MappedMinecraftProvider minecraftProvider) { + super(project, minecraftProvider); + } + + @Override + public String getTaskName(MinecraftJar.Type type) { + return "genSources"; } @Override @@ -44,10 +50,11 @@ public class SingleJarDecompileConfiguration extends DecompileConfiguration minecraftJars = minecraftProvider.getMinecraftJars(); assert minecraftJars.size() == 1; final MinecraftJar minecraftJar = minecraftJars.get(0); + final String taskBaseName = getTaskName(minecraftJar.getType()); LoomGradleExtension.get(project).getDecompilerOptions().forEach(options -> { final String decompilerName = options.getFormattedName(); - String taskName = "genSourcesWith" + decompilerName; + String taskName = "%sWith%s".formatted(taskBaseName, decompilerName); // Decompiler will be passed to the constructor of GenerateSourcesTask project.getTasks().register(taskName, GenerateSourcesTask.class, options).configure(task -> { task.getInputJarName().set(minecraftJar.getName()); @@ -64,7 +71,7 @@ public class SingleJarDecompileConfiguration extends DecompileConfiguration { + project.getTasks().register(taskBaseName, task -> { task.setDescription("Decompile minecraft using the default decompiler."); task.setGroup(Constants.TaskGroup.FABRIC); diff --git a/src/main/java/net/fabricmc/loom/configuration/decompile/SplitDecompileConfiguration.java b/src/main/java/net/fabricmc/loom/configuration/decompile/SplitDecompileConfiguration.java index 0cf8fbe3..cd2aa287 100644 --- a/src/main/java/net/fabricmc/loom/configuration/decompile/SplitDecompileConfiguration.java +++ b/src/main/java/net/fabricmc/loom/configuration/decompile/SplitDecompileConfiguration.java @@ -27,19 +27,28 @@ package net.fabricmc.loom.configuration.decompile; import java.io.File; import org.gradle.api.Action; +import org.gradle.api.Project; import org.gradle.api.Task; import org.gradle.api.tasks.TaskProvider; import net.fabricmc.loom.api.decompilers.DecompilerOptions; -import net.fabricmc.loom.configuration.ConfigContext; import net.fabricmc.loom.configuration.providers.minecraft.MinecraftJar; import net.fabricmc.loom.configuration.providers.minecraft.mapped.MappedMinecraftProvider; import net.fabricmc.loom.task.GenerateSourcesTask; import net.fabricmc.loom.util.Constants; public final class SplitDecompileConfiguration extends DecompileConfiguration { - public SplitDecompileConfiguration(ConfigContext configContext, MappedMinecraftProvider.Split minecraftProvider) { - super(configContext, minecraftProvider); + public SplitDecompileConfiguration(Project project, MappedMinecraftProvider.Split minecraftProvider) { + super(project, minecraftProvider); + } + + @Override + public String getTaskName(MinecraftJar.Type type) { + return switch (type) { + case COMMON -> "genCommonSources"; + case CLIENT_ONLY -> "genClientSources"; + default -> throw new AssertionError(); + }; } @Override diff --git a/src/main/java/net/fabricmc/loom/configuration/ide/idea/DownloadSourcesHook.java b/src/main/java/net/fabricmc/loom/configuration/ide/idea/DownloadSourcesHook.java new file mode 100644 index 00000000..98244e3a --- /dev/null +++ b/src/main/java/net/fabricmc/loom/configuration/ide/idea/DownloadSourcesHook.java @@ -0,0 +1,145 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 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.ide.idea; + +import java.io.File; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.util.List; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import org.gradle.api.Project; +import org.gradle.api.Task; +import org.jetbrains.annotations.Nullable; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import net.fabricmc.loom.LoomGradleExtension; +import net.fabricmc.loom.configuration.mods.dependency.LocalMavenHelper; +import net.fabricmc.loom.configuration.providers.minecraft.MinecraftJar; +import net.fabricmc.loom.configuration.providers.minecraft.mapped.NamedMinecraftProvider; + +// See: https://github.com/JetBrains/intellij-community/blob/a09b1b84ab64a699794c860bc96774766dd38958/plugins/gradle/java/src/util/GradleAttachSourcesProvider.java +record DownloadSourcesHook(Project project, Task task) { + public static final String INIT_SCRIPT_NAME = "ijDownloadSources"; + private static final Pattern NOTATION_PATTERN = Pattern.compile("dependencyNotation = '(?.*)'"); + private static final Logger LOGGER = LoggerFactory.getLogger(DownloadSourcesHook.class); + + public static boolean hasInitScript(Project project) { + List initScripts = project.getGradle().getStartParameter().getInitScripts(); + + for (File initScript : initScripts) { + if (initScript.getName().contains(INIT_SCRIPT_NAME)) { + return true; + } + } + + return false; + } + + void tryHook() { + List initScripts = project.getGradle().getStartParameter().getInitScripts(); + + for (File initScript : initScripts) { + if (!initScript.getName().contains(INIT_SCRIPT_NAME)) { + continue; + } + + try { + final String script = Files.readString(initScript.toPath(), StandardCharsets.UTF_8); + final String notation = parseInitScript(script); + + if (notation == null) { + LOGGER.debug("failed to parse init script dependency"); + continue; + } + + final MinecraftJar.Type jarType = getJarType(notation); + + if (jarType == null) { + LOGGER.debug("init script is trying to download sources for another Minecraft jar ({}) not used by this project ({})", notation, project.getPath()); + continue; + } + + String sourcesTaskName = getGenSourcesTaskName(jarType); + task.dependsOn(project.getTasks().named(sourcesTaskName)); + + LOGGER.info("Running genSources task: {} in project: {} for {}", sourcesTaskName, project.getPath(), notation); + break; + } catch (IOException e) { + // Ignore + } + } + } + + @Nullable + private String parseInitScript(String script) { + if (!script.contains("IjDownloadTask")) { + // Failed some basic sanity checks. + return null; + } + + // A little gross but should do the job nicely. + final Matcher matcher = NOTATION_PATTERN.matcher(script); + + if (matcher.find()) { + return matcher.group("notation"); + } + + return null; + } + + private String getGenSourcesTaskName(MinecraftJar.Type jarType) { + LoomGradleExtension extension = LoomGradleExtension.get(project); + return extension.getMinecraftJarConfiguration().get() + .getDecompileConfigurationBiFunction() + .apply(project, extension.getNamedMinecraftProvider()) + .getTaskName(jarType); + } + + // Return the jar type, or null when this jar isnt used by the project + @Nullable + private MinecraftJar.Type getJarType(String name) { + final LoomGradleExtension extension = LoomGradleExtension.get(project); + final NamedMinecraftProvider minecraftProvider = extension.getNamedMinecraftProvider(); + final List dependencyTypes = minecraftProvider.getDependencyTypes(); + + if (dependencyTypes.isEmpty()) { + throw new IllegalStateException(); + } + + for (MinecraftJar.Type type : dependencyTypes) { + final LocalMavenHelper mavenHelper = minecraftProvider.getMavenHelper(type).withClassifier("sources"); + + if (mavenHelper.getNotation().equals(name)) { + return type; + } + } + + return null; + } +} diff --git a/src/main/java/net/fabricmc/loom/configuration/ide/idea/IdeaConfiguration.java b/src/main/java/net/fabricmc/loom/configuration/ide/idea/IdeaConfiguration.java index 145b2a03..95cf1438 100644 --- a/src/main/java/net/fabricmc/loom/configuration/ide/idea/IdeaConfiguration.java +++ b/src/main/java/net/fabricmc/loom/configuration/ide/idea/IdeaConfiguration.java @@ -24,40 +24,27 @@ package net.fabricmc.loom.configuration.ide.idea; -import java.io.File; -import java.io.IOException; -import java.nio.charset.StandardCharsets; -import java.nio.file.Files; import java.util.ArrayList; import java.util.List; -import java.util.Locale; -import java.util.regex.Matcher; -import java.util.regex.Pattern; import javax.inject.Inject; import org.gradle.StartParameter; import org.gradle.TaskExecutionRequest; import org.gradle.api.Project; -import org.gradle.api.Task; -import org.gradle.api.tasks.TaskProvider; import org.gradle.internal.DefaultTaskExecutionRequest; -import org.jetbrains.annotations.Nullable; import net.fabricmc.loom.LoomGradleExtension; import net.fabricmc.loom.configuration.ide.RunConfigSettings; -import net.fabricmc.loom.configuration.providers.minecraft.MinecraftJarConfiguration; import net.fabricmc.loom.task.LoomTasks; +import net.fabricmc.loom.util.gradle.GradleUtils; public abstract class IdeaConfiguration implements Runnable { - private static final String INIT_SCRIPT_NAME = "ijmiscinit"; - private static final Pattern NOTATION_PATTERN = Pattern.compile("'net\\.minecraft:(?.*):(.*):sources'"); - @Inject protected abstract Project getProject(); public void run() { - TaskProvider ideaSyncTask = getProject().getTasks().register("ideaSyncTask", IdeaSyncTask.class, task -> { + getProject().getTasks().register("ideaSyncTask", IdeaSyncTask.class, task -> { if (LoomGradleExtension.get(getProject()).getRunConfigs().stream().anyMatch(RunConfigSettings::isIdeConfigGenerated)) { task.dependsOn(LoomTasks.getIDELaunchConfigureTaskName(getProject())); } else { @@ -65,11 +52,7 @@ public abstract class IdeaConfiguration implements Runnable { } }); - getProject().getTasks().configureEach(task -> { - if (task.getName().equals("DownloadSources")) { - hookDownloadSources(getProject(), task); - } - }); + hookDownloadSources(); if (!IdeaUtils.isIdeaSync()) { return; @@ -82,62 +65,27 @@ public abstract class IdeaConfiguration implements Runnable { startParameter.setTaskRequests(taskRequests); } - /* - "Parse" the init script enough to figure out what jar we are talking about. + private void hookDownloadSources() { + LoomGradleExtension extension = LoomGradleExtension.get(getProject()); - Intelij code: https://github.com/JetBrains/intellij-community/blob/a09b1b84ab64a699794c860bc96774766dd38958/plugins/gradle/java/src/util/GradleAttachSourcesProvider.java - */ - private static void hookDownloadSources(Project project, Task task) { - List initScripts = project.getGradle().getStartParameter().getInitScripts(); + if (!extension.isRootProject()) { + return; + } - for (File initScript : initScripts) { - if (!initScript.getName().contains(INIT_SCRIPT_NAME)) { - continue; + if (!DownloadSourcesHook.hasInitScript(getProject())) { + return; + } + + getProject().getTasks().configureEach(task -> { + if (task.getName().startsWith(DownloadSourcesHook.INIT_SCRIPT_NAME)) { + getProject().allprojects(subProject -> { + if (!GradleUtils.isLoomProject(subProject)) { + return; + } + + new DownloadSourcesHook(subProject, task).tryHook(); + }); } - - try { - final String script = Files.readString(initScript.toPath(), StandardCharsets.UTF_8); - final String notation = parseInitScript(project, script); - - if (notation != null) { - task.dependsOn(getGenSourcesTaskName(LoomGradleExtension.get(project), notation)); - } - } catch (IOException e) { - // Ignore - } - } - } - - @Nullable - private static String parseInitScript(Project project, String script) { - if (!script.contains("Attempt to download sources from") - || !script.contains("downloadSources_") - || !script.contains("'%s'".formatted(project.getPath()))) { - // Failed some basic sanity checks. - return null; - } - - // A little gross but should do the job nicely. - final Matcher matcher = NOTATION_PATTERN.matcher(script); - - if (matcher.find()) { - return matcher.group("name"); - } - - return null; - } - - private static String getGenSourcesTaskName(LoomGradleExtension extension, String notation) { - final MinecraftJarConfiguration configuration = extension.getMinecraftJarConfiguration().get(); - - if (configuration == MinecraftJarConfiguration.SPLIT) { - if (notation.toLowerCase(Locale.ROOT).contains("minecraft-clientonly")) { - return "genClientOnlySources"; - } - - return "genCommonSources"; - } - - return "genSources"; + }); } } diff --git a/src/main/java/net/fabricmc/loom/configuration/mods/dependency/LocalMavenHelper.java b/src/main/java/net/fabricmc/loom/configuration/mods/dependency/LocalMavenHelper.java index 33af9b96..c1a81b1a 100644 --- a/src/main/java/net/fabricmc/loom/configuration/mods/dependency/LocalMavenHelper.java +++ b/src/main/java/net/fabricmc/loom/configuration/mods/dependency/LocalMavenHelper.java @@ -34,22 +34,7 @@ import java.nio.file.StandardCopyOption; import org.jetbrains.annotations.Nullable; -public final class LocalMavenHelper { - private final String group; - private final String name; - private final String version; - @Nullable - private final String baseClassifier; - private final Path root; - - public LocalMavenHelper(String group, String name, String version, @Nullable String classifier, Path root) { - this.group = group; - this.name = name; - this.version = version; - this.baseClassifier = classifier; - this.root = root; - } - +public record LocalMavenHelper(String group, String name, String version, @Nullable String baseClassifier, Path root) { public Path copyToMaven(Path artifact, @Nullable String classifier) throws IOException { if (!artifact.getFileName().toString().endsWith(".jar")) { throw new UnsupportedOperationException(); @@ -108,4 +93,8 @@ public final class LocalMavenHelper { : String.format("%s-%s-%s.jar", name, version, classifier); return getDirectory().resolve(fileName); } + + public LocalMavenHelper withClassifier(String classifier) { + return new LocalMavenHelper(group, name, version, classifier, root); + } } diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/MinecraftJar.java b/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/MinecraftJar.java index 311d2021..d602be45 100644 --- a/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/MinecraftJar.java +++ b/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/MinecraftJar.java @@ -31,14 +31,14 @@ import java.util.Objects; public abstract sealed class MinecraftJar permits MinecraftJar.Client, MinecraftJar.ClientOnly, MinecraftJar.Common, MinecraftJar.Merged, MinecraftJar.Server { private final Path path; private final boolean merged, client, server; - private final String name; + private final Type type; - protected MinecraftJar(Path path, boolean merged, boolean client, boolean server, String name) { + protected MinecraftJar(Path path, boolean merged, boolean client, boolean server, Type type) { this.path = Objects.requireNonNull(path); this.merged = merged; this.client = client; this.server = server; - this.name = name; + this.type = type; } public Path getPath() { @@ -62,16 +62,18 @@ public abstract sealed class MinecraftJar permits MinecraftJar.Client, Minecraft } public String getName() { - return name; + return type.toString(); + } + + public Type getType() { + return type; } public abstract MinecraftJar forPath(Path path); public static final class Merged extends MinecraftJar { - public static final String NAME = "merged"; - public Merged(Path path) { - super(path, true, true, true, NAME); + super(path, true, true, true, Type.MERGED); } @Override @@ -81,10 +83,8 @@ public abstract sealed class MinecraftJar permits MinecraftJar.Client, Minecraft } public static final class Common extends MinecraftJar { - public static final String NAME = "common"; - public Common(Path path) { - super(path, false, false, true, NAME); + super(path, false, false, true, Type.COMMON); } @Override @@ -94,10 +94,8 @@ public abstract sealed class MinecraftJar permits MinecraftJar.Client, Minecraft } public static final class Server extends MinecraftJar { - public static final String NAME = "server"; - public Server(Path path) { - super(path, false, false, true, NAME); + super(path, false, false, true, Type.SERVER); } @Override @@ -108,10 +106,8 @@ public abstract sealed class MinecraftJar permits MinecraftJar.Client, Minecraft // Un-split client jar public static final class Client extends MinecraftJar { - public static final String NAME = "client"; - public Client(Path path) { - super(path, false, true, false, NAME); + super(path, false, true, false, Type.CLIENT); } @Override @@ -122,10 +118,8 @@ public abstract sealed class MinecraftJar permits MinecraftJar.Client, Minecraft // Split client jar public static final class ClientOnly extends MinecraftJar { - public static final String NAME = "clientOnly"; - public ClientOnly(Path path) { - super(path, false, true, false, NAME); + super(path, false, true, false, Type.CLIENT_ONLY); } @Override @@ -133,4 +127,28 @@ public abstract sealed class MinecraftJar permits MinecraftJar.Client, Minecraft return new ClientOnly(path); } } + + public enum Type { + // Merged jar + MERGED("merged"), + + // Regular jars, not merged or split + SERVER("server"), + CLIENT("client"), + + // Split jars + COMMON("common"), + CLIENT_ONLY("clientOnly"); + + private final String name; + + Type(String name) { + this.name = name; + } + + @Override + public String toString() { + return name; + } + } } diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/MinecraftJarConfiguration.java b/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/MinecraftJarConfiguration.java index 10aa592e..34e8f42e 100644 --- a/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/MinecraftJarConfiguration.java +++ b/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/MinecraftJarConfiguration.java @@ -78,7 +78,7 @@ public enum MinecraftJarConfiguration { private final BiFunction> intermediaryMinecraftProviderBiFunction; private final BiFunction> namedMinecraftProviderBiFunction; private final BiFunction, MinecraftJarProcessorManager, ProcessedNamedMinecraftProvider> processedNamedMinecraftProviderBiFunction; - private final BiFunction> decompileConfigurationBiFunction; + private final BiFunction> decompileConfigurationBiFunction; private final List supportedEnvironments; @SuppressWarnings("unchecked") // Just a bit of a generic mess :) @@ -87,14 +87,14 @@ public enum MinecraftJarConfiguration { BiFunction> intermediaryMinecraftProviderBiFunction, BiFunction namedMinecraftProviderBiFunction, BiFunction> processedNamedMinecraftProviderBiFunction, - BiFunction> decompileConfigurationBiFunction, + BiFunction> decompileConfigurationBiFunction, List supportedEnvironments ) { this.minecraftProviderFunction = (Function) minecraftProviderFunction; this.intermediaryMinecraftProviderBiFunction = (BiFunction>) (Object) intermediaryMinecraftProviderBiFunction; this.namedMinecraftProviderBiFunction = (BiFunction>) namedMinecraftProviderBiFunction; this.processedNamedMinecraftProviderBiFunction = (BiFunction, MinecraftJarProcessorManager, ProcessedNamedMinecraftProvider>) (Object) processedNamedMinecraftProviderBiFunction; - this.decompileConfigurationBiFunction = (BiFunction>) decompileConfigurationBiFunction; + this.decompileConfigurationBiFunction = (BiFunction>) decompileConfigurationBiFunction; this.supportedEnvironments = supportedEnvironments; } @@ -114,7 +114,7 @@ public enum MinecraftJarConfiguration { return processedNamedMinecraftProviderBiFunction; } - public BiFunction> getDecompileConfigurationBiFunction() { + public BiFunction> getDecompileConfigurationBiFunction() { return decompileConfigurationBiFunction; } diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/MinecraftSourceSets.java b/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/MinecraftSourceSets.java index 683aacb9..a43ccc20 100644 --- a/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/MinecraftSourceSets.java +++ b/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/MinecraftSourceSets.java @@ -45,7 +45,7 @@ public abstract sealed class MinecraftSourceSets permits MinecraftSourceSets.Sin return LoomGradleExtension.get(project).areEnvironmentSourceSetsSplit() ? Split.INSTANCE : Single.INSTANCE; } - public abstract void applyDependencies(BiConsumer consumer, List targets); + public abstract void applyDependencies(BiConsumer consumer, List targets); public abstract String getSourceSetForEnv(String env); @@ -100,8 +100,8 @@ public abstract sealed class MinecraftSourceSets permits MinecraftSourceSets.Sin private static final Single INSTANCE = new Single(); @Override - public void applyDependencies(BiConsumer consumer, List targets) { - for (String target : targets) { + public void applyDependencies(BiConsumer consumer, List targets) { + for (MinecraftJar.Type target : targets) { consumer.accept(MINECRAFT_NAMED.compile(), target); consumer.accept(MINECRAFT_NAMED.runtime(), target); } @@ -150,15 +150,15 @@ public abstract sealed class MinecraftSourceSets permits MinecraftSourceSets.Sin private static final Split INSTANCE = new Split(); @Override - public void applyDependencies(BiConsumer consumer, List targets) { + public void applyDependencies(BiConsumer consumer, List targets) { Preconditions.checkArgument(targets.size() == 2); - Preconditions.checkArgument(targets.contains(MinecraftJar.Common.NAME)); - Preconditions.checkArgument(targets.contains(MinecraftJar.ClientOnly.NAME)); + Preconditions.checkArgument(targets.contains(MinecraftJar.Type.COMMON)); + Preconditions.checkArgument(targets.contains(MinecraftJar.Type.CLIENT_ONLY)); - consumer.accept(MINECRAFT_COMMON_NAMED.runtime(), MinecraftJar.Common.NAME); - consumer.accept(MINECRAFT_CLIENT_ONLY_NAMED.runtime(), MinecraftJar.ClientOnly.NAME); - consumer.accept(MINECRAFT_COMMON_NAMED.compile(), MinecraftJar.Common.NAME); - consumer.accept(MINECRAFT_CLIENT_ONLY_NAMED.compile(), MinecraftJar.ClientOnly.NAME); + consumer.accept(MINECRAFT_COMMON_NAMED.runtime(), MinecraftJar.Type.COMMON); + consumer.accept(MINECRAFT_CLIENT_ONLY_NAMED.runtime(), MinecraftJar.Type.CLIENT_ONLY); + consumer.accept(MINECRAFT_COMMON_NAMED.compile(), MinecraftJar.Type.COMMON); + consumer.accept(MINECRAFT_CLIENT_ONLY_NAMED.compile(), MinecraftJar.Type.CLIENT_ONLY); } @Override diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/SingleJarEnvType.java b/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/SingleJarEnvType.java index 1101de5f..77745ace 100644 --- a/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/SingleJarEnvType.java +++ b/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/SingleJarEnvType.java @@ -28,22 +28,22 @@ import java.nio.file.Path; import java.util.function.Function; public enum SingleJarEnvType { - CLIENT(MinecraftJar.Client::new, MinecraftJar.Client.NAME), - SERVER(MinecraftJar.Server::new, MinecraftJar.Server.NAME); + CLIENT(MinecraftJar.Client::new, MinecraftJar.Type.CLIENT), + SERVER(MinecraftJar.Server::new, MinecraftJar.Type.SERVER); private final Function jarFunction; - private final String name; + private final MinecraftJar.Type type; - SingleJarEnvType(Function jarFunction, String name) { + SingleJarEnvType(Function jarFunction, MinecraftJar.Type type) { this.jarFunction = jarFunction; - this.name = name; + this.type = type; } public Function getJar() { return jarFunction; } - public String getName() { - return name; + public MinecraftJar.Type getType() { + return type; } } diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/mapped/AbstractMappedMinecraftProvider.java b/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/mapped/AbstractMappedMinecraftProvider.java index aaed5b8c..c81e3084 100644 --- a/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/mapped/AbstractMappedMinecraftProvider.java +++ b/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/mapped/AbstractMappedMinecraftProvider.java @@ -68,7 +68,8 @@ public abstract class AbstractMappedMinecraftProvider getRemappedJars(); - public List getDependencyTargets() { + // Returns a list of MinecraftJar.Type's that this provider exports to be used as a dependency + public List getDependencyTypes() { return Collections.emptyList(); } @@ -87,11 +88,11 @@ public abstract class AbstractMappedMinecraftProvider dependencyTargets = getDependencyTargets(); + final List dependencyTargets = getDependencyTypes(); if (!dependencyTargets.isEmpty()) { MinecraftSourceSets.get(getProject()).applyDependencies( - (configuration, name) -> getProject().getDependencies().add(configuration, getDependencyNotation(name)), + (configuration, type) -> getProject().getDependencies().add(configuration, getDependencyNotation(type)), dependencyTargets ); } @@ -109,8 +110,8 @@ public abstract class AbstractMappedMinecraftProvider remappedJars) { for (RemappedJars remappedJar : remappedJars) { - if (!getMavenHelper(remappedJar.name()).exists(null)) { + if (!getMavenHelper(remappedJar.type()).exists(null)) { return false; } } @@ -209,7 +210,7 @@ public abstract class AbstractMappedMinecraftProvider getMinecraftJars(); interface ProviderImpl extends MappedMinecraftProvider { - Path getJar(String name); + Path getJar(MinecraftJar.Type type); } interface Merged extends ProviderImpl { - String MERGED = MinecraftJar.Merged.NAME; - default MinecraftJar getMergedJar() { - return new MinecraftJar.Merged(getJar(MERGED)); + return new MinecraftJar.Merged(getJar(MinecraftJar.Type.MERGED)); } @Override @@ -55,15 +53,12 @@ public interface MappedMinecraftProvider { } interface Split extends ProviderImpl { - String COMMON = MinecraftJar.Common.NAME; - String CLIENT_ONLY = MinecraftJar.ClientOnly.NAME; - default MinecraftJar getCommonJar() { - return new MinecraftJar.Common(getJar(COMMON)); + return new MinecraftJar.Common(getJar(MinecraftJar.Type.COMMON)); } default MinecraftJar getClientOnlyJar() { - return new MinecraftJar.ClientOnly(getJar(CLIENT_ONLY)); + return new MinecraftJar.ClientOnly(getJar(MinecraftJar.Type.CLIENT_ONLY)); } @Override @@ -75,12 +70,12 @@ public interface MappedMinecraftProvider { interface SingleJar extends ProviderImpl { SingleJarEnvType env(); - default String envName() { - return env().getName(); + default MinecraftJar.Type envType() { + return env().getType(); } default MinecraftJar getEnvOnlyJar() { - return env().getJar().apply(getJar(envName())); + return env().getJar().apply(getJar(env().getType())); } @Override diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/mapped/NamedMinecraftProvider.java b/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/mapped/NamedMinecraftProvider.java index 7534fdb6..1bfdbe72 100644 --- a/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/mapped/NamedMinecraftProvider.java +++ b/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/mapped/NamedMinecraftProvider.java @@ -30,6 +30,7 @@ import org.gradle.api.Project; import net.fabricmc.loom.api.mappings.layered.MappingsNamespace; import net.fabricmc.loom.configuration.providers.minecraft.MergedMinecraftProvider; +import net.fabricmc.loom.configuration.providers.minecraft.MinecraftJar; import net.fabricmc.loom.configuration.providers.minecraft.MinecraftProvider; import net.fabricmc.loom.configuration.providers.minecraft.SingleJarEnvType; import net.fabricmc.loom.configuration.providers.minecraft.SingleJarMinecraftProvider; @@ -64,8 +65,8 @@ public abstract class NamedMinecraftProvider extend } @Override - public List getDependencyTargets() { - return List.of(MERGED); + public List getDependencyTypes() { + return List.of(MinecraftJar.Type.MERGED); } } @@ -88,8 +89,8 @@ public abstract class NamedMinecraftProvider extend } @Override - public List getDependencyTargets() { - return List.of(CLIENT_ONLY, COMMON); + public List getDependencyTypes() { + return List.of(MinecraftJar.Type.CLIENT_ONLY, MinecraftJar.Type.COMMON); } } @@ -117,8 +118,8 @@ public abstract class NamedMinecraftProvider extend } @Override - public List getDependencyTargets() { - return List.of(envName()); + public List getDependencyTypes() { + return List.of(envType()); } @Override diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/mapped/ProcessedNamedMinecraftProvider.java b/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/mapped/ProcessedNamedMinecraftProvider.java index d7dc8bb0..5d28071e 100644 --- a/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/mapped/ProcessedNamedMinecraftProvider.java +++ b/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/mapped/ProcessedNamedMinecraftProvider.java @@ -88,7 +88,7 @@ public abstract class ProcessedNamedMinecraftProvider getDependencyTypes() { + return parentMinecraftProvider.getDependencyTypes(); + } + private void applyDependencies() { - final List dependencyTargets = parentMinecraftProvider.getDependencyTargets(); + final List dependencyTargets = getDependencyTypes(); if (dependencyTargets.isEmpty()) { return; @@ -125,13 +130,13 @@ public abstract class ProcessedNamedMinecraftProvider Date: Wed, 20 Dec 2023 16:58:42 +0000 Subject: [PATCH 17/17] Optimise IncludedJarFactory & ZipReprocessorUtil No longer processes the jar multiple times, caches the jar in place. ZipReprocessorUtil now directly writes the file to disk, instead of first copying it to memory. --- .../build/nesting/IncludedJarFactory.java | 11 ++--- .../loom/task/AbstractRemapJarTask.java | 2 +- .../fabricmc/loom/util/SourceRemapper.java | 2 +- .../loom/util/ZipReprocessorUtil.java | 45 ++++++++++--------- .../loom/test/unit/ZipUtilsTest.groovy | 4 +- 5 files changed, 33 insertions(+), 31 deletions(-) diff --git a/src/main/java/net/fabricmc/loom/build/nesting/IncludedJarFactory.java b/src/main/java/net/fabricmc/loom/build/nesting/IncludedJarFactory.java index 925f5a7c..3cf13e03 100644 --- a/src/main/java/net/fabricmc/loom/build/nesting/IncludedJarFactory.java +++ b/src/main/java/net/fabricmc/loom/build/nesting/IncludedJarFactory.java @@ -94,7 +94,7 @@ public final class IncludedJarFactory { artifact.getClassifier() ); - files.from(project.provider(() -> getNestableJar(artifact.getFile(), metadata))); + files.from(getNestableJar(artifact.getFile(), metadata)); } } @@ -149,7 +149,8 @@ public final class IncludedJarFactory { } LoomGradleExtension extension = LoomGradleExtension.get(project); - File tempDir = new File(extension.getFiles().getProjectBuildCache(), "temp/modprocessing"); + String childName = "temp/modprocessing/%s/%s/%s/%s".formatted(metadata.group().replace(".", "/"), metadata.name(), metadata.version(), input.getName()); + File tempDir = new File(extension.getFiles().getProjectBuildCache(), childName); if (!tempDir.exists()) { tempDir.mkdirs(); @@ -157,13 +158,13 @@ public final class IncludedJarFactory { File tempFile = new File(tempDir, input.getName()); - if (tempFile.exists()) { - tempFile.delete(); + if (tempFile.exists() && FabricModJsonFactory.isModJar(tempFile)) { + return tempFile; } try { FileUtils.copyFile(input, tempFile); - ZipReprocessorUtil.appendZipEntry(tempFile, "fabric.mod.json", generateModForDependency(metadata).getBytes(StandardCharsets.UTF_8)); + ZipReprocessorUtil.appendZipEntry(tempFile.toPath(), "fabric.mod.json", generateModForDependency(metadata).getBytes(StandardCharsets.UTF_8)); } catch (IOException e) { throw new UncheckedIOException("Failed to add dummy mod while including %s".formatted(input), e); } diff --git a/src/main/java/net/fabricmc/loom/task/AbstractRemapJarTask.java b/src/main/java/net/fabricmc/loom/task/AbstractRemapJarTask.java index 7430e0e1..e0f2abf1 100644 --- a/src/main/java/net/fabricmc/loom/task/AbstractRemapJarTask.java +++ b/src/main/java/net/fabricmc/loom/task/AbstractRemapJarTask.java @@ -225,7 +225,7 @@ public abstract class AbstractRemapJarTask extends Jar { final ZipEntryCompression compression = getParameters().getEntryCompression().get(); if (isReproducibleFileOrder || !isPreserveFileTimestamps || compression != ZipEntryCompression.DEFLATED) { - ZipReprocessorUtil.reprocessZip(outputFile.toFile(), isReproducibleFileOrder, isPreserveFileTimestamps, compression); + ZipReprocessorUtil.reprocessZip(outputFile, isReproducibleFileOrder, isPreserveFileTimestamps, compression); } } } diff --git a/src/main/java/net/fabricmc/loom/util/SourceRemapper.java b/src/main/java/net/fabricmc/loom/util/SourceRemapper.java index 1f4fca35..f008293d 100644 --- a/src/main/java/net/fabricmc/loom/util/SourceRemapper.java +++ b/src/main/java/net/fabricmc/loom/util/SourceRemapper.java @@ -72,7 +72,7 @@ public class SourceRemapper { try { logger.progress("remapping sources - " + source.getName()); remapSourcesInner(source, destination); - ZipReprocessorUtil.reprocessZip(destination, reproducibleFileOrder, preserveFileTimestamps); + ZipReprocessorUtil.reprocessZip(destination.toPath(), reproducibleFileOrder, preserveFileTimestamps); // Set the remapped sources creation date to match the sources if we're likely succeeded in making it destination.setLastModified(source.lastModified()); diff --git a/src/main/java/net/fabricmc/loom/util/ZipReprocessorUtil.java b/src/main/java/net/fabricmc/loom/util/ZipReprocessorUtil.java index ab8f92a3..e8bbf075 100644 --- a/src/main/java/net/fabricmc/loom/util/ZipReprocessorUtil.java +++ b/src/main/java/net/fabricmc/loom/util/ZipReprocessorUtil.java @@ -24,12 +24,11 @@ package net.fabricmc.loom.util; -import java.io.ByteArrayOutputStream; -import java.io.File; -import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; +import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.StandardCopyOption; import java.util.Calendar; import java.util.Comparator; import java.util.GregorianCalendar; @@ -87,16 +86,19 @@ public class ZipReprocessorUtil { return name1.compareTo(name2); } - public static void reprocessZip(File file, boolean reproducibleFileOrder, boolean preserveFileTimestamps) throws IOException { + public static void reprocessZip(Path file, boolean reproducibleFileOrder, boolean preserveFileTimestamps) throws IOException { reprocessZip(file, reproducibleFileOrder, preserveFileTimestamps, ZipEntryCompression.DEFLATED); } - public static void reprocessZip(File file, boolean reproducibleFileOrder, boolean preserveFileTimestamps, ZipEntryCompression zipEntryCompression) throws IOException { + public static void reprocessZip(Path file, boolean reproducibleFileOrder, boolean preserveFileTimestamps, ZipEntryCompression zipEntryCompression) throws IOException { if (!reproducibleFileOrder && preserveFileTimestamps) { return; } - try (var zipFile = new ZipFile(file)) { + final Path tempFile = file.resolveSibling(file.getFileName() + ".tmp"); + + try (var zipFile = new ZipFile(file.toFile()); + var fileOutputStream = Files.newOutputStream(tempFile)) { ZipEntry[] entries; if (reproducibleFileOrder) { @@ -108,9 +110,7 @@ public class ZipReprocessorUtil { .toArray(ZipEntry[]::new); } - final var outZip = new ByteArrayOutputStream(entries.length); - - try (var zipOutputStream = new ZipOutputStream(outZip)) { + try (var zipOutputStream = new ZipOutputStream(fileOutputStream)) { zipOutputStream.setMethod(zipOutputStreamCompressionMethod(zipEntryCompression)); for (ZipEntry entry : entries) { @@ -125,11 +125,9 @@ public class ZipReprocessorUtil { copyZipEntry(zipOutputStream, newEntry, zipFile.getInputStream(entry)); } } - - try (var fileOutputStream = new FileOutputStream(file)) { - outZip.writeTo(fileOutputStream); - } } + + Files.move(tempFile, file, StandardCopyOption.REPLACE_EXISTING); } /** @@ -137,15 +135,20 @@ public class ZipReprocessorUtil { * The new entry is added with a constant time stamp to ensure reproducibility. * This method should only be used when a reproducible output is required, use {@link ZipUtils#add(Path, String, byte[])} normally. */ - public static void appendZipEntry(File file, String path, byte[] data) throws IOException { - try (var zipFile = new ZipFile(file)) { + public static void appendZipEntry(Path file, String path, byte[] data) throws IOException { + final Path tempFile = file.resolveSibling(file.getFileName() + ".tmp"); + + try (var zipFile = new ZipFile(file.toFile()); + var fileOutputStream = Files.newOutputStream(tempFile)) { ZipEntry[] entries = zipFile.stream().toArray(ZipEntry[]::new); - final var outZip = new ByteArrayOutputStream(entries.length); - - try (var zipOutputStream = new ZipOutputStream(outZip)) { + try (var zipOutputStream = new ZipOutputStream(fileOutputStream)) { // Copy existing entries for (ZipEntry entry : entries) { + if (entry.getName().equals(path)) { + throw new IllegalArgumentException("Zip file (%s) already contains entry (%s)".formatted(file.getFileName().toString(), path)); + } + copyZipEntry(zipOutputStream, entry, zipFile.getInputStream(entry)); } @@ -156,11 +159,9 @@ public class ZipReprocessorUtil { zipOutputStream.write(data, 0, data.length); zipOutputStream.closeEntry(); } - - try (var fileOutputStream = new FileOutputStream(file)) { - outZip.writeTo(fileOutputStream); - } } + + Files.move(tempFile, file, StandardCopyOption.REPLACE_EXISTING); } private static void copyZipEntry(ZipOutputStream zipOutputStream, ZipEntry entry, InputStream inputStream) throws IOException { diff --git a/src/test/groovy/net/fabricmc/loom/test/unit/ZipUtilsTest.groovy b/src/test/groovy/net/fabricmc/loom/test/unit/ZipUtilsTest.groovy index 1b40e3f3..e967638e 100644 --- a/src/test/groovy/net/fabricmc/loom/test/unit/ZipUtilsTest.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/unit/ZipUtilsTest.groovy @@ -165,11 +165,11 @@ class ZipUtilsTest extends Specification { def fileInside = dir.resolve("text.txt") Files.writeString(fileInside, "hello world") ZipUtils.pack(dir, zip) - ZipReprocessorUtil.reprocessZip(zip.toFile(), true, false) + ZipReprocessorUtil.reprocessZip(zip, true, false) when: // Add an entry to it - ZipReprocessorUtil.appendZipEntry(zip.toFile(), "fabric.mod.json", "Some text".getBytes(StandardCharsets.UTF_8)) + ZipReprocessorUtil.appendZipEntry(zip, "fabric.mod.json", "Some text".getBytes(StandardCharsets.UTF_8)) // Reset the timezone back TimeZone.setDefault(currentTimezone)