You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tajo.apache.org by jh...@apache.org on 2014/01/17 02:58:20 UTC

git commit: TAJO-508: Apply findbugs-excludeFilterFile to TajoQA. (jinho)

Updated Branches:
  refs/heads/master 35a710379 -> b822c2882


TAJO-508: Apply findbugs-excludeFilterFile to TajoQA. (jinho)


Project: http://git-wip-us.apache.org/repos/asf/incubator-tajo/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-tajo/commit/b822c288
Tree: http://git-wip-us.apache.org/repos/asf/incubator-tajo/tree/b822c288
Diff: http://git-wip-us.apache.org/repos/asf/incubator-tajo/diff/b822c288

Branch: refs/heads/master
Commit: b822c288274bd7863d8a4de9c41060b321906894
Parents: 35a7103
Author: jinossy <ji...@gmail.com>
Authored: Fri Jan 17 10:57:23 2014 +0900
Committer: jinossy <ji...@gmail.com>
Committed: Fri Jan 17 10:57:23 2014 +0900

----------------------------------------------------------------------
 CHANGES.txt                                     |   2 +
 dev-support/findbugs-exclude.xml                | 151 +++++++++++++++++++
 dev-support/test-patch.sh                       |  45 +++---
 pom.xml                                         |   1 +
 .../src/main/findbugs/findbugs-exclude.xml      |  26 ----
 tajo-core/tajo-core-storage/pom.xml             |  10 --
 tajo-project/pom.xml                            |  13 ++
 7 files changed, 189 insertions(+), 59 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-tajo/blob/b822c288/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index 49c5392..9919921 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -337,6 +337,8 @@ Release 0.8.0 - unreleased
 
   TASKS
 
+    TAJO-508: Apply findbugs-excludeFilterFile to TajoQA. (jinho)
+
     TAJO-457: Update committer list and contributor list. (hyunsik)
 
     TAJO-166: Automatic precommit test using Jenkins. (hyunsik)

