From 803ad104ff30655068b158172ea404fdbabd2ccb Mon Sep 17 00:00:00 2001 From: bneradt Date: Thu, 30 Jul 2020 22:27:32 +0000 Subject: [PATCH] Adding autopep8 as a pre-commit hook. --- CONTRIBUTING.md | 7 ++- Makefile.am | 10 +---- ci/jenkins/bin/clang-format.sh | 3 ++ tools/autopep8.sh | 79 ++++++++++++++++++++++++++++++++++ tools/clang-format.sh | 2 +- tools/git/pre-commit | 46 +++++++++++++++++--- 6 files changed, 128 insertions(+), 19 deletions(-) create mode 100755 tools/autopep8.sh diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 21b19b45bb8..21f5960dcc3 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -58,10 +58,13 @@ are a few simple rules to follow: 8. Make sure you run **clang-format** before making the _PR_. This is easiest done with e.g. "make clang-format", which works on macOS and Linux. -9. When making backports, make sure you mark the _PR_ for the appropriate +9. Make sure you run **autopep8** before making the _PR_. This is easiest + done with e.g. "make autopep8". + +10. When making backports, make sure you mark the _PR_ for the appropriate Github branch (e.g. **6.2.x**). -10. If you are making backports to an LTS branch, remember that the job of +11. If you are making backports to an LTS branch, remember that the job of merging such a _PR_ is the duty of the release manager. diff --git a/Makefile.am b/Makefile.am index 8eda4112454..756b97c6387 100644 --- a/Makefile.am +++ b/Makefile.am @@ -112,15 +112,7 @@ rat: java -jar $(top_srcdir)/ci/apache-rat-0.13-SNAPSHOT.jar -E $(top_srcdir)/ci/rat-regex.txt -d $(top_srcdir) autopep8: - @autopep8 \ - --ignore-local-config \ - -i \ - -j 0 \ - --exclude $(top_srcdir)/lib/yamlcpp \ - --max-line-length 132 \ - --aggressive \ - --aggressive \ - -r $(top_srcdir) + @$(top_srcdir)/tools/autopep8.sh $(top_srcdir) # # These are rules to make clang-format easy and fast to run. Run it with e.g. diff --git a/ci/jenkins/bin/clang-format.sh b/ci/jenkins/bin/clang-format.sh index 40170eead90..de9258f8668 100755 --- a/ci/jenkins/bin/clang-format.sh +++ b/ci/jenkins/bin/clang-format.sh @@ -44,6 +44,9 @@ autoreconf -if && ./configure ${ATS_MAKE} clang-format [ "0" != "$?" ] && exit 1 +${ATS_MAKE} autopep8 +[ "0" != "$?" ] && exit 1 + git diff --exit-code [ "0" != "$?" ] && exit 1 diff --git a/tools/autopep8.sh b/tools/autopep8.sh new file mode 100755 index 00000000000..f1a02b9fdff --- /dev/null +++ b/tools/autopep8.sh @@ -0,0 +1,79 @@ +#! /usr/bin/env bash +# +# Simple wrapper to run autopep8 on a directory. +# +# 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. + +# Update the PKGVERSION with the new desired autopep8 tag when a new autopep8 +# version is desired. +# See: +# https://github.com/hhatto/autopep8/tags +AUTOPEP8_VERSION="1.5.3" + +VERSION="autopep8 1.5.3 (pycodestyle: 2.6.0)" + +# Tie this to exactly the pycodestyle version that shows up in the setup.py of +# autopep8 so we know we run with the same version each time. +# See: +# https://github.com/hhatto/autopep8/blob/master/setup.py +PYCODESTYLE_TAG="2.6.0" + +function main() { + set -e # exit on error + + if ! type virtualenv >/dev/null 2>/dev/null + then + pip install -q virtualenv + fi + + AUTOPEP8_VENV=${AUTOPEP8_VENV:-$(cd $(dirname $0) && git rev-parse --show-toplevel)/.git/fmt/autopep8_${AUTOPEP8_VERSION}_venv} + if [ ! -e ${AUTOPEP8_VENV} ] + then + virtualenv ${AUTOPEP8_VENV} + fi + source ${AUTOPEP8_VENV}/bin/activate + + pip install -q "pycodestyle==${PYCODESTYLE_TAG}" + pip install -q "autopep8==${AUTOPEP8_VERSION}" + + ver=$(autopep8 --version) + if [ "$ver" != "$VERSION" ] + then + echo "Wrong version of autopep8!" + echo "Expected: \"${VERSION}\", got: \"${ver}\"" + exit 1 + fi + + DIR=${@:-.} + autopep8 \ + --ignore-local-config \ + -i \ + -j 0 \ + --exclude "${DIR}/lib/yamlcpp" \ + --max-line-length 132 \ + --aggressive \ + --aggressive \ + --verbose \ + -r ${DIR} + deactivate +} + +if [[ "$(basename -- "$0")" == 'autopep8.sh' ]]; then + main "$@" +else + AUTOPEP8_VENV=${AUTOPEP8_VENV:-$(git rev-parse --show-toplevel)/.git/fmt/autopep8_${AUTOPEP8_VERSION}_venv} +fi diff --git a/tools/clang-format.sh b/tools/clang-format.sh index cac18c19007..246edf0722d 100755 --- a/tools/clang-format.sh +++ b/tools/clang-format.sh @@ -25,7 +25,7 @@ function main() { set -e # exit on error ROOT=${ROOT:-$(cd $(dirname $0) && git rev-parse --show-toplevel)/.git/fmt/${PKGDATE}} - DIR=${1:-.} + DIR=${@:-.} PACKAGE="clang-format-${PKGDATE}.tar.bz2" VERSION="clang-format version 10.0.0 (https://github.com/llvm/llvm-project.git d32170dbd5b0d54436537b6b75beaf44324e0c28)" diff --git a/tools/git/pre-commit b/tools/git/pre-commit index 48cfaa108da..e5c997f7b84 100755 --- a/tools/git/pre-commit +++ b/tools/git/pre-commit @@ -42,30 +42,62 @@ if [ ! -x "$FORMAT" ]; then exit 1 fi +source "$GIT_TOP/tools/autopep8.sh" +if [ ! -d ${AUTOPEP8_VENV} ] +then + echo "Run \"make autopep8\"" + exit 1 +fi +source ${AUTOPEP8_VENV}/bin/activate + + # Where to store the patch -patch_file=$(mktemp -t clang-format.XXXXXXXXXX) -trap "rm -f $patch_file" 0 1 2 3 5 15 +clang_patch_file=$(mktemp -t clang-format.XXXXXXXXXX) +autopep8_patch_file=$(mktemp -t autopep8.XXXXXXXXXX) +trap "rm -f $clang_patch_file $autopep8_patch_file" 0 1 2 3 5 15 # Loop over all files that are changed, and produce a diff file git diff-index --cached --diff-filter=ACMR --name-only HEAD | grep -vE "lib/yamlcpp" | while read file; do case "$file" in *.cc | *.c | *.h | *.h.in) - ${FORMAT} "$file" | diff -u "$file" - >> "$patch_file" + ${FORMAT} "$file" | diff -u "$file" - >> "$clang_patch_file" ;; + *.py | *.cli.ext | *.test.ext) + autopep8 \ + --ignore-local-config \ + --exclude ${GIT_TOP}/lib/yamlcpp \ + --max-line-length 132 \ + --aggressive \ + --aggressive \ + --diff \ + "$file" >> "$autopep8_patch_file" esac done -if [ -s "$patch_file" ] ; then +if [ -s "$clang_patch_file" ] ; then echo "The commit is not accepted, because clang-format does not match current" echo "requirements. Easiest to fix this is to run:" echo echo " $ make -j clang-format" exit 1 +else + echo "This commit complies with the current clang-format indentation rules." + echo fi -echo "This commit complies with the current clang-format indentation rules." -echo + +if [ -s "$autopep8_patch_file" ] ; then + echo "The commit is not accepted because autopep8 reports issues with it." + echo "The easiest way to fix this is to run:" + echo + echo " $ make autopep8" + exit 1 +else + echo "This commit complies with the current autopep8 formatting rules." + echo +fi # Cleanup before exit -rm -f "$patch_file" +deactivate +rm -f "$clang_patch_file" "$autopep8_patch_file" exit 0