From 7dcec8750f56c84f675adcacbdbbb3bb5232f3c6 Mon Sep 17 00:00:00 2001 From: modmuss50 Date: Wed, 8 Nov 2023 22:07:28 +0000 Subject: [PATCH 1/9] Add Fabric-Loom-Mixin-Remap-Type manifest entry --- .../configuration/mods/ArtifactMetadata.java | 33 ++++- .../configuration/mods/MixinDetector.java | 74 ---------- .../mods/ModConfigurationRemapper.java | 2 +- .../loom/configuration/mods/ModProcessor.java | 21 ++- .../mods/dependency/ModDependency.java | 9 +- .../mods/dependency/ModDependencyFactory.java | 7 +- .../mods/dependency/SimpleModDependency.java | 5 +- .../mods/dependency/SplitModDependency.java | 5 +- .../net/fabricmc/loom/task/RemapJarTask.java | 7 + .../test/unit/ArtifactMetadataTest.groovy | 18 +++ .../loom/test/unit/MixinDetectorTest.groovy | 129 ------------------ 11 files changed, 88 insertions(+), 222 deletions(-) delete mode 100644 src/main/java/net/fabricmc/loom/configuration/mods/MixinDetector.java delete mode 100644 src/test/groovy/net/fabricmc/loom/test/unit/MixinDetectorTest.groovy diff --git a/src/main/java/net/fabricmc/loom/configuration/mods/ArtifactMetadata.java b/src/main/java/net/fabricmc/loom/configuration/mods/ArtifactMetadata.java index 024a35c88..adce82744 100644 --- a/src/main/java/net/fabricmc/loom/configuration/mods/ArtifactMetadata.java +++ b/src/main/java/net/fabricmc/loom/configuration/mods/ArtifactMetadata.java @@ -29,6 +29,7 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; +import java.util.Locale; import java.util.function.Predicate; import java.util.jar.Attributes; import java.util.jar.Manifest; @@ -41,15 +42,17 @@ import net.fabricmc.loom.util.FileSystemUtil; import net.fabricmc.loom.util.fmj.FabricModJsonFactory; -public record ArtifactMetadata(boolean isFabricMod, RemapRequirements remapRequirements, @Nullable InstallerData installerData) { +public record ArtifactMetadata(boolean isFabricMod, RemapRequirements remapRequirements, @Nullable InstallerData installerData, RefmapRemapType refmapRemapType) { private static final String INSTALLER_PATH = "fabric-installer.json"; private static final String MANIFEST_PATH = "META-INF/MANIFEST.MF"; private static final String MANIFEST_REMAP_KEY = "Fabric-Loom-Remap"; + public static final String MANIFEST_MIXIN_REMAP_TYPE = "Fabric-Loom-Mixin-Remap-Type"; public static ArtifactMetadata create(ArtifactRef artifact) throws IOException { boolean isFabricMod; RemapRequirements remapRequirements = RemapRequirements.DEFAULT; InstallerData installerData = null; + RefmapRemapType refmapRemapType = RefmapRemapType.MIXIN; try (FileSystemUtil.Delegate fs = FileSystemUtil.getJarFileSystem(artifact.path())) { isFabricMod = FabricModJsonFactory.containsMod(fs); @@ -58,11 +61,20 @@ public static ArtifactMetadata create(ArtifactRef artifact) throws IOException { if (Files.exists(manifestPath)) { final var manifest = new Manifest(new ByteArrayInputStream(Files.readAllBytes(manifestPath))); final Attributes mainAttributes = manifest.getMainAttributes(); - final String value = mainAttributes.getValue(MANIFEST_REMAP_KEY); + final String remapKey = mainAttributes.getValue(MANIFEST_REMAP_KEY); + final String mixinRemapType = mainAttributes.getValue(MANIFEST_MIXIN_REMAP_TYPE); - if (value != null) { + if (remapKey != null) { // Support opting into and out of remapping with "Fabric-Loom-Remap" manifest entry - remapRequirements = Boolean.parseBoolean(value) ? RemapRequirements.OPT_IN : RemapRequirements.OPT_OUT; + remapRequirements = Boolean.parseBoolean(remapKey) ? RemapRequirements.OPT_IN : RemapRequirements.OPT_OUT; + } + + if (mixinRemapType != null) { + try { + refmapRemapType = RefmapRemapType.valueOf(mixinRemapType.toUpperCase(Locale.ROOT)); + } catch (IllegalArgumentException e) { + throw new IllegalStateException("Unknown mixin remap type: " + mixinRemapType); + } } } @@ -74,7 +86,7 @@ public static ArtifactMetadata create(ArtifactRef artifact) throws IOException { } } - return new ArtifactMetadata(isFabricMod, remapRequirements, installerData); + return new ArtifactMetadata(isFabricMod, remapRequirements, installerData, refmapRemapType); } public boolean shouldRemap() { @@ -100,4 +112,15 @@ private Predicate getShouldRemap() { return shouldRemap; } } + + public enum RefmapRemapType { + // Jar uses refmaps, so will be remapped by mixin + MIXIN, + // Jar does not use refmaps, so will be remapped by tiny-remapper + STATIC; + + public String manifestValue() { + return name().toLowerCase(Locale.ROOT); + } + } } diff --git a/src/main/java/net/fabricmc/loom/configuration/mods/MixinDetector.java b/src/main/java/net/fabricmc/loom/configuration/mods/MixinDetector.java deleted file mode 100644 index 6b79c5fea..000000000 --- a/src/main/java/net/fabricmc/loom/configuration/mods/MixinDetector.java +++ /dev/null @@ -1,74 +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.configuration.mods; - -import java.io.BufferedReader; -import java.io.IOException; -import java.nio.file.Files; -import java.nio.file.Path; -import java.util.List; - -import com.google.gson.JsonObject; -import com.google.gson.JsonParseException; - -import net.fabricmc.loom.LoomGradlePlugin; -import net.fabricmc.loom.util.FileSystemUtil; -import net.fabricmc.loom.util.fmj.FabricModJson; -import net.fabricmc.loom.util.fmj.FabricModJsonFactory; - -public final class MixinDetector { - public static boolean hasMixinsWithoutRefmap(Path modJar) throws IOException { - try (FileSystemUtil.Delegate fs = FileSystemUtil.getJarFileSystem(modJar)) { - final List mixinConfigs = getMixinConfigs(modJar); - - if (!mixinConfigs.isEmpty()) { - for (String mixinConfig : mixinConfigs) { - final Path configPath = fs.getPath(mixinConfig); - if (Files.notExists(configPath)) continue; - - try (BufferedReader reader = Files.newBufferedReader(configPath)) { - final JsonObject json = LoomGradlePlugin.GSON.fromJson(reader, JsonObject.class); - - if (!json.has("refmap")) { - // We found a mixin config with no refmap, exit the loop. - return true; - } - } catch (JsonParseException e) { - throw new RuntimeException("Could not parse mixin config %s from jar %s".formatted(mixinConfig, modJar.toAbsolutePath()), e); - } - } - } - - return false; - } - } - - private static List getMixinConfigs(Path modJar) { - // Nullable because we don't care here if we can't read it. - // We can just assume there are no mixins. - final FabricModJson fabricModJson = FabricModJsonFactory.createFromZipNullable(modJar); - return fabricModJson != null ? fabricModJson.getMixinConfigurations() : List.of(); - } -} 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 7ff0fbc78..3d220a071 100644 --- a/src/main/java/net/fabricmc/loom/configuration/mods/ModConfigurationRemapper.java +++ b/src/main/java/net/fabricmc/loom/configuration/mods/ModConfigurationRemapper.java @@ -158,7 +158,7 @@ public static void supplyModConfigurations(Project project, SharedServiceManager continue; } - final ModDependency modDependency = ModDependencyFactory.create(artifact, remappedConfig, clientRemappedConfig, mappingsSuffix, project); + final ModDependency modDependency = ModDependencyFactory.create(artifact, artifactMetadata, remappedConfig, clientRemappedConfig, mappingsSuffix, project); scheduleSourcesRemapping(project, sourceRemapper, modDependency); modDependencies.add(modDependency); } 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 f1f768c15..5e74dd106 100644 --- a/src/main/java/net/fabricmc/loom/configuration/mods/ModProcessor.java +++ b/src/main/java/net/fabricmc/loom/configuration/mods/ModProcessor.java @@ -149,9 +149,14 @@ private void remapJars(List remapList) throws IOException { builder.extension(kotlinRemapperClassloader.getTinyRemapperExtension()); } - final Set hasMixinsWithoutRefmaps = new HashSet<>(); - // Configure the mixin extension to remap mixins from mod jars detected not to contain refmaps. - builder.extension(new MixinExtension(hasMixinsWithoutRefmaps::contains)); + final Set remapMixins = new HashSet<>(); + final boolean requiresStaticMixinRemap = remapList.stream() + .anyMatch(modDependency -> modDependency.getMetadata().refmapRemapType() == ArtifactMetadata.RefmapRemapType.STATIC); + + if (requiresStaticMixinRemap) { + // Configure the mixin extension to remap mixins from mod jars that were remapped with the mixin extension. + builder.extension(new MixinExtension(remapMixins::contains)); + } final TinyRemapper remapper = builder.build(); @@ -182,8 +187,14 @@ private void remapJars(List remapList) throws IOException { // Note: this is done at a jar level, not at the level of an individual mixin config. // If a mod has multiple mixin configs, it's assumed that either all or none of them have refmaps. - if (MixinDetector.hasMixinsWithoutRefmap(info.getInputFile())) { - hasMixinsWithoutRefmaps.add(tag); + if (info.getMetadata().refmapRemapType() == ArtifactMetadata.RefmapRemapType.STATIC) { + if (!requiresStaticMixinRemap) { + // Should be impossible but stranger things have happened. + throw new IllegalStateException("Was not configured for static remap, but a mod required it?!"); + } + + project.getLogger().info("Remapping mixins in {} statically", info.getInputFile()); + remapMixins.add(tag); } remapper.readInputsAsync(tag, info.getInputFile()); diff --git a/src/main/java/net/fabricmc/loom/configuration/mods/dependency/ModDependency.java b/src/main/java/net/fabricmc/loom/configuration/mods/dependency/ModDependency.java index bfcd0b022..166de041c 100644 --- a/src/main/java/net/fabricmc/loom/configuration/mods/dependency/ModDependency.java +++ b/src/main/java/net/fabricmc/loom/configuration/mods/dependency/ModDependency.java @@ -31,10 +31,12 @@ import org.jetbrains.annotations.Nullable; import net.fabricmc.loom.LoomGradleExtension; +import net.fabricmc.loom.configuration.mods.ArtifactMetadata; import net.fabricmc.loom.configuration.mods.ArtifactRef; public abstract sealed class ModDependency permits SplitModDependency, SimpleModDependency { private final ArtifactRef artifact; + private final ArtifactMetadata metadata; protected final String group; protected final String name; protected final String version; @@ -43,8 +45,9 @@ public abstract sealed class ModDependency permits SplitModDependency, SimpleMod protected final String mappingsSuffix; protected final Project project; - public ModDependency(ArtifactRef artifact, String mappingsSuffix, Project project) { + public ModDependency(ArtifactRef artifact, ArtifactMetadata metadata, String mappingsSuffix, Project project) { this.artifact = artifact; + this.metadata = metadata; this.group = artifact.group(); this.name = artifact.name(); this.version = artifact.version(); @@ -78,6 +81,10 @@ public ArtifactRef getInputArtifact() { return artifact; } + public ArtifactMetadata getMetadata() { + return metadata; + } + protected String getRemappedGroup() { return getMappingsPrefix() + "." + group; } diff --git a/src/main/java/net/fabricmc/loom/configuration/mods/dependency/ModDependencyFactory.java b/src/main/java/net/fabricmc/loom/configuration/mods/dependency/ModDependencyFactory.java index f16ddfee9..c1909b576 100644 --- a/src/main/java/net/fabricmc/loom/configuration/mods/dependency/ModDependencyFactory.java +++ b/src/main/java/net/fabricmc/loom/configuration/mods/dependency/ModDependencyFactory.java @@ -33,6 +33,7 @@ import org.jetbrains.annotations.Nullable; import net.fabricmc.loom.LoomGradleExtension; +import net.fabricmc.loom.configuration.mods.ArtifactMetadata; import net.fabricmc.loom.configuration.mods.ArtifactRef; import net.fabricmc.loom.configuration.mods.JarSplitter; import net.fabricmc.loom.util.AttributeHelper; @@ -40,7 +41,7 @@ public class ModDependencyFactory { private static final String TARGET_ATTRIBUTE_KEY = "loom-target"; - public static ModDependency create(ArtifactRef artifact, Configuration targetConfig, @Nullable Configuration targetClientConfig, String mappingsSuffix, Project project) { + public static ModDependency create(ArtifactRef artifact, ArtifactMetadata metadata, Configuration targetConfig, @Nullable Configuration targetClientConfig, String mappingsSuffix, Project project) { if (targetClientConfig != null && LoomGradleExtension.get(project).getSplitModDependencies().get()) { final Optional cachedTarget = readTarget(artifact); JarSplitter.Target target; @@ -53,11 +54,11 @@ public static ModDependency create(ArtifactRef artifact, Configuration targetCon } if (target != null) { - return new SplitModDependency(artifact, mappingsSuffix, targetConfig, targetClientConfig, target, project); + return new SplitModDependency(artifact, metadata, mappingsSuffix, targetConfig, targetClientConfig, target, project); } } - return new SimpleModDependency(artifact, mappingsSuffix, targetConfig, project); + return new SimpleModDependency(artifact, metadata, mappingsSuffix, targetConfig, project); } private static Optional readTarget(ArtifactRef artifact) { diff --git a/src/main/java/net/fabricmc/loom/configuration/mods/dependency/SimpleModDependency.java b/src/main/java/net/fabricmc/loom/configuration/mods/dependency/SimpleModDependency.java index 8a1bd82e6..34c82cf3e 100644 --- a/src/main/java/net/fabricmc/loom/configuration/mods/dependency/SimpleModDependency.java +++ b/src/main/java/net/fabricmc/loom/configuration/mods/dependency/SimpleModDependency.java @@ -32,6 +32,7 @@ import org.gradle.api.artifacts.Configuration; import org.jetbrains.annotations.Nullable; +import net.fabricmc.loom.configuration.mods.ArtifactMetadata; import net.fabricmc.loom.configuration.mods.ArtifactRef; // Single jar in and out @@ -39,8 +40,8 @@ public final class SimpleModDependency extends ModDependency { private final Configuration targetConfig; private final LocalMavenHelper maven; - public SimpleModDependency(ArtifactRef artifact, String mappingsSuffix, Configuration targetConfig, Project project) { - super(artifact, mappingsSuffix, project); + public SimpleModDependency(ArtifactRef artifact, ArtifactMetadata metadata, String mappingsSuffix, Configuration targetConfig, Project project) { + super(artifact, metadata, mappingsSuffix, project); this.targetConfig = Objects.requireNonNull(targetConfig); this.maven = createMaven(name); } diff --git a/src/main/java/net/fabricmc/loom/configuration/mods/dependency/SplitModDependency.java b/src/main/java/net/fabricmc/loom/configuration/mods/dependency/SplitModDependency.java index 0210036e4..7f088357d 100644 --- a/src/main/java/net/fabricmc/loom/configuration/mods/dependency/SplitModDependency.java +++ b/src/main/java/net/fabricmc/loom/configuration/mods/dependency/SplitModDependency.java @@ -34,6 +34,7 @@ import net.fabricmc.loom.LoomGradleExtension; import net.fabricmc.loom.api.ModSettings; +import net.fabricmc.loom.configuration.mods.ArtifactMetadata; import net.fabricmc.loom.configuration.mods.ArtifactRef; import net.fabricmc.loom.configuration.mods.JarSplitter; @@ -47,8 +48,8 @@ public final class SplitModDependency extends ModDependency { @Nullable private final LocalMavenHelper clientMaven; - public SplitModDependency(ArtifactRef artifact, String mappingsSuffix, Configuration targetCommonConfig, Configuration targetClientConfig, JarSplitter.Target target, Project project) { - super(artifact, mappingsSuffix, project); + public SplitModDependency(ArtifactRef artifact, ArtifactMetadata metadata, String mappingsSuffix, Configuration targetCommonConfig, Configuration targetClientConfig, JarSplitter.Target target, Project project) { + super(artifact, metadata, mappingsSuffix, project); this.targetCommonConfig = Objects.requireNonNull(targetCommonConfig); this.targetClientConfig = Objects.requireNonNull(targetClientConfig); this.target = Objects.requireNonNull(target); diff --git a/src/main/java/net/fabricmc/loom/task/RemapJarTask.java b/src/main/java/net/fabricmc/loom/task/RemapJarTask.java index 9639f3d18..1de71cb4e 100644 --- a/src/main/java/net/fabricmc/loom/task/RemapJarTask.java +++ b/src/main/java/net/fabricmc/loom/task/RemapJarTask.java @@ -62,6 +62,7 @@ import net.fabricmc.loom.build.nesting.IncludedJarFactory; import net.fabricmc.loom.build.nesting.JarNester; import net.fabricmc.loom.configuration.accesswidener.AccessWidenerFile; +import net.fabricmc.loom.configuration.mods.ArtifactMetadata; import net.fabricmc.loom.extension.MixinExtension; import net.fabricmc.loom.task.service.TinyRemapperService; import net.fabricmc.loom.util.Constants; @@ -146,6 +147,12 @@ public void run() { if (mixinAp) { setupLegacyMixinRefmapRemapping(params); } + + // Add the mixin refmap remap type to the manifest + // This is used by the mod dependency remapper to determine if it should remap the refmap + // or if the refmap should be remapped by mixin at runtime. + final var refmapRemapType = mixinAp ? ArtifactMetadata.RefmapRemapType.MIXIN : ArtifactMetadata.RefmapRemapType.STATIC; + params.getManifestAttributes().put(ArtifactMetadata.MANIFEST_MIXIN_REMAP_TYPE, refmapRemapType.manifestValue()); }); } diff --git a/src/test/groovy/net/fabricmc/loom/test/unit/ArtifactMetadataTest.groovy b/src/test/groovy/net/fabricmc/loom/test/unit/ArtifactMetadataTest.groovy index 88e881240..5d22b6cc1 100644 --- a/src/test/groovy/net/fabricmc/loom/test/unit/ArtifactMetadataTest.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/unit/ArtifactMetadataTest.groovy @@ -31,6 +31,8 @@ import spock.lang.Specification import net.fabricmc.loom.configuration.mods.ArtifactMetadata import net.fabricmc.loom.configuration.mods.ArtifactRef +import static net.fabricmc.loom.configuration.mods.ArtifactMetadata.RefmapRemapType.MIXIN +import static net.fabricmc.loom.configuration.mods.ArtifactMetadata.RefmapRemapType.STATIC import static net.fabricmc.loom.configuration.mods.ArtifactMetadata.RemapRequirements.* import static net.fabricmc.loom.test.util.ZipTestUtils.* @@ -101,6 +103,22 @@ class ArtifactMetadataTest extends Specification { false | ["fabric.mod.json": "{}"] // Fabric mod, no installer data } + def "Refmap remap type" () { + given: + def zip = createZip(entries) + when: + def metadata = createMetadata(zip) + def result = metadata.refmapRemapType() + then: + result == type + where: + type | entries + MIXIN | ["hello.json": "{}"] // None Mod jar + MIXIN | ["fabric.mod.json": "{}"] // Fabric mod without manfiest file + MIXIN | ["fabric.mod.json": "{}", "META-INF/MANIFEST.MF": manifest("Fabric-Loom-Mixin-Remap-Type", "mixin")] // Fabric mod without remap type entry + STATIC | ["fabric.mod.json": "{}", "META-INF/MANIFEST.MF": manifest("Fabric-Loom-Mixin-Remap-Type", "static")] // Fabric mod opt-in + } + private static ArtifactMetadata createMetadata(Path zip) { return ArtifactMetadata.create(createArtifact(zip)) } diff --git a/src/test/groovy/net/fabricmc/loom/test/unit/MixinDetectorTest.groovy b/src/test/groovy/net/fabricmc/loom/test/unit/MixinDetectorTest.groovy deleted file mode 100644 index 586063600..000000000 --- a/src/test/groovy/net/fabricmc/loom/test/unit/MixinDetectorTest.groovy +++ /dev/null @@ -1,129 +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.test.unit - -import java.nio.file.Path - -import groovy.json.JsonOutput -import spock.lang.Specification -import spock.lang.TempDir - -import net.fabricmc.loom.configuration.mods.MixinDetector -import net.fabricmc.loom.util.FileSystemUtil - -class MixinDetectorTest extends Specification { - @TempDir - Path tempDir - - private Path makeJar(Map mixinConfigs) { - def path = tempDir.resolve("test.jar") - def fs = FileSystemUtil.getJarFileSystem(path, true) - - try { - // Create fabric.mod.json - def fabricModJson = JsonOutput.toJson([ - schemaVersion: 1, - id: 'test', - version: '1', - mixins: mixinConfigs.keySet() - ]) - fs.getPath('fabric.mod.json').text = fabricModJson - - // Write all mixin configs - mixinConfigs.forEach { name, content -> - fs.getPath(name).text = content - } - } finally { - fs.close() - } - - return path - } - - def "jar without mixins has no mixins without refmaps"() { - setup: - def jarPath = makeJar([:]) - - when: - def hasMixinsWithoutRefmaps = MixinDetector.hasMixinsWithoutRefmap(jarPath) - - then: - !hasMixinsWithoutRefmaps // no mixins - } - - def "jar with one mixin config with refmap has no mixins without refmaps"() { - setup: - def jarPath = makeJar([ - 'test.mixins.json': JsonOutput.toJson([ - 'package': 'com.example.test', - 'mixins': ['TestMixin'], - 'refmap': 'test-refmap.json' - ]) - ]) - - when: - def hasMixinsWithoutRefmaps = MixinDetector.hasMixinsWithoutRefmap(jarPath) - - then: - !hasMixinsWithoutRefmaps // no mixins with refmaps - } - - def "jar with one mixin config without refmap has mixins without refmaps"() { - setup: - def jarPath = makeJar([ - 'test.mixins.json': JsonOutput.toJson([ - 'package': 'com.example.test', - 'mixins': ['TestMixin'] - ]) - ]) - - when: - def hasMixinsWithoutRefmaps = MixinDetector.hasMixinsWithoutRefmap(jarPath) - - then: - hasMixinsWithoutRefmaps // mixins with refmaps - } - - def "jar with mixed mixin configs has mixins without refmaps"() { - setup: - def jarPath = makeJar([ - 'test.mixins.json': JsonOutput.toJson([ - 'package': 'com.example.test', - 'mixins': ['TestMixin'] - ]), - 'test2.mixins.json': JsonOutput.toJson([ - 'package': 'com.example.test2', - 'mixins': ['TestMixin2'], - 'refmap': 'test2-refmap.json' - ]) - ]) - - when: - def hasMixinsWithoutRefmaps = MixinDetector.hasMixinsWithoutRefmap(jarPath) - - then: - hasMixinsWithoutRefmaps // mixins with refmaps - } -} From 78b01fdb6926dcc962969423e5b680fe95190885 Mon Sep 17 00:00:00 2001 From: modmuss50 Date: Thu, 9 Nov 2023 13:03:19 +0000 Subject: [PATCH 2/9] Review fixes --- .../loom/configuration/mods/ArtifactMetadata.java | 14 +++++++------- .../loom/configuration/mods/ModProcessor.java | 4 ++-- .../java/net/fabricmc/loom/task/RemapJarTask.java | 2 +- .../loom/test/unit/ArtifactMetadataTest.groovy | 6 +++--- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/main/java/net/fabricmc/loom/configuration/mods/ArtifactMetadata.java b/src/main/java/net/fabricmc/loom/configuration/mods/ArtifactMetadata.java index adce82744..2a7b4859e 100644 --- a/src/main/java/net/fabricmc/loom/configuration/mods/ArtifactMetadata.java +++ b/src/main/java/net/fabricmc/loom/configuration/mods/ArtifactMetadata.java @@ -42,7 +42,7 @@ import net.fabricmc.loom.util.FileSystemUtil; import net.fabricmc.loom.util.fmj.FabricModJsonFactory; -public record ArtifactMetadata(boolean isFabricMod, RemapRequirements remapRequirements, @Nullable InstallerData installerData, RefmapRemapType refmapRemapType) { +public record ArtifactMetadata(boolean isFabricMod, RemapRequirements remapRequirements, @Nullable InstallerData installerData, MixinRemapType mixinRemapType) { private static final String INSTALLER_PATH = "fabric-installer.json"; private static final String MANIFEST_PATH = "META-INF/MANIFEST.MF"; private static final String MANIFEST_REMAP_KEY = "Fabric-Loom-Remap"; @@ -52,7 +52,7 @@ public static ArtifactMetadata create(ArtifactRef artifact) throws IOException { boolean isFabricMod; RemapRequirements remapRequirements = RemapRequirements.DEFAULT; InstallerData installerData = null; - RefmapRemapType refmapRemapType = RefmapRemapType.MIXIN; + MixinRemapType refmapRemapType = MixinRemapType.MIXIN; try (FileSystemUtil.Delegate fs = FileSystemUtil.getJarFileSystem(artifact.path())) { isFabricMod = FabricModJsonFactory.containsMod(fs); @@ -61,17 +61,17 @@ public static ArtifactMetadata create(ArtifactRef artifact) throws IOException { if (Files.exists(manifestPath)) { final var manifest = new Manifest(new ByteArrayInputStream(Files.readAllBytes(manifestPath))); final Attributes mainAttributes = manifest.getMainAttributes(); - final String remapKey = mainAttributes.getValue(MANIFEST_REMAP_KEY); + final String remapValue = mainAttributes.getValue(MANIFEST_REMAP_KEY); final String mixinRemapType = mainAttributes.getValue(MANIFEST_MIXIN_REMAP_TYPE); - if (remapKey != null) { + if (remapValue != null) { // Support opting into and out of remapping with "Fabric-Loom-Remap" manifest entry - remapRequirements = Boolean.parseBoolean(remapKey) ? RemapRequirements.OPT_IN : RemapRequirements.OPT_OUT; + remapRequirements = Boolean.parseBoolean(remapValue) ? RemapRequirements.OPT_IN : RemapRequirements.OPT_OUT; } if (mixinRemapType != null) { try { - refmapRemapType = RefmapRemapType.valueOf(mixinRemapType.toUpperCase(Locale.ROOT)); + refmapRemapType = MixinRemapType.valueOf(mixinRemapType.toUpperCase(Locale.ROOT)); } catch (IllegalArgumentException e) { throw new IllegalStateException("Unknown mixin remap type: " + mixinRemapType); } @@ -113,7 +113,7 @@ private Predicate getShouldRemap() { } } - public enum RefmapRemapType { + public enum MixinRemapType { // Jar uses refmaps, so will be remapped by mixin MIXIN, // Jar does not use refmaps, so will be remapped by tiny-remapper 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 5e74dd106..d3a3d6c3a 100644 --- a/src/main/java/net/fabricmc/loom/configuration/mods/ModProcessor.java +++ b/src/main/java/net/fabricmc/loom/configuration/mods/ModProcessor.java @@ -151,7 +151,7 @@ private void remapJars(List remapList) throws IOException { final Set remapMixins = new HashSet<>(); final boolean requiresStaticMixinRemap = remapList.stream() - .anyMatch(modDependency -> modDependency.getMetadata().refmapRemapType() == ArtifactMetadata.RefmapRemapType.STATIC); + .anyMatch(modDependency -> modDependency.getMetadata().mixinRemapType() == ArtifactMetadata.MixinRemapType.STATIC); if (requiresStaticMixinRemap) { // Configure the mixin extension to remap mixins from mod jars that were remapped with the mixin extension. @@ -187,7 +187,7 @@ private void remapJars(List remapList) throws IOException { // Note: this is done at a jar level, not at the level of an individual mixin config. // If a mod has multiple mixin configs, it's assumed that either all or none of them have refmaps. - if (info.getMetadata().refmapRemapType() == ArtifactMetadata.RefmapRemapType.STATIC) { + if (info.getMetadata().mixinRemapType() == ArtifactMetadata.MixinRemapType.STATIC) { if (!requiresStaticMixinRemap) { // Should be impossible but stranger things have happened. throw new IllegalStateException("Was not configured for static remap, but a mod required it?!"); diff --git a/src/main/java/net/fabricmc/loom/task/RemapJarTask.java b/src/main/java/net/fabricmc/loom/task/RemapJarTask.java index 1de71cb4e..a2ae73688 100644 --- a/src/main/java/net/fabricmc/loom/task/RemapJarTask.java +++ b/src/main/java/net/fabricmc/loom/task/RemapJarTask.java @@ -151,7 +151,7 @@ public void run() { // Add the mixin refmap remap type to the manifest // This is used by the mod dependency remapper to determine if it should remap the refmap // or if the refmap should be remapped by mixin at runtime. - final var refmapRemapType = mixinAp ? ArtifactMetadata.RefmapRemapType.MIXIN : ArtifactMetadata.RefmapRemapType.STATIC; + final var refmapRemapType = mixinAp ? ArtifactMetadata.MixinRemapType.MIXIN : ArtifactMetadata.MixinRemapType.STATIC; params.getManifestAttributes().put(ArtifactMetadata.MANIFEST_MIXIN_REMAP_TYPE, refmapRemapType.manifestValue()); }); } diff --git a/src/test/groovy/net/fabricmc/loom/test/unit/ArtifactMetadataTest.groovy b/src/test/groovy/net/fabricmc/loom/test/unit/ArtifactMetadataTest.groovy index 5d22b6cc1..09406ee98 100644 --- a/src/test/groovy/net/fabricmc/loom/test/unit/ArtifactMetadataTest.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/unit/ArtifactMetadataTest.groovy @@ -31,8 +31,8 @@ import spock.lang.Specification import net.fabricmc.loom.configuration.mods.ArtifactMetadata import net.fabricmc.loom.configuration.mods.ArtifactRef -import static net.fabricmc.loom.configuration.mods.ArtifactMetadata.RefmapRemapType.MIXIN -import static net.fabricmc.loom.configuration.mods.ArtifactMetadata.RefmapRemapType.STATIC +import static net.fabricmc.loom.configuration.mods.ArtifactMetadata.MixinRemapType.MIXIN +import static net.fabricmc.loom.configuration.mods.ArtifactMetadata.MixinRemapType.STATIC import static net.fabricmc.loom.configuration.mods.ArtifactMetadata.RemapRequirements.* import static net.fabricmc.loom.test.util.ZipTestUtils.* @@ -108,7 +108,7 @@ class ArtifactMetadataTest extends Specification { def zip = createZip(entries) when: def metadata = createMetadata(zip) - def result = metadata.refmapRemapType() + def result = metadata.mixinRemapType() then: result == type where: From 057eda721b99d495c62f87a193123866b0f1c655 Mon Sep 17 00:00:00 2001 From: modmuss50 Date: Thu, 9 Nov 2023 13:35:10 +0000 Subject: [PATCH 3/9] Improve fabric API test --- gradle/test.libs.versions.toml | 2 +- .../test/integration/FabricAPITest.groovy | 58 ++++++++++++++----- src/test/resources/patches/fabric_api.patch | 30 +++++++--- 3 files changed, 66 insertions(+), 24 deletions(-) diff --git a/gradle/test.libs.versions.toml b/gradle/test.libs.versions.toml index 1c637f779..d986e9669 100644 --- a/gradle/test.libs.versions.toml +++ b/gradle/test.libs.versions.toml @@ -7,7 +7,7 @@ java-debug = "0.48.0" mixin = "0.11.4+mixin.0.8.5" gradle-nightly = "8.5-20230908221250+0000" -fabric-loader = "0.14.22" +fabric-loader = "0.14.24" fabric-installer = "0.11.1" [libraries] diff --git a/src/test/groovy/net/fabricmc/loom/test/integration/FabricAPITest.groovy b/src/test/groovy/net/fabricmc/loom/test/integration/FabricAPITest.groovy index 4b2600af6..bc0f38095 100644 --- a/src/test/groovy/net/fabricmc/loom/test/integration/FabricAPITest.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/integration/FabricAPITest.groovy @@ -32,20 +32,19 @@ import spock.lang.Unroll import net.fabricmc.loom.test.util.GradleProjectTestTrait import net.fabricmc.loom.test.util.ServerRunner +import net.fabricmc.loom.util.ZipUtils import static net.fabricmc.loom.test.LoomTestConstants.* import static org.gradle.testkit.runner.TaskOutcome.SUCCESS @Timeout(value = 30, unit = TimeUnit.MINUTES) class FabricAPITest extends Specification implements GradleProjectTestTrait { - private static final String API_VERSION = "0.0.0+loom" - @Unroll def "build and run (gradle #version, mixin ap disabled: #disableMixinAp)"() { setup: def gradle = gradleProject( repo: "https://github.com/FabricMC/fabric.git", - commit: "f091af96c53963fadf9dbc391c67bb40e5678a96", + commit: "23e8616e7457d7d4a65119b93952d134607ffc5c", version: version, patch: "fabric_api" ) @@ -53,24 +52,39 @@ class FabricAPITest extends Specification implements GradleProjectTestTrait { gradle.enableMultiProjectOptimisation() // Disable the mixin ap if needed. Fabric API is a large enough test project to see if something breaks. - def mixinApPatch = "" - if (disableMixinAp) { - mixinApPatch = """ - + gradle.buildGradle << """ allprojects { loom.mixin.useLegacyMixinAp = false } """.stripIndent() } - // Set the version to something constant - gradle.buildGradle.text = gradle.buildGradle.text.replace('project.version + "+" + (ENV.GITHUB_RUN_NUMBER ? "" : "local-") + getBranch()', "\"$API_VERSION\"") + mixinApPatch + def minecraftVersion = "23w45a" + def server = ServerRunner.create(gradle.projectDir, minecraftVersion) + .withMod(gradle.getOutputFile("fabric-api-999.0.0.jar")) + + // Test that the dependent mod can be built against the previously built fabric-api + def dependentMod = gradleProject(project: "minimalBase", version: version) + dependentMod.buildGradle << """ + repositories { + mavenLocal() + } + + loom { + loom.mixin.useLegacyMixinAp = ${!disableMixinAp} + } + + dependencies { + minecraft "com.mojang:minecraft:${minecraftVersion}" + mappings "net.fabricmc:yarn:${minecraftVersion}+build.1:v2" - def server = ServerRunner.create(gradle.projectDir, "23w33a") - .withMod(gradle.getOutputFile("fabric-api-${API_VERSION}.jar")) + modImplementation "net.fabricmc.fabric-api:fabric-api:999.0.0" + } + """ when: def result = gradle.run(tasks: [ + "clean", "build", "publishToMavenLocal" ], args: [ @@ -85,19 +99,31 @@ class FabricAPITest extends Specification implements GradleProjectTestTrait { gradle.printOutputFiles() def serverResult = server.run() + def dependentModResult = dependentMod.run(task: "build") + then: result.task(":build").outcome == SUCCESS result.task(":prepareRemapJar").outcome == SUCCESS - new File(gradle.mavenLocalDir, "net/fabricmc/fabric-api/fabric-biome-api-v1/13.0.11/fabric-biome-api-v1-13.0.11.jar").exists() - new File(gradle.mavenLocalDir, "net/fabricmc/fabric-api/fabric-biome-api-v1/13.0.11/fabric-biome-api-v1-13.0.11-sources.jar").exists() + def biomeApiJar = new File(gradle.mavenLocalDir, "net/fabricmc/fabric-api/fabric-biome-api-v1/999.0.0/fabric-biome-api-v1-999.0.0.jar") + def manifest = ZipUtils.unpack(biomeApiJar.toPath(), "META-INF/MANIFEST.MF").toString() + new File(gradle.mavenLocalDir, "net/fabricmc/fabric-api/fabric-biome-api-v1/999.0.0/fabric-biome-api-v1-999.0.0-sources.jar").exists() + + if (disableMixinAp) { + manifest.contains("Fabric-Loom-Mixin-Remap-Type=static") + } else { + manifest.contains("Fabric-Loom-Mixin-Remap-Type=mixin") + } serverResult.successful() - serverResult.output.contains("- fabric-api $API_VERSION") + serverResult.output.contains("- fabric-api 999.0.0") + + dependentModResult.task(":build").outcome == SUCCESS + where: [version, disableMixinAp] << [ - [DEFAULT_GRADLE], - [false, true] + [PRE_RELEASE_GRADLE], + [false, true].shuffled() ].combinations() } } diff --git a/src/test/resources/patches/fabric_api.patch b/src/test/resources/patches/fabric_api.patch index 30d09fef2..77b7e86c7 100644 --- a/src/test/resources/patches/fabric_api.patch +++ b/src/test/resources/patches/fabric_api.patch @@ -1,10 +1,26 @@ diff --git a/build.gradle b/build.gradle ---- a/build.gradle (revision 14d319c0729baf781e171e3c9f845fda55670f1b) -+++ b/build.gradle (date 1688330748664) -@@ -37,17 +37,7 @@ - throw new NullPointerException("Could not find version for " + project.name) - } - +--- a/build.gradle (revision 23e8616e7457d7d4a65119b93952d134607ffc5c) ++++ b/build.gradle (date 1699535194191) +@@ -13,7 +13,7 @@ + + def ENV = System.getenv() + +-version = project.version + "+" + (ENV.GITHUB_RUN_NUMBER ? "" : "local-") + getBranch() ++version = "999.0.0" + logger.lifecycle("Building Fabric: " + version) + + +@@ -22,24 +22,7 @@ + import org.apache.commons.codec.digest.DigestUtils + + def getSubprojectVersion(project) { +- // Get the version from the gradle.properties file +- def version = project.properties["${project.name}-version"] +- +- if (!version) { +- throw new NullPointerException("Could not find version for " + project.name) +- } +- - if (grgit == null) { - return version + "+nogit" - } @@ -16,7 +32,7 @@ diff --git a/build.gradle b/build.gradle - } - - return version + "+" + latestCommits.get(0).id.substring(0, 8) + DigestUtils.sha256Hex(project.rootProject.minecraft_version).substring(0, 2) -+ return version ++ return "999.0.0" } def getBranch() { From 4c7e9c01aacdd91655a50c6f3672361bef6066f3 Mon Sep 17 00:00:00 2001 From: modmuss50 Date: Thu, 9 Nov 2023 14:57:34 +0000 Subject: [PATCH 4/9] Move manifest keys to constants --- .../configuration/mods/ArtifactMetadata.java | 10 +++---- .../loom/configuration/mods/JarSplitter.java | 27 ++++++++++--------- .../loom/configuration/mods/ModProcessor.java | 5 ++-- .../minecraft/MinecraftJarSplitter.java | 6 ++--- .../loom/task/AbstractRemapJarTask.java | 20 +++++--------- .../net/fabricmc/loom/task/RemapJarTask.java | 2 +- .../loom/task/service/JarManifestService.java | 21 +++++++-------- .../net/fabricmc/loom/util/Constants.java | 20 ++++++++++++++ .../loom/util/ZipReprocessorUtil.java | 5 ++-- 9 files changed, 62 insertions(+), 54 deletions(-) diff --git a/src/main/java/net/fabricmc/loom/configuration/mods/ArtifactMetadata.java b/src/main/java/net/fabricmc/loom/configuration/mods/ArtifactMetadata.java index 2a7b4859e..969b53284 100644 --- a/src/main/java/net/fabricmc/loom/configuration/mods/ArtifactMetadata.java +++ b/src/main/java/net/fabricmc/loom/configuration/mods/ArtifactMetadata.java @@ -39,14 +39,12 @@ import net.fabricmc.loom.LoomGradlePlugin; import net.fabricmc.loom.configuration.InstallerData; +import net.fabricmc.loom.util.Constants; import net.fabricmc.loom.util.FileSystemUtil; import net.fabricmc.loom.util.fmj.FabricModJsonFactory; public record ArtifactMetadata(boolean isFabricMod, RemapRequirements remapRequirements, @Nullable InstallerData installerData, MixinRemapType mixinRemapType) { private static final String INSTALLER_PATH = "fabric-installer.json"; - private static final String MANIFEST_PATH = "META-INF/MANIFEST.MF"; - private static final String MANIFEST_REMAP_KEY = "Fabric-Loom-Remap"; - public static final String MANIFEST_MIXIN_REMAP_TYPE = "Fabric-Loom-Mixin-Remap-Type"; public static ArtifactMetadata create(ArtifactRef artifact) throws IOException { boolean isFabricMod; @@ -56,13 +54,13 @@ public static ArtifactMetadata create(ArtifactRef artifact) throws IOException { try (FileSystemUtil.Delegate fs = FileSystemUtil.getJarFileSystem(artifact.path())) { isFabricMod = FabricModJsonFactory.containsMod(fs); - final Path manifestPath = fs.getPath(MANIFEST_PATH); + final Path manifestPath = fs.getPath(Constants.Manifest.PATH); if (Files.exists(manifestPath)) { final var manifest = new Manifest(new ByteArrayInputStream(Files.readAllBytes(manifestPath))); final Attributes mainAttributes = manifest.getMainAttributes(); - final String remapValue = mainAttributes.getValue(MANIFEST_REMAP_KEY); - final String mixinRemapType = mainAttributes.getValue(MANIFEST_MIXIN_REMAP_TYPE); + final String remapValue = mainAttributes.getValue(Constants.Manifest.REMAP_KEY); + final String mixinRemapType = mainAttributes.getValue(Constants.Manifest.MIXIN_REMAP_TYPE); if (remapValue != null) { // Support opting into and out of remapping with "Fabric-Loom-Remap" manifest entry diff --git a/src/main/java/net/fabricmc/loom/configuration/mods/JarSplitter.java b/src/main/java/net/fabricmc/loom/configuration/mods/JarSplitter.java index 6090d0eca..50cb05c32 100644 --- a/src/main/java/net/fabricmc/loom/configuration/mods/JarSplitter.java +++ b/src/main/java/net/fabricmc/loom/configuration/mods/JarSplitter.java @@ -43,11 +43,12 @@ import org.jetbrains.annotations.Nullable; -import net.fabricmc.loom.task.AbstractRemapJarTask; +import net.fabricmc.loom.util.Constants; import net.fabricmc.loom.util.FileSystemUtil; public class JarSplitter { - public static final String MANIFEST_SPLIT_ENV_NAME_KEY = "Fabric-Loom-Split-Environment-Name"; + private static final Attributes.Name MANIFEST_SPLIT_ENV_NAME = new Attributes.Name(Constants.Manifest.SPLIT_ENV); + private static final Attributes.Name MANIFEST_CLIENT_ENTRIES_NAME = new Attributes.Name(Constants.Manifest.CLIENT_ENTRIES); final Path inputJar; @@ -58,9 +59,9 @@ public JarSplitter(Path inputJar) { @Nullable public Target analyseTarget() { try (FileSystemUtil.Delegate input = FileSystemUtil.getJarFileSystem(inputJar)) { - final Manifest manifest = input.fromInputStream(Manifest::new, AbstractRemapJarTask.MANIFEST_PATH); + final Manifest manifest = input.fromInputStream(Manifest::new, Constants.Manifest.PATH); - if (!Boolean.parseBoolean(manifest.getMainAttributes().getValue(AbstractRemapJarTask.MANIFEST_SPLIT_ENV_KEY))) { + if (!Boolean.parseBoolean(manifest.getMainAttributes().getValue(Constants.Manifest.SPLIT_ENV))) { // Jar was not built with splitting enabled. return null; } @@ -122,9 +123,9 @@ public boolean split(Path commonOutputJar, Path clientOutputJar) throws IOExcept Files.deleteIfExists(clientOutputJar); try (FileSystemUtil.Delegate input = FileSystemUtil.getJarFileSystem(inputJar)) { - final Manifest manifest = input.fromInputStream(Manifest::new, AbstractRemapJarTask.MANIFEST_PATH); + final Manifest manifest = input.fromInputStream(Manifest::new, Constants.Manifest.PATH); - if (!Boolean.parseBoolean(manifest.getMainAttributes().getValue(AbstractRemapJarTask.MANIFEST_SPLIT_ENV_KEY))) { + if (!Boolean.parseBoolean(manifest.getMainAttributes().getValue(Constants.Manifest.SPLIT_ENV))) { throw new UnsupportedOperationException("Cannot split jar that has not been built with a split env"); } @@ -157,7 +158,7 @@ public boolean split(Path commonOutputJar, Path clientOutputJar) throws IOExcept final String entryPath = relativePath.toString(); - if (entryPath.equals(AbstractRemapJarTask.MANIFEST_PATH)) { + if (entryPath.equals(Constants.Manifest.PATH)) { continue; } @@ -183,11 +184,11 @@ public boolean split(Path commonOutputJar, Path clientOutputJar) throws IOExcept stripSignatureData(outManifest); attributes.remove(Attributes.Name.SIGNATURE_VERSION); - Objects.requireNonNull(attributes.remove(AbstractRemapJarTask.MANIFEST_SPLIT_ENV_NAME)); - Objects.requireNonNull(attributes.remove(AbstractRemapJarTask.MANIFEST_CLIENT_ENTRIES_NAME)); + Objects.requireNonNull(attributes.remove(MANIFEST_SPLIT_ENV_NAME)); + Objects.requireNonNull(attributes.remove(MANIFEST_CLIENT_ENTRIES_NAME)); - writeBytes(writeWithEnvironment(outManifest, "common"), commonOutput.getPath(AbstractRemapJarTask.MANIFEST_PATH)); - writeBytes(writeWithEnvironment(outManifest, "client"), clientOutput.getPath(AbstractRemapJarTask.MANIFEST_PATH)); + writeBytes(writeWithEnvironment(outManifest, "common"), commonOutput.getPath(Constants.Manifest.PATH)); + writeBytes(writeWithEnvironment(outManifest, "client"), clientOutput.getPath(Constants.Manifest.PATH)); } } @@ -197,7 +198,7 @@ public boolean split(Path commonOutputJar, Path clientOutputJar) throws IOExcept private byte[] writeWithEnvironment(Manifest in, String value) throws IOException { final Manifest manifest = new Manifest(in); final Attributes attributes = manifest.getMainAttributes(); - attributes.putValue(MANIFEST_SPLIT_ENV_NAME_KEY, value); + attributes.putValue(Constants.Manifest.SPLIT_ENV_NAME, value); final ByteArrayOutputStream out = new ByteArrayOutputStream(); manifest.write(out); @@ -206,7 +207,7 @@ private byte[] writeWithEnvironment(Manifest in, String value) throws IOExceptio private List readClientEntries(Manifest manifest) { final Attributes attributes = manifest.getMainAttributes(); - final String clientEntriesValue = attributes.getValue(AbstractRemapJarTask.MANIFEST_CLIENT_ENTRIES_KEY); + final String clientEntriesValue = attributes.getValue(Constants.Manifest.CLIENT_ENTRIES); if (clientEntriesValue == null || clientEntriesValue.isBlank()) { return Collections.emptyList(); 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 d3a3d6c3a..a4cf4503e 100644 --- a/src/main/java/net/fabricmc/loom/configuration/mods/ModProcessor.java +++ b/src/main/java/net/fabricmc/loom/configuration/mods/ModProcessor.java @@ -51,7 +51,6 @@ 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.task.RemapJarTask; import net.fabricmc.loom.util.Constants; import net.fabricmc.loom.util.Pair; import net.fabricmc.loom.util.TinyRemapperHelper; @@ -254,10 +253,10 @@ private static Path getRemappedOutput(ModDependency dependency) { } private void remapJarManifestEntries(Path jar) throws IOException { - ZipUtils.transform(jar, Map.of(RemapJarTask.MANIFEST_PATH, bytes -> { + ZipUtils.transform(jar, Map.of(Constants.Manifest.PATH, bytes -> { var manifest = new Manifest(new ByteArrayInputStream(bytes)); - manifest.getMainAttributes().putValue(RemapJarTask.MANIFEST_NAMESPACE_KEY, toM); + manifest.getMainAttributes().putValue(Constants.Manifest.MAPPING_NAMESPACE, toM); ByteArrayOutputStream out = new ByteArrayOutputStream(); manifest.write(out); diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/MinecraftJarSplitter.java b/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/MinecraftJarSplitter.java index 7e11eba69..6df66011c 100644 --- a/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/MinecraftJarSplitter.java +++ b/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/MinecraftJarSplitter.java @@ -39,7 +39,7 @@ import com.google.common.collect.Sets; -import net.fabricmc.loom.configuration.mods.JarSplitter; +import net.fabricmc.loom.util.Constants; import net.fabricmc.loom.util.FileSystemUtil; public class MinecraftJarSplitter implements AutoCloseable { @@ -132,11 +132,11 @@ private void copyEntriesToJar(Set entries, Path inputJar, Path outputJar private void writeManifest(FileSystemUtil.Delegate outputFs, String env) throws IOException { final Manifest manifest = new Manifest(); manifest.getMainAttributes().put(Attributes.Name.MANIFEST_VERSION, "1.0"); - manifest.getMainAttributes().putValue(JarSplitter.MANIFEST_SPLIT_ENV_NAME_KEY, env); + manifest.getMainAttributes().putValue(Constants.Manifest.SPLIT_ENV_NAME, env); ByteArrayOutputStream out = new ByteArrayOutputStream(); manifest.write(out); Files.createDirectories(outputFs.get().getPath("META-INF")); - Files.write(outputFs.get().getPath("META-INF/MANIFEST.MF"), out.toByteArray()); + Files.write(outputFs.get().getPath(Constants.Manifest.PATH), out.toByteArray()); } @Override diff --git a/src/main/java/net/fabricmc/loom/task/AbstractRemapJarTask.java b/src/main/java/net/fabricmc/loom/task/AbstractRemapJarTask.java index 7da1ea1e5..b2ff10608 100644 --- a/src/main/java/net/fabricmc/loom/task/AbstractRemapJarTask.java +++ b/src/main/java/net/fabricmc/loom/task/AbstractRemapJarTask.java @@ -35,7 +35,6 @@ import java.util.Map; import java.util.Set; import java.util.function.Function; -import java.util.jar.Attributes; import java.util.jar.Manifest; import javax.inject.Inject; @@ -66,19 +65,12 @@ import net.fabricmc.loom.LoomGradleExtension; import net.fabricmc.loom.api.mappings.layered.MappingsNamespace; import net.fabricmc.loom.task.service.JarManifestService; +import net.fabricmc.loom.util.Constants; import net.fabricmc.loom.util.ZipReprocessorUtil; import net.fabricmc.loom.util.ZipUtils; import net.fabricmc.loom.util.gradle.SourceSetHelper; public abstract class AbstractRemapJarTask extends Jar { - public static final String MANIFEST_PATH = "META-INF/MANIFEST.MF"; - public static final String MANIFEST_NAMESPACE_KEY = "Fabric-Mapping-Namespace"; - public static final String MANIFEST_SPLIT_ENV_KEY = "Fabric-Loom-Split-Environment"; - public static final String MANIFEST_CLIENT_ENTRIES_KEY = "Fabric-Loom-Client-Only-Entries"; - public static final String MANIFEST_JAR_TYPE_KEY = "Fabric-Jar-Type"; - public static final Attributes.Name MANIFEST_SPLIT_ENV_NAME = new Attributes.Name(MANIFEST_SPLIT_ENV_KEY); - public static final Attributes.Name MANIFEST_CLIENT_ENTRIES_NAME = new Attributes.Name(MANIFEST_CLIENT_ENTRIES_KEY); - @InputFile public abstract RegularFileProperty getInputFile(); @@ -157,7 +149,7 @@ public final

void submitWork(Class entries) { params.getManifestAttributes().set(Map.of( - MANIFEST_SPLIT_ENV_KEY, "true", - MANIFEST_CLIENT_ENTRIES_KEY, String.join(";", entries) + Constants.Manifest.SPLIT_ENV, "true", + Constants.Manifest.CLIENT_ENTRIES, String.join(";", entries) )); } @@ -201,11 +193,11 @@ public AbstractRemapAction() { } protected void modifyJarManifest() throws IOException { - int count = ZipUtils.transform(outputFile, Map.of(MANIFEST_PATH, bytes -> { + int count = ZipUtils.transform(outputFile, Map.of(Constants.Manifest.PATH, bytes -> { var manifest = new Manifest(new ByteArrayInputStream(bytes)); getParameters().getJarManifestService().get().apply(manifest, getParameters().getManifestAttributes().get()); - manifest.getMainAttributes().putValue(MANIFEST_NAMESPACE_KEY, getParameters().getTargetNamespace().get()); + manifest.getMainAttributes().putValue(Constants.Manifest.MAPPING_NAMESPACE, getParameters().getTargetNamespace().get()); ByteArrayOutputStream out = new ByteArrayOutputStream(); manifest.write(out); diff --git a/src/main/java/net/fabricmc/loom/task/RemapJarTask.java b/src/main/java/net/fabricmc/loom/task/RemapJarTask.java index a2ae73688..d04280a37 100644 --- a/src/main/java/net/fabricmc/loom/task/RemapJarTask.java +++ b/src/main/java/net/fabricmc/loom/task/RemapJarTask.java @@ -152,7 +152,7 @@ public void run() { // This is used by the mod dependency remapper to determine if it should remap the refmap // or if the refmap should be remapped by mixin at runtime. final var refmapRemapType = mixinAp ? ArtifactMetadata.MixinRemapType.MIXIN : ArtifactMetadata.MixinRemapType.STATIC; - params.getManifestAttributes().put(ArtifactMetadata.MANIFEST_MIXIN_REMAP_TYPE, refmapRemapType.manifestValue()); + params.getManifestAttributes().put(Constants.Manifest.MIXIN_REMAP_TYPE, refmapRemapType.manifestValue()); }); } diff --git a/src/main/java/net/fabricmc/loom/task/service/JarManifestService.java b/src/main/java/net/fabricmc/loom/task/service/JarManifestService.java index 5b86fbc8a..08f176328 100644 --- a/src/main/java/net/fabricmc/loom/task/service/JarManifestService.java +++ b/src/main/java/net/fabricmc/loom/task/service/JarManifestService.java @@ -25,7 +25,6 @@ package net.fabricmc.loom.task.service; import java.io.Serializable; -import java.util.Comparator; import java.util.Map; import java.util.Optional; import java.util.jar.Attributes; @@ -78,7 +77,7 @@ public void apply(Manifest manifest, Map extraValues) { Attributes attributes = manifest.getMainAttributes(); extraValues.entrySet().stream() - .sorted(Comparator.comparing(Map.Entry::getKey)) + .sorted(Map.Entry.comparingByKey()) .forEach(entry -> { attributes.putValue(entry.getKey(), entry.getValue()); }); @@ -90,17 +89,17 @@ public void apply(Manifest manifest, Map extraValues) { Params p = getParameters(); - attributes.putValue("Fabric-Gradle-Version", p.getGradleVersion().get()); - attributes.putValue("Fabric-Loom-Version", p.getLoomVersion().get()); - attributes.putValue("Fabric-Mixin-Compile-Extensions-Version", p.getMCEVersion().get()); - attributes.putValue("Fabric-Minecraft-Version", p.getMinecraftVersion().get()); - attributes.putValue("Fabric-Tiny-Remapper-Version", p.getTinyRemapperVersion().get()); - attributes.putValue("Fabric-Loader-Version", p.getFabricLoaderVersion().get()); + attributes.putValue(Constants.Manifest.GRADLE_VERSION, p.getGradleVersion().get()); + attributes.putValue(Constants.Manifest.LOOM_VERSION, p.getLoomVersion().get()); + attributes.putValue(Constants.Manifest.MIXIN_COMPILE_EXTENSIONS_VERSION, p.getMCEVersion().get()); + attributes.putValue(Constants.Manifest.MINECRAFT_VERSION, p.getMinecraftVersion().get()); + attributes.putValue(Constants.Manifest.TINY_REMAPPER_VERSION, p.getTinyRemapperVersion().get()); + attributes.putValue(Constants.Manifest.FABRIC_LOADER_VERSION, p.getFabricLoaderVersion().get()); // This can be overridden by mods if required - if (!attributes.containsKey("Fabric-Mixin-Version")) { - attributes.putValue("Fabric-Mixin-Version", p.getMixinVersion().get().version()); - attributes.putValue("Fabric-Mixin-Group", p.getMixinVersion().get().group()); + if (!attributes.containsKey(Constants.Manifest.MIXIN_VERSION)) { + attributes.putValue(Constants.Manifest.MIXIN_VERSION, p.getMixinVersion().get().version()); + attributes.putValue(Constants.Manifest.MIXIN_GROUP, p.getMixinVersion().get().group()); } } diff --git a/src/main/java/net/fabricmc/loom/util/Constants.java b/src/main/java/net/fabricmc/loom/util/Constants.java index 42692aabc..100835711 100644 --- a/src/main/java/net/fabricmc/loom/util/Constants.java +++ b/src/main/java/net/fabricmc/loom/util/Constants.java @@ -124,4 +124,24 @@ public static final class Properties { public static final String DISABLE_PROJECT_DEPENDENT_MODS = "fabric.loom.disableProjectDependentMods"; public static final String LIBRARY_PROCESSORS = "fabric.loom.libraryProcessors"; } + + public static final class Manifest { + public static final String PATH = "META-INF/MANIFEST.MF"; + + public static final String REMAP_KEY = "Fabric-Loom-Remap"; + public static final String MIXIN_REMAP_TYPE = "Fabric-Loom-Mixin-Remap-Type"; + public static final String MAPPING_NAMESPACE = "Fabric-Mapping-Namespace"; + public static final String SPLIT_ENV = "Fabric-Loom-Split-Environment"; + public static final String SPLIT_ENV_NAME = "Fabric-Loom-Split-Environment-Name"; + public static final String CLIENT_ENTRIES = "Fabric-Loom-Client-Only-Entries"; + public static final String JAR_TYPE = "Fabric-Jar-Type"; + public static final String GRADLE_VERSION = "Fabric-Gradle-Version"; + public static final String LOOM_VERSION = "Fabric-Loom-Version"; + public static final String MIXIN_COMPILE_EXTENSIONS_VERSION = "Fabric-Mixin-Compile-Extensions-Version"; + public static final String MINECRAFT_VERSION = "Fabric-Minecraft-Version"; + public static final String TINY_REMAPPER_VERSION = "Fabric-Tiny-Remapper-Version"; + public static final String FABRIC_LOADER_VERSION = "Fabric-Loader-Version"; + public static final String MIXIN_VERSION = "Fabric-Mixin-Version"; + public static final String MIXIN_GROUP = "Fabric-Mixin-Group"; + } } diff --git a/src/main/java/net/fabricmc/loom/util/ZipReprocessorUtil.java b/src/main/java/net/fabricmc/loom/util/ZipReprocessorUtil.java index d962be8c4..ab8f92a33 100644 --- a/src/main/java/net/fabricmc/loom/util/ZipReprocessorUtil.java +++ b/src/main/java/net/fabricmc/loom/util/ZipReprocessorUtil.java @@ -43,7 +43,6 @@ public class ZipReprocessorUtil { private ZipReprocessorUtil() { } - private static final String MANIFEST_LOCATION = "META-INF/MANIFEST.MF"; private static final String META_INF = "META-INF/"; // See https://docs.oracle.com/en/java/javase/20/docs/specs/jar/jar.html#signed-jar-file @@ -68,9 +67,9 @@ private static boolean isSpecialFile(String zipEntryName) { private static int specialOrdering(String name1, String name2) { if (name1.equals(name2)) { return 0; - } else if (name1.equals(MANIFEST_LOCATION)) { + } else if (name1.equals(Constants.Manifest.PATH)) { return -1; - } else if (name2.equals(MANIFEST_LOCATION)) { + } else if (name2.equals(Constants.Manifest.PATH)) { return 1; } From d0612f307e5b36fe9d00bce95a46a4507b8b3c75 Mon Sep 17 00:00:00 2001 From: modmuss50 Date: Wed, 15 Nov 2023 14:01:44 +0000 Subject: [PATCH 5/9] Fix build --- .../fabricmc/loom/test/integration/MultiMcVersionTest.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/groovy/net/fabricmc/loom/test/integration/MultiMcVersionTest.groovy b/src/test/groovy/net/fabricmc/loom/test/integration/MultiMcVersionTest.groovy index 8b15a8fd4..d6280a468 100644 --- a/src/test/groovy/net/fabricmc/loom/test/integration/MultiMcVersionTest.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/integration/MultiMcVersionTest.groovy @@ -33,7 +33,7 @@ import static net.fabricmc.loom.test.LoomTestConstants.STANDARD_TEST_VERSIONS import static org.gradle.testkit.runner.TaskOutcome.SUCCESS class MultiMcVersionTest extends Specification implements GradleProjectTestTrait { - static def versions = [ + static List versions = [ 'fabric-1.14.4', 'fabric-1.15', 'fabric-1.15.2', From 49da9bc9314673123ff61da87d9ae90350965fb3 Mon Sep 17 00:00:00 2001 From: modmuss50 Date: Wed, 15 Nov 2023 14:20:17 +0000 Subject: [PATCH 6/9] Prevent depending on jars built with newer loom versions. --- .../configuration/mods/ArtifactMetadata.java | 26 +++++++++- .../mods/ModConfigurationRemapper.java | 6 ++- .../test/unit/ArtifactMetadataTest.groovy | 49 +++++++++++++++++-- 3 files changed, 75 insertions(+), 6 deletions(-) diff --git a/src/main/java/net/fabricmc/loom/configuration/mods/ArtifactMetadata.java b/src/main/java/net/fabricmc/loom/configuration/mods/ArtifactMetadata.java index 969b53284..d2aa3a198 100644 --- a/src/main/java/net/fabricmc/loom/configuration/mods/ArtifactMetadata.java +++ b/src/main/java/net/fabricmc/loom/configuration/mods/ArtifactMetadata.java @@ -46,7 +46,7 @@ public record ArtifactMetadata(boolean isFabricMod, RemapRequirements remapRequirements, @Nullable InstallerData installerData, MixinRemapType mixinRemapType) { private static final String INSTALLER_PATH = "fabric-installer.json"; - public static ArtifactMetadata create(ArtifactRef artifact) throws IOException { + public static ArtifactMetadata create(ArtifactRef artifact, String currentLoomVersion) throws IOException { boolean isFabricMod; RemapRequirements remapRequirements = RemapRequirements.DEFAULT; InstallerData installerData = null; @@ -60,6 +60,7 @@ public static ArtifactMetadata create(ArtifactRef artifact) throws IOException { final var manifest = new Manifest(new ByteArrayInputStream(Files.readAllBytes(manifestPath))); final Attributes mainAttributes = manifest.getMainAttributes(); final String remapValue = mainAttributes.getValue(Constants.Manifest.REMAP_KEY); + final String loomVersion = mainAttributes.getValue(Constants.Manifest.LOOM_VERSION); final String mixinRemapType = mainAttributes.getValue(Constants.Manifest.MIXIN_REMAP_TYPE); if (remapValue != null) { @@ -67,6 +68,10 @@ public static ArtifactMetadata create(ArtifactRef artifact) throws IOException { remapRequirements = Boolean.parseBoolean(remapValue) ? RemapRequirements.OPT_IN : RemapRequirements.OPT_OUT; } + if (loomVersion != null) { + validateLoomVersion(loomVersion, currentLoomVersion); + } + if (mixinRemapType != null) { try { refmapRemapType = MixinRemapType.valueOf(mixinRemapType.toUpperCase(Locale.ROOT)); @@ -87,6 +92,25 @@ public static ArtifactMetadata create(ArtifactRef artifact) throws IOException { return new ArtifactMetadata(isFabricMod, remapRequirements, installerData, refmapRemapType); } + // Validates that the version matches or is less than the current loom version + private static void validateLoomVersion(String version, String currentLoomVersion) { + final String[] versionParts = version.split("\\."); + final String[] currentVersionParts = currentLoomVersion.split("\\."); + + // Check major and minor version + for (int i = 0; i < 2; i++) { + final int versionPart = Integer.parseInt(versionParts[i]); + final int currentVersionPart = Integer.parseInt(currentVersionParts[i]); + + if (versionPart > currentVersionPart) { + throw new IllegalStateException("Mod was built with a newer version of Loom (%s), you are using Loom (%s)".formatted(version, currentLoomVersion)); + } else if (versionPart < currentVersionPart) { + // Older version, no need to check further + break; + } + } + } + public boolean shouldRemap() { return remapRequirements().getShouldRemap().test(this); } 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 3d220a071..166bf59da 100644 --- a/src/main/java/net/fabricmc/loom/configuration/mods/ModConfigurationRemapper.java +++ b/src/main/java/net/fabricmc/loom/configuration/mods/ModConfigurationRemapper.java @@ -56,6 +56,7 @@ import org.jetbrains.annotations.Nullable; import net.fabricmc.loom.LoomGradleExtension; +import net.fabricmc.loom.LoomGradlePlugin; import net.fabricmc.loom.api.RemapConfigurationSettings; import net.fabricmc.loom.configuration.RemapConfigurations; import net.fabricmc.loom.configuration.mods.dependency.ModDependency; @@ -63,6 +64,7 @@ import net.fabricmc.loom.configuration.providers.minecraft.MinecraftSourceSets; import net.fabricmc.loom.util.Checksum; import net.fabricmc.loom.util.Constants; +import net.fabricmc.loom.util.ExceptionUtil; import net.fabricmc.loom.util.SourceRemapper; import net.fabricmc.loom.util.gradle.SourceSetHelper; import net.fabricmc.loom.util.service.SharedServiceManager; @@ -137,9 +139,9 @@ public static void supplyModConfigurations(Project project, SharedServiceManager final ArtifactMetadata artifactMetadata; try { - artifactMetadata = ArtifactMetadata.create(artifact); + artifactMetadata = ArtifactMetadata.create(artifact, LoomGradlePlugin.LOOM_VERSION); } catch (IOException e) { - throw new UncheckedIOException("Failed to read metadata from" + artifact.path(), e); + throw ExceptionUtil.createDescriptiveWrapper(UncheckedIOException::new, "Failed to read metadata from " + artifact.path(), e); } if (artifactMetadata.installerData() != null) { diff --git a/src/test/groovy/net/fabricmc/loom/test/unit/ArtifactMetadataTest.groovy b/src/test/groovy/net/fabricmc/loom/test/unit/ArtifactMetadataTest.groovy index 09406ee98..105aa58a3 100644 --- a/src/test/groovy/net/fabricmc/loom/test/unit/ArtifactMetadataTest.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/unit/ArtifactMetadataTest.groovy @@ -34,7 +34,8 @@ import net.fabricmc.loom.configuration.mods.ArtifactRef import static net.fabricmc.loom.configuration.mods.ArtifactMetadata.MixinRemapType.MIXIN import static net.fabricmc.loom.configuration.mods.ArtifactMetadata.MixinRemapType.STATIC import static net.fabricmc.loom.configuration.mods.ArtifactMetadata.RemapRequirements.* -import static net.fabricmc.loom.test.util.ZipTestUtils.* +import static net.fabricmc.loom.test.util.ZipTestUtils.createZip +import static net.fabricmc.loom.test.util.ZipTestUtils.manifest class ArtifactMetadataTest extends Specification { def "is fabric mod"() { @@ -119,8 +120,50 @@ class ArtifactMetadataTest extends Specification { STATIC | ["fabric.mod.json": "{}", "META-INF/MANIFEST.MF": manifest("Fabric-Loom-Mixin-Remap-Type", "static")] // Fabric mod opt-in } - private static ArtifactMetadata createMetadata(Path zip) { - return ArtifactMetadata.create(createArtifact(zip)) + // Test that a mod with the same or older version of loom can be read + def "Valid loom version"() { + given: + def zip = createMod(modLoomVersion) + when: + def metadata = createMetadata(zip, loomVersion) + then: + metadata != null + where: + loomVersion | modLoomVersion + "1.4" | "1.0.1" + "1.4" | "1.0.99" + "1.4" | "1.4" + "1.4" | "1.4.0" + "1.4" | "1.4.1" + "1.4" | "1.4.99" + "1.4" | "1.4.local" + "1.5" | "1.4.99" + "2.0" | "1.4.99" + } + + // Test that a mod with the same or older version of loom can be read + def "Invalid loom version"() { + given: + def zip = createMod(modLoomVersion) + when: + def metadata = createMetadata(zip, loomVersion) + then: + def e = thrown(IllegalStateException) + e.message == "Mod was built with a newer version of Loom ($modLoomVersion), you are using Loom ($loomVersion)" + where: + loomVersion | modLoomVersion + "1.4" | "1.5" + "1.4" | "1.5.00" + "1.4" | "2.0" + "1.4" | "2.4" + } + + private static Path createMod(String loomVersion) { + return createZip(["fabric.mod.json": "{}", "META-INF/MANIFEST.MF": manifest("Fabric-Loom-Version", loomVersion)]) + } + + private static ArtifactMetadata createMetadata(Path zip, String loomVersion = "1.4") { + return ArtifactMetadata.create(createArtifact(zip), loomVersion) } private static ArtifactRef createArtifact(Path zip) { From 17a7524d7d843d6e1604b58ceecff6b2140d9cf1 Mon Sep 17 00:00:00 2001 From: modmuss50 Date: Wed, 15 Nov 2023 18:55:15 +0000 Subject: [PATCH 7/9] Only validate loom versions for jars remapped with TR. --- .../configuration/mods/ArtifactMetadata.java | 14 +++++--- .../test/unit/ArtifactMetadataTest.groovy | 34 ++++++++++++++++--- .../loom/test/util/ZipTestUtils.groovy | 8 ++++- 3 files changed, 47 insertions(+), 9 deletions(-) diff --git a/src/main/java/net/fabricmc/loom/configuration/mods/ArtifactMetadata.java b/src/main/java/net/fabricmc/loom/configuration/mods/ArtifactMetadata.java index d2aa3a198..057cde42e 100644 --- a/src/main/java/net/fabricmc/loom/configuration/mods/ArtifactMetadata.java +++ b/src/main/java/net/fabricmc/loom/configuration/mods/ArtifactMetadata.java @@ -68,10 +68,6 @@ public static ArtifactMetadata create(ArtifactRef artifact, String currentLoomVe remapRequirements = Boolean.parseBoolean(remapValue) ? RemapRequirements.OPT_IN : RemapRequirements.OPT_OUT; } - if (loomVersion != null) { - validateLoomVersion(loomVersion, currentLoomVersion); - } - if (mixinRemapType != null) { try { refmapRemapType = MixinRemapType.valueOf(mixinRemapType.toUpperCase(Locale.ROOT)); @@ -79,6 +75,10 @@ public static ArtifactMetadata create(ArtifactRef artifact, String currentLoomVe throw new IllegalStateException("Unknown mixin remap type: " + mixinRemapType); } } + + if (loomVersion != null && refmapRemapType != MixinRemapType.STATIC) { + validateLoomVersion(loomVersion, currentLoomVersion); + } } final Path installerPath = fs.getPath(INSTALLER_PATH); @@ -93,7 +93,13 @@ public static ArtifactMetadata create(ArtifactRef artifact, String currentLoomVe } // Validates that the version matches or is less than the current loom version + // This is only done for jars with tiny-remapper remapped mixins. private static void validateLoomVersion(String version, String currentLoomVersion) { + if ("0.0.0+unknown".equals(currentLoomVersion)) { + // Unknown version, skip validation. This is the case when running from source (tests) + return; + } + final String[] versionParts = version.split("\\."); final String[] currentVersionParts = currentLoomVersion.split("\\."); diff --git a/src/test/groovy/net/fabricmc/loom/test/unit/ArtifactMetadataTest.groovy b/src/test/groovy/net/fabricmc/loom/test/unit/ArtifactMetadataTest.groovy index 105aa58a3..6625993b8 100644 --- a/src/test/groovy/net/fabricmc/loom/test/unit/ArtifactMetadataTest.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/unit/ArtifactMetadataTest.groovy @@ -123,7 +123,7 @@ class ArtifactMetadataTest extends Specification { // Test that a mod with the same or older version of loom can be read def "Valid loom version"() { given: - def zip = createMod(modLoomVersion) + def zip = createMod(modLoomVersion, "mixin") when: def metadata = createMetadata(zip, loomVersion) then: @@ -144,7 +144,7 @@ class ArtifactMetadataTest extends Specification { // Test that a mod with the same or older version of loom can be read def "Invalid loom version"() { given: - def zip = createMod(modLoomVersion) + def zip = createMod(modLoomVersion, "mixin") when: def metadata = createMetadata(zip, loomVersion) then: @@ -158,8 +158,34 @@ class ArtifactMetadataTest extends Specification { "1.4" | "2.4" } - private static Path createMod(String loomVersion) { - return createZip(["fabric.mod.json": "{}", "META-INF/MANIFEST.MF": manifest("Fabric-Loom-Version", loomVersion)]) + def "Accepts all Loom versions"() { + given: + def zip = createMod(modLoomVersion, "static") + when: + def metadata = createMetadata(zip, loomVersion) + then: + metadata != null + where: + loomVersion | modLoomVersion + // Valid + "1.4" | "1.0.1" + "1.4" | "1.0.99" + "1.4" | "1.4" + "1.4" | "1.4.0" + "1.4" | "1.4.1" + "1.4" | "1.4.99" + "1.4" | "1.4.local" + "1.5" | "1.4.99" + "2.0" | "1.4.99" + // Usually invalid + "1.4" | "1.5" + "1.4" | "1.5.00" + "1.4" | "2.0" + "1.4" | "2.4" + } + + private static Path createMod(String loomVersion, String remapType) { + return createZip(["fabric.mod.json": "{}", "META-INF/MANIFEST.MF": manifest(["Fabric-Loom-Version": loomVersion, "Fabric-Loom-Mixin-Remap-Type": remapType])]) } private static ArtifactMetadata createMetadata(Path zip, String loomVersion = "1.4") { diff --git a/src/test/groovy/net/fabricmc/loom/test/util/ZipTestUtils.groovy b/src/test/groovy/net/fabricmc/loom/test/util/ZipTestUtils.groovy index c948c14b6..fd3241985 100644 --- a/src/test/groovy/net/fabricmc/loom/test/util/ZipTestUtils.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/util/ZipTestUtils.groovy @@ -50,9 +50,15 @@ class ZipTestUtils { } static String manifest(String key, String value) { + return manifest(Map.of(key, value)) + } + + static String manifest(Map entries) { def manifest = new Manifest() manifest.getMainAttributes().put(Attributes.Name.MANIFEST_VERSION, "1.0") - manifest.getMainAttributes().putValue(key, value) + entries.forEach { key, value -> + manifest.getMainAttributes().putValue(key, value) + } def out = new ByteArrayOutputStream() manifest.write(out) From dbed6a7e97473193b296abe76ed4854ca51a4f86 Mon Sep 17 00:00:00 2001 From: modmuss50 Date: Mon, 20 Nov 2023 13:55:08 +0000 Subject: [PATCH 8/9] Fix ReproducibleBuildTest --- .../loom/test/integration/ReproducibleBuildTest.groovy | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 d8e6a1ee5..4c51170d1 100644 --- a/src/test/groovy/net/fabricmc/loom/test/integration/ReproducibleBuildTest.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/integration/ReproducibleBuildTest.groovy @@ -55,11 +55,11 @@ class ReproducibleBuildTest extends Specification implements GradleProjectTestTr where: version | modHash | sourceHash - DEFAULT_GRADLE | "4bb8acb5e575a4080a8fe1282f8e1994" | [ + DEFAULT_GRADLE | "207bd75aa34fc996a97e962dd98b61d5" | [ "8e8fac2a5e32fc872e6cf0f9ccc55cfd", "ed331b6fae5677797a0104eba014e255" ] - PRE_RELEASE_GRADLE | "4bb8acb5e575a4080a8fe1282f8e1994" | [ + PRE_RELEASE_GRADLE | "207bd75aa34fc996a97e962dd98b61d5" | [ "8e8fac2a5e32fc872e6cf0f9ccc55cfd", "ed331b6fae5677797a0104eba014e255" ] From 6582908eecca822b278b8396bf2a5cd995f0e046 Mon Sep 17 00:00:00 2001 From: modmuss50 Date: Mon, 20 Nov 2023 14:08:29 +0000 Subject: [PATCH 9/9] Fix bad merge --- .../java/net/fabricmc/loom/task/RemapJarTask.java | 12 ++++++------ 1 file changed, 6 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 e113c4b7d..6dd2cd5a8 100644 --- a/src/main/java/net/fabricmc/loom/task/RemapJarTask.java +++ b/src/main/java/net/fabricmc/loom/task/RemapJarTask.java @@ -151,13 +151,13 @@ public void run() { if (mixinAp) { setupLegacyMixinRefmapRemapping(params); } - } - // Add the mixin refmap remap type to the manifest - // This is used by the mod dependency remapper to determine if it should remap the refmap - // or if the refmap should be remapped by mixin at runtime. - final var refmapRemapType = mixinAp ? ArtifactMetadata.MixinRemapType.MIXIN : ArtifactMetadata.MixinRemapType.STATIC; - params.getManifestAttributes().put(Constants.Manifest.MIXIN_REMAP_TYPE, refmapRemapType.manifestValue()); + // Add the mixin refmap remap type to the manifest + // This is used by the mod dependency remapper to determine if it should remap the refmap + // or if the refmap should be remapped by mixin at runtime. + final var refmapRemapType = mixinAp ? ArtifactMetadata.MixinRemapType.MIXIN : ArtifactMetadata.MixinRemapType.STATIC; + params.getManifestAttributes().put(Constants.Manifest.MIXIN_REMAP_TYPE, refmapRemapType.manifestValue()); + } }); }