Adding autopep8 as a pre-commit hook. (apache#7071)

Co-authored-by: bneradt <bneradt@verizonmedia.com>

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.
 
 

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.

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
 

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

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)"
 

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