Skip to content

Commit

Permalink
Support local pre-commit checks.
Browse files Browse the repository at this point in the history
This change adds a setup script that will install or re-install a
pre-commit hook to run pants lints.  The pre-commit script is also
runnable directly to support a centralized stable script developers can
run to execute all lints independent of performing a commit.

Testing Done:
Local testing:
```console
# Install
$ rm -f .git/hooks/pre-commit
$ ./build-support/bin/setup.sh
Pre-commit checks installed from /home/jsirois/dev/3rdparty/jsirois-pants3/build-support/bin/pre-commit.sh to /home/jsirois/dev/3rdparty/jsirois-pants3/.git/hooks/pre-commit
$ ./build-support/bin/setup.sh
Pre-commit checks up to date.

# Unanticipated drift
$ rm -f .git/hooks/pre-commit
$ touch /home/jsirois/dev/3rdparty/jsirois-pants3/.git/hooks/pre-commit
$ ./build-support/bin/setup.sh
A pre-commit script already exists, replace with /home/jsirois/dev/3rdparty/jsirois-pants3/build-support/bin/pre-commit.sh? [Yn]n
Pre-commit checks not installed
$ ./build-support/bin/setup.sh
A pre-commit script already exists, replace with /home/jsirois/dev/3rdparty/jsirois-pants3/build-support/bin/pre-commit.sh? [Yn]
Pre-commit checks installed from /home/jsirois/dev/3rdparty/jsirois-pants3/build-support/bin/pre-commit.sh to /home/jsirois/dev/3rdparty/jsirois-pants3/.git/hooks/pre-commit
$ ./build-support/bin/setup.sh
Pre-commit checks up to date.

# Dogfood on this commit
$ git commit
Checking packages
Checking imports
Checking headers
Success
[jsirois/lints/commit_hooks 8458f3e] Support local pre-commit checks.
 9 files changed, 71 insertions(+), 14 deletions(-)
 create mode 100755 build-support/bin/pre-commit.sh
 create mode 100755 build-support/bin/setup.sh
```

CI went green here:
  https://travis-ci.org/pantsbuild/pants/builds/53506978

Bugs closed: 1222

Reviewed at https://rbcommons.com/s/twitter/r/1883/
  • Loading branch information
jsirois committed Mar 9, 2015
1 parent efb1112 commit 360f964
Show file tree
Hide file tree
Showing 9 changed files with 84 additions and 14 deletions.
2 changes: 1 addition & 1 deletion build-support/bin/check_header.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
set -euo pipefail
IFS=$'\n\t'

REPO_ROOT=$(cd $(dirname "${BASH_SOURCE[0]}") && cd "$(git rev-parse --show-toplevel)" && pwd)
REPO_ROOT="$(git rev-parse --show-toplevel)"
cd ${REPO_ROOT}

expected_header=$(cat <<EOF
Expand Down
2 changes: 1 addition & 1 deletion build-support/bin/check_packages.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
set -euo pipefail
IFS=$'\n\t'

REPO_ROOT=$(cd $(dirname "${BASH_SOURCE[0]}") && cd "$(git rev-parse --show-toplevel)" && pwd)
REPO_ROOT="$(git rev-parse --show-toplevel)"
cd ${REPO_ROOT}

DIRS_TO_CHECK=(
Expand Down
11 changes: 4 additions & 7 deletions build-support/bin/ci.sh
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ bootstrap_compile_args=(
while getopts "hfxbkmsrlpnci:eat" opt; do
case ${opt} in
h) usage ;;
f) skip_formatting_checks="true" ;;
f) skip_pre_commit_checks="true" ;;
x) skip_bootstrap_clean="true" ;;
b) skip_bootstrap="true" ;;
k) bootstrap_compile_args=() ;;
Expand Down Expand Up @@ -86,13 +86,10 @@ else
banner "CI BEGINS"
fi

if [[ "${skip_formatting_checks:-false}" == "false" ]]; then
banner "Checking python code formatting"
if [[ "${skip_pre_commit_checks:-false}" == "false" ]]; then
banner "Running pre-commit checks"

./build-support/bin/check_packages.sh || exit 1
./build-support/bin/isort.sh || \
die "To fix import sort order, run \`build-support/bin/isort.sh -f\`"
./build-support/bin/check_header.sh || exit 1
./build-support/bin/pre-commit.sh || exit 1
fi

