From e126a243074ffbe628a9c8425694db5fe93efb8a Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Fri, 6 Dec 2024 15:23:17 +1100 Subject: [PATCH] Add API script Add a script to generate API files using `cargo public-api` for the crates that we are trying to stabalise (the so called 'leaf crates'). - hashes - io - primitives - units We already ran the script and committed the files last patch. The fact that this patch does not include any changes to the `api/` directory and that CI passes is enough to convince us that last patch was valid. Add a CI job that runs the script and checks there are no changes to the committed text files thereby proving no API changes are made in a PR without explicitly modifying the API text files. Add documentation to `CONTRIBUTING.md` on what is expected of devs. --- .github/workflows/rust.yml | 20 +++++++ CONTRIBUTING.md | 12 +++++ contrib/check-for-api-changes.sh | 93 ++++++++++++++++++++++++++++++++ justfile | 4 ++ 4 files changed, 129 insertions(+) create mode 100755 contrib/check-for-api-changes.sh diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index cf06c57ae..c9c230cd2 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -305,3 +305,23 @@ jobs: with: args: "--only-codegen" + API: + name: API - nightly toolchain + needs: Prepare + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + dep: [recent] + steps: + - name: "Checkout repo" + uses: actions/checkout@v4 + - name: "Select toolchain" + uses: dtolnay/rust-toolchain@v1 + with: + toolchain: ${{ needs.Prepare.outputs.nightly_version }} + - name: "Install cargo-public-api" + # Pin version so that updates don't introduce changes to the text files. + run: cargo +stable install --locked cargo-public-api@0.42.0 + - name: "Run API checker script" + run: ./contrib/check-for-api-changes.sh diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d9139bacc..ff1d58665 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -145,6 +145,18 @@ test out the patch set and opine on the technical merits of the patch. Please, first review PR on the conceptual level before focusing on code style or grammar fixes. +### API changes + +The API of the following crates is almost stable. Changing it is supposed to be non-trivial. To +assist in this effort ll PRs that change the public API of any these crates must include a patch to +the `api/` text files. This should be a separate final patch to the PR that is the diff created by +running `just check-api`. + +- `hashes` +- `io` +- `primitives` +- `units` + ### Repository maintainers Pull request merge requirements: diff --git a/contrib/check-for-api-changes.sh b/contrib/check-for-api-changes.sh new file mode 100755 index 000000000..dc305dca0 --- /dev/null +++ b/contrib/check-for-api-changes.sh @@ -0,0 +1,93 @@ +#!/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 -euo pipefail + +REPO_DIR=$(git rev-parse --show-toplevel) +API_DIR="$REPO_DIR/api" + +NIGHTLY=$(cat nightly-version) +# Our docs have broken intra doc links if all features are not enabled. +RUSTDOCFLAGS="-A rustdoc::broken_intra_doc_links" + +# `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() { + need_nightly + + generate_api_files "hashes" + generate_api_files "io" + generate_api_files "primitives" + generate_api_files "units" + + check_for_changes +} + +# Uses `CARGO` to generate API files in the specified crate. +# +# Files: +# +# - no-features.txt +# - alloc-only.txt +# - all-features.txt +generate_api_files() { + local crate=$1 + pushd "$REPO_DIR/$crate" > /dev/null + + run_cargo --no-default-features | $SORT | uniq > "$API_DIR/$crate/no-features.txt" + run_cargo --no-default-features --features=alloc | $SORT | uniq > "$API_DIR/$crate/alloc-only.txt" + run_cargo_all_features | $SORT | uniq > "$API_DIR/$crate/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 + err "You have introduced changes to the public API, commit the changes to api/ currently in your working directory" + else + echo "No changes to the current public API" + fi + + popd > /dev/null +} + +# Run cargo when --all-features is not used. +run_cargo() { + RUSTDOCFLAGS="$RUSTDOCFLAGS" cargo +"$NIGHTLY" public-api --simplified "$@" +} + +# Run cargo with all features enabled. +run_cargo_all_features() { + cargo +"$NIGHTLY" public-api --simplified --all-features +} + +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 diff --git a/justfile b/justfile index ce7af6170..76f5f549e 100644 --- a/justfile +++ b/justfile @@ -36,6 +36,10 @@ sane: lint # Make an attempt to catch feature gate problems in doctests cargo test --manifest-path bitcoin/Cargo.toml --doc --no-default-features > /dev/null || exit 1 +# Check for API changes. +check-api: + contrib/check-for-api-changes.sh + # Update the recent and minimal lock files. update-lock-files: contrib/update-lock-files.sh