Migrate ToolExecBase from JavaExec to DefaultTask

Gradle please get your shit together.

When configuration cache is enabled, log levels passed into
`Task#getLogging`'s standard output captures are discarded. I'm just
going to try using `ExecOperations#javaexec` manually to get what I
want.

Author: Jonathing

gradleutils-shared/src/main/java/net/minecraftforge/gradleutils/shared/EnhancedProblems.java

@@ -223,34 +223,6 @@ final RuntimeException pluginNotYetApplied(Exception e) {
     }
     //endregion
 
-    //region ToolExecBase
-    final void reportToolExecNotEnhanced(Task task) {
-        String className = task.getClass().getSimpleName().replace('.', '-');
-        task.getLogger().warn("WARNING: {} doesn't implement EnhancedTask", className);
-        this.report(String.format("%s-not-enhanced", className.toLowerCase(Locale.ROOT)), String.format("%s doesn't implement EnhancedTask", className), spec -> spec
-            .details("""
-                Implementing subclasses of ToolExecBase should also implement (a subclass of) EnhancedTask.
-                Not doing so will result in global caches being ignored. Please check your implementations.
-                Affected task: %s (%s)""".formatted(task, task.getClass()))
-            .severity(Severity.WARNING)
-            .stackLocation()
-            .solution("Double check your task implementation."));
-    }
-
-    final void reportToolExecEagerArgs(Task task) {
-        String className = task.getClass().getSimpleName().replace('.', '-');
-        task.getLogger().warn("WARNING: {} implementation adds arguments without using addArguments()", className);
-        this.report(String.format("%s-eager-args", className.toLowerCase(Locale.ROOT)), String.format("%s implementation adds arguments without using addArguments()", className), spec -> spec
-            .details("""
-                A ToolExecBase task is eagerly adding arguments using JavaExec#args without using ToolExecBase#addArguments.
-                This may cause unintended behavior as the arguemnts given to the tool will contain the arguments in both of these method calls.
-                Affected task: %s (%s)""".formatted(task, task.getClass()))
-            .severity(Severity.WARNING)
-            .stackLocation()
-            .solution("Use ToolExecBase#addArguments"));
-    }
-    //endregion
-
     //region Utilities
 
     /// A utility method to ensure that a [FileSystemLocation] [Provider] has (its parent) directory created. If the

gradleutils-shared/src/main/java/net/minecraftforge/gradleutils/shared/SharedUtil.java

@@ -15,6 +15,8 @@
 import org.gradle.api.artifacts.Dependency;
 import org.gradle.api.artifacts.FileCollectionDependency;
 import org.gradle.api.artifacts.ModuleVersionSelector;
+import org.gradle.api.logging.LogLevel;
+import org.gradle.api.logging.Logger;
 import org.gradle.api.plugins.ExtensionContainer;
 import org.gradle.api.plugins.JavaPluginExtension;
 import org.gradle.api.provider.Property;
