Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add git pre-push hook #728

Merged
merged 17 commits into from
Feb 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
144 changes: 7 additions & 137 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -328,13 +328,7 @@ jobs:
steps:
- uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
- name: Check Rust formatting
run: |
set -eo pipefail
cargo fmt --check -p zerocopy
cargo fmt --check -p zerocopy-derive
shopt -s globstar
rustfmt --check tests/**/*.rs
rustfmt --check zerocopy-derive/tests/**/*.rs
run: ./ci/check_fmt.sh

check_readme:
needs: generate_cache
Expand All @@ -347,16 +341,7 @@ jobs:
uses: ./.github/actions/cache

- name: Check README.md
run: |
set -eo pipefail

# Install again in case the installation failed during the
# `generate_cache` step. We treat that step as best-effort and
# suppress all errors from it.
cargo install cargo-readme --version 3.2.0

diff <(./generate-readme.sh) README.md
exit $?
run: ./ci/check_readme.sh

check_msrv:
needs: generate_cache
Expand All @@ -378,25 +363,7 @@ jobs:
# that compiling zerocopy with the `derive` feature enabled would fail
# on its own published MSRV)
- name: Check MSRVs match
run: |
set -eo pipefail

# Usage: msrv <crate-name>
function msrv {
cargo metadata --format-version 1 | jq -r ".packages[] | select(.name == \"$1\").rust_version"
}

ver_zerocopy=$(msrv zerocopy)
ver_zerocopy_derive=$(msrv zerocopy-derive)

if [[ "$ver_zerocopy" == "$ver_zerocopy_derive" ]]; then
echo "Same MSRV ($ver_zerocopy) found for zerocopy and zerocopy-derive." | tee -a $GITHUB_STEP_SUMMARY
exit 0
else
echo "Different MSRVs found for zerocopy ($ver_zerocopy) and zerocopy-derive ($ver_zerocopy_derive)." \
| tee -a $GITHUB_STEP_SUMMARY >&2
exit 1
fi
run: ./ci/check_msrv.sh

check_versions:
needs: generate_cache
Expand All @@ -412,67 +379,8 @@ jobs:
# depends exactly upon the current version of zerocopy-derive. See
# `INTERNAL.md` for an explanation of why we do this.
- name: Check crate versions match
run: |
set -eo pipefail

# Usage: version <crate-name>
function version {
cargo metadata --format-version 1 | jq -r ".packages[] | select(.name == \"$1\").version"
}

ver_zerocopy=$(version zerocopy)
ver_zerocopy_derive=$(version zerocopy-derive)

# Usage: dependency-version <kind> <target>
function dependency-version {
KIND="$1"
TARGET="$2"
cargo metadata --format-version 1 \
| jq -r ".packages[] | select(.name == \"zerocopy\").dependencies[] | select((.name == \"zerocopy-derive\") and .kind == $KIND and .target == $TARGET).req"
}

# The non-dev dependency version (kind `null` filters out the dev
# dependency, and target `null` filters out the targeted version).
zerocopy_derive_dep_ver=$(dependency-version null null)

# The non-dev dependency, targeted version (kind `null` filters out
# the dev dependency).
zerocopy_derive_targeted_ver=$(dependency-version null '"cfg(any())"')

# The dev dependency version (kind `"dev"` selects only the dev
# dependency).
zerocopy_derive_dev_dep_ver=$(dependency-version '"dev"' null)

function assert-match {
VER_A="$1"
VER_B="$2"
SUCCESS_MSG="$3"
FAILURE_MSG="$4"
if [[ "$ver_zerocopy" == "$ver_zerocopy_derive" ]]; then
echo "$SUCCESS_MSG" | tee -a $GITHUB_STEP_SUMMARY
else
echo "$FAILURE_MSG" | tee -a $GITHUB_STEP_SUMMARY >&2
exit 1
fi
}

assert-match "$ver_zerocopy" "$ver_zerocopy_derive" \
"Same crate version ($ver_zerocopy) found for zerocopy and zerocopy-derive." \
"Different crate versions found for zerocopy ($ver_zerocopy) and zerocopy-derive ($ver_zerocopy_derive)."

