Revert 58 to fix spotbugs/spotbugs!819

.travis.yml

@@ -8,7 +8,6 @@ jdk:
 env:
  matrix:
  - GRADLE_VERSION=default
- - GRADLE_VERSION=5.0
  - GRADLE_VERSION=5.1
  global:
  - SONAR_HOST_URL="https://sonarcloud.io"

CHANGELOG.md

@@ -8,9 +8,15 @@ Currently the versioning policy of this project follows [Semantic Versioning](ht
 
 ### Fixed
 
+* "Trying to add already registered factory" problem reported as [a spotbugs issue](https:/spotbugs/spotbugs/issues/819)
 * Result caching works across checkouts. Previously it was using absolute paths and therefore didn't work. ([#96](https:/spotbugs/spotbugs-gradle-plugin/pull/96))
 * Restore compatiblity with Gradle 4.0~4.6 ([#101](https:/spotbugs/spotbugs-gradle-plugin/issues/101))
 
+### Removed
+
+* Usage of worker API introduced at [#57](https:/spotbugs/spotbugs-gradle-plugin/issues/57)
+* Drop support for Gradle 4, that causes [SLF4J related problem](https:/gradle/gradle/issues/2657)
+
 ### Changed
 
 * Use SpotBugs 3.1.11 by default

build.gradle

@@ -14,6 +14,7 @@ apply from: "$rootDir/gradle/deploy.gradle"
 version = '1.6.10-SNAPSHOT'
 group = "com.github.spotbugs"
 def spotBugsVersion = '3.1.11'
+def slf4jVersion = '1.8.0-beta2'
 sourceCompatibility = 1.8
 targetCompatibility = 1.8
 
@@ -55,6 +56,7 @@ task processVersionFile(type: WriteProperties) {
  outputFile file('src/main/resources/spotbugs-gradle-plugin.properties')
 
  property 'spotbugs-version', spotBugsVersion
+ property 'slf4j-version', slf4jVersion
 }
 tasks.processResources.dependsOn processVersionFile
 

gradle/wrapper/gradle-wrapper.properties

@@ -1,5 +1,5 @@
 distributionBase=GRADLE_USER_HOME
 distributionPath=wrapper/dists
-distributionUrl=https\://services.gradle.org/distributions/gradle-4.10.3-bin.zip
+distributionUrl=https\://services.gradle.org/distributions/gradle-5.0-bin.zip
 zipStoreBase=GRADLE_USER_HOME
 zipStorePath=wrapper/dists

src/main/java/com/github/spotbugs/SpotBugsPlugin.java

@@ -5,6 +5,8 @@
 import java.io.InputStream;
 import java.io.UncheckedIOException;
 import java.net.URL;
+import java.util.HashMap;
+import java.util.Map;
 import java.util.Properties;
 import java.util.concurrent.Callable;
 import java.util.stream.StreamSupport;
@@ -42,7 +44,7 @@ public class SpotBugsPlugin extends AbstractCodeQualityPlugin<SpotBugsTask> {
  *
  * Package-protected access is for testing purposes
  */
- static final GradleVersion SUPPORTED_VERSION = GradleVersion.version("4.0");
+ static final GradleVersion SUPPORTED_VERSION = GradleVersion.version("5.0");
 
  private SpotBugsExtension extension;
 
@@ -124,11 +126,19 @@ protected CodeQualityExtension createExtension() {
  }
 
  String loadToolVersion() {
+ return loadVersion("spotbugs-version");
+ }
+
+ String loadSlf4jVersion() {
+ return loadVersion("slf4j-version");
+ }
+
+ private String loadVersion(String name) {
  URL url = SpotBugsPlugin.class.getClassLoader().getResource("spotbugs-gradle-plugin.properties");
  try (InputStream input = url.openStream()) {
  Properties prop = new Properties();
  prop.load(input);
- return prop.getProperty("spotbugs-version");
+ return prop.getProperty(name);
  } catch (IOException e) {
  throw new UncheckedIOException(e);
  }
@@ -148,10 +158,37 @@ protected void configureConfiguration(Configuration configuration) {
  // https:/spotbugs/spotbugs-gradle-plugin/issues/22
  }
 
+ /**
+ * Overriding this method, to include SLF4J into {@code spotbugsClasspath}. SLF4J is necessary in worker process.
+ */
+ @Override
+ protected void createConfigurations() {
+ Configuration configuration = project.getConfigurations().create(getConfigurationName());
+ configuration.setVisible(false);
+ configuration.setTransitive(true);
+ configuration.setDescription("The " + getToolName() + " libraries to be used for this project.");
+ // Don't need these things, they're provided by the runtime
+ configuration.exclude(excludeProperties("ant", "ant"));
+ configuration.exclude(excludeProperties("org.apache.ant", "ant"));
+ configuration.exclude(excludeProperties("org.apache.ant", "ant-launcher"));
+ configuration.exclude(excludeProperties("org.slf4j", "jcl-over-slf4j"));
+ configuration.exclude(excludeProperties("org.slf4j", "log4j-over-slf4j"));
+ configuration.exclude(excludeProperties("commons-logging", "commons-logging"));
+ configuration.exclude(excludeProperties("log4j", "log4j"));
+ configureConfiguration(configuration);
+ }
+
+ private Map<String, String> excludeProperties(String group, String module) {
+ Map<String, String> map = new HashMap<>();
+ map.put("group", group);
+ map.put("module", module);
+ return map;
+ }
  private void configureDefaultDependencies(Configuration configuration) {
- configuration.defaultDependencies((DependencySet dependencies) ->
- dependencies.add(project.getDependencies().create("com.github.spotbugs:spotbugs:" + extension.getToolVersion()))
- );
+ configuration.defaultDependencies((DependencySet dependencies) -> {
+ dependencies.add(project.getDependencies().create("org.slf4j:slf4j-simple:" + loadSlf4jVersion()));
+ dependencies.add(project.getDependencies().create("com.github.spotbugs:spotbugs:" + extension.getToolVersion()));
+ });
  }
 
  private void configureTaskConventionMapping(Configuration configuration, SpotBugsTask task) {

src/main/java/com/github/spotbugs/SpotBugsTask.java

@@ -1,6 +1,7 @@
 package com.github.spotbugs;
 
 import java.io.File;
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
@@ -9,15 +10,15 @@
 
 import javax.inject.Inject;
 
-import com.github.spotbugs.internal.spotbugs.SpotBugsRunner;
-import groovy.lang.Closure;
 import org.gradle.api.Action;
+import org.gradle.api.GradleException;
 import org.gradle.api.Incubating;
 import org.gradle.api.JavaVersion;
-import org.gradle.api.Task;
 import org.gradle.api.file.FileCollection;
 import org.gradle.api.file.FileTree;
+import org.gradle.api.logging.LogLevel;
 import org.gradle.api.reporting.Reporting;
+import org.gradle.api.reporting.SingleFileReport;
 import org.gradle.api.resources.TextResource;
 import org.gradle.api.tasks.CacheableTask;
 import org.gradle.api.tasks.Classpath;
@@ -33,19 +34,21 @@
 import org.gradle.api.tasks.SourceTask;
 import org.gradle.api.tasks.TaskAction;
 import org.gradle.api.tasks.VerificationTask;
+import org.gradle.internal.logging.ConsoleRenderer;
+import org.gradle.internal.reflect.Instantiator;
+import org.gradle.process.internal.worker.WorkerProcessFactory;
 import org.gradle.util.ConfigureUtil;
-import org.gradle.util.DeprecationLogger;
 import org.gradle.util.GradleVersion;
-import org.gradle.workers.ForkMode;
-import org.gradle.workers.IsolationMode;
-import org.gradle.workers.WorkerExecutor;
 
 import com.github.spotbugs.internal.SpotBugsReportsImpl;
 import com.github.spotbugs.internal.SpotBugsReportsInternal;
 import com.github.spotbugs.internal.spotbugs.SpotBugsClasspathValidator;
+import com.github.spotbugs.internal.spotbugs.SpotBugsResult;
 import com.github.spotbugs.internal.spotbugs.SpotBugsSpec;
 import com.github.spotbugs.internal.spotbugs.SpotBugsSpecBuilder;
+import com.github.spotbugs.internal.spotbugs.SpotBugsWorkerManager;
 
+import groovy.lang.Closure;
 
 /**
  * Analyzes code with <a href="https://spotbugs.github.io/">SpotBugs</a>. See the
@@ -91,32 +94,18 @@ public class SpotBugsTask extends SourceTask implements VerificationTask, Report
  @Nested
  private final SpotBugsReportsInternal reports;
 
- @Inject
- public SpotBugsTask(WorkerExecutor workerExecutor) {
- super();
- this.workerExecutor = workerExecutor;
- reports = createReports(this);
- }
-
- private SpotBugsReportsInternal createReports(Task task) {
- return DeprecationLogger.whileDisabled(() -> {
- if (GradleVersion.current().compareTo(GRADLE_42()) < 0) {
- return new SpotBugsReportsImpl(task);
- } else {
- //ObjectFactory#newInstance was introduced in Gradle 4.2
- return getProject().getObjects().newInstance(SpotBugsReportsImpl.class, task);
- }
- });
+ public SpotBugsTask() {
+ reports = getInstantiator().newInstance(SpotBugsReportsImpl.class, this);
  }
 
- private final WorkerExecutor workerExecutor;
-
- private GradleVersion GRADLE_42() {
- return GradleVersion.version("4.2");
+ @Inject
+ public Instantiator getInstantiator() {
+ throw new UnsupportedOperationException();
  }
 
- private GradleVersion GRADLE_47() {
- return GradleVersion.version("4.7");
+ @Inject
+ public WorkerProcessFactory getWorkerProcessBuilderFactory() {
+ throw new UnsupportedOperationException();
  }
 
  /**
@@ -128,11 +117,9 @@ private GradleVersion GRADLE_47() {
  * @return true if any reports are enabled, otherwise logs a warning and returns false
  */
  private boolean hasEnabledReports() {
- boolean hasEnabledReports = GradleVersion.current().compareTo(GRADLE_47()) >= 0
- ? !reports.getEnabledReports().isEmpty()
- : !reports.getEnabled().getAsMap().isEmpty();
+ boolean hasEnabledReports = !reports.getEnabledReports().isEmpty();
 
- if(!hasEnabledReports) {
+ if (!hasEnabledReports) {
  getProject().getLogger().lifecycle("WARNING: No SpotBugs report(s) were configured; aborting execution of {}", getPath());
  }
  return hasEnabledReports;
@@ -260,27 +247,22 @@ public void setExcludeBugsFilter(File filter) {
  }
 
  @TaskAction
- public void run() {
+ public void run() throws IOException, InterruptedException {
  new SpotBugsClasspathValidator(JavaVersion.current()).validateClasspath(
  getSpotbugsClasspath().getFiles().stream().map(File::getName).collect(Collectors.toSet()));
  SpotBugsSpec spec = generateSpec();
+ SpotBugsWorkerManager manager = new SpotBugsWorkerManager();
+
+ getLogging().captureStandardOutput(LogLevel.DEBUG);
+ getLogging().captureStandardError(LogLevel.DEBUG);
 
  //workaround for https:/spotbugs/spotbugs-gradle-plugin/issues/61
  if(!hasEnabledReports()) {
  return;
  }
 
- workerExecutor.submit(SpotBugsRunner.class, config -> {
- config.params(spec, getIgnoreFailures(), reports.getFirstEnabled().getDestination());
- config.setClasspath(getSpotbugsClasspath());
- config.setForkMode(ForkMode.ALWAYS);
- config.forkOptions( options -> {
- options.setDebug(spec.isDebugEnabled());
- options.setJvmArgs(spec.getJvmArgs());
- options.setMaxHeapSize(spec.getMaxHeapSize());
- });
- config.setIsolationMode(IsolationMode.PROCESS);
- });
+ SpotBugsResult result = manager.runWorker(getProject().getProjectDir(), getWorkerProcessBuilderFactory(), getSpotbugsClasspath(), spec);
+ evaluateResult(result);
  }
 
  SpotBugsSpec generateSpec() {
@@ -305,6 +287,33 @@ SpotBugsSpec generateSpec() {
  return specBuilder.build();
  }
 
+ void evaluateResult(SpotBugsResult result) {
+ if (result.getException() != null) {
+ throw new GradleException("SpotBugs encountered an error. Run with --debug to get more information.", result.getException());
+ }
+
+ if (result.getErrorCount() > 0) {
+ throw new GradleException("SpotBugs encountered an error. Run with --debug to get more information.");
+ }
+
+ if (result.getBugCount() > 0) {
+ String message = "SpotBugs rule violations were found.";
+ SingleFileReport report = reports.getFirstEnabled();
+ if (report != null) {
+ String reportUrl = new ConsoleRenderer().asClickableFileUrl(report.getDestination());
+ message += " See the report at: " + reportUrl;
+ }
+
+ if (getIgnoreFailures()) {
+ getLogger().warn(message);
+ } else {
+ throw new GradleException(message);
+ }
+
+ }
+
+ }
+
  public SpotBugsTask extraArgs(Iterable<String> arguments) {
  for (String argument : arguments) {
  extraArgs.add(argument);

src/main/java/com/github/spotbugs/internal/SpotBugsReportsImpl.java

@@ -1,6 +1,5 @@
 package com.github.spotbugs.internal;
 
-import javax.inject.Inject;
 import org.gradle.api.Task;
 import org.gradle.api.reporting.SingleFileReport;
 import org.gradle.api.reporting.internal.CustomizableHtmlReportImpl;
@@ -12,7 +11,6 @@
 
 public class SpotBugsReportsImpl extends TaskReportContainer<SingleFileReport> implements SpotBugsReportsInternal {
 
- @Inject
  public SpotBugsReportsImpl(Task task) {
  super(SingleFileReport.class, task);
 

src/main/java/com/github/spotbugs/internal/spotbugs/SpotBugsExecutor.java

@@ -0,0 +1,37 @@
+package com.github.spotbugs.internal.spotbugs;
+
+import java.io.IOException;
+import java.util.List;
+
+import edu.umd.cs.findbugs.FindBugs;
+import edu.umd.cs.findbugs.FindBugs2;
+import edu.umd.cs.findbugs.IFindBugsEngine;
+import edu.umd.cs.findbugs.TextUICommandLine;
+
+public class SpotBugsExecutor implements SpotBugsWorker {
+ @Override
+ public SpotBugsResult runSpotbugs(SpotBugsSpec spec) throws IOException, InterruptedException {
+ final ClassLoader contextClassLoader = Thread.currentThread().getContextClassLoader();
+ try {
+ final List<String> args = spec.getArguments();
+ String[] strArray = args.toArray(new String[0]);
+
+ Thread.currentThread().setContextClassLoader(FindBugs2.class.getClassLoader());
+ FindBugs2 findBugs2 = new FindBugs2();
+ TextUICommandLine commandLine = new TextUICommandLine();
+ FindBugs.processCommandLine(commandLine, strArray, findBugs2);
+ findBugs2.execute();
+
+ return createSpotbugsResult(findBugs2);
+ } finally {
+ Thread.currentThread().setContextClassLoader(contextClassLoader);
+ }
+ }
+
+ SpotBugsResult createSpotbugsResult(IFindBugsEngine findBugs) {
+ int bugCount = findBugs.getBugCount();
+ int missingClassCount = findBugs.getMissingClassCount();
+ int errorCount = findBugs.getErrorCount();
+ return new SpotBugsResult(bugCount, missingClassCount, errorCount);
+ }
+}