Skip remapping in AbstractRemapJarTasks when source and target name…

…spaces 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
FabricMC · Nov 20, 2023 · f63a4f4 · f63a4f4
1 parent 846d16c
commit f63a4f4
Show file tree

Hide file tree

Showing 5 changed files with 84 additions and 20 deletions.
diff --git 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<String> getSourceNamespace();
 		Property<String> 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<Boolean> getArchivePreserveFileTimestamps();
 		Property<Boolean> getArchiveReproducibleFileOrder();
 		Property<ZipEntryCompression> getEntryCompression();

diff --git 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,25 +202,31 @@ record RefmapData(List<String> mixinConfigs, String refmapName) implements Seria
 	public abstract static class RemapAction extends AbstractRemapAction<RemapParams> {
 		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
 		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
@@ -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<RemapSourcesParams> {
 		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
@@ -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
@@ -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)
+}