Skip to content

Commit

Permalink
Merge #141: Check for changes to the public API
Browse files Browse the repository at this point in the history
8241316 Check for public API changes (Tobin C. Harding)

Pull request description:

  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.

ACKs for top commit:
  clarkmoody:
    ACK 8241316
  apoelstra:
    ACK 8241316

Tree-SHA512: 22a47e456ecedd7f1019c3f90bafbebf40ed841d24b577c08f456883638b1b49ab2f2c86b0a642e8b425f528b1bf25015642627b3bc3c6c2b19fb3f4b1605939
  • Loading branch information
apoelstra committed Feb 13, 2024
2 parents 92146d7 + 8241316 commit ea5385e
Show file tree
Hide file tree
Showing 8 changed files with 3,049 additions and 0 deletions.
15 changes: 15 additions & 0 deletions .github/workflows/rust.yml
Expand Up @@ -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
7 changes: 7 additions & 0 deletions README.md
Expand Up @@ -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`.
11 changes: 11 additions & 0 deletions 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
1,045 changes: 1,045 additions & 0 deletions api/all-features.txt

Large diffs are not rendered by default.

988 changes: 988 additions & 0 deletions api/alloc-only.txt

Large diffs are not rendered by default.

916 changes: 916 additions & 0 deletions api/no-features.txt

Large diffs are not rendered by default.

63 changes: 63 additions & 0 deletions contrib/check-for-api-changes.sh
@@ -0,0 +1,63 @@
#!/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 -n -u` doesn't work for some reason.
SORT="sort --numeric-sort"

# Sort order is effected by locale. See `man sort`.
# > Set LC_ALL=C to get the traditional sort order that uses native byte values.
export LC_ALL=C

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 | uniq > "$API_DIR/no-features.txt"
$CARGO --no-default-features --features=alloc | $SORT | uniq > "$API_DIR/alloc-only.txt"
$CARGO --all-features | $SORT | uniq > "$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
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
4 changes: 4 additions & 0 deletions justfile
Expand Up @@ -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

0 comments on commit ea5385e

Please sign in to comment.