Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

provide better support for Gradle config cache #554

Merged
merged 7 commits into from
Sep 7, 2021
Merged

provide better support for Gradle config cache #554

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
cebd47a
fix: stop getting properties during execution phase
KengoTODA Sep 7, 2021
8d4eba3
refactor: remove duplicated task initialization
KengoTODA Sep 7, 2021
1e11846
fix: stop getting plugin manager during execution phase
KengoTODA Sep 7, 2021
aebd27d
chore: keep the comment to explain WHY
KengoTODA Sep 7, 2021
616a4c3
chore: remove needless method parameter
KengoTODA Sep 7, 2021
68df6b7
test: manually confirm the cachability at Gradle build scan
KengoTODA Sep 7, 2021
a362d58
test: configure Gradle enterprise only for Gradle 6+
KengoTODA Sep 7, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 19 additions & 4 deletions src/functionalTest/groovy/com/github/spotbugs/snom/CacheabilityFunctionalTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package com.github.spotbugs.snom
import org.gradle.testkit.runner.BuildResult
import org.gradle.testkit.runner.GradleRunner
import org.gradle.util.GradleVersion
import spock.lang.Requires
import spock.lang.Specification

import java.nio.file.Files
Expand All @@ -41,8 +42,8 @@ class CacheabilityFunctionalTest extends Specification {

def version = System.getProperty('snom.test.functional.gradle', GradleVersion.current().version)

initializeBuildFile(buildDir1)
initializeBuildFile(buildDir2)
initializeBuildFile(buildDir1, !version.startsWith('5'))
initializeBuildFile(buildDir2, !version.startsWith('5'))

when:
BuildResult result1 =
Expand All @@ -62,7 +63,7 @@ class CacheabilityFunctionalTest extends Specification {
BuildResult result2 =
GradleRunner.create()
.withProjectDir(buildDir2)
.withArguments(':spotbugsMain')
.withArguments(':spotbugsMain', '--scan')
.withPluginClasspath()
.forwardOutput()
.withGradleVersion(version)
Expand All @@ -78,8 +79,9 @@ class CacheabilityFunctionalTest extends Specification {
return result.output.find('Build cache key for task \':spotbugsMain\' is .*')
}

private static void initializeBuildFile(File buildDir) {
private static void initializeBuildFile(File buildDir, boolean runScan) {
File buildFile = new File(buildDir, 'build.gradle')
File settingsFile = new File(buildDir, 'settings.gradle')
File propertiesFile = new File(buildDir, 'gradle.properties')

buildFile << '''
Expand All @@ -95,6 +97,19 @@ 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()
}
File sourceDir = buildDir.toPath().resolve('src').resolve('main').resolve('java').toFile()
sourceDir.mkdirs()
File sourceFile = new File(sourceDir, 'Foo.java')
Expand Down
26 changes: 25 additions & 1 deletion src/main/groovy/com/github/spotbugs/snom/SpotBugsBasePlugin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@
import org.gradle.util.GradleVersion;

public class SpotBugsBasePlugin implements Plugin<Project> {
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";

/**
* Supported Gradle version described at <a
* href="http://spotbugs.readthedocs.io/en/latest/gradle.html">official manual site</a>. <a
Expand All @@ -36,13 +40,27 @@ public class SpotBugsBasePlugin implements Plugin<Project> {

@Override
public void apply(Project project) {
// use XML report by default, only when SpotBugs plugin is applied
boolean isSpotBugsPluginApplied = project.getPluginManager().hasPlugin("com.github.spotbugs");
verifyGradleVersion(GradleVersion.current());
project.getPluginManager().apply(ReportingBasePlugin.class);

SpotBugsExtension extension = createExtension(project);
createConfiguration(project, extension);
createPluginConfiguration(project);
project.getTasks().withType(SpotBugsTask.class).configureEach(task -> task.init(extension));

String enableWorkerApi = getPropertyOrDefault(project, FEATURE_FLAG_WORKER_API, "true");
String enableHybridWorker = getPropertyOrDefault(project, FEATURE_FLAG_HYBRID_WORKER, "false");
project
.getTasks()
.withType(SpotBugsTask.class)
.configureEach(
task ->
task.init(
extension,
isSpotBugsPluginApplied,
Boolean.parseBoolean(enableWorkerApi),
Boolean.parseBoolean(enableHybridWorker)));
}

private SpotBugsExtension createExtension(Project project) {
Expand Down Expand Up @@ -117,4 +135,10 @@ void verifyGradleVersion(GradleVersion version) {
throw new IllegalArgumentException(message);
}
}

private String getPropertyOrDefault(Project project, String propertyName, String defaultValue) {
return project.hasProperty(propertyName)
? project.property(propertyName).toString()
: defaultValue;
}
}
7 changes: 3 additions & 4 deletions src/main/groovy/com/github/spotbugs/snom/SpotBugsPlugin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ public class SpotBugsPlugin implements Plugin<Project> {
@Override
public void apply(Project project) {
project.getPluginManager().apply(SpotBugsBasePlugin.class);
SpotBugsExtension extension = project.getExtensions().findByType(SpotBugsExtension.class);
project
.getPluginManager()
.withPlugin(
Expand All @@ -45,10 +44,10 @@ public void apply(Project project) {
.configure(
task -> task.dependsOn(project.getTasks().withType(SpotBugsTask.class)));
});
createTasks(project, extension);
createTasks(project);
}

private void createTasks(Project project, SpotBugsExtension extension) {
new SpotBugsTaskFactory().generate(project, task -> task.init(extension));
private void createTasks(Project project) {
new SpotBugsTaskFactory().generate(project);
}
}
23 changes: 11 additions & 12 deletions src/main/groovy/com/github/spotbugs/snom/SpotBugsTask.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,6 @@ import javax.inject.Inject

@CacheableTask
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;
Expand Down Expand Up @@ -262,6 +260,10 @@ class SpotBugsTask extends DefaultTask implements VerificationTask {

private FileCollection classes;

private boolean enableWorkerApi;
private boolean enableHybridWorker;
private boolean isSpotBugsPluginApplied;

void setClasses(FileCollection fileCollection) {
this.classes = fileCollection
}
Expand Down Expand Up @@ -335,7 +337,7 @@ class SpotBugsTask extends DefaultTask implements VerificationTask {
*
* @param extension the source extension to copy the properties.
*/
void init(SpotBugsExtension extension) {
void init(SpotBugsExtension extension, boolean isSpotBugsPluginApplied, boolean enableWorkerApi, boolean enableHybridWorker) {
ignoreFailures.convention(extension.ignoreFailures)
showStackTraces.convention(extension.showStackTraces)
showProgress.convention(extension.showProgress)
Expand All @@ -355,17 +357,17 @@ class SpotBugsTask extends DefaultTask implements VerificationTask {
extraArgs.convention(extension.extraArgs)
maxHeapSize.convention(extension.maxHeapSize)
useAuxclasspathFile.convention(extension.useAuxclasspathFile)
this.isSpotBugsPluginApplied = isSpotBugsPluginApplied
this.enableWorkerApi = enableWorkerApi
this.enableHybridWorker = enableHybridWorker
}

@TaskAction
void run() {
String enableWorkerApi = project.properties.getOrDefault(FEATURE_FLAG_WORKER_API, "true")
String enableHybridWorker = project.properties.getOrDefault(FEATURE_FLAG_HYBRID_WORKER, "false")

if (enableWorkerApi == "false") {
if (!enableWorkerApi) {
log.info("Running SpotBugs by JavaExec...");
new SpotBugsRunnerForJavaExec().run(this);
} else if (enableHybridWorker == "true" && GradleVersion.current() >= GradleVersion.version("6.0")) {
} else if (enableHybridWorker && GradleVersion.current() >= GradleVersion.version("6.0")) {
// ExecOperations is supported from Gradle 6.0
log.info("Running SpotBugs by Gradle no-isolated Worker...");
new SpotBugsRunnerForHybrid(workerExecutor).run(this);
Expand Down Expand Up @@ -406,11 +408,8 @@ class SpotBugsTask extends DefaultTask implements VerificationTask {
@Optional
@Nested
SpotBugsReport getFirstEnabledReport() {
// use XML report by default, only when SpotBugs plugin is applied
boolean isSpotBugsPluingApplied = project.pluginManager.hasPlugin("com.github.spotbugs")

java.util.Optional<SpotBugsReport> report = reports.stream().filter({ report -> report.enabled}).findFirst()
if (isSpotBugsPluingApplied) {
if (isSpotBugsPluginApplied) {
return report.orElse(reports.create("xml"))
} else {
return report.orElse(null)
Expand Down
13 changes: 5 additions & 8 deletions src/main/groovy/com/github/spotbugs/snom/internal/SpotBugsTaskFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,12 @@
public class SpotBugsTaskFactory {
private final Logger log = LoggerFactory.getLogger(SpotBugsTaskFactory.class);

public void generate(Project project, Action<? super SpotBugsTask> configurationAction) {
generateForJava(project, configurationAction);
generateForAndroid(project, configurationAction);
public void generate(Project project) {
generateForJava(project);
generateForAndroid(project);
}

private void generateForJava(Project project, Action<? super SpotBugsTask> configurationAction) {
private void generateForJava(Project project) {
project
.getPlugins()
.withType(JavaBasePlugin.class)
Expand All @@ -62,14 +62,12 @@ private void generateForJava(Project project, Action<? super SpotBugsTask> confi
sourceSet.getAllSource().getSourceDirectories());
task.setClassDirs(sourceSet.getOutput());
task.setAuxClassPaths(sourceSet.getCompileClasspath());
configurationAction.execute(task);
});
});
});
}

private void generateForAndroid(
Project project, Action<? super SpotBugsTask> configurationAction) {
private void generateForAndroid(Project project) {

@SuppressWarnings("rawtypes")
final Action<? super Plugin> action =
Expand Down Expand Up @@ -103,7 +101,6 @@ private void generateForAndroid(
project.files(javaCompile.getDestinationDir()));
spotbugsTask.setAuxClassPaths(javaCompile.getClasspath());
spotbugsTask.dependsOn(javaCompile);
configurationAction.execute(spotbugsTask);
});
});
};
Expand Down