# Note the leading `=` sign - the dependency needs to be an exact one.
assert-match "=$ver_zerocopy_derive" "$zerocopy_derive_dep_ver" \
"zerocopy depends upon same version of zerocopy-derive in-tree ($zerocopy_derive_dep_ver)." \
"zerocopy depends upon different version of zerocopy-derive ($zerocopy_derive_dep_ver) than the one in-tree ($ver_zerocopy_derive)."

# Note the leading `=` sign - the dependency needs to be an exact one.
assert-match "=$ver_zerocopy_derive" "$zerocopy_derive_dev_dep_ver" \
"In dev mode, zerocopy depends upon same version of zerocopy-derive in-tree ($zerocopy_derive_dev_dep_ver)." \
"In dev mode, zerocopy depends upon different version of zerocopy-derive ($zerocopy_derive_dev_dep_ver) than the one in-tree ($ver_zerocopy_derive)."

assert-match "$zerocopy_derive_dep_ver" "$zerocopy_derive_targeted_ver" \
"Same crate version ($zerocopy_derive_dep_ver) found for optional and targeted zerocopy-derive dependency." \
"Different crate versions found for optional ($zerocopy_derive_dep_ver) and targeted ($zerocopy_derive_targeted_ver) dependency."
run: ./ci/check_versions.sh

generate_cache:
runs-on: ubuntu-latest
name: Generate cache
Expand Down Expand Up @@ -511,22 +419,8 @@ jobs:
run: go install github.com/mikefarah/yq/v4@latest
- uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
- name: Run check
run: |
set -eo pipefail
run: ./ci/check_all_toolchains_tested.sh

# Check whether the set of toolchains tested in this file (other than
# 'msrv', 'stable', and 'nightly') is equal to the set of toolchains
# listed in the 'package.metadata.build-rs' section of Cargo.toml.
#
# If the inputs to `diff` are not identical, `diff` exits with a
# non-zero error code, which causes this script to fail (thanks to
# `set -e`).
diff \
<(cat .github/workflows/ci.yml | yq '.jobs.build_test.strategy.matrix.toolchain | .[]' | \
sort -u | grep -v '^\(msrv\|stable\|nightly\)$') \
<(cargo metadata --format-version 1 | \
jq -r ".packages[] | select(.name == \"zerocopy\").metadata.\"build-rs\" | keys | .[]" | \
sort -u)
check-job-dependencies:
runs-on: ubuntu-latest
name: Check all-jobs-succeeded depends on all jobs
Expand All @@ -535,31 +429,7 @@ jobs:
run: go install github.com/mikefarah/yq/v4@latest
- uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
- name: Run dependency check
run: |
set -eo pipefail
jobs=$(for i in $(find .github -iname '*.yaml' -or -iname '*.yml')
do
# Select jobs that are triggered by pull request.
if yq -e '.on | has("pull_request")' "$i" 2>/dev/null >/dev/null
then
# This gets the list of jobs that all-jobs-succeed does not depend on.
comm -23 \
<(yq -r '.jobs | keys | .[]' "$i" | sort | uniq) \
<(yq -r '.jobs.all-jobs-succeed.needs[]' "$i" | sort | uniq)
fi

# The grep call here excludes all-jobs-succeed from the list of jobs that
# all-jobs-succeed does not depend on. If all-jobs-succeed does
# not depend on itself, we do not care about it.
done | sort | uniq | (grep -v '^all-jobs-succeed$' || true)
)

if [ -n "$jobs" ]
then
missing_jobs="$(echo "$jobs" | tr ' ' '\n')"
echo "all-jobs-succeed missing dependencies on some jobs: $missing_jobs" | tee -a $GITHUB_STEP_SUMMARY
exit 1
fi
run: ./ci/check_job_dependencies.sh

# Used to signal to branch protections that all other jobs have succeeded.
all-jobs-succeed:
Expand Down
15 changes: 15 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,21 @@ Commit messages should never contain references to any of:
https://github.com/google/zerocopy/commit/789b3deb)
1. Other entities which may not make sense to arbitrary future readers

