From c37469f4a4a921a22c1b8692c69fc7e3250b6f2c Mon Sep 17 00:00:00 2001 From: Will Date: Mon, 20 Jan 2025 15:10:04 -0800 Subject: [PATCH] fix `generateDiff` task when target version doesn't exist (#671) * rename task generateDiff -> diffTarget make diffTarget a TargetVersionConsumingTask * add lazilyDiffTarget task * make DiffTargetTask and DecompileTargetVineflowerTask implement UnpickVersionsMatchConsumingTask as they depend on other UnpickVersionsMatchConsumingTasks give lazilyDiffTarget a custom type that implements UnpickVersionsMatchConsumingTask make lazilyDiffTarget's conditional dependency use a provider to make it lazier use try-with-resources when writing the output in DiffDirectoriesTask --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- .../plugin/ProcessMappingsPlugin.java | 2 +- .../internal/plugin/TargetDiffPlugin.java | 37 +++++++++++++++---- .../diff/DecompileTargetVineflowerTask.java | 2 +- .../task/diff/DiffDirectoriesTask.java | 23 ++++++------ .../internal/task/diff/DiffTargetTask.java | 11 ++++++ .../task/diff/LazilyDiffTargetTask.java | 25 +++++++++++++ 6 files changed, 79 insertions(+), 21 deletions(-) create mode 100644 buildSrc/src/main/java/quilt/internal/task/diff/DiffTargetTask.java create mode 100644 buildSrc/src/main/java/quilt/internal/task/diff/LazilyDiffTargetTask.java diff --git a/buildSrc/src/main/java/quilt/internal/plugin/ProcessMappingsPlugin.java b/buildSrc/src/main/java/quilt/internal/plugin/ProcessMappingsPlugin.java index 8ba24273bd..bb925aefa6 100644 --- a/buildSrc/src/main/java/quilt/internal/plugin/ProcessMappingsPlugin.java +++ b/buildSrc/src/main/java/quilt/internal/plugin/ProcessMappingsPlugin.java @@ -133,7 +133,7 @@ protected ProcessMappingsExtension applyImpl(@NotNull Project project) { insertAutoGeneratedMappings.flatMap(AddProposedMappingsTask::getOutputMappings) )); - // TODO LATER move this to build/ once generate-diff.yml uses generateDiff + // TODO LATER move this to build/ once generate-diff.yml uses lazilyDiffTarget task.getOutput().convention(this.getProjectDir().dir("namedSrc")); } ); diff --git a/buildSrc/src/main/java/quilt/internal/plugin/TargetDiffPlugin.java b/buildSrc/src/main/java/quilt/internal/plugin/TargetDiffPlugin.java index 6a4397f292..fcbf567876 100644 --- a/buildSrc/src/main/java/quilt/internal/plugin/TargetDiffPlugin.java +++ b/buildSrc/src/main/java/quilt/internal/plugin/TargetDiffPlugin.java @@ -27,8 +27,10 @@ import quilt.internal.task.decompile.DecompileVineflowerTask; import quilt.internal.task.diff.DecompileTargetVineflowerTask; import quilt.internal.task.diff.DiffDirectoriesTask; +import quilt.internal.task.diff.DiffTargetTask; import quilt.internal.task.diff.DownloadTargetMappingJarTask; import quilt.internal.task.diff.ExtractTargetMappingJarTask; +import quilt.internal.task.diff.LazilyDiffTargetTask; import quilt.internal.task.diff.RemapTargetMinecraftJarTask; import quilt.internal.task.diff.RemapTargetUnpickDefinitionsTask; import quilt.internal.task.diff.TargetVersionConsumingTask; @@ -39,6 +41,7 @@ import java.io.FileReader; import java.io.IOException; +import java.util.Collections; import static quilt.internal.constants.Constants.UNPICK_NAME; import static quilt.internal.task.build.MappingsV2JarTask.JAR_MAPPINGS_PATH; @@ -46,8 +49,9 @@ /** * {@linkplain TaskContainer#register Registers} tasks that download the latest published Quilt Mappings for the current * {@link QuiltMappingsExtension#getMinecraftVersion() minecraftVersion} so the - * {@value DiffDirectoriesTask#GENERATE_DIFF_TASK_NAME} task can {@value DiffDirectoriesTask#DIFF_COMMAND} - * them with this project's mappings. + * {@value DiffTargetTask#DIFF_TARGET_TASK_NAME} + * (or {@value LazilyDiffTargetTask#LAZILY_DIFF_TARGET_TASK_NAME}) task can + * {@value DiffDirectoriesTask#DIFF_COMMAND} them with this project's mappings. *

