Skip to content

Commit

Permalink
ARROW-11101: [Rust] rewrite pre-commit hook
Browse files Browse the repository at this point in the history
Currently,  the client side git pre-commit hook checks and runs `cargo fmt` with stable version, but the CI check may fail in nightly version occasionally even if the code has been formatted with stable.

It seems that, this problem can be resolved by: running `cargo +nighty fmt` before  `cargo +stable fmt`. Thus `pre-commit.sh` should be updated in this way.

In this PR:

1. only check staged *.rs files and exit if no changes
2. cargo clippy is moved before cargo fmt
3. run `cargo +nighty fmt` before  cargo +stable fmt
4. show more concise tip messages
5. abort the commit if there are files changed by `cargo fmt`

Closes apache#9072 from mqy/rewrite-pre-commit-hook

Authored-by: mqy <meng.qingyou@gmail.com>
Signed-off-by: Andrew Lamb <andrew@nerdnetworks.org>
  • Loading branch information
mqy authored and GeorgeAp committed Jun 7, 2021
1 parent d4b336a commit ca984ed
Showing 1 changed file with 39 additions and 37 deletions.
76 changes: 39 additions & 37 deletions rust/pre-commit.sh
Expand Up @@ -21,9 +21,9 @@
#
# Soft link it as git hook under top dir of apache arrow git repository:
# $ ln -s ../../rust/pre-commit.sh .git/hooks/pre-commit

# NOTE: colorized output may not work as expected.
# I've seen the difference between /bin/sh and GUN bash 5 on macOS.
#
# This file be run directly:
# $ ./pre-commit.sh

function RED() {
echo "\033[0;31m$@\033[0m"
Expand All @@ -37,50 +37,52 @@ function BYELLOW() {
echo "\033[1;33m$@\033[0m"
}

MSG="git pre-commit hook"
RUST_DIR="rust"
CARGO_FMT="cargo +stable fmt --all"
CARGO_CLIPPY="cargo clippy"

NUM_CHANGES=$(git diff --cached --name-only "${RUST_DIR}" |
grep -e ".*/*.rs$" -o -e "^rustfmt.toml$" -o -e "^rust-toolchain$" |
awk '{print $1}' |
wc -l)

if [ ${NUM_CHANGES} -eq 0 ]; then
echo -e "$(GREEN INFO) ${MSG}: no changes in: *rs, rustfmt.toml, rust-toolchain, skip fmt/clippy"
#exit 0
# env GIT_DIR is set by git when run a pre-commit hook.
if [ -z "${GIT_DIR}" ]; then
GIT_DIR=$(git rev-parse --show-toplevel)
fi

cd ${RUST_DIR}
cd ${GIT_DIR}/${RUST_DIR}

# 1. Abort on cargo fmt error.

echo -e "$(GREEN INFO) ${MSG}: cargo fmt checking ..."
NUM_CHANGES=$(git diff --cached --name-only . |
grep -e ".*/*.rs$" |
awk '{print $1}' |
wc -l)

$CARGO_FMT -q -- --check 2>/dev/null
if [ $? -eq 0 ]; then
echo -e "$(GREEN INFO) ${MSG}: $(GREEN cargo fmt check passed)"
else
echo -e "$(GREEN INFO) ${MSG}: cargo fmt formatting ..."
$CARGO_FMT
echo
echo -e "$(BYELLOW WARN) ${MSG}: cargo fmt $(BYELLOW fixed some files)"
if [ ${NUM_CHANGES} -eq 0 ]; then
echo -e "$(GREEN INFO): no staged changes in *.rs, $(GREEN skip cargo fmt/clippy)"
exit 0
fi

# 2. Warn on cargo clippy errors/warnings.
# 1. cargo clippy

echo
echo "===================================================="
echo
echo -e "$(GREEN INFO) ${MSG}: cargo clippy ..."
echo
echo -e "$(GREEN INFO): cargo clippy ..."

# Cargo clippy always return exit code 0, and tee doesnt work.
# Cargo clippy always return exit code 0, and `tee` doesn't work.
# So let's just run cargo clippy.
$CARGO_CLIPPY
echo
echo -e "$(BYELLOW WARN) ${MSG}: please try fix $(BYELLOW clippy issues) if any"
echo
cargo clippy
echo -e "$(GREEN INFO): cargo clippy done"

# 2. cargo fmt: format with nightly and stable.

CHANGED_BY_CARGO_FMT=false
echo -e "$(GREEN INFO): cargo fmt with nightly and stable ..."

for version in nightly stable; do
CMD="cargo +${version} fmt"
${CMD} --all -q -- --check 2>/dev/null
if [ $? -ne 0 ]; then
${CMD} --all
echo -e "$(BYELLOW WARN): ${CMD} changed some files"
CHANGED_BY_CARGO_FMT=true
fi
done

if ${CHANGED_BY_CARGO_FMT}; then
echo -e "$(RED FAIL): git commit $(RED ABORTED), please have a look and run git add/commit again"
exit 1
fi

exit 0

0 comments on commit ca984ed

Please sign in to comment.