fix: remaining configuration cache issues (#1245)

* fix: remaining configuration cache issues * fix: broken serialization of NamedDomainObjectFactory on Gradle 7
spotbugs · Dec 3, 2024 · e8ba804 · e8ba804
1 parent b26219a
commit e8ba804
Show file tree

Hide file tree

Showing 5 changed files with 35 additions and 21 deletions.
diff --git a/api/spotbugs-gradle-plugin.api b/api/spotbugs-gradle-plugin.api
@@ -108,6 +108,7 @@ public abstract class com/github/spotbugs/snom/SpotBugsTask : org/gradle/api/Def
 	public final fun getClasses ()Lorg/gradle/api/file/FileCollection;
 	public abstract fun getEffort ()Lorg/gradle/api/provider/Property;
 	public abstract fun getExcludeFilter ()Lorg/gradle/api/file/RegularFileProperty;
+	public abstract fun getExecOps ()Lorg/gradle/process/ExecOperations;
 	public abstract fun getExtraArgs ()Lorg/gradle/api/provider/ListProperty;
 	public fun getIgnoreFailures ()Z
 	public abstract fun getIncludeFilter ()Lorg/gradle/api/file/RegularFileProperty;

diff --git a/src/functionalTest/groovy/com/github/spotbugs/snom/BaseFunctionalTest.groovy b/src/functionalTest/groovy/com/github/spotbugs/snom/BaseFunctionalTest.groovy
@@ -29,7 +29,7 @@ abstract class BaseFunctionalTest extends Specification {
         return new TestGradleRunner()
                 .withGradleVersion(gradleVersion)
                 .withProjectDir(rootDir)
-                .withArguments('--info', '--stacktrace', '--warning-mode=fail')
+                .withArguments('--configuration-cache', '--info', '--stacktrace', '--warning-mode=fail')
                 .withTestKitDir(testKitDir)
                 .forwardOutput()
                 .withPluginClasspath()

diff --git a/src/functionalTest/groovy/com/github/spotbugs/snom/CacheabilityFunctionalTest.groovy b/src/functionalTest/groovy/com/github/spotbugs/snom/CacheabilityFunctionalTest.groovy
@@ -35,7 +35,7 @@ class CacheabilityFunctionalTest extends BaseFunctionalTest {
 
         when:
         BuildResult result = gradleRunner
-                .withArguments(':spotbugsMain', '--configuration-cache')
+                .withArguments(':spotbugsMain')
                 .build()
 
         then:
@@ -44,7 +44,7 @@ class CacheabilityFunctionalTest extends BaseFunctionalTest {
 
         when:
         BuildResult resultOfCachedBuild = gradleRunner
-                .withArguments(':spotbugsMain', '--configuration-cache')
+                .withArguments(':spotbugsMain')
                 .build()
         then:
         resultOfCachedBuild.task(":spotbugsMain").outcome == TaskOutcome.UP_TO_DATE
@@ -156,7 +156,7 @@ class CacheabilityFunctionalTest extends BaseFunctionalTest {
 
         when:
         def result = gradleRunner
-                .withArguments(':spotbugsMain', '--configuration-cache')
+                .withArguments(':spotbugsMain')
                 .build()
 
         then:

diff --git a/src/main/kotlin/com/github/spotbugs/snom/SpotBugsTask.kt b/src/main/kotlin/com/github/spotbugs/snom/SpotBugsTask.kt
@@ -24,6 +24,7 @@ import org.gradle.api.Action
 import org.gradle.api.DefaultTask
 import org.gradle.api.InvalidUserDataException
 import org.gradle.api.NamedDomainObjectContainer
+import org.gradle.api.NamedDomainObjectFactory
 import org.gradle.api.file.ConfigurableFileCollection
 import org.gradle.api.file.DirectoryProperty
 import org.gradle.api.file.FileCollection
@@ -48,6 +49,7 @@ import org.gradle.api.tasks.TaskAction
 import org.gradle.api.tasks.VerificationTask
 import org.gradle.jvm.toolchain.JavaLauncher
 import org.gradle.jvm.toolchain.JavaToolchainService
+import org.gradle.process.ExecOperations
 import org.gradle.workers.WorkerExecutor
 import org.slf4j.LoggerFactory
 
@@ -93,6 +95,9 @@ abstract class SpotBugsTask : DefaultTask(), VerificationTask {
     @get:Inject
     abstract val workerExecutor: WorkerExecutor
 
+    @get:Inject
+    abstract val execOps: ExecOperations
+
     @Input
     override fun getIgnoreFailures(): Boolean = ignoreFailures.get()
 
@@ -304,17 +309,26 @@ abstract class SpotBugsTask : DefaultTask(), VerificationTask {
 
     init {
         val objects = project.objects
-        reports = objects.domainObjectContainer(SpotBugsReport::class.java) { name: String ->
-            when (name) {
-                "html" -> objects.newInstance(SpotBugsHtmlReport::class.java, name, objects, this)
-                "xml" -> objects.newInstance(SpotBugsXmlReport::class.java, name, objects, this)
-                "text" -> objects.newInstance(SpotBugsTextReport::class.java, name, objects, this)
-                "sarif" -> objects.newInstance(SpotBugsSarifReport::class.java, name, objects, this)
-                else -> throw InvalidUserDataException("$name is invalid as the report name")
-            }.also {
-                (outputs as org.gradle.api.tasks.TaskOutputs).file(it.outputLocation)
-            }
-        }
+        val taskRef = this
+        reports = objects.domainObjectContainer(
+            SpotBugsReport::class.java,
+            // This anonymous object is necessary
+            // Otherwise the serialization of this lambda is broken with config-cache on Gradle 7
+            @Suppress("ObjectLiteralToLambda")
+            object : NamedDomainObjectFactory<SpotBugsReport> {
+                override fun create(name: String): SpotBugsReport {
+                    return when (name) {
+                        "html" -> objects.newInstance(SpotBugsHtmlReport::class.java, name, objects, taskRef)
+                        "xml" -> objects.newInstance(SpotBugsXmlReport::class.java, name, objects, taskRef)
+                        "text" -> objects.newInstance(SpotBugsTextReport::class.java, name, objects, taskRef)
+                        "sarif" -> objects.newInstance(SpotBugsSarifReport::class.java, name, objects, taskRef)
+                        else -> throw InvalidUserDataException("$name is invalid as the report name")
+                    }.also {
+                        (outputs as org.gradle.api.tasks.TaskOutputs).file(it.outputLocation)
+                    }
+                }
+            },
+        )
         description = "Run SpotBugs analysis."
         group = JavaBasePlugin.VERIFICATION_GROUP
     }
@@ -389,7 +403,7 @@ abstract class SpotBugsTask : DefaultTask(), VerificationTask {
             SpotBugsRunnerForHybrid(workerExecutor, launcher).run(this)
         } else {
             log.info("Running SpotBugs by JavaExec...")
-            SpotBugsRunnerForJavaExec(launcher).run(this)
+            SpotBugsRunnerForJavaExec(execOps, launcher).run(this)
         }
     }
 

diff --git a/src/main/kotlin/com/github/spotbugs/snom/internal/SpotBugsRunnerForJavaExec.kt b/src/main/kotlin/com/github/spotbugs/snom/internal/SpotBugsRunnerForJavaExec.kt
@@ -25,10 +25,12 @@ import org.gradle.api.file.RegularFileProperty
 import org.gradle.api.provider.Property
 import org.gradle.api.provider.Provider
 import org.gradle.jvm.toolchain.JavaLauncher
+import org.gradle.process.ExecOperations
 import org.gradle.process.JavaExecSpec
 import org.slf4j.LoggerFactory
 
 internal class SpotBugsRunnerForJavaExec @Inject constructor(
+    private val execOps: ExecOperations,
     private val javaLauncher: Property<JavaLauncher>,
 ) : SpotBugsRunner() {
     private val log = LoggerFactory.getLogger(SpotBugsRunnerForJavaExec::class.java)
@@ -37,14 +39,11 @@ internal class SpotBugsRunnerForJavaExec @Inject constructor(
     override fun run(task: SpotBugsTask) {
         // TODO print version of SpotBugs and Plugins
         try {
-            task.project.javaexec(configureJavaExec(task)).rethrowFailure().assertNormalExitValue()
+            execOps.javaexec(configureJavaExec(task)).rethrowFailure().assertNormalExitValue()
             if (stderrOutputScanner.isFailedToReport && !task.getIgnoreFailures()) {
                 throw GradleException("SpotBugs analysis succeeded but report generation failed")
             }
-        } catch (
-            // TODO remove this internal API usage.
-            @Suppress("InternalGradleApiUsage") e: org.gradle.process.internal.ExecException,
-        ) {
+        } catch (e: GradleException) {
             if (task.getIgnoreFailures()) {
                 log.warn(
                     "SpotBugs reported failures",