Remove multi project optimisation (#1159)

* Remove multi project optimisation

* Fix build

* Fix FAPI test

src/main/java/net/fabricmc/loom/LoomGradleExtension.java

@@ -110,14 +110,6 @@ public interface LoomGradleExtension extends LoomGradleExtensionAPI {
 
 	void setRefreshDeps(boolean refreshDeps);
 
-	/**
-	 * If true, multi-project optimisation mode is enabled. This mode makes builds with many Loom projects
-	 * much faster by increasing sharing and disabling some functionality.
-	 *
-	 * <p>You can enable it by setting the Gradle property {@code fabric.loom.multiProjectOptimisation} to {@code true}.
-	 */
-	boolean multiProjectOptimisation();
-
 	ListProperty<LibraryProcessorManager.LibraryProcessorFactory> getLibraryProcessors();
 
 	ListProperty<RemapperExtensionHolder> getRemapperExtensions();

src/main/java/net/fabricmc/loom/build/mixin/AnnotationProcessorInvoker.java

@@ -45,7 +45,6 @@ import net.fabricmc.loom.LoomGradleExtension;
 import net.fabricmc.loom.configuration.ide.idea.IdeaUtils;
 import net.fabricmc.loom.configuration.providers.minecraft.MinecraftSourceSets;
 import net.fabricmc.loom.extension.MixinExtension;
-import net.fabricmc.loom.task.PrepareJarRemapTask;
 import net.fabricmc.loom.util.Constants;
 import net.fabricmc.loom.util.LoomVersions;
 
@@ -121,11 +120,6 @@ public abstract class AnnotationProcessorInvoker<T extends Task> {
 				args.put("MSG_" + key, value);
 			});
 
-			if (loomExtension.multiProjectOptimisation()) {
-				// Ensure that all of the mixin mappings have been generated before we create the mixin mappings.
-				runBeforePrepare(project, task);
-			}
-
 			project.getLogger().debug("Outputting refmap to dir: " + getRefmapDestinationDir(task) + " for compile task: " + task);
 			args.forEach((k, v) -> passArgument(task, k, v));
 		} catch (IOException e) {
@@ -157,12 +151,6 @@ public abstract class AnnotationProcessorInvoker<T extends Task> {
 		}
 	}
 
-	private void runBeforePrepare(Project project, Task compileTask) {
-		project.getGradle().allprojects(otherProject -> {
-			otherProject.getTasks().withType(PrepareJarRemapTask.class, prepareRemapTask -> prepareRemapTask.mustRunAfter(compileTask));
-		});
-	}
-
 	private static void checkPattern(String input, Pattern pattern) {
 		final Matcher matcher = pattern.matcher(input);
 

src/main/java/net/fabricmc/loom/extension/LoomGradleExtensionImpl.java

@@ -39,7 +39,6 @@ import org.gradle.api.configuration.BuildFeatures;
 import org.gradle.api.file.ConfigurableFileCollection;
 import org.gradle.api.file.FileCollection;
 import org.gradle.api.provider.ListProperty;
-import org.gradle.api.provider.Provider;
 
 import net.fabricmc.loom.LoomGradleExtension;
 import net.fabricmc.loom.api.mappings.intermediate.IntermediateMappingsProvider;
@@ -55,10 +54,8 @@ import net.fabricmc.loom.configuration.providers.minecraft.MinecraftProvider;
 import net.fabricmc.loom.configuration.providers.minecraft.library.LibraryProcessorManager;
 import net.fabricmc.loom.configuration.providers.minecraft.mapped.IntermediaryMinecraftProvider;
 import net.fabricmc.loom.configuration.providers.minecraft.mapped.NamedMinecraftProvider;
-import net.fabricmc.loom.util.Constants;
 import net.fabricmc.loom.util.download.Download;
 import net.fabricmc.loom.util.download.DownloadBuilder;
-import net.fabricmc.loom.util.gradle.GradleUtils;
 
 public abstract class LoomGradleExtensionImpl extends LoomGradleExtensionApiImpl implements LoomGradleExtension {
 	private final Project project;
@@ -75,7 +72,6 @@ public abstract class LoomGradleExtensionImpl extends LoomGradleExtensionApiImpl
 	private IntermediaryMinecraftProvider<?> intermediaryMinecraftProvider;
 	private InstallerData installerData;
 	private boolean refreshDeps;
-	private final Provider<Boolean> multiProjectOptimisation;
 	private final ListProperty<LibraryProcessorManager.LibraryProcessorFactory> libraryProcessorFactories;
 	private final LoomProblemReporter problemReporter;
 	private final boolean configurationCacheActive;
@@ -103,7 +99,6 @@ public abstract class LoomGradleExtensionImpl extends LoomGradleExtensionApiImpl
 		});
 
 		refreshDeps = manualRefreshDeps();
