Skip to content

Commit

Permalink
Make SAT thread-safe, improve performance and update SpotBugs
Browse files Browse the repository at this point in the history 
This PR makes the SAT Maven plugins thread-safe when used in parallel Maven builds.

Fixes the following theading issues:

* SpotBugs is only thread-safe when it uses the forking mode which is enabled by default but disabled in the SAT configuration.
  SpotBugs uses Apache Commons BCEL as dependency which is not thread-safe. Forking the SpotBugs execution works around this issue.
  Without forking enabled parallel builds using SpotBugs will also lock up (see spotbugs/spotbugs-maven-plugin#104).
* Use lock files for synchronizing the SAT summary report updates.
  Because multiple classloaders are used in Maven builds, it's not possible to use a static lock for synchronization in the ReportUtility Mojo.

The Mojos have also been marked as threadSafe so Maven no longer shows warnings when they are executed in parallel.

For the SpotBugs updates see:

* Spotbugs 3.1.12 release notes:
  https://github.com/spotbugs/spotbugs/blob/3.1.12/CHANGELOG.md#3112---2019-02-28
* findbugs-slf4j 1.5.0 release notes:
  https://github.com/KengoTODA/findbugs-slf4j/blob/master/CHANGELOG.md#150---2019-07-04

The errors resulting from the findbugs-slf4j upgrade have already been fixed by:

* openhab/openhab-core#1213
* openhab/openhab2-addons#6394

Performance has also been improved considerably by moving the HTML summary generation to an extension.
This way SAT can periodically update the HTML summary instead of continuously.
Updating the HTML summaries can take 20 seconds or more when there are many findings.

Fixes openhab#200

Signed-off-by: Wouter Born <github@maindrain.net>
  • Loading branch information
@wborn
wborn committed Nov 18, 2019
1 parent cefddfa commit ee4c893
Show file tree
Hide file tree
Showing 16 changed files with 799 additions and 117 deletions.
6 changes: 3 additions & 3 deletions docs/maven-plugin.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,10 +116,10 @@ Parameters:
| **spotbugsRuleset** | String | Relative path to the XML that specifies the bug detectors which should be run. If not set the default file will be used|
| **spotbugsInclude** | String | Relative path to the XML that specifies the bug instances that will be included in the report. If not set the default file will be used|
| **spotbugsExclude** | String | Relative path to the XML that specifies the bug instances that will be excluded from the report. If not set the default file will be used|
| **maven.spotbugs.version** | String | The version of the spotbugs-maven-plugin that will be used (default value is **3.1.6**)|
| **spotbugs.version** | String | The version of SpotBugs that will be used (default value is **3.1.7**)|
| **maven.spotbugs.version** | String | The version of the spotbugs-maven-plugin that will be used (default value is **3.1.12.2**)|
| **spotbugs.version** | String | The version of SpotBugs that will be used (default value is **3.1.12**)|
| **spotbugsPlugins** | List<Dependency> | A list with artifacts that contain additional detectors/patterns for SpotBugs |
| **findbugs.slf4j.version** | String | The version of the findbugs-slf4j plugin that will be used (default value is **1.2.4**)|
| **findbugs.slf4j.version** | String | The version of the findbugs-slf4j plugin that will be used (default value is **1.5.0**)|

### sat-plugin:report

Expand Down
26 changes: 14 additions & 12 deletions pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
<version>0.9.0-SNAPSHOT</version>
<packaging>pom</packaging>

<name>Static Code Analysis Tool Parent Pom</name>
<name>Static Code Analysis Tool Parent POM</name>
<description>Tool that aggregates reports of PMD, Checkstyle and FindBugs</description>
<url>https://github.com/openhab/static-code-analysis</url>

Expand All @@ -21,8 +21,8 @@
</licenses>

<organization>
<name>openhab.org</name>
<url>http://www.openhab.org</url>
<name>openHAB.org</name>
<url>https://www.openhab.org</url>
</organization>

<developers>
Expand All @@ -37,7 +37,7 @@
<url>git@github.com:openhab/static-code-analysis.git</url>
</scm>
<issueManagement>
<system>github</system>
<system>GitHub</system>
<url>https://github.com/openhab/static-code-analysis/issues</url>
</issueManagement>

Expand All @@ -51,20 +51,21 @@
<maven.resources.version>2.4</maven.resources.version>
<pmd.version>6.7.0</pmd.version>
<checkstyle.version>8.12</checkstyle.version>
<spotbugs.version>3.1.7</spotbugs.version>
<maven.plugin.api.version>3.1.1</maven.plugin.api.version>
<spotbugs.version>3.1.12</spotbugs.version>
<maven.core.version>3.5.0</maven.core.version>
<maven.plugin.api.version>3.5.0</maven.plugin.api.version>
<maven.plugin.annotations.version>3.5</maven.plugin.annotations.version>
<maven.plugin.plugin.version>3.5</maven.plugin.plugin.version>
<maven.plugin.compiler.version>3.6.1</maven.plugin.compiler.version>
<mojo.executor.version>2.2.0</mojo.executor.version>
<mojo.executor.version>2.3.0</mojo.executor.version>
<org.apache.ivy.version>2.4.0</org.apache.ivy.version>
<org.jsoup.version>1.7.1</org.jsoup.version>
<pde.core.version>3.11.1</pde.core.version>
<sat.version>0.6.1</sat.version>
<sat.version>0.8.0</sat.version>
<jdt-annotations.version>2.1.0</jdt-annotations.version>
<flexmark.version>0.28.6</flexmark.version>
<maven.surefire.plugin.version>2.12.4</maven.surefire.plugin.version>
<jetty.server.version>9.3.14.v20161028</jetty.server.version>
<jetty.server.version>9.4.20.v20190813</jetty.server.version>
<saxon.version>9.1.0.8</saxon.version>
</properties>

Expand Down Expand Up @@ -186,7 +187,7 @@
<version>2.4.3-01</version>
</dependency>
</dependencies>
</plugin>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-source-plugin</artifactId>
Expand Down Expand Up @@ -227,7 +228,7 @@
</plugin>
</plugins>
</build>

<dependencies>
<dependency>
<groupId>junit</groupId>
Expand Down Expand Up @@ -262,9 +263,10 @@
<scope>test</scope>
</dependency>
</dependencies>

<modules>
<module>custom-checks</module>
<module>sat-extension</module>
<module>sat-plugin</module>
<module>codestyle</module>
</modules>
Expand Down
89 changes: 89 additions & 0 deletions sat-extension/pom.xml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns="http://maven.apache.org/POM/4.0.0"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">

<modelVersion>4.0.0</modelVersion>

<parent>
<groupId>org.openhab.tools.sat</groupId>
<artifactId>pom</artifactId>
<version>0.9.0-SNAPSHOT</version>
</parent>

<artifactId>sat-extension</artifactId>

<name>Static Code Analysis Tool Maven Extension</name>
<description>Generates HTML summaries from analysis results</description>

<dependencies>
<!-- Maven dependencies -->
<dependency>
<groupId>org.apache.maven</groupId>
<artifactId>maven-core</artifactId>
<version>${maven.core.version}</version>
<scope>provided</scope>
</dependency>

<!-- SAT dependency -->
<dependency>
<groupId>org.openhab.tools.sat</groupId>
<artifactId>sat-plugin</artifactId>
<version>${project.version}</version>
</dependency>

<!-- Saxon dependency -->
<dependency>
<groupId>net.sourceforge.saxon</groupId>
<artifactId>saxon</artifactId>
<version>${saxon.version}</version>
</dependency>
</dependencies>

<build>
<plugins>
<plugin>
<groupId>org.codehaus.plexus</groupId>
<artifactId>plexus-component-metadata</artifactId>
<version>1.7.1</version>
<executions>
<execution>
<goals>
<goal>generate-metadata</goal>
<goal>generate-test-metadata</goal>
</goals>
</execution>
</executions>
</plugin>
</plugins>
<pluginManagement>
<plugins>
<plugin>
<groupId>org.eclipse.m2e</groupId>
<artifactId>lifecycle-mapping</artifactId>
<version>1.0.0</version>
<configuration>
<lifecycleMappingMetadata>
<pluginExecutions>
<pluginExecution>
<pluginExecutionFilter>
<groupId>org.codehaus.plexus</groupId>
<artifactId>plexus-component-metadata</artifactId>
<versionRange>[1.7.1,)</versionRange>
<goals>
<goal>generate-metadata</goal>
<goal>generate-test-metadata</goal>
</goals>
</pluginExecutionFilter>
<action>
<ignore></ignore>
</action>
</pluginExecution>
</pluginExecutions>
</lifecycleMappingMetadata>
</configuration>
</plugin>
</plugins>
</pluginManagement>
</build>

</project>
Loading

0 comments on commit ee4c893

Please sign in to comment.