You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kafka.apache.org by li...@apache.org on 2018/09/10 20:14:22 UTC
[kafka] branch trunk updated: KAFKA-5887;
Replace findBugs with spotBugs and upgrade to Gradle 4.10
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 <is...@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 <is...@juma.me.uk>
Reviewers: Colin Patrick McCabe <co...@cmccabe.xyz>, Dong Lin <li...@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 "$@"