### Git hooks

There is a pre-push git hook available at `./githooks/pre-push`. This hook runs
some quick checks locally before pushing so that these same checks won't later
fail during CI. The intention is to minimise needing to fix trivial breakages
after having already just pushed (which can be annoying).

Git won't use this hook automatically. If you would like to use it, set
repository config `core.hooksPath` to `githooks`. This can be done by running
(at repository root):

```sh
git config --local core.hooksPath githooks
```

## Community Guidelines

This project follows [Google's Open Source Community
Expand Down
16 changes: 16 additions & 0 deletions ci/check_all_toolchains_tested.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
#!/usr/bin/env bash
set -eo pipefail

# Check whether the set of toolchains tested in this file (other than
# 'msrv', 'stable', and 'nightly') is equal to the set of toolchains
# listed in the 'package.metadata.build-rs' section of Cargo.toml.
#
# If the inputs to `diff` are not identical, `diff` exits with a
# non-zero error code, which causes this script to fail (thanks to
# `set -e`).
diff \
<(cat .github/workflows/ci.yml | yq '.jobs.build_test.strategy.matrix.toolchain | .[]' | \
sort -u | grep -v '^\(msrv\|stable\|nightly\)$') \
<(cargo metadata --format-version 1 | \
jq -r ".packages[] | select(.name == \"zerocopy\").metadata.\"build-rs\" | keys | .[]" | \
sort -u)
9 changes: 9 additions & 0 deletions ci/check_fmt.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#!/usr/bin/env bash
set -eo pipefail
files=$(find . -iname '*.rs' -type f -not -path './target/*')
# check that find succeeded
if [[ -z $files ]]
then
exit 1
fi
rustfmt --check $files
26 changes: 26 additions & 0 deletions ci/check_job_dependencies.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
#!/usr/bin/env bash
set -eo pipefail
which yq > /dev/null
jobs=$(for i in $(find .github -iname '*.yaml' -or -iname '*.yml')
do
# Select jobs that are triggered by pull request.
if yq -e '.on | has("pull_request")' "$i" 2>/dev/null >/dev/null
then
# This gets the list of jobs that all-jobs-succeed does not depend on.
comm -23 \
<(yq -r '.jobs | keys | .[]' "$i" | sort | uniq) \
<(yq -r '.jobs.all-jobs-succeed.needs[]' "$i" | sort | uniq)
fi

# The grep call here excludes all-jobs-succeed from the list of jobs that
# all-jobs-succeed does not depend on. If all-jobs-succeed does
# not depend on itself, we do not care about it.
done | sort | uniq | (grep -v '^all-jobs-succeed$' || true)
)

if [ -n "$jobs" ]
then
missing_jobs="$(echo "$jobs" | tr ' ' '\n')"
echo "all-jobs-succeed missing dependencies on some jobs: $missing_jobs" | tee -a $GITHUB_STEP_SUMMARY
exit 1
fi
19 changes: 19 additions & 0 deletions ci/check_msrv.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
#!/usr/bin/env bash
set -eo pipefail

# Usage: msrv <crate-name>
function msrv {
cargo metadata --format-version 1 | jq -r ".packages[] | select(.name == \"$1\").rust_version"
}

ver_zerocopy=$(msrv zerocopy)
ver_zerocopy_derive=$(msrv zerocopy-derive)

if [[ "$ver_zerocopy" == "$ver_zerocopy_derive" ]]; then
echo "Same MSRV ($ver_zerocopy) found for zerocopy and zerocopy-derive." | tee -a $GITHUB_STEP_SUMMARY
exit 0
else
echo "Different MSRVs found for zerocopy ($ver_zerocopy) and zerocopy-derive ($ver_zerocopy_derive)." \
| tee -a $GITHUB_STEP_SUMMARY >&2
exit 1
fi
10 changes: 10 additions & 0 deletions ci/check_readme.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
#!/usr/bin/env bash
set -eo pipefail

# Install again in case the installation failed during the
# `generate_cache` step. We treat that step as best-effort and
# suppress all errors from it.
cargo install cargo-readme --version 3.2.0 -q

diff <(./generate-readme.sh) README.md
exit $?
61 changes: 61 additions & 0 deletions ci/check_versions.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
#!/usr/bin/env bash
set -eo pipefail

# Usage: version <crate-name>
function version {
cargo metadata --format-version 1 | jq -r ".packages[] | select(.name == \"$1\").version"
}

ver_zerocopy=$(version zerocopy)
ver_zerocopy_derive=$(version zerocopy-derive)

# Usage: dependency-version <kind> <target>
function dependency-version {
KIND="$1"
TARGET="$2"
cargo metadata --format-version 1 \
| jq -r ".packages[] | select(.name == \"zerocopy\").dependencies[] | select((.name == \"zerocopy-derive\") and .kind == $KIND and .target == $TARGET).req"
}

# The non-dev dependency version (kind `null` filters out the dev
# dependency, and target `null` filters out the targeted version).
zerocopy_derive_dep_ver=$(dependency-version null null)

# The non-dev dependency, targeted version (kind `null` filters out
# the dev dependency).
zerocopy_derive_targeted_ver=$(dependency-version null '"cfg(any())"')

# The dev dependency version (kind `"dev"` selects only the dev
# dependency).
zerocopy_derive_dev_dep_ver=$(dependency-version '"dev"' null)

function assert-match {
VER_A="$1"
VER_B="$2"
SUCCESS_MSG="$3"
FAILURE_MSG="$4"
if [[ "$VER_A" == "$VER_B" ]]; then
echo "$SUCCESS_MSG" | tee -a $GITHUB_STEP_SUMMARY
else
echo "$FAILURE_MSG" | tee -a $GITHUB_STEP_SUMMARY >&2
exit 1
fi
}

assert-match "$ver_zerocopy" "$ver_zerocopy_derive" \
"Same crate version ($ver_zerocopy) found for zerocopy and zerocopy-derive." \
"Different crate versions found for zerocopy ($ver_zerocopy) and zerocopy-derive ($ver_zerocopy_derive)."

# Note the leading `=` sign - the dependency needs to be an exact one.
assert-match "=$ver_zerocopy_derive" "$zerocopy_derive_dep_ver" \
"zerocopy depends upon same version of zerocopy-derive in-tree ($zerocopy_derive_dep_ver)." \
"zerocopy depends upon different version of zerocopy-derive ($zerocopy_derive_dep_ver) than the one in-tree ($ver_zerocopy_derive)."

# Note the leading `=` sign - the dependency needs to be an exact one.
assert-match "=$ver_zerocopy_derive" "$zerocopy_derive_dev_dep_ver" \
"In dev mode, zerocopy depends upon same version of zerocopy-derive in-tree ($zerocopy_derive_dev_dep_ver)." \
"In dev mode, zerocopy depends upon different version of zerocopy-derive ($zerocopy_derive_dev_dep_ver) than the one in-tree ($ver_zerocopy_derive)."

assert-match "$zerocopy_derive_dep_ver" "$zerocopy_derive_targeted_ver" \
"Same crate version ($zerocopy_derive_dep_ver) found for optional and targeted zerocopy-derive dependency." \
"Different crate versions found for optional ($zerocopy_derive_dep_ver) and targeted ($zerocopy_derive_targeted_ver) dependency."
10 changes: 10 additions & 0 deletions githooks/pre-push
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
#!/usr/bin/env bash
set -eo pipefail
echo "Running pre-push git hook: $0"
# forego redirecting stdout to /dev/null on check_fmt.sh because the output
# from cargofmt is useful (and the good stuff is not delivered by stderr)
./ci/check_fmt.sh
tommy-gilligan marked this conversation as resolved.
Show resolved Hide resolved
./ci/check_job_dependencies.sh > /dev/null
./ci/check_msrv.sh > /dev/null
./ci/check_readme.sh > /dev/null
./ci/check_versions.sh > /dev/null
Loading