From 706bc67c7b685d5acf39878537772b24ea0f8608 Mon Sep 17 00:00:00 2001 From: lukasgraef Date: Mon, 16 Sep 2024 21:02:02 +0200 Subject: [PATCH 1/2] fix: remaining configuration cache issues --- api/spotbugs-gradle-plugin.api | 1 + .../com/github/spotbugs/snom/BaseFunctionalTest.groovy | 2 +- .../spotbugs/snom/CacheabilityFunctionalTest.groovy | 8 ++++---- .../github/spotbugs/snom/StandardFunctionalTest.groovy | 4 ++-- src/main/kotlin/com/github/spotbugs/snom/SpotBugsTask.kt | 6 +++++- .../spotbugs/snom/internal/SpotBugsRunnerForJavaExec.kt | 9 ++++----- 6 files changed, 17 insertions(+), 13 deletions(-) diff --git a/api/spotbugs-gradle-plugin.api b/api/spotbugs-gradle-plugin.api index 64248acc..6abe9fdb 100644 --- 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 index 0a46cab2..b38c3e0e 100644 --- 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 index d4906568..6fba61ba 100644 --- 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 @@ -115,7 +115,7 @@ class CacheabilityFunctionalTest extends BaseFunctionalTest { when: gradleRunner - .withArguments(':spotbugsMain', '--build-cache') + .withArguments(':spotbugsMain', '--no-configuration-cache', '--build-cache') .build() BuildResult result = gradleRunner .withArguments(':spotbugsMain', '--build-cache') @@ -156,7 +156,7 @@ class CacheabilityFunctionalTest extends BaseFunctionalTest { when: def result = gradleRunner - .withArguments(':spotbugsMain', '--configuration-cache') + .withArguments(':spotbugsMain') .build() then: diff --git a/src/functionalTest/groovy/com/github/spotbugs/snom/StandardFunctionalTest.groovy b/src/functionalTest/groovy/com/github/spotbugs/snom/StandardFunctionalTest.groovy index 96c38f15..2f1c7dff 100644 --- a/src/functionalTest/groovy/com/github/spotbugs/snom/StandardFunctionalTest.groovy +++ b/src/functionalTest/groovy/com/github/spotbugs/snom/StandardFunctionalTest.groovy @@ -159,7 +159,7 @@ spotbugsMain { when: gradleRunner - .withArguments(":spotbugsMain") + .withArguments(":spotbugsMain", "--no-configuration-cache") .build() def result = gradleRunner .withArguments(":spotbugsMain") @@ -486,7 +486,7 @@ public class MyFoo { when: BuildResult result = gradleRunner - .withArguments('--debug', "spotbugsMain") + .withArguments('--debug', "--no-configuration-cache", "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 index 4ff6067b..99a86107 100644 --- a/src/main/kotlin/com/github/spotbugs/snom/SpotBugsTask.kt +++ b/src/main/kotlin/com/github/spotbugs/snom/SpotBugsTask.kt @@ -48,6 +48,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 +94,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() @@ -389,7 +393,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 index 21faf9ea..c3094594 100644 --- 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, ) : 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", From f1c4ea78a2081b03f2280f07de6e92486df94456 Mon Sep 17 00:00:00 2001 From: lukasgraef Date: Sun, 1 Dec 2024 15:03:10 +0100 Subject: [PATCH 2/2] fix: broken serialization of NamedDomainObjectFactory on Gradle 7 --- .../snom/CacheabilityFunctionalTest.groovy | 2 +- .../snom/StandardFunctionalTest.groovy | 4 +-- .../com/github/spotbugs/snom/SpotBugsTask.kt | 32 ++++++++++++------- 3 files changed, 24 insertions(+), 14 deletions(-) diff --git a/src/functionalTest/groovy/com/github/spotbugs/snom/CacheabilityFunctionalTest.groovy b/src/functionalTest/groovy/com/github/spotbugs/snom/CacheabilityFunctionalTest.groovy index 6fba61ba..8dfb6a31 100644 --- a/src/functionalTest/groovy/com/github/spotbugs/snom/CacheabilityFunctionalTest.groovy +++ b/src/functionalTest/groovy/com/github/spotbugs/snom/CacheabilityFunctionalTest.groovy @@ -115,7 +115,7 @@ class CacheabilityFunctionalTest extends BaseFunctionalTest { when: gradleRunner - .withArguments(':spotbugsMain', '--no-configuration-cache', '--build-cache') + .withArguments(':spotbugsMain', '--build-cache') .build() BuildResult result = gradleRunner .withArguments(':spotbugsMain', '--build-cache') diff --git a/src/functionalTest/groovy/com/github/spotbugs/snom/StandardFunctionalTest.groovy b/src/functionalTest/groovy/com/github/spotbugs/snom/StandardFunctionalTest.groovy index 2f1c7dff..96c38f15 100644 --- a/src/functionalTest/groovy/com/github/spotbugs/snom/StandardFunctionalTest.groovy +++ b/src/functionalTest/groovy/com/github/spotbugs/snom/StandardFunctionalTest.groovy @@ -159,7 +159,7 @@ spotbugsMain { when: gradleRunner - .withArguments(":spotbugsMain", "--no-configuration-cache") + .withArguments(":spotbugsMain") .build() def result = gradleRunner .withArguments(":spotbugsMain") @@ -486,7 +486,7 @@ public class MyFoo { when: BuildResult result = gradleRunner - .withArguments('--debug', "--no-configuration-cache", "spotbugsMain") + .withArguments('--debug', "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 index 99a86107..83944622 100644 --- 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 @@ -308,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 { + 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 }