Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/pull_request_template.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ Include the following checklist in your PR:
* [ ] Patches conform to the [coding conventions](https://github.com/haskell/cabal/blob/master/CONTRIBUTING.md#other-conventions).
* [ ] Any changes that could be relevant to users [have been recorded in the changelog](https://github.com/haskell/cabal/blob/master/CONTRIBUTING.md#changelog).
* [ ] [Is the change significant?](https://github.com/haskell/cabal/blob/master/CONTRIBUTING.md#is-my-change-significant) If so, remember to add `significance: significant` in the changelog file.
* [ ] The `Check API` job is clean, or the API changes are documented in a changelog file.
* [ ] The documentation has been updated, if necessary.
* [ ] [Manual QA notes](https://github.com/haskell/cabal/blob/master/CONTRIBUTING.md#qa-notes) have been included.
* [ ] Tests have been added. (*Ask for help if you don’t know how to write them! Ask for an exemption if tests are too complex for too little coverage!*)
Expand All @@ -22,4 +23,5 @@ Include the following checklist in your PR:
Include the following checklist in your PR:

* [ ] Patches conform to the [coding conventions](https://github.com/haskell/cabal/blob/master/CONTRIBUTING.md#other-conventions).
* [ ] The `Check API` job is clean, or the API changes are documented in a changelog file.
* [ ] Is this a PR that fixes CI? If so, it will need to be backported to older cabal release branches (ask maintainers for directions).
79 changes: 79 additions & 0 deletions .github/scripts/api-changelog-check.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
import os
import re
import sys

# Checks that an API change found by packdiff is documented in a changelog
# entry added by the PR:
# 1. (blocking) some changelog file added/modified by the PR must list the
# package in its `packages:` frontmatter field;
# 2. (warning) the entries matching the package should mention at least one
# of the changed module/declaration names from the packdiff output.
#
# Usage: api-changelog-check.py PACKAGE API_DIFF_FILE [CHANGELOG_FILE ...]

package = sys.argv[1]
diff_path = sys.argv[2]
changelog_files = sys.argv[3:]

# ---- collect the changed names from the packdiff output ---------------------

names = set()
for line in open(diff_path, encoding="utf-8"):
m = re.match(r"^\[[ARC]\] ([\w'.]+)", line) # top level: a changed module
if m:
names.add(m.group(1))
continue
m = re.match(r"^\s+\[[ARCD]\] ([\w'.]+)", line) # nested: a declaration
if m:
names.add(m.group(1).rsplit(".", 1)[-1])

# ---- parse the `packages:` field of each changelog file ---------------------


def front_packages(path):
text = open(path, encoding="utf-8").read()
# Entries are either .cabal-style key/value files or markdown files with a
# YAML front matter; in both cases `packages:` is a top-level line.
m = re.search(r"^packages:\s*(.+?)\s*$", text, re.M)
if not m:
return []
return [p for p in re.split(r"[\s,\[\]#]+", m.group(1)) if p]


matching = [p for p in changelog_files if package in front_packages(p)]

if not matching:
listing = "\n".join(f" - {p}" for p in changelog_files) or " (none)"
print(
f"::error::The API of {package} changed with respect to the base"
f" revision, but none of the changelog entries added by this PR lists"
f" {package} in its `packages:` field. See the job summary for the"
f" diff. Add or update an entry under changelog.d/ with"
f" `packages: [{package}]`, or revert the API change."
f"\nChangelog files changed by this PR:\n{listing}"
)
sys.exit(1)

# ---- soft check: do the entries mention the changed API? --------------------

text = "\n".join(open(p, encoding="utf-8").read() for p in matching)


def mentioned(name):
pattern = r"(?<![A-Za-z0-9_'])" + re.escape(name) + r"(?![A-Za-z0-9_'])"
return re.search(pattern, text) is not None


missing = sorted(n for n in names if not mentioned(n))

if names and len(missing) == len(names):
print(
"::warning::The API of {0} changed and a changelog entry for {0} is"
" present, but it does not mention any of the changed modules or"
" declarations: {1}. Consider mentioning them explicitly, and check"
" the PVP version bump and backport implications.".format(
package, ", ".join(f"`{n}`" for n in missing[:10])
)
)
else:
print(f"The changelog entry for {package} mentions the changed API.")
129 changes: 129 additions & 0 deletions .github/workflows/check-api.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
name: Check API

# Informational API diff job: it shows which public API of the Cabal
# packages changes with respect to the PR base. The job fails when the API
# changed and the PR adds no changelog entry; if a changelog entry is
# present, the job passes while still showing the diff in its summary.

on:
pull_request:
paths-ignore:
- 'doc/**'
- '**/*.md'
- 'changelog.d/**'
- 'release-notes/**'
push:
branches:
- '3.*'
paths-ignore:
- 'doc/**'
- '**/*.md'
- 'changelog.d/**'
- 'release-notes/**'
workflow_dispatch:

permissions:
contents: read

env:
# The API depends on the GHC version, so both revisions are always built
# with a single pinned GHC. Use the GHC used for releases, see GHC_FOR_RELEASE
# in .github/workflows/validate.yml, and bump both together.
GHC_VERSION: "9.10.3"
# packdiff is not on Hackage yet; it is installed from this pinned commit
# (the same one is pinned in the `api-install` target of the Makefile).
PACKDIFF_COMMIT: "54e786de55f091cdd3b912bd72ccc0e5e252aa77"

jobs:
check-api:
name: API diff ${{ matrix.package }}
runs-on: ubuntu-latest
timeout-minutes: 60
strategy:
fail-fast: false
matrix:
package:
- Cabal-syntax
- Cabal
- cabal-install
- cabal-install-solver
- Cabal-hooks

steps:
# fetch-depth: 0 is needed because packdiff checks out the base
# revision itself and builds it.
- uses: actions/checkout@v7
with:
fetch-depth: 0
ref: ${{ github.event.pull_request.head.sha }}

- uses: haskell-actions/setup@v2
id: setup-haskell
with:
ghc-version: ${{ env.GHC_VERSION }}
cabal-version: latest

# runner.os isn't sufficient for binary compatible caches
- name: Get runner OS/version for cache keys
id: get-osver
run: echo "osver=$ImageOS" >> "$GITHUB_OUTPUT"

- uses: actions/cache@v6
with:
path: ${{ steps.setup-haskell.outputs.cabal-store }}
key: ${{ steps.get-osver.outputs.osver }}-check-api-${{ env.GHC_VERSION }}-${{ github.sha }}
restore-keys: ${{ steps.get-osver.outputs.osver }}-check-api-${{ env.GHC_VERSION }}-

- name: "Work around git problem https://bugs.launchpad.net/ubuntu/+source/git/+bug/1993586 (cabal PR #8546)"
run: git config --global protocol.file.allow always

- name: Install packdiff
run: |
mkdir -p "$HOME/.local/bin"
curl -sSL "https://github.com/composewell/packdiff/archive/${PACKDIFF_COMMIT}.tar.gz" | tar -xz -C "$RUNNER_TEMP"
cd "$RUNNER_TEMP/packdiff-${PACKDIFF_COMMIT}"
cabal install exe:packdiff --installdir="$HOME/.local/bin" --overwrite-policy=always

- name: Run packdiff
run: |
set -o pipefail
case "${{ github.event_name }}" in
pull_request) base="${{ github.event.pull_request.base.sha }}" ;;
push) base="${{ github.event.before }}" ;;
*) base="" ;;
esac
# github.event.before (push) is the all-zero commit id when the
# branch has no previous commit, e.g. it was newly created or
# force-pushed; fall back to the merge base in that case.
zero_sha="0000000000000000000000000000000000000000"
if [ -z "$base" ] || [ "$base" = "$zero_sha" ]; then
base="$(git merge-base origin/master HEAD)"
fi
head="$(git rev-parse HEAD)"
echo "Diffing ${{ matrix.package }} API: $base -> $head"
packdiff diff ${{ matrix.package }} "$base" ${{ matrix.package }} "$head" | tee api-diff.txt

sed -n '/API Annotations/,$p' api-diff.txt > api-diff-summary.txt
{
echo "## API diff \`${{ matrix.package }}\`: \`$(git rev-parse --short "$base")\` -> \`$(git rev-parse --short "$head")\`"
echo '```'
cat api-diff-summary.txt
echo '```'
} >> "$GITHUB_STEP_SUMMARY"

# Diff entries look like "[C] Module.Name"; the annotation legend
# ("[A] : Added") has a colon right after the marker and must not match.
if ! grep -qE '^\[[ARC]\] [^ :]' api-diff.txt; then
echo "No API changes detected in ${{ matrix.package }}."
exit 0
fi

# The API changed. This is acceptable when the PR documents the
# change in a changelog entry (see the PR template checklist):
# the job passes, but keeps showing the diff in its summary.
changed_changelogs="$(git diff --name-only --diff-filter=AM "$base" "$head" -- changelog.d/)"
if [ -z "$changed_changelogs" ]; then
echo "::error::The API of ${{ matrix.package }} changed with respect to the base revision, and this PR adds no changelog entry. See the job summary for the diff. Add a changelog file under changelog.d/ describing the change (or revert the API change)."
exit 1
fi
python3 .github/scripts/api-changelog-check.py "${{ matrix.package }}" api-diff.txt $changed_changelogs
21 changes: 21 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,27 @@ fail annoyingly once you push it. `make checks` will do these checks. The list o
checks is expected to grow over time, to make it easier to avoid CI turnaround on
simple problems.

## API diff

CI runs an informational "Check API" job (`.github/workflows/check-api.yml`) on
every pull request. It computes the public API diff of `Cabal-syntax`, `Cabal`,
`cabal-install`, `cabal-install-solver` and `Cabal-hooks` between the base revision and the head
of the PR using [packdiff](https://github.com/composewell/packdiff), and prints
it in the job log and the job summary.

> [!NOTE]
> `packdiff` literally runs `git checkout` on both revisions.
>
> `packdiff` derives its output from the haddock hoogle files, so it does not cover `other-modules`.

To run the same diff locally:

```console
$ make api-install # once; installs packdiff from the pinned commit
$ make api-diff # all library packages, against origin/master
$ make api-diff PKG=Cabal-syntax API_BASE=3.14 # one package, against a branch/tag
```

## QA Notes

Manual Quality Assurance (QA) is performed to ensure that the changes impacting
Expand Down
36 changes: 36 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,42 @@ cabal-install-test-accept:
rm -rf .ghc.environment.*
cd cabal-testsuite && `cabal list-bin cabal-tests` --with-cabal=`cabal list-bin cabal` --hide-successes -j3 --accept ${TEST}

# API diff (packdiff)
##############################################################################

# https://github.com/composewell/packdiff; the same commit is pinned in
# .github/workflows/check-api.yml and used by the "Check API" CI job.
# packdiff is not on Hackage yet, so it is installed from the pinned commit.
PACKDIFF_COMMIT := 54e786de55f091cdd3b912bd72ccc0e5e252aa77
PACKDIFF_URL := https://github.com/composewell/packdiff/archive/$(PACKDIFF_COMMIT).tar.gz

API_PACKAGES := Cabal-syntax Cabal cabal-install cabal-install-solver Cabal-hooks
API_BASE ?= origin/master

.PHONY: api-install
api-install: ## Install the packdiff tool used for API diffing.
rm -rf "$${TMPDIR:-/tmp}/packdiff-install"
mkdir -p "$${TMPDIR:-/tmp}/packdiff-install"
curl -sSL $(PACKDIFF_URL) | tar -xz -C "$${TMPDIR:-/tmp}/packdiff-install"
cd "$${TMPDIR:-/tmp}/packdiff-install/packdiff-$(PACKDIFF_COMMIT)" && \
cabal install exe:packdiff --installdir=$(HOME)/.local/bin --overwrite-policy=always

.PHONY: api-diff
api-diff: ## API diff of PKG (default: all library packages) between API_BASE (default: origin/master) and HEAD.
@command -v packdiff >/dev/null || { echo "packdiff not found; run 'make api-install'"; exit 1; }
@# NB: packdiff literally runs 'git checkout' on the given revisions, so:
@# * revisions are resolved to SHAs *before* running (a literal "HEAD"
@# would be re-resolved after switching to the base revision);
@# * the worktree must be clean (commit or stash first);
@# * the base revision must be buildable with the local GHC;
@# * on success the worktree is left at HEAD, on failure at the base.
@base=$$(git rev-parse $(API_BASE)); \
head=$$(git rev-parse HEAD); \
for pkg in $(if $(PKG),$(PKG),$(API_PACKAGES)); do \
echo "== packdiff $$pkg: $(API_BASE) -> HEAD =="; \
packdiff diff $$pkg $$base $$pkg $$head; \
done

# Docker validation

# Use this carefully, on big machine you can say
Expand Down
Loading