Fix configuration cache issues with the APT plugin #218
Conversation
This does introduce a few necessary binary breaking changes, but none that should affect consumers if they aren't making direct instances of these objects using the "new" keyword.
| IMPL.configureTasks( | ||
| project, | ||
| compileTaskClass, | ||
| task -> { | ||
| CompileOptions compileOptions = getCompileOptions.apply(task); | ||
| final AptOptions aptOptions = IMPL.createAptOptions(); | ||
| task.getExtensions().add(AptOptions.class, "aptOptions", aptOptions); | ||
| IMPL.configureCompileTask(task, compileOptions, aptOptions); | ||
| }); | ||
| project.afterEvaluate(p -> { | ||
| for (T task : p.getTasks().withType(compileTaskClass)) { | ||
| CompileOptions compileOptions = getCompileOptions.apply(task); | ||
| final AptOptions aptOptions = IMPL.createAptOptions(); | ||
| task.getExtensions().add(AptOptions.class, "aptOptions", aptOptions); | ||
| IMPL.configureCompileTask(task, compileOptions, aptOptions); | ||
| } | ||
| }); |
There was a problem hiding this comment.
While Gradle's own eclipseClasspath task is not currently compatible with the configuration cache, adding to task extensions using this configuration block is disallowed because it could be resolved at execution time. Adding to these tasks' extensions in Project#afterEvaluate guarantees that this doesn't happen, and that your extensions to eclipseClasspath will not hinder the performance of Gradle's task once they update it.
| for (Configuration configuration : factorypathModel.getPlusConfigurations()) { | ||
| for (FileCollection configuration : factorypathModel.getPlusConfigurations()) { | ||
| entries.addAll(configuration.getFiles()); | ||
| } | ||
| for (Configuration configuration : factorypathModel.getMinusConfigurations()) { | ||
| for (FileCollection configuration : factorypathModel.getMinusConfigurations()) { |
There was a problem hiding this comment.
When the configuration cache is enabled, Gradle will automatically convert disallowed types into cache-friendly types that are used at execution. This means that when storing configurations in tasks, at execution, you can only refer to them as the FileCollection type.
| jdtApt.setProcessorOptions( | ||
| () -> | ||
| project | ||
| .getTasks() | ||
| .getByName(mainSourceSet.getCompileJavaTaskName()) | ||
| .getExtensions() | ||
| .getByType(AptPlugin.AptOptions.class) | ||
| .getProcessorArgs()); | ||
| MapProperty<String, ?> processorOptions = objects | ||
| .mapProperty(String.class, Object.class) | ||
| .value(project.getTasks().named(mainSourceSet.getCompileJavaTaskName()).map(task -> | ||
| task.getExtensions().getByType(AptPlugin.AptOptions.class).getProcessorArgs() | ||
| )); | ||
| jdtApt.setProcessorOptions(processorOptions::getOrNull); | ||
| project.afterEvaluate(p -> processorOptions.finalizeValue()); |
There was a problem hiding this comment.
Using a MapProperty object here allows Gradle to finalize its value before task execution. In configuration cache mode, all references to Project in objects are nullified, so this would cause an NPE unless configuration cache was disabled.
|
Published in |
|
It is. I'm targeting Gradle 9 anyway for all our new stuff. |
|
A fix has been published in |
3 participants
This PR fixes Gradle 9 errors and configuration cache issues with the APT plugin. This was done by using
@javax.inject.Injecton some constructors to inject Gradle services into types that could use them, and avoiding the usage oforg.gradle.api.Projectwherever possible.I'll include some comments on the code itself to help explain some of my changes.
We still use this over at Minecraft Forge, so I'd appreciate it if you could review and merge at your earliest convenience.