http://git-wip-us.apache.org/repos/asf/incubator-tajo/blob/b822c288/dev-support/findbugs-exclude.xml
----------------------------------------------------------------------
diff --git a/dev-support/findbugs-exclude.xml b/dev-support/findbugs-exclude.xml
new file mode 100644
index 0000000..99f316c
--- /dev/null
+++ b/dev-support/findbugs-exclude.xml
@@ -0,0 +1,151 @@
+<!--
+   Licensed to the Apache Software Foundation (ASF) under one or more
+   contributor license agreements.  See the NOTICE file distributed with
+   this work for additional information regarding copyright ownership.
+   The ASF licenses this file to You under the Apache License, Version 2.0
+   (the "License"); you may not use this file except in compliance with
+   the License.  You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+-->
+<FindBugsFilter>
+  <Match>
+    <Class name="~org.apache.tajo.engine.parser.*"/>
+  </Match>
+
+  <Match>
+    <Class name="~org\.apache\.tajo\..*Proto.*"/>
+  </Match>
+
+  <Match>
+    <!--
+    A mutable static field could be changed by malicious code or by accident. The field could be
+    made package protected to avoid this vulnerability.
+    !-->
+    <Bug pattern="MS_PKGPROTECT"/>
+  </Match>
+  <Match>
+    <Bug pattern="MS_OOI_PKGPROTECT"/>
+  </Match>
+
+
+  <Match>
+    <!--
+    Returning a reference to a mutable object value stored in one of the object's fields exposes
+    the internal representation of the object.  If instances are accessed by untrusted code,
+    and unchecked changes to the mutable object would compromise security or other important
+    properties, you will need to do something different. Returning a new copy of the object is
+    better approach in many situations.
+
+    We have getters on our internal fields. Questionable, but out of findbugs scope. Returning a
+    copy is not practical in most cases.
+    !-->
+    <Bug pattern="EI_EXPOSE_REP"/>
+  </Match>
+  <Match>
+    <Bug pattern="EI_EXPOSE_REP2"/>
+  </Match>
+
+
+  <Match>
+    <!--
+    This class implements the Comparator interface. You should consider whether or not it should
+    also implement the Serializable interface. If a comparator is used to construct an ordered
+    collection such as a TreeMap, then the TreeMap will be serializable only if the comparator
+    is also serializable. As most comparators have little or no state, making them serializable
+    is generally easy and good defensive programming.
+    !-->
+    <Bug pattern="SE_COMPARATOR_SHOULD_BE_SERIALIZABLE"/>
+  </Match>
+
+  <Match>
+
+    <!--
+    This method performs synchronization an object that is an instance of a class from
+    the java.util.concurrent package (or its subclasses). Instances of these classes have their own
+    concurrency control mechanisms that are orthogonal to the synchronization provided by the Java
+    keyword synchronized. For example, synchronizing on an AtomicBoolean will not prevent other
+    threads from modifying the AtomicBoolean.
+
+    Such code may be correct, but should be carefully reviewed and documented, and may confuse people
+    who have to maintain the code at a later date.
+
+    We do that all the time to save lock objects.
+    !-->
+    <Bug pattern="JLM_JSR166_UTILCONCURRENT_MONITORENTER"/>
+  </Match>
+
+  <Match>
+    <!--
+    Found a call to a method which will perform a byte to String (or String to byte) conversion,
+    and will assume that the default platform encoding is suitable. This will cause the
+    application behaviour to vary between platforms. Use an alternative API and specify a
+    charset name or Charset object explicitly.
+    !-->
+    <Bug pattern="DM_DEFAULT_ENCODING"/>
+  </Match>
+
+  <Match>
+    <!--
+     Invoking System.exit shuts down the entire Java virtual machine. This should only been
+     done when it is appropriate. Such calls make it hard or impossible for your code to be
+     invoked by other code. Consider throwing a RuntimeException instead.
+
+     It's so bad that the reviews will catch all the wrong cases.
+    !-->
+    <Bug pattern="DM_EXIT"/>
+  </Match>
+
+  <Match>
+    <!--
+     This method returns a value that is not checked. The return value should be checked since
+     it can indicate an unusual or unexpected function execution. For example, the
+     File.delete() method returns false if the file could not be successfully deleted
+     (rather than throwing an Exception). If you don't check the result, you won't notice
+     if the method invocation signals unexpected behavior by returning an atypical return
+     value.
+
+     It's so bad that the reviews will catch all the wrong cases.
+    !-->
+    <Bug pattern="RV_RETURN_VALUE_IGNORED_BAD_PRACTICE"/>
+  </Match>
+  <Match>
+    <Bug pattern="RV_RETURN_VALUE_IGNORED_INFERRED"/>
+  </Match>
+
+
+  <Match>
+    <!--
+     This method contains a redundant check of a known non-null value against the constant null.
+
+     Most of the time we're securing ourselves, does no much harm.
+    !-->
+    <Bug pattern="RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE"/>
+  </Match>
+
+  <Match>
+    <!--
+    A final static field references an array and can be accessed by malicious code or by
+    accident from another package. This code can freely modify the contents of the array.
+
+     We've got this all over the place... Cloning the array by security is not a general
+     solution from a performance point of view
+    !-->
+    <Bug pattern="MS_MUTABLE_ARRAY"/>
+  </Match>
+
+  <Match>
+    <!--
+     The logic explicitly checks equality of two floating point numbers. Ignore the warning
+    !-->
+    <Class name="org.apache.hadoop.hbase.master.AssignmentVerificationReport"/>
+    <Bug pattern="FE_FLOATING_POINT_EQUALITY"/>
+  </Match>
+
+</FindBugsFilter>

