diff --git a/hooks/README.md b/hooks/README.md index fa17587b5e..7d89619944 100644 --- a/hooks/README.md +++ b/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. diff --git a/hooks/boilerplate.sh b/hooks/boilerplate.sh deleted file mode 100755 index 236f84c707..0000000000 --- a/hooks/boilerplate.sh +++ /dev/null @@ -1,65 +0,0 @@ -#!/bin/bash - -# Copyright 2014 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. - -# Print 1 if the file in $1 has the correct boilerplate header, 0 otherwise. -# -# See hooks/boilerplate.py.txt or hooks/boilerplate.sh.txt for examples for -# python files and bash scripts, respectively. -FILE=$1 -EXT=${FILE##*.} - -REF_FILE="$(dirname $0)/boilerplate.${EXT}.txt" - -if [ ! -e $REF_FILE ]; then - echo "1" - exit 0 -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 -DIFFER=$(echo "${HEADER}" | diff - "${REF_FILE}") - -if [[ -z "${DIFFER}" ]]; then - # Header does not differ. - echo "1" - exit 0 -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. - LINE_PREFIX='# Copyright ' - LINE_SUFFIX=' Google Inc[.] All rights reserved[.]' - HEADER_LINE=${LINE_PREFIX}'[0-9]+'${LINE_SUFFIX} - DIFF_PREFIX='[[:alnum:]]+[[:space:]]*< ' - DIFF_MIDDLE='[[:space:]]*---[[:space:]]*> ' - DIFF_PATTERN='^'${DIFF_PREFIX}${HEADER_LINE}${DIFF_MIDDLE}${HEADER_LINE}'$' - if [[ "${DIFFER}" =~ $DIFF_PATTERN ]]; then - echo "1" - exit 0 - fi -fi - -echo "0" diff --git a/hooks/check b/hooks/check new file mode 100755 index 0000000000..c9020be0a2 --- /dev/null +++ b/hooks/check @@ -0,0 +1,74 @@ +#!/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. Set this to an empty +# string if unset to avoid "unbound variable" failures. +PREPARE_COMMIT_HOOK="${PREPARE_COMMIT_HOOK:-}" + +# Execute all the checks first, they'll print informational messages on stderr. +status=0 + +if ! "${HOOKS_DIR}/lib/check-unittest.sh"; then + echo "*** ERROR: *** Unit test failure." + if [ -n "$PREPARE_COMMIT_HOOK" ]; then + echo "Your commit will be aborted unless you fix these." + echo " COMMIT_BLOCKED_ON_UNIT_TESTS" + fi + status=1 +fi + +files_need_boilerplate=($("${HOOKS_DIR}/lib/check-boilerplate.sh" "$@")) + +files_with_lint_errors=($("${HOOKS_DIR}/lib/check-lint.sh" "$@")) + +# Now print reports for all 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/boilerplate.txt:" + for file in "${files_need_boilerplate[@]}"; do + echo " ${file}" + done + echo "See hooks/boilerplate.*.txt for required headers." + if [ -n "$PREPARE_COMMIT_HOOK" ]; 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 "$PREPARE_COMMIT_HOOK" ]; then + echo "Your commit will be aborted unless you fix these." + echo " COMMIT_BLOCKED_ON_LINT" + fi + status=1 +fi + +exit $status diff --git a/hooks/lint.py.sh b/hooks/check-everything similarity index 67% rename from hooks/lint.py.sh rename to hooks/check-everything index cb3ff1c507..5b31a0e728 100755 --- a/hooks/lint.py.sh +++ b/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 diff --git a/hooks/commit-msg b/hooks/commit-msg index 7eaf1baae1..1d4e5a5e29 100755 --- a/hooks/commit-msg +++ b/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 diff --git a/hooks/install.sh b/hooks/install.sh index 49ffd19446..b83a78e8eb 100755 --- a/hooks/install.sh +++ b/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 diff --git a/hooks/boilerplate.py.txt b/hooks/lib/boilerplate.py.txt similarity index 100% rename from hooks/boilerplate.py.txt rename to hooks/lib/boilerplate.py.txt diff --git a/hooks/boilerplate.sh.txt b/hooks/lib/boilerplate.sh.txt similarity index 100% rename from hooks/boilerplate.sh.txt rename to hooks/lib/boilerplate.sh.txt diff --git a/hooks/lib/check-boilerplate.sh b/hooks/lib/check-boilerplate.sh new file mode 100755 index 0000000000..d7785145dc --- /dev/null +++ b/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 diff --git a/hooks/lib/check-lint.sh b/hooks/lib/check-lint.sh new file mode 100755 index 0000000000..f6d84a6aad --- /dev/null +++ b/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 diff --git a/hooks/lint.sh b/hooks/lib/check-unittest.sh similarity index 61% rename from hooks/lint.sh rename to hooks/lib/check-unittest.sh index e7ef06291d..b0caf2accb 100755 --- a/hooks/lint.sh +++ b/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 diff --git a/hooks/pre-push b/hooks/pre-push new file mode 100755 index 0000000000..457fa3f277 --- /dev/null +++ b/hooks/pre-push @@ -0,0 +1,58 @@ +#!/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. + +# Pre-push hooks are tricky since there's no easy way to distinguish +# which parts of the history are new and which are inherited from +# parents without knowing the branch base point. For now, make this +# check opt-in. + +set -euo pipefail + +HOOKS_DIR="$(dirname "$(test -L "$0" && echo "$(dirname $0)/$(readlink "$0")" || echo "$0")")" +REMOTE="$1" +URL="$2" +Z40=0000000000000000000000000000000000000000 + +if [[ -z "${PKB_BASE:-}" ]]; then + echo >&2 "Skipping pre-push check. Set PKB_BASE to a base branch such as 'upstream/dev' to enable." + exit 0 +fi + +if ! git rev-parse --verify --quiet "$PKB_BASE" >/dev/null; then + echo >&2 "Skipping pre-push check: cannot find PKB_BASE='$PKB_BASE'." + exit 0 +fi + +# Loop over the provided refs being pushed and collect the +# union set of changed files. +modified_files=($( + IFS=' ' + while read local_ref local_sha remote_ref remote_sha; do + # Nothing to do for delete. + [ "$local_sha" = "$Z40" ] && continue + + # Find a common ancestor with the origin branch. + base=$(git merge-base "$PKB_BASE" "$local_sha") + + # Get list of files added/changed/moved relative to the + # common ancestor. + git diff --name-only --diff-filter ACM "$base..$local_sha" + done | sort -u +)) + +if [[ "${#modified_files[@]}" -ne 0 ]]; then + "${HOOKS_DIR}/check" "${modified_files[@]}" >&2 +fi diff --git a/hooks/prepare-commit-msg b/hooks/prepare-commit-msg index 958bbcdfef..cb25489fa1 100755 --- a/hooks/prepare-commit-msg +++ b/hooks/prepare-commit-msg @@ -14,78 +14,21 @@ # 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")")" +OUT_FILE="${1:-/dev/stdout}" -# Lint python files -files_with_lint_errors=() -files_need_boilerplate=() -for file in $(git diff --cached --name-only --diff-filter ACM | grep "\.py"); do - # Check for files without the required boilerplate. - boilerplate=$("${HOOKS_DIR}/boilerplate.sh" "${file}") - if [[ "$boilerplate" -eq "0" ]]; then - files_need_boilerplate+=("${file}") - fi - - # Check for lint errors - lint_err=$("${HOOKS_DIR}/lint.sh" "${file}") - if [[ "$lint_err" -eq "0" ]]; then - files_with_lint_errors+=("${file}") - fi -done - -# Check sh files for boilerplate -for file in $(git diff --cached --name-only --diff-filter ACM | grep "\.sh"); do - # Check for files without the required boilerplate. - boilerplate=$("${HOOKS_DIR}/boilerplate.sh" "${file}") - if [[ "$boilerplate" -eq "0" ]]; then - files_need_boilerplate+=("${file}") - fi -done +# Set of modified files is those currently staged for commit (cached). +modified_files=($(git diff --cached --name-only --diff-filter ACM)) -if [[ "${#files_need_boilerplate[@]}" -ne 0 ]]; then - ( - echo - echo "# *** ERROR: *** Some files are missing the required boilerplate" - echo "# header from hooks/boilerplate.txt:" - for file in "${files_need_boilerplate[@]}"; do - echo "# ${file}" - done - echo "# See hooks/boilerplate.\${EXT}.txt for required header." - echo "#" - echo "# Your commit will be aborted unless you fix these." - echo "# COMMIT_BLOCKED_ON_BOILERPLATE" - echo - ) >> $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 - echo "# Your commit will be aborted unless you fix these." - echo "# COMMIT_BLOCKED_ON_LINT" - echo - ) >> $1 -fi +# The checker script has a nonzero exit code on errors and reports +# problems on STDOUT, with informational messages on STDERR. Append +# errors to the commit message with leading comment markers added, and +# return success so that we don't block the message. -if [[ -z $(command -v tox) ]]; then - >&2 echo "Missing tox. Install it via 'pip' to enable unit testing." - exit 1 -else - UNITTEST_RESULT=$(tox -e py27 || echo "FAILED") - if [[ "$UNITTEST_RESULT" == *"FAILED"* ]]; then - ( - echo - echo "# *** ERROR: *** Unit test failure." - echo "# Your commit will be aborted unless you fix these." - echo "# COMMIT_BLOCKED_ON_UNIT_TESTS" - echo - ) >> $1 - fi +if [[ "${#modified_files[@]}" -ne 0 ]]; then + PREPARE_COMMIT_HOOK="true" \ + "${HOOKS_DIR}/check" "${modified_files[@]}" | \ + sed 's/^/# /' >>"$OUT_FILE" || true fi