You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-commits@hadoop.apache.org by aw...@apache.org on 2015/05/01 00:15:38 UTC
hadoop git commit: HADOOP-11866. increase readability and reliability
of checkstyle, shellcheck, and whitespace reports (aw)
Repository: hadoop
Updated Branches:
refs/heads/trunk f0db797be -> 5f8112ffd
HADOOP-11866. increase readability and reliability of checkstyle, shellcheck, and whitespace reports (aw)
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo
Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/5f8112ff
Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/5f8112ff
Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/5f8112ff
Branch: refs/heads/trunk
Commit: 5f8112ffd220598d997c92f681d3f69022898110
Parents: f0db797
Author: Allen Wittenauer <aw...@apache.org>
Authored: Thu Apr 30 15:15:32 2015 -0700
Committer: Allen Wittenauer <aw...@apache.org>
Committed: Thu Apr 30 15:15:32 2015 -0700
----------------------------------------------------------------------
dev-support/test-patch.d/checkstyle.sh | 216 ++++++++++++-------
dev-support/test-patch.d/shellcheck.sh | 52 ++++-
dev-support/test-patch.d/whitespace.sh | 12 +-
dev-support/test-patch.sh | 73 ++++++-
hadoop-common-project/hadoop-common/CHANGES.txt | 3 +
5 files changed, 255 insertions(+), 101 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hadoop/blob/5f8112ff/dev-support/test-patch.d/checkstyle.sh
----------------------------------------------------------------------
diff --git a/dev-support/test-patch.d/checkstyle.sh b/dev-support/test-patch.d/checkstyle.sh
index 460709e..6311584 100755
--- a/dev-support/test-patch.d/checkstyle.sh
+++ b/dev-support/test-patch.d/checkstyle.sh
@@ -29,8 +29,39 @@ function checkstyle_filefilter
fi
}
+function checkstyle_mvnrunner
+{
+ local logfile=$1
+ local output=$2
+ local tmp=${PATCH_DIR}/$$.${RANDOM}
+ local j
+
+ "${MVN}" clean test checkstyle:checkstyle -DskipTests \
+ -Dcheckstyle.consoleOutput=true \
+ "-D${PROJECT_NAME}PatchProcess" 2>&1 \
+ | tee "${logfile}" \
+ | ${GREP} ^/ \
+ | ${SED} -e "s,${BASEDIR},.,g" \
+ > "${tmp}"
+
+ # the checkstyle output files are massive, so
+ # let's reduce the work by filtering out files
+ # that weren't changed. Some modules are
+ # MASSIVE and this can cut the output down to
+ # by orders of magnitude!!
+ for j in ${CHANGED_FILES}; do
+ ${GREP} "${j}" "${tmp}" >> "${output}"
+ done
+
+ rm "${tmp}" 2>/dev/null
+}
+
function checkstyle_preapply
{
+ local module_suffix
+ local modules=${CHANGED_MODULES}
+ local module
+
verify_needed_test checkstyle
if [[ $? == 0 ]]; then
@@ -40,23 +71,78 @@ function checkstyle_preapply
big_console_header "checkstyle plugin: prepatch"
start_clock
- echo_and_redirect "${PATCH_DIR}/${PATCH_BRANCH}checkstyle.txt" "${MVN}" test checkstyle:checkstyle-aggregate -DskipTests "-D${PROJECT_NAME}PatchProcess"
- if [[ $? != 0 ]] ; then
- echo "Pre-patch ${PATCH_BRANCH} checkstyle compilation is broken?"
- add_jira_table -1 checkstyle "Pre-patch ${PATCH_BRANCH} checkstyle compilation may be broken."
- return 1
- fi
- cp -p "${BASEDIR}/target/checkstyle-result.xml" \
- "${PATCH_DIR}/checkstyle-result-${PATCH_BRANCH}.xml"
+ for module in ${modules}
+ do
+ pushd "${module}" >/dev/null
+ echo " Running checkstyle in ${module}"
+ module_suffix=$(basename "${module}")
+
+ checkstyle_mvnrunner \
+ "${PATCH_DIR}/maven-${PATCH_BRANCH}checkstyle-${module_suffix}.txt" \
+ "${PATCH_DIR}/${PATCH_BRANCH}checkstyle${module_suffix}.txt"
+
+ if [[ $? != 0 ]] ; then
+ echo "Pre-patch ${PATCH_BRANCH} checkstyle compilation is broken?"
+ add_jira_table -1 checkstyle "Pre-patch ${PATCH_BRANCH} ${module} checkstyle compilation may be broken."
+ fi
+ popd >/dev/null
+ done
# keep track of how much as elapsed for us already
CHECKSTYLE_TIMER=$(stop_clock)
return 0
}
+function checkstyle_calcdiffs
+{
+ local orig=$1
+ local new=$2
+ local diffout=$3
+ local tmp=${PATCH_DIR}/cs.$$.${RANDOM}
+ local count=0
+ local j
+
+ # first, pull out just the errors
+ # shellcheck disable=SC2016
+ ${AWK} -F: '{print $NF}' "${orig}" >> "${tmp}.branch"
+
+ # shellcheck disable=SC2016
+ ${AWK} -F: '{print $NF}' "${new}" >> "${tmp}.patch"
+
+ # compare the errors, generating a string of line
+ # numbers. Sorry portability: GNU diff makes this too easy
+ ${DIFF} --unchanged-line-format="" \
+ --old-line-format="" \
+ --new-line-format="%dn " \
+ "${tmp}.branch" \
+ "${tmp}.patch" > "${tmp}.lined"
+
+ # now, pull out those lines of the raw output
+ # shellcheck disable=SC2013
+ for j in $(cat "${tmp}.lined"); do
+ # shellcheck disable=SC2086
+ head -${j} "${new}" | tail -1 >> "${diffout}"
+ done
+
+ if [[ -f "${diffout}" ]]; then
+ # shellcheck disable=SC2016
+ count=$(wc -l "${diffout}" | ${AWK} '{print $1}' )
+ fi
+ rm "${tmp}.branch" "${tmp}.patch" "${tmp}.lined" 2>/dev/null
+ echo "${count}"
+}
+
function checkstyle_postapply
{
+ local rc=0
+ local module
+ local modules=${CHANGED_MODULES}
+ local module_suffix
+ local numprepatch=0
+ local numpostpatch=0
+ local diffpostpatch=0
+
verify_needed_test checkstyle
if [[ $? == 0 ]]; then
@@ -71,79 +157,49 @@ function checkstyle_postapply
# by setting the clock back
offset_clock "${CHECKSTYLE_TIMER}"
- echo_and_redirect "${PATCH_DIR}/patchcheckstyle.txt" "${MVN}" test checkstyle:checkstyle-aggregate -DskipTests "-D${PROJECT_NAME}PatchProcess"
- if [[ $? != 0 ]] ; then
- echo "Post-patch checkstyle compilation is broken."
- add_jira_table -1 checkstyle "Post-patch checkstyle compilation is broken."
- return 1
- fi
-
- cp -p "${BASEDIR}/target/checkstyle-result.xml" \
- "${PATCH_DIR}/checkstyle-result-patch.xml"
-
- checkstyle_runcomparison
-
- # shellcheck disable=SC2016
- CHECKSTYLE_POSTPATCH=$(wc -l "${PATCH_DIR}/checkstyle-result-diff.txt" | ${AWK} '{print $1}')
-
- if [[ ${CHECKSTYLE_POSTPATCH} -gt 0 ]] ; then
-
- add_jira_table -1 checkstyle "The applied patch generated "\
- "${CHECKSTYLE_POSTPATCH}" \
- " additional checkstyle issues."
- add_jira_footer checkstyle "@@BASE@@/checkstyle-result-diff.txt"
-
+ for module in ${modules}
+ do
+ pushd "${module}" >/dev/null
+ echo " Running checkstyle in ${module}"
+ module_suffix=$(basename "${module}")
+
+ checkstyle_mvnrunner \
+ "${PATCH_DIR}/maven-patchcheckstyle-${module_suffix}.txt" \
+ "${PATCH_DIR}/patchcheckstyle${module_suffix}.txt"
+
+ if [[ $? != 0 ]] ; then
+ ((rc = rc +1))
+ echo "Post-patch checkstyle compilation is broken."
+ add_jira_table -1 checkstyle "Post-patch checkstyle ${module} compilation is broken."
+ continue
+ fi
+
+ #shellcheck disable=SC2016
+ diffpostpatch=$(checkstyle_calcdiffs \
+ "${PATCH_DIR}/${PATCH_BRANCH}checkstyle${module_suffix}.txt" \
+ "${PATCH_DIR}/patchcheckstyle${module_suffix}.txt" \
+ "${PATCH_DIR}/diffcheckstyle${module_suffix}.txt" )
+
+ if [[ ${diffpostpatch} -gt 0 ]] ; then
+ ((rc = rc + 1))
+
+ # shellcheck disable=SC2016
+ numprepatch=$(wc -l "${PATCH_DIR}/${PATCH_BRANCH}checkstyle${module_suffix}.txt" | ${AWK} '{print $1}')
+ # shellcheck disable=SC2016
+ numpostpatch=$(wc -l "${PATCH_DIR}/patchcheckstyle${module_suffix}.txt" | ${AWK} '{print $1}')
+
+ add_jira_table -1 checkstyle "The applied patch generated "\
+ "${diffpostpatch} new checkstyle issues (total was ${numprepatch}, now ${numpostpatch})."
+ footer="${footer} @@BASE@@/diffcheckstyle${module_suffix}.txt"
+ fi
+
+ popd >/dev/null
+ done
+
+ if [[ ${rc} -gt 0 ]] ; then
+ add_jira_footer checkstyle "${footer}"
return 1
fi
add_jira_table +1 checkstyle "There were no new checkstyle issues."
return 0
-}
-
-
-function checkstyle_runcomparison
-{
-
- python <(cat <<EOF
-import os
-import sys
-import xml.etree.ElementTree as etree
-from collections import defaultdict
-
-if len(sys.argv) != 3 :
- print "usage: %s checkstyle-result-master.xml checkstyle-result-patch.xml" % sys.argv[0]
- exit(1)
-
-def path_key(x):
- path = x.attrib['name']
- return path[path.find('${PROJECT_NAME}-'):]
-
-def print_row(path, master_errors, patch_errors):
- print '%s\t%s\t%s' % (k,master_dict[k],child_errors)
-
-master = etree.parse(sys.argv[1])
-patch = etree.parse(sys.argv[2])
-
-master_dict = defaultdict(int)
-
-for child in master.getroot().getchildren():
- if child.tag != 'file':
- continue
- child_errors = len(child.getchildren())
- if child_errors == 0:
- continue
- master_dict[path_key(child)] = child_errors
-
-for child in patch.getroot().getchildren():
- if child.tag != 'file':
- continue
- child_errors = len(child.getchildren())
- if child_errors == 0:
- continue
- k = path_key(child)
- if child_errors > master_dict[k]:
- print_row(k, master_dict[k], child_errors)
-
-EOF
-) "${PATCH_DIR}/checkstyle-result-${PATCH_BRANCH}.xml" "${PATCH_DIR}/checkstyle-result-patch.xml" > "${PATCH_DIR}/checkstyle-result-diff.txt"
-
-}
+}
\ No newline at end of file
http://git-wip-us.apache.org/repos/asf/hadoop/blob/5f8112ff/dev-support/test-patch.d/shellcheck.sh
----------------------------------------------------------------------
diff --git a/dev-support/test-patch.d/shellcheck.sh b/dev-support/test-patch.d/shellcheck.sh
index c1084a2..5f38b6a 100755
--- a/dev-support/test-patch.d/shellcheck.sh
+++ b/dev-support/test-patch.d/shellcheck.sh
@@ -87,6 +87,45 @@ function shellcheck_preapply
return 0
}
+function shellcheck_calcdiffs
+{
+ local orig=$1
+ local new=$2
+ local diffout=$3
+ local tmp=${PATCH_DIR}/sc.$$.${RANDOM}
+ local count=0
+ local j
+
+ # first, pull out just the errors
+ # shellcheck disable=SC2016
+ ${AWK} -F: '{print $NF}' "${orig}" >> "${tmp}.branch"
+
+ # shellcheck disable=SC2016
+ ${AWK} -F: '{print $NF}' "${new}" >> "${tmp}.patch"
+
+ # compare the errors, generating a string of line
+ # numbers. Sorry portability: GNU diff makes this too easy
+ ${DIFF} --unchanged-line-format="" \
+ --old-line-format="" \
+ --new-line-format="%dn " \
+ "${tmp}.branch" \
+ "${tmp}.patch" > "${tmp}.lined"
+
+ # now, pull out those lines of the raw output
+ # shellcheck disable=SC2013
+ for j in $(cat "${tmp}.lined"); do
+ # shellcheck disable=SC2086
+ head -${j} "${new}" | tail -1 >> "${diffout}"
+ done
+
+ if [[ -f "${diffout}" ]]; then
+ # shellcheck disable=SC2016
+ count=$(wc -l "${diffout}" | ${AWK} '{print $1}' )
+ fi
+ rm "${tmp}.branch" "${tmp}.patch" "${tmp}.lined" 2>/dev/null
+ echo "${count}"
+}
+
function shellcheck_postapply
{
local i
@@ -121,16 +160,13 @@ function shellcheck_postapply
# shellcheck disable=SC2016
numPostpatch=$(wc -l "${PATCH_DIR}/patchshellcheck-result.txt" | ${AWK} '{print $1}')
- ${DIFF} -u "${PATCH_DIR}/${PATCH_BRANCH}shellcheck-result.txt" \
+ diffPostpatch=$(shellcheck_calcdiffs \
+ "${PATCH_DIR}/${PATCH_BRANCH}shellcheck-result.txt" \
"${PATCH_DIR}/patchshellcheck-result.txt" \
- | ${GREP} '^+\.' \
- > "${PATCH_DIR}/diffpatchshellcheck.txt"
-
- # shellcheck disable=SC2016
- diffPostpatch=$(wc -l "${PATCH_DIR}/diffpatchshellcheck.txt" | ${AWK} '{print $1}')
+ "${PATCH_DIR}/diffpatchshellcheck.txt"
+ )
- if [[ ${diffPostpatch} -gt 0
- && ${numPostpatch} -gt ${numPrepatch} ]] ; then
+ if [[ ${diffPostpatch} -gt 0 ]] ; then
add_jira_table -1 shellcheck "The applied patch generated "\
"${diffPostpatch} new shellcheck (v${SHELLCHECK_VERSION}) issues (total was ${numPrepatch}, now ${numPostpatch})."
add_jira_footer shellcheck "@@BASE@@/diffpatchshellcheck.txt"
http://git-wip-us.apache.org/repos/asf/hadoop/blob/5f8112ff/dev-support/test-patch.d/whitespace.sh
----------------------------------------------------------------------
diff --git a/dev-support/test-patch.d/whitespace.sh b/dev-support/test-patch.d/whitespace.sh
index deac654..324481c 100755
--- a/dev-support/test-patch.d/whitespace.sh
+++ b/dev-support/test-patch.d/whitespace.sh
@@ -16,25 +16,31 @@
add_plugin whitespace
-function whitespace_preapply
+function whitespace_postapply
{
local count
+ local j
big_console_header "Checking for whitespace at the end of lines"
start_clock
- ${GREP} '^+' "${PATCH_DIR}/patch" | ${GREP} '[[:blank:]]$' > "${PATCH_DIR}/whitespace.txt"
+ pushd "${BASEDIR}" >/dev/null
+ for j in ${CHANGED_FILES}; do
+ ${GREP} -nHE '[[:blank:]]$' "./${j}" | ${GREP} -f "${GITDIFFLINES}" >> "${PATCH_DIR}/whitespace.txt"
+ done
# shellcheck disable=SC2016
count=$(wc -l "${PATCH_DIR}/whitespace.txt" | ${AWK} '{print $1}')
if [[ ${count} -gt 0 ]]; then
add_jira_table -1 whitespace "The patch has ${count}"\
- " line(s) that end in whitespace."
+ " line(s) that end in whitespace. Use git apply --whitespace=fix."
add_jira_footer whitespace "@@BASE@@/whitespace.txt"
+ popd >/dev/null
return 1
fi
+ popd >/dev/null
add_jira_table +1 whitespace "The patch has no lines that end in whitespace."
return 0
}
http://git-wip-us.apache.org/repos/asf/hadoop/blob/5f8112ff/dev-support/test-patch.sh
----------------------------------------------------------------------
diff --git a/dev-support/test-patch.sh b/dev-support/test-patch.sh
index ae21837..b6e1b03 100755
--- a/dev-support/test-patch.sh
+++ b/dev-support/test-patch.sh
@@ -67,7 +67,7 @@ function setup_defaults
EGREP=${EGREP:-/usr/xpg4/bin/egrep}
GREP=${GREP:-/usr/xpg4/bin/grep}
PATCH=${PATCH:-patch}
- DIFF=${DIFF:-diff}
+ DIFF=${DIFF:-/usr/gnu/bin/diff}
JIRACLI=${JIRA:-jira}
;;
*)
@@ -449,7 +449,7 @@ function verify_patchdir_still_exists
if [[ ! -d ${PATCH_DIR} ]]; then
rm "${commentfile}" 2>/dev/null
- echo "(!) The patch artifact directory on has been removed! " > "${commentfile}"
+ echo "(!) The patch artifact directory has been removed! " > "${commentfile}"
echo "This is a fatal error for test-patch.sh. Aborting. " >> "${commentfile}"
echo
cat ${commentfile}
@@ -468,6 +468,49 @@ function verify_patchdir_still_exists
fi
}
+## @description generate a list of all files and line numbers that
+## @description that were added/changed in the source repo
+## @audience private
+## @stability stable
+## @params filename
+## @replaceable no
+function compute_gitdiff
+{
+ local outfile=$1
+ local file
+ local line
+ local startline
+ local counter
+ local numlines
+ local actual
+
+ pushd "${BASEDIR}" >/dev/null
+ while read line; do
+ if [[ ${line} =~ ^\+\+\+ ]]; then
+ file="./"$(echo "${line}" | cut -f2- -d/)
+ continue
+ elif [[ ${line} =~ ^@@ ]]; then
+ startline=$(echo "${line}" | cut -f3 -d' ' | cut -f1 -d, | tr -d + )
+ numlines=$(echo "${line}" | cut -f3 -d' ' | cut -s -f2 -d, )
+ # if this is empty, then just this line
+ # if it is 0, then no lines were added and this part of the patch
+ # is strictly a delete
+ if [[ ${numlines} == 0 ]]; then
+ continue
+ elif [[ -z ${numlines} ]]; then
+ numlines=1
+ fi
+ counter=0
+ until [[ ${counter} -gt ${numlines} ]]; do
+ ((actual=counter+startline))
+ echo "${file}:${actual}:" >> "${outfile}"
+ ((counter=counter+1))
+ done
+ fi
+ done < <("${GIT}" diff --unified=0 --no-color)
+ popd >/dev/null
+}
+
## @description Print the command to be executing to the screen. Then
## @description run the command, sending stdout and stderr to the given filename
## @description This will also ensure that any directories in ${BASEDIR} have
@@ -481,7 +524,7 @@ function verify_patchdir_still_exists
## @returns $?
function echo_and_redirect
{
- logfile=$1
+ local logfile=$1
shift
verify_patchdir_still_exists
@@ -522,7 +565,7 @@ function hadoop_usage
echo "Shell binary overrides:"
echo "--awk-cmd=<cmd> The 'awk' command to use (default 'awk')"
- echo "--diff-cmd=<cmd> The 'diff' command to use (default 'diff')"
+ echo "--diff-cmd=<cmd> The GNU-compatible 'diff' command to use (default 'diff')"
echo "--git-cmd=<cmd> The 'git' command to use (default 'git')"
echo "--grep-cmd=<cmd> The 'grep' command to use (default 'grep')"
echo "--mvn-cmd=<cmd> The 'mvn' command to use (default \${MAVEN_HOME}/bin/mvn, or 'mvn')"
@@ -585,6 +628,10 @@ function parse_args
--grep-cmd=*)
GREP=${i#*=}
;;
+ --help|-help|-h|help|--h|--\?|-\?|\?)
+ hadoop_usage
+ exit 0
+ ;;
--java-home)
JAVA_HOME=${i#*=}
;;
@@ -680,6 +727,8 @@ function parse_args
cleanup_and_exit 1
fi
fi
+
+ GITDIFFLINES=${PATCH_DIR}/gitdifflines.txt
}
## @description Locate the pom.xml file for a given directory
@@ -716,12 +765,14 @@ function find_changed_files
# get a list of all of the files that have been changed,
# except for /dev/null (which would be present for new files).
# Additionally, remove any a/ b/ patterns at the front
- # of the patch filenames
+ # of the patch filenames and any revision info at the end
+ # shellcheck disable=SC2016
CHANGED_FILES=$(${GREP} -E '^(\+\+\+|---) ' "${PATCH_DIR}/patch" \
| ${SED} \
-e 's,^....,,' \
-e 's,^[ab]/,,' \
| ${GREP} -v /dev/null \
+ | ${AWK} '{print $1}' \
| sort -u)
}
@@ -1552,7 +1603,7 @@ function check_javac
> "${PATCH_DIR}/diffJavacWarnings.txt"
add_jira_table -1 javac "The applied patch generated "\
- "$((patchJavacWarnings-branchJavacWarnings))" \
+ "$((patchJavacWarnings-${PATCH_BRANCH}JavacWarnings))" \
" additional warning messages."
add_jira_footer javac "@@BASE@@/diffJavacWarnings.txt"
@@ -1712,6 +1763,7 @@ function check_findbugs
"${PATCH_DIR}/patchFindbugsWarnings${module_suffix}.xml" \
"${PATCH_DIR}/patchFindbugsWarnings${module_suffix}.xml"
+ #shellcheck disable=SC2016
newFindbugsWarnings=$("${FINDBUGS_HOME}/bin/filterBugs" \
-first "01/01/2000" "${PATCH_DIR}/patchFindbugsWarnings${module_suffix}.xml" \
"${PATCH_DIR}/newPatchFindbugsWarnings${module_suffix}.xml" \
@@ -1887,10 +1939,12 @@ function check_unittests
test_timeouts="${test_timeouts} ${module_test_timeouts}"
result=1
fi
- #shellcheck disable=SC2026,SC2038
+
+ #shellcheck disable=SC2026,SC2038,SC2016
module_failed_tests=$(find . -name 'TEST*.xml'\
| xargs "${GREP}" -l -E "<failure|<error"\
| ${AWK} -F/ '{sub("TEST-org.apache.",""); sub(".xml",""); print $NF}')
+
if [[ -n "${module_failed_tests}" ]] ; then
failed_tests="${failed_tests} ${module_failed_tests}"
result=1
@@ -2054,8 +2108,6 @@ function output_to_console
printf "%s\n" "${comment}"
((i=i+1))
done
-
-
}
## @description Print out the finished details to the JIRA issue
@@ -2189,7 +2241,6 @@ function postcheckout
#shellcheck disable=SC2086
${plugin}_postcheckout
-
(( RESULT = RESULT + $? ))
if [[ ${RESULT} != 0 ]] ; then
output_to_console 1
@@ -2244,6 +2295,8 @@ function postapply
local plugin
local retval
+ compute_gitdiff "${GITDIFFLINES}"
+
check_javac
retval=$?
if [[ ${retval} -gt 1 ]] ; then
http://git-wip-us.apache.org/repos/asf/hadoop/blob/5f8112ff/hadoop-common-project/hadoop-common/CHANGES.txt
----------------------------------------------------------------------
diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt
index d2e1d4a..0a53396 100644
--- a/hadoop-common-project/hadoop-common/CHANGES.txt
+++ b/hadoop-common-project/hadoop-common/CHANGES.txt
@@ -582,6 +582,9 @@ Release 2.8.0 - UNRELEASED
HADOOP-11821. Fix findbugs warnings in hadoop-sls.
(Brahma Reddy Battula via aajisaka)
+ HADOOP-11866. increase readability and reliability of checkstyle,
+ shellcheck, and whitespace reports (aw)
+
Release 2.7.1 - UNRELEASED
INCOMPATIBLE CHANGES