-		multiProjectOptimisation = GradleUtils.getBooleanPropertyProvider(project, Constants.Properties.MULTI_PROJECT_OPTIMISATION);
 		libraryProcessorFactories = project.getObjects().listProperty(LibraryProcessorManager.LibraryProcessorFactory.class);
 		libraryProcessorFactories.addAll(LibraryProcessorManager.DEFAULT_LIBRARY_PROCESSORS);
 		libraryProcessorFactories.finalizeValueOnRead();
@@ -111,15 +106,6 @@ public abstract class LoomGradleExtensionImpl extends LoomGradleExtensionApiImpl
 		configurationCacheActive = getBuildFeatures().getConfigurationCache().getActive().get();
 		isolatedProjectsActive = getBuildFeatures().getIsolatedProjects().getActive().get();
 
-		// Fundamentally impossible to support multi-project optimisation with the configuration cache and/or isolated projects.
-		if (multiProjectOptimisation.get() && configurationCacheActive) {
-			throw new UnsupportedOperationException("Multi-project optimisation is not supported with the configuration cache");
-		}
-
-		if (multiProjectOptimisation.get() && isolatedProjectsActive) {
-			throw new UnsupportedOperationException("Isolated projects are not supported with multi-project optimisation");
-		}
-
 		if (configurationCacheActive) {
 			project.getLogger().warn("Loom support for the Gradle configuration cache is highly experimental and may not work as expected. Please report any issues you encounter.");
 		}
@@ -274,11 +260,6 @@ public abstract class LoomGradleExtensionImpl extends LoomGradleExtensionApiImpl
 		this.refreshDeps = refreshDeps;
 	}
 