# TODO(John sirois): Re-plumb build such that it grabs constraints from the built python_binary
Expand Down
2 changes: 1 addition & 1 deletion build-support/bin/isort.sh
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#!/usr/bin/env bash

REPO_ROOT=$(cd $(dirname "${BASH_SOURCE[0]}") && cd "$(git rev-parse --show-toplevel)" && pwd)
REPO_ROOT="$(git rev-parse --show-toplevel)"
cd ${REPO_ROOT}

source build-support/common.sh
Expand Down
11 changes: 11 additions & 0 deletions build-support/bin/pre-commit.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#!/usr/bin/env bash

# NB: pre-commit runs in the context of GIT_WORK_TREE, ie: pwd == REPO_ROOT

source build-support/common.sh

echo "Checking packages" && ./build-support/bin/check_packages.sh || exit 1
echo "Checking imports" && ./build-support/bin/isort.sh || \
die "To fix import sort order, run \`build-support/bin/isort.sh -f\`"
echo "Checking headers" && ./build-support/bin/check_header.sh || exit 1
echo "Success"
50 changes: 50 additions & 0 deletions build-support/bin/setup.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
#!/usr/bin/env bash

# Set up the development environment.
# Currently this just installs local git hooks.

REPO_ROOT="$(git rev-parse --show-toplevel)"

HOOK_DIR="${GIT_DIR:-${REPO_ROOT}/.git}/hooks"
PRE_COMMIT_DEST="${HOOK_DIR}/pre-commit"
PRE_COMMIT_SRC="${REPO_ROOT}/build-support/bin/pre-commit.sh"
PRE_COMMIT_RELSRC="$(cat << EOF | python2.7
import os
print(os.path.relpath("${PRE_COMMIT_SRC}", "${HOOK_DIR}"))
EOF
)"

source "${REPO_ROOT}/build-support/common.sh"

function install_pre_commit_hook() {
(
cd "${HOOK_DIR}" && \
rm -f pre-commit && \
ln -s "${PRE_COMMIT_RELSRC}" pre-commit && \
echo "Pre-commit checks linked from ${PRE_COMMIT_SRC} to $(pwd)/pre-commit";
)
}

if [[ ! -e "${PRE_COMMIT_DEST}" ]]
then
install_pre_commit_hook
else
existing_hook_sig="$(cat "${PRE_COMMIT_DEST}" | fingerprint_data)"
canonical_hook_sig="$(cat "${PRE_COMMIT_SRC}" | fingerprint_data)"
if [[ "${existing_hook_sig}" != "${canonical_hook_sig}" ]]
then
read -p "A pre-commit script already exists, replace with ${PRE_COMMIT_SRC}? [Yn]" ok
if [[ "${ok:-Y}" =~ ^[yY]([eE][sS])?$ ]]
then
install_pre_commit_hook
else
echo "Pre-commit checks not installed"
exit 1
fi
else
echo "Pre-commit checks up to date."
fi
fi
exit 0

4 changes: 4 additions & 0 deletions build-support/common.sh
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,7 @@ function banner() {
echo "[== $(elapsed) $@ ==]"
echo
}

function fingerprint_data() {
openssl md5 | cut -d' ' -f2
}
4 changes: 0 additions & 4 deletions build-support/pants_venv
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,6 @@ function venv_dir() {
fi
}

function fingerprint_data() {
openssl md5 | cut -d' ' -f2
}

function activate_venv() {
source "$(venv_dir)/bin/activate"
}
Expand Down
12 changes: 12 additions & 0 deletions src/python/pants/docs/howto_contribute.md
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,18 @@ branch using the [syncing a
fork](https://help.github.com/articles/syncing-a-fork/) instructions on
github.

Whether you've cloned the repo or your fork of the repo, you should setup the
local pre-commit hooks to ensure your commits meet minimum compliance checks
before pushing branches to ci:

:::bash
$ ./build-support/bin/setup.sh

You can always run the pre-commit checks manually via:

:::bash
$ ./build-support/bin/pre-commit.sh

**Pro tip:** If you want your local master branch to be an exact copy of
the `pantsbuild/pants` repo's master branch, use these commands:

Expand Down

0 comments on commit 360f964

Please sign in to comment.