diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a73406e6..95328c8d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -459,8 +459,14 @@ jobs: - name: Configure run: cmake --preset iwyu - - name: Run include-what-you-use + # IWYU is advisory: the two-pass "should add / should remove" output is + # hard to interpret correctly under time pressure, and the per-PR + # friction it causes outweighs the steady-state value (S24.13 / E24). + # Findings still surface via the uploaded artifact below; release prep + # is responsible for clearing them. + - name: Run include-what-you-use (advisory) shell: bash + continue-on-error: true run: set -o pipefail && cmake --build --preset iwyu --target iwyu 2>&1 | tee build/iwyu/iwyu-output.txt - name: Upload iwyu output @@ -526,8 +532,10 @@ jobs: - name: Configure run: cmake --preset iwyu -DCMAKE_C_COMPILER=clang-19 -DCMAKE_CXX_COMPILER=clang++-19 - - name: Run include-what-you-use + # Advisory per S24.13 — see analyze-iwyu above for rationale. + - name: Run include-what-you-use (advisory) shell: bash + continue-on-error: true run: set -o pipefail && cmake --build --preset iwyu --target iwyu 2>&1 | tee build/iwyu/iwyu-output.txt - name: Upload iwyu output diff --git a/DEVLOG.md b/DEVLOG.md index eeb372a2..8b8071b8 100644 --- a/DEVLOG.md +++ b/DEVLOG.md @@ -1,5 +1,71 @@ # Dev Log +## 2026-05-28 — S24.13 IWYU lanes switched to advisory + +Pivot story under E24. Started out scoped as "replace IWYU with +clang-tidy's `misc-include-cleaner`, diff-scoped" — followed the +proven `analyze-tidy*` lane pattern S24.11 established (clang-tidy in +the cpputest-freertos image) so the swap would have been a +configuration-only change with no separate lane. Halfway through the +finding survey, misc-include-cleaner turned out to have substantial +stdlib-mapping gaps on POSIX extensions: `CLOCK_REALTIME` not mapped +to ``, `SOL_SOCKET` / `SO_KEEPALIVE` not mapped to +``, `timeval` not mapped to ``. About 75 % +of the findings on `Platform/Posix/` were false positives the +two-knob check (`IgnoreHeaders` + `DeduplicateFindings`) couldn't +filter cleanly. Confirmed with a minimal repro: no combination of +`` + `` satisfies the check for `CLOCK_REALTIME`. + +The original motivation for the swap was IWYU's per-PR friction +(two-pass output, container drift, line-shift cascades). The simpler +move that actually buys us throughput is to keep IWYU and stop it +blocking PRs. Two `continue-on-error: true` additions on the IWYU +`Run` steps in `.github/workflows/ci.yml`. cppcheck stays required — +S24.12 just audited its suppressions and the FP rate is low. + +`docs/local-checks.md` updated: Tier B drops the IWYU-pre-push +requirement and `clang-format -i` over touched files is now Tier B's +sole expensive step. Release prep picks up the IWYU artifact sweep. + +Reverted the partial misc-include-cleaner work I'd already started +(eight Core/Source files with newly-added `` and +`SolidSyslogPrival.h` includes plus a `Tests/.clang-tidy` +`IgnoreHeaders` entry for CppUTest) — those were legitimate +improvements but stopped being in scope once the story shape changed. +The misc-include-cleaner investigation, plus a note on the standalone +`clang-include-cleaner` tool (which does support IWYU-style mapping +files and may close the POSIX gap), goes back into E24's candidate +ideas to revisit if we want to retry the swap later. + +### What stays + +- `analyze-iwyu` / `analyze-iwyu-freertos-plustcp` lanes still run on + every PR. +- Both lanes still upload `iwyu-report` / `iwyu-report-freertos-plustcp` + artifacts. +- The `iwyu` CMake preset and the `ENABLE_IWYU` plumbing stay — the + local-run recipe in `docs/local-checks.md` still works. +- Branch protection list unchanged. Step-level `continue-on-error` + makes the job report success on findings, so the required-check + contract is still satisfied without a UI edit. + +### What changed + +- The IWYU step in both lanes is renamed `Run include-what-you-use (advisory)` + and gets `continue-on-error: true`. +- `docs/local-checks.md` Tier B narrative drops IWYU as a required + step and gains an "(optional, advisory)" subsection. + +### Open follow-ups + +- Future story under E24 — investigate `clang-include-cleaner` + standalone (mapping-file support) as a replacement candidate. Land + it via a feature-branch experiment that runs side-by-side with + advisory IWYU for one PR cycle, only promote if FP rate clears + ~95 %. +- Release-prep checklist gains "sweep the IWYU artifact" — will land + alongside whatever release-prep doc owns checklists. + ## 2026-05-28 — S28.05 SolidSyslogLwipRawTcpStream Fifth story in E28 (#465, parent #439). TCP stream adapter for the diff --git a/docs/local-checks.md b/docs/local-checks.md index b3fcc11f..5be16c86 100644 --- a/docs/local-checks.md +++ b/docs/local-checks.md @@ -10,9 +10,14 @@ when, so the wait stays under a few minutes and CI catches the rest. | Tier | When | What | Wall-clock | |---|---|---|---| | **A** — fast feedback | Every commit on the branch | `cmake --build --preset debug --target junit` for whatever preset matches the diff (gcc / clang / freertos-host) | ~30–60 s | -| **B** — pre-push | First push to the branch and any push that changes production source | A + `iwyu` + format reflowed includes + `misra_renumber.py` | ~3–5 min | +| **B** — pre-push | First push to the branch and any push that changes production source | A + format reflowed includes + `misra_renumber.py` | ~2–3 min | | **C** — none | — | — | — | -| **CI** — everything else | After push | `tidy`, `sanitize`, `coverage`, Windows, BDD, integration, FreeRTOS host/cross, IWYU on cpputest-clang, MISRA on cpputest | runs in parallel; results in ~10–15 min | +| **CI** — everything else | After push | `tidy`, `sanitize`, `coverage`, Windows, BDD, integration, FreeRTOS host/cross, advisory IWYU, MISRA on cpputest | runs in parallel; results in ~10–15 min | + +**IWYU is advisory** (S24.13 / E24). The lanes still run on every PR and +the report is uploaded as an artifact, but findings no longer fail the +build. Sweep the IWYU artifact when you do a release cleanup; do not +treat it as a per-PR blocker. Format-on-save in the editor handles formatting per-edit, so no separate `analyze-format` step locally. If you skip an editor with format-on-save, @@ -20,32 +25,41 @@ add a `clang-format -i` sweep over touched files to Tier A. ## Path-gating Tier B -Tier B does the expensive bits (IWYU, MISRA), so scope it to what changed: +Tier B does MISRA-line-drift cleanup, so scope it to what changed: - **Touched only `Tests/`, `Bdd/Targets/`, `docs/`, `cmake/`, or `*.md`** — skip Tier B entirely. Push and let CI run. - **Touched any `Core/Source/`, `Platform/*/Source/`, or public-header file** - — run Tier B against the container that compiles that tree (see below). + — run `clang-format -i` over touched files and + `scripts/misra_renumber.py --apply` to update the suppressions. ## Running Tier B -The IWYU lane runs against clang's parser. cpputest-freertos and cpputest -both ship the same IWYU binary now (clang_19 branch), so pick the container -whose env vars include the trees you touched: +### MISRA — fix line-number drift + +When edits shift production lines, `misra_suppressions.txt` entries go +stale. Fix in one step: + +```bash +# In any container that has cppcheck (all of them do): +scripts/misra_renumber.py # show proposed renumbers +scripts/misra_renumber.py --apply # write back updated suppressions +``` + +The script bails on genuine new findings (mismatched counts per +rule+file) — those need manual review. See the script's docstring. + +### IWYU (optional, advisory) -### Touched only Core / Posix / Windows / OpenSSL trees +If you want a local look before push, the lane is still wired: ```bash -# Inside the gcc devcontainer (Ctrl+Shift+B-able), or: docker compose -f .devcontainer/docker-compose.yml run --rm clang \ bash -c 'cmake --preset iwyu && cmake --build --preset iwyu --target iwyu' ``` -Read the full IWYU output — head/tail truncation hides findings. After -fixing IWYU's complaints, format the file (`clang-format -i path`) — the -include reorder can put forward-decls on lines that need reflowing. - -### Touched any FreeRTOS / Plus-TCP / lwIP / MbedTLS / FatFs tree +For FreeRTOS / Plus-TCP / lwIP / MbedTLS / FatFs trees, use `freertos-host` +with the clang-19 overrides instead: ```bash docker compose -f .devcontainer/docker-compose.yml run --rm freertos-host \ @@ -54,23 +68,8 @@ docker compose -f .devcontainer/docker-compose.yml run --rm freertos-host \ && cmake --build --preset iwyu --target iwyu' ``` -The `-DCMAKE_C_COMPILER=clang-19` overrides match the -`analyze-iwyu-freertos-plustcp` CI lane — the gcc base image carries -`clang-19` but no `clang` alternative. - -### MISRA — after IWYU, fix line-number drift - -When IWYU's include reorder shifts production lines, `misra_suppressions.txt` -entries go stale. Fix in one step: - -```bash -# In any container that has cppcheck (all of them do): -scripts/misra_renumber.py # show proposed renumbers -scripts/misra_renumber.py --apply # write back updated suppressions -``` - -The script bails on genuine new findings (mismatched counts per -rule+file) — those need manual review. See the script's docstring. +CI runs both lanes advisory — findings appear in the `iwyu-report` and +`iwyu-report-freertos-plustcp` artifacts and don't block the build. ## What CI runs and you should not run locally @@ -78,8 +77,6 @@ rule+file) — those need manual review. See the script's docstring. - Windows MSVC + BDD + integration — depend on tools you may not have - BDD-linux-syslog-ng, BDD-windows-otel, BDD-freertos-qemu — heavy multi-container stacks -- The cpputest-clang IWYU lane and the cpputest-freertos IWYU/tidy lanes — - CI runs both; you only need one locally for the trees you touched If CI surfaces a finding you missed locally, fix in another commit on the same branch — cheaper than running every CI lane on every push.