Implement OOM detection in decompiler execution and update related configurations

.github/workflows/test-prs.yml

@@ -1,5 +1,5 @@
 name: Test PRs
-run-name: Tests for PR ${{ github.event.pull_request.number }}
+run-name: Tests for ${{ github.event.pull_request.number || github.event.inputs.gradle-version }}
 
 on:
   pull_request:
@@ -8,17 +8,40 @@ on:
       - opened
       - ready_for_review
       - reopened
+  workflow_dispatch:
+    inputs:
+      gradle-version:
+        description: 'Gradle version to use (required for manual run)'
+        required: true
+        default: ''
 
 concurrency:
   group: ci-${{ github.head_ref || github.run_id }}
   cancel-in-progress: true
 
 jobs:
+  set-gradle-version:
+    name: Set Gradle Version
+    runs-on: ubuntu-latest
+    outputs:
+      gradle-version: ${{ steps.set-version.outputs.gradle-version }}
+    steps:
+      - name: Determine gradle version
+        id: set-version
+        run: |
+          if [ "${{ github.event_name }}" = "pull_request" ]; then
+            echo "gradle-version=default" >> "$GITHUB_OUTPUT"
+          else
+            echo "gradle-version=${{ github.event.inputs.gradle-version }}" >> "$GITHUB_OUTPUT"
+          fi
+
   setup:
     name: Setup
     runs-on: ubuntu-latest
+    needs: set-gradle-version
     outputs:
       tests-to-run: ${{ steps.test.outputs.tests-to-run }}
+      gradle-version: ${{ needs.set-gradle-version.outputs.gradle-version }}
     steps:
       - name: Checkout repository
         uses: neoforged/actions/checkout@main
@@ -42,10 +65,12 @@ jobs:
           #!/bin/bash
           ./.github/scripts/collect-tests.sh
 
-
   build:
     name: Build
     runs-on: ubuntu-latest
+    needs: set-gradle-version
+    env:
+      GRADLE_VERSION: ${{ needs.set-gradle-version.outputs.gradle-version }}
     steps:
       - name: Checkout repository
         uses: neoforged/actions/checkout@main
@@ -60,18 +85,20 @@ jobs:
         uses: gradle/gradle-build-action@v3
 
       - name: Build
-        run: ./gradlew --info -s -x assemble
+        run: ./gradlew --info -s -x assemble -Ptraining-wheels.gradle-version=${{ env.GRADLE_VERSION }}
 
   test:
     name: "${{ matrix.test.displayName }} (${{ matrix.os }})"
     runs-on: "${{ matrix.os }}-latest"
-    needs: setup
+    needs: [setup, set-gradle-version]
     strategy:
       max-parallel: 15
       fail-fast: false
       matrix:
         test: ${{ fromJSON(needs.setup.outputs.tests-to-run) }}
         os: [ubuntu, windows, macos]
+    env:
+      GRADLE_VERSION: ${{ needs.set-gradle-version.outputs.gradle-version }}
     steps:
       - name: Checkout repository
         uses: neoforged/actions/checkout@main
@@ -87,11 +114,11 @@ jobs:
 
       - name: Test
         if: ${{ matrix.test.filter != null }}
-        run: ./gradlew --info -s ${{ matrix.test.path }} --tests "${{ matrix.test.filter }}"
+        run: ./gradlew --info -s ${{ matrix.test.path }} --tests "${{ matrix.test.filter }}" "-Ptraining-wheels.gradle-version=${{ env.GRADLE_VERSION }}"
 
       - name: Test
         if: ${{ matrix.test.filter == null }}
-        run: ./gradlew --info -s ${{ matrix.test.path }}
+        run: ./gradlew --info -s ${{ matrix.test.path }} "-Ptraining-wheels.gradle-version=${{ env.GRADLE_VERSION }}"
 
       # Always upload test results
       - name: Merge Test Reports
