From 17302ec2c27eb571eb38aaec63e98915aee7fc7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karol=20Bary=C5=82a?= Date: Sun, 28 Jan 2024 17:20:08 +0100 Subject: [PATCH 1/6] Add script and makefile enteries for cargo-semver-checks This allows to run the tool manually. Right now the script doesn't do much, but when we want to run it with various feature sets or other flags it will be more useful. --- Makefile | 8 ++++++++ scripts/semver-checks.sh | 5 +++++ 2 files changed, 13 insertions(+) create mode 100755 scripts/semver-checks.sh diff --git a/Makefile b/Makefile index e09ee6bc60..91d4485bf9 100644 --- a/Makefile +++ b/Makefile @@ -48,6 +48,14 @@ build: docs: mdbook build docs +.PHONY: semver-rev +semver-rev: + ./scripts/semver-checks.sh $(if $(rev),--baseline-rev $(rev),--baseline-rev main) + +.PHONY: semver-version +semver-version: + ./scripts/semver-checks.sh $(if $(version),--baseline-version $(version),) + .PHONY: up up: $(COMPOSE) up -d --wait diff --git a/scripts/semver-checks.sh b/scripts/semver-checks.sh new file mode 100755 index 0000000000..eafdb4629d --- /dev/null +++ b/scripts/semver-checks.sh @@ -0,0 +1,5 @@ +#!/bin/sh + +set -x + +cargo semver-checks -p scylla -p scylla-cql $@ From 509941ae0d2b4cf5173eba62e559e018b3053f09 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karol=20Bary=C5=82a?= Date: Wed, 10 Jan 2024 17:06:32 +0100 Subject: [PATCH 2/6] CI: Introduce cargo-semver-checks This workflow looks for breaking API changes using popular cargo-semver-checks tool. This comit introduces it only for PRs. If PR breaks API, appropriate label is added, otherwise it is removed. --- .github/workflows/semver_checks.yml | 81 +++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+) create mode 100644 .github/workflows/semver_checks.yml diff --git a/.github/workflows/semver_checks.yml b/.github/workflows/semver_checks.yml new file mode 100644 index 0000000000..81618685aa --- /dev/null +++ b/.github/workflows/semver_checks.yml @@ -0,0 +1,81 @@ +# This workflow tests semver compatibilty. +# For PRs it checks if PR makes any API breaking changes, and assings appropriate label if so. +name: Semver checks + +on: + pull_request_target: + branches: + - main + - 'branch-*' + +env: + CARGO_TERM_COLOR: always + RUST_BACKTRACE: full + PR_BASE: ${{ github.event.pull_request.base.sha }} + PR_HEAD: ${{ github.event.pull_request.head.sha }} + PR_ID: ${{ github.event.number }} + +jobs: + semver-pull-request-check: + runs-on: ubuntu-latest + if: github.event_name == 'pull_request_target' + # Disable all permissions + # This is important, because this job runs on untrusted input from + # the user and it's possible for the user to take over the job, + # for example by adding malicious build.rs file. If the job had, + # for example, `pull_requests: write` permission, malicous user + # could do us a lot of harm. This is also the reason that there are + # 2 jobs - it's so that it's not possible to take over a job that + # has permissions. + permissions: {} + timeout-minutes: 30 + outputs: + exitcode: ${{ steps.semver-pr-check.outputs.exitcode }} + steps: + - name: Checkout + uses: actions/checkout@v3 + with: + fetch-depth: "2" + ref: "refs/pull/${{ env.PR_ID }}/merge" + # Check if there was another push before this job started. + # If there was, wrong commit would be checked out. + - name: Sanity check + run: | + [[ "$(git rev-parse 'HEAD^2')" == "$PR_HEAD" ]] + # I don't know any way to do this using checkout action + - name: Fetch PR base + run: git fetch origin "$PR_BASE" + - name: Install semver-checks + # Official action uses binary releases fetched from GitHub + # If this pipeline becomes too slow, we should do this too + run: cargo install cargo-semver-checks --no-default-features + - name: Verify the API compatibilty with PR base + id: semver-pr-check + run: | + set +e + make semver-rev rev="$PR_BASE" + exitcode=$? + echo "exitcode=$exitcode" >> $GITHUB_OUTPUT + exit "$exitcode" + continue-on-error: true + + semver-pull-request-label: + runs-on: ubuntu-latest + if: github.event_name == 'pull_request_target' + permissions: + pull-requests: write + needs: semver-pull-request-check + timeout-minutes: 3 + steps: + - name: Remove breaking label on success + run: gh pr edit "$PR_ID" --remove-label semver-checks-breaking + if: needs.semver-pull-request-check.outputs.exitcode == '0' + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GH_REPO: ${{ github.repository }} + - name: Add breaking label on failure + run: gh pr edit "$PR_ID" --add-label semver-checks-breaking + if: needs.semver-pull-request-check.outputs.exitcode != '0' + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GH_REPO: ${{ github.repository }} From 3e908769defcf648b59f69156cfa73283247060c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karol=20Bary=C5=82a?= Date: Thu, 11 Jan 2024 00:28:53 +0100 Subject: [PATCH 3/6] CI: Verify semver compatibility when tags is pushed Add a job that checks compatibility when a tag is pushed. It gets current version from Cargo.toml, calculates previous version (unless current version is on crates.io - which should not be the case) and checks that the release type (major / minor / patch) is correct. --- .github/workflows/semver_checks.yml | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/.github/workflows/semver_checks.yml b/.github/workflows/semver_checks.yml index 81618685aa..d960a56d28 100644 --- a/.github/workflows/semver_checks.yml +++ b/.github/workflows/semver_checks.yml @@ -7,6 +7,9 @@ on: branches: - main - 'branch-*' + push: + tags: + - v*.*.* env: CARGO_TERM_COLOR: always @@ -79,3 +82,14 @@ jobs: env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} GH_REPO: ${{ github.repository }} + + semver-push-tag: + runs-on: ubuntu-latest + if: github.event_name == 'push' + timeout-minutes: 30 + steps: + - uses: actions/checkout@v3 + - name: Install semver-checks + run: cargo install cargo-semver-checks --no-default-features + - name: Run semver-checks to see if it agrees with version updates + run: make semver-version From 1e4c551bdd03c7fb51b1b5df69b133e7233fdc81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karol=20Bary=C5=82a?= Date: Sun, 28 Jan 2024 17:19:54 +0100 Subject: [PATCH 4/6] Contributing.md: add info about semver checks --- CONTRIBUTING.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ec4c7c6bed..bb005944dd 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -86,6 +86,22 @@ There are a few scenarios: using command like `cargo update -p toml_datetime --precise 0.6.3` and go back to step 3. 5. Rename `Cargo.lock` to `Cargo.lock.msrv`. +### Semver checking + +Our CI runs cargo semver-checks and labels PRs that introduce breaking changes. +If you don't intend to change public API, you can perform the checks locally, +using command `make semver-rev`. Make sure you have semver-checks installed first, +you can install it using `cargo install cargo-semver-checks`. + +`make semver-rev` will check for API breaking changes using `main` branch as baseline. +To use different branch / commit call `make semver-rev rev=BRANCH`. + +The tool is NOT perfect and only checks some aspect of semver-compatibility. +It is NOT a replacement for a human reviewer, it is only supposed to help them +and catch some erros that they might have missed. + +Tool that we curently use: https://github.com/obi1kenobi/cargo-semver-checks + ## Contributing to the book The documentation book is written using [mdbook](https://github.com/rust-lang/mdBook)\ From 4297204fd5e0726a6678fc3a29a0dad9000be83f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karol=20Bary=C5=82a?= Date: Sun, 28 Jan 2024 19:20:21 +0100 Subject: [PATCH 5/6] CI: Post output from semver-checks as a comment --- .github/workflows/semver_checks.yml | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/.github/workflows/semver_checks.yml b/.github/workflows/semver_checks.yml index d960a56d28..5c70107ba8 100644 --- a/.github/workflows/semver_checks.yml +++ b/.github/workflows/semver_checks.yml @@ -34,6 +34,7 @@ jobs: timeout-minutes: 30 outputs: exitcode: ${{ steps.semver-pr-check.outputs.exitcode }} + output: ${{ steps.semver-pr-check.outputs.output }} steps: - name: Checkout uses: actions/checkout@v3 @@ -56,8 +57,11 @@ jobs: id: semver-pr-check run: | set +e - make semver-rev rev="$PR_BASE" - exitcode=$? + echo "output<> $GITHUB_OUTPUT + # Weird sed strip ANSI colors from output + make semver-rev rev="$PR_BASE" |& tee /dev/tty | sed -r "s/\x1B\[([0-9]{1,2}(;[0-9]{1,2})?)?[mGK]//g" >> $GITHUB_OUTPUT + exitcode=${PIPESTATUS[0]} + echo "SEMVER_STDOUT_EOF" >> $GITHUB_OUTPUT echo "exitcode=$exitcode" >> $GITHUB_OUTPUT exit "$exitcode" continue-on-error: true @@ -82,6 +86,26 @@ jobs: env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} GH_REPO: ${{ github.repository }} + - name: Post report on semver break + run: | + gh pr comment "$PR_ID" --body "\ + \`cargo semver-checks\` detected some API incompatibilities in this PR. + See the following report for details: +
+ cargo semver-checks output + + \`\`\` + $SEMVER_OUTPUT + \`\`\` + +
+ " + if: needs.semver-pull-request-check.outputs.exitcode != '0' + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GH_REPO: ${{ github.repository }} + SEMVER_OUTPUT: ${{ needs.semver-pull-request-check.outputs.output }} + semver-push-tag: runs-on: ubuntu-latest From f6d68d369d2039d0d7814151d13c5bf51e5d0750 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karol=20Bary=C5=82a?= Date: Fri, 2 Feb 2024 00:27:49 +0100 Subject: [PATCH 6/6] CI: Add concurrency control to semver-checks PR job This is to avoid a race condition where a job scheduled for a first push finished after the job scheduled for a second push, resulting in incorrectly assigning / removing a label to PR. --- .github/workflows/semver_checks.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.github/workflows/semver_checks.yml b/.github/workflows/semver_checks.yml index 5c70107ba8..a52b751f76 100644 --- a/.github/workflows/semver_checks.yml +++ b/.github/workflows/semver_checks.yml @@ -32,6 +32,13 @@ jobs: # has permissions. permissions: {} timeout-minutes: 30 + # This is to prevent a situation, when job A triggered by push 1 finishes + # after job B triggered by push 2. That could result in incorrectly assigning + # or removing a PR label. + concurrency: + # Can't use `env.PR_ID` because concurrency doesn't have access to env context. + group: semver-pull-request-check-${{ github.event.number }} + cancel-in-progress: true outputs: exitcode: ${{ steps.semver-pr-check.outputs.exitcode }} output: ${{ steps.semver-pr-check.outputs.output }}