Repository: yetus Updated Branches: refs/heads/master 1a5683f39 -> 8e55427b3
YETUS-187. maven javac/javadoc can't use calcdiffs Signed-off-by: Kengo Seki <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/yetus/repo Commit: http://git-wip-us.apache.org/repos/asf/yetus/commit/8e55427b Tree: http://git-wip-us.apache.org/repos/asf/yetus/tree/8e55427b Diff: http://git-wip-us.apache.org/repos/asf/yetus/diff/8e55427b Branch: refs/heads/master Commit: 8e55427b35925982fd991297a05671eece2bc0d9 Parents: 1a5683f Author: Allen Wittenauer <[email protected]> Authored: Wed Dec 16 05:57:32 2015 -0800 Committer: Kengo Seki <[email protected]> Committed: Tue Dec 29 05:38:32 2015 +0900 ---------------------------------------------------------------------- .../in-progress/precommit-advanced.md | 6 +- .../in-progress/precommit-buildtools.md | 4 + precommit/test-patch.d/checkstyle.sh | 120 +++++++++++++++++-- precommit/test-patch.d/maven.sh | 67 +++++++++++ precommit/test-patch.d/perlcritic.sh | 18 ++- precommit/test-patch.d/rubocop.sh | 18 ++- precommit/test-patch.d/ruby-lint.sh | 49 +++++++- precommit/test-patch.d/shellcheck.sh | 13 ++ precommit/test-patch.sh | 93 ++++++++++++-- 9 files changed, 367 insertions(+), 21 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/yetus/blob/8e55427b/asf-site-src/source/documentation/in-progress/precommit-advanced.md ---------------------------------------------------------------------- diff --git a/asf-site-src/source/documentation/in-progress/precommit-advanced.md b/asf-site-src/source/documentation/in-progress/precommit-advanced.md index 878821d..a864cf8 100644 --- a/asf-site-src/source/documentation/in-progress/precommit-advanced.md +++ b/asf-site-src/source/documentation/in-progress/precommit-advanced.md @@ -111,7 +111,7 @@ If the `--skip-system-plugins` flag is passed, then only core.d is imported. ## Test Plug-ins -Plug-ins geared towards independent tests are registed via: +Plug-ins geared towards independent tests are registered via: ```bash @@ -124,6 +124,10 @@ add_test_type <pluginname> * pluginname\_tests - executed after the unit tests have completed. +* pluginname\_calcdiffs + + - This allows for custom log file difference calculation used to determine the before and after views. The default is to use the last column of a colon delimited line of output and perform a diff. If the plug-in does not provide enough context, this may result in error skew. For example, if three lines in a row have "Missing period." as the error, test-patch will not be able to determine exactly which line caused the error. Plug-ins that have this issue will want to use this or potentially modify the normal tool's output (e.g., checkstyle) to provide a more accurate way to determine differences. + # Personalities http://git-wip-us.apache.org/repos/asf/yetus/blob/8e55427b/asf-site-src/source/documentation/in-progress/precommit-buildtools.md ---------------------------------------------------------------------- diff --git a/asf-site-src/source/documentation/in-progress/precommit-buildtools.md b/asf-site-src/source/documentation/in-progress/precommit-buildtools.md index 923bb86..84a8309 100644 --- a/asf-site-src/source/documentation/in-progress/precommit-buildtools.md +++ b/asf-site-src/source/documentation/in-progress/precommit-buildtools.md @@ -78,6 +78,10 @@ For example, the gradle build tool does not have a standard way to execute check - Certain language test plug-ins require assistance from the build tool to count problems in the compile log due to some tools having custom handling for those languages. The test plug-in name should be in the (test) part of the function name. +* pluginname\_(test)_calcdiffs + + - Some build tools (e.g., maven) use custom output for certain types of compilations (e.g., java). This allows for custom log file difference calculation used to determine the before and after views. + * pluginname\_docker\_support - If this build tool requires extra settings on the `docker run` command line, this function should be defined and write those options into a file called `${PATCH_DIR}/buildtool-docker-params.txt`. This is particularly useful for things like mounting volumes for repository caches. http://git-wip-us.apache.org/repos/asf/yetus/blob/8e55427b/precommit/test-patch.d/checkstyle.sh ---------------------------------------------------------------------- diff --git a/precommit/test-patch.d/checkstyle.sh b/precommit/test-patch.d/checkstyle.sh index 1d45819..09bdbf1 100755 --- a/precommit/test-patch.d/checkstyle.sh +++ b/precommit/test-patch.d/checkstyle.sh @@ -62,7 +62,52 @@ function checkstyle_parse_args done } +## @description checkstyle plug-in specific difference calculator +## @audience private +## @stability evolving +## @replaceable no +## @param branchlog +## @param patchlog +## @return differences +function checkstyle_calcdiffs +{ + declare orig=$1 + declare new=$2 + declare tmp=${PATCH_DIR}/pl.$$.${RANDOM} + declare j + + # first, strip filenames:line: + # this keeps column: in an attempt to increase + # accuracy in case of multiple, repeated errors + # since the column number shouldn't change + # if the line of code hasn't been touched + # shellcheck disable=SC2016 + cut -f3- -d: "${orig}" > "${tmp}.branch" + # shellcheck disable=SC2016 + cut -f3- -d: "${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 + done + + rm "${tmp}.branch" "${tmp}.patch" "${tmp}.lined" 2>/dev/null +} +## @description execute checkstyle +## @audience private +## @stability stable +## @replaceable no function checkstyle_runner { local repostatus=$1 @@ -77,7 +122,13 @@ function checkstyle_runner local repo local modulesuffix local cmd + local logline + local text + local linenum + local codeline + + # first, let's clear out any previous run information modules_reset if [[ ${repostatus} == branch ]]; then @@ -86,14 +137,18 @@ function checkstyle_runner repo="the patch" fi + # loop through the modules we've been given #shellcheck disable=SC2153 until [[ $i -eq ${#MODULE[@]} ]]; do + + # start the clock per module, setup some help vars, etc start_clock fn=$(module_file_fragment "${MODULE[${i}]}") modulesuffix=$(basename "${MODULE[${i}]}") output="${PATCH_DIR}/${repostatus}-checkstyle-${fn}.txt" logfile="${PATCH_DIR}/maven-${repostatus}-checkstyle-${fn}.txt" + # set up the command line, etc, to build... if [[ ${BUILDTOOLCWD} == true ]]; then pushd "${BASEDIR}/${MODULE[${i}]}" >/dev/null fi @@ -117,6 +172,10 @@ function checkstyle_runner ;; esac + # we're going to execute it and pull out + # anything that beings with a /. that's + # almost certainly checkstyle output + #shellcheck disable=SC2086 echo ${cmd} "> ${logfile}" #shellcheck disable=SC2086 @@ -132,16 +191,58 @@ function checkstyle_runner module_status ${i} -1 "${logfile}" "${modulesuffix} in ${repo} failed checkstyle" ((result = result + 1)) fi + + # if we have some output, we need to do more work: + if [[ -s ${tmp} ]]; then + + # first, let's pull out all of the files that + # we actually care about, esp since that run + # above is likely from the entire source + # this will grealy cut down how much work we + # have to do later + + for j in ${CHANGED_FILES}; do + ${GREP} "${j}" "${tmp}" >> "${tmp}.1" + done + + # now that we have just the files we care about, + # let's unscrew it. You see... + + # checkstyle seems to do everything it possibly can + # to make it hard to process, including inconsistent + # output (sometimes it has columns, sometimes it doesn't!) + # and giving very generic errors when context would be + # helpful, esp when doing diffs. + + # in order to help calcdiff and the user out, we're + # going to reprocess the output to include the code + # line being flagged. When calcdiff gets a hold of this + # it will have the code to act as context to help + # report the correct line + + # file:linenum:(column:)error ====> + # file:linenum:code(:column):error + pushd "${BASEDIR}" >/dev/null + while read -r logline; do + file=$(echo "${logline}" | cut -f1 -d:) + linenum=$(echo "${logline}" | cut -f2 -d:) + text=$(echo "${logline}" | cut -f3- -d:) + codeline=$(head -n "+${linenum}" "${file}" | tail -1 ) + echo "${file}:${linenum}:${codeline}:${text}" >> "${output}" + done < <(cat "${tmp}.1") + + popd >/dev/null + # later on, calcdiff will turn this into code(:column):error + # compare, and then put the file:line back onto it. + + fi + + rm "${tmp}" "${tmp}.1" 2>/dev/null + savestop=$(stop_clock) #shellcheck disable=SC2034 MODULE_STATUS_TIMER[${i}]=${savestop} - for j in ${CHANGED_FILES}; do - ${GREP} "${j}" "${tmp}" >> "${output}" - done - - rm "${tmp}" 2>/dev/null - if [[ ${BUILDTOOLCWD} == true ]]; then popd >/dev/null fi @@ -238,7 +339,12 @@ function checkstyle_postapply touch "${PATCH_DIR}/branch-checkstyle-${fn}.txt" fi - calcdiffs "${PATCH_DIR}/branch-checkstyle-${fn}.txt" "${PATCH_DIR}/patch-checkstyle-${fn}.txt" > "${PATCH_DIR}/diff-checkstyle-${fn}.txt" + # call calcdiffs to allow overrides + calcdiffs \ + "${PATCH_DIR}/branch-checkstyle-${fn}.txt" \ + "${PATCH_DIR}/patch-checkstyle-${fn}.txt" \ + checkstyle \ + > "${PATCH_DIR}/diff-checkstyle-${fn}.txt" #shellcheck disable=SC2016 diffpostpatch=$(wc -l "${PATCH_DIR}/diff-checkstyle-${fn}.txt" | ${AWK} '{print $1}') http://git-wip-us.apache.org/repos/asf/yetus/blob/8e55427b/precommit/test-patch.d/maven.sh ---------------------------------------------------------------------- diff --git a/precommit/test-patch.d/maven.sh b/precommit/test-patch.d/maven.sh index 5eb5f73..435c7a1 100755 --- a/precommit/test-patch.d/maven.sh +++ b/precommit/test-patch.d/maven.sh @@ -243,6 +243,73 @@ function maven_javadoc_logfilter ${GREP} -E '\[(ERROR|WARNING)\] /.*\.java:' "${input}" > "${output}" } +## @description Calculate the differences between the specified files +## @description and output it to stdout, the maven way +## @audience private +## @stability evolving +## @replaceable no +## @param branchlog +## @param patchlog +## @return differences +function maven_java_calcdiffs +{ + declare orig=$1 + declare new=$2 + declare tmp=${PATCH_DIR}/pl.$$.${RANDOM} + declare j + + # first, strip :[line + # this keeps file,column in an attempt to increase + # accuracy in case of multiple, repeated errors + # since the column number shouldn't change + # if the line of code hasn't been touched + # shellcheck disable=SC2016 + ${SED} -e 's#:\[[0-9]*,#:#' "${orig}" > "${tmp}.branch" + # shellcheck disable=SC2016 + ${SED} -e 's#:\[[0-9]*,#:#' "${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 + done + + rm "${tmp}.branch" "${tmp}.patch" "${tmp}.lined" 2>/dev/null +} + +## @description handle diffing maven javac errors +## @audience private +## @stability evolving +## @replaceable no +## @param branchlog +## @param patchlog +## @return differences +function maven_javac_calcdiffs +{ + maven_java_calcdiffs "$@" +} + +## @description handle diffing maven javadoc errors +## @audience private +## @stability evolving +## @replaceable no +## @param branchlog +## @param patchlog +## @return differences +function maven_javadoc_calcdiffs +{ + maven_java_calcdiffs "$@" +} + function maven_builtin_personality_modules { local repostatus=$1 http://git-wip-us.apache.org/repos/asf/yetus/blob/8e55427b/precommit/test-patch.d/perlcritic.sh ---------------------------------------------------------------------- diff --git a/precommit/test-patch.d/perlcritic.sh b/precommit/test-patch.d/perlcritic.sh index d97326d..98e10e5 100755 --- a/precommit/test-patch.d/perlcritic.sh +++ b/precommit/test-patch.d/perlcritic.sh @@ -82,6 +82,18 @@ function perlcritic_preapply return 0 } +## @description Wrapper to call column_calcdiffs +## @audience private +## @stability evolving +## @replaceable no +## @param branchlog +## @param patchlog +## @return differences +function perlcritic_calcdiffs +{ + column_calcdiffs "$@" +} + function perlcritic_postapply { local i @@ -115,7 +127,11 @@ function perlcritic_postapply PERLCRITIC_VERSION=$(${PERLCRITIC} --version 2>/dev/null) add_footer_table perlcritic "v${PERLCRITIC_VERSION}" - calcdiffs "${PATCH_DIR}/branch-perlcritic-result.txt" "${PATCH_DIR}/patch-perlcritic-result.txt" > "${PATCH_DIR}/diff-patch-perlcritic.txt" + calcdiffs \ + "${PATCH_DIR}/branch-perlcritic-result.txt" \ + "${PATCH_DIR}/patch-perlcritic-result.txt" \ + perlcritic \ + > "${PATCH_DIR}/diff-patch-perlcritic.txt" # shellcheck disable=SC2016 diffPostpatch=$(wc -l "${PATCH_DIR}/diff-patch-perlcritic.txt" | ${AWK} '{print $1}') http://git-wip-us.apache.org/repos/asf/yetus/blob/8e55427b/precommit/test-patch.d/rubocop.sh ---------------------------------------------------------------------- diff --git a/precommit/test-patch.d/rubocop.sh b/precommit/test-patch.d/rubocop.sh index ddea0ca..94cb3a4 100755 --- a/precommit/test-patch.d/rubocop.sh +++ b/precommit/test-patch.d/rubocop.sh @@ -82,6 +82,18 @@ function rubocop_preapply return 0 } +## @description Wrapper to call column_calcdiffs +## @audience private +## @stability evolving +## @replaceable no +## @param branchlog +## @param patchlog +## @return differences +function rubocop_calcdiffs +{ + column_calcdiffs "$@" +} + function rubocop_postapply { local i @@ -116,7 +128,11 @@ function rubocop_postapply RUBOCOP_VERSION=$(${RUBOCOP} -v | ${AWK} '{print $NF}') add_footer_table rubocop "v${RUBOCOP_VERSION}" - calcdiffs "${PATCH_DIR}/branch-rubocop-result.txt" "${PATCH_DIR}/patch-rubocop-result.txt" > "${PATCH_DIR}/diff-patch-rubocop.txt" + calcdiffs \ + "${PATCH_DIR}/branch-rubocop-result.txt" \ + "${PATCH_DIR}/patch-rubocop-result.txt" \ + rubocop \ + > "${PATCH_DIR}/diff-patch-rubocop.txt" diffPostpatch=$(${AWK} -F: 'BEGIN {sum=0} 4<NF {sum+=1} END {print sum}' "${PATCH_DIR}/diff-patch-rubocop.txt") if [[ ${diffPostpatch} -gt 0 ]] ; then http://git-wip-us.apache.org/repos/asf/yetus/blob/8e55427b/precommit/test-patch.d/ruby-lint.sh ---------------------------------------------------------------------- diff --git a/precommit/test-patch.d/ruby-lint.sh b/precommit/test-patch.d/ruby-lint.sh index 862c5b4..eddc48f 100755 --- a/precommit/test-patch.d/ruby-lint.sh +++ b/precommit/test-patch.d/ruby-lint.sh @@ -81,6 +81,49 @@ function ruby_lint_preapply return 0 } +## @description Calculate the differences between the specified files +## @description using columns and output it to stdout +## @audience private +## @stability evolving +## @replaceable no +## @param branchlog +## @param patchlog +## @return differences +function ruby_lint_calcdiffs +{ + declare orig=$1 + declare new=$2 + declare tmp=${PATCH_DIR}/pl.$$.${RANDOM} + declare j + + # first, strip filenames:line: + # this keeps column: in an attempt to increase + # accuracy in case of multiple, repeated errors + # since the column number shouldn't change + # if the line of code hasn't been touched + # shellcheck disable=SC2016 + cut -f4- -d: "${orig}" > "${tmp}.branch" + # shellcheck disable=SC2016 + cut -f4- -d: "${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 + done + + rm "${tmp}.branch" "${tmp}.patch" "${tmp}.lined" 2>/dev/null +} + function ruby_lint_postapply { local i @@ -115,7 +158,11 @@ function ruby_lint_postapply RUBY_LINT_VERSION=$(${RUBY_LINT} -v | ${AWK} '{print $2}') add_footer_table ruby-lint "${RUBY_LINT_VERSION}" - calcdiffs "${PATCH_DIR}/branch-ruby-lint-result.txt" "${PATCH_DIR}/patch-ruby-lint-result.txt" > "${PATCH_DIR}/diff-patch-ruby-lint.txt" + calcdiffs \ + "${PATCH_DIR}/branch-ruby-lint-result.txt" \ + "${PATCH_DIR}/patch-ruby-lint-result.txt" \ + ruby_lint \ + > "${PATCH_DIR}/diff-patch-ruby-lint.txt" diffPostpatch=$(${AWK} -F: 'BEGIN {sum=0} 4<NF {sum+=1} END {print sum}' "${PATCH_DIR}/diff-patch-ruby-lint.txt") if [[ ${diffPostpatch} -gt 0 ]] ; then http://git-wip-us.apache.org/repos/asf/yetus/blob/8e55427b/precommit/test-patch.d/shellcheck.sh ---------------------------------------------------------------------- diff --git a/precommit/test-patch.d/shellcheck.sh b/precommit/test-patch.d/shellcheck.sh index 9043acb..c42822d 100755 --- a/precommit/test-patch.d/shellcheck.sh +++ b/precommit/test-patch.d/shellcheck.sh @@ -95,6 +95,18 @@ function shellcheck_preapply return 0 } +## @description Wrapper to call column_calcdiffs +## @audience private +## @stability evolving +## @replaceable no +## @param branchlog +## @param patchlog +## @return differences +function shellcheck_calcdiffs +{ + column_calcdiffs "$@" +} + function shellcheck_postapply { local i @@ -139,6 +151,7 @@ function shellcheck_postapply calcdiffs \ "${PATCH_DIR}/branch-shellcheck-result.txt" \ "${PATCH_DIR}/patch-shellcheck-result.txt" \ + shellcheck \ > "${PATCH_DIR}/diff-patch-shellcheck.txt" # shellcheck disable=SC2016 diffPostpatch=$(wc -l "${PATCH_DIR}/diff-patch-shellcheck.txt" | ${AWK} '{print $1}') http://git-wip-us.apache.org/repos/asf/yetus/blob/8e55427b/precommit/test-patch.sh ---------------------------------------------------------------------- diff --git a/precommit/test-patch.sh b/precommit/test-patch.sh index 69f4a12..4d9e4c0 100755 --- a/precommit/test-patch.sh +++ b/precommit/test-patch.sh @@ -2089,24 +2089,71 @@ function runtests } ## @description Calculate the differences between the specified files -## @description and output it to stdout. +## @description using just the column+ messages (third+ column in a +## @descriptoin colon delimated flie) and output it to stdout. ## @audience public ## @stability evolving ## @replaceable no -function calcdiffs +## @param branchlog +## @param patchlog +## @return differences +function column_calcdiffs { - local orig=$1 - local new=$2 - local tmp=${PATCH_DIR}/pl.$$.${RANDOM} - local count=0 - local j + declare branch=$1 + declare patch=$2 + declare tmp=${PATCH_DIR}/pl.$$.${RANDOM} + declare j + + # first, strip filenames:line: + # this keeps column: in an attempt to increase + # accuracy in case of multiple, repeated errors + # since the column number shouldn't change + # if the line of code hasn't been touched + # shellcheck disable=SC2016 + cut -f3- -d: "${branch}" > "${tmp}.branch" + # shellcheck disable=SC2016 + cut -f3- -d: "${patch}" > "${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} "${patch}" | tail -1 + done + + rm "${tmp}.branch" "${tmp}.patch" "${tmp}.lined" 2>/dev/null +} + +## @description Calculate the differences between the specified files +## @description using just the error messages (last column in a +## @descriptoin colon delimated flie) and output it to stdout. +## @audience public +## @stability evolving +## @replaceable no +## @param branchlog +## @param patchlog +## @return differences +function error_calcdiffs +{ + declare branch=$1 + declare patch=$2 + declare tmp=${PATCH_DIR}/pl.$$.${RANDOM} + declare j # first, pull out just the errors # shellcheck disable=SC2016 - ${AWK} -F: '{print $NF}' "${orig}" > "${tmp}.branch" + ${AWK} -F: '{print $NF}' "${branch}" > "${tmp}.branch" # shellcheck disable=SC2016 - ${AWK} -F: '{print $NF}' "${new}" > "${tmp}.patch" + ${AWK} -F: '{print $NF}' "${patch}" > "${tmp}.patch" # compare the errors, generating a string of line # numbers. Sorry portability: GNU diff makes this too easy @@ -2120,12 +2167,37 @@ function calcdiffs # shellcheck disable=SC2013 for j in $(cat "${tmp}.lined"); do # shellcheck disable=SC2086 - head -${j} "${new}" | tail -1 + head -${j} "${patch}" | tail -1 done rm "${tmp}.branch" "${tmp}.patch" "${tmp}.lined" 2>/dev/null } +## @description Wrapper to call specific version of calcdiffs if available +## @description otherwise calls generic_calcdiffs +## @audience public +## @stability evolving +## @replaceable no +## @param branchlog +## @param patchlog +## @param testtype +## @return differences +function calcdiffs +{ + declare branch=$1 + declare patch=$2 + declare testtype=$3 + + if declare -f ${PROJECT_NAME}_${testtype}_calcdiffs >/dev/null; then + "${PROJECT_NAME}_${testtype}_calcdiffs" "${branch}" "${patch}" + elif declare -f ${BUILDTOOL}_${testtype}_calcdiffs >/dev/null; then + "${BUILDTOOL}_${testtype}_calcdiffs" "${branch}" "${patch}" + elif declare -f ${testtype}_calcdiffs >/dev/null; then + "${testtype}_calcdiffs" "${branch}" "${patch}" + else + error_calcdiffs "${branch}" "${patch}" + fi +} ## @description Helper routine for plugins to ask projects, etc ## @description to count problems in a log file @@ -2266,6 +2338,7 @@ function generic_postlog_compare calcdiffs \ "${PATCH_DIR}/branch-${origlog}-${testtype}-${fn}.txt" \ "${PATCH_DIR}/patch-${origlog}-${testtype}-${fn}.txt" \ + "${testtype}" \ > "${PATCH_DIR}/diff-${origlog}-${testtype}-${fn}.txt" diffpatch=$(wc -l "${PATCH_DIR}/diff-${origlog}-${testtype}-${fn}.txt" | ${AWK} '{print $1}')
