From ac3fa8d410f392bcc0e589bc284a35a64775c4a7 Mon Sep 17 00:00:00 2001 From: modmuss50 Date: Fri, 2 Feb 2024 18:16:46 +0000 Subject: [PATCH 01/13] Start on Loom 1.6 --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index de84c29f..9158ae8c 100644 --- a/build.gradle +++ b/build.gradle @@ -50,7 +50,7 @@ tasks.withType(org.jetbrains.kotlin.gradle.tasks.KotlinCompile).all { } group = 'net.fabricmc' -def baseVersion = '1.5' +def baseVersion = '1.6' def ENV = System.getenv() if (ENV.BUILD_NUMBER) { From f26c49c36f14bd22bf929a4f8dbca330f9b4e03b Mon Sep 17 00:00:00 2001 From: modmuss50 Date: Sat, 3 Feb 2024 23:25:06 +0000 Subject: [PATCH 02/13] Disable DebugLineNumbersTest in CI tests. --- build.gradle | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/build.gradle b/build.gradle index 9158ae8c..464c2217 100644 --- a/build.gradle +++ b/build.gradle @@ -370,6 +370,11 @@ task writeActionsTestMatrix() { return } + if (it.name.endsWith("DebugLineNumbersTest.groovy")) { + // Known flakey test + return + } + def className = it.name.replace(".groovy", "") testMatrix.add("net.fabricmc.loom.test.integration.${className}") } From 8250b509a41a76d289e4817edb5bfc27045835ec Mon Sep 17 00:00:00 2001 From: modmuss Date: Sat, 3 Feb 2024 23:26:33 +0000 Subject: [PATCH 03/13] Prepare for SelfResolvingDependency's removal & Update to Gradle 8.6 (#1038) * Prepare for SelfResolvingDependency's removal Update to Gradle 8.6 * Update docker images * ProjectDependency is also a SRD * Throw if layered mappings are created too late. --- .github/workflows/test-push.yml | 6 +- .../bootstrap/LoomGradlePluginBootstrap.java | 2 +- gradle/libs.versions.toml | 4 +- gradle/test.libs.versions.toml | 2 +- gradle/wrapper/gradle-wrapper.properties | 2 +- .../fabricmc/loom/LoomGradleExtension.java | 7 + .../configuration/CompileConfiguration.java | 4 + .../loom/configuration/DependencyInfo.java | 16 +- .../configuration/FileDependencyInfo.java | 20 ++- .../loom/configuration/mods/ArtifactRef.java | 2 +- .../LayeredMappingSpecBuilderImpl.java | 6 + ...dency.java => LayeredMappingsFactory.java} | 141 +++++++----------- .../mappings/utils/DependencyFileSpec.java | 28 +++- .../extension/LoomGradleExtensionApiImpl.java | 19 ++- .../extension/LoomGradleExtensionImpl.java | 19 ++- .../loom/extension/LoomProblemReporter.java | 51 +++++++ .../loom/task/MigrateMappingsTask.java | 16 +- .../gradle/SelfResolvingDependencyUtils.java | 114 ++++++++++++++ .../loom/test/integration/KotlinTest.groovy | 2 +- .../projects/kotlin/build.gradle.kts | 19 ++- 20 files changed, 350 insertions(+), 130 deletions(-) rename src/main/java/net/fabricmc/loom/configuration/providers/mappings/{LayeredMappingsDependency.java => LayeredMappingsFactory.java} (62%) create mode 100644 src/main/java/net/fabricmc/loom/extension/LoomProblemReporter.java create mode 100644 src/main/java/net/fabricmc/loom/util/gradle/SelfResolvingDependencyUtils.java diff --git a/.github/workflows/test-push.yml b/.github/workflows/test-push.yml index 54e2b1b0..1cc59364 100644 --- a/.github/workflows/test-push.yml +++ b/.github/workflows/test-push.yml @@ -10,7 +10,7 @@ jobs: strategy: fail-fast: false matrix: - version: [8.3.0-jdk17] + version: [8.6.0-jdk17] runs-on: ubuntu-22.04 container: image: gradle:${{ matrix.version }} @@ -39,7 +39,7 @@ jobs: runs-on: ubuntu-22.04 container: - image: gradle:8.3.0-jdk17 + image: gradle:8.6.0-jdk17 options: --user root steps: @@ -58,7 +58,7 @@ jobs: strategy: fail-fast: false matrix: - version: [8.3.0-jdk17] + version: [8.6.0-jdk17] test: ${{ fromJson(needs.prepare_test_matrix.outputs.matrix) }} runs-on: ubuntu-22.04 diff --git a/bootstrap/src/main/java/net/fabricmc/loom/bootstrap/LoomGradlePluginBootstrap.java b/bootstrap/src/main/java/net/fabricmc/loom/bootstrap/LoomGradlePluginBootstrap.java index bd420dd0..8fbf513d 100644 --- a/bootstrap/src/main/java/net/fabricmc/loom/bootstrap/LoomGradlePluginBootstrap.java +++ b/bootstrap/src/main/java/net/fabricmc/loom/bootstrap/LoomGradlePluginBootstrap.java @@ -14,7 +14,7 @@ import org.gradle.util.GradleVersion; */ @SuppressWarnings("unused") public class LoomGradlePluginBootstrap implements Plugin { - private static final String MIN_SUPPORTED_GRADLE_VERSION = "8.3"; + private static final String MIN_SUPPORTED_GRADLE_VERSION = "8.6"; private static final int MIN_SUPPORTED_MAJOR_JAVA_VERSION = 17; private static final int MIN_SUPPORTED_MAJOR_IDEA_VERSION = 2021; diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 0b2075ca..b715786e 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -1,5 +1,5 @@ [versions] -kotlin = "1.9.0" +kotlin = "1.9.20" asm = "9.6" commons-io = "2.15.1" gson = "2.10.1" @@ -11,7 +11,7 @@ access-widener = "2.1.0" mapping-io = "0.5.1" lorenz-tiny = "4.0.2" mercury = "0.4.1" -kotlinx-metadata = "0.8.0" +kotlinx-metadata = "0.9.0" # Plugins spotless = "6.20.0" diff --git a/gradle/test.libs.versions.toml b/gradle/test.libs.versions.toml index 08fa5b91..97e556b5 100644 --- a/gradle/test.libs.versions.toml +++ b/gradle/test.libs.versions.toml @@ -6,7 +6,7 @@ mockito = "5.8.0" java-debug = "0.50.0" mixin = "0.11.4+mixin.0.8.5" -gradle-nightly = "8.7-20240104001326+0000" +gradle-nightly = "8.7-20240202001338+0000" fabric-loader = "0.15.3" fabric-installer = "1.0.0" diff --git a/gradle/wrapper/gradle-wrapper.properties b/gradle/wrapper/gradle-wrapper.properties index d11cdd90..2ea3535d 100644 --- a/gradle/wrapper/gradle-wrapper.properties +++ b/gradle/wrapper/gradle-wrapper.properties @@ -1,6 +1,6 @@ distributionBase=GRADLE_USER_HOME distributionPath=wrapper/dists -distributionUrl=https\://services.gradle.org/distributions/gradle-8.3-all.zip +distributionUrl=https\://services.gradle.org/distributions/gradle-8.6-all.zip networkTimeout=10000 validateDistributionUrl=true zipStoreBase=GRADLE_USER_HOME diff --git a/src/main/java/net/fabricmc/loom/LoomGradleExtension.java b/src/main/java/net/fabricmc/loom/LoomGradleExtension.java index 836138ba..15bb0cd4 100644 --- a/src/main/java/net/fabricmc/loom/LoomGradleExtension.java +++ b/src/main/java/net/fabricmc/loom/LoomGradleExtension.java @@ -25,6 +25,7 @@ package net.fabricmc.loom; import java.nio.file.Path; +import java.util.Collection; import java.util.List; import org.gradle.api.Project; @@ -38,12 +39,14 @@ import net.fabricmc.loom.api.mappings.layered.MappingsNamespace; import net.fabricmc.loom.configuration.InstallerData; import net.fabricmc.loom.configuration.LoomDependencyManager; import net.fabricmc.loom.configuration.accesswidener.AccessWidenerFile; +import net.fabricmc.loom.configuration.providers.mappings.LayeredMappingsFactory; import net.fabricmc.loom.configuration.providers.mappings.MappingConfiguration; 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.extension.LoomFiles; +import net.fabricmc.loom.extension.LoomProblemReporter; import net.fabricmc.loom.extension.MixinExtension; import net.fabricmc.loom.extension.RemapperExtensionHolder; import net.fabricmc.loom.util.download.DownloadBuilder; @@ -118,4 +121,8 @@ public interface LoomGradleExtension extends LoomGradleExtensionAPI { ListProperty getLibraryProcessors(); ListProperty getRemapperExtensions(); + + Collection getLayeredMappingFactories(); + + LoomProblemReporter getProblemReporter(); } diff --git a/src/main/java/net/fabricmc/loom/configuration/CompileConfiguration.java b/src/main/java/net/fabricmc/loom/configuration/CompileConfiguration.java index 041bc42d..302c9f93 100644 --- a/src/main/java/net/fabricmc/loom/configuration/CompileConfiguration.java +++ b/src/main/java/net/fabricmc/loom/configuration/CompileConfiguration.java @@ -53,6 +53,7 @@ import net.fabricmc.loom.configuration.accesswidener.AccessWidenerJarProcessor; import net.fabricmc.loom.configuration.ifaceinject.InterfaceInjectionProcessor; import net.fabricmc.loom.configuration.processors.MinecraftJarProcessorManager; import net.fabricmc.loom.configuration.processors.ModJavadocProcessor; +import net.fabricmc.loom.configuration.providers.mappings.LayeredMappingsFactory; import net.fabricmc.loom.configuration.providers.mappings.MappingConfiguration; import net.fabricmc.loom.configuration.providers.minecraft.MinecraftJarConfiguration; import net.fabricmc.loom.configuration.providers.minecraft.MinecraftProvider; @@ -148,6 +149,9 @@ public abstract class CompileConfiguration implements Runnable { extension.setMinecraftProvider(minecraftProvider); minecraftProvider.provide(); + // Created any layered mapping files. + LayeredMappingsFactory.afterEvaluate(configContext); + final DependencyInfo mappingsDep = DependencyInfo.create(getProject(), Configurations.MAPPINGS); final MappingConfiguration mappingConfiguration = MappingConfiguration.create(getProject(), configContext.serviceManager(), mappingsDep, minecraftProvider); extension.setMappingConfiguration(mappingConfiguration); diff --git a/src/main/java/net/fabricmc/loom/configuration/DependencyInfo.java b/src/main/java/net/fabricmc/loom/configuration/DependencyInfo.java index 3c09e2bf..f186476d 100644 --- a/src/main/java/net/fabricmc/loom/configuration/DependencyInfo.java +++ b/src/main/java/net/fabricmc/loom/configuration/DependencyInfo.java @@ -32,8 +32,11 @@ import org.gradle.api.Project; import org.gradle.api.artifacts.Configuration; import org.gradle.api.artifacts.Dependency; import org.gradle.api.artifacts.DependencySet; +import org.gradle.api.artifacts.FileCollectionDependency; import org.gradle.api.artifacts.ResolvedDependency; -import org.gradle.api.artifacts.SelfResolvingDependency; + +import net.fabricmc.loom.LoomGradleExtension; +import net.fabricmc.loom.util.gradle.SelfResolvingDependencyUtils; public class DependencyInfo { final Project project; @@ -61,8 +64,11 @@ public class DependencyInfo { } public static DependencyInfo create(Project project, Dependency dependency, Configuration sourceConfiguration) { - if (dependency instanceof SelfResolvingDependency selfResolvingDependency) { - return new FileDependencyInfo(project, selfResolvingDependency, sourceConfiguration); + if (SelfResolvingDependencyUtils.isExplicitSRD(dependency)) { + LoomGradleExtension.get(project).getProblemReporter().reportSelfResolvingDependencyUsage(); + return FileDependencyInfo.createForDeprecatedSRD(project, dependency, sourceConfiguration); + } else if (dependency instanceof FileCollectionDependency fileCollectionDependency) { + return new FileDependencyInfo(project, fileCollectionDependency, sourceConfiguration); } else { return new DependencyInfo(project, dependency, sourceConfiguration); } @@ -99,10 +105,6 @@ public class DependencyInfo { } public Set resolve() { - if (dependency instanceof SelfResolvingDependency selfResolvingDependency) { - return selfResolvingDependency.resolve(); - } - return sourceConfiguration.files(dependency); } diff --git a/src/main/java/net/fabricmc/loom/configuration/FileDependencyInfo.java b/src/main/java/net/fabricmc/loom/configuration/FileDependencyInfo.java index 2f55bbe0..73bbc878 100644 --- a/src/main/java/net/fabricmc/loom/configuration/FileDependencyInfo.java +++ b/src/main/java/net/fabricmc/loom/configuration/FileDependencyInfo.java @@ -42,19 +42,24 @@ import org.apache.commons.io.FilenameUtils; import org.gradle.api.InvalidUserDataException; import org.gradle.api.Project; import org.gradle.api.artifacts.Configuration; -import org.gradle.api.artifacts.SelfResolvingDependency; +import org.gradle.api.artifacts.Dependency; +import org.gradle.api.artifacts.FileCollectionDependency; import net.fabricmc.loom.util.ZipUtils; +import net.fabricmc.loom.util.gradle.SelfResolvingDependencyUtils; public class FileDependencyInfo extends DependencyInfo { protected final Map classifierToFile = new HashMap<>(); protected final Set resolvedFiles; protected final String group, name, version; - FileDependencyInfo(Project project, SelfResolvingDependency dependency, Configuration configuration) { + FileDependencyInfo(Project project, FileCollectionDependency dependency, Configuration configuration) { + this(project, dependency, configuration, dependency.getFiles().getFiles()); + } + + private FileDependencyInfo(Project project, Dependency dependency, Configuration configuration, Set files) { super(project, dependency, configuration); - Set files = dependency.resolve(); this.resolvedFiles = files; switch (files.size()) { case 0 -> //Don't think Gradle would ever let you do this @@ -126,6 +131,15 @@ public class FileDependencyInfo extends DependencyInfo { } } + @Deprecated // Remove in Gradle 9 + public static FileDependencyInfo createForDeprecatedSRD(Project project, Dependency dependency, Configuration configuration) { + if (!SelfResolvingDependencyUtils.isExplicitSRD(dependency)) { + throw new IllegalArgumentException("Dependency is a FileCollectionDependency"); + } + + return new FileDependencyInfo(project, dependency, configuration, SelfResolvingDependencyUtils.resolve(dependency)); + } + @Override public String getResolvedVersion() { return version; diff --git a/src/main/java/net/fabricmc/loom/configuration/mods/ArtifactRef.java b/src/main/java/net/fabricmc/loom/configuration/mods/ArtifactRef.java index 1e6798d0..b5c9ca8d 100644 --- a/src/main/java/net/fabricmc/loom/configuration/mods/ArtifactRef.java +++ b/src/main/java/net/fabricmc/loom/configuration/mods/ArtifactRef.java @@ -80,7 +80,7 @@ public interface ArtifactRef { public void applyToConfiguration(Project project, Configuration configuration) { final DependencyHandler dependencies = project.getDependencies(); - Dependency dep = dependencies.module(artifact.getModuleVersion() + (artifact.getClassifier() == null ? "" : ':' + artifact.getClassifier())); // the owning module of the artifact + Dependency dep = dependencies.create(artifact.getModuleVersion() + (artifact.getClassifier() == null ? "" : ':' + artifact.getClassifier())); // the owning module of the artifact if (dep instanceof ModuleDependency moduleDependency) { moduleDependency.setTransitive(false); diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/LayeredMappingSpecBuilderImpl.java b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/LayeredMappingSpecBuilderImpl.java index 26dd21b9..1be17b7e 100644 --- a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/LayeredMappingSpecBuilderImpl.java +++ b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/LayeredMappingSpecBuilderImpl.java @@ -85,4 +85,10 @@ public class LayeredMappingSpecBuilderImpl implements LayeredMappingSpecBuilder return new LayeredMappingSpec(Collections.unmodifiableList(builtLayers)); } + + public static LayeredMappingSpec buildOfficialMojangMappings() { + var builder = new LayeredMappingSpecBuilderImpl(); + builder.officialMojangMappings(); + return builder.build(); + } } diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/LayeredMappingsDependency.java b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/LayeredMappingsFactory.java similarity index 62% rename from src/main/java/net/fabricmc/loom/configuration/providers/mappings/LayeredMappingsDependency.java rename to src/main/java/net/fabricmc/loom/configuration/providers/mappings/LayeredMappingsFactory.java index e99f4551..24058594 100644 --- a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/LayeredMappingsDependency.java +++ b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/LayeredMappingsFactory.java @@ -24,9 +24,9 @@ package net.fabricmc.loom.configuration.providers.mappings; -import java.io.File; import java.io.IOException; import java.io.StringWriter; +import java.io.UncheckedIOException; import java.io.Writer; import java.nio.charset.StandardCharsets; import java.nio.file.Files; @@ -34,20 +34,18 @@ import java.nio.file.Path; import java.util.Collections; import java.util.List; import java.util.Map; -import java.util.Objects; -import java.util.Set; import org.gradle.api.Project; import org.gradle.api.artifacts.Dependency; -import org.gradle.api.artifacts.FileCollectionDependency; -import org.gradle.api.artifacts.SelfResolvingDependency; -import org.gradle.api.file.FileCollection; -import org.gradle.api.tasks.TaskDependency; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import net.fabricmc.loom.LoomGradlePlugin; import net.fabricmc.loom.api.mappings.layered.MappingContext; import net.fabricmc.loom.api.mappings.layered.MappingLayer; import net.fabricmc.loom.api.mappings.layered.MappingsNamespace; +import net.fabricmc.loom.configuration.ConfigContext; +import net.fabricmc.loom.configuration.mods.dependency.LocalMavenHelper; import net.fabricmc.loom.configuration.providers.mappings.extras.unpick.UnpickLayer; import net.fabricmc.loom.configuration.providers.mappings.utils.AddConstructorMappingVisitor; import net.fabricmc.loom.util.ZipUtils; @@ -56,43 +54,61 @@ import net.fabricmc.mappingio.adapter.MappingSourceNsSwitch; import net.fabricmc.mappingio.format.tiny.Tiny2FileWriter; import net.fabricmc.mappingio.tree.MemoryMappingTree; -public class LayeredMappingsDependency implements SelfResolvingDependency, FileCollectionDependency { +public record LayeredMappingsFactory(LayeredMappingSpec spec) { private static final String GROUP = "loom"; private static final String MODULE = "mappings"; + private static final Logger LOGGER = LoggerFactory.getLogger(LayeredMappingsFactory.class); - private final Project project; - private final MappingContext mappingContext; - private final LayeredMappingSpec layeredMappingSpec; - private final String version; - - public LayeredMappingsDependency(Project project, MappingContext mappingContext, LayeredMappingSpec layeredMappingSpec, String version) { - this.project = project; - this.mappingContext = mappingContext; - this.layeredMappingSpec = layeredMappingSpec; - this.version = version; - } - - @Override - public Set resolve() { - Path mappingsDir = mappingContext.minecraftProvider().dir("layered").toPath(); - Path mappingsFile = mappingsDir.resolve(String.format("%s.%s-%s.tiny", GROUP, MODULE, getVersion())); - - if (!Files.exists(mappingsFile) || mappingContext.refreshDeps()) { + /* + As we no longer have SelfResolvingDependency we now always create the mappings file after evaluation. + This works in a similar way to how remapped mods are handled. + */ + public static void afterEvaluate(ConfigContext configContext) { + for (LayeredMappingsFactory layeredMappingFactory : configContext.extension().getLayeredMappingFactories()) { try { - var processor = new LayeredMappingsProcessor(layeredMappingSpec); - List layers = processor.resolveLayers(mappingContext); - - Files.deleteIfExists(mappingsFile); - - writeMapping(processor, layers, mappingsFile); - writeSignatureFixes(processor, layers, mappingsFile); - writeUnpickData(processor, layers, mappingsFile); + layeredMappingFactory.evaluate(configContext); } catch (IOException e) { - throw new RuntimeException("Failed to resolve layered mappings", e); + throw new UncheckedIOException("Failed to setup layered mappings: %s".formatted(layeredMappingFactory.mavenNotation()), e); } } + } - return Collections.singleton(mappingsFile.toFile()); + private void evaluate(ConfigContext configContext) throws IOException { + LOGGER.info("Evaluating layer mapping: {}", mavenNotation()); + + final Path mavenRepoDir = configContext.extension().getFiles().getGlobalMinecraftRepo().toPath(); + final LocalMavenHelper maven = new LocalMavenHelper(GROUP, MODULE, spec().getVersion(), null, mavenRepoDir); + final Path jar = resolve(configContext.project()); + maven.copyToMaven(jar, null); + } + + public Path resolve(Project project) throws IOException { + final MappingContext mappingContext = new GradleMappingContext(project, spec.getVersion().replace("+", "_").replace(".", "_")); + final Path mappingsDir = mappingContext.minecraftProvider().dir("layered").toPath(); + final Path mappingsZip = mappingsDir.resolve(String.format("%s.%s-%s.jar", GROUP, MODULE, spec.getVersion())); + + if (Files.exists(mappingsZip) && !mappingContext.refreshDeps()) { + return mappingsZip; + } + + var processor = new LayeredMappingsProcessor(spec); + List layers = processor.resolveLayers(mappingContext); + + Files.deleteIfExists(mappingsZip); + + writeMapping(processor, layers, mappingsZip); + writeSignatureFixes(processor, layers, mappingsZip); + writeUnpickData(processor, layers, mappingsZip); + + return mappingsZip; + } + + public Dependency createDependency(Project project) { + return project.getDependencies().create(mavenNotation()); + } + + public String mavenNotation() { + return String.format("%s:%s:%s", GROUP, MODULE, spec.getVersion()); } private void writeMapping(LayeredMappingsProcessor processor, List layers, Path mappingsFile) throws IOException { @@ -133,57 +149,4 @@ public class LayeredMappingsDependency implements SelfResolvingDependency, FileC ZipUtils.add(mappingsFile, "extras/definitions.unpick", unpickData.definitions()); ZipUtils.add(mappingsFile, "extras/unpick.json", unpickData.metadata().asJson()); } - - @Override - public Set resolve(boolean transitive) { - return resolve(); - } - - @Override - public TaskDependency getBuildDependencies() { - return task -> Collections.emptySet(); - } - - @Override - public String getGroup() { - return GROUP; - } - - @Override - public String getName() { - return MODULE; - } - - @Override - public String getVersion() { - return version; - } - - @Override - public boolean contentEquals(Dependency dependency) { - if (dependency instanceof LayeredMappingsDependency layeredMappingsDependency) { - return Objects.equals(layeredMappingsDependency.getVersion(), this.getVersion()); - } - - return false; - } - - @Override - public Dependency copy() { - return new LayeredMappingsDependency(project, mappingContext, layeredMappingSpec, version); - } - - @Override - public String getReason() { - return null; - } - - @Override - public void because(String s) { - } - - @Override - public FileCollection getFiles() { - return project.files(resolve()); - } } diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/utils/DependencyFileSpec.java b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/utils/DependencyFileSpec.java index 99c9d7d3..0eac2fe5 100644 --- a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/utils/DependencyFileSpec.java +++ b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/utils/DependencyFileSpec.java @@ -30,21 +30,37 @@ import java.util.Objects; import java.util.Set; import org.gradle.api.artifacts.Dependency; -import org.gradle.api.artifacts.SelfResolvingDependency; +import org.gradle.api.artifacts.FileCollectionDependency; import net.fabricmc.loom.api.mappings.layered.MappingContext; import net.fabricmc.loom.api.mappings.layered.spec.FileSpec; +import net.fabricmc.loom.configuration.providers.mappings.GradleMappingContext; +import net.fabricmc.loom.util.gradle.SelfResolvingDependencyUtils; public record DependencyFileSpec(Dependency dependency) implements FileSpec { @Override public Path get(MappingContext context) { - if (dependency instanceof SelfResolvingDependency selfResolvingDependency) { - Set files = selfResolvingDependency.resolve(); + if (SelfResolvingDependencyUtils.isExplicitSRD(dependency)) { + if (context instanceof GradleMappingContext gradleMappingContext) { + gradleMappingContext.getExtension().getProblemReporter().reportSelfResolvingDependencyUsage(); + } - if (files.size() == 0) { - throw new RuntimeException("SelfResolvingDependency (%s) resolved no files".formatted(selfResolvingDependency.toString())); + Set files = SelfResolvingDependencyUtils.resolve(dependency); + + if (files.isEmpty()) { + throw new RuntimeException("SelfResolvingDependency (%s) resolved no files".formatted(dependency.toString())); } else if (files.size() > 1) { - throw new RuntimeException("SelfResolvingDependency (%s) resolved too many files (%d) only 1 is expected".formatted(selfResolvingDependency.toString(), files.size())); + throw new RuntimeException("SelfResolvingDependency (%s) resolved too many files (%d) only 1 is expected".formatted(dependency.toString(), files.size())); + } + + return files.iterator().next().toPath(); + } else if (dependency instanceof FileCollectionDependency fileCollectionDependency) { + Set files = fileCollectionDependency.getFiles().getFiles(); + + if (files.isEmpty()) { + throw new RuntimeException("FileCollectionDependency (%s) resolved no files".formatted(fileCollectionDependency.toString())); + } else if (files.size() > 1) { + throw new RuntimeException("FileCollectionDependency (%s) resolved too many files (%d) only 1 is expected".formatted(fileCollectionDependency.toString(), files.size())); } return files.iterator().next().toPath(); diff --git a/src/main/java/net/fabricmc/loom/extension/LoomGradleExtensionApiImpl.java b/src/main/java/net/fabricmc/loom/extension/LoomGradleExtensionApiImpl.java index d9abf760..56d28ee6 100644 --- a/src/main/java/net/fabricmc/loom/extension/LoomGradleExtensionApiImpl.java +++ b/src/main/java/net/fabricmc/loom/extension/LoomGradleExtensionApiImpl.java @@ -1,7 +1,7 @@ /* * This file is part of fabric-loom, licensed under the MIT License (MIT). * - * Copyright (c) 2021-2022 FabricMC + * Copyright (c) 2021-2024 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 @@ -26,6 +26,8 @@ package net.fabricmc.loom.extension; import java.io.File; import java.io.IOException; +import java.util.HashMap; +import java.util.Map; import java.util.Set; import org.gradle.api.Action; @@ -59,10 +61,9 @@ import net.fabricmc.loom.api.remapping.RemapperParameters; import net.fabricmc.loom.configuration.RemapConfigurations; import net.fabricmc.loom.configuration.ide.RunConfigSettings; import net.fabricmc.loom.configuration.processors.JarProcessor; -import net.fabricmc.loom.configuration.providers.mappings.GradleMappingContext; import net.fabricmc.loom.configuration.providers.mappings.LayeredMappingSpec; import net.fabricmc.loom.configuration.providers.mappings.LayeredMappingSpecBuilderImpl; -import net.fabricmc.loom.configuration.providers.mappings.LayeredMappingsDependency; +import net.fabricmc.loom.configuration.providers.mappings.LayeredMappingsFactory; import net.fabricmc.loom.configuration.providers.minecraft.MinecraftJarConfiguration; import net.fabricmc.loom.configuration.providers.minecraft.MinecraftSourceSets; import net.fabricmc.loom.task.GenerateSourcesTask; @@ -102,6 +103,9 @@ public abstract class LoomGradleExtensionApiImpl implements LoomGradleExtensionA // A common mistake with layered mappings is to call the wrong `officialMojangMappings` method, use this to keep track of when we are building a layered mapping spec. protected final ThreadLocal layeredSpecBuilderScope = ThreadLocal.withInitial(() -> false); + protected boolean hasEvaluatedLayeredMappings = false; + protected final Map layeredMappingsDependencyMap = new HashMap<>(); + protected LoomGradleExtensionApiImpl(Project project, LoomFiles directories) { this.jarProcessors = project.getObjects().listProperty(JarProcessor.class) .empty(); @@ -210,14 +214,19 @@ public abstract class LoomGradleExtensionApiImpl implements LoomGradleExtensionA @Override public Dependency layered(Action action) { + if (hasEvaluatedLayeredMappings) { + throw new IllegalStateException("Layered mappings have already been evaluated"); + } + LayeredMappingSpecBuilderImpl builder = new LayeredMappingSpecBuilderImpl(); layeredSpecBuilderScope.set(true); action.execute(builder); layeredSpecBuilderScope.set(false); - LayeredMappingSpec builtSpec = builder.build(); - return new LayeredMappingsDependency(getProject(), new GradleMappingContext(getProject(), builtSpec.getVersion().replace("+", "_").replace(".", "_")), builtSpec, builtSpec.getVersion()); + final LayeredMappingSpec builtSpec = builder.build(); + final LayeredMappingsFactory layeredMappingsFactory = layeredMappingsDependencyMap.computeIfAbsent(builtSpec, LayeredMappingsFactory::new); + return layeredMappingsFactory.createDependency(getProject()); } @Override diff --git a/src/main/java/net/fabricmc/loom/extension/LoomGradleExtensionImpl.java b/src/main/java/net/fabricmc/loom/extension/LoomGradleExtensionImpl.java index 7fe809fc..382fbc14 100644 --- a/src/main/java/net/fabricmc/loom/extension/LoomGradleExtensionImpl.java +++ b/src/main/java/net/fabricmc/loom/extension/LoomGradleExtensionImpl.java @@ -1,7 +1,7 @@ /* * This file is part of fabric-loom, licensed under the MIT License (MIT). * - * Copyright (c) 2021 FabricMC + * Copyright (c) 2021-2024 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 @@ -27,6 +27,8 @@ package net.fabricmc.loom.extension; import java.net.URISyntaxException; import java.nio.file.Path; import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; import java.util.List; import java.util.Objects; @@ -43,6 +45,7 @@ import net.fabricmc.loom.configuration.InstallerData; import net.fabricmc.loom.configuration.LoomDependencyManager; import net.fabricmc.loom.configuration.accesswidener.AccessWidenerFile; import net.fabricmc.loom.configuration.providers.mappings.IntermediaryMappingsProvider; +import net.fabricmc.loom.configuration.providers.mappings.LayeredMappingsFactory; import net.fabricmc.loom.configuration.providers.mappings.MappingConfiguration; import net.fabricmc.loom.configuration.providers.minecraft.MinecraftProvider; import net.fabricmc.loom.configuration.providers.minecraft.library.LibraryProcessorManager; @@ -70,6 +73,7 @@ public class LoomGradleExtensionImpl extends LoomGradleExtensionApiImpl implemen private boolean refreshDeps; private Provider multiProjectOptimisation; private final ListProperty libraryProcessorFactories; + private final LoomProblemReporter problemReporter; public LoomGradleExtensionImpl(Project project, LoomFiles files) { super(project, files); @@ -97,6 +101,8 @@ public class LoomGradleExtensionImpl extends LoomGradleExtensionApiImpl implemen if (refreshDeps) { project.getLogger().lifecycle("Refresh dependencies is in use, loom will be significantly slower."); } + + problemReporter = project.getObjects().newInstance(LoomProblemReporter.class); } @Override @@ -252,6 +258,12 @@ public class LoomGradleExtensionImpl extends LoomGradleExtensionApiImpl implemen return remapperExtensions; } + @Override + public Collection getLayeredMappingFactories() { + hasEvaluatedLayeredMappings = true; + return Collections.unmodifiableCollection(layeredMappingsDependencyMap.values()); + } + @Override protected void configureIntermediateMappingsProviderInternal(T provider) { provider.getMinecraftVersion().set(getProject().provider(() -> getMinecraftProvider().minecraftVersion())); @@ -260,4 +272,9 @@ public class LoomGradleExtensionImpl extends LoomGradleExtensionApiImpl implemen provider.getDownloader().set(this::download); provider.getDownloader().disallowChanges(); } + + @Override + public LoomProblemReporter getProblemReporter() { + return problemReporter; + } } diff --git a/src/main/java/net/fabricmc/loom/extension/LoomProblemReporter.java b/src/main/java/net/fabricmc/loom/extension/LoomProblemReporter.java new file mode 100644 index 00000000..d8eee138 --- /dev/null +++ b/src/main/java/net/fabricmc/loom/extension/LoomProblemReporter.java @@ -0,0 +1,51 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2024 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.extension; + +import javax.inject.Inject; + +import org.gradle.api.problems.ProblemReporter; +import org.gradle.api.problems.Problems; +import org.gradle.api.problems.Severity; + +public abstract class LoomProblemReporter { + private final ProblemReporter problemReporter; + + @Inject + public LoomProblemReporter(Problems problems) { + this.problemReporter = problems.forNamespace("net.fabricmc.loom"); + } + + public void reportSelfResolvingDependencyUsage() { + problemReporter.reporting(spec -> spec + .label("SelfResolvingDependency is deprecated") + .details("SelfResolvingDependency has been deprecated for removal in Gradle 9") + .solution("Please replace usages of SelfResolvingDependency") + .documentedAt("https://github.com/gradle/gradle/pull/27420") + .severity(Severity.WARNING) + .stackLocation() + ); + } +} diff --git a/src/main/java/net/fabricmc/loom/task/MigrateMappingsTask.java b/src/main/java/net/fabricmc/loom/task/MigrateMappingsTask.java index 64e55626..d40bd3dd 100644 --- a/src/main/java/net/fabricmc/loom/task/MigrateMappingsTask.java +++ b/src/main/java/net/fabricmc/loom/task/MigrateMappingsTask.java @@ -26,8 +26,10 @@ package net.fabricmc.loom.task; import java.io.File; import java.io.IOException; +import java.io.UncheckedIOException; import java.nio.file.Files; import java.nio.file.Path; +import java.util.Collections; import java.util.Set; import com.google.common.collect.ImmutableMap; @@ -50,8 +52,8 @@ import org.gradle.work.DisableCachingByDefault; import net.fabricmc.loom.LoomGradleExtension; import net.fabricmc.loom.api.mappings.layered.MappingsNamespace; -import net.fabricmc.loom.api.mappings.layered.spec.LayeredMappingSpecBuilder; -import net.fabricmc.loom.configuration.providers.mappings.LayeredMappingsDependency; +import net.fabricmc.loom.configuration.providers.mappings.LayeredMappingSpecBuilderImpl; +import net.fabricmc.loom.configuration.providers.mappings.LayeredMappingsFactory; import net.fabricmc.loom.configuration.providers.mappings.MappingConfiguration; import net.fabricmc.loom.util.FileSystemUtil; import net.fabricmc.loom.util.SourceRemapper; @@ -133,8 +135,8 @@ public abstract class MigrateMappingsTask extends AbstractLoomTask { throw new UnsupportedOperationException("Migrating Mojang mappings is currently only supported for the specified minecraft version"); } - LayeredMappingsDependency dep = (LayeredMappingsDependency) getExtension().layered(LayeredMappingSpecBuilder::officialMojangMappings); - files = dep.resolve(); + LayeredMappingsFactory dep = new LayeredMappingsFactory(LayeredMappingSpecBuilderImpl.buildOfficialMojangMappings()); + files = Collections.singleton(dep.resolve(getProject()).toFile()); } else { Dependency dependency = project.getDependencies().create(mappings); files = project.getConfigurations().detachedConfiguration(dependency).resolve(); @@ -143,11 +145,13 @@ public abstract class MigrateMappingsTask extends AbstractLoomTask { project.getLogger().info("Could not locate mappings, presuming V2 Yarn"); try { - files = project.getConfigurations().detachedConfiguration(project.getDependencies().module(ImmutableMap.of("group", "net.fabricmc", "name", "yarn", "version", mappings, "classifier", "v2"))).resolve(); + files = project.getConfigurations().detachedConfiguration(project.getDependencies().create(ImmutableMap.of("group", "net.fabricmc", "name", "yarn", "version", mappings, "classifier", "v2"))).resolve(); } catch (GradleException ignored2) { project.getLogger().info("Could not locate mappings, presuming V1 Yarn"); - files = project.getConfigurations().detachedConfiguration(project.getDependencies().module(ImmutableMap.of("group", "net.fabricmc", "name", "yarn", "version", mappings))).resolve(); + files = project.getConfigurations().detachedConfiguration(project.getDependencies().create(ImmutableMap.of("group", "net.fabricmc", "name", "yarn", "version", mappings))).resolve(); } + } catch (IOException e) { + throw new UncheckedIOException("Failed to resolve mappings", e); } if (files.isEmpty()) { diff --git a/src/main/java/net/fabricmc/loom/util/gradle/SelfResolvingDependencyUtils.java b/src/main/java/net/fabricmc/loom/util/gradle/SelfResolvingDependencyUtils.java new file mode 100644 index 00000000..cfbddd98 --- /dev/null +++ b/src/main/java/net/fabricmc/loom/util/gradle/SelfResolvingDependencyUtils.java @@ -0,0 +1,114 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2024 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.util.gradle; + +import java.io.File; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.util.Set; + +import javax.annotation.Nullable; + +import org.gradle.api.artifacts.Dependency; +import org.gradle.api.artifacts.FileCollectionDependency; +import org.gradle.api.artifacts.ProjectDependency; + +// SelfResolvingDependency is deprecated for removal, use reflection to ensure backwards compat. +@Deprecated +public class SelfResolvingDependencyUtils { + // Set this system prop to disable SRD support before Gradle does. + public static final boolean DISABLE_SRD_SUPPORT = System.getProperty("fabric.loom.disable.srd") != null; + + private static final String SELF_RESOLVING_DEPENDENCY_CLASS_NAME = "org.gradle.api.artifacts.SelfResolvingDependency"; + @Nullable + private static final Class SELF_RESOLVING_DEPENDENCY_CLASS = getSelfResolvingDependencyOrNull(); + @Nullable + private static final Method RESOLVE_METHOD = getResolveMethod(SELF_RESOLVING_DEPENDENCY_CLASS); + + /** + * @return true when dependency is a SelfResolvingDependency but NOT a FileCollectionDependency. + */ + public static boolean isExplicitSRD(Dependency dependency) { + // FileCollectionDependency is usually the replacement for SelfResolvingDependency + if (dependency instanceof FileCollectionDependency) { + return false; + } else if (dependency instanceof ProjectDependency) { + return false; + } + + return isSRD(dependency); + } + + private static boolean isSRD(Dependency dependency) { + if (SELF_RESOLVING_DEPENDENCY_CLASS == null) { + return false; + } + + return dependency.getClass().isAssignableFrom(SELF_RESOLVING_DEPENDENCY_CLASS); + } + + public static Set resolve(Dependency dependency) { + if (!isSRD(dependency)) { + throw new IllegalStateException("dependency is not a SelfResolvingDependency"); + } + + try { + return (Set) RESOLVE_METHOD.invoke(dependency); + } catch (IllegalAccessException | InvocationTargetException e) { + throw new RuntimeException("Failed to resolve SelfResolvingDependency", e); + } + } + + @Nullable + private static Class getSelfResolvingDependencyOrNull() { + if (DISABLE_SRD_SUPPORT) { + // Lets pretend SRD doesnt exist. + return null; + } + + try { + return Class.forName(SELF_RESOLVING_DEPENDENCY_CLASS_NAME); + } catch (ClassNotFoundException e) { + // Gradle 9+ + return null; + } + } + + @Nullable + private static Method getResolveMethod(Class clazz) { + if (clazz == null) { + // Gradle 9+ + return null; + } + + try { + var method = clazz.getDeclaredMethod("resolve"); + method.setAccessible(true); + return method; + } catch (NoSuchMethodException e) { + throw new IllegalStateException("Failed to get SelfResolvingDependency.resolve() method", e); + } + } +} diff --git a/src/test/groovy/net/fabricmc/loom/test/integration/KotlinTest.groovy b/src/test/groovy/net/fabricmc/loom/test/integration/KotlinTest.groovy index 93a8140c..5b59c390 100644 --- a/src/test/groovy/net/fabricmc/loom/test/integration/KotlinTest.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/integration/KotlinTest.groovy @@ -40,7 +40,7 @@ class KotlinTest extends Specification implements GradleProjectTestTrait { def gradle = gradleProject(project: "kotlin", version: version) def server = ServerRunner.create(gradle.projectDir, "1.16.5") .withMod(gradle.getOutputFile("fabric-example-mod-0.0.1.jar")) - .downloadMod(ServerRunner.FABRIC_LANG_KOTLIN, "fabric-language-kotlin-1.8.7+kotlin.1.7.22.jar") + .downloadMod(ServerRunner.FABRIC_LANG_KOTLIN, "fabric-language-kotlin-1.10.17+kotlin.1.9.22.jar") when: def result = gradle.run(tasks: [ diff --git a/src/test/resources/projects/kotlin/build.gradle.kts b/src/test/resources/projects/kotlin/build.gradle.kts index 7f2e0f5d..75a1015d 100644 --- a/src/test/resources/projects/kotlin/build.gradle.kts +++ b/src/test/resources/projects/kotlin/build.gradle.kts @@ -1,8 +1,10 @@ import java.util.Properties +import org.jetbrains.kotlin.gradle.dsl.KotlinCompile +import org.jetbrains.kotlin.gradle.dsl.KotlinJvmOptions plugins { - kotlin("jvm") version "1.7.22" - kotlin("plugin.serialization") version "1.7.22" + kotlin("jvm") version "1.9.22" + kotlin("plugin.serialization") version "1.9.22" id("fabric-loom") `maven-publish` } @@ -12,6 +14,17 @@ java { targetCompatibility = JavaVersion.VERSION_1_8 } +tasks { + withType { + options.release.set(8) + } + withType> { + kotlinOptions { + jvmTarget = "1.8" + } + } +} + group = "com.example" version = "0.0.1" @@ -19,7 +32,7 @@ dependencies { minecraft(group = "com.mojang", name = "minecraft", version = "1.16.5") mappings(group = "net.fabricmc", name = "yarn", version = "1.16.5+build.5", classifier = "v2") modImplementation("net.fabricmc:fabric-loader:0.12.12") - modImplementation(group = "net.fabricmc", name = "fabric-language-kotlin", version = "1.8.7+kotlin.1.7.22") + modImplementation(group = "net.fabricmc", name = "fabric-language-kotlin", version = "1.10.17+kotlin.1.9.22") } publishing { From b2376a098152fa5cc4434697ae7d44b1dcad8662 Mon Sep 17 00:00:00 2001 From: modmuss50 Date: Sat, 3 Feb 2024 23:30:13 +0000 Subject: [PATCH 04/13] Fallback to maven central for artifacts such as sources or javadocs that are not mirrored on Mojang's repo. Fixes #1032 --- src/main/java/net/fabricmc/loom/LoomRepositoryPlugin.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/main/java/net/fabricmc/loom/LoomRepositoryPlugin.java b/src/main/java/net/fabricmc/loom/LoomRepositoryPlugin.java index 29b4a526..ef9bd58e 100644 --- a/src/main/java/net/fabricmc/loom/LoomRepositoryPlugin.java +++ b/src/main/java/net/fabricmc/loom/LoomRepositoryPlugin.java @@ -117,6 +117,10 @@ public class LoomRepositoryPlugin implements Plugin { sources.artifact(); sources.ignoreGradleMetadataRedirection(); }); + + // Fallback to maven central for artifacts such as sources or javadocs that are not mirrored on Mojang's repo. + // See: https://github.com/FabricMC/fabric-loom/issues/1032 + repo.artifactUrls(ArtifactRepositoryContainer.MAVEN_CENTRAL_URL); }); } From 2a385b3e2b015cbf64f59fd29110bdbc287ea570 Mon Sep 17 00:00:00 2001 From: Jason Penilla <11360596+jpenilla@users.noreply.github.com> Date: Sat, 3 Feb 2024 16:33:28 -0700 Subject: [PATCH 05/13] Improve locking strategy for concurrent loom executions (#1031) * Improve locking strategy for concurrent loom executions This is especially useful for when IntelliJ decides to randomly sync the Gradle project while I am running Gradle from the command line already. * Fix style violations * Adjust feedback messages and use Duration for timeout * Fixup message --- .../configuration/CompileConfiguration.java | 117 +++++++++++++++--- 1 file changed, 103 insertions(+), 14 deletions(-) diff --git a/src/main/java/net/fabricmc/loom/configuration/CompileConfiguration.java b/src/main/java/net/fabricmc/loom/configuration/CompileConfiguration.java index 302c9f93..2d77629d 100644 --- a/src/main/java/net/fabricmc/loom/configuration/CompileConfiguration.java +++ b/src/main/java/net/fabricmc/loom/configuration/CompileConfiguration.java @@ -31,11 +31,15 @@ import java.io.UncheckedIOException; import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; +import java.time.Duration; import java.util.function.Consumer; import javax.inject.Inject; +import org.gradle.api.GradleException; import org.gradle.api.Project; +import org.gradle.api.logging.Logger; +import org.gradle.api.logging.Logging; import org.gradle.api.plugins.JavaPlugin; import org.gradle.api.tasks.AbstractCopyTask; import org.gradle.api.tasks.SourceSet; @@ -92,8 +96,10 @@ public abstract class CompileConfiguration implements Runnable { final boolean previousRefreshDeps = extension.refreshDeps(); - if (getAndLock()) { - getProject().getLogger().lifecycle("Found existing cache lock file, rebuilding loom cache. This may have been caused by a failed or canceled build."); + final LockResult lockResult = acquireProcessLockWaiting(getLockFile()); + + if (lockResult != LockResult.ACQUIRED_CLEAN) { + getProject().getLogger().lifecycle("Found existing cache lock file ({}), rebuilding loom cache. This may have been caused by a failed or canceled build.", lockResult); extension.setRefreshDeps(true); } @@ -232,31 +238,114 @@ public abstract class CompileConfiguration implements Runnable { .afterEvaluation(); } - private Path getLockFile() { + private LockFile getLockFile() { final LoomGradleExtension extension = LoomGradleExtension.get(getProject()); final Path cacheDirectory = extension.getFiles().getUserCache().toPath(); final String pathHash = Checksum.projectHash(getProject()); - return cacheDirectory.resolve("." + pathHash + ".lock"); + return new LockFile( + cacheDirectory.resolve("." + pathHash + ".lock"), + "Lock for cache='%s', project='%s'".formatted( + cacheDirectory, getProject().absoluteProjectPath(getProject().getPath()) + ) + ); } - private boolean getAndLock() { - final Path lock = getLockFile(); - - if (Files.exists(lock)) { - return true; + record LockFile(Path file, String description) { + @Override + public String toString() { + return this.description; } + } + enum LockResult { + // acquired immediately or after waiting for another process to release + ACQUIRED_CLEAN, + // already owned by current pid + ACQUIRED_ALREADY_OWNED, + // acquired due to current owner not existing + ACQUIRED_PREVIOUS_OWNER_MISSING + } + + private LockResult acquireProcessLockWaiting(LockFile lockFile) { + // one hour + return this.acquireProcessLockWaiting(lockFile, Duration.ofHours(1)); + } + + private LockResult acquireProcessLockWaiting(LockFile lockFile, Duration timeout) { try { - Files.createFile(lock); - } catch (IOException e) { - throw new UncheckedIOException("Failed to acquire getProject() configuration lock", e); + return this.acquireProcessLockWaiting_(lockFile, timeout); + } catch (final IOException e) { + throw new RuntimeException("Exception acquiring lock " + lockFile, e); + } + } + + // Returns true if our process already owns the lock + @SuppressWarnings("BusyWait") + private LockResult acquireProcessLockWaiting_(LockFile lockFile, Duration timeout) throws IOException { + final long timeoutMs = timeout.toMillis(); + final Logger logger = Logging.getLogger("loom_acquireProcessLockWaiting"); + final long currentPid = ProcessHandle.current().pid(); + boolean abrupt = false; + + if (Files.exists(lockFile.file)) { + long lockingProcessId; + + try { + lockingProcessId = Long.parseLong(Files.readString(lockFile.file)); + } catch (final Exception e) { + lockingProcessId = -1; + } + + if (lockingProcessId == currentPid) { + return LockResult.ACQUIRED_ALREADY_OWNED; + } + + logger.lifecycle("\"{}\" is currently held by pid '{}'.", lockFile, lockingProcessId); + + if (ProcessHandle.of(lockingProcessId).isEmpty()) { + logger.lifecycle("Locking process does not exist, assuming abrupt termination and deleting lock file."); + Files.deleteIfExists(lockFile.file); + abrupt = true; + } else { + logger.lifecycle("Waiting for lock to be released..."); + long sleptMs = 0; + + while (Files.exists(lockFile.file)) { + try { + Thread.sleep(100); + } catch (final InterruptedException e) { + Thread.currentThread().interrupt(); + } + + sleptMs += 100; + + if (sleptMs >= 1000 * 60 && sleptMs % (1000 * 60) == 0L) { + logger.lifecycle( + """ + Have been waiting on "{}" held by pid '{}' for {} minute(s). + If this persists for an unreasonable length of time, kill this process, run './gradlew --stop' and then try again.""", + lockFile, lockingProcessId, sleptMs / 1000 / 60 + ); + } + + if (sleptMs >= timeoutMs) { + throw new GradleException("Have been waiting on lock file '%s' for %s ms. Giving up as timeout is %s ms." + .formatted(lockFile, sleptMs, timeoutMs)); + } + } + } } - return false; + if (!Files.exists(lockFile.file.getParent())) { + Files.createDirectories(lockFile.file.getParent()); + } + + Files.writeString(lockFile.file, String.valueOf(currentPid)); + return abrupt ? LockResult.ACQUIRED_PREVIOUS_OWNER_MISSING : LockResult.ACQUIRED_CLEAN; } private void releaseLock() { - final Path lock = getLockFile(); + final Path lock = getLockFile().file; if (!Files.exists(lock)) { return; From 910963a81c6ecfaf98fa97ed036d7ef21907e81d Mon Sep 17 00:00:00 2001 From: Jason Penilla <11360596+jpenilla@users.noreply.github.com> Date: Thu, 8 Feb 2024 15:20:10 -0700 Subject: [PATCH 06/13] Download intermediary using Gradle (#1042) * Download intermediary using Gradle * address review * Remove ApiWrapper * Use detached configuration --- .../IntermediaryMappingsProvider.java | 56 ++++++++++++++++--- .../IntermediateMappingsProviderInternal.java | 44 +++++++++++++++ .../mappings/IntermediateMappingsService.java | 10 +++- .../extension/LoomGradleExtensionApiImpl.java | 3 +- .../LayeredMappingsSpecification.groovy | 2 +- 5 files changed, 101 insertions(+), 14 deletions(-) create mode 100644 src/main/java/net/fabricmc/loom/configuration/providers/mappings/IntermediateMappingsProviderInternal.java diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/IntermediaryMappingsProvider.java b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/IntermediaryMappingsProvider.java index b56d93e8..bf56a959 100644 --- a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/IntermediaryMappingsProvider.java +++ b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/IntermediaryMappingsProvider.java @@ -27,25 +27,42 @@ package net.fabricmc.loom.configuration.providers.mappings; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.StandardCopyOption; + +import javax.inject.Inject; import com.google.common.net.UrlEscapers; +import org.gradle.api.Action; +import org.gradle.api.Project; +import org.gradle.api.artifacts.Configuration; +import org.gradle.api.artifacts.DependencyArtifact; +import org.gradle.api.artifacts.ModuleDependency; +import org.gradle.api.artifacts.dsl.DependencyFactory; import org.gradle.api.provider.Property; +import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import net.fabricmc.loom.api.mappings.intermediate.IntermediateMappingsProvider; +import net.fabricmc.loom.extension.LoomGradleExtensionApiImpl; -public abstract class IntermediaryMappingsProvider extends IntermediateMappingsProvider { +@ApiStatus.Internal +public abstract class IntermediaryMappingsProvider extends IntermediateMappingsProviderInternal { public static final String NAME = "intermediary-v2"; + private static final String FABRIC_INTERMEDIARY_GROUP_NAME = "net.fabricmc:intermediary"; private static final Logger LOGGER = LoggerFactory.getLogger(IntermediateMappingsProvider.class); public abstract Property getIntermediaryUrl(); public abstract Property getRefreshDeps(); + @Inject + public abstract DependencyFactory getDependencyFactory(); + @Override - public void provide(Path tinyMappings) throws IOException { + public void provide(Path tinyMappings, @Nullable Project project) throws IOException { if (Files.exists(tinyMappings) && !getRefreshDeps().get()) { return; } @@ -53,16 +70,37 @@ public abstract class IntermediaryMappingsProvider extends IntermediateMappingsP // Download and extract intermediary final Path intermediaryJarPath = Files.createTempFile(getName(), ".jar"); final String encodedMcVersion = UrlEscapers.urlFragmentEscaper().escape(getMinecraftVersion().get()); - final String url = getIntermediaryUrl().get().formatted(encodedMcVersion); + final String urlRaw = getIntermediaryUrl().get(); - LOGGER.info("Downloading intermediary from {}", url); + if (project != null && urlRaw.equals(LoomGradleExtensionApiImpl.DEFAULT_INTERMEDIARY_URL)) { + final ModuleDependency intermediaryDep = getDependencyFactory() + .create(FABRIC_INTERMEDIARY_GROUP_NAME + ':' + encodedMcVersion); + intermediaryDep.artifact(new Action() { + @Override + public void execute(final DependencyArtifact dependencyArtifact) { + dependencyArtifact.setClassifier("v2"); + } + }); + final Configuration config = project.getConfigurations().detachedConfiguration(intermediaryDep); - Files.deleteIfExists(tinyMappings); - Files.deleteIfExists(intermediaryJarPath); + Files.copy( + config.getSingleFile().toPath(), + intermediaryJarPath, + StandardCopyOption.REPLACE_EXISTING + ); + Files.deleteIfExists(tinyMappings); + } else { + final String url = urlRaw.formatted(encodedMcVersion); - getDownloader().get().apply(url) - .defaultCache() - .downloadPath(intermediaryJarPath); + LOGGER.info("Downloading intermediary from {}", url); + + Files.deleteIfExists(tinyMappings); + Files.deleteIfExists(intermediaryJarPath); + + getDownloader().get().apply(url) + .defaultCache() + .downloadPath(intermediaryJarPath); + } MappingConfiguration.extractMappings(intermediaryJarPath, tinyMappings); } diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/IntermediateMappingsProviderInternal.java b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/IntermediateMappingsProviderInternal.java new file mode 100644 index 00000000..8a52448a --- /dev/null +++ b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/IntermediateMappingsProviderInternal.java @@ -0,0 +1,44 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2024 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.providers.mappings; + +import java.io.IOException; +import java.nio.file.Path; + +import org.gradle.api.Project; +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.Nullable; + +import net.fabricmc.loom.api.mappings.intermediate.IntermediateMappingsProvider; + +@ApiStatus.Internal +public abstract class IntermediateMappingsProviderInternal extends IntermediateMappingsProvider { + public abstract void provide(Path tinyMappings, @Nullable Project project) throws IOException; + + @Override + public void provide(Path tinyMappings) throws IOException { + this.provide(tinyMappings, null); + } +} diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/IntermediateMappingsService.java b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/IntermediateMappingsService.java index 80171fa1..75af308c 100644 --- a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/IntermediateMappingsService.java +++ b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/IntermediateMappingsService.java @@ -61,15 +61,19 @@ public final class IntermediateMappingsService implements SharedService { final IntermediateMappingsProvider intermediateProvider = extension.getIntermediateMappingsProvider(); final String id = "IntermediateMappingsService:%s:%s".formatted(intermediateProvider.getName(), intermediateProvider.getMinecraftVersion().get()); - return sharedServiceManager.getOrCreateService(id, () -> create(intermediateProvider, minecraftProvider)); + return sharedServiceManager.getOrCreateService(id, () -> create(intermediateProvider, minecraftProvider, project)); } @VisibleForTesting - public static IntermediateMappingsService create(IntermediateMappingsProvider intermediateMappingsProvider, MinecraftProvider minecraftProvider) { + public static IntermediateMappingsService create(IntermediateMappingsProvider intermediateMappingsProvider, MinecraftProvider minecraftProvider, Project project) { final Path intermediaryTiny = minecraftProvider.file(intermediateMappingsProvider.getName() + ".tiny").toPath(); try { - intermediateMappingsProvider.provide(intermediaryTiny); + if (intermediateMappingsProvider instanceof IntermediateMappingsProviderInternal internal) { + internal.provide(intermediaryTiny, project); + } else { + intermediateMappingsProvider.provide(intermediaryTiny); + } } catch (IOException e) { try { Files.deleteIfExists(intermediaryTiny); diff --git a/src/main/java/net/fabricmc/loom/extension/LoomGradleExtensionApiImpl.java b/src/main/java/net/fabricmc/loom/extension/LoomGradleExtensionApiImpl.java index 56d28ee6..30c63cc9 100644 --- a/src/main/java/net/fabricmc/loom/extension/LoomGradleExtensionApiImpl.java +++ b/src/main/java/net/fabricmc/loom/extension/LoomGradleExtensionApiImpl.java @@ -102,6 +102,7 @@ public abstract class LoomGradleExtensionApiImpl implements LoomGradleExtensionA // A common mistake with layered mappings is to call the wrong `officialMojangMappings` method, use this to keep track of when we are building a layered mapping spec. protected final ThreadLocal layeredSpecBuilderScope = ThreadLocal.withInitial(() -> false); + public static final String DEFAULT_INTERMEDIARY_URL = "https://maven.fabricmc.net/net/fabricmc/intermediary/%1$s/intermediary-%1$s-v2.jar"; protected boolean hasEvaluatedLayeredMappings = false; protected final Map layeredMappingsDependencyMap = new HashMap<>(); @@ -125,7 +126,7 @@ public abstract class LoomGradleExtensionApiImpl implements LoomGradleExtensionA .convention(true); this.modProvidedJavadoc.finalizeValueOnRead(); this.intermediary = project.getObjects().property(String.class) - .convention("https://maven.fabricmc.net/net/fabricmc/intermediary/%1$s/intermediary-%1$s-v2.jar"); + .convention(DEFAULT_INTERMEDIARY_URL); this.intermediateMappingsProvider = project.getObjects().property(IntermediateMappingsProvider.class); this.intermediateMappingsProvider.finalizeValueOnRead(); diff --git a/src/test/groovy/net/fabricmc/loom/test/unit/layeredmappings/LayeredMappingsSpecification.groovy b/src/test/groovy/net/fabricmc/loom/test/unit/layeredmappings/LayeredMappingsSpecification.groovy index 629167e9..ff2dd138 100644 --- a/src/test/groovy/net/fabricmc/loom/test/unit/layeredmappings/LayeredMappingsSpecification.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/unit/layeredmappings/LayeredMappingsSpecification.groovy @@ -141,7 +141,7 @@ abstract class LayeredMappingsSpecification extends Specification implements Lay @Override Supplier intermediaryTree() { return { - IntermediateMappingsService.create(LoomMocks.intermediaryMappingsProviderMock("test", intermediaryUrl), minecraftProvider()).memoryMappingTree + IntermediateMappingsService.create(LoomMocks.intermediaryMappingsProviderMock("test", intermediaryUrl), minecraftProvider(), null).memoryMappingTree } } From f0df5a5a715bd2fab91c483bc096e4e283cf5285 Mon Sep 17 00:00:00 2001 From: modmuss Date: Mon, 19 Feb 2024 15:06:33 +0000 Subject: [PATCH 07/13] Add interface injection unit test (#1052) * Add interface injection unit test * Cleanup and fixes --- .../InterfaceInjectionProcessorTest.groovy | 169 ++++++++++++++++++ .../processor/classes/SimpleInterface.java | 31 ++++ .../processor/classes/SimpleTargetClass.java | 30 ++++ 3 files changed, 230 insertions(+) create mode 100644 src/test/groovy/net/fabricmc/loom/test/unit/processor/InterfaceInjectionProcessorTest.groovy create mode 100644 src/test/java/net/fabricmc/loom/test/unit/processor/classes/SimpleInterface.java create mode 100644 src/test/java/net/fabricmc/loom/test/unit/processor/classes/SimpleTargetClass.java diff --git a/src/test/groovy/net/fabricmc/loom/test/unit/processor/InterfaceInjectionProcessorTest.groovy b/src/test/groovy/net/fabricmc/loom/test/unit/processor/InterfaceInjectionProcessorTest.groovy new file mode 100644 index 00000000..bad322cc --- /dev/null +++ b/src/test/groovy/net/fabricmc/loom/test/unit/processor/InterfaceInjectionProcessorTest.groovy @@ -0,0 +1,169 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2024 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.test.unit.processor + +import java.nio.file.Path +import java.util.function.Consumer + +import com.google.gson.JsonArray +import com.google.gson.JsonObject +import spock.lang.Specification +import spock.lang.TempDir + +import net.fabricmc.loom.api.processor.ProcessorContext +import net.fabricmc.loom.api.processor.SpecContext +import net.fabricmc.loom.configuration.ifaceinject.InterfaceInjectionProcessor +import net.fabricmc.loom.test.unit.processor.classes.SimpleInterface +import net.fabricmc.loom.test.unit.processor.classes.SimpleTargetClass +import net.fabricmc.loom.util.Constants +import net.fabricmc.loom.util.Pair +import net.fabricmc.loom.util.ZipUtils +import net.fabricmc.loom.util.fmj.FabricModJson +import net.fabricmc.mappingio.MappingReader +import net.fabricmc.mappingio.tree.MemoryMappingTree + +class InterfaceInjectionProcessorTest extends Specification { + @TempDir + Path tempDir + + def "interface injection"() { + given: + def fmj = Mock(FabricModJson.Mockable) + fmj.getId() >> "modid" + fmj.getCustom(Constants.CustomModJsonKeys.INJECTED_INTERFACE) >> createCustomObject(key, value) + + def specContext = Mock(SpecContext) + specContext.localMods() >> [fmj] + specContext.modDependenciesCompileRuntime() >> [] + + def processorContext = Mock(ProcessorContext) + processorContext.getMappings() >> createMappings() + + def jar = tempDir.resolve("test.jar") + packageJar(jar) + + when: + def processor = new TestInterfaceInjectionProcessor() + def spec = processor.buildSpec(specContext) + processor.processJar(jar, spec, processorContext) + + then: + spec != null + + withTargetClass(jar, target, validator) + + where: + key | value | target | validator + // Simple class with a simple interface + "class_1" | "net/fabricmc/loom/test/unit/processor/classes/SimpleInterface" | SimpleTargetClass.class | { Class loadedClass -> + loadedClass.interfaces.first().name == "net/fabricmc/loom/test/unit/processor/classes/SimpleInterface" + loadedClass.constructors.first().newInstance().injectedMethod() == 123 + } + + // Inner class with a simple interface + "class_1\$class_2" | "net/fabricmc/loom/test/unit/processor/classes/SimpleInterface" | SimpleTargetClass.Inner.class | { Class loadedClass -> + loadedClass.interfaces.first().name == "net/fabricmc/loom/test/unit/processor/classes/SimpleInterface" + loadedClass.constructors.first().newInstance().injectedMethod() == 123 + } + } + + def "nothing to inject"() { + given: + def specContext = Mock(SpecContext) + specContext.localMods() >> [] + specContext.modDependenciesCompileRuntime() >> [] + + when: + def processor = new TestInterfaceInjectionProcessor() + def spec = processor.buildSpec(specContext) + + then: + spec == null + } + + // Create the custom FMJ entry for the injected interface + static JsonObject createCustomObject(String key, String value) { + def jsonObject = new JsonObject() + def jsonArray = new JsonArray() + jsonArray.add(value) + jsonObject.add(key, jsonArray) + return jsonObject + } + + // Package the test classes into a jar file + static void packageJar(Path path) { + def entries = CLASSES_TO_PACKAGE.collect { + def entryName = it.name.replace('.', '/') + ".class" + new Pair(entryName, getClassBytes(it)) + } + + ZipUtils.add(path, entries) + } + + static MemoryMappingTree createMappings() { + def mappings = new MemoryMappingTree() + new StringReader(MAPPINGS).withCloseable { + MappingReader.read(it, mappings) + } + return mappings + } + + // Load a class from a jar file and execute a closure with it + static void withTargetClass(Path jar, Class clazz, Consumer> closure) { + // Groovy is needed as the test classes are compiled with it + URL[] urls = [ + jar.toUri().toURL(), + GroovyObject.class.protectionDomain.codeSource.location + ] + + new URLClassLoader("InterfaceInjectionTest", urls, null).withCloseable { + def loadedClass = Class.forName(clazz.name, true, it) + closure(loadedClass) + } + } + + static byte[] getClassBytes(Class clazz) { + return clazz.classLoader.getResourceAsStream(clazz.name.replace('.', '/') + ".class").withCloseable { + it.bytes + } + } + + private class TestInterfaceInjectionProcessor extends InterfaceInjectionProcessor { + TestInterfaceInjectionProcessor() { + super("InterfaceInjection", true) + } + } + + private static final List> CLASSES_TO_PACKAGE = [ + SimpleTargetClass.class, + SimpleTargetClass.Inner.class, + SimpleInterface.class + ] + private static final String MAPPINGS = """ +tiny\t2\t0\tintermediary\tnamed +c\tclass_1\tnet/fabricmc/loom/test/unit/processor/classes/SimpleTargetClass +c\tclass_1\$class_2\tnet/fabricmc/loom/test/unit/processor/classes/SimpleTargetClass\$Inner +""".trim() +} \ No newline at end of file diff --git a/src/test/java/net/fabricmc/loom/test/unit/processor/classes/SimpleInterface.java b/src/test/java/net/fabricmc/loom/test/unit/processor/classes/SimpleInterface.java new file mode 100644 index 00000000..d14ee167 --- /dev/null +++ b/src/test/java/net/fabricmc/loom/test/unit/processor/classes/SimpleInterface.java @@ -0,0 +1,31 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2024 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.test.unit.processor.classes; + +public interface SimpleInterface { + default int injectedMethod() { + return 123; + } +} diff --git a/src/test/java/net/fabricmc/loom/test/unit/processor/classes/SimpleTargetClass.java b/src/test/java/net/fabricmc/loom/test/unit/processor/classes/SimpleTargetClass.java new file mode 100644 index 00000000..a5105e49 --- /dev/null +++ b/src/test/java/net/fabricmc/loom/test/unit/processor/classes/SimpleTargetClass.java @@ -0,0 +1,30 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2024 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.test.unit.processor.classes; + +public class SimpleTargetClass { + public static class Inner { + } +} From 9ea10ff7597188c7bcb59cedbc9b85e5e1a25b84 Mon Sep 17 00:00:00 2001 From: modmuss Date: Mon, 19 Feb 2024 18:35:43 +0000 Subject: [PATCH 08/13] Add Offline mode / locking test (#1046) * Add Offline mode / locking test * Fix * Make offline tests more resilient --- .../test/integration/OfflineModeTest.groovy | 73 +++++++++++++++++++ 1 file changed, 73 insertions(+) create mode 100644 src/test/groovy/net/fabricmc/loom/test/integration/OfflineModeTest.groovy diff --git a/src/test/groovy/net/fabricmc/loom/test/integration/OfflineModeTest.groovy b/src/test/groovy/net/fabricmc/loom/test/integration/OfflineModeTest.groovy new file mode 100644 index 00000000..2d5efa17 --- /dev/null +++ b/src/test/groovy/net/fabricmc/loom/test/integration/OfflineModeTest.groovy @@ -0,0 +1,73 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2024 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.test.integration + +import spock.lang.Specification +import spock.lang.Unroll + +import net.fabricmc.loom.test.util.GradleProjectTestTrait + +import static net.fabricmc.loom.test.LoomTestConstants.PRE_RELEASE_GRADLE +import static org.gradle.testkit.runner.TaskOutcome.SUCCESS + +class OfflineModeTest extends Specification implements GradleProjectTestTrait { + // Test offline mode when running on a previously locked project + @Unroll + def "Offline refresh deps"() { + setup: + def gradle = gradleProject(project: "minimalBase", version: PRE_RELEASE_GRADLE) + gradle.buildGradle << """ + dependencies { + minecraft 'com.mojang:minecraft:1.20.4' + mappings 'net.fabricmc:yarn:1.20.4+build.3:v2' + modImplementation 'net.fabricmc:fabric-loader:0.15.6' + modImplementation 'net.fabricmc.fabric-api:fabric-api:0.95.4+1.20.4' + } + + import net.fabricmc.loom.util.Checksum + def projectHash = Checksum.projectHash(getProject()) + println("%%" + projectHash + "%%") + + """.stripIndent() + when: + // Normal online run to populate the caches + def result1 = gradle.run(task: "build") + + def projectHash = result1.output.split("%%")[1] + + // Create a dummy lock file to ensure that the loom cache is rebuilt on the next run + def lockFile = new File(gradle.gradleHomeDir, "caches/fabric-loom/.${projectHash}.lock") + lockFile.text = "12345" + + // Run with --offline to ensure that nothing is downloaded. + def result2 = gradle.run(tasks: ["clean", "build"], args: ["--offline"]) + then: + result1.task(":build").outcome == SUCCESS + result2.task(":build").outcome == SUCCESS + + result2.output.contains("is currently held by pid '12345'") + result2.output.contains("rebuilding loom cache") + } +} From 1412f65e10be16b201663ded6eef5fa5cbf7a874 Mon Sep 17 00:00:00 2001 From: Jason Penilla <11360596+jpenilla@users.noreply.github.com> Date: Mon, 19 Feb 2024 11:35:58 -0700 Subject: [PATCH 09/13] Print info about process lock owner (#1040) * Print info about process lock owner * format * Fix extraneous space with empty args * Avoid leaking sensitive info in program arguments by default * fix indent * switch var usage around --- .../configuration/CompileConfiguration.java | 72 ++++++++++++++++++- 1 file changed, 71 insertions(+), 1 deletion(-) diff --git a/src/main/java/net/fabricmc/loom/configuration/CompileConfiguration.java b/src/main/java/net/fabricmc/loom/configuration/CompileConfiguration.java index 2d77629d..db6e8847 100644 --- a/src/main/java/net/fabricmc/loom/configuration/CompileConfiguration.java +++ b/src/main/java/net/fabricmc/loom/configuration/CompileConfiguration.java @@ -32,12 +32,16 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.time.Duration; +import java.util.ArrayList; +import java.util.List; +import java.util.Optional; import java.util.function.Consumer; import javax.inject.Inject; import org.gradle.api.GradleException; import org.gradle.api.Project; +import org.gradle.api.logging.LogLevel; import org.gradle.api.logging.Logger; import org.gradle.api.logging.Logging; import org.gradle.api.plugins.JavaPlugin; @@ -302,11 +306,14 @@ public abstract class CompileConfiguration implements Runnable { logger.lifecycle("\"{}\" is currently held by pid '{}'.", lockFile, lockingProcessId); - if (ProcessHandle.of(lockingProcessId).isEmpty()) { + Optional handle = ProcessHandle.of(lockingProcessId); + + if (handle.isEmpty()) { logger.lifecycle("Locking process does not exist, assuming abrupt termination and deleting lock file."); Files.deleteIfExists(lockFile.file); abrupt = true; } else { + logger.lifecycle(printWithParents(handle.get())); logger.lifecycle("Waiting for lock to be released..."); long sleptMs = 0; @@ -344,6 +351,69 @@ public abstract class CompileConfiguration implements Runnable { return abrupt ? LockResult.ACQUIRED_PREVIOUS_OWNER_MISSING : LockResult.ACQUIRED_CLEAN; } + private String printWithParents(ProcessHandle processHandle) { + var output = new StringBuilder(); + + List chain = getParentChain(null, processHandle); + + for (int i = 0; i < chain.size(); i++) { + ProcessHandle handle = chain.get(i); + + output.append("\t".repeat(i)); + + if (i != 0) { + output.append("└─ "); + } + + output.append(getInfoString(handle)); + + if (i < chain.size() - 1) { + output.append('\n'); + } + } + + return output.toString(); + } + + private String getInfoString(ProcessHandle handle) { + return "(%s) pid %s '%s%s'%s".formatted( + handle.info().user().orElse("unknown user"), + handle.pid(), + handle.info().command().orElse("unknown command"), + handle.info().arguments().map(arr -> { + if (getProject().getGradle().getStartParameter().getLogLevel() != LogLevel.INFO + && getProject().getGradle().getStartParameter().getLogLevel() != LogLevel.DEBUG) { + return " (run with --info or --debug to show arguments, may reveal sensitive info)"; + } + + String join = String.join(" ", arr); + + if (join.isBlank()) { + return ""; + } + + return " " + join; + }).orElse(" (unknown arguments)"), + handle.info().startInstant().map(instant -> " started at " + instant).orElse("") + ); + } + + private List getParentChain(List collectTo, ProcessHandle processHandle) { + if (collectTo == null) { + collectTo = new ArrayList<>(); + } + + Optional parent = processHandle.parent(); + + if (parent.isPresent()) { + getParentChain(collectTo, parent.get()); + } + + collectTo.add(processHandle); + + return collectTo; + } + private void releaseLock() { final Path lock = getLockFile().file; From 79041416778e59f7e0573ad3341caf7f86757520 Mon Sep 17 00:00:00 2001 From: modmuss50 Date: Mon, 19 Feb 2024 22:02:24 +0000 Subject: [PATCH 10/13] Prevent Gradle from running vscode task asynchronously Closes #1048 --- .../net/fabricmc/loom/task/GenVsCodeProjectTask.java | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/main/java/net/fabricmc/loom/task/GenVsCodeProjectTask.java b/src/main/java/net/fabricmc/loom/task/GenVsCodeProjectTask.java index b3550e1d..5524dc05 100644 --- a/src/main/java/net/fabricmc/loom/task/GenVsCodeProjectTask.java +++ b/src/main/java/net/fabricmc/loom/task/GenVsCodeProjectTask.java @@ -37,15 +37,22 @@ import java.util.Map; import com.google.gson.JsonArray; import com.google.gson.JsonElement; import com.google.gson.JsonObject; +import org.gradle.api.provider.Property; +import org.gradle.api.services.ServiceReference; import org.gradle.api.tasks.TaskAction; import net.fabricmc.loom.LoomGradlePlugin; import net.fabricmc.loom.configuration.ide.RunConfig; import net.fabricmc.loom.configuration.ide.RunConfigSettings; +import net.fabricmc.loom.util.gradle.SyncTaskBuildService; // Recommended vscode plugin pack: // https://marketplace.visualstudio.com/items?itemName=vscjava.vscode-java-pack -public class GenVsCodeProjectTask extends AbstractLoomTask { +public abstract class GenVsCodeProjectTask extends AbstractLoomTask { + // Prevent Gradle from running vscode task asynchronously + @ServiceReference(SyncTaskBuildService.NAME) + abstract Property getSyncTask(); + @TaskAction public void genRuns() throws IOException { final Path projectDir = getProject().getRootDir().toPath().resolve(".vscode"); From cfba0b18cdfdb0046a1573531d88081c736bf702 Mon Sep 17 00:00:00 2001 From: modmuss50 Date: Mon, 19 Feb 2024 22:10:38 +0000 Subject: [PATCH 11/13] Update wrapper validation task --- .github/workflows/publish.yml | 2 +- .github/workflows/test-push.yml | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index df447afb..1eb016a1 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -12,7 +12,7 @@ jobs: options: --user root steps: - uses: actions/checkout@v4 - - uses: gradle/wrapper-validation-action@v1 + - uses: gradle/wrapper-validation-action@v2 # Generate the build number based on tags to allow per branch build numbers, not something github provides by default. - name: Generate build number diff --git a/.github/workflows/test-push.yml b/.github/workflows/test-push.yml index 1cc59364..5de37f2a 100644 --- a/.github/workflows/test-push.yml +++ b/.github/workflows/test-push.yml @@ -17,7 +17,7 @@ jobs: options: --user root steps: - uses: actions/checkout@v4 - - uses: gradle/wrapper-validation-action@v1 + - uses: gradle/wrapper-validation-action@v2 - run: gradle build check -x test --stacktrace --warning-mode fail build_windows: @@ -29,7 +29,7 @@ jobs: with: java-version: 17 distribution: 'temurin' - - uses: gradle/wrapper-validation-action@v1 + - uses: gradle/wrapper-validation-action@v2 - run: ./gradlew build check -x test --stacktrace --warning-mode fail # This job is used to feed the test matrix of next job to allow the tests to run in parallel From 0dc1ba012ac739f45409b92e5cf218a975d23538 Mon Sep 17 00:00:00 2001 From: modmuss Date: Sat, 24 Feb 2024 15:04:56 +0000 Subject: [PATCH 12/13] Update deps (#1054) --- .github/workflows/test-push.yml | 14 +++---- build.gradle | 7 ++++ gradle/libs.versions.toml | 8 ++-- gradle/runtime.libs.versions.toml | 2 +- gradle/test.libs.versions.toml | 14 +++---- ...ClassMetadataRemappingAnnotationVisitor.kt | 42 ++++++++++++++----- .../kotlin/remapping/KotlinClassRemapper.kt | 11 ++--- .../KotlinMetadataRemappingClassVisitor.kt | 5 ++- ...KotlinMetadataTinyRemapperExtensionImpl.kt | 5 ++- ...sMetadataRemappingAnnotationVisitorTest.kt | 14 ++++--- 10 files changed, 79 insertions(+), 43 deletions(-) diff --git a/.github/workflows/test-push.yml b/.github/workflows/test-push.yml index 5de37f2a..90e010c2 100644 --- a/.github/workflows/test-push.yml +++ b/.github/workflows/test-push.yml @@ -73,12 +73,12 @@ jobs: TEST_WARNING_MODE: fail id: test - - uses: actions/upload-artifact@v3 + - uses: actions/upload-artifact@v4 if: ${{ failure() }} with: name: ${{ steps.test.outputs.test }} Results path: build/reports/ - - uses: actions/upload-artifact@v3 + - uses: actions/upload-artifact@v4 if: ${{ failure() }} with: name: ${{ steps.test.outputs.test }} Heap Dump @@ -107,12 +107,12 @@ jobs: TEST_WARNING_MODE: fail id: test - - uses: actions/upload-artifact@v3 + - uses: actions/upload-artifact@v4 if: ${{ failure() }} with: name: ${{ steps.test.outputs.test }} (${{ matrix.java }}) Results (Windows) path: build/reports/ - - uses: actions/upload-artifact@v3 + - uses: actions/upload-artifact@v4 if: ${{ failure() }} with: name: ${{ steps.test.outputs.test }} Heap Dump (Windows) @@ -125,8 +125,8 @@ jobs: strategy: fail-fast: false matrix: - java: [ 17, 20 ] - os: [ windows-2022, ubuntu-22.04, macos-12 ] + java: [ 17, 21 ] + os: [ windows-2022, ubuntu-22.04, macos-14 ] runs-on: ${{ matrix.os }} steps: @@ -138,7 +138,7 @@ jobs: - run: ./gradlew test --tests *ReproducibleBuildTest --stacktrace --warning-mode fail - - uses: actions/upload-artifact@v3 + - uses: actions/upload-artifact@v4 if: ${{ failure() }} with: name: Reproducible Build ${{ matrix.os }} (${{ matrix.java }}) Results diff --git a/build.gradle b/build.gradle index 464c2217..c15a009d 100644 --- a/build.gradle +++ b/build.gradle @@ -245,6 +245,13 @@ checkstyle { toolVersion = libs.versions.checkstyle.get() } +// Workaround https://github.com/gradle/gradle/issues/27035 +configurations.checkstyle { + resolutionStrategy.capabilitiesResolution.withCapability("com.google.collections:google-collections") { + select("com.google.guava:guava:0") + } +} + codenarc { toolVersion = libs.versions.codenarc.get() configFile = file("codenarc.groovy") diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index b715786e..acb5aa82 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -6,7 +6,7 @@ gson = "2.10.1" guava = "33.0.0-jre" stitch = "0.6.2" -tiny-remapper = "0.10.0" +tiny-remapper = "0.10.1" access-widener = "2.1.0" mapping-io = "0.5.1" lorenz-tiny = "4.0.2" @@ -14,10 +14,10 @@ mercury = "0.4.1" kotlinx-metadata = "0.9.0" # Plugins -spotless = "6.20.0" +spotless = "6.25.0" test-retry = "1.5.6" -checkstyle = "10.12.5" -codenarc = "3.3.0" +checkstyle = "10.13.0" +codenarc = "3.4.0" jacoco = "0.8.11" [libraries] diff --git a/gradle/runtime.libs.versions.toml b/gradle/runtime.libs.versions.toml index 8b6872bf..cf2a01c7 100644 --- a/gradle/runtime.libs.versions.toml +++ b/gradle/runtime.libs.versions.toml @@ -1,7 +1,7 @@ [versions] # Decompilers fernflower = "2.0.0" -cfr = "0.2.1" +cfr = "0.2.2" vineflower = "1.9.3" # Runtime depedencies diff --git a/gradle/test.libs.versions.toml b/gradle/test.libs.versions.toml index 97e556b5..3498517b 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.1" -javalin = "5.6.3" -mockito = "5.8.0" -java-debug = "0.50.0" -mixin = "0.11.4+mixin.0.8.5" +junit = "5.10.2" +javalin = "6.1.0" +mockito = "5.10.0" +java-debug = "0.51.0" +mixin = "0.12.5+mixin.0.8.5" -gradle-nightly = "8.7-20240202001338+0000" -fabric-loader = "0.15.3" +gradle-nightly = "8.8-20240224001421+0000" +fabric-loader = "0.15.6" fabric-installer = "1.0.0" [libraries] 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 8b529604..483d9f6d 100644 --- a/src/main/kotlin/net/fabricmc/loom/kotlin/remapping/KotlinClassMetadataRemappingAnnotationVisitor.kt +++ b/src/main/kotlin/net/fabricmc/loom/kotlin/remapping/KotlinClassMetadataRemappingAnnotationVisitor.kt @@ -32,16 +32,19 @@ import org.objectweb.asm.commons.Remapper import org.objectweb.asm.tree.AnnotationNode import org.slf4j.LoggerFactory -class KotlinClassMetadataRemappingAnnotationVisitor(private val remapper: Remapper, val next: AnnotationVisitor, val className: String?) : +class KotlinClassMetadataRemappingAnnotationVisitor( + private val remapper: Remapper, + val next: AnnotationVisitor, + val className: String?, +) : AnnotationNode(Opcodes.ASM9, KotlinMetadataRemappingClassVisitor.ANNOTATION_DESCRIPTOR) { - private val logger = LoggerFactory.getLogger(javaClass) - private var _name: String? = null - - override fun visit(name: String?, value: Any?) { + override fun visit( + name: String?, + value: Any?, + ) { super.visit(name, value) - this._name = name } override fun visitEnd() { @@ -53,7 +56,11 @@ class KotlinClassMetadataRemappingAnnotationVisitor(private val remapper: Remapp val currentMinorVersion = KotlinVersion(KotlinVersion.CURRENT.major, KotlinVersion.CURRENT.minor, 0) if (headerVersion != currentMinorVersion) { - logger.info("Kotlin metadata for class ($className) as it was built using a different major Kotlin version (${header.metadataVersion[0]}.${header.metadataVersion[1]}.x) while the remapper is using (${KotlinVersion.CURRENT}).") + logger.info( + "Kotlin metadata for class ($className) as it was built using a different major Kotlin " + + "version (${header.metadataVersion[0]}.${header.metadataVersion[1]}.x) while the remapper " + + "is using (${KotlinVersion.CURRENT}).", + ) } when (val metadata = KotlinClassMetadata.readLenient(header)) { @@ -86,7 +93,13 @@ class KotlinClassMetadataRemappingAnnotationVisitor(private val remapper: Remapp is KotlinClassMetadata.MultiFileClassPart -> { var kpackage = metadata.kmPackage kpackage = KotlinClassRemapper(remapper).remap(kpackage) - val remapped = KotlinClassMetadata.MultiFileClassPart(kpackage, metadata.facadeClassName, metadata.version, metadata.flags).write() + val remapped = + KotlinClassMetadata.MultiFileClassPart( + kpackage, + metadata.facadeClassName, + metadata.version, + metadata.flags, + ).write() writeClassHeader(remapped) validateKotlinClassHeader(remapped, header) } @@ -147,10 +160,17 @@ class KotlinClassMetadataRemappingAnnotationVisitor(private val remapper: Remapp newNode.accept(next) } - private fun validateKotlinClassHeader(remapped: Metadata, original: Metadata) { - // This can happen when the remapper is ran on a kotlin version that does not match the version the class was compiled with. + private fun validateKotlinClassHeader( + remapped: Metadata, + original: Metadata, + ) { + // This can happen when the remapper is ran on a kotlin version + // that does not match the version the class was compiled with. if (remapped.data2.size != original.data2.size) { - logger.info("Kotlin class metadata size mismatch: data2 size does not match original in class $className. New: ${remapped.data2.size} Old: ${original.data2.size}") + logger.info( + "Kotlin class metadata size mismatch: data2 size does not match original in class $className. " + + "New: ${remapped.data2.size} Old: ${original.data2.size}", + ) } } } diff --git a/src/main/kotlin/net/fabricmc/loom/kotlin/remapping/KotlinClassRemapper.kt b/src/main/kotlin/net/fabricmc/loom/kotlin/remapping/KotlinClassRemapper.kt index e3d00bb6..ffb59ccc 100644 --- a/src/main/kotlin/net/fabricmc/loom/kotlin/remapping/KotlinClassRemapper.kt +++ b/src/main/kotlin/net/fabricmc/loom/kotlin/remapping/KotlinClassRemapper.kt @@ -97,11 +97,12 @@ class KotlinClassRemapper(private val remapper: Remapper) { } private fun remap(type: KmType): KmType { - type.classifier = when (val classifier = type.classifier) { - is KmClassifier.Class -> KmClassifier.Class(remap(classifier.name)) - is KmClassifier.TypeParameter -> KmClassifier.TypeParameter(classifier.id) - is KmClassifier.TypeAlias -> KmClassifier.TypeAlias(remap(classifier.name)) - } + type.classifier = + when (val classifier = type.classifier) { + is KmClassifier.Class -> KmClassifier.Class(remap(classifier.name)) + is KmClassifier.TypeParameter -> KmClassifier.TypeParameter(classifier.id) + is KmClassifier.TypeAlias -> KmClassifier.TypeAlias(remap(classifier.name)) + } type.arguments.replaceAll(this::remap) type.abbreviatedType = type.abbreviatedType?.let { remap(it) } type.outerType = type.outerType?.let { remap(it) } diff --git a/src/main/kotlin/net/fabricmc/loom/kotlin/remapping/KotlinMetadataRemappingClassVisitor.kt b/src/main/kotlin/net/fabricmc/loom/kotlin/remapping/KotlinMetadataRemappingClassVisitor.kt index 744f2374..074fe2c0 100644 --- a/src/main/kotlin/net/fabricmc/loom/kotlin/remapping/KotlinMetadataRemappingClassVisitor.kt +++ b/src/main/kotlin/net/fabricmc/loom/kotlin/remapping/KotlinMetadataRemappingClassVisitor.kt @@ -50,7 +50,10 @@ class KotlinMetadataRemappingClassVisitor(private val remapper: Remapper, next: super.visit(version, access, name, signature, superName, interfaces) } - override fun visitAnnotation(descriptor: String, visible: Boolean): AnnotationVisitor? { + override fun visitAnnotation( + descriptor: String, + visible: Boolean, + ): AnnotationVisitor? { var result: AnnotationVisitor? = super.visitAnnotation(descriptor, visible) if (descriptor == ANNOTATION_DESCRIPTOR && result != null) { diff --git a/src/main/kotlin/net/fabricmc/loom/kotlin/remapping/KotlinMetadataTinyRemapperExtensionImpl.kt b/src/main/kotlin/net/fabricmc/loom/kotlin/remapping/KotlinMetadataTinyRemapperExtensionImpl.kt index 5ba16756..5f087eae 100644 --- a/src/main/kotlin/net/fabricmc/loom/kotlin/remapping/KotlinMetadataTinyRemapperExtensionImpl.kt +++ b/src/main/kotlin/net/fabricmc/loom/kotlin/remapping/KotlinMetadataTinyRemapperExtensionImpl.kt @@ -30,7 +30,10 @@ import net.fabricmc.tinyremapper.api.TrClass import org.objectweb.asm.ClassVisitor object KotlinMetadataTinyRemapperExtensionImpl : KotlinMetadataTinyRemapperExtension { - override fun insertApplyVisitor(cls: TrClass, next: ClassVisitor?): ClassVisitor { + override fun insertApplyVisitor( + cls: TrClass, + next: ClassVisitor?, + ): ClassVisitor { return KotlinMetadataRemappingClassVisitor(cls.environment.remapper, next) } diff --git a/src/test/kotlin/net/fabricmc/loom/test/kotlin/KotlinClassMetadataRemappingAnnotationVisitorTest.kt b/src/test/kotlin/net/fabricmc/loom/test/kotlin/KotlinClassMetadataRemappingAnnotationVisitorTest.kt index 7c862b66..28e67d55 100644 --- a/src/test/kotlin/net/fabricmc/loom/test/kotlin/KotlinClassMetadataRemappingAnnotationVisitorTest.kt +++ b/src/test/kotlin/net/fabricmc/loom/test/kotlin/KotlinClassMetadataRemappingAnnotationVisitorTest.kt @@ -55,9 +55,10 @@ class KotlinClassMetadataRemappingAnnotationVisitorTest { val classReader = ClassReader(inputPosInChunk) - val tinyRemapper = TinyRemapper.newRemapper() - .withMappings(readMappings("PosInChunk")) - .build() + val tinyRemapper = + TinyRemapper.newRemapper() + .withMappings(readMappings("PosInChunk")) + .build() val inputWriter = StringWriter() classReader.accept(stringWriterVisitor(inputWriter), 0) @@ -77,9 +78,10 @@ class KotlinClassMetadataRemappingAnnotationVisitorTest { val input = getClassBytes("TestExtensionKt") val classReader = ClassReader(input) - val tinyRemapper = TinyRemapper.newRemapper() - .withMappings(readMappings("TestExtensionKt")) - .build() + val tinyRemapper = + TinyRemapper.newRemapper() + .withMappings(readMappings("TestExtensionKt")) + .build() val inputWriter = StringWriter() classReader.accept(stringWriterVisitor(inputWriter), 0) From ae1ba0ab8612f8e8855fcb26bbf45d8a30c29e10 Mon Sep 17 00:00:00 2001 From: FirstMegaGame4 <84094287+FirstMegaGame4@users.noreply.github.com> Date: Sun, 25 Feb 2024 16:26:00 +0100 Subject: [PATCH 13/13] Injected Interfaces, Generics Support (#1050) --- .../InterfaceInjectionProcessor.java | 166 ++++++++++++++++-- .../InterfaceInjectionProcessorTest.groovy | 84 ++++++++- .../classes/AdvancedGenericInterface.java | 31 ++++ .../classes/AdvancedGenericTargetClass.java | 32 ++++ .../classes/DoubleGenericTargetClass.java | 28 +++ .../classes/FirstGenericInterface.java | 31 ++++ .../processor/classes/GenericInterface.java | 31 ++++ .../processor/classes/GenericTargetClass.java | 28 +++ .../classes/PassingGenericTargetClass.java | 28 +++ .../classes/SecondGenericInterface.java | 31 ++++ .../classes/SelfGenericInterface.java | 31 ++++ .../classes/SelfGenericTargetClass.java | 28 +++ .../src/main/java/ExampleMod.java | 3 + .../main/java/GenericInjectedInterface.java | 6 + .../src/main/java/OwnInjectedInterface.java | 6 +- .../src/main/resources/fabric.mod.json | 7 +- 16 files changed, 552 insertions(+), 19 deletions(-) create mode 100644 src/test/java/net/fabricmc/loom/test/unit/processor/classes/AdvancedGenericInterface.java create mode 100644 src/test/java/net/fabricmc/loom/test/unit/processor/classes/AdvancedGenericTargetClass.java create mode 100644 src/test/java/net/fabricmc/loom/test/unit/processor/classes/DoubleGenericTargetClass.java create mode 100644 src/test/java/net/fabricmc/loom/test/unit/processor/classes/FirstGenericInterface.java create mode 100644 src/test/java/net/fabricmc/loom/test/unit/processor/classes/GenericInterface.java create mode 100644 src/test/java/net/fabricmc/loom/test/unit/processor/classes/GenericTargetClass.java create mode 100644 src/test/java/net/fabricmc/loom/test/unit/processor/classes/PassingGenericTargetClass.java create mode 100644 src/test/java/net/fabricmc/loom/test/unit/processor/classes/SecondGenericInterface.java create mode 100644 src/test/java/net/fabricmc/loom/test/unit/processor/classes/SelfGenericInterface.java create mode 100644 src/test/java/net/fabricmc/loom/test/unit/processor/classes/SelfGenericTargetClass.java create mode 100644 src/test/resources/projects/interfaceInjection/src/main/java/GenericInjectedInterface.java diff --git a/src/main/java/net/fabricmc/loom/configuration/ifaceinject/InterfaceInjectionProcessor.java b/src/main/java/net/fabricmc/loom/configuration/ifaceinject/InterfaceInjectionProcessor.java index 98a4a798..1ced9ba3 100644 --- a/src/main/java/net/fabricmc/loom/configuration/ifaceinject/InterfaceInjectionProcessor.java +++ b/src/main/java/net/fabricmc/loom/configuration/ifaceinject/InterfaceInjectionProcessor.java @@ -46,6 +46,9 @@ import org.objectweb.asm.ClassReader; import org.objectweb.asm.ClassVisitor; import org.objectweb.asm.ClassWriter; import org.objectweb.asm.Opcodes; +import org.objectweb.asm.signature.SignatureReader; +import org.objectweb.asm.signature.SignatureVisitor; +import org.objectweb.asm.util.CheckSignatureAdapter; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -54,11 +57,14 @@ import net.fabricmc.loom.api.processor.MinecraftJarProcessor; import net.fabricmc.loom.api.processor.ProcessorContext; import net.fabricmc.loom.api.processor.SpecContext; import net.fabricmc.loom.util.Constants; +import net.fabricmc.loom.util.LazyCloseable; import net.fabricmc.loom.util.Pair; import net.fabricmc.loom.util.ZipUtils; import net.fabricmc.loom.util.fmj.FabricModJson; import net.fabricmc.mappingio.tree.MappingTree; import net.fabricmc.mappingio.tree.MemoryMappingTree; +import net.fabricmc.tinyremapper.TinyRemapper; +import net.fabricmc.tinyremapper.api.TrRemapper; public abstract class InterfaceInjectionProcessor implements MinecraftJarProcessor { private static final Logger LOGGER = LoggerFactory.getLogger(InterfaceInjectionProcessor.class); @@ -105,22 +111,36 @@ public abstract class InterfaceInjectionProcessor implements MinecraftJarProcess final MemoryMappingTree mappings = context.getMappings(); final int intermediaryIndex = mappings.getNamespaceId(MappingsNamespace.INTERMEDIARY.toString()); final int namedIndex = mappings.getNamespaceId(MappingsNamespace.NAMED.toString()); - final List remappedInjectedInterfaces = spec.injectedInterfaces().stream() - .map(injectedInterface -> remap(injectedInterface, s -> mappings.mapClassName(s, intermediaryIndex, namedIndex))) - .toList(); - try { - ZipUtils.transform(jar, getTransformers(remappedInjectedInterfaces)); - } catch (IOException e) { - throw new RuntimeException("Failed to apply interface injections to " + jar, e); + try (LazyCloseable tinyRemapper = context.createRemapper(MappingsNamespace.INTERMEDIARY, MappingsNamespace.NAMED)) { + final List remappedInjectedInterfaces = spec.injectedInterfaces().stream() + .map(injectedInterface -> remap( + injectedInterface, + s -> mappings.mapClassName(s, intermediaryIndex, namedIndex), + tinyRemapper.get().getEnvironment().getRemapper() + )) + .toList(); + try { + ZipUtils.transform(jar, getTransformers(remappedInjectedInterfaces)); + } catch (IOException e) { + throw new RuntimeException("Failed to apply interface injections to " + jar, e); + } } } - private InjectedInterface remap(InjectedInterface in, Function remapper) { + private InjectedInterface remap(InjectedInterface in, Function remapper, TrRemapper signatureRemapper) { + String generics = null; + + if (in.generics() != null) { + String fakeSignature = signatureRemapper.mapSignature("Ljava/lang/Object" + in.generics() + ";", false); // Turning the raw generics string into a fake signature + generics = fakeSignature.substring("Ljava/lang/Object".length(), fakeSignature.length() - 1); // Retrieving the remapped raw generics string from the remapped fake signature + } + return new InjectedInterface( in.modId(), remapper.apply(in.className()), - remapper.apply(in.ifaceName()) + remapper.apply(in.ifaceName()), + generics ); } @@ -196,7 +216,7 @@ public abstract class InterfaceInjectionProcessor implements MinecraftJarProcess return comment; } - private record InjectedInterface(String modId, String className, String ifaceName) { + private record InjectedInterface(String modId, String className, String ifaceName, @Nullable String generics) { public static List fromMod(FabricModJson fabricModJson) { final String modId = fabricModJson.getId(); final JsonElement jsonElement = fabricModJson.getCustom(Constants.CustomModJsonKeys.INJECTED_INTERFACE); @@ -210,10 +230,25 @@ public abstract class InterfaceInjectionProcessor implements MinecraftJarProcess final List result = new ArrayList<>(); for (String className : addedIfaces.keySet()) { - final JsonArray ifaceNames = addedIfaces.getAsJsonArray(className); + final JsonArray ifacesInfo = addedIfaces.getAsJsonArray(className); - for (JsonElement ifaceName : ifaceNames) { - result.add(new InjectedInterface(modId, className, ifaceName.getAsString())); + for (JsonElement ifaceElement : ifacesInfo) { + String ifaceInfo = ifaceElement.getAsString(); + + String name = ifaceInfo; + String generics = null; + + if (ifaceInfo.contains("<") && ifaceInfo.contains(">")) { + name = ifaceInfo.substring(0, ifaceInfo.indexOf("<")); + generics = ifaceInfo.substring(ifaceInfo.indexOf("<")); + + // First Generics Check, if there are generics, are them correctly written? + SignatureReader reader = new SignatureReader("Ljava/lang/Object" + generics + ";"); + CheckSignatureAdapter checker = new CheckSignatureAdapter(CheckSignatureAdapter.CLASS_SIGNATURE, null); + reader.accept(checker); + } + + result.add(new InjectedInterface(modId, className, name, generics)); } } @@ -226,6 +261,16 @@ public abstract class InterfaceInjectionProcessor implements MinecraftJarProcess .flatMap(List::stream) .toList(); } + + public static boolean containsGenerics(List injectedInterfaces) { + for (InjectedInterface injectedInterface : injectedInterfaces) { + if (injectedInterface.generics() != null) { + return true; + } + } + + return false; + } } private static class InjectingClassVisitor extends ClassVisitor { @@ -241,6 +286,7 @@ public abstract class InterfaceInjectionProcessor implements MinecraftJarProcess @Override public void visit(int version, int access, String name, String signature, String superName, String[] interfaces) { + String[] baseInterfaces = interfaces.clone(); Set modifiedInterfaces = new LinkedHashSet<>(interfaces.length + injectedInterfaces.size()); Collections.addAll(modifiedInterfaces, interfaces); @@ -249,11 +295,35 @@ public abstract class InterfaceInjectionProcessor implements MinecraftJarProcess } // See JVMS: https://docs.oracle.com/javase/specs/jvms/se17/html/jvms-4.html#jvms-ClassSignature + if (InjectedInterface.containsGenerics(injectedInterfaces) && signature == null) { + // Classes that are not using generics don't need signatures, so their signatures are null + // If the class is not using generics but that an injected interface targeting the class is using them, we are creating the class signature + StringBuilder baseSignatureBuilder = new StringBuilder("L" + superName + ";"); + + for (String baseInterface : baseInterfaces) { + baseSignatureBuilder.append("L").append(baseInterface).append(";"); + } + + signature = baseSignatureBuilder.toString(); + } + if (signature != null) { + SignatureReader reader = new SignatureReader(signature); + + // Second Generics Check, if there are passed generics, are all of them present in the target class? + GenericsChecker checker = new GenericsChecker(Constants.ASM_VERSION, injectedInterfaces); + reader.accept(checker); + var resultingSignature = new StringBuilder(signature); for (InjectedInterface injectedInterface : injectedInterfaces) { - String superinterfaceSignature = "L" + injectedInterface.ifaceName() + ";"; + String superinterfaceSignature; + + if (injectedInterface.generics() != null) { + superinterfaceSignature = "L" + injectedInterface.ifaceName() + injectedInterface.generics() + ";"; + } else { + superinterfaceSignature = "L" + injectedInterface.ifaceName() + ";"; + } if (resultingSignature.indexOf(superinterfaceSignature) == -1) { resultingSignature.append(superinterfaceSignature); @@ -314,4 +384,72 @@ public abstract class InterfaceInjectionProcessor implements MinecraftJarProcess super.visitEnd(); } } + + private static class GenericsChecker extends SignatureVisitor { + private final List typeParameters; + + private final List injectedInterfaces; + + GenericsChecker(int asmVersion, List injectedInterfaces) { + super(asmVersion); + this.typeParameters = new ArrayList<>(); + this.injectedInterfaces = injectedInterfaces; + } + + @Override + public void visitFormalTypeParameter(String name) { + this.typeParameters.add(name); + super.visitFormalTypeParameter(name); + } + + @Override + public void visitEnd() { + for (InjectedInterface injectedInterface : this.injectedInterfaces) { + if (injectedInterface.generics() != null) { + SignatureReader reader = new SignatureReader("Ljava/lang/Object" + injectedInterface.generics() + ";"); + GenericsConfirm confirm = new GenericsConfirm( + Constants.ASM_VERSION, + injectedInterface.className(), + injectedInterface.ifaceName(), + this.typeParameters + ); + reader.accept(confirm); + } + } + + super.visitEnd(); + } + + public static class GenericsConfirm extends SignatureVisitor { + private final String className; + + private final String interfaceName; + + private final List acceptedTypeVariables; + + GenericsConfirm(int asmVersion, String className, String interfaceName, List acceptedTypeVariables) { + super(asmVersion); + this.className = className; + this.interfaceName = interfaceName; + this.acceptedTypeVariables = acceptedTypeVariables; + } + + @Override + public void visitTypeVariable(String name) { + if (!this.acceptedTypeVariables.contains(name)) { + throw new IllegalStateException( + "Interface " + + this.interfaceName + + " attempted to use a type variable named " + + name + + " which is not present in the " + + this.className + + " class" + ); + } + + super.visitTypeVariable(name); + } + } + } } diff --git a/src/test/groovy/net/fabricmc/loom/test/unit/processor/InterfaceInjectionProcessorTest.groovy b/src/test/groovy/net/fabricmc/loom/test/unit/processor/InterfaceInjectionProcessorTest.groovy index bad322cc..cf66e0c8 100644 --- a/src/test/groovy/net/fabricmc/loom/test/unit/processor/InterfaceInjectionProcessorTest.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/unit/processor/InterfaceInjectionProcessorTest.groovy @@ -32,17 +32,31 @@ import com.google.gson.JsonObject import spock.lang.Specification import spock.lang.TempDir +import net.fabricmc.loom.api.mappings.layered.MappingsNamespace import net.fabricmc.loom.api.processor.ProcessorContext import net.fabricmc.loom.api.processor.SpecContext import net.fabricmc.loom.configuration.ifaceinject.InterfaceInjectionProcessor +import net.fabricmc.loom.test.unit.processor.classes.AdvancedGenericInterface +import net.fabricmc.loom.test.unit.processor.classes.AdvancedGenericTargetClass +import net.fabricmc.loom.test.unit.processor.classes.DoubleGenericTargetClass +import net.fabricmc.loom.test.unit.processor.classes.FirstGenericInterface +import net.fabricmc.loom.test.unit.processor.classes.GenericInterface +import net.fabricmc.loom.test.unit.processor.classes.GenericTargetClass +import net.fabricmc.loom.test.unit.processor.classes.PassingGenericTargetClass +import net.fabricmc.loom.test.unit.processor.classes.SecondGenericInterface +import net.fabricmc.loom.test.unit.processor.classes.SelfGenericInterface +import net.fabricmc.loom.test.unit.processor.classes.SelfGenericTargetClass import net.fabricmc.loom.test.unit.processor.classes.SimpleInterface import net.fabricmc.loom.test.unit.processor.classes.SimpleTargetClass import net.fabricmc.loom.util.Constants +import net.fabricmc.loom.util.LazyCloseable import net.fabricmc.loom.util.Pair +import net.fabricmc.loom.util.TinyRemapperHelper import net.fabricmc.loom.util.ZipUtils import net.fabricmc.loom.util.fmj.FabricModJson import net.fabricmc.mappingio.MappingReader import net.fabricmc.mappingio.tree.MemoryMappingTree +import net.fabricmc.tinyremapper.TinyRemapper class InterfaceInjectionProcessorTest extends Specification { @TempDir @@ -64,6 +78,8 @@ class InterfaceInjectionProcessorTest extends Specification { def jar = tempDir.resolve("test.jar") packageJar(jar) + processorContext.createRemapper(MappingsNamespace.INTERMEDIARY, MappingsNamespace.NAMED) >> createRemapper(jar, processorContext.getMappings()) + when: def processor = new TestInterfaceInjectionProcessor() def spec = processor.buildSpec(specContext) @@ -83,10 +99,44 @@ class InterfaceInjectionProcessorTest extends Specification { } // Inner class with a simple interface - "class_1\$class_2" | "net/fabricmc/loom/test/unit/processor/classes/SimpleInterface" | SimpleTargetClass.Inner.class | { Class loadedClass -> + "class_1\$class_2" | "net/fabricmc/loom/test/unit/processor/classes/SimpleInterface" | SimpleTargetClass.Inner.class | { Class loadedClass -> loadedClass.interfaces.first().name == "net/fabricmc/loom/test/unit/processor/classes/SimpleInterface" loadedClass.constructors.first().newInstance().injectedMethod() == 123 } + + // Class using interface with generics + "class_3" | "net/fabricmc/loom/test/unit/processor/classes/GenericInterface" | GenericTargetClass.class | { Class loadedClass -> + loadedClass.interfaces.first().name == "net/fabricmc/loom/test/unit/processor/classes/GenericInterface" + loadedClass.constructors.first().newInstance().genericInjectedMethod() == null + } + + // Class using generics and passing them to interface + "class_4" | "net/fabricmc/loom/test/unit/processor/classes/GenericInterface" | PassingGenericTargetClass.class | { Class loadedClass -> + loadedClass.interfaces.first().name == "net/fabricmc/loom/test/unit/processor/classes/GenericInterface" + loadedClass.constructors.first().newInstance().genericInjectedMethod() == null + } + + // Class having one injected interface with two generics, including one provided by the class + "class_5" | "net/fabricmc/loom/test/unit/processor/classes/AdvancedGenericInterface;Ljava/lang/Integer;>" | AdvancedGenericTargetClass.class | { Class loadedClass -> + loadedClass.interfaces.first().name == "net/fabricmc/loom/test/unit/processor/classes/AdvancedGenericInterface" + loadedClass.constructors.first().newInstance().advancedGenericInjectedMethod().getClass() == AdvancedGenericTargetClass.Pair.class + } + + // Class having two injected interfaces with one generic for each of them, including one provided by the class + "class_7" | "net/fabricmc/loom/test/unit/processor/classes/FirstGenericInterface;>" | DoubleGenericTargetClass.class | { Class loadedClass -> + loadedClass.interfaces.first().name == "net/fabricmc/loom/test/unit/processor/classes/FirstGenericInterface" + loadedClass.constructors.first().newInstance().firstGenericInjectedMethod() == null + } + "class_7" | "net/fabricmc/loom/test/unit/processor/classes/SecondGenericInterface" | DoubleGenericTargetClass.class | { Class loadedClass -> + loadedClass.interfaces.last().name == "net/fabricmc/loom/test/unit/processor/classes/SecondGenericInterface" + loadedClass.constructors.last().newInstance().secondGenericInjectedMethod() == null + } + + // Self Generic Types + Signature Remapping Check + "class_8" | "net/fabricmc/loom/test/unit/processor/classes/SelfGenericInterface" | SelfGenericTargetClass.class | { Class loadedClass -> + loadedClass.interfaces.first().name == "net/fabricmc/loom/test/unit/proessor/classes/SelfGenericInterface" + loadedClass.constructors.first().newInstance().selfGenericInjectedMethod() == null + } } def "nothing to inject"() { @@ -130,6 +180,16 @@ class InterfaceInjectionProcessorTest extends Specification { return mappings } + static LazyCloseable createRemapper(Path jar, MemoryMappingTree mappings) { + return new LazyCloseable<>({ + TinyRemapper.Builder builder = TinyRemapper.newRemapper() + builder.withMappings(TinyRemapperHelper.create(mappings, MappingsNamespace.INTERMEDIARY.toString(), MappingsNamespace.NAMED.toString(), false)) + TinyRemapper tinyRemapper = builder.build() + tinyRemapper.readClassPath(jar) + return tinyRemapper + }, { tinyRemapper -> tinyRemapper.finish() }) + } + // Load a class from a jar file and execute a closure with it static void withTargetClass(Path jar, Class clazz, Consumer> closure) { // Groovy is needed as the test classes are compiled with it @@ -159,11 +219,29 @@ class InterfaceInjectionProcessorTest extends Specification { private static final List> CLASSES_TO_PACKAGE = [ SimpleTargetClass.class, SimpleTargetClass.Inner.class, - SimpleInterface.class + SimpleInterface.class, + GenericTargetClass.class, + PassingGenericTargetClass.class, + GenericInterface.class, + AdvancedGenericTargetClass.class, + AdvancedGenericTargetClass.Pair.class, + AdvancedGenericInterface.class, + DoubleGenericTargetClass.class, + FirstGenericInterface.class, + SecondGenericInterface.class, + SelfGenericTargetClass.class, + SelfGenericInterface.class ] + private static final String MAPPINGS = """ tiny\t2\t0\tintermediary\tnamed c\tclass_1\tnet/fabricmc/loom/test/unit/processor/classes/SimpleTargetClass c\tclass_1\$class_2\tnet/fabricmc/loom/test/unit/processor/classes/SimpleTargetClass\$Inner +c\tclass_3\tnet/fabricmc/loom/test/unit/processor/classes/GenericTargetClass +c\tclass_4\tnet/fabricmc/loom/test/unit/processor/classes/PassingGenericTargetClass +c\tclass_5\tnet/fabricmc/loom/test/unit/processor/classes/AdvancedGenericTargetClass +c\tclass_5\$class_6\tnet/fabricmc/loom/test/unit/processor/classes/AdvancedGenericTargetClass\$Pair +c\tclass_7\tnet/fabricmc/loom/test/unit/processor/classes/DoubleGenericTargetClass +c\tclass_8\tnet/fabricmc/loom/test/unit/processor/classes/SelfGenericTargetClass """.trim() -} \ No newline at end of file +} diff --git a/src/test/java/net/fabricmc/loom/test/unit/processor/classes/AdvancedGenericInterface.java b/src/test/java/net/fabricmc/loom/test/unit/processor/classes/AdvancedGenericInterface.java new file mode 100644 index 00000000..d1cce704 --- /dev/null +++ b/src/test/java/net/fabricmc/loom/test/unit/processor/classes/AdvancedGenericInterface.java @@ -0,0 +1,31 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2024 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.test.unit.processor.classes; + +public interface AdvancedGenericInterface { + default AdvancedGenericTargetClass.Pair advancedGenericInjectedMethod() { + return new AdvancedGenericTargetClass.Pair<>(null, null); + } +} diff --git a/src/test/java/net/fabricmc/loom/test/unit/processor/classes/AdvancedGenericTargetClass.java b/src/test/java/net/fabricmc/loom/test/unit/processor/classes/AdvancedGenericTargetClass.java new file mode 100644 index 00000000..4e3936c8 --- /dev/null +++ b/src/test/java/net/fabricmc/loom/test/unit/processor/classes/AdvancedGenericTargetClass.java @@ -0,0 +1,32 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2024 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.test.unit.processor.classes; + +public class AdvancedGenericTargetClass { + public static class Pair { + Pair(F ignoredF, S ignoredS) { + } + } +} diff --git a/src/test/java/net/fabricmc/loom/test/unit/processor/classes/DoubleGenericTargetClass.java b/src/test/java/net/fabricmc/loom/test/unit/processor/classes/DoubleGenericTargetClass.java new file mode 100644 index 00000000..793828fc --- /dev/null +++ b/src/test/java/net/fabricmc/loom/test/unit/processor/classes/DoubleGenericTargetClass.java @@ -0,0 +1,28 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2024 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.test.unit.processor.classes; + +public class DoubleGenericTargetClass { +} diff --git a/src/test/java/net/fabricmc/loom/test/unit/processor/classes/FirstGenericInterface.java b/src/test/java/net/fabricmc/loom/test/unit/processor/classes/FirstGenericInterface.java new file mode 100644 index 00000000..2f029b39 --- /dev/null +++ b/src/test/java/net/fabricmc/loom/test/unit/processor/classes/FirstGenericInterface.java @@ -0,0 +1,31 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2024 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.test.unit.processor.classes; + +public interface FirstGenericInterface { + default T firstGenericInjectedMethod() { + return null; + } +} diff --git a/src/test/java/net/fabricmc/loom/test/unit/processor/classes/GenericInterface.java b/src/test/java/net/fabricmc/loom/test/unit/processor/classes/GenericInterface.java new file mode 100644 index 00000000..92b186fc --- /dev/null +++ b/src/test/java/net/fabricmc/loom/test/unit/processor/classes/GenericInterface.java @@ -0,0 +1,31 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2024 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.test.unit.processor.classes; + +public interface GenericInterface { + default T genericInjectedMethod() { + return null; + } +} diff --git a/src/test/java/net/fabricmc/loom/test/unit/processor/classes/GenericTargetClass.java b/src/test/java/net/fabricmc/loom/test/unit/processor/classes/GenericTargetClass.java new file mode 100644 index 00000000..0a3d8283 --- /dev/null +++ b/src/test/java/net/fabricmc/loom/test/unit/processor/classes/GenericTargetClass.java @@ -0,0 +1,28 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2024 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.test.unit.processor.classes; + +public class GenericTargetClass { +} diff --git a/src/test/java/net/fabricmc/loom/test/unit/processor/classes/PassingGenericTargetClass.java b/src/test/java/net/fabricmc/loom/test/unit/processor/classes/PassingGenericTargetClass.java new file mode 100644 index 00000000..942b6805 --- /dev/null +++ b/src/test/java/net/fabricmc/loom/test/unit/processor/classes/PassingGenericTargetClass.java @@ -0,0 +1,28 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2024 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.test.unit.processor.classes; + +public class PassingGenericTargetClass { +} diff --git a/src/test/java/net/fabricmc/loom/test/unit/processor/classes/SecondGenericInterface.java b/src/test/java/net/fabricmc/loom/test/unit/processor/classes/SecondGenericInterface.java new file mode 100644 index 00000000..40bc0a81 --- /dev/null +++ b/src/test/java/net/fabricmc/loom/test/unit/processor/classes/SecondGenericInterface.java @@ -0,0 +1,31 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2024 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.test.unit.processor.classes; + +public interface SecondGenericInterface { + default T secondGenericInjectedMethod() { + return null; + } +} diff --git a/src/test/java/net/fabricmc/loom/test/unit/processor/classes/SelfGenericInterface.java b/src/test/java/net/fabricmc/loom/test/unit/processor/classes/SelfGenericInterface.java new file mode 100644 index 00000000..b11d17cd --- /dev/null +++ b/src/test/java/net/fabricmc/loom/test/unit/processor/classes/SelfGenericInterface.java @@ -0,0 +1,31 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2024 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.test.unit.processor.classes; + +public interface SelfGenericInterface> { + default S selfGenericInjectedMethod() { + return null; + } +} diff --git a/src/test/java/net/fabricmc/loom/test/unit/processor/classes/SelfGenericTargetClass.java b/src/test/java/net/fabricmc/loom/test/unit/processor/classes/SelfGenericTargetClass.java new file mode 100644 index 00000000..ee45491b --- /dev/null +++ b/src/test/java/net/fabricmc/loom/test/unit/processor/classes/SelfGenericTargetClass.java @@ -0,0 +1,28 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2024 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.test.unit.processor.classes; + +public class SelfGenericTargetClass { +} diff --git a/src/test/resources/projects/interfaceInjection/src/main/java/ExampleMod.java b/src/test/resources/projects/interfaceInjection/src/main/java/ExampleMod.java index d808f6fc..5502d2be 100644 --- a/src/test/resources/projects/interfaceInjection/src/main/java/ExampleMod.java +++ b/src/test/resources/projects/interfaceInjection/src/main/java/ExampleMod.java @@ -1,4 +1,5 @@ import net.minecraft.block.Blocks; +import net.minecraft.util.registry.Registry; import net.fabricmc.api.ModInitializer; @@ -7,5 +8,7 @@ public class ExampleMod implements ModInitializer { public void onInitialize() { Blocks.AIR.newMethodThatDidNotExist(); Blocks.AIR.anotherNewMethodThatDidNotExist(); + Blocks.AIR.typedMethodThatDidNotExist(); + Registry.BLOCK_KEY.genericMethodThatDidNotExist(); } } diff --git a/src/test/resources/projects/interfaceInjection/src/main/java/GenericInjectedInterface.java b/src/test/resources/projects/interfaceInjection/src/main/java/GenericInjectedInterface.java new file mode 100644 index 00000000..94028e75 --- /dev/null +++ b/src/test/resources/projects/interfaceInjection/src/main/java/GenericInjectedInterface.java @@ -0,0 +1,6 @@ + +public interface GenericInjectedInterface { + default T genericMethodThatDidNotExist() { + return null; + } +} diff --git a/src/test/resources/projects/interfaceInjection/src/main/java/OwnInjectedInterface.java b/src/test/resources/projects/interfaceInjection/src/main/java/OwnInjectedInterface.java index 4a96812d..6c507c47 100644 --- a/src/test/resources/projects/interfaceInjection/src/main/java/OwnInjectedInterface.java +++ b/src/test/resources/projects/interfaceInjection/src/main/java/OwnInjectedInterface.java @@ -1,5 +1,9 @@ -public interface OwnInjectedInterface { +public interface OwnInjectedInterface { default void anotherNewMethodThatDidNotExist() { } + + default T typedMethodThatDidNotExist() { + return null; + } } diff --git a/src/test/resources/projects/interfaceInjection/src/main/resources/fabric.mod.json b/src/test/resources/projects/interfaceInjection/src/main/resources/fabric.mod.json index c5d78c1f..c3815217 100644 --- a/src/test/resources/projects/interfaceInjection/src/main/resources/fabric.mod.json +++ b/src/test/resources/projects/interfaceInjection/src/main/resources/fabric.mod.json @@ -5,7 +5,12 @@ "name": "Own Dummy Mod", "custom": { "loom:injected_interfaces": { - "net/minecraft/class_2248": ["OwnInjectedInterface"] + "net/minecraft/class_2248": [ + "OwnInjectedInterface" + ], + "net/minecraft/class_5321": [ + "GenericInjectedInterface" + ] } } }