Merge pull request #363 from GoogleCloudPlatform/dev-klausw-pre-push-…

…hook

Refactor hooks, add opt-in pre-push hook.

hooks/README.md

@@ -10,6 +10,44 @@ Install them by running:
 
 from the root of the repository. This will symlink the hooks into `.git/hooks/`.
 
+The pre-push hook is currently opt-in. To enable it, set the `PKB_BASE`
+environment variable to the name of a base branch, for example
+`PKB_BASE=upstream/dev` for a fork, or `PKB_BASE=origin/dev` when
+working on the main repository.
+
+You can also run checks manually:
+
+```bash
+# Run all checks on all files under version control.
+hooks/check-everything
+
+# Run all checks on a specific set of files.
+hooks/check FILE ...
+```
+
 [1]: http://git-scm.com/docs/githooks
 [2]: http://github.com/GoogleCloudPlatform/kubernetes
 [3]: https://pypi.python.org/pypi/flake8
+
+
+# Implementation notes
+
+The hook implementation is split into three layers:
+
+- Toplevel hook scripts determine which files need to be checked, run the
+  `hooks/check` script on them as needed, and report back errors. They use a
+  nonzero return code when actions should be blocked (`commit-msg` or
+  `pre-push`), and do other actions where needed. (`prepare-commit-msg` modifies
+  the message.)
+
+- The `hooks/check` script expects a list of files to check as arguments, and
+  runs individual `lib/check-*` scripts on them. It prints optional
+  human-readable diagnostics on STDERR, a formatted report on STDOUT (including
+  offending filenames where available), and returns a nonzero exit code if
+  any checks failed.
+
+- `lib/check-*` scripts take a list of files to check as arguments. They can
+  report file-specific issues by printing a list of bad files on STDOUT along
+  with a zero exit code, or exit with a nonzero exit code if they find issues
+  that can't be associated with a specific file such as a failed unit test or
+  invalid testing setup.

hooks/check

@@ -0,0 +1,82 @@
+#!/bin/bash
+
+# Copyright 2015 Google Inc. All rights reserved.
+#
+# Licensed 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.
+
+set -euo pipefail
+
+HOOKS_DIR="$(dirname "$(test -L "$0" && echo "$(dirname $0)/$(readlink "$0")" || echo "$0")")"
+
+# The prepare-commit-msg hook sets an environment variable to indicate
+# that we should add details to the report. Use an empty string if
+# unset to avoid "unbound variable" failures.
+is_prepare_commit="${PREPARE_COMMIT_HOOK:-}"
+
+# Execute all the checks first, they'll print informational messages on stderr.
+status=0
+
+files_need_boilerplate=($("${HOOKS_DIR}/lib/check-boilerplate.sh" "$@"))
+
+files_with_lint_errors=($("${HOOKS_DIR}/lib/check-lint.sh" "$@"))
+
+# Do unit tests last since these don't have a separate reporting step.
+# This will show unit test diagnostics on STDERR followed by the report.
+if ! "${HOOKS_DIR}/lib/check-unittest.sh"; then
+  echo "*** ERROR: *** Unit test failure."
+  if [ -n "$is_prepare_commit" ]; then
+    echo "Your commit will be aborted unless you fix these."
+    echo "  COMMIT_BLOCKED_ON_UNIT_TESTS"
+  fi
+  status=1
+fi
+
+# Now print reports for previously collected errors to ensure that
+# they are all visible together. This way, they won't be scrolled
+# offscreen by diagnostics.
+
+if [[ "${#files_need_boilerplate[@]}" -ne 0 ]]; then
+  echo
+  echo "*** ERROR: *** Some files are missing the required boilerplate"
+  echo "header from hooks/lib/boilerplate.*.txt:"
+  for file in "${files_need_boilerplate[@]}"; do
+    echo "  ${file}"
+  done
+  echo "See hooks/boilerplate.*.txt for required headers."
+  if [ -n "$is_prepare_commit" ]; then
+    echo "Your commit will be aborted unless you fix these."
+    echo "  COMMIT_BLOCKED_ON_BOILERPLATE"
+  fi
+  status=1
+fi
+
+if [[ "${#files_with_lint_errors[@]}" -ne 0 ]]; then
+  echo
+  echo "*** ERROR: *** Some files have lint errors:"
+  for file in "${files_with_lint_errors[@]}"; do
+    echo "  ${file}"
+  done
+  if [ -n "$is_prepare_commit" ]; then
+    echo "Your commit will be aborted unless you fix these."
+    echo "  COMMIT_BLOCKED_ON_LINT"
+  fi
+  status=1
+fi
+
+if [ -n "$is_prepare_commit" ]; then
+  # For prepare-commit, only return a failure status if there were
+  # unexpected problems. Failed checks should just produce a report.
+  exit 0
+fi
+
+exit $status

hooks/lint.py.sh → hooks/check-everything

@@ -1,6 +1,6 @@
 #!/bin/bash
 
-# Copyright 2014 Google Inc. All rights reserved.
+# Copyright 2015 Google Inc. All rights reserved.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -14,15 +14,11 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-set -o errexit
-set -o nounset
-set -o pipefail
+set -euo pipefail
 
-readonly FILE=$1
+HOOKS_DIR="$(dirname "$(test -L "$0" && echo "$(dirname $0)/$(readlink "$0")" || echo "$0")")"
 
-if [[ -z "$(command -v flake8)" ]]; then
-  >&2 echo "Missing flake8. Install it via 'pip' to enable linting."
-  exit 1
+all_files=($(git ls-files))
+if [[ "${#all_files[0]}" -ne 0 ]]; then
+  "${HOOKS_DIR}/check" "${all_files[@]}"
 fi