http://git-wip-us.apache.org/repos/asf/incubator-tajo/blob/b822c288/dev-support/test-patch.sh
----------------------------------------------------------------------
diff --git a/dev-support/test-patch.sh b/dev-support/test-patch.sh
index 2533aab..1a5c247 100644
--- a/dev-support/test-patch.sh
+++ b/dev-support/test-patch.sh
@@ -288,7 +288,7 @@ verifyPatch () {
       echo "PATCH APPLICATION FAILED"
       JIRA_COMMENT="$JIRA_COMMENT
 
-      {color:red}-1 patch.  The patch command could not apply the patch.{color:red}"
+      {color:red}-1 patch.{color}  The patch command could not apply the patch."
       return -1
     else
       return 1
@@ -340,12 +340,12 @@ checkAuthor () {
   if [[ $authorTags != 0 ]] ; then
     JIRA_COMMENT="$JIRA_COMMENT
 
-    {color:red}-1 @author.  The patch appears to contain $authorTags @author tags which the Tajo community has agreed to not allow in code contributions.{color:red}"
+    {color:red}-1 @author.{color}  The patch appears to contain $authorTags @author tags which the Tajo community has agreed to not allow in code contributions."
     return 1
   fi
   JIRA_COMMENT="$JIRA_COMMENT
 
-    {color:green}+1 @author.  The patch does not contain any @author tags.{color:green}"
+    {color:green}+1 @author.{color}  The patch does not contain any @author tags."
   return 0
 }
 
@@ -376,14 +376,14 @@ checkTests () {
     fi
     JIRA_COMMENT="$JIRA_COMMENT
 
-    {color:red}-1 tests included.  The patch doesn't appear to include any new or modified tests.{color:red}
+    {color:red}-1 tests included.{color}  The patch doesn't appear to include any new or modified tests.
                         Please justify why no new tests are needed for this patch.
                         Also please list what manual steps were performed to verify this patch."
     return 1
   fi
   JIRA_COMMENT="$JIRA_COMMENT
 
-    {color:green}+1 tests included.  The patch appears to include $testReferences new or modified test files.{color:green}"
+    {color:green}+1 tests included.{color}  The patch appears to include $testReferences new or modified test files."
   return 0
 }
 
@@ -414,7 +414,7 @@ applyPatch () {
     echo "PATCH APPLICATION FAILED"
     JIRA_COMMENT="$JIRA_COMMENT
 
-    {color:red}-1 patch.  The patch command could not apply the patch.{color:red}"
+    {color:red}-1 patch.{color}  The patch command could not apply the patch."
     return 1
   fi
   return 0
@@ -449,13 +449,13 @@ checkJavadocWarnings () {
     ### if patch warning greater than trunk warning
     JIRA_COMMENT="$JIRA_COMMENT
 
-    {color:red}-1 javadoc.  The applied patch generated $patchJavadocWarnings javadoc warnings (more than the trunk's current $trunkJavadocWarnings warnings).{color:red}"
+    {color:red}-1 javadoc.{color}  The applied patch generated $patchJavadocWarnings javadoc warnings (more than the trunk's current $trunkJavadocWarnings warnings)."
     return 1
     fi
   fi
   JIRA_COMMENT="$JIRA_COMMENT
 
-    {color:green}+1 javadoc.  The applied patch does not increase the total number of javadoc warnings.{color:green}"
+    {color:green}+1 javadoc.{color}  The applied patch does not increase the total number of javadoc warnings."
   return 0
 }
 
@@ -476,7 +476,7 @@ checkJavacWarnings () {
   if [[ $? != 0 ]] ; then
     JIRA_COMMENT="$JIRA_COMMENT
 
-    {color:red}-1 javac.  The patch appears to cause the build to fail.{color:red}"
+    {color:red}-1 javac.{color}  The patch appears to cause the build to fail."
     return 2
   fi
   ### Compare trunk and patch javac warning numbers
@@ -492,7 +492,7 @@ checkJavacWarnings () {
       if [[ $patchJavacWarnings -gt $trunkJavacWarnings ]] ; then
         JIRA_COMMENT="$JIRA_COMMENT
 
-    {color:red}-1 javac.  The applied patch generated $patchJavacWarnings javac compiler warnings (more than the trunk's current $trunkJavacWarnings warnings).{color:red}"
+    {color:red}-1 javac.{color}  The applied patch generated $patchJavacWarnings javac compiler warnings (more than the trunk's current $trunkJavacWarnings warnings)."
 
     $DIFF $PATCH_DIR/filteredTrunkJavacWarnings.txt $PATCH_DIR/filteredPatchJavacWarnings.txt > $PATCH_DIR/diffJavacWarnings.txt 
         JIRA_COMMENT_FOOTER="Javac warnings: $BUILD_URL/artifact/incubator-tajo/patchprocess/diffJavacWarnings.txt
@@ -504,7 +504,7 @@ $JIRA_COMMENT_FOOTER"
   fi
   JIRA_COMMENT="$JIRA_COMMENT
 
-    {color:green}+1 javac.  The applied patch does not increase the total number of javac compiler warnings.{color:green}"
+    {color:green}+1 javac.{color}  The applied patch does not increase the total number of javac compiler warnings."
   return 0
 }
 
@@ -534,7 +534,7 @@ checkReleaseAuditWarnings () {
       if [[ $patchReleaseAuditWarnings -gt 0 ]] ; then
         JIRA_COMMENT="$JIRA_COMMENT
 
-    {color:red}-1 release audit.  The applied patch generated $patchReleaseAuditWarnings release audit warnings.{color:red}"
+    {color:red}-1 release audit.{color}  The applied patch generated $patchReleaseAuditWarnings release audit warnings."
         $GREP '\!?????' $PATCH_DIR/patchReleaseAuditWarnings.txt > $PATCH_DIR/patchReleaseAuditProblems.txt
         echo "Lines that start with ????? in the release audit report indicate files that do not have an Apache license header." >> $PATCH_DIR/patchReleaseAuditProblems.txt
         JIRA_COMMENT_FOOTER="Release audit warnings: $BUILD_URL/artifact/incubator-tajo/patchprocess/patchReleaseAuditProblems.txt
@@ -545,7 +545,7 @@ $JIRA_COMMENT_FOOTER"
   fi
   JIRA_COMMENT="$JIRA_COMMENT
 
-    {color:green}+1 release audit.  The applied patch does not increase the total number of release audit warnings.{color:green}"
+    {color:green}+1 release audit.{color}  The applied patch does not increase the total number of release audit warnings."
   return 0
 }
 
@@ -580,12 +580,12 @@ checkStyle () {
   if [[ $patchStyleErrors != 0 ]] ; then
     JIRA_COMMENT="$JIRA_COMMENT
 
-    {color:red}-1 checkstyle.  The patch generated $patchStyleErrors code style errors.{color:red}"
+    {color:red}-1 checkstyle.{color}  The patch generated $patchStyleErrors code style errors."
     return 1
   fi
   JIRA_COMMENT="$JIRA_COMMENT
 
-    {color:green}+1 checkstyle.  The patch generated 0 code style errors.{color:green}"
+    {color:green}+1 checkstyle.{color}  The patch generated 0 code style errors."
   return 0
 }
 
@@ -640,7 +640,7 @@ $JIRA_COMMENT_FOOTER"
 
     JIRA_COMMENT="$JIRA_COMMENT
 
-    {color:red}-1 findbugs.  The patch appears to cause Findbugs (version ${findbugs_version}) to fail.{color:red}"
+    {color:red}-1 findbugs.{color}  The patch appears to cause Findbugs (version ${findbugs_version}) to fail."
     return 1
   fi
     
@@ -673,12 +673,12 @@ $JIRA_COMMENT_FOOTER"
   if [[ $findbugsWarnings -gt 0 ]] ; then
     JIRA_COMMENT="$JIRA_COMMENT
 
-    {color:red}-1 findbugs.  The patch appears to introduce $findbugsWarnings new Findbugs (version ${findbugs_version}) warnings.{color:red}"
+    {color:red}-1 findbugs.{color}  The patch appears to introduce $findbugsWarnings new Findbugs (version ${findbugs_version}) warnings."
     return 1
   fi
   JIRA_COMMENT="$JIRA_COMMENT
 
-    {color:green}+1 findbugs.  The patch does not introduce any new Findbugs (version ${findbugs_version}) warnings.{color:green}"
+    {color:green}+1 findbugs.{color}  The patch does not introduce any new Findbugs (version ${findbugs_version}) warnings."
   return 0
 }
 
@@ -716,13 +716,12 @@ ${module_failed_tests}"
   if [[ -n "$failed_tests" ]] ; then
     JIRA_COMMENT="$JIRA_COMMENT
 
-    {color:red}-1 core tests.  The patch failed these unit tests in $modules:
-$failed_tests{color:red}"
+    {color:red}-1 core tests.{color}  The patch failed these unit tests in $modules: $failed_tests"
     return 1
   fi
   JIRA_COMMENT="$JIRA_COMMENT
 
-    {color:green}+1 core tests.  The patch passed unit tests in $modules.{color:green}"
+    {color:green}+1 core tests.{color}  The patch passed unit tests in $modules."
   return 0
 }
 
@@ -781,11 +780,11 @@ submitJiraComment () {
     JIRA_COMMENT_FOOTER=""
   fi
   if [[ $result == 0 ]] ; then
-    comment="{color:green}*+1 overall.*{color:green}  $JIRA_COMMENT
+    comment="{color:green}*+1 overall.*{color}  $JIRA_COMMENT
 
 $JIRA_COMMENT_FOOTER"
   else
-    comment="{color:red}*-1 overall.*{color:red}  $JIRA_COMMENT
+    comment="{color:red}*-1 overall.*{color}  $JIRA_COMMENT
 
 $JIRA_COMMENT_FOOTER"
   fi

http://git-wip-us.apache.org/repos/asf/incubator-tajo/blob/b822c288/pom.xml
----------------------------------------------------------------------
diff --git a/pom.xml b/pom.xml
index 5b0effb..bcec0cf 100644
--- a/pom.xml
+++ b/pom.xml
@@ -74,6 +74,7 @@
     <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
     <project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
     <tajo.version>0.8.0-SNAPSHOT</tajo.version>
+    <tajo.root>${basedir}</tajo.root>
   </properties>
 
   <modules>

http://git-wip-us.apache.org/repos/asf/incubator-tajo/blob/b822c288/tajo-core/tajo-core-backend/src/main/findbugs/findbugs-exclude.xml
----------------------------------------------------------------------
diff --git a/tajo-core/tajo-core-backend/src/main/findbugs/findbugs-exclude.xml b/tajo-core/tajo-core-backend/src/main/findbugs/findbugs-exclude.xml
deleted file mode 100644
index 52fe45c..0000000
--- a/tajo-core/tajo-core-backend/src/main/findbugs/findbugs-exclude.xml
+++ /dev/null
@@ -1,26 +0,0 @@
-<!--
-  Licensed to the Apache Software Foundation (ASF) under one
-  or more contributor license agreements.  See the NOTICE file
-  distributed with this work for additional information
-  regarding copyright ownership.  The ASF licenses this file
-  to you under the Apache License, Version 2.0 (the
-  "License"); you may not use this file except in compliance
-  with the License.  You may obtain a copy of the License at
-
-      http://www.apache.org/licenses/LICENSE-2.0
-
-  Unless required by applicable law or agreed to in writing, software
-  distributed under the License is distributed on an "AS IS" BASIS,
-  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-  See the License for the specific language governing permissions and
-  limitations under the License.
-  -->
-
-<FindBugsFilter>
-     <Match>
-		<Package name="~org\.apache\.tajo\.engine\.parser" />
-	</Match>
-	<Match>
-		<Class name="~.*\.*Protos" />
-	</Match>
-</FindBugsFilter>

http://git-wip-us.apache.org/repos/asf/incubator-tajo/blob/b822c288/tajo-core/tajo-core-storage/pom.xml
----------------------------------------------------------------------
diff --git a/tajo-core/tajo-core-storage/pom.xml b/tajo-core/tajo-core-storage/pom.xml
index 3f6f9c2..dcbde44 100644
--- a/tajo-core/tajo-core-storage/pom.xml
+++ b/tajo-core/tajo-core-storage/pom.xml
@@ -287,16 +287,6 @@
   <reporting>
     <plugins>
       <plugin>
-        <groupId>org.codehaus.mojo</groupId>
-        <artifactId>findbugs-maven-plugin</artifactId>
-        <version>2.3.2</version>
-        <configuration>
-          <xmlOutput>true</xmlOutput>
-          <xmlOutputDirectory>target/findbugs</xmlOutputDirectory>
-          <!--<excludeFilterFile>src/main/findbugs/findbugs-exclude.xml</excludeFilterFile>-->
-        </configuration>
-      </plugin>
-      <plugin>
         <groupId>org.apache.maven.plugins</groupId>
         <artifactId>maven-project-info-reports-plugin</artifactId>
         <version>2.4</version>

http://git-wip-us.apache.org/repos/asf/incubator-tajo/blob/b822c288/tajo-project/pom.xml
----------------------------------------------------------------------
diff --git a/tajo-project/pom.xml b/tajo-project/pom.xml
index b29a1a2..ee000d8 100644
--- a/tajo-project/pom.xml
+++ b/tajo-project/pom.xml
@@ -38,6 +38,7 @@
     <tajo.version>0.8.0-SNAPSHOT</tajo.version>
     <hadoop.version>2.2.0</hadoop.version>
     <protobuf.version>2.5.0</protobuf.version>
+    <tajo.root>${project.parent.relativePath}/..</tajo.root>
   </properties>
 
   <licenses>
@@ -393,6 +394,18 @@
           <artifactId>maven-surefire-report-plugin</artifactId>
           <version>2.15</version>
         </plugin>
+        <plugin>
+          <groupId>org.codehaus.mojo</groupId>
+          <artifactId>findbugs-maven-plugin</artifactId>
+          <version>2.3.2</version>
+          <configuration>
+            <findbugsXmlOutput>true</findbugsXmlOutput>
+            <xmlOutput>true</xmlOutput>
+            <effort>Max</effort>
+            <excludeFilterFile>${tajo.root}/dev-support/findbugs-exclude.xml</excludeFilterFile>
+          </configuration>
+        </plugin>
+
         <!--This plugin's configuration is used to store Eclipse m2e settings only. 
           It has no influence on the Maven build itself. -->
         <plugin>