-	@Override
-	public boolean multiProjectOptimisation() {
-		return multiProjectOptimisation.getOrElse(false);
-	}
-
 	@Override
 	public ListProperty<LibraryProcessorManager.LibraryProcessorFactory> getLibraryProcessors() {
 		return libraryProcessorFactories;

src/main/java/net/fabricmc/loom/task/AbstractRemapJarTask.java

@@ -83,12 +83,6 @@ public abstract class AbstractRemapJarTask extends Jar {
 	@Input
 	public abstract Property<String> getTargetNamespace();
 
-	/**
-	 * When enabled the TinyRemapperService will not be shared across sub projects.
-	 */
-	@Input
-	public abstract Property<Boolean> getRemapperIsolation();
-
 	@Inject
 	protected abstract WorkerExecutor getWorkerExecutor();
 
@@ -116,7 +110,6 @@ public abstract class AbstractRemapJarTask extends Jar {
 	public AbstractRemapJarTask() {
 		getSourceNamespace().convention(MappingsNamespace.NAMED.toString()).finalizeValueOnRead();
 		getTargetNamespace().convention(MappingsNamespace.INTERMEDIARY.toString()).finalizeValueOnRead();
-		getRemapperIsolation().convention(false).finalizeValueOnRead();
 		getIncludesClientOnlyClasses().convention(false).finalizeValueOnRead();
 		getJarType().finalizeValueOnRead();
 

src/main/java/net/fabricmc/loom/task/PrepareJarRemapTask.java

@@ -1,105 +0,0 @@
-/*
- * This file is part of fabric-loom, licensed under the MIT License (MIT).
- *
- * Copyright (c) 2022 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.task;
-
-import java.nio.file.Path;
-
-import javax.inject.Inject;
-
-import org.gradle.api.file.RegularFileProperty;
-import org.gradle.api.provider.Property;
-import org.gradle.api.tasks.InputFile;
-import org.gradle.api.tasks.TaskAction;
-import org.gradle.workers.WorkAction;
-import org.gradle.workers.WorkParameters;
-import org.gradle.workers.WorkQueue;
-import org.gradle.workers.WorkerExecutor;
-
-import net.fabricmc.loom.task.service.TinyRemapperService;
-import net.fabricmc.loom.util.service.UnsafeWorkQueueHelper;
-
-/**
- * The prepare remap task runs before all other jar remap tasks, should be used to setup tiny remapper.
- */
-public abstract class PrepareJarRemapTask extends AbstractLoomTask {
-	private final RemapJarTask remapJarTask;
-	@InputFile
-	public abstract RegularFileProperty getInputFile();
-
-	@Inject
-	public PrepareJarRemapTask(RemapJarTask remapJarTask) {
-		this.remapJarTask = remapJarTask;
-
-		getInputFile().set(remapJarTask.getInputFile());
-		// TODO can this be up-to-date when the main task is up-to date?
-		getOutputs().upToDateWhen((o) -> false);
-
-		getProject().getGradle().allprojects(project -> {
-			project.getTasks().withType(PrepareJarRemapTask.class, otherTask -> {
-				if (otherTask == this) return;
-
-				// Ensure that all other prepare tasks inputs have completed
-				dependsOn(otherTask.getInputs());
-				mustRunAfter(otherTask.getInputs());
-			});
-		});
-	}
-
-	@Inject
-	protected abstract WorkerExecutor getWorkerExecutor();
-
-	@TaskAction
-	public void run() {
-		final WorkQueue workQueue = getWorkerExecutor().noIsolation();
-
-		workQueue.submit(ReadInputsAction.class, params -> {
-			params.getTinyRemapperBuildServiceUuid().set(UnsafeWorkQueueHelper.create(remapJarTask.getTinyRemapperService()));
-			params.getInputFile().set(getInputFile());
-		});
-	}
-
-	public interface ReadInputsParams extends WorkParameters {
-		Property<String> getTinyRemapperBuildServiceUuid();
-		RegularFileProperty getInputFile();
-	}
-
-	public abstract static class ReadInputsAction implements WorkAction<ReadInputsParams> {
-		private final TinyRemapperService tinyRemapperService;
-
-		public ReadInputsAction() {
-			this.tinyRemapperService = UnsafeWorkQueueHelper.get(getParameters().getTinyRemapperBuildServiceUuid(), TinyRemapperService.class);
-		}
-
-		@Override
-		public void execute() {
-			final Path inputFile = getParameters().getInputFile().getAsFile().get().toPath();
-			prepare(tinyRemapperService, inputFile);
-		}
-	}
-
-	static void prepare(TinyRemapperService tinyRemapperService, Path inputFile) {
-		tinyRemapperService.getTinyRemapperForInputs().readInputsAsync(tinyRemapperService.getOrCreateTag(inputFile), inputFile);
-	}
-}

src/main/java/net/fabricmc/loom/task/RemapJarTask.java

@@ -117,10 +117,6 @@ public abstract class RemapJarTask extends AbstractRemapJarTask {
 
 		getUseMixinAP().set(LoomGradleExtension.get(getProject()).getMixin().getUseLegacyMixinAp());
 
-		if (getLoomExtension().multiProjectOptimisation()) {
-			setupPreparationTask();
-		}
-
 		// Make outputs reproducible by default
 		setReproducibleFileOrder(true);
 		setPreserveFileTimestamps(false);
@@ -128,19 +124,6 @@ public abstract class RemapJarTask extends AbstractRemapJarTask {
 		getJarType().set("classes");
 	}
 
-	private void setupPreparationTask() {
-		PrepareJarRemapTask prepareJarTask = getProject().getTasks().create("prepare" + getName().substring(0, 1).toUpperCase() + getName().substring(1), PrepareJarRemapTask.class, this);
-
-		dependsOn(prepareJarTask);
-		mustRunAfter(prepareJarTask);
-
-		getProject().getGradle().allprojects(project -> {
-			project.getTasks()
-					.withType(PrepareJarRemapTask.class)
-					.configureEach(this::mustRunAfter);
-		});
-	}
-
 	@TaskAction
 	public void run() {
 		submitWork(RemapAction.class, params -> {
@@ -152,8 +135,6 @@ public abstract class RemapJarTask extends AbstractRemapJarTask {
 				params.getTinyRemapperBuildServiceUuid().set(UnsafeWorkQueueHelper.create(getTinyRemapperService()));
 				params.getRemapClasspath().from(getClasspath());
 
-				params.getMultiProjectOptimisation().set(getLoomExtension().multiProjectOptimisation());
-
 				final boolean mixinAp = getUseMixinAP().get();
 				params.getUseMixinExtension().set(!mixinAp);
 
@@ -209,7 +190,6 @@ public abstract class RemapJarTask extends AbstractRemapJarTask {
 		ConfigurableFileCollection getRemapClasspath();
 
 		Property<Boolean> getUseMixinExtension();
-		Property<Boolean> getMultiProjectOptimisation();
 		Property<Boolean> getOptimizeFmj();
 
 		record RefmapData(List<String> mixinConfigs, String refmapName) implements Serializable { }
@@ -235,9 +215,7 @@ public abstract class RemapJarTask extends AbstractRemapJarTask {
 			try {
 				LOGGER.info("Remapping {} to {}", inputFile, outputFile);
 
-				if (!getParameters().getMultiProjectOptimisation().getOrElse(false)) {
-					prepare();
-				}
+				prepare();
 
 				if (tinyRemapperService != null) {
 					tinyRemapper = tinyRemapperService.getTinyRemapperForRemapping();
@@ -261,7 +239,7 @@ public abstract class RemapJarTask extends AbstractRemapJarTask {
 					optimizeFMJ();
 				}
 
-				if (tinyRemapperService != null && !getParameters().getMultiProjectOptimisation().get()) {
+				if (tinyRemapperService != null) {
 					tinyRemapperService.close();
 				}
 
@@ -281,7 +259,7 @@ public abstract class RemapJarTask extends AbstractRemapJarTask {
 			final Path inputFile = getParameters().getInputFile().getAsFile().get().toPath();
 
 			if (tinyRemapperService != null) {
-				PrepareJarRemapTask.prepare(tinyRemapperService, inputFile);
+				tinyRemapperService.getTinyRemapperForInputs().readInputsAsync(tinyRemapperService.getOrCreateTag(inputFile), inputFile);
 			}
 		}
 

src/main/java/net/fabricmc/loom/task/service/TinyRemapperService.java

@@ -46,7 +46,6 @@ import org.gradle.api.tasks.SourceSet;
 import org.jetbrains.annotations.Nullable;
 
 import net.fabricmc.loom.LoomGradleExtension;
-import net.fabricmc.loom.api.mappings.layered.MappingsNamespace;
 import net.fabricmc.loom.build.mixin.AnnotationProcessorInvoker;
 import net.fabricmc.loom.extension.RemapperExtensionHolder;
 import net.fabricmc.loom.task.AbstractRemapJarTask;
@@ -70,7 +69,6 @@ public class TinyRemapperService implements SharedService {
 		final LoomGradleExtension extension = LoomGradleExtension.get(project);
 		final boolean legacyMixin = extension.getMixin().getUseLegacyMixinAp().get();
 		final @Nullable KotlinClasspathService kotlinClasspathService = KotlinClasspathService.getOrCreateIfRequired(serviceManager, project);
-		boolean multiProjectOptimisation = extension.multiProjectOptimisation();
 
 		// Generates an id that is used to share the remapper across projects. This tasks in the remap jar task name to handle custom remap jar tasks separately.
 		final var joiner = new StringJoiner(":");
@@ -81,9 +79,8 @@ public class TinyRemapperService implements SharedService {
 			joiner.add("kotlin-" + kotlinClasspathService.version());
 		}
 
-		if (remapJarTask.getRemapperIsolation().get() || !multiProjectOptimisation) {
-			joiner.add(project.getPath());
-		}
+		// TODO remove this when removing shared service manager.
+		joiner.add(project.getPath());
 
 		extension.getKnownIndyBsms().get().stream().sorted().forEach(joiner::add);
 
@@ -103,20 +100,6 @@ public class TinyRemapperService implements SharedService {
 		final ConfigurationContainer configurations = project.getConfigurations();
 		ConfigurableFileCollection excludedMinecraftJars = project.files();
 
-		// Exclude none root minecraft jars.
-		if (multiProjectOptimisation && !extension.isRootProject()) {
-			MappingsNamespace mappingsNamespace = MappingsNamespace.of(from);
-
-			if (mappingsNamespace != null) {
-				for (Path minecraftJar : extension.getMinecraftJars(mappingsNamespace)) {
-					excludedMinecraftJars.from(minecraftJar.toFile());
-				}
-			} else {
-				// None fatal as this is a performance optimisation.
-				project.getLogger().warn("Unable to find minecraft jar for namespace {}", from);
-			}
-		}
-
 		List<Path> classPath = remapJarTask.getClasspath()
 				.minus(configurations.getByName(Constants.Configurations.MINECRAFT_COMPILE_LIBRARIES))
 				.minus(configurations.getByName(Constants.Configurations.MINECRAFT_RUNTIME_LIBRARIES))

src/main/java/net/fabricmc/loom/util/Constants.java

@@ -128,7 +128,6 @@ public class Constants {
 	}
 
 	public static final class Properties {
-		public static final String MULTI_PROJECT_OPTIMISATION = "fabric.loom.multiProjectOptimisation";
 		public static final String DONT_REMAP = "fabric.loom.dontRemap";
 		public static final String DISABLE_REMAPPED_VARIANTS = "fabric.loom.disableRemappedVariants";
 		public static final String DISABLE_PROJECT_DEPENDENT_MODS = "fabric.loom.disableProjectDependentMods";

src/test/groovy/net/fabricmc/loom/test/benchmark/FabricAPIBenchmark.groovy

@@ -48,8 +48,6 @@ class FabricAPIBenchmark implements GradleProjectTestTrait {
 				patch: "fabric_api"
 				)
 
-		gradle.enableMultiProjectOptimisation()
-
 		if (!gradle.buildGradle.text.contains("loom.mixin.useLegacyMixinAp")) {
 			gradle.buildGradle << """
 				allprojects {

src/test/groovy/net/fabricmc/loom/test/integration/FabricAPITest.groovy

@@ -49,8 +49,6 @@ class FabricAPITest extends Specification implements GradleProjectTestTrait {
 				patch: "fabric_api"
 				)
 
-		gradle.enableMultiProjectOptimisation()
-
 		// Disable the mixin ap if needed. Fabric API is a large enough test project to see if something breaks.
 		if (disableMixinAp) {
 			gradle.buildGradle << """
@@ -103,7 +101,6 @@ class FabricAPITest extends Specification implements GradleProjectTestTrait {
 
 		then:
 		result.task(":build").outcome == SUCCESS
-		result.task(":prepareRemapJar").outcome == SUCCESS
 
 		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")
 		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()

src/test/groovy/net/fabricmc/loom/test/util/GradleProjectTestTrait.groovy

@@ -33,7 +33,6 @@ import org.gradle.util.GradleVersion
 import spock.lang.Shared
 
 import net.fabricmc.loom.test.LoomTestConstants
-import net.fabricmc.loom.util.Constants
 import net.fabricmc.loom.util.ZipUtils
 
 trait GradleProjectTestTrait {
@@ -332,9 +331,5 @@ trait GradleProjectTestTrait {
                 }
             """
 		}
-
-		void enableMultiProjectOptimisation() {
-			getGradleProperties() << "\n${Constants.Properties.MULTI_PROJECT_OPTIMISATION}=true"
-		}
 	}
 }