@@ -123,7 +150,7 @@ jobs:
         run: |
           # We need to remove all invalid characters from the test name:
           # Invalid characters include:  Double quote ", Colon :, Less than <, Greater than >, Vertical bar |, Asterisk *, Question mark ?, Carriage return \r, Line feed \n, Backslash \, Forward slash /
-          NAME=$(echo "${{ matrix.test.displayName }}" | tr '":<>|*?\\/' '-' | tr -d ' ')          
+          NAME=$(echo "${{ matrix.test.displayName }}" | tr '":<>|*?\\/' '-' | tr -d ' ')
           # Check if the GITHUB_OUTPUT is set
           echo "Determined name to be $NAME-${{ matrix.os }}"
           if [ -z "$GITHUB_OUTPUT" ]; then
@@ -191,4 +218,3 @@ jobs:
         with:
           script: |
             core.setFailed('Test build failure!')
-

README.md

@@ -486,17 +486,17 @@ subsystems {
 
 ## Advanced Settings
 
-### Override Decompiler Settings
+### <a id="common-decompiler-settings" /> Override Decompiler Settings
 
 The settings used by the decompiler when preparing Minecraft dependencies can be overridden
 using [Gradle properties](https://docs.gradle.org/current/userguide/project_properties.html).
 This can be useful to run NeoGradle on lower-end machines, at the cost of slower build times.
 
-| Property                                     | Description                                                                                                                |
-|----------------------------------------------|----------------------------------------------------------------------------------------------------------------------------|
-| `neogradle.subsystems.decompiler.maxMemory`  | How much heap memory is given to the decompiler. Can be specified either in gigabyte (`4g`) or megabyte (`4096m`).         |
-| `neogradle.subsystems.decompiler.maxThreads` | By default the decompiler uses all available CPU cores. This setting can be used to limit it to a given number of threads. |
-| `neogradle.subsystems.decompiler.logLevel`   | Can be used to override the [decompiler loglevel](https://vineflower.org/usage/#cmdoption-log).                            |
+| Property                                     | Description                                                                                                                                                                                                                     |
+|----------------------------------------------|---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
+| `neogradle.subsystems.decompiler.maxMemory`  | How much heap memory is given to the decompiler. Can be specified either in gigabyte (`4g`) or megabyte (`4096m`).                                                                                                              |
+| `neogradle.subsystems.decompiler.maxThreads` | By default the decompiler uses all available CPU cores. This setting can be used to limit it to a given number of threads, the lower the amount of threads the less memory is consumed. If set to 0 then the limit is disabled. |
+| `neogradle.subsystems.decompiler.logLevel`   | Can be used to override the [decompiler loglevel](https://vineflower.org/usage/#cmdoption-log).                                                                                                                                 |
 
 ### Override Recompiler Settings
 

build.gradle

@@ -228,6 +228,12 @@ subprojects.forEach { subProject ->
             classpath = evalSubProject.sourceSets.functionalTest.runtimeClasspath
 
             it.extensions.add('test-source-set', evalSubProject.sourceSets.functionalTest)
+
+            if (evalSubProject.rootProject.getProperties().get("training-wheels.gradle-version") != null)
+                it.systemProperty(
+                        "training-wheels.gradle-version",
+                        evalSubProject.rootProject.getProperties().get("training-wheels.gradle-version")
+                )
         }
 
         //Wire them up so they run as part of the check task (and as such through build, but not through test!)

common/src/main/java/net/neoforged/gradle/common/runtime/tasks/DecompilerExecute.java

@@ -0,0 +1,112 @@
+package net.neoforged.gradle.common.runtime.tasks;
+
+import net.neoforged.gradle.common.extensions.problems.IProblemReporter;
+import net.neoforged.gradle.dsl.common.tasks.Execute;
+import org.gradle.api.GradleException;
+import org.gradle.api.problems.Problems;
+import org.gradle.api.tasks.CacheableTask;
+import org.gradle.api.tasks.Internal;
+
+import javax.inject.Inject;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.OutputStream;
+
+@CacheableTask
+public abstract class DecompilerExecute extends DefaultExecute
+{
+
+    private final OOMDetector detector = new OOMDetector();
+
+    @Override
+    public OutputStream createErrorOutputStream()
+    {
+        return new BifurcatingOutputStream(
+            super.createErrorOutputStream(),
+            new OOMDetectorStream(this.detector)
+        );
+    }
+
+    @Internal
+    public OOMDetector getDetector()
+    {
+        return detector;
+    }
+
+    @Override
+    public void doExecute() throws Exception
+    {
+        try {
+            super.doExecute();
+        } catch (Exception ex) {
+            detectError(false);
+            throw ex;
+        }
+
+        detectError(true);
+    }
+
+    private void detectError(boolean throwError)
+    {
+        if (getDetector().failed()) {
+            //We failed, so we can access the project now, to get the reporter
+            //We should not need to care about the config cache here,
+            getProject().getExtensions().getByType(IProblemReporter.class)
+                .reporting(problem -> {
+                    problem.contextualLabel("decompiler")
+                        .id("decompiler", "memory")
+                        .details("The Decompiler could not successfully decompile Minecraft because it ran out of memory. Modify your runtime configuration and the decompiler subsystems configuration, and try again.")
+                        .solution("Either increase the memory allowance for the decompiler, or reduce the concurrency on the decompiler to reduce memory consumption.")
+                        .section("common-decompiler-settings");
+                }, getLogger());
+
+            if (throwError) {
+                throw new GradleException("The decompiler failed to run. It ran out of memory.");
+            }
+        }
+    }
+
+    public static final class OOMDetector {
+        private final StringBuilder resultBuilder = new StringBuilder();
+
+        private OOMDetector() {
+        }
+
+        private void addLog(String log) {
+            resultBuilder.append(log);
+        }
+
+        public boolean failed() {
+            return this.resultBuilder.toString().contains("java.lang.OutOfMemoryError");
+        }
+    }
+
+    public static final class OOMDetectorStream extends OutputStream {
+        private final OOMDetector detector;
+        private final ByteArrayOutputStream collectionDelegate = new ByteArrayOutputStream();
+
+        public OOMDetectorStream(final OOMDetector detector) {this.detector = detector;}
+
+        @Override
+        public void write(final int b) throws IOException
+        {
+            collectionDelegate.write(b);
+        }
+
+        @Override
+        public void flush() throws IOException
+        {
+            super.flush();
+            collectionDelegate.flush();
+        }
+
+        @Override
+        public void close() throws IOException
+        {
+            super.close();
+            collectionDelegate.close();
+
+            this.detector.addLog(collectionDelegate.toString());
+        }
+    }
+}

common/src/main/java/net/neoforged/gradle/common/runtime/tasks/RecompileSourceJar.java

@@ -46,10 +46,6 @@ public abstract class RecompileSourceJar extends JavaCompile implements Runtime
     public RecompileSourceJar() {
         super();
 
-        //We use a custom instance here that marks the sourcepath as an incremental field, allowing us to provide the compiler
-        //with all required elements directly while keeping incremental compile support for II.
-        ReflectionUtils.setFinalFieldUnchecked(this, "compileOptions", getObjectFactory().newInstance(RecompileOptions.class));
-
         arguments = getObjectFactory().newInstance(RuntimeArgumentsImpl.class, getProviderFactory());
         multiArguments = getObjectFactory().newInstance(RuntimeMultiArgumentsImpl.class, getProviderFactory());
 
@@ -90,8 +86,8 @@ public RecompileSourceJar() {
         getOptions().setWarnings(false);
         getOptions().setVerbose(false);
         getOptions().setDeprecation(false);
-        getOptions().setIncremental(true);
-        getOptions().getIncrementalAfterFailure().set(true);
+        getOptions().setIncremental(false);
+        getOptions().getIncrementalAfterFailure().set(false);
 
         setSource(getCompileFileRoot());
 
@@ -230,25 +226,4 @@ public Provider<FileTree> getOutputAsTree()
     {
         return getOutput().map(it -> getArchiveOperations().zipTree(it));
     }
-
-    public static abstract class RecompileOptions extends CompileOptions {
-
-        @Inject
-        public RecompileOptions(final ObjectFactory objectFactory)
-        {
-            super(objectFactory);
-        }
-
-        @Incremental
-        @Optional
-        @IgnoreEmptyDirectories
-        @PathSensitive(PathSensitivity.RELATIVE)
-        @InputFiles
-        @ToBeReplacedByLazyProperty
-        @Override
-        public @Nullable FileCollection getSourcepath()
-        {
-            return super.getSourcepath();
-        }
-    }
 }

common/src/main/java/net/neoforged/gradle/common/util/DependencyCollectorInjector.java

@@ -0,0 +1,12 @@
+package net.neoforged.gradle.common.util;
+
+import org.gradle.api.artifacts.dsl.DependencyCollector;
+
+import javax.inject.Inject;
+
+public interface DependencyCollectorInjector
+{
+
+    @Inject
+    public DependencyCollector dependencyCollector();
+}

common/src/main/java/net/neoforged/gradle/common/util/ReflectionUtils.java

@@ -1,6 +1,9 @@
 package net.neoforged.gradle.common.util;
 
-import java.lang.reflect.Field;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Collectors;
 
 public class ReflectionUtils {
     /**
@@ -44,4 +47,25 @@ public static void setFinalFieldUnchecked(Object target, String fieldName, Objec
             throw new RuntimeException("Failed to set final field '" + fieldName + "'", e);
         }
     }
+
+    public static void setFinalFieldUncheckedWithAlternatives(Object target, Object value, String... fieldNames) {
+        final List<ReflectiveOperationException> exceptions = new ArrayList<>(fieldNames.length);
+        for (final String fieldName : fieldNames)
+        {
+            try {
+                setFinalField(target, fieldName, value);
+                return;
+            } catch (ReflectiveOperationException e) {
+                exceptions.add(e);
+            }
+        }
+
+        final RuntimeException ex = new RuntimeException("Failed to set final field '" + String.join(", ", fieldNames) + "'");
+        for (final ReflectiveOperationException exception : exceptions)
+        {
+            ex.addSuppressed(exception);
+        }
+
+        throw ex;
+    }
 }

common/src/main/java/net/neoforged/gradle/common/util/ToolUtilities.java

@@ -5,18 +5,13 @@
 import net.neoforged.gradle.util.ModuleDependencyUtils;
 import org.gradle.api.Project;
 import org.gradle.api.artifacts.Configuration;
-import org.gradle.api.artifacts.ConfigurationContainer;
 import org.gradle.api.artifacts.Dependency;
 import org.gradle.api.artifacts.ResolvedArtifact;
-import org.gradle.api.artifacts.dsl.Dependencies;
 import org.gradle.api.artifacts.dsl.DependencyCollector;
-import org.gradle.api.artifacts.dsl.DependencyHandler;
 import org.gradle.api.artifacts.result.ResolvedArtifactResult;
-import org.gradle.api.file.RegularFileProperty;
 import org.gradle.api.provider.Provider;
 
 import java.io.File;
-import java.util.List;
 import java.util.function.Function;
 import java.util.function.Supplier;
 
@@ -41,7 +36,8 @@ public static Provider<File> resolveTool(final Project project, final Provider<S
         //If we were to use the provider directly, for example via map, the lambda would need to capture
         //the project that converts the string to a dependency.
         //This breaks the configuration cache as Projects can not be serialized.
-        final DependencyCollector collector = project.getObjects().dependencyCollector();
+        final DependencyCollectorInjector inject = project.getObjects().newInstance(DependencyCollectorInjector.class);
+        final DependencyCollector collector = inject.dependencyCollector();
         collector.add(tool.map(project.getDependencies()::create));
         final Configuration config = ConfigurationUtils.temporaryUnhandledConfiguration(
                 project.getConfigurations(),