Skip to content

Commit 38c9c88

Browse files
authored
CI improvements: caching, gating, and robustness (#60)
## Summary Improvements to the CI and publish workflows. No library code is changed. ## Changes **Speed & hygiene** (`ci.yml`) - Add ccache to the host `build`/`test`/`conformance` jobs, which otherwise recompile all ~260 libopus `.c` files from scratch every run. Uses apt-installed ccache + the existing pinned `actions/cache`; no new actions. - `concurrency` so new commits supersede stale runs. Labeling a PR no longer cancels a running suite, and only the `ci-run-qemu` label triggers a QEMU run. - `timeout-minutes` on every job (a hung build or QEMU run no longer sits until the 6-hour default). - Add `workflow_dispatch` to run CI manually from the Actions tab. **Publish safety** (`publish.yml`) - `verify-version` job: fail if `library.json` / `idf_component.yml` disagree with the release tag before anything ships. - `ci-gate` job: block publish unless the `CI success` aggregate passed on the release commit. **Lint reproducibility** - Pin the lint job to `clang-tidy-18` (matching the pinned clang-format). `script/clang-tidy.sh` honors a preset `$CLANG_TIDY` so the runner's preinstalled binary can't shadow it, and errors early if that binary is missing. **Branch protection** - Add a `ci-success` aggregating job so branch protection can require one check instead of enumerating every job. Skipped jobs count as passing.
1 parent 329bdb0 commit 38c9c88

3 files changed

Lines changed: 160 additions & 15 deletions

File tree

.github/workflows/ci.yml

Lines changed: 99 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,18 +9,30 @@ on:
99
# `labeled` lets the `ci-run-qemu` label start a run. Every job except changes/cross-qemu is
1010
# gated on `github.event.action != 'labeled'`, so labeling re-runs only QEMU, not the whole suite.
1111
types: [opened, synchronize, reopened, labeled]
12+
# Manual "Run workflow" from the Actions tab. The changes job's fail-safe (unreachable base) runs
13+
# the full suite including QEMU on a dispatch, which is what you want from a manual trigger.
14+
workflow_dispatch:
1215

1316
# Least privilege: every job here only reads the repo (checkout, build, test, cache). Jobs that
1417
# need more should add a narrower job-level permissions block rather than widening this default.
1518
permissions:
1619
contents: read
1720

21+
concurrency:
22+
group: ${{ github.workflow }}-${{ github.ref }}
23+
# Don't cancel an in-progress run when a label is added. The `labeled` trigger fires for every
24+
# label (it exists so `ci-run-qemu` can force a QEMU run), and a labeled event shares this group,
25+
# so cancelling here would kill a running suite just because the PR was labeled. New commits
26+
# (push/synchronize) still supersede a run.
27+
cancel-in-progress: ${{ github.event.action != 'labeled' }}
28+
1829
jobs:
1930
pre-commit:
2031
name: Pre-commit checks
2132
# Skip on label-only events so labeling runs only the QEMU job (see pull_request types).
2233
if: github.event.action != 'labeled'
2334
runs-on: ubuntu-latest
35+
timeout-minutes: 10
2436
steps:
2537
- uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
2638

@@ -34,6 +46,7 @@ jobs:
3446
name: Static analysis
3547
if: github.event.action != 'labeled'
3648
runs-on: ubuntu-latest
49+
timeout-minutes: 15
3750
steps:
3851
- uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
3952
with:
@@ -42,22 +55,41 @@ jobs:
4255
- name: Install clang-tidy
4356
run: |
4457
sudo apt-get update
45-
sudo apt-get install -y clang-tidy
58+
sudo apt-get install -y clang-tidy-18
4659
4760
- name: Run clang-tidy
61+
env:
62+
CLANG_TIDY: clang-tidy-18
4863
run: ./script/clang-tidy.sh
4964

5065
build:
5166
name: Build
5267
if: github.event.action != 'labeled'
5368
runs-on: ubuntu-latest
69+
timeout-minutes: 15
70+
# libopus is ~260 .c files rebuilt from scratch in every host job; ccache reuses objects across runs.
71+
env:
72+
CCACHE_DIR: ${{ github.workspace }}/.ccache
5473
steps:
5574
- uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
5675
with:
5776
submodules: recursive
5877

78+
- name: Install ccache
79+
run: sudo apt-get update && sudo apt-get install -y ccache
80+
81+
- name: Cache ccache
82+
uses: actions/cache@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5.0.5
83+
with:
84+
path: ${{ github.workspace }}/.ccache
85+
key: ccache-build-${{ github.sha }}
86+
restore-keys: ccache-build-
87+
5988
- name: Configure CMake
60-
run: cmake -B build host_examples/opus_to_wav
89+
run: >
90+
cmake -B build
91+
-DCMAKE_C_COMPILER_LAUNCHER=ccache -DCMAKE_CXX_COMPILER_LAUNCHER=ccache
92+
host_examples/opus_to_wav
6193
6294
- name: Build
6395
run: cmake --build build
@@ -66,6 +98,9 @@ jobs:
6698
name: Unit tests (${{ matrix.alloc_mode }})
6799
if: github.event.action != 'labeled'
68100
runs-on: ubuntu-latest
101+
timeout-minutes: 20
102+
env:
103+
CCACHE_DIR: ${{ github.workspace }}/.ccache
69104
strategy:
70105
fail-fast: false
71106
matrix:
@@ -77,9 +112,20 @@ jobs:
77112
with:
78113
submodules: recursive
79114

115+
- name: Install ccache
116+
run: sudo apt-get update && sudo apt-get install -y ccache
117+
118+
- name: Cache ccache
119+
uses: actions/cache@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5.0.5
120+
with:
121+
path: ${{ github.workspace }}/.ccache
122+
key: ccache-test-${{ matrix.alloc_mode }}-${{ github.sha }}
123+
restore-keys: ccache-test-${{ matrix.alloc_mode }}-
124+
80125
- name: Configure CMake with sanitizers
81126
run: >
82127
cmake -B build -DENABLE_SANITIZERS=ON
128+
-DCMAKE_C_COMPILER_LAUNCHER=ccache -DCMAKE_CXX_COMPILER_LAUNCHER=ccache
83129
-DOPUS_ALLOCATION_MODE=${{ matrix.alloc_mode }} tests
84130
85131
- name: Build
@@ -92,11 +138,24 @@ jobs:
92138
name: opus_compare conformance
93139
if: github.event.action != 'labeled'
94140
runs-on: ubuntu-latest
141+
timeout-minutes: 30
142+
env:
143+
CCACHE_DIR: ${{ github.workspace }}/.ccache
95144
steps:
96145
- uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
97146
with:
98147
submodules: recursive
99148

149+
- name: Install ccache
150+
run: sudo apt-get update && sudo apt-get install -y ccache
151+
152+
- name: Cache ccache
153+
uses: actions/cache@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5.0.5
154+
with:
155+
path: ${{ github.workspace }}/.ccache
156+
key: ccache-conformance-${{ github.sha }}
157+
restore-keys: ccache-conformance-
158+
100159
# The RFC 8251 test vectors are static; cache them so we don't re-download every run. Keying on
101160
# the fetch script means the cache auto-invalidates if the vector URL ever changes.
102161
- name: Cache test vectors
@@ -111,7 +170,10 @@ jobs:
111170
run: tests/fetch_vectors.sh
112171

113172
- name: Configure CMake
114-
run: cmake -B build tests
173+
run: >
174+
cmake -B build
175+
-DCMAKE_C_COMPILER_LAUNCHER=ccache -DCMAKE_CXX_COMPILER_LAUNCHER=ccache
176+
tests
115177
116178
- name: Build
117179
run: cmake --build build
@@ -128,6 +190,7 @@ jobs:
128190
changes:
129191
name: Detect on-target changes
130192
runs-on: ubuntu-latest
193+
timeout-minutes: 10
131194
outputs:
132195
target: ${{ steps.filter.outputs.target }}
133196
steps:
@@ -162,11 +225,13 @@ jobs:
162225
cross-qemu:
163226
name: Cross (Xtensa QEMU conformance, ${{ matrix.variant }})
164227
needs: changes
165-
# Skip on wrapper-only changes. Add the `ci-run-qemu` label to force a run.
228+
# Skip on wrapper-only changes. Add the `ci-run-qemu` label to force a run. On a `labeled` event
229+
# only that label triggers QEMU, so adding an unrelated label doesn't kick off a redundant run.
166230
if: >
167-
needs.changes.outputs.target == 'true' ||
231+
(github.event.action != 'labeled' && needs.changes.outputs.target == 'true') ||
168232
contains(github.event.pull_request.labels.*.name, 'ci-run-qemu')
169233
runs-on: ubuntu-latest
234+
timeout-minutes: 40
170235
# esp-idf version matches the PlatformIO framework (esp-idf v5.4.x) so the
171236
# xtensa-gcc codegen is the same as the local run. Xtensa emulation is
172237
# native-speed on the amd64 runner.
@@ -240,6 +305,7 @@ jobs:
240305
name: PlatformIO Build
241306
if: github.event.action != 'labeled'
242307
runs-on: ubuntu-latest
308+
timeout-minutes: 30
243309
strategy:
244310
matrix:
245311
example: [decode_benchmark, encode_benchmark]
@@ -267,3 +333,31 @@ jobs:
267333

268334
- name: Build ${{ matrix.example }}
269335
run: pio run -d examples/${{ matrix.example }}
336+
337+
# Single aggregating status for branch protection: require only "CI success" instead of every job.
338+
# always() so it reports even when a job fails or the conditional cross-qemu job is skipped; skipped
339+
# dependencies are treated as passing (only failure/cancelled blocks).
340+
ci-success:
341+
name: CI success
342+
if: always()
343+
needs:
344+
- pre-commit
345+
- lint
346+
- build
347+
- test
348+
- conformance
349+
- changes
350+
- cross-qemu
351+
- platformio
352+
runs-on: ubuntu-latest
353+
timeout-minutes: 5
354+
steps:
355+
- name: Fail if any required job failed or was cancelled
356+
if: ${{ contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled') }}
357+
run: |
358+
echo "One or more CI jobs failed or were cancelled:"
359+
echo '${{ toJSON(needs) }}'
360+
exit 1
361+
362+
- name: Report success
363+
run: echo "All required CI jobs passed (skipped jobs are allowed)."

.github/workflows/publish.yml

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,54 @@ permissions:
1010
contents: read
1111

1212
jobs:
13+
verify-version:
14+
name: Verify version matches tag
15+
runs-on: ubuntu-latest
16+
steps:
17+
- uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
18+
- name: Check library.json and idf_component.yml against the release tag
19+
env:
20+
TAG: ${{ github.event.release.tag_name }}
21+
run: |
22+
expected="${TAG#v}"
23+
lib="$(jq -r '.version' library.json)"
24+
idf="$(grep -m1 '^version:' idf_component.yml | sed -e 's/^version:[[:space:]]*//' -e 's/"//g' -e 's/[[:space:]]*$//')"
25+
echo "tag=$TAG expected=$expected library.json=$lib idf_component.yml=$idf"
26+
fail=0
27+
[ "$lib" = "$expected" ] || { echo "::error::library.json version ($lib) != release tag ($expected)"; fail=1; }
28+
[ "$idf" = "$expected" ] || { echo "::error::idf_component.yml version ($idf) != release tag ($expected)"; fail=1; }
29+
exit $fail
30+
31+
ci-gate:
32+
name: CI gate
33+
runs-on: ubuntu-latest
34+
permissions:
35+
checks: read
36+
steps:
37+
- name: Verify CI passed on the release commit
38+
env:
39+
GH_TOKEN: ${{ github.token }}
40+
run: |
41+
# Block the release unless the aggregate "CI success" check passed on this commit. That job
42+
# (ci-success in ci.yml) needs every other job and fails if any failed or was cancelled,
43+
# with skipped jobs (e.g. the conditional QEMU run) treated as passing, so gating on it
44+
# alone covers the whole suite and tracks future job changes. The release tag name is the
45+
# check-runs ref so the API resolves it to the underlying commit (github.sha can be an
46+
# annotated tag's object SHA, which carries no check runs). Take the most recently completed
47+
# "CI success" run in case the commit was built more than once.
48+
conclusion="$(gh api "repos/${{ github.repository }}/commits/${{ github.event.release.tag_name }}/check-runs" \
49+
--paginate --jq '.check_runs[] | select(.name == "CI success" and .status == "completed") | "\(.completed_at) \(.conclusion)"' \
50+
| sort | tail -1 | awk '{print $NF}')"
51+
echo "Latest completed 'CI success' conclusion: ${conclusion:-<none found>}"
52+
if [ "$conclusion" != "success" ]; then
53+
echo "::error::CI success did not pass on the release commit (got '${conclusion:-not found}'). Refusing to publish."
54+
exit 1
55+
fi
56+
echo "CI success passed on the release commit; proceeding to publish."
57+
1358
publish-platformio:
1459
name: Publish to PlatformIO
60+
needs: [verify-version, ci-gate]
1561
runs-on: ubuntu-latest
1662
environment: platformio
1763
steps:
@@ -31,6 +77,7 @@ jobs:
3177

3278
publish-espressif:
3379
name: Publish to Espressif
80+
needs: [verify-version, ci-gate]
3481
runs-on: ubuntu-latest
3582
environment: espressif
3683
permissions:

script/clang-tidy.sh

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,16 @@ SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)"
1111
ROOT_DIR="$(dirname "$SCRIPT_DIR")"
1212
BUILD_DIR="${ROOT_DIR}/host_examples/opus_to_wav/build"
1313

14-
# Find clang-tidy
15-
CLANG_TIDY=""
16-
for name in clang-tidy clang-tidy-18 clang-tidy-17 clang-tidy-16 clang-tidy-15; do
17-
if command -v "$name" &> /dev/null; then
18-
CLANG_TIDY="$name"
19-
break
20-
fi
21-
done
14+
# Find clang-tidy. A pre-set $CLANG_TIDY (CI pins it to clang-tidy-18) wins over PATH discovery.
15+
CLANG_TIDY="${CLANG_TIDY:-}"
16+
if [ -z "$CLANG_TIDY" ]; then
17+
for name in clang-tidy clang-tidy-18 clang-tidy-17 clang-tidy-16 clang-tidy-15; do
18+
if command -v "$name" &> /dev/null; then
19+
CLANG_TIDY="$name"
20+
break
21+
fi
22+
done
23+
fi
2224

2325
# Check Homebrew LLVM paths on macOS
2426
if [ -z "$CLANG_TIDY" ]; then
@@ -30,8 +32,10 @@ if [ -z "$CLANG_TIDY" ]; then
3032
done
3133
fi
3234

33-
if [ -z "$CLANG_TIDY" ]; then
34-
echo "Error: clang-tidy not found"
35+
# Validate the resolved binary up front: catches both an empty result and a bogus pre-set
36+
# $CLANG_TIDY, instead of failing later with a bare "command not found".
37+
if ! command -v "$CLANG_TIDY" &> /dev/null; then
38+
echo "Error: clang-tidy not found or not executable: '${CLANG_TIDY:-unset}'"
3539
exit 1
3640
fi
3741

0 commit comments

Comments
 (0)