kafka-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From lind...@apache.org
Subject [kafka] branch trunk updated: KAFKA-5887; Replace findBugs with spotBugs and upgrade to Gradle 4.10
Date Mon, 10 Sep 2018 20:14:22 GMT
This is an automated email from the ASF dual-hosted git repository.

lindong pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/kafka.git


The following commit(s) were added to refs/heads/trunk by this push:
     new f123d2f  KAFKA-5887; Replace findBugs with spotBugs and upgrade to Gradle 4.10
f123d2f is described below

commit f123d2f18c55b1cf2edb452aeb87e6ad0743c292
Author: Ismael Juma <ismael@juma.me.uk>
AuthorDate: Mon Sep 10 13:14:00 2018 -0700

    KAFKA-5887; Replace findBugs with spotBugs and upgrade to Gradle 4.10
    
    findBugs is abandoned, it doesn't work with Java 9 and the Gradle plugin will be deprecated
in
    Gradle 5.0: https://github.com/gradle/gradle/pull/6664
    
    spotBugs is actively maintained and it supports Java 8, 9 and 10. Java 11 is not supported
yet,
    but it's likely to happen soon.
    
    Also fixed a file leak in Connect identified by spotbugs.
    
    Manually tested spotBugsMain, jarAll and importing kafka in IntelliJ and running
    a build in the IDE.
    
    Author: Ismael Juma <ismael@juma.me.uk>
    
    Reviewers: Colin Patrick McCabe <colin@cmccabe.xyz>, Dong Lin <lindong28@gmail.com>
    
    Closes #5625 from ijuma/kafka-5887-spotbugs
---
 README.md                                          | 16 +++++------
 build.gradle                                       | 31 +++++++++++++---------
 .../org/apache/kafka/common/config/ConfigDef.java  |  2 +-
 .../auth/extension/PropertyFileLoginModule.java    |  5 +++-
 .../scala/kafka/server/ClientQuotaManager.scala    |  2 +-
 .../{findbugs-exclude.xml => spotbugs-exclude.xml} | 12 ++++-----
 jenkins.sh                                         |  2 +-
 7 files changed, 39 insertions(+), 31 deletions(-)