* The generated {@value DiffDirectoriesTask#DIFF_COMMAND} is useful when reviewing new mappings. *

@@ -231,15 +235,14 @@ public void apply(@NotNull Project project) { .map(dest -> dest.file(JAR_MAPPINGS_PATH)) )); - // TODO LATER move this to build/ once generate-diff.yml uses generateDiff + // TODO LATER move this to build/ once generate-diff.yml uses lazilyDiffTarget task.getOutput().convention(this.getProjectDir().dir("namedTargetSrc")); } ); - // TODO LATER use this in generate-diff.yml - tasks.register( - DiffDirectoriesTask.GENERATE_DIFF_TASK_NAME, - DiffDirectoriesTask.class, + final var diffTarget = tasks.register( + DiffTargetTask.DIFF_TARGET_TASK_NAME, + DiffTargetTask.class, task -> { task.getAdditionalArgs().add("-bur"); @@ -250,6 +253,26 @@ public void apply(@NotNull Project project) { task.getDest().convention(this.getBuildDir().file("target.diff")); } ); + + // TODO LATER use this in generate-diff.yml + tasks.register( + LazilyDiffTargetTask.LAZILY_DIFF_TARGET_TASK_NAME, + LazilyDiffTargetTask.class, + task -> { + // This provider is safe to get when dependOn is evaluated because it comes from a ValueSource. + // Configuring diffTarget's targetVersion to a provider mapped from a task output would break this. + task.dependsOn(this.getProviders().provider(() -> { + final Property targetVersion = diffTarget.get().getTargetVersion(); + targetVersion.finalizeValue(); + + if (targetVersion.isPresent()) { + return diffTarget; + } else { + return Collections.emptyList(); + } + })); + } + ); } public Provider getTargetsBuildDir() { diff --git a/buildSrc/src/main/java/quilt/internal/task/diff/DecompileTargetVineflowerTask.java b/buildSrc/src/main/java/quilt/internal/task/diff/DecompileTargetVineflowerTask.java index e2e91b4ef9..5694952758 100644 --- a/buildSrc/src/main/java/quilt/internal/task/diff/DecompileTargetVineflowerTask.java +++ b/buildSrc/src/main/java/quilt/internal/task/diff/DecompileTargetVineflowerTask.java @@ -8,7 +8,7 @@ * @see TargetDiffPlugin TargetDiffPlugin's configureEach */ public abstract class DecompileTargetVineflowerTask extends DecompileVineflowerTask implements - TargetVersionConsumingTask { + UnpickVersionsMatchConsumingTask { /** * {@linkplain TaskContainer#register Registered} by {@link TargetDiffPlugin}. */ diff --git a/buildSrc/src/main/java/quilt/internal/task/diff/DiffDirectoriesTask.java b/buildSrc/src/main/java/quilt/internal/task/diff/DiffDirectoriesTask.java index c162e838b4..7faf4b487b 100644 --- a/buildSrc/src/main/java/quilt/internal/task/diff/DiffDirectoriesTask.java +++ b/buildSrc/src/main/java/quilt/internal/task/diff/DiffDirectoriesTask.java @@ -14,12 +14,11 @@ import org.gradle.api.tasks.PathSensitive; import org.gradle.api.tasks.PathSensitivity; import org.gradle.api.tasks.TaskAction; -import org.gradle.api.tasks.TaskContainer; import org.gradle.api.tasks.options.Option; import quilt.internal.constants.Groups; -import quilt.internal.plugin.TargetDiffPlugin; import java.io.File; +import java.io.FileNotFoundException; import java.io.FileOutputStream; import java.io.IOException; import java.util.ArrayList; @@ -39,11 +38,6 @@ */ @CacheableTask public abstract class DiffDirectoriesTask extends Exec { - /** - * {@linkplain TaskContainer#register Registered} by {@link TargetDiffPlugin}. - */ - public static final String GENERATE_DIFF_TASK_NAME = "generateDiff"; - public static final String DIFF_COMMAND = "diff"; private static final String DIFF_COMMAND_PHRASE = DIFF_COMMAND + " command"; @@ -115,19 +109,24 @@ public DiffDirectoriesTask() { @Override @TaskAction public void exec() { + final File dest; try { - final File dest = this.getDest().get().getAsFile(); + dest = this.getDest().get().getAsFile(); dest.getParentFile().mkdirs(); dest.createNewFile(); - - this.setStandardOutput(new FileOutputStream(dest.getAbsolutePath())); } catch (IOException e) { - throw new GradleException("Failed to access destination file", e); + throw new GradleException("Failed to create destination file", e); } - super.exec(); + try (var out = new FileOutputStream(dest.getAbsolutePath())) { + this.setStandardOutput(out); + + super.exec(); + } catch (IOException e) { + throw new GradleException("Failed to write to destination", e); + } final int exitValue = this.getExecutionResult().get().getExitValue(); switch (exitValue) { diff --git a/buildSrc/src/main/java/quilt/internal/task/diff/DiffTargetTask.java b/buildSrc/src/main/java/quilt/internal/task/diff/DiffTargetTask.java new file mode 100644 index 0000000000..9bfe431418 --- /dev/null +++ b/buildSrc/src/main/java/quilt/internal/task/diff/DiffTargetTask.java @@ -0,0 +1,11 @@ +package quilt.internal.task.diff; + +import org.gradle.api.tasks.TaskContainer; +import quilt.internal.plugin.TargetDiffPlugin; + +public abstract class DiffTargetTask extends DiffDirectoriesTask implements UnpickVersionsMatchConsumingTask { + /** + * {@linkplain TaskContainer#register Registered} by {@link TargetDiffPlugin}. + */ + public static final String DIFF_TARGET_TASK_NAME = "diffTarget"; +} diff --git a/buildSrc/src/main/java/quilt/internal/task/diff/LazilyDiffTargetTask.java b/buildSrc/src/main/java/quilt/internal/task/diff/LazilyDiffTargetTask.java new file mode 100644 index 0000000000..b29efcbfce --- /dev/null +++ b/buildSrc/src/main/java/quilt/internal/task/diff/LazilyDiffTargetTask.java @@ -0,0 +1,25 @@ +package quilt.internal.task.diff; + +import org.gradle.api.DefaultTask; +import org.gradle.api.provider.Provider; +import org.gradle.api.tasks.TaskContainer; +import quilt.internal.constants.Groups; +import quilt.internal.plugin.TargetDiffPlugin; + +public abstract class LazilyDiffTargetTask extends DefaultTask implements UnpickVersionsMatchConsumingTask { + /** + * {@linkplain TaskContainer#register Registered} by {@link TargetDiffPlugin}. + *

+ * A wrapper for {@value DiffTargetTask#DIFF_TARGET_TASK_NAME} that conditionally depends on + * {@value DiffTargetTask#DIFF_TARGET_TASK_NAME} only if its + * {@link TargetVersionConsumingTask#getTargetVersion() targetVersion} {@link Provider#isPresent() isPresent}. + *

+ * This is a hack to prevent unnecessarily generating sources using local mappings when + * {@link TargetVersionConsumingTask#getTargetVersion() targetVersion} isn't present. + */ + public static final String LAZILY_DIFF_TARGET_TASK_NAME = "lazilyDiffTarget"; + + public LazilyDiffTargetTask() { + this.setGroup(Groups.DIFF); + } +}