From 846d16ce2d91ac14acf4e304ab4d284ba5a74631 Mon Sep 17 00:00:00 2001 From: modmuss Date: Sun, 19 Nov 2023 16:59:35 +0000 Subject: [PATCH 1/4] Update deps (#983) --- gradle/libs.versions.toml | 18 +++++++++--------- gradle/runtime.libs.versions.toml | 2 +- gradle/test.libs.versions.toml | 12 ++++++------ .../test/integration/MultiMcVersionTest.groovy | 2 +- 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 17c7ec21..1a6692f3 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -1,25 +1,25 @@ [versions] kotlin = "1.9.0" -asm = "9.5" -commons-io = "2.13.0" +asm = "9.6" +commons-io = "2.15.0" gson = "2.10.1" -jackson = "2.15.2" -guava = "32.1.2-jre" +jackson = "2.16.0" +guava = "32.1.3-jre" stitch = "0.6.2" tiny-remapper = "0.8.11" access-widener = "2.1.0" -mapping-io = "0.5.0-beta.3" +mapping-io = "0.5.0" lorenz-tiny = "4.0.2" -mercury = "0.4.0" +mercury = "0.4.1" kotlinx-metadata = "0.7.0" # Plugins spotless = "6.20.0" -test-retry = "1.5.4" -checkstyle = "10.12.2" +test-retry = "1.5.6" +checkstyle = "10.12.5" codenarc = "3.3.0" -jacoco = "0.8.10" +jacoco = "0.8.11" [libraries] # Loom compile libraries diff --git a/gradle/runtime.libs.versions.toml b/gradle/runtime.libs.versions.toml index fc707f6c..84f7acc3 100644 --- a/gradle/runtime.libs.versions.toml +++ b/gradle/runtime.libs.versions.toml @@ -8,7 +8,7 @@ vineflower = "1.9.3" mixin-compile-extensions = "0.6.0" dev-launch-injector = "0.2.1+build.8" terminal-console-appender = "1.2.0" -jetbrains-annotations = "24.0.1" +jetbrains-annotations = "24.1.0" native-support = "1.0.1" [libraries] diff --git a/gradle/test.libs.versions.toml b/gradle/test.libs.versions.toml index 8d77fb39..3bca2dd8 100644 --- a/gradle/test.libs.versions.toml +++ b/gradle/test.libs.versions.toml @@ -1,13 +1,13 @@ [versions] spock = "2.3-groovy-3.0" -junit = "5.10.0" -javalin = "5.6.2" -mockito = "5.4.0" -java-debug = "0.48.0" +junit = "5.10.1" +javalin = "5.6.3" +mockito = "5.7.0" +java-debug = "0.49.0" mixin = "0.11.4+mixin.0.8.5" -gradle-nightly = "8.6-20231107135843+0000" -fabric-loader = "0.14.22" +gradle-nightly = "8.6-20231118001259+0000" +fabric-loader = "0.14.24" fabric-installer = "0.11.1" [libraries] 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 8b15a8fd..d6280a46 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 f63a4f4d25d4598c20063962ed1e5870ad9d3385 Mon Sep 17 00:00:00 2001 From: Jason Penilla <11360596+jpenilla@users.noreply.github.com> Date: Mon, 20 Nov 2023 03:35:46 -0700 Subject: [PATCH 2/4] Skip remapping in `AbstractRemapJarTask`s when source and target namespaces match (#985) * Skip remapping in `AbstractRemapJarTask`s when source and target namespaces match The "remap jar" tasks have much more functionality than simply remapping jars at this point, such as adding namespace metadata, nesting jars, ensuring reproducible builds, etc. Some custom build logic may want to take advantage of these features without the full overhead of no-op remapping with TinyRemapper/Mercury. * Add test --- .../loom/task/AbstractRemapJarTask.java | 12 ++++ .../net/fabricmc/loom/task/RemapJarTask.java | 55 +++++++++++++------ .../loom/task/RemapSourcesJarTask.java | 18 ++++-- .../test/integration/SimpleProjectTest.groovy | 4 ++ .../resources/projects/simple/build.gradle | 15 +++++ 5 files changed, 84 insertions(+), 20 deletions(-) diff --git a/src/main/java/net/fabricmc/loom/task/AbstractRemapJarTask.java b/src/main/java/net/fabricmc/loom/task/AbstractRemapJarTask.java index 7da1ea1e..38028d77 100644 --- a/src/main/java/net/fabricmc/loom/task/AbstractRemapJarTask.java +++ b/src/main/java/net/fabricmc/loom/task/AbstractRemapJarTask.java @@ -173,6 +173,18 @@ public abstract class AbstractRemapJarTask extends Jar { Property getSourceNamespace(); Property getTargetNamespace(); + /** + * Checks whether {@link #getSourceNamespace()} and {@link #getTargetNamespace()} + * have the same value. When this is {@code true}, the user does not intend for any + * remapping to occur. They are using the task for its other features, such as adding + * namespace to the manifest, nesting jars, reproducible builds, etc. + * + * @return whether the source and target namespaces match + */ + default boolean namespacesMatch() { + return this.getSourceNamespace().get().equals(this.getTargetNamespace().get()); + } + Property getArchivePreserveFileTimestamps(); Property getArchiveReproducibleFileOrder(); Property getEntryCompression(); diff --git a/src/main/java/net/fabricmc/loom/task/RemapJarTask.java b/src/main/java/net/fabricmc/loom/task/RemapJarTask.java index 9639f3d1..90a38c9e 100644 --- a/src/main/java/net/fabricmc/loom/task/RemapJarTask.java +++ b/src/main/java/net/fabricmc/loom/task/RemapJarTask.java @@ -28,6 +28,7 @@ import java.io.IOException; import java.io.Serializable; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.StandardCopyOption; import java.util.ArrayList; import java.util.Collection; import java.util.List; @@ -52,6 +53,7 @@ import org.gradle.api.tasks.Internal; import org.gradle.api.tasks.SourceSet; import org.gradle.api.tasks.TaskAction; import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -136,15 +138,18 @@ public abstract class RemapJarTask extends AbstractRemapJarTask { params.getNestedJars().from(getNestedJars()); } - params.getTinyRemapperBuildServiceUuid().set(UnsafeWorkQueueHelper.create(getTinyRemapperService())); - params.getRemapClasspath().from(getClasspath()); - params.getMultiProjectOptimisation().set(getLoomExtension().multiProjectOptimisation()); + if (!params.namespacesMatch()) { + params.getTinyRemapperBuildServiceUuid().set(UnsafeWorkQueueHelper.create(getTinyRemapperService())); + params.getRemapClasspath().from(getClasspath()); - final boolean mixinAp = getUseMixinAP().get(); - params.getUseMixinExtension().set(!mixinAp); + params.getMultiProjectOptimisation().set(getLoomExtension().multiProjectOptimisation()); - if (mixinAp) { - setupLegacyMixinRefmapRemapping(params); + final boolean mixinAp = getUseMixinAP().get(); + params.getUseMixinExtension().set(!mixinAp); + + if (mixinAp) { + setupLegacyMixinRefmapRemapping(params); + } } }); } @@ -197,11 +202,13 @@ public abstract class RemapJarTask extends AbstractRemapJarTask { public abstract static class RemapAction extends AbstractRemapAction { private static final Logger LOGGER = LoggerFactory.getLogger(RemapAction.class); - private final TinyRemapperService tinyRemapperService; - private TinyRemapper tinyRemapper; + private final @Nullable TinyRemapperService tinyRemapperService; + private @Nullable TinyRemapper tinyRemapper; public RemapAction() { - this.tinyRemapperService = UnsafeWorkQueueHelper.get(getParameters().getTinyRemapperBuildServiceUuid(), TinyRemapperService.class); + this.tinyRemapperService = getParameters().getTinyRemapperBuildServiceUuid().isPresent() + ? UnsafeWorkQueueHelper.get(getParameters().getTinyRemapperBuildServiceUuid(), TinyRemapperService.class) + : null; } @Override @@ -209,13 +216,17 @@ public abstract class RemapJarTask extends AbstractRemapJarTask { try { LOGGER.info("Remapping {} to {}", inputFile, outputFile); - if (!getParameters().getMultiProjectOptimisation().get()) { + if (!getParameters().getMultiProjectOptimisation().getOrElse(false)) { prepare(); } - tinyRemapper = tinyRemapperService.getTinyRemapperForRemapping(); + if (tinyRemapperService != null) { + tinyRemapper = tinyRemapperService.getTinyRemapperForRemapping(); - remap(); + remap(); + } else { + Files.copy(inputFile, outputFile, StandardCopyOption.REPLACE_EXISTING); + } if (getParameters().getClientOnlyEntries().isPresent()) { markClientOnlyClasses(); @@ -227,7 +238,7 @@ public abstract class RemapJarTask extends AbstractRemapJarTask { modifyJarManifest(); rewriteJar(); - if (!getParameters().getMultiProjectOptimisation().get()) { + if (tinyRemapperService != null && !getParameters().getMultiProjectOptimisation().get()) { tinyRemapperService.close(); } @@ -245,10 +256,16 @@ public abstract class RemapJarTask extends AbstractRemapJarTask { private void prepare() { final Path inputFile = getParameters().getInputFile().getAsFile().get().toPath(); - PrepareJarRemapTask.prepare(tinyRemapperService, inputFile); + + if (tinyRemapperService != null) { + PrepareJarRemapTask.prepare(tinyRemapperService, inputFile); + } } private void remap() throws IOException { + Objects.requireNonNull(tinyRemapperService, "tinyRemapperService"); + Objects.requireNonNull(tinyRemapper, "tinyRemapper"); + try (OutputConsumerPath outputConsumer = new OutputConsumerPath.Builder(outputFile).build()) { outputConsumer.addNonClassFiles(inputFile); tinyRemapper.apply(outputConsumer, tinyRemapperService.getOrCreateTag(inputFile)); @@ -265,6 +282,10 @@ public abstract class RemapJarTask extends AbstractRemapJarTask { } private void remapAccessWidener() throws IOException { + if (getParameters().namespacesMatch()) { + return; + } + final AccessWidenerFile accessWidenerFile = AccessWidenerFile.fromModJar(inputFile); if (accessWidenerFile == null) { @@ -278,6 +299,8 @@ public abstract class RemapJarTask extends AbstractRemapJarTask { } private byte[] remapAccessWidener(byte[] input) { + Objects.requireNonNull(tinyRemapper, "tinyRemapper"); + int version = AccessWidenerReader.readVersion(input); AccessWidenerWriter writer = new AccessWidenerWriter(version); @@ -305,7 +328,7 @@ public abstract class RemapJarTask extends AbstractRemapJarTask { } private void addRefmaps() throws IOException { - if (getParameters().getUseMixinExtension().get()) { + if (getParameters().getUseMixinExtension().getOrElse(false)) { return; } diff --git a/src/main/java/net/fabricmc/loom/task/RemapSourcesJarTask.java b/src/main/java/net/fabricmc/loom/task/RemapSourcesJarTask.java index 2a9424ac..f0aa6d89 100644 --- a/src/main/java/net/fabricmc/loom/task/RemapSourcesJarTask.java +++ b/src/main/java/net/fabricmc/loom/task/RemapSourcesJarTask.java @@ -26,6 +26,7 @@ package net.fabricmc.loom.task; import java.io.IOException; import java.nio.file.Files; +import java.nio.file.StandardCopyOption; import java.util.List; import javax.inject.Inject; @@ -35,6 +36,7 @@ import org.gradle.api.provider.Property; import org.gradle.api.provider.Provider; import org.gradle.api.tasks.SourceSet; import org.gradle.api.tasks.TaskAction; +import org.jetbrains.annotations.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -57,7 +59,9 @@ public abstract class RemapSourcesJarTask extends AbstractRemapJarTask { @TaskAction public void run() { submitWork(RemapSourcesAction.class, params -> { - params.getSourcesRemapperServiceUuid().set(UnsafeWorkQueueHelper.create(SourceRemapperService.create(serviceManagerProvider.get().get(), this))); + if (!params.namespacesMatch()) { + params.getSourcesRemapperServiceUuid().set(UnsafeWorkQueueHelper.create(SourceRemapperService.create(serviceManagerProvider.get().get(), this))); + } }); } @@ -75,18 +79,24 @@ public abstract class RemapSourcesJarTask extends AbstractRemapJarTask { public abstract static class RemapSourcesAction extends AbstractRemapAction { private static final Logger LOGGER = LoggerFactory.getLogger(RemapSourcesAction.class); - private final SourceRemapperService sourceRemapperService; + private final @Nullable SourceRemapperService sourceRemapperService; public RemapSourcesAction() { super(); - sourceRemapperService = UnsafeWorkQueueHelper.get(getParameters().getSourcesRemapperServiceUuid(), SourceRemapperService.class); + sourceRemapperService = getParameters().getSourcesRemapperServiceUuid().isPresent() + ? UnsafeWorkQueueHelper.get(getParameters().getSourcesRemapperServiceUuid(), SourceRemapperService.class) + : null; } @Override public void execute() { try { - sourceRemapperService.remapSourcesJar(inputFile, outputFile); + if (sourceRemapperService != null) { + sourceRemapperService.remapSourcesJar(inputFile, outputFile); + } else { + Files.copy(inputFile, outputFile, StandardCopyOption.REPLACE_EXISTING); + } modifyJarManifest(); rewriteJar(); 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 d434f05d..3fc12edd 100644 --- a/src/test/groovy/net/fabricmc/loom/test/integration/SimpleProjectTest.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/integration/SimpleProjectTest.groovy @@ -54,6 +54,10 @@ class SimpleProjectTest extends Specification implements GradleProjectTestTrait gradle.getOutputZipEntry("fabric-example-mod-1.0.0.jar", "META-INF/MANIFEST.MF").contains("Fabric-Loom-Version: 0.0.0+unknown") gradle.getOutputZipEntry("fabric-example-mod-1.0.0-sources.jar", "net/fabricmc/example/mixin/ExampleMixin.java").contains("class_442") // Very basic test to ensure sources got remapped + // test same-namespace remapJar tasks + gradle.getOutputZipEntry("fabric-example-mod-1.0.0-no-remap.jar", "META-INF/MANIFEST.MF").contains("Fabric-Loom-Version: 0.0.0+unknown") + gradle.getOutputZipEntry("fabric-example-mod-1.0.0-no-remap-sources.jar", "net/fabricmc/example/mixin/ExampleMixin.java").contains("TitleScreen.class") // Very basic test to ensure sources did not get remapped :) + serverResult.successful() serverResult.output.contains("Hello simple Fabric mod") // A check to ensure our mod init was actually called where: diff --git a/src/test/resources/projects/simple/build.gradle b/src/test/resources/projects/simple/build.gradle index ed7d9e49..da624e48 100644 --- a/src/test/resources/projects/simple/build.gradle +++ b/src/test/resources/projects/simple/build.gradle @@ -86,3 +86,18 @@ publishing { // retrieving dependencies. } } + +// test same-namespace remapJar tasks +def noRemap = tasks.register('sameNamespaceRemapJar', net.fabricmc.loom.task.RemapJarTask) { + sourceNamespace = net.fabricmc.loom.api.mappings.layered.MappingsNamespace.INTERMEDIARY.toString() + inputFile = jar.archiveFile + archiveClassifier = "no-remap" +} +def noRemapSource = tasks.register('sameNamespaceRemapSourcesJar', net.fabricmc.loom.task.RemapSourcesJarTask) { + sourceNamespace = net.fabricmc.loom.api.mappings.layered.MappingsNamespace.INTERMEDIARY.toString() + inputFile = sourcesJar.archiveFile + archiveClassifier = "no-remap-sources" +} +tasks.assemble { + dependsOn(noRemap, noRemapSource) +} From 99380d98e52702629c90a11a446a17d4b635ca52 Mon Sep 17 00:00:00 2001 From: modmuss Date: Mon, 20 Nov 2023 15:19:48 +0000 Subject: [PATCH 3/4] Add Fabric-Loom-Mixin-Remap-Type manifest entry (#980) --- .../configuration/mods/ArtifactMetadata.java | 69 ++++++++-- .../loom/configuration/mods/JarSplitter.java | 27 ++-- .../configuration/mods/MixinDetector.java | 74 ---------- .../mods/ModConfigurationRemapper.java | 8 +- .../loom/configuration/mods/ModProcessor.java | 26 ++-- .../mods/dependency/ModDependency.java | 9 +- .../mods/dependency/ModDependencyFactory.java | 7 +- .../mods/dependency/SimpleModDependency.java | 5 +- .../mods/dependency/SplitModDependency.java | 5 +- .../minecraft/MinecraftJarSplitter.java | 6 +- .../loom/task/AbstractRemapJarTask.java | 20 +-- .../net/fabricmc/loom/task/RemapJarTask.java | 7 + .../loom/task/service/JarManifestService.java | 21 ++- .../net/fabricmc/loom/util/Constants.java | 20 +++ .../loom/util/ZipReprocessorUtil.java | 5 +- .../test/integration/FabricAPITest.groovy | 58 +++++--- .../integration/ReproducibleBuildTest.groovy | 4 +- .../test/unit/ArtifactMetadataTest.groovy | 93 ++++++++++++- .../loom/test/unit/MixinDetectorTest.groovy | 129 ------------------ .../loom/test/util/ZipTestUtils.groovy | 8 +- src/test/resources/patches/fabric_api.patch | 30 +++- 21 files changed, 327 insertions(+), 304 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 024a35c8..057cde42 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.io.IOException; 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; @@ -38,31 +39,45 @@ import org.jetbrains.annotations.Nullable; 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) { +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 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; + MixinRemapType refmapRemapType = MixinRemapType.MIXIN; 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 value = mainAttributes.getValue(MANIFEST_REMAP_KEY); + 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 (value != null) { + if (remapValue != 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(remapValue) ? RemapRequirements.OPT_IN : RemapRequirements.OPT_OUT; + } + + if (mixinRemapType != null) { + try { + refmapRemapType = MixinRemapType.valueOf(mixinRemapType.toUpperCase(Locale.ROOT)); + } catch (IllegalArgumentException e) { + throw new IllegalStateException("Unknown mixin remap type: " + mixinRemapType); + } + } + + if (loomVersion != null && refmapRemapType != MixinRemapType.STATIC) { + validateLoomVersion(loomVersion, currentLoomVersion); } } @@ -74,7 +89,32 @@ public record ArtifactMetadata(boolean isFabricMod, RemapRequirements remapRequi } } - return new ArtifactMetadata(isFabricMod, remapRequirements, installerData); + return new ArtifactMetadata(isFabricMod, remapRequirements, installerData, refmapRemapType); + } + + // 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("\\."); + + // 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() { @@ -100,4 +140,15 @@ public record ArtifactMetadata(boolean isFabricMod, RemapRequirements remapRequi return shouldRemap; } } + + 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 + STATIC; + + public String manifestValue() { + return name().toLowerCase(Locale.ROOT); + } + } } 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 6090d0ec..50cb05c3 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 java.util.stream.Stream; 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 class JarSplitter { @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 class JarSplitter { 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 class JarSplitter { final String entryPath = relativePath.toString(); - if (entryPath.equals(AbstractRemapJarTask.MANIFEST_PATH)) { + if (entryPath.equals(Constants.Manifest.PATH)) { continue; } @@ -183,11 +184,11 @@ public class JarSplitter { 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 class JarSplitter { 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 @@ public class JarSplitter { 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/MixinDetector.java b/src/main/java/net/fabricmc/loom/configuration/mods/MixinDetector.java deleted file mode 100644 index 6b79c5fe..00000000 --- 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 7ff0fbc7..166bf59d 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.gradle.language.base.artifact.SourcesArtifact; 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.mods.dependency.ModDependencyFactory; 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 class ModConfigurationRemapper { 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) { @@ -158,7 +160,7 @@ public class ModConfigurationRemapper { 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 f1f768c1..a4cf4503 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.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.task.RemapJarTask; import net.fabricmc.loom.util.Constants; import net.fabricmc.loom.util.Pair; import net.fabricmc.loom.util.TinyRemapperHelper; @@ -149,9 +148,14 @@ public class ModProcessor { 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().mixinRemapType() == ArtifactMetadata.MixinRemapType.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 +186,14 @@ public class ModProcessor { // 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().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?!"); + } + + project.getLogger().info("Remapping mixins in {} statically", info.getInputFile()); + remapMixins.add(tag); } remapper.readInputsAsync(tag, info.getInputFile()); @@ -243,10 +253,10 @@ public class ModProcessor { } 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/mods/dependency/ModDependency.java b/src/main/java/net/fabricmc/loom/configuration/mods/dependency/ModDependency.java index bfcd0b02..166de041 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.gradle.api.Project; 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 abstract sealed class ModDependency permits SplitModDependency, SimpleMod 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 f16ddfee..c1909b57 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.gradle.api.artifacts.Configuration; 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 @@ import net.fabricmc.loom.util.AttributeHelper; 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 class ModDependencyFactory { } 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 8a1bd82e..34c82cf3 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.Project; 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 0210036e..7f088357 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 org.jetbrains.annotations.Nullable; 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/configuration/providers/minecraft/MinecraftJarSplitter.java b/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/MinecraftJarSplitter.java index 7e11eba6..6df66011 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 java.util.stream.Stream; 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 @@ public class MinecraftJarSplitter implements AutoCloseable { 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 38028d77..7430e0e1 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.List; 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 org.jetbrains.annotations.ApiStatus; 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 abstract class AbstractRemapJarTask extends Jar { } if (getJarType().isPresent()) { - params.getManifestAttributes().put(MANIFEST_JAR_TYPE_KEY, getJarType().get()); + params.getManifestAttributes().put(Constants.Manifest.JAR_TYPE, getJarType().get()); } action.execute(params); @@ -197,8 +189,8 @@ public abstract class AbstractRemapJarTask extends Jar { protected void applyClientOnlyManifestAttributes(AbstractRemapParams params, List 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) )); } @@ -213,11 +205,11 @@ public abstract class AbstractRemapJarTask extends Jar { } 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 90a38c9e..6dd2cd5a 100644 --- a/src/main/java/net/fabricmc/loom/task/RemapJarTask.java +++ b/src/main/java/net/fabricmc/loom/task/RemapJarTask.java @@ -64,6 +64,7 @@ import net.fabricmc.loom.LoomGradleExtension; 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; @@ -150,6 +151,12 @@ public abstract class RemapJarTask extends AbstractRemapJarTask { 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()); } }); } 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 5b86fbc8..08f17632 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 abstract class JarManifestService implements BuildService { attributes.putValue(entry.getKey(), entry.getValue()); }); @@ -90,17 +89,17 @@ public abstract class JarManifestService implements BuildService 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 - } -} 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 c948c14b..fd324198 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) diff --git a/src/test/resources/patches/fabric_api.patch b/src/test/resources/patches/fabric_api.patch index 30d09fef..77b7e86c 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 92da5adb483a2b68ab8bd1b14ea7dff247089a2d Mon Sep 17 00:00:00 2001 From: modmuss Date: Mon, 20 Nov 2023 19:15:30 +0000 Subject: [PATCH 4/4] Remove Minecraft libraries from the remap classpath. (#987) --- .../loom/configuration/processors/ContextImplHelper.java | 1 - .../minecraft/mapped/AbstractMappedMinecraftProvider.java | 1 - src/main/java/net/fabricmc/loom/task/RemapJarTask.java | 7 ++++++- .../java/net/fabricmc/loom/util/TinyRemapperHelper.java | 6 ------ 4 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/main/java/net/fabricmc/loom/configuration/processors/ContextImplHelper.java b/src/main/java/net/fabricmc/loom/configuration/processors/ContextImplHelper.java index b18e1d8a..283f2cc7 100644 --- a/src/main/java/net/fabricmc/loom/configuration/processors/ContextImplHelper.java +++ b/src/main/java/net/fabricmc/loom/configuration/processors/ContextImplHelper.java @@ -42,7 +42,6 @@ public final class ContextImplHelper { return new LazyCloseable<>(() -> { try { TinyRemapper tinyRemapper = TinyRemapperHelper.getTinyRemapper(configContext.project(), configContext.serviceManager(), from.toString(), to.toString()); - tinyRemapper.readClassPath(TinyRemapperHelper.getMinecraftCompileLibraries(configContext.project())); for (Path minecraftJar : configContext.extension().getMinecraftJars(MappingsNamespace.INTERMEDIARY)) { tinyRemapper.readClassPath(minecraftJar); 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 4aed85df..aaed5b8c 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 @@ -196,7 +196,6 @@ public abstract class AbstractMappedMinecraftProvider !minecraftCompileLibraries.getFiles().contains(file)); + + getClasspath().from(remapClasspath); getAddNestedDependencies().convention(true).finalizeValueOnRead(); Configuration includeConfiguration = getProject().getConfigurations().getByName(Constants.Configurations.INCLUDE); diff --git a/src/main/java/net/fabricmc/loom/util/TinyRemapperHelper.java b/src/main/java/net/fabricmc/loom/util/TinyRemapperHelper.java index 648ba105..0381a268 100644 --- a/src/main/java/net/fabricmc/loom/util/TinyRemapperHelper.java +++ b/src/main/java/net/fabricmc/loom/util/TinyRemapperHelper.java @@ -24,7 +24,6 @@ package net.fabricmc.loom.util; -import java.io.File; import java.io.IOException; import java.nio.file.Path; import java.util.Map; @@ -95,11 +94,6 @@ public final class TinyRemapperHelper { return builder.build(); } - public static Path[] getMinecraftCompileLibraries(Project project) { - return project.getConfigurations().getByName(Constants.Configurations.MINECRAFT_COMPILE_LIBRARIES).getFiles() - .stream().map(File::toPath).toArray(Path[]::new); - } - private static IMappingProvider.Member memberOf(String className, String memberName, String descriptor) { return new IMappingProvider.Member(className, memberName, descriptor); }