From ba591fbb3d8016c28bb2e6d7231087e286cccc8a Mon Sep 17 00:00:00 2001 From: Kengo TODA Date: Fri, 21 Jan 2022 13:51:51 +0800 Subject: [PATCH] fix: support Gradle configuration cache (#663) * test: reproduce the reported problem as a functional test refs #662 * fix: do not resolve task.project at execution time * fix: do not resolve task.project at execution time * fix: do not resolve task.project at execution time * chore: follow the suggestion made by SQ https://sonarcloud.io/project/issues?id=com.github.spotbugs.gradle&issues=AX5MfO0ow8sHw4bh7DpY&open=AX5MfO0ow8sHw4bh7DpY&pullRequest=663 * test: check not only failure but also success * test: confirm that cached entry has been reused by the following build --- .../snom/CacheabilityFunctionalTest.groovy | 67 ++++++++++++++----- .../github/spotbugs/snom/SpotBugsTask.groovy | 58 ++++++++++++---- .../snom/internal/SpotBugsRunner.java | 31 +-------- 3 files changed, 97 insertions(+), 59 deletions(-) diff --git a/src/functionalTest/groovy/com/github/spotbugs/snom/CacheabilityFunctionalTest.groovy b/src/functionalTest/groovy/com/github/spotbugs/snom/CacheabilityFunctionalTest.groovy index fe4b0a86..c561a406 100644 --- a/src/functionalTest/groovy/com/github/spotbugs/snom/CacheabilityFunctionalTest.groovy +++ b/src/functionalTest/groovy/com/github/spotbugs/snom/CacheabilityFunctionalTest.groovy @@ -15,14 +15,49 @@ package com.github.spotbugs.snom import org.gradle.testkit.runner.BuildResult import org.gradle.testkit.runner.GradleRunner +import org.gradle.testkit.runner.TaskOutcome import org.gradle.util.GradleVersion -import spock.lang.Requires import spock.lang.Specification import java.nio.file.Files class CacheabilityFunctionalTest extends Specification { + /** + * @see GitHub Issues + */ + def 'spotbugsMain task runs with configuration cache'() { + given: + def buildDir = Files.createTempDirectory(null).toFile() + def version = System.getProperty('snom.test.functional.gradle', GradleVersion.current().version) + initializeBuildFile(buildDir) + + when: + BuildResult result = + GradleRunner.create() + .withProjectDir(buildDir) + .withArguments(':spotbugsMain', '--configuration-cache') + .withPluginClasspath() + .forwardOutput() + .withGradleVersion(version) + .build() + + then: + !result.output.contains("Configuration cache problems found in this build") + result.output.contains("Configuration cache entry stored.") + + when: + BuildResult resultOfCachedBuild = GradleRunner.create() + .withProjectDir(buildDir) + .withArguments(':spotbugsMain', '--configuration-cache') + .withPluginClasspath() + .forwardOutput() + .withGradleVersion(version) + .build() + then: + resultOfCachedBuild.task(":spotbugsMain").outcome == TaskOutcome.UP_TO_DATE + resultOfCachedBuild.output.contains("Configuration cache entry reused.") + } /** * Verifies the cacheability of {@link SpotBugsTask} by invoking the same code @@ -42,8 +77,8 @@ class CacheabilityFunctionalTest extends Specification { def version = System.getProperty('snom.test.functional.gradle', GradleVersion.current().version) - initializeBuildFile(buildDir1, !version.startsWith('5')) - initializeBuildFile(buildDir2, !version.startsWith('5')) + initializeBuildFile(buildDir1) + initializeBuildFile(buildDir2) when: BuildResult result1 = @@ -79,7 +114,7 @@ class CacheabilityFunctionalTest extends Specification { return result.output.find('Build cache key for task \':spotbugsMain\' is .*') } - private static void initializeBuildFile(File buildDir, boolean runScan) { + private static void initializeBuildFile(File buildDir) { File buildFile = new File(buildDir, 'build.gradle') File settingsFile = new File(buildDir, 'settings.gradle') File propertiesFile = new File(buildDir, 'gradle.properties') @@ -102,19 +137,17 @@ class CacheabilityFunctionalTest extends Specification { |} |'''.stripMargin() - if (runScan) { - settingsFile << ''' - |plugins { - | id "com.gradle.enterprise" version "3.6.4" - |} - |gradleEnterprise { - | buildScan { - | termsOfServiceUrl = "https://gradle.com/terms-of-service" - | termsOfServiceAgree = "yes" - | } - |} - '''.stripMargin() - } + settingsFile << ''' + |plugins { + | id "com.gradle.enterprise" version "3.6.4" + |} + |gradleEnterprise { + | buildScan { + | termsOfServiceUrl = "https://gradle.com/terms-of-service" + | termsOfServiceAgree = "yes" + | } + |} + '''.stripMargin() File sourceDir = buildDir.toPath().resolve('src').resolve('main').resolve('java').toFile() sourceDir.mkdirs() File sourceFile = new File(sourceDir, 'Foo.java') diff --git a/src/main/groovy/com/github/spotbugs/snom/SpotBugsTask.groovy b/src/main/groovy/com/github/spotbugs/snom/SpotBugsTask.groovy index 4a9b835f..3a013d45 100644 --- a/src/main/groovy/com/github/spotbugs/snom/SpotBugsTask.groovy +++ b/src/main/groovy/com/github/spotbugs/snom/SpotBugsTask.groovy @@ -11,8 +11,9 @@ * express or implied. See the License for the specific language governing permissions and * limitations under the License. */ -package com.github.spotbugs.snom; +package com.github.spotbugs.snom +import com.github.spotbugs.snom.internal.SemanticVersion; import com.github.spotbugs.snom.internal.SpotBugsHtmlReport import com.github.spotbugs.snom.internal.SpotBugsRunnerForHybrid; import com.github.spotbugs.snom.internal.SpotBugsRunnerForJavaExec; @@ -21,7 +22,8 @@ import com.github.spotbugs.snom.internal.SpotBugsSarifReport; import com.github.spotbugs.snom.internal.SpotBugsTextReport; import com.github.spotbugs.snom.internal.SpotBugsXmlReport; import edu.umd.cs.findbugs.annotations.NonNull -import edu.umd.cs.findbugs.annotations.Nullable; +import edu.umd.cs.findbugs.annotations.Nullable +import org.gradle.api.artifacts.Dependency; import org.gradle.api.file.DirectoryProperty import org.gradle.api.file.RegularFileProperty import org.gradle.api.plugins.JavaPluginExtension @@ -33,7 +35,6 @@ import org.gradle.api.Action; import org.gradle.api.DefaultTask; import org.gradle.api.InvalidUserDataException; import org.gradle.api.NamedDomainObjectContainer; -import org.gradle.api.artifacts.Configuration; import org.gradle.api.file.FileCollection; import org.gradle.api.model.ObjectFactory; import org.gradle.api.provider.ListProperty; @@ -53,7 +54,6 @@ import org.gradle.api.tasks.VerificationTask import org.gradle.jvm.toolchain.JavaLauncher import org.gradle.jvm.toolchain.JavaToolchainService; import org.gradle.util.ClosureBackedAction -import org.gradle.util.GradleVersion; import org.gradle.workers.WorkerExecutor; import org.slf4j.Logger; import org.slf4j.LoggerFactory @@ -95,9 +95,6 @@ import javax.inject.Inject @CacheableTask abstract class SpotBugsTask extends DefaultTask implements VerificationTask { - private static final String FEATURE_FLAG_WORKER_API = "com.github.spotbugs.snom.worker"; - private static final String FEATURE_FLAG_HYBRID_WORKER = "com.github.spotbugs.snom.javaexec-in-worker"; - private final Logger log = LoggerFactory.getLogger(SpotBugsTask); private final WorkerExecutor workerExecutor; @@ -269,6 +266,10 @@ abstract class SpotBugsTask extends DefaultTask implements VerificationTask { private boolean enableWorkerApi; private boolean enableHybridWorker; + private FileCollection pluginJarFiles + private FileCollection spotbugsClasspath + + private Provider isSupportingMultipleReports void setClasses(FileCollection fileCollection) { this.classes = fileCollection @@ -341,6 +342,32 @@ abstract class SpotBugsTask extends DefaultTask implements VerificationTask { useAuxclasspathFile = objects.property(Boolean) setDescription("Run SpotBugs analysis.") setGroup(JavaBasePlugin.VERIFICATION_GROUP) + def pluginConfiguration = project.getConfigurations().getByName(SpotBugsPlugin.PLUGINS_CONFIG_NAME) + pluginJarFiles = project.layout.files { + pluginConfiguration.files + } + + def configuration = project.getConfigurations().getByName(SpotBugsPlugin.CONFIG_NAME) + def logger = this.log + isSupportingMultipleReports = project.provider { + configuration.resolve() + java.util.Optional spotbugs = + configuration.getDependencies().stream() + .filter { dependency -> "com.github.spotbugs" == dependency.group && "spotbugs" == dependency.name } + .findFirst() + if (!spotbugs.isPresent()) { + logger.warn("No spotbugs found in the {} configuration", SpotBugsPlugin.CONFIG_NAME) + return false + } + SemanticVersion version = new SemanticVersion(spotbugs.get().getVersion()) + logger.debug("Using SpotBugs version {}", version) + return version >= new SemanticVersion("4.5.0") + } + + def spotbugsSlf4j = project.configurations.getByName(SpotBugsPlugin.SLF4J_CONFIG_NAME) + spotbugsClasspath = project.layout.files { + spotbugsSlf4j.files + configuration.files + } } /** @@ -418,16 +445,13 @@ abstract class SpotBugsTask extends DefaultTask implements VerificationTask { @NonNull @Internal Set getPluginJar() { - return getProject().getConfigurations().getByName(SpotBugsPlugin.PLUGINS_CONFIG_NAME).getFiles() + return pluginJarFiles.files } @NonNull @Internal FileCollection getSpotbugsClasspath() { - Configuration config = getProject().getConfigurations().getByName(SpotBugsPlugin.CONFIG_NAME) - Configuration spotbugsSlf4j = getProject().getConfigurations().getByName(SpotBugsPlugin.SLF4J_CONFIG_NAME) - - return getProject().files(config, spotbugsSlf4j) + return spotbugsClasspath } @Nullable @@ -481,6 +505,16 @@ abstract class SpotBugsTask extends DefaultTask implements VerificationTask { showStackTraces.get(); } + /** + * The multiple reports feature is available from SpotBugs 4.5.0 + * + * @see GitHub Releases + */ + @Internal + boolean isSupportingMultipleReports() { + return isSupportingMultipleReports.getOrElse(Boolean.FALSE).booleanValue() + } + @Internal String getBaseName() { String prunedName = name.replaceFirst("spotbugs", "") diff --git a/src/main/groovy/com/github/spotbugs/snom/internal/SpotBugsRunner.java b/src/main/groovy/com/github/spotbugs/snom/internal/SpotBugsRunner.java index 5d810846..c935883e 100644 --- a/src/main/groovy/com/github/spotbugs/snom/internal/SpotBugsRunner.java +++ b/src/main/groovy/com/github/spotbugs/snom/internal/SpotBugsRunner.java @@ -13,7 +13,6 @@ */ package com.github.spotbugs.snom.internal; -import com.github.spotbugs.snom.SpotBugsPlugin; import com.github.spotbugs.snom.SpotBugsReport; import com.github.spotbugs.snom.SpotBugsTask; import edu.umd.cs.findbugs.annotations.NonNull; @@ -28,14 +27,10 @@ import java.util.Collection; import java.util.Collections; import java.util.List; -import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; import org.gradle.api.GradleException; -import org.gradle.api.Project; import org.gradle.api.Task; -import org.gradle.api.artifacts.Configuration; -import org.gradle.api.artifacts.Dependency; import org.gradle.api.file.FileCollection; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -45,30 +40,6 @@ public abstract class SpotBugsRunner { public abstract void run(@NonNull SpotBugsTask task); - /** - * The multiple reports feature is available from SpotBugs 4.5.0 - * - * @see GitHub Releases - */ - private boolean isSupportingMultipleReports(Project project) { - Configuration configuration = project.getConfigurations().getByName(SpotBugsPlugin.CONFIG_NAME); - configuration.resolve(); - Optional spotbugs = - configuration.getDependencies().stream() - .filter( - dependency -> - "com.github.spotbugs".equals(dependency.getGroup()) - && "spotbugs".equals(dependency.getName())) - .findFirst(); - if (!spotbugs.isPresent()) { - log.warn("No spotbugs found in the {} configuration", SpotBugsPlugin.CONFIG_NAME); - return false; - } - SemanticVersion version = new SemanticVersion(spotbugs.get().getVersion()); - log.debug("Using SpotBugs version {}", version); - return version.compareTo(new SemanticVersion("4.5.0")) >= 0; - } - protected List buildArguments(SpotBugsTask task) { List args = new ArrayList<>(); @@ -98,7 +69,7 @@ protected List buildArguments(SpotBugsTask task) { args.add("-progress"); } - if (isSupportingMultipleReports(task.getProject())) { + if (task.isSupportingMultipleReports()) { for (SpotBugsReport report : task.getEnabledReports()) { File reportFile = report.getOutputLocation().getAsFile().get(); File dir = reportFile.getParentFile();