-
-flake8 $1 2>&1

hooks/commit-msg

@@ -14,10 +14,10 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-if [[ "$(grep -c "COMMIT_BLOCKED" $1)" -gt 0 ]]; then
+if [[ "$(grep -c "COMMIT_BLOCKED" "$1")" -gt 0 ]]; then
   echo "FAILED: Unresolved errors - aborting the commit."
   echo "The message of your attempted commit was:"
-  cat $1
+  cat "$1"
   exit 1
 fi
 

hooks/install.sh

@@ -21,7 +21,7 @@ set -o nounset
 set -o pipefail
 
 readonly HOOKS_DIR="$(dirname "$(test -L "$0" && echo "$(dirname $0)/$(readlink "$0")" || echo "$0")")"
-readonly HOOK_FILES=(prepare-commit-msg commit-msg)
+readonly HOOK_FILES=(prepare-commit-msg commit-msg pre-push)
 
 pushd $HOOKS_DIR/.. > /dev/null
 

hooks/lib/check-boilerplate.sh

@@ -0,0 +1,68 @@
+#!/bin/bash
+
+# Copyright 2015 Google Inc. All rights reserved.
+#
+# Licensed 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.
+
+set -euo pipefail
+
+HOOKS_LIB="$(dirname "$(test -L "$0" && echo "$(dirname $0)/$(readlink "$0")" || echo "$0")")"
+
+# Check for files without the required boilerplate.
+for file in "$@"; do
+  ext=${file##*.}
+  ref_file="${HOOKS_LIB}/boilerplate.${ext}.txt"
+
+  if [ ! -e $ref_file ]; then
+    continue
+  fi
+
+  lines=$(cat "${ref_file}" | wc -l | tr -d ' ')
+  if [[ "${ext}" == "py" && -x "${file}" ]]; then
+    # remove shabang and blank line from top of executable python files.
+    header=$(cat "${file}" | tail --lines=+3 | head "-${lines}")
+  else
+    header=$(head "-${lines}" "${file}")
+  fi
+
+  # Treat errors from diff other than the usual "files differ"
+  # code 1 as fatal problems.
+  differ=$(echo "${header}" | diff - "${ref_file}" || [[ $? -le 1 ]])
+
+  if [[ -z "${differ}" ]]; then
+    # Header does not differ.
+    continue
+  fi
+
+  if [ "$(echo "${differ}" | wc -l)" -eq 4 ]; then
+    # Header differs by one line. Check if only copyright date differs. If so,
+    # the contents of "${differ}" should resemble:
+    #     1c1
+    #     < # Copyright 2015 Google Inc. All rights reserved.
+    #     ---
+    #     > # Copyright 2014 Google Inc. All rights reserved.
+    header_line='# Copyright [0-9]+ Google Inc[.] All rights reserved[.]'
+    diff_prefix='[[:alnum:]]+[[:space:]]*< '
+    diff_middle='[[:space:]]*---[[:space:]]*> '
+    diff_pattern="^${diff_prefix}${header_line}${diff_middle}${header_line}\$"
+    if [[ "${differ}" =~ $diff_pattern ]]; then
+      # Change matches acceptable variation.
+      continue
+    fi
+  fi
+
+  # Report file as invalid boilerplate.
+  echo "$file"
+done
+
+exit 0

hooks/lib/check-lint.sh

@@ -0,0 +1,36 @@
+#!/bin/bash
+
+# Copyright 2015 Google Inc. All rights reserved.
+#
+# Licensed 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.
+
+set -euo pipefail
+
+if [[ -z "$(command -v flake8)" ]]; then
+  >&2 echo "Missing flake8. Install it via 'pip' to enable linting."
+  exit 1
+fi
+
+# Treat errors from grep other than the "not found" code 1 as fatal problems.
+modified_py=($(printf -- '%s\n' "$@" | grep '\.py$' || [[ $? -le 1 ]]))
+
+# Don't run linter with no arguments, that would check all files.
+if [[ "${#modified_py[@]}" -ne 0 ]]; then
+  # First, run flake with normal output so that the user sees errors.
+  # If there are problems, re-run flake8, printing filenames only.
+  if ! flake8 "${modified_py[@]}" >&2; then
+    flake8 -q "${modified_py[@]}" || true
+  fi
+fi
+
+exit 0

hooks/lint.sh → hooks/lib/check-unittest.sh

@@ -1,6 +1,6 @@
 #!/bin/bash
 
-# Copyright 2014 Google Inc. All rights reserved.
+# Copyright 2015 Google Inc. All rights reserved.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -14,23 +14,14 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-# Print 1 if the file in $1 passes linting, 0 otherwise
-FILE=$1
-EXT=${FILE##*.}
+set -euo pipefail
 
-LINTER="$(dirname $0)/lint.${EXT}.sh"
-
-if [ ! -e "$LINTER" ]; then
-  echo "1"
-  exit 0
-fi
-
-LINT=$($LINTER $FILE)
-if [[ ! -z "$LINT" ]]; then
-  >&2 echo "$LINT"
-  echo "0"
-  exit 0
+if [[ -z $(command -v tox) ]]; then
+  >&2 echo "Missing tox. Install it via 'pip' to enable unit testing."
+  exit 1
 fi
 
-echo "1"
-exit 0
+# Unit test failures can't easily be mapped to specific input files.
+# Always run all tests, and report a failure via nonzero exit code if
+# any test fails.
+tox -e py27 >&2