@@ -222,6 +224,14 @@ public List<String> getArgs() {
 
     //region Action Logging
 
+    /// Creates an output stream that logs to the given action.
+    ///
+    /// @param logger The logger to log to
+    /// @return The output stream
+    public static PipedOutputStream toLog(Logger logger, LogLevel level) {
+        return toLog(s -> logger.log(level, s));
+    }
+
     /// Creates an output stream that logs to the given action.
     ///
     /// @param logger The logger to log to

gradleutils-shared/src/main/java/net/minecraftforge/gradleutils/shared/ToolExecBase.java

@@ -4,24 +4,45 @@
  */
 package net.minecraftforge.gradleutils.shared;
 
+import org.codehaus.groovy.runtime.DefaultGroovyMethods;
+import org.gradle.api.DefaultTask;
 import org.gradle.api.Transformer;
+import org.gradle.api.file.ConfigurableFileCollection;
 import org.gradle.api.file.DirectoryProperty;
 import org.gradle.api.file.FileSystemLocation;
 import org.gradle.api.file.FileSystemLocationProperty;
 import org.gradle.api.logging.LogLevel;
+import org.gradle.api.logging.LoggingManager;
+import org.gradle.api.model.ObjectFactory;
 import org.gradle.api.provider.ListProperty;
+import org.gradle.api.provider.MapProperty;
+import org.gradle.api.provider.Property;
 import org.gradle.api.provider.Provider;
+import org.gradle.api.provider.ProviderFactory;
+import org.gradle.api.tasks.Classpath;
+import org.gradle.api.tasks.Console;
 import org.gradle.api.tasks.Input;
+import org.gradle.api.tasks.InputFiles;
 import org.gradle.api.tasks.Internal;
 import org.gradle.api.tasks.JavaExec;
+import org.gradle.api.tasks.Nested;
 import org.gradle.api.tasks.Optional;
+import org.gradle.api.tasks.TaskAction;
+import org.gradle.jvm.toolchain.JavaLauncher;
+import org.gradle.process.ExecOperations;
+import org.gradle.process.ExecResult;
 import org.jetbrains.annotations.MustBeInvokedByOverriders;
+import org.jetbrains.annotations.Nullable;
 import org.jetbrains.annotations.UnknownNullability;
 
 import javax.inject.Inject;
 import java.io.File;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
 import java.util.Locale;
 import java.util.Map;
+import java.util.Objects;
 import java.util.concurrent.Callable;
 
 /// This tool execution task is a template on top of [JavaExec] to make executing [tools][Tool] much easier and more
@@ -32,12 +53,12 @@
 /// should extend from.
 /// @see JavaExec
 /// @see Tool
-public abstract class ToolExecBase<P extends EnhancedProblems> extends JavaExec implements EnhancedTask<P> {
-    private final P problems;
+public abstract class ToolExecBase<P extends EnhancedProblems> extends DefaultTask implements EnhancedTask<P> {
+    private final P problems = this.getObjects().newInstance(this.problemsType());
 
     /// The default tool directory (usage is not required).
-    protected final DirectoryProperty defaultToolDir = this.getObjectFactory().directoryProperty();
-    private final ListProperty<String> additionalArgs = this.getObjectFactory().listProperty(String.class);
+    protected final DirectoryProperty defaultToolDir = this.getObjects().directoryProperty();
+    private final ListProperty<String> additionalArgs = this.getObjects().listProperty(String.class);
 
     /// Additional arguments to use when invoking the tool. Use in configuration instead of [#args].
     ///
@@ -46,6 +67,31 @@ public abstract class ToolExecBase<P extends EnhancedProblems> extends JavaExec
         return this.additionalArgs;
     }
 
+    //region JavaExec
+    protected abstract @InputFiles @Classpath ConfigurableFileCollection getClasspath();
+
+    protected abstract @Input @Optional Property<String> getMainClass();
+
+    protected abstract @Nested Property<JavaLauncher> getJavaLauncher();
+    //endregion
+
+    //region Logging
+    @Deprecated
+    @Override public LoggingManager getLogging() {
+        return super.getLogging();
+    }
+
+    protected abstract @Console Property<LogLevel> getStandardOutputLogLevel();
+
+    protected abstract @Console Property<LogLevel> getStandardErrorLogLevel();
+    //endregion
+
+    protected abstract @Inject ObjectFactory getObjects();
+
+    protected abstract @Inject ProviderFactory getProviders();
+
+    protected abstract @Inject ExecOperations getExecOperations();
+
     /// Creates a new task instance using the given types and tool information.
     ///
     /// @param tool The tool to use for this task
@@ -55,22 +101,19 @@ public abstract class ToolExecBase<P extends EnhancedProblems> extends JavaExec
     /// best practice is to make a single `ToolExec` class for the implementing plugin to use, which other tasks can
     /// extend off of.
     protected ToolExecBase(Tool tool) {
-        this.problems = this.getObjectFactory().newInstance(this.problemsType());
-
         var resolved = this.getTool(tool);
-        this.defaultToolDir.value(
+        SharedUtil.finalizeProperty(this.defaultToolDir.value(
             this.globalCaches().dir(tool.getName().toLowerCase(Locale.ENGLISH)).map(this.ensureFileLocationInternal())
-        );
-
-        this.setClasspath(resolved.getClasspath());
+        ));
 
-        SharedUtil.finalizeProperty(this.defaultToolDir);
+        this.getClasspath().setFrom(resolved.getClasspath());
 
         if (resolved.hasMainClass())
             this.getMainClass().set(resolved.getMainClass());
         this.getJavaLauncher().set(resolved.getJavaLauncher());
 
-        this.getLogging().captureStandardOutput(LogLevel.LIFECYCLE).captureStandardError(LogLevel.ERROR);
+        this.getStandardOutputLogLevel().convention(LogLevel.LIFECYCLE);
+        this.getStandardErrorLogLevel().convention(LogLevel.ERROR);
     }
 
     /// The enhanced problems instance to use for this task.
@@ -87,22 +130,65 @@ private <T extends FileSystemLocation> Transformer<T, T> ensureFileLocationInter
     /// This method should be overridden by subclasses to add arguments to this task via [JavaExec#args]. To preserve
     /// arguments added by superclasses, this method [must be invoked by overriders][MustBeInvokedByOverriders].
     @MustBeInvokedByOverriders
-    protected void addArguments() {
-        this.args(this.getAdditionalArgs().get());
-    }
+    protected void addArguments() { }
+
+    private transient boolean executing = false;
+    private transient @Nullable List<Provider<String>> args;
+    private transient @Nullable List<Provider<String>> jvmArgs;
+    private transient @Nullable Map<String, String> environment;
+    private transient @Nullable Map<String, String> systemProperties;
 
     /// @implNote Not invoking this method from an overriding method *will* result in the tool never being executed and
     /// [#addArguments()] never being run.
-    @Override
-    public void exec() {
-        if (!this.getArgs().isEmpty())
-            this.getProblems().reportToolExecEagerArgs(this);
+    @TaskAction
+    protected ExecResult exec() {
+        this.executing = true;
+        this.args = new ArrayList<>();
+        this.jvmArgs = new ArrayList<>();
+        this.environment = new HashMap<>();
+        this.systemProperties = new HashMap<>();
 
         this.addArguments();
+        this.args(this.getAdditionalArgs().get());
 
-        this.getLogger().info("{} {}", this.getClasspath().getAsPath(), String.join(" ", this.getArgs()));
+        var args = DefaultGroovyMethods.collect(this.args, Closures.<Provider<String>, String>function(Provider::get));
+        var jvmArgs = DefaultGroovyMethods.collect(this.jvmArgs, Closures.<Provider<String>, String>function(Provider::get));
+        this.getLogger().info("{} {}", this.getClasspath().getAsPath(), String.join(" ", args));
 
-        super.exec();
+        var stdOutLevel = this.getStandardOutputLogLevel().get();
+        var stdErrLevel = this.getStandardErrorLogLevel().get();
+        return this.getExecOperations().javaexec(spec -> {
+            spec.setClasspath(this.getClasspath());
+            spec.getMainClass().set(this.getMainClass());
+            spec.setExecutable(this.getJavaLauncher().get().getExecutablePath().getAsFile().getAbsolutePath());
+            spec.setArgs(args);
+            spec.setJvmArgs(jvmArgs);
+            spec.setEnvironment(this.environment);
+            spec.setSystemProperties(this.systemProperties);
+
+            spec.setStandardOutput(SharedUtil.toLog(this.getLogger(), stdOutLevel));
+            spec.setErrorOutput(SharedUtil.toLog(this.getLogger(), stdErrLevel));
+        });
+    }
+
+    protected final void args(Object... args) {
+        try {
+            for (var arg : args) {
+                Objects.requireNonNull(this.args).add(this.getProviders().provider(arg::toString));
+            }
+        } catch (NullPointerException e) {
+            throw new IllegalStateException("ToolExecBase#jvmArgs can only be called inside of #addArguments()", e);
+        }
+    }
+
+    protected final void args(Iterable<?> args) {
+        try {
+            for (var arg : args) {
+                Objects.requireNonNull(this.args).add(this.getProviders().provider(arg::toString));
+            }
+        } catch (NullPointerException e) {
+            throw new IllegalStateException("ToolExecBase#jvmArgs can only be called inside of #addArguments()", e);
+        }
     }
 
     /// Adds each file to the arguments preceded by the given argument. Designed to work well with
@@ -152,9 +238,45 @@ protected final void args(Map<?, ?> args) {
             var key = entry.getKey();
             var value = entry.getValue();
             this.args(
-                key instanceof Provider<?> provider ? provider.map(Object::toString).get() : this.getProviderFactory().provider(key::toString).get(),
-                value instanceof Provider<?> provider ? (provider instanceof FileSystemLocationProperty<?> file ? file.getLocationOnly() : provider) : this.getProviderFactory().provider(() -> value)
+                key instanceof Provider<?> provider ? provider.map(Object::toString).get() : this.getProviders().provider(key::toString).get(),
+                value instanceof Provider<?> provider ? (provider instanceof FileSystemLocationProperty<?> file ? file.getLocationOnly() : provider) : this.getProviders().provider(() -> value)
             );
         }
     }
+
+    protected final void jvmArgs(Object... args) {
+        try {
+            for (var arg : args) {
+                Objects.requireNonNull(this.jvmArgs).add(this.getProviders().provider(arg::toString));
+            }
+        } catch (NullPointerException e) {
+            throw new IllegalStateException("ToolExecBase#jvmArgs can only be called inside of #addArguments()", e);
+        }
+    }
+
+    protected final void jvmArgs(Iterable<?> args) {
+        try {
+            for (var arg : args) {
+                Objects.requireNonNull(this.jvmArgs).add(this.getProviders().provider(arg::toString));
+            }
+        } catch (NullPointerException e) {
+            throw new IllegalStateException("ToolExecBase#jvmArgs can only be called inside of #addArguments()", e);
+        }
+    }
+
+    protected final void environment(String key, String value) {
+        try {
+            Objects.requireNonNull(this.environment).put(key, value);
+        } catch (NullPointerException e) {
+            throw new IllegalStateException("ToolExecBase#environment can only be called inside of #addArguments()", e);
+        }
+    }
+
+    protected final void systemProperty(String key, String value) {
+        try {
+            Objects.requireNonNull(this.systemProperties).put(key, value);
+        } catch (NullPointerException e) {
+            throw new IllegalStateException("ToolExecBase#systemProperty can only be called inside of #addArguments()", e);
+        }
+    }
 }