From 1b6fa615c48083b97afdda4fd350e43c67ae3a39 Mon Sep 17 00:00:00 2001 From: modmuss50 Date: Thu, 19 May 2022 20:17:54 +0100 Subject: [PATCH 1/5] Preserve extraInt in kotlin metadata remapper The `extraInt` (`xs`, [1]) parameter of Kotlin's Metadata annotation does not become part of the KmClass/KmLambda and must be preserved explicitly to not be lost during remapping. It does carry important information though, so we must not neglect it. In particular, bit 7 indicates that the class is used in the scope of an inline function and is therefore implicitly part of the public API. If this flag is missing from a class which requires it, then it becomes impossible to use that inline function because the Kotlin compiler will crash hitting an assertion while trying to inline the required code [2]. Based on the descriptions in [1], it looks like none of this data should be affected by our remapping in any way, so it seems safe to simply copy over the value from the original annotation. And that's what this commit does. [1]: https://kotlinlang.org/api/latest/jvm/stdlib/kotlin/-metadata/extra-int.html [2]: https://i.johni0702.de/sOpA8.png Co-authored-by: Jonas Herzig --- .../KotlinClassMetadataRemappingAnnotationVisitor.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/kotlin/net/fabricmc/loom/kotlin/remapping/KotlinClassMetadataRemappingAnnotationVisitor.kt b/src/main/kotlin/net/fabricmc/loom/kotlin/remapping/KotlinClassMetadataRemappingAnnotationVisitor.kt index 57fca33b..b8a10620 100644 --- a/src/main/kotlin/net/fabricmc/loom/kotlin/remapping/KotlinClassMetadataRemappingAnnotationVisitor.kt +++ b/src/main/kotlin/net/fabricmc/loom/kotlin/remapping/KotlinClassMetadataRemappingAnnotationVisitor.kt @@ -61,7 +61,7 @@ class KotlinClassMetadataRemappingAnnotationVisitor(private val remapper: Remapp val klass = metadata.toKmClass() val writer = KotlinClassMetadata.Class.Writer() klass.accept(RemappingKmVisitors(remapper).RemappingKmClassVisitor(writer)) - val remapped = writer.write(header.metadataVersion).header + val remapped = writer.write(header.metadataVersion, header.extraInt).header writeClassHeader(remapped) validateKotlinClassHeader(remapped, header) } @@ -71,7 +71,7 @@ class KotlinClassMetadataRemappingAnnotationVisitor(private val remapper: Remapp if (klambda != null) { val writer = KotlinClassMetadata.SyntheticClass.Writer() klambda.accept(RemappingKmVisitors(remapper).RemappingKmLambdaVisitor(writer)) - val remapped = writer.write(header.metadataVersion).header + val remapped = writer.write(header.metadataVersion, header.extraInt).header writeClassHeader(remapped) validateKotlinClassHeader(remapped, header) } else { @@ -82,7 +82,7 @@ class KotlinClassMetadataRemappingAnnotationVisitor(private val remapper: Remapp val kpackage = metadata.toKmPackage() val writer = KotlinClassMetadata.FileFacade.Writer() kpackage.accept(RemappingKmVisitors(remapper).RemappingKmPackageVisitor(writer)) - val remapped = writer.write(header.metadataVersion).header + val remapped = writer.write(header.metadataVersion, header.extraInt).header writeClassHeader(remapped) validateKotlinClassHeader(remapped, header) } From e1dc1f0684257a65477c0d8de6ce7ff8ed8ac7ff Mon Sep 17 00:00:00 2001 From: modmuss50 Date: Thu, 19 May 2022 20:42:54 +0100 Subject: [PATCH 2/5] Ensure nightly gradle version exists before using it to run tests. Will stop all tests failing when nightly version is removed. --- .../loom/test/LoomTestConstants.groovy | 20 ++++++++++++++++--- .../test/integration/SimpleProjectTest.groovy | 4 +--- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/src/test/groovy/net/fabricmc/loom/test/LoomTestConstants.groovy b/src/test/groovy/net/fabricmc/loom/test/LoomTestConstants.groovy index ddad9497..ea5c33ec 100644 --- a/src/test/groovy/net/fabricmc/loom/test/LoomTestConstants.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/LoomTestConstants.groovy @@ -27,8 +27,22 @@ package net.fabricmc.loom.test import org.gradle.util.GradleVersion class LoomTestConstants { - public final static String DEFAULT_GRADLE = GradleVersion.current().getVersion() - public final static String PRE_RELEASE_GRADLE = "7.6-20220516224938+0000" + private final static String NIGHTLY_VERSION = "7.6-20220519002827+0000" + private final static boolean NIGHTLY_EXISTS = nightlyExists(NIGHTLY_VERSION) - public final static String[] STANDARD_TEST_VERSIONS = [DEFAULT_GRADLE, PRE_RELEASE_GRADLE] + public final static String DEFAULT_GRADLE = GradleVersion.current().getVersion() + // Tests that depend specifically on the nightly will run on the current version when the nightly is not available. + public final static String PRE_RELEASE_GRADLE = NIGHTLY_EXISTS ? "7.6-20220516224938+0000" : DEFAULT_GRADLE + public final static String[] STANDARD_TEST_VERSIONS = NIGHTLY_EXISTS ? [DEFAULT_GRADLE, PRE_RELEASE_GRADLE] : [DEFAULT_GRADLE] + + /** + * Nightly gradle versions get removed after a certain amount of time, lets check to see if its still online before running the tests. + */ + private static boolean nightlyExists(String version) { + def url = "https://services.gradle.org/distributions-snapshots/gradle-${version}-bin.zip" + def con = new URL(url).openConnection() as HttpURLConnection + con.setRequestMethod("HEAD") // No need to request the whole file. + + return con.getResponseCode() == HttpURLConnection.HTTP_OK + } } 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 2ceb2a01..ced58f6b 100644 --- a/src/test/groovy/net/fabricmc/loom/test/integration/SimpleProjectTest.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/integration/SimpleProjectTest.groovy @@ -56,9 +56,7 @@ class SimpleProjectTest extends Specification implements GradleProjectTestTrait serverResult.successful() serverResult.output.contains("Hello simple Fabric mod") // A check to ensure our mod init was actually called where: - version | _ - DEFAULT_GRADLE | _ - PRE_RELEASE_GRADLE | _ + version << STANDARD_TEST_VERSIONS } @Unroll From 743e009068f6776c4453239774d8018476eab2b7 Mon Sep 17 00:00:00 2001 From: modmuss50 Date: Fri, 20 May 2022 01:01:01 +0100 Subject: [PATCH 3/5] Fix default client run config name when using split sourcesets. --- .../java/net/fabricmc/loom/configuration/ide/RunConfig.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/main/java/net/fabricmc/loom/configuration/ide/RunConfig.java b/src/main/java/net/fabricmc/loom/configuration/ide/RunConfig.java index 8a010518..9343cb16 100644 --- a/src/main/java/net/fabricmc/loom/configuration/ide/RunConfig.java +++ b/src/main/java/net/fabricmc/loom/configuration/ide/RunConfig.java @@ -140,7 +140,11 @@ public class RunConfig { configName = ""; String srcName = sourceSet.getName(); - if (!srcName.equals(SourceSet.MAIN_SOURCE_SET_NAME)) { + final boolean isSplitClientSourceSet = extension.areEnvironmentSourceSetsSplit() + && srcName.equals("client") + && environment.equals("client"); + + if (!srcName.equals(SourceSet.MAIN_SOURCE_SET_NAME) && !isSplitClientSourceSet) { configName += capitalizeCamelCaseName(srcName) + " "; } From 40b3ebc80e1bb7ec440297d3dfafd062bca2f486 Mon Sep 17 00:00:00 2001 From: modmuss50 Date: Fri, 20 May 2022 11:22:05 +0100 Subject: [PATCH 4/5] Add property to control if a remap jar includes client only entries. --- src/main/java/net/fabricmc/loom/task/RemapJarTask.java | 10 +++++++++- .../net/fabricmc/loom/task/RemapTaskConfiguration.java | 1 + 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/main/java/net/fabricmc/loom/task/RemapJarTask.java b/src/main/java/net/fabricmc/loom/task/RemapJarTask.java index fe1e53b4..ccdcccde 100644 --- a/src/main/java/net/fabricmc/loom/task/RemapJarTask.java +++ b/src/main/java/net/fabricmc/loom/task/RemapJarTask.java @@ -93,6 +93,9 @@ public abstract class RemapJarTask extends AbstractRemapJarTask { @Input public abstract Property getAddNestedDependencies(); + @Input + public abstract Property getIncludesClientOnlyClasses(); + private Supplier tinyRemapperService = Suppliers.memoize(() -> TinyRemapperService.getOrCreate(this)); @Inject @@ -101,6 +104,7 @@ public abstract class RemapJarTask extends AbstractRemapJarTask { getClasspath().from(getProject().getConfigurations().getByName(JavaPlugin.COMPILE_CLASSPATH_CONFIGURATION_NAME)); getAddNestedDependencies().convention(true).finalizeValueOnRead(); + getIncludesClientOnlyClasses().convention(false).finalizeValueOnRead(); Configuration includeConfiguration = getProject().getConfigurations().getByName(Constants.Configurations.INCLUDE); getNestedJars().from(new IncludedJarFactory(getProject()).getNestedJars(includeConfiguration)); @@ -144,7 +148,11 @@ public abstract class RemapJarTask extends AbstractRemapJarTask { setupLegacyMixinRefmapRemapping(params); } - if (extension.areEnvironmentSourceSetsSplit()) { + if (getIncludesClientOnlyClasses().get()) { + if (!extension.areEnvironmentSourceSetsSplit()) { + throw new UnsupportedOperationException("Jar cannot include client only classes as the sources are not split"); + } + final List clientOnlyJarEntries = getClientOnlyJarEntries(); params.getManifestAttributes().set(Map.of( "Fabric-Loom-Split-Environment", "true", diff --git a/src/main/java/net/fabricmc/loom/task/RemapTaskConfiguration.java b/src/main/java/net/fabricmc/loom/task/RemapTaskConfiguration.java index bedd5c52..4d54eefb 100644 --- a/src/main/java/net/fabricmc/loom/task/RemapTaskConfiguration.java +++ b/src/main/java/net/fabricmc/loom/task/RemapTaskConfiguration.java @@ -68,6 +68,7 @@ public class RemapTaskConfiguration { // Setup the input file and the nested deps task.getInputFile().convention(jarTask.getArchiveFile()); task.dependsOn(tasks.named(JavaPlugin.JAR_TASK_NAME)); + task.getIncludesClientOnlyClasses().set(project.provider(extension::areEnvironmentSourceSetsSplit)); }); // Configure the default jar task From 79668102c0f9bf9dc3d3301fc954ec038a5fecb4 Mon Sep 17 00:00:00 2001 From: modmuss50 Date: Fri, 20 May 2022 13:46:43 +0100 Subject: [PATCH 5/5] Fix crash when syncing idea when server only is enabled. Closes #652 --- .../minecraft/MinecraftLibraryProvider.java | 11 ----- .../net/fabricmc/loom/task/LoomTasks.java | 48 ++++++++++++++----- .../test/integration/MCJarConfigTest.groovy | 8 ++-- 3 files changed, 40 insertions(+), 27 deletions(-) diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/MinecraftLibraryProvider.java b/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/MinecraftLibraryProvider.java index b82ca2dc..94d33a8d 100644 --- a/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/MinecraftLibraryProvider.java +++ b/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/MinecraftLibraryProvider.java @@ -29,13 +29,10 @@ import java.util.regex.Pattern; import org.gradle.api.Project; import org.gradle.api.artifacts.ExternalModuleDependency; -import org.gradle.api.tasks.TaskContainer; -import org.gradle.api.tasks.TaskProvider; import net.fabricmc.loom.LoomGradleExtension; import net.fabricmc.loom.LoomRepositoryPlugin; import net.fabricmc.loom.configuration.providers.BundleMetadata; -import net.fabricmc.loom.task.ExtractNativesTask; import net.fabricmc.loom.util.Constants; import net.fabricmc.loom.util.OperatingSystem; @@ -57,15 +54,7 @@ public class MinecraftLibraryProvider { } if (versionInfo.hasNativesToExtract()) { - final TaskContainer tasks = project.getTasks(); - extension.createLazyConfiguration(Constants.Configurations.MINECRAFT_NATIVES, configuration -> configuration.setTransitive(false)); - - TaskProvider extractNativesTask = tasks.register("extractNatives", ExtractNativesTask.class, t -> { - t.setDescription("Extracts the minecraft platform specific natives."); - }); - - tasks.named("configureClientLaunch", configureClientLaunch -> configureClientLaunch.dependsOn(extractNativesTask)); } for (MinecraftVersionMeta.Library library : versionInfo.libraries()) { diff --git a/src/main/java/net/fabricmc/loom/task/LoomTasks.java b/src/main/java/net/fabricmc/loom/task/LoomTasks.java index 4bdbb39b..2d2e803e 100644 --- a/src/main/java/net/fabricmc/loom/task/LoomTasks.java +++ b/src/main/java/net/fabricmc/loom/task/LoomTasks.java @@ -52,10 +52,6 @@ public final class LoomTasks { }); RemapTaskConfiguration.setupRemap(project); - - tasks.register("downloadAssets", DownloadAssetsTask.class, t -> { - t.setDescription("Downloads required assets for Fabric."); - }); tasks.register("generateDLIConfig", GenerateDLIConfigTask.class, t -> { t.setDescription("Generate the DevLaunchInjector config file"); @@ -79,14 +75,6 @@ public final class LoomTasks { task.setGroup(Constants.TaskGroup.FABRIC); }); - tasks.register("configureClientLaunch", task -> { - task.dependsOn(tasks.named("downloadAssets")); - task.dependsOn(tasks.named("configureLaunch")); - - task.setDescription("Setup the required files to launch the Minecraft client"); - task.setGroup(Constants.TaskGroup.FABRIC); - }); - TaskProvider validateAccessWidener = tasks.register("validateAccessWidener", ValidateAccessWidenerTask.class, t -> { t.setDescription("Validate all the rules in the access widener against the Minecraft jar"); t.setGroup("verification"); @@ -96,6 +84,18 @@ public final class LoomTasks { registerIDETasks(tasks); registerRunTasks(tasks, project); + + // Must be done in afterEvaluate to allow time for the build script to configure the jar config. + project.afterEvaluate(p -> { + LoomGradleExtension extension = LoomGradleExtension.get(project); + + if (extension.getMinecraftJarConfiguration().get() == MinecraftJarConfiguration.SERVER_ONLY) { + // Server only, nothing more to do. + return; + } + + registerClientSetupTasks(project.getTasks(), extension.getMinecraftProvider().getVersionInfo().hasNativesToExtract()); + }); } private static void registerIDETasks(TaskContainer tasks) { @@ -157,6 +157,30 @@ public final class LoomTasks { }); } + private static void registerClientSetupTasks(TaskContainer tasks, boolean extractNatives) { + tasks.register("downloadAssets", DownloadAssetsTask.class, t -> { + t.setDescription("Downloads required game assets for Minecraft."); + }); + + if (extractNatives) { + tasks.register("extractNatives", ExtractNativesTask.class, t -> { + t.setDescription("Extracts the Minecraft platform specific natives."); + }); + } + + tasks.register("configureClientLaunch", task -> { + task.dependsOn(tasks.named("downloadAssets")); + task.dependsOn(tasks.named("configureLaunch")); + + if (extractNatives) { + task.dependsOn(tasks.named("extractNatives")); + } + + task.setDescription("Setup the required files to launch the Minecraft client"); + task.setGroup(Constants.TaskGroup.FABRIC); + }); + } + public static Provider getIDELaunchConfigureTaskName(Project project) { return project.provider(() -> { final MinecraftJarConfiguration jarConfiguration = LoomGradleExtension.get(project).getMinecraftJarConfiguration().get(); diff --git a/src/test/groovy/net/fabricmc/loom/test/integration/MCJarConfigTest.groovy b/src/test/groovy/net/fabricmc/loom/test/integration/MCJarConfigTest.groovy index d58d0741..66deb25f 100644 --- a/src/test/groovy/net/fabricmc/loom/test/integration/MCJarConfigTest.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/integration/MCJarConfigTest.groovy @@ -50,7 +50,7 @@ class MCJarConfigTest extends Specification implements GradleProjectTestTrait { ''' when: - def result = gradle.run(task: "build") + def result = gradle.run(tasks: ["build", "ideaSyncTask"]) then: result.task(":build").outcome == SUCCESS @@ -77,7 +77,7 @@ class MCJarConfigTest extends Specification implements GradleProjectTestTrait { ''' when: - def result = gradle.run(task: "build") + def result = gradle.run(tasks: ["build", "ideaSyncTask"]) then: result.task(":build").outcome == SUCCESS @@ -104,7 +104,7 @@ class MCJarConfigTest extends Specification implements GradleProjectTestTrait { ''' when: - def result = gradle.run(task: "build") + def result = gradle.run(tasks: ["build", "ideaSyncTask"]) then: result.task(":build").outcome == SUCCESS @@ -131,7 +131,7 @@ class MCJarConfigTest extends Specification implements GradleProjectTestTrait { ''' when: - def result = gradle.run(task: "build") + def result = gradle.run(tasks: ["build", "ideaSyncTask"]) then: result.task(":build").outcome == SUCCESS