From 8fccd49ac65a8deda5534521163cf9c027edb26b Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Tue, 17 Oct 2023 12:48:57 +1100 Subject: [PATCH] Check for changes to the public API We would like to get to a stage where we can commit to the public API. To help us achieve this add a script that generates the public API and checks it against three committed files, one for each feature set: no features, alloc, std. The idea is that with this applied any PR that changes the public API should include a final patch that is just the changes to the api/*.txt files, that way reviewers can discuss the changes without even needing to look at the code, quickly giving concept ACK/NACKs. We also run the script in CI to make sure we have not accidentally changed the public API so that we can be confident that don't break semver during releases. The script can also be used to diff between two release versions to get a complete list of API changes, useful for writing release notes and for users upgrading. There is a development burden involved if we apply this patch. --- .github/workflows/rust.yml | 15 +++++++++ README.md | 7 ++++ api/README.md | 11 +++++++ api/all-features.txt | 0 api/alloc-only.txt | 0 api/no-features.txt | 0 contrib/check-for-api-changes.sh | 55 ++++++++++++++++++++++++++++++++ justfile | 4 +++ 8 files changed, 92 insertions(+) create mode 100644 api/README.md create mode 100644 api/all-features.txt create mode 100644 api/alloc-only.txt create mode 100644 api/no-features.txt create mode 100755 contrib/check-for-api-changes.sh diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 15f9b86d7..40da8238f 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -140,3 +140,18 @@ jobs: run: cargo install cross --locked - name: run cross test run: cross test --target s390x-unknown-linux-gnu + + API: + name: Check for changes to the public API + runs-on: ubuntu-latest + strategy: + fail-fast: false + steps: + - name: Checkout Crate + uses: actions/checkout@v3 + - name: Checkout Toolchain + uses: dtolnay/rust-toolchain@nightly + - name: Install cargo-public-api + run: cargo install --locked cargo-public-api + - name: Running API checker script + run: ./contrib/check-for-api-changes.sh diff --git a/README.md b/README.md index 773f9b589..7067c9ab4 100644 --- a/README.md +++ b/README.md @@ -33,3 +33,10 @@ Alternatively add symlinks in your `.git/hooks` directory to any of the githooks We use a custom Rust compiler configuration conditional to guard the benchmark code. To run the benchmarks use: `RUSTFLAGS='--cfg=bench' cargo +nightly bench`. + + +## API changes + +All PRs that change the public API of `rust-bech32` must include a patch to the +`api/` text files. For PRs that include API changes, add a separate patch to the PR +that is the diff created by running `contrib/check-for-api-changes.sh`. diff --git a/api/README.md b/api/README.md new file mode 100644 index 000000000..e2289b096 --- /dev/null +++ b/api/README.md @@ -0,0 +1,11 @@ +API text files +============== + +Each file here lists the public API when built with some set of features +enabled. To create these files run `../contrib/check-for-api-changes.sh`: + +Requires `cargo-public-api`, install with: + + `cargo +stable install cargo-public-api --locked`. + +ref: https://github.com/enselic/cargo-public-api diff --git a/api/all-features.txt b/api/all-features.txt new file mode 100644 index 000000000..e69de29bb diff --git a/api/alloc-only.txt b/api/alloc-only.txt new file mode 100644 index 000000000..e69de29bb diff --git a/api/no-features.txt b/api/no-features.txt new file mode 100644 index 000000000..e69de29bb diff --git a/contrib/check-for-api-changes.sh b/contrib/check-for-api-changes.sh new file mode 100755 index 000000000..59f6a26ac --- /dev/null +++ b/contrib/check-for-api-changes.sh @@ -0,0 +1,55 @@ +#!/usr/bin/env bash +# +# Checks the public API of crates, exits with non-zero if there are currently +# changes to the public API not already committed to in the various api/*.txt +# files. + +set -e + +REPO_DIR=$(git rev-parse --show-toplevel) +API_DIR="$REPO_DIR/api" +CARGO="cargo +nightly public-api --simplified" +SORT="sort --numeric-sort --ignore-case --ignore-nonprinting --stable --unique" + +main() { + # cargo public-api uses nightly so the toolchain must be available. + if ! cargo +nightly --version > /dev/null; then + echo "script requires a nightly toolchain to be installed (possibly >= nightly-2023-05-24)" >&2 + exit 1 + fi + + generate_api_files + check_for_changes +} + +generate_api_files() { + pushd "$REPO_DIR" > /dev/null + + $CARGO --no-default-features | $SORT > "$API_DIR/no-features.txt" + $CARGO --no-default-features --features=alloc | $SORT > "$API_DIR/alloc-only.txt" + $CARGO --all-features | $SORT > "$API_DIR/all-features.txt" + + popd > /dev/null +} + +# Check if there are changes (dirty git index) to the `api/` directory. +check_for_changes() { + pushd "$REPO_DIR" > /dev/null + + if [[ $(git status --porcelain api) ]]; then + git diff --color=always + echo "You have introduced changes to the public API, commit the changes to api/ currently in your working directory" >&2 + exit 1 + + else + echo "No changes to the current public API" + fi + + popd > /dev/null +} + +# +# Main script +# +main "$@" +exit 0 diff --git a/justfile b/justfile index 2e556e906..e365358e7 100644 --- a/justfile +++ b/justfile @@ -30,3 +30,7 @@ clippy: # run `cargo clippy --fix` on everything clippy-fix: cargo clippy --locked --offline --workspace --all-targets --fix + +# Check for API changes. +check-api: + contrib/check-for-api-changes.sh