From eeae225cfca149d669b0ba7e623faca970e82a65 Mon Sep 17 00:00:00 2001 From: Jose Storopoli Date: Mon, 1 Jul 2024 18:18:05 +0000 Subject: [PATCH] ci: add cargo-semver-checks --- .github/workflows/rust.yml | 33 ++++++ CONTRIBUTING.md | 8 ++ contrib/check-semver.sh | 202 +++++++++++++++++++++++++++++++++++++ 3 files changed, 243 insertions(+) create mode 100755 contrib/check-semver.sh diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index bad19238c..78e9fde47 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -302,3 +302,36 @@ jobs: uses: model-checking/kani-github-action@v1.1 with: args: "--only-codegen" + + API: + needs: Prepare + name: API - nightly toolchain + runs-on: ubuntu-latest + strategy: + fail-fast: false + steps: + - name: "Checkout repo" + uses: actions/checkout@v4 + with: + fetch-depth: 0 # we need full history for cargo semver-checks + - name: "Select toolchain" + uses: dtolnay/rust-toolchain@v1 + with: + toolchain: ${{ needs.Prepare.outputs.nightly_version }} + - name: "Install cargo-binstall" + uses: cargo-bins/cargo-binstall@main + - name: "Binstall cargo-semver-checks" + run: cargo binstall cargo-semver-checks --no-confirm + - name: "Run semver checker script" + run: ./contrib/check-semver.sh + - name: "Add PR label to breaking changes" + uses: actions-ecosystem/action-add-labels@v1 + if: ${{ hashFiles('semver-break') != '' }} + with: + labels: "API break" + - name: Comment PR + uses: thollander/actions-comment-pull-request@v2 + if: ${{ hashFiles('semver-break') != '' }} + with: + message: | + :rotating_light: API BREAKING CHANGE DETECTED diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index add1f9132..0ee149cbb 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -194,6 +194,14 @@ Use of `unsafe` code is prohibited unless there is a unanimous decision among library maintainers on the exclusion from this rule. In such cases there is a requirement to test unsafe code with sanitizers including Miri. +### API changes + +All PRs that change the public API of `rust-bitcoin` will be checked on CI for +semversioning compliance. This means that if the PR changes the public API in a +way that is not backwards compatible, the PR will be flagged as a breaking change. +Please check the [Rust workflow](.github/workflows/rust.yml). +Under the hood we use [`cargo-semver-checks`](https://github.com/obi1kenobi/cargo-semver-checks). + ### Policy diff --git a/contrib/check-semver.sh b/contrib/check-semver.sh new file mode 100755 index 000000000..2868606c8 --- /dev/null +++ b/contrib/check-semver.sh @@ -0,0 +1,202 @@ +#!/usr/bin/env bash +# +# Checks semver compatibility between the current and target branches. +# Under the hood uses cargo semver-checks to check for breaking changes. +# We cannot use it directly since it only supports checking against published +# crates. +# That's the intended use case for cargo semver-checks: +# you run before publishing a new version of a crate to check semver breaks. +# Here we are hacking it by first generating JSON files from cargo doc +# and then using those files to check for breaking changes with +# cargo semver-checks. + +set -euo pipefail + +# Our nightly version. +NIGHTLY=$(cat nightly-version) + +# These are the hardcoded flags that cargo semver-checks uses +# under the hood to invoke rustdoc. +RUSTDOCFLAGS="-Z unstable-options --document-private-items --document-hidden-items --output-format=json --cap-lints=allow" + +# These will be set to the commit SHA from the PR's target branch +# GitHub Actions CI. +# NOTE: if running locally this will be set to master. +if [ -n "${GITHUB_BASE_REF+x}" ]; then + TARGET_COMMIT=$GITHUB_BASE_REF # running on CI +else + TARGET_COMMIT=$(git rev-parse master) # running locally +fi + +main() { + # we need cargo nightly to generate the JSON files from cargo doc. + need_nightly + + # On current commit: + # 1. bitcoin: all-features and no-default-features. + generate_json_files_all_features "bitcoin" "current" + generate_json_files_no_default_features "bitcoin" "current" + + # 2. base58ck: all-features and no-default-features. + generate_json_files_all_features "base58ck" "current" + generate_json_files_no_default_features "base58ck" "current" + + # 3. bitcoin_hashes: no-default-features and alloc feature. + generate_json_files_no_default_features "bitcoin_hashes" "current" + generate_json_files_features_alloc "bitcoin_hashes" "current" + + # 4. bitcoin-units: no-default-features and alloc feature. + generate_json_files_no_default_features "bitcoin-units" "current" + generate_json_files_features_alloc "bitcoin-units" "current" + + # 5. bitcoin-io: no-default-features and alloc feature. + generate_json_files_no_default_features "bitcoin-io" "current" + generate_json_files_features_alloc "bitcoin-io" "current" + + + # Switch to target commit. + echo "Checking out target commit at $TARGET_COMMIT" + git checkout "$TARGET_COMMIT" + + # On target commit: + # 1. bitcoin: all-features and no-default-features. + generate_json_files_all_features "bitcoin" "master" + generate_json_files_no_default_features "bitcoin" "master" + + # 2. base58ck: all-features and no-default-features. + generate_json_files_all_features "base58ck" "master" + generate_json_files_no_default_features "base58ck" "master" + + # 3. bitcoin_hashes: no-default-features and alloc feature. + generate_json_files_no_default_features "bitcoin_hashes" "master" + generate_json_files_features_alloc "bitcoin_hashes" "master" + + # 4. bitcoin-units: no-default-features and alloc feature. + generate_json_files_no_default_features "bitcoin-units" "master" + generate_json_files_features_alloc "bitcoin-units" "master" + + # 5. bitcoin-io: no-default-features and alloc feature. + generate_json_files_no_default_features "bitcoin-io" "master" + generate_json_files_features_alloc "bitcoin-io" "master" + + # Check for API semver breaks on all the generated JSON files above. + run_cargo_semver_check "bitcoin" "all-features" + run_cargo_semver_check "bitcoin" "no-default-features" + run_cargo_semver_check "base58ck" "all-features" + run_cargo_semver_check "base58ck" "no-default-features" + run_cargo_semver_check "bitcoin_hashes" "no-default-features" + run_cargo_semver_check "bitcoin_hashes" "alloc" + run_cargo_semver_check "bitcoin-units" "no-default-features" + run_cargo_semver_check "bitcoin-units" "alloc" + run_cargo_semver_check "bitcoin-io" "no-default-features" + run_cargo_semver_check "bitcoin-io" "alloc" + + # Invoke cargo semver-checks to check for breaking changes + # in all generated files. + check_for_breaking_changes +} + +# Run cargo doc with the cargo semver-checks rustdoc flags. +# We don't care about dependencies. +run_cargo_doc() { + RUSTDOCFLAGS="$RUSTDOCFLAGS" cargo +"$NIGHTLY" doc --no-deps "$@" +} + +# Run cargo semver-check +run_cargo_semver_check() { + local crate="$1" + local variant="$2" + + echo "Running cargo semver-checks for $crate $variant" + # Hack to not fail on errors. + # This is necessary since cargo semver-checks will fail if the + # semver check fails. + # We check that manually later. + set +e + cargo +"$NIGHTLY" semver-checks -v --baseline-rustdoc "$crate-master-$variant.json" --current-rustdoc "$crate-current-$variant.json" > "$crate-$variant-semver.txt" 2>&1 + set -e +} + +# The following function uses cargo doc to generate JSON files that +# cargo semver-checks can use. +# - no-default-features: generate JSON doc files with no default features. +generate_json_files_no_default_features() { + local crate="$1" + local version="$2" + + echo "Running cargo doc no-default-features for $crate $version" + run_cargo_doc --no-default-features -p "$crate" + + # replace _ for - in crate name. + # This is necessary since some crates have - in their name + # which will be converted to _ in the output file by cargo doc. + mv "target/doc/${crate//-/_}.json" "$crate-$version-no-default-features.json" +} +# - all-features: generate JSON doc files with all features. +generate_json_files_all_features() { + local crate="$1" + local version="$2" + + echo "Running cargo doc all-features for $crate $version" + run_cargo_doc --all-features -p "$crate" + + # replace _ for - in crate name. + # This is necessary since some crates have - in their name + # which will be converted to _ in the output file by cargo doc. + mv -v "target/doc/${crate//-/_}.json" "$crate-$version-all-features.json" +} +# - alloc: generate JSON doc files with the alloc feature. +generate_json_files_features_alloc() { + local crate="$1" + local version="$2" + + echo "Running cargo doc --features alloc for $crate $version" + run_cargo_doc --no-default-features --features alloc -p "$crate" + + # replace _ for - in crate name. + # This is necessary since some crates have - in their name + # which will be converted to _ in the output file by cargo doc. + mv -v "target/doc/${crate//-/_}.json" "$crate-$version-alloc.json" +} + +# Check if there are breaking changes. +# We loop through all the generated files and check if there is a FAIL +# in the cargo semver-checks output. +# If we detect a fail, we create an empty file semver-break. +# If the following CI step finds this file, it will add: +# 1. a comment on the PR. +# 2. a label to the PR. +check_for_breaking_changes() { + for file in *semver.txt; do + echo "Checking $file" + if grep -q "FAIL" "$file"; then + echo "You have introduced changes to the public API" + echo "FAIL found in $file" + # flag it as a breaking change + # Handle the case where FAIL is found + touch semver-break + fi + done + if ! [ -f semver-break ]; then + echo "No breaking changes found" + fi +} + +# Safekeeping: check if we have a nightly compiler. +need_nightly() { + cargo_ver=$(cargo +"$NIGHTLY" --version) + if echo "$cargo_ver" | grep -q -v nightly; then + err "Need a nightly compiler; have $cargo_ver" + fi +} + +err() { + echo "$1" >&2 + exit 1 +} + +# +# Main script +# +main "$@" +exit 0 \ No newline at end of file