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] 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 7da1ea1e5..38028d777 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 interface AbstractRemapParams extends WorkParameters { 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 9639f3d18..90a38c9e7 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.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.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 void run() { 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 @@ record RefmapData(List mixinConfigs, String refmapName) implements Seria 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 void execute() { 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 void execute() { modifyJarManifest(); rewriteJar(); - if (!getParameters().getMultiProjectOptimisation().get()) { + if (tinyRemapperService != null && !getParameters().getMultiProjectOptimisation().get()) { tinyRemapperService.close(); } @@ -245,10 +256,16 @@ public void execute() { 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 @@ private void markClientOnlyClasses() throws IOException { } private void remapAccessWidener() throws IOException { + if (getParameters().namespacesMatch()) { + return; + } + final AccessWidenerFile accessWidenerFile = AccessWidenerFile.fromModJar(inputFile); if (accessWidenerFile == null) { @@ -278,6 +299,8 @@ private void remapAccessWidener() throws IOException { } private byte[] remapAccessWidener(byte[] input) { + Objects.requireNonNull(tinyRemapper, "tinyRemapper"); + int version = AccessWidenerReader.readVersion(input); AccessWidenerWriter writer = new AccessWidenerWriter(version); @@ -305,7 +328,7 @@ private void addNestedJars() { } 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 2a9424acc..f0aa6d89e 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 @@ 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.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 RemapSourcesJarTask() { @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 interface RemapSourcesParams extends AbstractRemapParams { 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 d434f05d7..3fc12eddf 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 ed7d9e49f..da624e48f 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) +}