From a38facb323373b33361570f12c2507a47e191545 Mon Sep 17 00:00:00 2001 From: Alex Clemmer Date: Sat, 30 Dec 2017 22:16:04 -0700 Subject: [PATCH] Automatically add DCO signoff with commit hooks (#2283) Fixes #2281. DCO signoff is mandatory for all contributors. Currently this is enforced via CI. This commit smoothes this process somewhat by introducing: A git commit hook, which will check the commit message for a signoff, and if not present, add one to the bottom of the message. A script, support/bootstrap, which will symlink these commit hooks into the Envoy root .git directory, to minimize the effort it takes to use them. Signed-off-by: Alex Clemmer --- CONTRIBUTING.md | 11 +++++ DEVELOPER.md | 2 + README.md | 1 + support/README.md | 37 +++++++++++++++++ support/bootstrap | 58 +++++++++++++++++++++++++++ support/hooks/pre-push | 69 ++++++++++++++++++++++++++++++++ support/hooks/prepare-commit-msg | 23 +++++++++++ tools/pre-commit.sh | 11 ----- 8 files changed, 201 insertions(+), 11 deletions(-) create mode 100644 support/README.md create mode 100755 support/bootstrap create mode 100755 support/hooks/pre-push create mode 100755 support/hooks/prepare-commit-msg delete mode 100755 tools/pre-commit.sh diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b7e5568fa159..0a296815667e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -112,6 +112,17 @@ maximize the chances of your PR being merged. # DCO: Sign your work +Envoy ships commit hooks that allow you to auto-generate the DCO signoff line if +it doesn't exist when you run `git commit`. Simply navigate to the Envoy project +root and run: + +```bash +./support/bootstrap +``` + +From here, simply commit as normal, and you will see the signoff at the bottom +of each commit. + The sign-off is a simple line at the end of the explanation for the patch, which certifies that you wrote it or otherwise have the right to pass it on as an open-source patch. The rules are pretty simple: if you diff --git a/DEVELOPER.md b/DEVELOPER.md index f4f737e8fc78..6044f98cbb75 100644 --- a/DEVELOPER.md +++ b/DEVELOPER.md @@ -4,6 +4,8 @@ Envoy is built using the Bazel build system. Travis CI builds, tests, and runs c To get started building Envoy locally, see the [Bazel quick start](https://github.com/envoyproxy/envoy/blob/master/bazel/README.md#quick-start-bazel-build-for-developers). To run tests, there are Bazel [targets](https://github.com/envoyproxy/envoy/blob/master/bazel/README.md#testing-envoy-with-bazel) for Google Test. To generate a coverage report, use the tooling for [gcovr](https://github.com/envoyproxy/envoy/blob/master/bazel/README.md#coverage-builds). +If you plan to contribute to Envoy, you may find it useful to install the Envoy [development support toolchain](https://github.com/envoyproxy/envoy/blob/master/support/README.md), which helps automate parts of the development process, particularly those involving code review. + Below is a list of additional documentation to aid the development process: - [General build and installation documentation](https://www.envoyproxy.io/docs/envoy/latest/install/install) diff --git a/README.md b/README.md index f0c0aefaf1af..9d6a09557b97 100644 --- a/README.md +++ b/README.md @@ -55,6 +55,7 @@ have prior experience. To get started: * [Beginner issues](https://github.com/envoyproxy/envoy/issues?q=is%3Aopen+is%3Aissue+label%3Abeginner) * [Build/test quick start using docker](ci#building-and-running-tests-as-a-developer) * [Developer guide](DEVELOPER.md) +* Consider installing the Envoy [development support toolchain](https://github.com/envoyproxy/envoy/blob/master/support/README.md), which helps automate parts of the development process, particularly those involving code review. * Please make sure that you let us know if you are working on an issue so we don't duplicate work! ## Community Meeting diff --git a/support/README.md b/support/README.md new file mode 100644 index 000000000000..240bf200a221 --- /dev/null +++ b/support/README.md @@ -0,0 +1,37 @@ +# Support tools + +A collection of CLI tools meant to support and automate various aspects of +developing Envoy, particularly those related to code review. For example, +automatic DCO signoff and pre-commit format checking. + +## Usage + +To get started, you need only navigate to the Envoy project root and run: + +```bash +./support/bootstrap +``` + +This will set up the development support toolchain automatically. The toolchain +uses git hooks extensively, copying them from `support/hooks` to the `.git` +folder. + +The commit hook checks can be skipped using the `-n` / `--no-verify` flags, as +so: + +```bash +git commit --no-verify +``` + +## Functionality + +Currently the development support toolchain exposes two main pieces of +functionality: + +* Automatically appending DCO signoff to the end of a commit message if it + doesn't exist yet. Correctly covers edge cases like `commit --amend` and + `rebase`. +* Automatically running DCO and format checks on all files in the diff, before + push. + +[filter]: https://github.com/envoyproxy/envoy-filter-example diff --git a/support/bootstrap b/support/bootstrap new file mode 100755 index 000000000000..0b1c52c00705 --- /dev/null +++ b/support/bootstrap @@ -0,0 +1,58 @@ +#!/usr/bin/env bash +# +# Bootstraps the developer tools and development environment. This includes +# copying project-standard git commit hooks that do things like ensure DCO +# signoff is present during commit. + +# Best-effort check that we're in the envoy root directory. +# +# TODO(hausdorff): If compatibility becomes an issue here, come back and do this +# "the right way". This is hard to do in general, since `realpath` is not +# standard. +if [ ! "$PWD" == "$(git rev-parse --show-toplevel)" ]; then + cat >&2 <<__EOF__ +ERROR: this script must be run at the root of the envoy source tree +__EOF__ + exit 1 +fi + +# Helper functions that calculate `abspath` and `relpath`. Taken from Mesos +# commit 82b040a60561cf94dec3197ea88ae15e57bcaa97, which also carries the Apache +# V2 license, and has deployed this code successfully for some time. +abspath() { + cd "$(dirname "${1}")" + echo "${PWD}"/"$(basename "${1}")" + cd "${OLDPWD}" +} +relpath() { + local FROM TO UP + FROM="$(abspath "${1%/}")" TO="$(abspath "${2%/}"/)" + while test "${TO}" = "${TO#"${FROM}"/}" \ + -a "${TO}" != "${FROM}"; do + FROM="${FROM%/*}" UP="../${UP}" + done + TO="${UP%/}${TO#${FROM}}" + echo "${TO:-.}" +} + +# Try to find the `.git` directory, even if it's not in Envoy project root (as +# it wouldn't be if, say, this were in a submodule). The "blessed" but fairly +# new way to do this is to use `--git-common-dir`. +DOT_GIT_DIR=$(git rev-parse --git-common-dir) +if test ! -d "${DOT_GIT_DIR}"; then + # If `--git-common-dir` is not available, fall back to older way of doing it. + DOT_GIT_DIR=$(git rev-parse --git-dir) +fi + +HOOKS_DIR="${DOT_GIT_DIR}/hooks" +HOOKS_DIR_RELPATH=$(relpath "${HOOKS_DIR}" "${PWD}") + +if [ ! -e "${HOOKS_DIR}/prepare-commit-msg" ]; then + echo "Installing hook 'prepare-commit-msg'" + ln -s "${HOOKS_DIR_RELPATH}/support/hooks/prepare-commit-msg" "${HOOKS_DIR}/prepare-commit-msg" +fi + +if [ ! -e "${HOOKS_DIR}/pre-push" ]; then + echo "Installing hook 'pre-push'" + ln -s "${HOOKS_DIR_RELPATH}/support/hooks/pre-push" "${HOOKS_DIR}/pre-push" +fi diff --git a/support/hooks/pre-push b/support/hooks/pre-push new file mode 100755 index 000000000000..52b480161ff7 --- /dev/null +++ b/support/hooks/pre-push @@ -0,0 +1,69 @@ +#!/usr/bin/env bash +# +# A git commit hook that will automatically run format checking and DCO signoff +# checking before the push is successful. +# +# To enable this hook, run `bootstrap`, or run the following from the root of +# the repo. (Note that `bootstrap` will correctly install this even if this code +# is located in a submodule, while the following won't.) +# +# $ ln -s ../../support/hooks/pre-push .git/hooks/pre-push + +DUMMY_SHA=0000000000000000000000000000000000000000 + +echo "Running pre-push check; to skip this step use 'push --no-verify'" + +while read LOCAL_REF LOCAL_SHA REMOTE_REF REMOTE_SHA +do + if [ "$LOCAL_SHA" = $DUMMY_SHA ] + then + # Branch deleted. Do nothing. + exit 0 + else + if [ "$REMOTE_SHA" = $DUMMY_SHA ] + then + # New branch. Verify last 15 commits, since verification of the entire + # history would take too much time. + RANGE="$LOCAL_SHA~15..$LOCAL_SHA" + else + # Updating branch. Verify new commits. + RANGE="$REMOTE_SHA..$LOCAL_SHA" + fi + + # Verify DCO signoff. We do this before the format checker, since it has + # some probability of failing spuriously, while this check never should. + # + # In general, we can't assume that the commits are signed off by author + # pushing, so we settle for just checking that there is a signoff at all. + SIGNED_OFF=$(git rev-list --grep "^Signed-off-by: " "$RANGE") + NOT_SIGNED_OFF=$(git rev-list "$RANGE" | grep -Fxv "$SIGNED_OFF") + if [ -n "$NOT_SIGNED_OFF" ] + then + echo >&2 "ERROR: The following commits do not have DCO signoff:" + while read -r commit; do + echo " $(git log --pretty=oneline --abbrev-commit -n 1 $commit)" + done <<< "$NOT_SIGNED_OFF" + exit 1 + fi + + # NOTE: The `tools` directory will be the same relative to the support + # directory, independent of whether we're in a submodule, so no need to use + # our `relpath` helper. + SCRIPT_DIR="$(dirname "$(realpath "$0")")/../../tools" + + # TODO(hausdorff): We should have a more graceful failure story when the + # user does not have all the tools set up correctly. This script assumes + # `$CLANG_FORMAT` and `$BUILDIFY` are defined, or that the default values it + # assumes for these variables correspond to real binaries on the system. If + # either of these things aren't true, the check fails. + for i in $(git diff --name-only $RANGE --diff-filter=ACMR 2>&1); do + echo " Checking format for $i" + "$SCRIPT_DIR"/check_format.py check $i + if [[ $? -ne 0 ]]; then + exit 1 + fi + done + fi +done + +exit 0 diff --git a/support/hooks/prepare-commit-msg b/support/hooks/prepare-commit-msg new file mode 100755 index 000000000000..38f2c3d16e33 --- /dev/null +++ b/support/hooks/prepare-commit-msg @@ -0,0 +1,23 @@ +#!/usr/bin/env bash +# +# A git commit hook that will automatically append a DCO signoff to the bottom +# of any commit message that doesn't have one. This append happens after the git +# default message is generated, but before the user is dropped into the commit +# message editor. +# +# To enable this hook, run `bootstrap`, or run the following from the root of +# the repo. (Note that `bootstrap` will correctly install this even if this code +# is located in a submodule, while the following won't.) +# +# $ ln -s ../../support/hooks/prepare-commit-msg .git/hooks/prepare-commit-msg + +COMMIT_MESSAGE_FILE="$1" +AUTHOR=$(git var GIT_AUTHOR_IDENT) +SIGNOFF=$(echo $AUTHOR | sed -n 's/^\(.*>\).*$/Signed-off-by: \1/p') + +# Check for DCO signoff message. If one doesn't exist, append one and then warn +# the user that you did so. +if ! $(grep -qs "^$SIGNOFF" "$COMMIT_MESSAGE_FILE") ; then + echo -e "\n$SIGNOFF" >> "$COMMIT_MESSAGE_FILE" + echo -e "Appended the following signoff to the end of the commit message:\n $SIGNOFF\n" +fi diff --git a/tools/pre-commit.sh b/tools/pre-commit.sh deleted file mode 100755 index 9d2754997ffc..000000000000 --- a/tools/pre-commit.sh +++ /dev/null @@ -1,11 +0,0 @@ -#!/bin/bash - -SCRIPT_DIR="$(dirname "$(realpath "$0")")" - -for i in `git diff-index --name-only --diff-filter=ACM HEAD 2>&1`; do - echo "Checking format for $i" - "$SCRIPT_DIR"/check_format.py check $i - if [[ $? -ne 0 ]]; then - exit 1 - fi -done