Skip to content

Commit

Permalink
fix generateDiff task when target version doesn't exist (#671)
Browse files Browse the repository at this point in the history
* 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>
  • Loading branch information
supersaiyansubtlety and github-actions[bot] authored Jan 20, 2025
1 parent 960316d commit c37469f
Show file tree
Hide file tree
Showing 6 changed files with 79 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
}
);
Expand Down
37 changes: 30 additions & 7 deletions buildSrc/src/main/java/quilt/internal/plugin/TargetDiffPlugin.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -39,15 +41,17 @@

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;

/**
* {@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.
* <p>
* The generated {@value DiffDirectoriesTask#DIFF_COMMAND} is useful when reviewing new mappings.
* <p>
Expand Down Expand Up @@ -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");

Expand All @@ -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<String> targetVersion = diffTarget.get().getTargetVersion();
targetVersion.finalizeValue();

if (targetVersion.isPresent()) {
return diffTarget;
} else {
return Collections.emptyList();
}
}));
}
);
}

public Provider<Directory> getTargetsBuildDir() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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";
Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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";
}
Original file line number Diff line number Diff line change
@@ -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}.
* <p>
* 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}.
* <p>
* 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);
}
}

1 comment on commit c37469f

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No difference between head and target.

Please sign in to comment.