From 06e9fb16e5405c8f9249ab4c010cfef0df091f9c Mon Sep 17 00:00:00 2001 From: Juuz <6596629+Juuxel@users.noreply.github.com> Date: Sun, 22 Jan 2023 01:03:42 +0200 Subject: [PATCH] Rewrite the internals of mod configuration remapping to fix bugs (#807) * Add test for #801 * Add test for #572 * Rewrite mod configuration remapping internals to fix bugs Fixes #801. Fixes #572. - Instead of individual mod configs getting remapped, Loom now remaps "collector configs": `mod[Compile/Runtime]Classpath[source set name]` (eg `modRuntimeClasspathMain` -> `modRuntimeClasspathMainRemapped`) - This lets us use Gradle's `org.gradle.usage` attributes to select whether it's a compile-time or runtime dependency - Note: Remap configurations sourcing from `api` are partially left intact due to their special logic in the `namedElements` configuration. - Setting the proper usages fixes #801. - No dependency files are added to the real target configurations (like `api` and `implementation`) anymore. - This fixes #572, since `runtimeClasspath` and `compileClasspath` don't leak transitive dependencies unlike `implementation` etc. * Fix checkstyle * Fix split env dependency consumers * Only use collector configs for remapped deps and let the inputs stay intact This way the code has less duplication. * Improve log messages * Update year * Add some comments * Fix compilation * Use LinkedHashMap for reliable iteration order through remapped configs * Use ImmutableMap.of instead of Map.of for reliable ordering * ModConfigurationRemapper: Move namedElements handling out of forEach * Add regression test for a crash where loader is resolved after other mods * Fix the aforementioned bug * Rename InstallerDataTest -> DependencyOrderTest * Add TODO about refresh dependencies The code currently processes the same deps multiple times when --refresh-dependencies/-Dloom.refresh=true/a cache invalidation is applied. Co-authored-by: modmuss50 --- .../loom/api/RemapConfigurationSettings.java | 44 +--- .../configuration/RemapConfigurations.java | 99 ++++++--- .../mods/ModConfigurationRemapper.java | 203 +++++++++++------- .../loom/configuration/mods/ModProcessor.java | 36 +++- .../mods/dependency/SplitModDependency.java | 12 +- .../minecraft/MinecraftSourceSets.java | 4 +- .../java/net/fabricmc/loom/util/Strings.java | 47 ++++ .../integration/DependencyOrderTest.groovy | 58 +++++ .../loom/test/integration/KotlinTest.groovy | 9 +- .../loom/test/unit/StringsTest.groovy | 47 ++++ .../projects/kotlin/build.gradle.kts | 12 +- .../resources/projects/maven/build.gradle | 30 ++- .../projects/mavenLibrary/build.gradle | 2 + 13 files changed, 441 insertions(+), 162 deletions(-) create mode 100644 src/main/java/net/fabricmc/loom/util/Strings.java create mode 100644 src/test/groovy/net/fabricmc/loom/test/integration/DependencyOrderTest.groovy create mode 100644 src/test/groovy/net/fabricmc/loom/test/unit/StringsTest.groovy diff --git a/src/main/java/net/fabricmc/loom/api/RemapConfigurationSettings.java b/src/main/java/net/fabricmc/loom/api/RemapConfigurationSettings.java index b1b00e0b..8eb0aeeb 100644 --- a/src/main/java/net/fabricmc/loom/api/RemapConfigurationSettings.java +++ b/src/main/java/net/fabricmc/loom/api/RemapConfigurationSettings.java @@ -1,7 +1,7 @@ /* * This file is part of fabric-loom, licensed under the MIT License (MIT). * - * Copyright (c) 2022 FabricMC + * Copyright (c) 2022-2023 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 @@ -28,7 +28,6 @@ import java.util.Set; import javax.inject.Inject; -import com.google.common.base.Preconditions; import org.gradle.api.Named; import org.gradle.api.NamedDomainObjectProvider; import org.gradle.api.Project; @@ -41,9 +40,6 @@ import org.gradle.api.tasks.SourceSet; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; -import net.fabricmc.loom.LoomGradleExtension; -import net.fabricmc.loom.configuration.providers.minecraft.MinecraftJarConfiguration; - /** * A {@link Named} object for configuring "proxy" configurations that remap artifacts. */ @@ -132,50 +128,12 @@ public abstract class RemapConfigurationSettings implements Named { return getName() + "Mapped"; } - @ApiStatus.Internal - @Internal - public final Provider getClientRemappedConfigurationName() { - return getClientSourceConfigurationName().map(s -> s + "Mapped"); - } - @ApiStatus.Internal @Internal public final NamedDomainObjectProvider getSourceConfiguration() { return getConfigurationByName(getName()); } - @ApiStatus.Internal - @Internal - public final NamedDomainObjectProvider getRemappedConfiguration() { - return getConfigurationByName(getRemappedConfigurationName()); - } - - @ApiStatus.Internal - @Internal - public final NamedDomainObjectProvider getTargetConfiguration() { - return getConfigurationByName(getTargetConfigurationName().get()); - } - - @ApiStatus.Internal - @Internal - public final Provider getClientTargetConfiguration() { - return getProject().provider(() -> { - boolean split = LoomGradleExtension.get(getProject()).getMinecraftJarConfiguration().get() == MinecraftJarConfiguration.SPLIT; - Preconditions.checkArgument(split, "Cannot get client target configuration when project is not split"); - return getConfigurationByName(getClientSourceConfigurationName().get()).get(); - }); - } - - @ApiStatus.Internal - @Internal - public final Provider getClientRemappedConfiguration() { - return getProject().provider(() -> { - boolean split = LoomGradleExtension.get(getProject()).getMinecraftJarConfiguration().get() == MinecraftJarConfiguration.SPLIT; - Preconditions.checkArgument(split, "Cannot get client remapped configuration when project is not split"); - return getConfigurationByName(getClientRemappedConfigurationName().get()).get(); - }); - } - @Internal private NamedDomainObjectProvider getConfigurationByName(String name) { return getProject().getConfigurations().named(name); diff --git a/src/main/java/net/fabricmc/loom/configuration/RemapConfigurations.java b/src/main/java/net/fabricmc/loom/configuration/RemapConfigurations.java index 125aee50..1428c8ee 100644 --- a/src/main/java/net/fabricmc/loom/configuration/RemapConfigurations.java +++ b/src/main/java/net/fabricmc/loom/configuration/RemapConfigurations.java @@ -1,7 +1,7 @@ /* * This file is part of fabric-loom, licensed under the MIT License (MIT). * - * Copyright (c) 2022 FabricMC + * Copyright (c) 2022-2023 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 @@ -32,6 +32,8 @@ import org.gradle.api.Action; import org.gradle.api.NamedDomainObjectList; import org.gradle.api.Project; import org.gradle.api.artifacts.Configuration; +import org.gradle.api.artifacts.ConfigurationContainer; +import org.gradle.api.attributes.Usage; import org.gradle.api.plugins.JavaPlugin; import org.gradle.api.tasks.SourceSet; import org.jetbrains.annotations.VisibleForTesting; @@ -39,6 +41,7 @@ import org.jetbrains.annotations.VisibleForTesting; import net.fabricmc.loom.LoomGradleExtension; import net.fabricmc.loom.api.RemapConfigurationSettings; import net.fabricmc.loom.util.Constants; +import net.fabricmc.loom.util.Strings; import net.fabricmc.loom.util.gradle.SourceSetHelper; public final class RemapConfigurations { @@ -85,54 +88,88 @@ public final class RemapConfigurations { } settings.getClientSourceConfigurationName().set(name); - createClientMappedConfiguration(project, settings, clientSourceSet); }); } } - private static void createClientMappedConfiguration(Project project, RemapConfigurationSettings settings, SourceSet clientSourceSet) { - final Configuration remappedConfiguration = project.getConfigurations().create(settings.getClientRemappedConfigurationName().get()); - // Don't get transitive deps of already remapped mods - remappedConfiguration.setTransitive(false); + /** + * Gets or creates the collector configuration for a {@link SourceSet}. + * The collector configuration receives all compile-time or runtime remapped mod dependency files. + * + * @param project the project + * @param settings the remap configuration settings + * @param runtime if {@code true}, returns the runtime configuration; + * if {@code false}, returns the compile-time one + * @return the collector configuration + */ + public static Configuration getOrCreateCollectorConfiguration(Project project, RemapConfigurationSettings settings, boolean runtime) { + return getOrCreateCollectorConfiguration(project, settings.getSourceSet().get(), runtime); + } - if (settings.getOnCompileClasspath().get()) { - extendsFrom(Constants.Configurations.MOD_COMPILE_CLASSPATH_MAPPED, remappedConfiguration, project); + /** + * Gets or creates the collector configuration for a {@link RemapConfigurationSettings} instance. + * The collector configuration receives all compile-time or runtime remapped mod dependency files. + * + * @param project the project + * @param sourceSet the source set to apply the collector config to, should generally match {@link RemapConfigurationSettings#getSourceSet()} + * @param runtime if {@code true}, returns the runtime configuration; + * if {@code false}, returns the compile-time one + * @return the collector configuration + */ + // Note: this method is generally called on demand, so these configurations + // won't exist at buildscript evaluation time. There's no need for them anyway + // since they're internals. + public static Configuration getOrCreateCollectorConfiguration(Project project, SourceSet sourceSet, boolean runtime) { + final String configurationName = "mod" + + (runtime ? "Runtime" : "Compile") + + "Classpath" + + Strings.capitalize(sourceSet.getName()) + + "Mapped"; + final ConfigurationContainer configurations = project.getConfigurations(); + Configuration configuration = configurations.findByName(configurationName); - extendsFrom(clientSourceSet.getCompileClasspathConfigurationName(), remappedConfiguration, project); + if (configuration == null) { + configuration = configurations.create(configurationName); + + // Don't get transitive deps of already remapped mods + configuration.setTransitive(false); + + // Set the usage attribute to fetch the correct artifacts. + // Note: Even though most deps are resolved via copies of mod* configurations, + // non-remapped mods that get added straight to these collectors will need the attribute. + final Usage usage = project.getObjects().named(Usage.class, runtime ? Usage.JAVA_RUNTIME : Usage.JAVA_API); + configuration.attributes(attributes -> attributes.attribute(Usage.USAGE_ATTRIBUTE, usage)); + + // The main classpath also applies to the test source set like with normal dependencies. + final boolean isMainSourceSet = sourceSet.getName().equals("main"); + + if (runtime) { + extendsFrom(sourceSet.getRuntimeClasspathConfigurationName(), configuration, project); + + if (isMainSourceSet) { + extendsFrom(JavaPlugin.TEST_RUNTIME_CLASSPATH_CONFIGURATION_NAME, configuration, project); + } + } else { + extendsFrom(sourceSet.getCompileClasspathConfigurationName(), configuration, project); + extendsFrom(Constants.Configurations.MOD_COMPILE_CLASSPATH_MAPPED, configuration, project); + + if (isMainSourceSet) { + extendsFrom(JavaPlugin.TEST_COMPILE_CLASSPATH_CONFIGURATION_NAME, configuration, project); + } + } } - if (settings.getOnRuntimeClasspath().get()) { - extendsFrom(clientSourceSet.getRuntimeClasspathConfigurationName(), remappedConfiguration, project); - } + return configuration; } public static void applyToProject(Project project, RemapConfigurationSettings settings) { - final SourceSet sourceSet = settings.getSourceSet().get(); - final boolean isMainSourceSet = sourceSet.getName().equals("main"); // No point bothering to make it lazily, gradle realises configurations right away. // - final Configuration remappedConfiguration = project.getConfigurations().create(settings.getRemappedConfigurationName()); final Configuration configuration = project.getConfigurations().create(settings.getName()); configuration.setTransitive(true); - // Don't get transitive deps of already remapped mods - remappedConfiguration.setTransitive(false); if (settings.getOnCompileClasspath().get()) { extendsFrom(Constants.Configurations.MOD_COMPILE_CLASSPATH, configuration, project); - extendsFrom(Constants.Configurations.MOD_COMPILE_CLASSPATH_MAPPED, remappedConfiguration, project); - extendsFrom(sourceSet.getCompileClasspathConfigurationName(), remappedConfiguration, project); - - if (isMainSourceSet) { - extendsFrom(JavaPlugin.TEST_COMPILE_CLASSPATH_CONFIGURATION_NAME, remappedConfiguration, project); - } - } - - if (settings.getOnRuntimeClasspath().get()) { - extendsFrom(sourceSet.getRuntimeClasspathConfigurationName(), remappedConfiguration, project); - - if (isMainSourceSet) { - extendsFrom(JavaPlugin.TEST_RUNTIME_CLASSPATH_CONFIGURATION_NAME, remappedConfiguration, project); - } } for (String outgoingConfigurationName : settings.getPublishingMode().get().outgoingConfigurations()) { diff --git a/src/main/java/net/fabricmc/loom/configuration/mods/ModConfigurationRemapper.java b/src/main/java/net/fabricmc/loom/configuration/mods/ModConfigurationRemapper.java index d5321428..e646405d 100644 --- a/src/main/java/net/fabricmc/loom/configuration/mods/ModConfigurationRemapper.java +++ b/src/main/java/net/fabricmc/loom/configuration/mods/ModConfigurationRemapper.java @@ -1,7 +1,7 @@ /* * This file is part of fabric-loom, licensed under the MIT License (MIT). * - * Copyright (c) 2019-2022 FabricMC + * Copyright (c) 2019-2023 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 @@ -30,9 +30,13 @@ import java.io.UncheckedIOException; import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayList; +import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; +import java.util.Map; import java.util.function.Supplier; +import com.google.common.collect.ImmutableMap; import org.gradle.api.Project; import org.gradle.api.artifacts.Configuration; import org.gradle.api.artifacts.FileCollectionDependency; @@ -43,20 +47,25 @@ import org.gradle.api.artifacts.query.ArtifactResolutionQuery; import org.gradle.api.artifacts.result.ArtifactResult; import org.gradle.api.artifacts.result.ComponentArtifactsResult; import org.gradle.api.artifacts.result.ResolvedArtifactResult; +import org.gradle.api.attributes.Usage; import org.gradle.api.file.FileCollection; import org.gradle.api.plugins.JavaPlugin; +import org.gradle.api.tasks.SourceSet; import org.gradle.jvm.JvmLibrary; import org.gradle.language.base.artifact.SourcesArtifact; import org.jetbrains.annotations.Nullable; import net.fabricmc.loom.LoomGradleExtension; import net.fabricmc.loom.api.RemapConfigurationSettings; +import net.fabricmc.loom.configuration.RemapConfigurations; import net.fabricmc.loom.configuration.mods.dependency.ModDependency; import net.fabricmc.loom.configuration.mods.dependency.ModDependencyFactory; +import net.fabricmc.loom.configuration.providers.minecraft.MinecraftSourceSets; import net.fabricmc.loom.util.Checksum; import net.fabricmc.loom.util.Constants; import net.fabricmc.loom.util.OperatingSystem; import net.fabricmc.loom.util.SourceRemapper; +import net.fabricmc.loom.util.gradle.SourceSetHelper; import net.fabricmc.loom.util.service.SharedServiceManager; @SuppressWarnings("UnstableApiUsage") @@ -67,88 +76,132 @@ public class ModConfigurationRemapper { public static void supplyModConfigurations(Project project, SharedServiceManager serviceManager, String mappingsSuffix, LoomGradleExtension extension, SourceRemapper sourceRemapper) { final DependencyHandler dependencies = project.getDependencies(); + // The configurations where the source and remapped artifacts go. + // key: source, value: target + final Map configsToRemap = new LinkedHashMap<>(); + // Client remapped dep collectors for split source sets. Same keys and values. + final Map clientConfigsToRemap = new HashMap<>(); for (RemapConfigurationSettings entry : extension.getRemapConfigurations()) { - entry.getRemappedConfiguration().configure(remappedConfig -> { - /* - sourceConfig - The source configuration where the intermediary named artifacts come from. i.e "modApi" - remappedConfig - an intermediate configuration where the remapped artifacts go - targetConfig - extends from the remappedConfig, such as "api" - */ - final Configuration sourceConfig = entry.getSourceConfiguration().get(); - final Configuration targetConfig = entry.getTargetConfiguration().get(); - final boolean hasClientTarget = entry.getClientSourceConfigurationName().isPresent(); + // key: true if runtime, false if compile + final Map envToEnabled = ImmutableMap.of( + false, entry.getOnCompileClasspath().get(), + true, entry.getOnRuntimeClasspath().get() + ); - Configuration clientRemappedConfig = null; + envToEnabled.forEach((runtime, enabled) -> { + if (!enabled) return; - if (hasClientTarget) { - clientRemappedConfig = entry.getClientRemappedConfiguration().get(); - } + final Configuration target = RemapConfigurations.getOrCreateCollectorConfiguration(project, entry, runtime); + // We copy the source with the desired usage type to get only the runtime or api jars, not both. + final Configuration sourceCopy = entry.getSourceConfiguration().get().copy(); + final Usage usage = project.getObjects().named(Usage.class, runtime ? Usage.JAVA_RUNTIME : Usage.JAVA_API); + sourceCopy.attributes(attributes -> attributes.attribute(Usage.USAGE_ATTRIBUTE, usage)); + configsToRemap.put(sourceCopy, target); - final List modDependencies = new ArrayList<>(); - - for (ArtifactRef artifact : resolveArtifacts(project, sourceConfig)) { - final ArtifactMetadata artifactMetadata; - - try { - artifactMetadata = ArtifactMetadata.create(artifact); - } catch (IOException e) { - throw new UncheckedIOException("Failed to read metadata from" + artifact.path(), e); - } - - if (artifactMetadata.installerData() != null) { - if (extension.getInstallerData() != null) { - project.getLogger().info("Found another installer JSON in ({}), ignoring", artifact.path()); - } else { - project.getLogger().info("Applying installer data from {}", artifact.path()); - artifactMetadata.installerData().applyToProject(project); - } - } - - if (!artifactMetadata.shouldRemap()) { - artifact.applyToConfiguration(project, targetConfig); - continue; - } - - final ModDependency modDependency = ModDependencyFactory.create(artifact, remappedConfig, clientRemappedConfig, mappingsSuffix, project); - scheduleSourcesRemapping(project, sourceRemapper, modDependency); - modDependencies.add(modDependency); - } - - if (modDependencies.isEmpty()) { - // Nothing else to do - return; - } - - final boolean refreshDeps = LoomGradleExtension.get(project).refreshDeps(); - final List toRemap = modDependencies.stream() - .filter(dependency -> refreshDeps || dependency.isCacheInvalid(project, null)) - .toList(); - - if (!toRemap.isEmpty()) { - try { - new ModProcessor(project, sourceConfig, serviceManager).processMods(toRemap); - } catch (IOException e) { - throw new UncheckedIOException("Failed to remap mods", e); - } - } - - // Add all of the remapped mods onto the config - for (ModDependency info : modDependencies) { - info.applyToProject(project); - createConstraints(info.getInputArtifact(), targetConfig, sourceConfig, dependencies); - - if (clientRemappedConfig != null) { - createConstraints(info.getInputArtifact(), entry.getClientTargetConfiguration().get(), sourceConfig, dependencies); - } - } - - // Export to other projects - if (entry.getTargetConfigurationName().get().equals(JavaPlugin.API_CONFIGURATION_NAME)) { - project.getConfigurations().getByName(Constants.Configurations.NAMED_ELEMENTS).extendsFrom(remappedConfig); + // If our remap configuration entry targets the client source set as well, + // let's set up a collector for it too. + if (entry.getClientSourceConfigurationName().isPresent()) { + final SourceSet clientSourceSet = SourceSetHelper.getSourceSetByName(MinecraftSourceSets.Split.CLIENT_ONLY_SOURCE_SET_NAME, project); + final Configuration clientTarget = RemapConfigurations.getOrCreateCollectorConfiguration(project, clientSourceSet, runtime); + clientConfigsToRemap.put(sourceCopy, clientTarget); } }); + + // Export to other projects. + if (entry.getTargetConfigurationName().get().equals(JavaPlugin.API_CONFIGURATION_NAME)) { + // Note: legacy (pre-1.1) behavior is kept for this remapping since + // we don't have a modApiElements/modRuntimeElements kind of configuration. + // TODO: Expose API/runtime usage attributes for namedElements to make it work like normal project dependencies. + final Configuration remappedConfig = project.getConfigurations().maybeCreate(entry.getRemappedConfigurationName()); + remappedConfig.setTransitive(false); + project.getConfigurations().getByName(Constants.Configurations.NAMED_ELEMENTS).extendsFrom(remappedConfig); + configsToRemap.put(entry.getSourceConfiguration().get(), remappedConfig); + } } + + // Round 1: Discovery + // Go through all the configs to find artifacts to remap and + // the installer data. The installer data has to be added before + // any mods are remapped since remapping needs the dependencies provided by that data. + final Map> dependenciesBySourceConfig = new HashMap<>(); + configsToRemap.forEach((sourceConfig, remappedConfig) -> { + /* + sourceConfig - The source configuration where the intermediary named artifacts come from. i.e "modApi" + remappedConfig - The target configuration where the remapped artifacts go + */ + final Configuration clientRemappedConfig = clientConfigsToRemap.get(sourceConfig); + final List modDependencies = new ArrayList<>(); + + for (ArtifactRef artifact : resolveArtifacts(project, sourceConfig)) { + final ArtifactMetadata artifactMetadata; + + try { + artifactMetadata = ArtifactMetadata.create(artifact); + } catch (IOException e) { + throw new UncheckedIOException("Failed to read metadata from" + artifact.path(), e); + } + + if (artifactMetadata.installerData() != null) { + if (extension.getInstallerData() != null) { + project.getLogger().info("Found another installer JSON in ({}), ignoring", artifact.path()); + } else { + project.getLogger().info("Applying installer data from {}", artifact.path()); + artifactMetadata.installerData().applyToProject(project); + } + } + + if (!artifactMetadata.shouldRemap()) { + // Note: not applying to any type of vanilla Gradle target config like + // api or implementation to fix https://github.com/FabricMC/fabric-loom/issues/572. + artifact.applyToConfiguration(project, remappedConfig); + continue; + } + + final ModDependency modDependency = ModDependencyFactory.create(artifact, remappedConfig, clientRemappedConfig, mappingsSuffix, project); + scheduleSourcesRemapping(project, sourceRemapper, modDependency); + modDependencies.add(modDependency); + } + + dependenciesBySourceConfig.put(sourceConfig, modDependencies); + }); + + // Round 2: Remapping + // Remap all discovered artifacts. + configsToRemap.forEach((sourceConfig, remappedConfig) -> { + final List modDependencies = dependenciesBySourceConfig.get(sourceConfig); + + if (modDependencies.isEmpty()) { + // Nothing else to do + return; + } + + final Configuration clientRemappedConfig = clientConfigsToRemap.get(sourceConfig); + final boolean refreshDeps = LoomGradleExtension.get(project).refreshDeps(); + // TODO: With the same artifacts being considered multiple times for their different + // usage attributes, this should probably not process them multiple times even with refreshDeps. + final List toRemap = modDependencies.stream() + .filter(dependency -> refreshDeps || dependency.isCacheInvalid(project, null)) + .toList(); + + if (!toRemap.isEmpty()) { + try { + new ModProcessor(project, sourceConfig, serviceManager).processMods(toRemap); + } catch (IOException e) { + throw new UncheckedIOException("Failed to remap mods", e); + } + } + + // Add all of the remapped mods onto the config + for (ModDependency info : modDependencies) { + info.applyToProject(project); + createConstraints(info.getInputArtifact(), remappedConfig, sourceConfig, dependencies); + + if (clientRemappedConfig != null) { + createConstraints(info.getInputArtifact(), clientRemappedConfig, sourceConfig, dependencies); + } + } + }); } private static void createConstraints(ArtifactRef artifact, Configuration targetConfig, Configuration sourceConfig, DependencyHandler dependencies) { diff --git a/src/main/java/net/fabricmc/loom/configuration/mods/ModProcessor.java b/src/main/java/net/fabricmc/loom/configuration/mods/ModProcessor.java index f5fe6fc3..2ee3ffd9 100644 --- a/src/main/java/net/fabricmc/loom/configuration/mods/ModProcessor.java +++ b/src/main/java/net/fabricmc/loom/configuration/mods/ModProcessor.java @@ -1,7 +1,7 @@ /* * This file is part of fabric-loom, licensed under the MIT License (MIT). * - * Copyright (c) 2018-2021 FabricMC + * Copyright (c) 2018-2023 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 @@ -36,10 +36,13 @@ import java.util.List; import java.util.Locale; import java.util.Map; import java.util.jar.Manifest; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import com.google.gson.JsonObject; import org.gradle.api.Project; import org.gradle.api.artifacts.Configuration; +import org.gradle.api.attributes.Usage; import net.fabricmc.loom.LoomGradleExtension; import net.fabricmc.loom.api.RemapConfigurationSettings; @@ -63,6 +66,8 @@ public class ModProcessor { private static final String fromM = MappingsNamespace.INTERMEDIARY.toString(); private static final String toM = MappingsNamespace.NAMED.toString(); + private static final Pattern COPY_CONFIGURATION_PATTERN = Pattern.compile("^(.+)Copy[0-9]*$"); + private final Project project; private final Configuration sourceConfiguration; private final SharedServiceManager serviceManager; @@ -75,13 +80,40 @@ public class ModProcessor { public void processMods(List remapList) throws IOException { try { - project.getLogger().lifecycle(":remapping {} mods from {}", remapList.size(), sourceConfiguration.getName()); + project.getLogger().lifecycle(":remapping {} mods from {}", remapList.size(), describeConfiguration(sourceConfiguration)); remapJars(remapList); } catch (Exception e) { throw new RuntimeException(String.format(Locale.ENGLISH, "Failed to remap %d mods", remapList.size()), e); } } + // Creates a human-readable descriptive string for the configuration. + // This consists primarily of the name with any copy suffixes stripped + // (they're not informative), and the usage attribute if present. + private String describeConfiguration(Configuration configuration) { + String description = configuration.getName(); + final Matcher copyMatcher = COPY_CONFIGURATION_PATTERN.matcher(description); + + // If we find a copy suffix, remove it. + if (copyMatcher.matches()) { + final String realName = copyMatcher.group(1); + + // It's only a copy if we find a non-copy version. + if (project.getConfigurations().findByName(realName) != null) { + description = realName; + } + } + + // Add the usage if present, e.g. "modImplementation (java-api)" + final Usage usage = configuration.getAttributes().getAttribute(Usage.USAGE_ATTRIBUTE); + + if (usage != null) { + description += " (" + usage.getName() + ")"; + } + + return description; + } + private void stripNestedJars(Path path) { // Strip out all contained jar info as we dont want loader to try and load the jars contained in dev. try { diff --git a/src/main/java/net/fabricmc/loom/configuration/mods/dependency/SplitModDependency.java b/src/main/java/net/fabricmc/loom/configuration/mods/dependency/SplitModDependency.java index 3eabeb3a..0210036e 100644 --- a/src/main/java/net/fabricmc/loom/configuration/mods/dependency/SplitModDependency.java +++ b/src/main/java/net/fabricmc/loom/configuration/mods/dependency/SplitModDependency.java @@ -1,7 +1,7 @@ /* * This file is part of fabric-loom, licensed under the MIT License (MIT). * - * Copyright (c) 2022 FabricMC + * Copyright (c) 2022-2023 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 @@ -33,6 +33,7 @@ import org.gradle.api.artifacts.Configuration; import org.jetbrains.annotations.Nullable; import net.fabricmc.loom.LoomGradleExtension; +import net.fabricmc.loom.api.ModSettings; import net.fabricmc.loom.configuration.mods.ArtifactRef; import net.fabricmc.loom.configuration.mods.JarSplitter; @@ -120,11 +121,10 @@ public final class SplitModDependency extends ModDependency { private void createModGroup(Path commonJar, Path clientJar) { LoomGradleExtension extension = LoomGradleExtension.get(project); - extension.getMods().register(String.format("%s-%s-%s", getRemappedGroup(), name, version), modSettings -> - modSettings.getModFiles().from( - commonJar.toFile(), - clientJar.toFile() - ) + final ModSettings modSettings = extension.getMods().maybeCreate(String.format("%s-%s-%s", getRemappedGroup(), name, version)); + modSettings.getModFiles().from( + commonJar.toFile(), + clientJar.toFile() ); } diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/MinecraftSourceSets.java b/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/MinecraftSourceSets.java index 34f1d1eb..b0c6b204 100644 --- a/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/MinecraftSourceSets.java +++ b/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/MinecraftSourceSets.java @@ -1,7 +1,7 @@ /* * This file is part of fabric-loom, licensed under the MIT License (MIT). * - * Copyright (c) 2022 FabricMC + * Copyright (c) 2022-2023 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 @@ -126,7 +126,7 @@ public abstract sealed class MinecraftSourceSets permits MinecraftSourceSets.Sin private static final String MINECRAFT_CLIENT_ONLY_NAMED = "minecraftClientOnlyNamed"; private static final String MINECRAFT_COMBINED_NAMED = "minecraftCombinedNamed"; - private static final String CLIENT_ONLY_SOURCE_SET_NAME = "client"; + public static final String CLIENT_ONLY_SOURCE_SET_NAME = "client"; private static final Split INSTANCE = new Split(); diff --git a/src/main/java/net/fabricmc/loom/util/Strings.java b/src/main/java/net/fabricmc/loom/util/Strings.java new file mode 100644 index 00000000..c56f0bf2 --- /dev/null +++ b/src/main/java/net/fabricmc/loom/util/Strings.java @@ -0,0 +1,47 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2023 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; + +public final class Strings { + public static String capitalize(String word) { + // Don't capitalize empty strings + if (word.isBlank()) { + return word; + } + + final StringBuilder builder = new StringBuilder(); + final int codePointCount = word.codePointCount(0, word.length()); + final int first = Character.toUpperCase(word.codePointAt(0)); + builder.append(Character.toString(first)); + + if (codePointCount > 1) { + for (int codePoint = 1; codePoint < codePointCount; codePoint++) { + builder.append(Character.toString(word.codePointAt(codePoint))); + } + } + + return builder.toString(); + } +} diff --git a/src/test/groovy/net/fabricmc/loom/test/integration/DependencyOrderTest.groovy b/src/test/groovy/net/fabricmc/loom/test/integration/DependencyOrderTest.groovy new file mode 100644 index 00000000..3900c5b3 --- /dev/null +++ b/src/test/groovy/net/fabricmc/loom/test/integration/DependencyOrderTest.groovy @@ -0,0 +1,58 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2023 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 net.fabricmc.loom.test.util.GradleProjectTestTrait +import spock.lang.Specification +import spock.lang.Unroll + +import static net.fabricmc.loom.test.LoomTestConstants.* +import static org.gradle.testkit.runner.TaskOutcome.SUCCESS + +class DependencyOrderTest extends Specification implements GradleProjectTestTrait { + // Regression test for a bug introduced in 1.1 development where + // if Fabric Loader is resolved after another mod dependency, + // Gradle will crash because loaderLibraries has been resolved before + // Loader's dependencies have been added to it. + @Unroll + def "build with loader as the second dependency (gradle #version)"() { + setup: + def gradle = gradleProject(project: "minimalBase", version: version) + gradle.buildGradle << """ + dependencies { + minecraft 'com.mojang:minecraft:1.19.3' + mappings 'net.fabricmc:yarn:1.19.3+build.5:v2' + modApi 'net.fabricmc.fabric-api:fabric-api:0.73.0+1.19.3' + modImplementation 'net.fabricmc:fabric-loader:0.14.13' + } + """.stripIndent() + when: + def result = gradle.run(task: "build") + then: + result.task(":build").outcome == SUCCESS + where: + version << STANDARD_TEST_VERSIONS + } +} 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 b94d4612..64302890 100644 --- a/src/test/groovy/net/fabricmc/loom/test/integration/KotlinTest.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/integration/KotlinTest.groovy @@ -42,13 +42,20 @@ class KotlinTest extends Specification implements GradleProjectTestTrait { .downloadMod(ServerRunner.FABRIC_LANG_KOTLIN, "fabric-language-kotlin-1.8.7+kotlin.1.7.22.jar") when: - def result = gradle.run(task: "build") + def result = gradle.run(tasks: ["build", "publishToMavenLocal"]) def serverResult = server.run() then: result.task(":build").outcome == SUCCESS serverResult.successful() + // Check POM file to see that it doesn't contain transitive deps of FLK. + // See https://github.com/FabricMC/fabric-loom/issues/572. + result.task(":publishToMavenLocal").outcome == SUCCESS + def pom = new File(gradle.projectDir, "build/publications/mavenKotlin/pom-default.xml").text + // FLK depends on kotlin-reflect unlike our test project. + pom.contains('kotlin-reflect') == false + where: version << STANDARD_TEST_VERSIONS } diff --git a/src/test/groovy/net/fabricmc/loom/test/unit/StringsTest.groovy b/src/test/groovy/net/fabricmc/loom/test/unit/StringsTest.groovy new file mode 100644 index 00000000..c395910a --- /dev/null +++ b/src/test/groovy/net/fabricmc/loom/test/unit/StringsTest.groovy @@ -0,0 +1,47 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2023 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 + +import net.fabricmc.loom.util.Strings +import spock.lang.Specification +import spock.lang.Unroll + +class StringsTest extends Specification { + @Unroll + def "capitalize '#input'"() { + when: + def result = Strings.capitalize(input) + then: + result == expected + + where: + input | expected + '' | '' + ' \n ' | ' \n ' + 'world' | 'World' + 'helloWorld' | 'HelloWorld' + '\u00E4mp\u00E4ri' | '\u00C4mp\u00E4ri' + } +} diff --git a/src/test/resources/projects/kotlin/build.gradle.kts b/src/test/resources/projects/kotlin/build.gradle.kts index c24d3375..7f2e0f5d 100644 --- a/src/test/resources/projects/kotlin/build.gradle.kts +++ b/src/test/resources/projects/kotlin/build.gradle.kts @@ -4,6 +4,7 @@ plugins { kotlin("jvm") version "1.7.22" kotlin("plugin.serialization") version "1.7.22" id("fabric-loom") + `maven-publish` } java { @@ -11,6 +12,7 @@ java { targetCompatibility = JavaVersion.VERSION_1_8 } +group = "com.example" version = "0.0.1" dependencies { @@ -18,4 +20,12 @@ dependencies { 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") -} \ No newline at end of file +} + +publishing { + publications { + create("mavenKotlin") { + from(components["java"]) + } + } +} diff --git a/src/test/resources/projects/maven/build.gradle b/src/test/resources/projects/maven/build.gradle index 56930fcd..0b19a3ad 100644 --- a/src/test/resources/projects/maven/build.gradle +++ b/src/test/resources/projects/maven/build.gradle @@ -22,4 +22,32 @@ dependencies { modImplementation "net.fabricmc:fabric-loader:0.11.2" modImplementation System.getProperty("loom.test.resolve") -} \ No newline at end of file +} + +task checkClasspaths { + // Check that only compileClasspath contains the biome API and only runtimeClasspath contains the command API. + // See https://github.com/FabricMC/fabric-loom/issues/801. + + doLast { + def compile = configurations.compileClasspath.resolve().collect { it.name } + def runtime = configurations.runtimeClasspath.resolve().collect { it.name } + + if (!compile.any { it.contains('biome-api') }) { + throw new RuntimeException('No biome API on compile classpath') + } + + if (compile.any { it.contains('command-api') }) { + throw new RuntimeException('Command API on compile classpath') + } + + if (!runtime.any { it.contains('command-api') }) { + throw new RuntimeException('No command API on runtime classpath') + } + + if (runtime.any { it.contains('biome-api') }) { + throw new RuntimeException('Biome API on runtime classpath') + } + } +} + +check.dependsOn checkClasspaths diff --git a/src/test/resources/projects/mavenLibrary/build.gradle b/src/test/resources/projects/mavenLibrary/build.gradle index ff53dcc3..0001e2b7 100644 --- a/src/test/resources/projects/mavenLibrary/build.gradle +++ b/src/test/resources/projects/mavenLibrary/build.gradle @@ -13,6 +13,8 @@ dependencies { minecraft "com.mojang:minecraft:1.16.5" mappings "net.fabricmc:yarn:1.16.5+build.5:v2" modImplementation "net.fabricmc:fabric-loader:0.11.2" + modCompileOnlyApi fabricApi.module('fabric-biome-api-v1', '0.42.0+1.16') + modRuntimeOnly fabricApi.module('fabric-command-api-v1', '0.42.0+1.16') } processResources {