diff --git a/README.md b/README.md
index 2285814..22ef66f 100644
--- a/README.md
+++ b/README.md
@@ -168,7 +168,7 @@ Please note for this to work you should create/update user maven settings
(typic
     ./gradlew dependencyUpdates
 
 ### Running code quality checks ###
-There are two code quality analysis tools that we regularly run, findbugs and checkstyle.
+There are two code quality analysis tools that we regularly run, spotbugs and checkstyle.
 
 #### Checkstyle ####
 Checkstyle enforces a consistent coding style in Kafka.
@@ -179,14 +179,14 @@ You can run checkstyle using:
 The checkstyle warnings will be found in `reports/checkstyle/reports/main.html` and `reports/checkstyle/reports/test.html`
files in the
 subproject build directories. They are also are printed to the console. The build will fail
if Checkstyle fails.
 
-#### Findbugs ####
-Findbugs uses static analysis to look for bugs in the code.
-You can run findbugs using:
+#### Spotbugs ####
+Spotbugs uses static analysis to look for bugs in the code.
+You can run spotbugs using:
 
-    ./gradlew findbugsMain findbugsTest -x test
+    ./gradlew spotbugsMain spotbugsTest -x test
 
-The findbugs warnings will be found in `reports/findbugs/main.html` and `reports/findbugs/test.html`
files in the subproject build
-directories.  Use -PxmlFindBugsReport=true to generate an XML report instead of an HTML one.
+The spotbugs warnings will be found in `reports/spotbugs/main.html` and `reports/spotbugs/test.html`
files in the subproject build
+directories.  Use -PxmlSpotBugsReport=true to generate an XML report instead of an HTML one.
 
 ### Common build options ###
 
@@ -198,7 +198,7 @@ The following options should be set with a `-P` switch, for example `./gradlew
-
 * `showStandardStreams`: shows standard out and standard error of the test JVM(s) on the
console.
 * `skipSigning`: skips signing of artifacts.
 * `testLoggingEvents`: unit test events to be logged, separated by comma. For example `./gradlew
-PtestLoggingEvents=started,passed,skipped,failed test`.
-* `xmlFindBugsReport`: enable XML reports for findBugs. This also disables HTML reports as
only one can be enabled at a time.
+* `xmlSpotBugsReport`: enable XML reports for spotBugs. This also disables HTML reports as
only one can be enabled at a time.
 
 ### Running in Vagrant ###
 
diff --git a/build.gradle b/build.gradle
index 9ebdfe9..c9663ab 100644
--- a/build.gradle
+++ b/build.gradle
@@ -19,6 +19,9 @@ buildscript {
   repositories {
     mavenCentral()
     jcenter()
+    maven {
+      url "https://plugins.gradle.org/m2/"
+    }
   }
   apply from: file('gradle/buildscript.gradle'), to: buildscript
 
@@ -30,6 +33,7 @@ buildscript {
     classpath 'com.github.jengelman.gradle.plugins:shadow:2.0.4'
     classpath 'org.owasp:dependency-check-gradle:3.2.1'
     classpath "com.diffplug.spotless:spotless-plugin-gradle:3.10.0"
+    classpath "gradle.plugin.com.github.spotbugs:spotbugs-gradle-plugin:1.6.3"
   }
 }
 
@@ -85,7 +89,7 @@ allprojects {
 }
 
 ext {
-  gradleVersion = "4.8.1"
+  gradleVersion = "4.10"
   minJavaVersion = "8"
   buildVersionFileName = "kafka-version.properties"
 
@@ -145,9 +149,8 @@ subprojects {
   apply plugin: 'maven'
   apply plugin: 'signing'
   apply plugin: 'checkstyle'
-
-  if (!JavaVersion.current().isJava9Compatible())
-    apply plugin: 'findbugs'
+  if (!JavaVersion.current().isJava11Compatible())
+    apply plugin: "com.github.spotbugs"
 
   sourceCompatibility = minJavaVersion
   targetCompatibility = minJavaVersion
@@ -364,18 +367,20 @@ subprojects {
   }
   test.dependsOn('checkstyleMain', 'checkstyleTest')
 
-  if (!JavaVersion.current().isJava9Compatible()) {
-    findbugs {
-      toolVersion = "3.0.1"
-      excludeFilter = file("$rootDir/gradle/findbugs-exclude.xml")
+  if (!JavaVersion.current().isJava11Compatible()) {
+    spotbugs {
+      // 3.1.6 has a regression that breaks our build, seems to be https://github.com/spotbugs/spotbugs/pull/688
+      toolVersion = '3.1.5'
+      excludeFilter = file("$rootDir/gradle/spotbugs-exclude.xml")
       ignoreFailures = false
     }
-    test.dependsOn('findbugsMain')
+    test.dependsOn('spotbugsMain')
 
-    tasks.withType(FindBugs) {
+    tasks.withType(com.github.spotbugs.SpotBugsTask) {
       reports {
-        xml.enabled(project.hasProperty('xmlFindBugsReport'))
-        html.enabled(!project.hasProperty('xmlFindBugsReport'))
+        // Continue supporting `xmlFindBugsReport` for compatibility
+        xml.enabled(project.hasProperty('xmlSpotBugsReport') || project.hasProperty('xmlFindBugsReport'))
+        html.enabled(!project.hasProperty('xmlSpotBugsReport') && !project.hasProperty('xmlFindBugsReport'))
       }
     }
   }
@@ -408,7 +413,7 @@ subprojects {
 }
 
 gradle.taskGraph.whenReady { taskGraph ->
-  taskGraph.getAllTasks().findAll { it.name.contains('findbugsScoverage') || it.name.contains('findbugsTest')
}.each { task ->
+  taskGraph.getAllTasks().findAll { it.name.contains('spotbugsScoverage') || it.name.contains('spotbugsTest')
}.each { task ->
     task.enabled = false
   }
 }
diff --git a/clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java b/clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java
index 3868ed5..7669f1d 100644
--- a/clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java
+++ b/clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java
@@ -942,7 +942,7 @@ public class ConfigDef {
         @Override
         public void ensureValid(String name, Object value) {
             if (value == null) {
-                // Pass in the string null to avoid the findbugs warning
+                // Pass in the string null to avoid the spotbugs warning
                 throw new ConfigException(name, "null", "entry must be non null");
             }
         }
diff --git a/connect/basic-auth-extension/src/main/java/org/apache/kafka/connect/rest/basic/auth/extension/PropertyFileLoginModule.java
b/connect/basic-auth-extension/src/main/java/org/apache/kafka/connect/rest/basic/auth/extension/PropertyFileLoginModule.java
index 101c6f4..8a013330 100644
--- a/connect/basic-auth-extension/src/main/java/org/apache/kafka/connect/rest/basic/auth/extension/PropertyFileLoginModule.java
+++ b/connect/basic-auth-extension/src/main/java/org/apache/kafka/connect/rest/basic/auth/extension/PropertyFileLoginModule.java
@@ -22,6 +22,7 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import java.io.IOException;
+import java.io.InputStream;
 import java.nio.file.Files;
 import java.nio.file.Paths;
 import java.util.Map;
@@ -64,7 +65,9 @@ public class PropertyFileLoginModule implements LoginModule {
         if (!credentialPropertiesMap.containsKey(fileName)) {
             Properties credentialProperties = new Properties();
             try {
-                credentialProperties.load(Files.newInputStream(Paths.get(fileName)));
+                try (InputStream inputStream = Files.newInputStream(Paths.get(fileName)))
{
+                    credentialProperties.load(inputStream);
+                }
                 credentialPropertiesMap.putIfAbsent(fileName, credentialProperties);
             } catch (IOException e) {
                 log.error("Error loading credentials file ", e);
diff --git a/core/src/main/scala/kafka/server/ClientQuotaManager.scala b/core/src/main/scala/kafka/server/ClientQuotaManager.scala
index 41ee420..9817f21 100644
--- a/core/src/main/scala/kafka/server/ClientQuotaManager.scala
+++ b/core/src/main/scala/kafka/server/ClientQuotaManager.scala
@@ -180,7 +180,7 @@ class ClientQuotaManager(private val config: ClientQuotaManagerConfig,
   delayQueueSensor.add(metrics.metricName("queue-size",
     quotaType.toString,
     "Tracks the size of the delay queue"), new Total())
-  start() // Use start method to keep findbugs happy
+  start() // Use start method to keep spotbugs happy
   private def start() {
     throttledChannelReaper.start()
   }
diff --git a/gradle/findbugs-exclude.xml b/gradle/spotbugs-exclude.xml
similarity index 97%
rename from gradle/findbugs-exclude.xml
rename to gradle/spotbugs-exclude.xml
index 62240d8..99a657e 100644
--- a/gradle/findbugs-exclude.xml
+++ b/gradle/spotbugs-exclude.xml
@@ -15,12 +15,12 @@
    limitations under the License.
 -->
 
-<!-- Findbugs filtering.
+<!-- Spotbugs filtering.
 
-Findbugs is a static code analysis tool run as part of the "check" phase of the build.
-This file dictates which categories of bugs and individual false positives that we supress.
+Spotbugs is a static code analysis tool run as part of the "check" phase of the build.
+This file dictates which categories of bugs and individual false positives that we suppress.
 
-For a detailed description of findbugs bug categories, see http://findbugs.sourceforge.net/bugDescriptions.html
+For a detailed description of spotbugs bug categories, see https://spotbugs.readthedocs.io/en/latest/bugDescriptions.html
 -->
 <FindBugsFilter>
     <Match>
@@ -44,8 +44,8 @@ For a detailed description of findbugs bug categories, see http://findbugs.sourc
     </Match>
 
     <Match>
-        <!-- Findbugs tends to work a little bit better with Java than with Scala.  We
suppress
-             some categories of bug reports when using Scala, since findbugs generates huge
+        <!-- Spotbugs tends to work a little bit better with Java than with Scala.  We
suppress
+             some categories of bug reports when using Scala, since spotbugs generates huge
              numbers of false positives in some of these categories when examining Scala
code.
 
             NP_LOAD_OF_KNOWN_NULL_VALUE: The variable referenced at this point is known to
be null
diff --git a/jenkins.sh b/jenkins.sh
index 43973ac..dd85dde 100755
--- a/jenkins.sh
+++ b/jenkins.sh
@@ -17,4 +17,4 @@
 # This script is used for verifying changes in Jenkins. In order to provide faster feedback,
the tasks are ordered so
 # that faster tasks are executed in every module before slower tasks (if possible). For example,
the unit tests for all
 # the modules are executed before the integration tests.
-./gradlew clean compileJava compileScala compileTestJava compileTestScala spotlessScalaCheck
checkstyleMain checkstyleTest findbugsMain unitTest rat integrationTest --no-daemon --continue
-PxmlFindBugsReport=true -PtestLoggingEvents=started,passed,skipped,failed "$@"
+./gradlew clean compileJava compileScala compileTestJava compileTestScala spotlessScalaCheck
checkstyleMain checkstyleTest spotbugsMain unitTest rat integrationTest --no-daemon --continue
-PxmlSpotBugsReport=true -PtestLoggingEvents=started,passed,skipped,failed "$@"


Mime
View raw message