From f96ddbaa1e11a98f8076df48ba73026191c28399 Mon Sep 17 00:00:00 2001 From: hathach Date: Fri, 21 Aug 2026 11:09:03 +0700 Subject: docs: record the CI selection design and its plan The binding rule table (17 rows x 3 answer columns), the measured effect per PR shape, and the reasoning behind the parts that look surprising: why empty means empty, why hw/mcu and lib are rules rather than full-matrix paths, why get_deps.py is diffed as data, and which build system is the reference. The plan is the task-by-task record of how it was built, kept as the origin trail. --- .../2026-08-19-ci-build-family-filter-design.md | 501 +++++++++++++++++++++ 1 file changed, 501 insertions(+) create mode 100644 docs/superpowers/specs/2026-08-19-ci-build-family-filter-design.md (limited to 'docs/superpowers/specs') diff --git a/docs/superpowers/specs/2026-08-19-ci-build-family-filter-design.md b/docs/superpowers/specs/2026-08-19-ci-build-family-filter-design.md new file mode 100644 index 000000000..9fa358bee --- /dev/null +++ b/docs/superpowers/specs/2026-08-19-ci-build-family-filter-design.md @@ -0,0 +1,501 @@ +# PR-scoped CI selection: promoting hil_select to tools/ci_select.py + +**Date:** 2026-08-19 +**Branch:** `build-filter` + +## Motivation + +Every PR builds every example on one board per family, on both CI providers: **74 legs / +2494 example-builds** on the GitHub Actions `cmake` job, and 129 family-legs per build system +on CircleCI (which runs cmake *and* make, plus clang/IAR). Most PRs touch one port, one class, +or one example, and a `hid_host.c` change cannot break an MSC device example on msp430. + +`hil-build` is worse in a different way: it builds **1702 example-builds** (37 board-builds × +46 examples, `--target all`) to run a test suite that needs at most **515**. The HIL example +universe is only 21 of the 46 examples in tree, and the median board needs 15 of them. + +`test/hil/helper/hil_select.py` already maps a PR diff to affected boards and per-board test +lists for HIL, and already owns both mappings the build matrix needs: port-to-family, and +class-macro-to-example +(`docs/superpowers/specs/2026-07-29-hil-pr-scoped-selection-design.md`). That design listed +"scoping the non-HIL build jobs" as an explicit non-goal; this is that follow-up. + +## Goal / non-goals + +**Goal:** promote the selector to a repo-wide `tools/ci_select.py` whose single classification +of a diff drives **all three** CI axes from one rule table — which families to build, which +example targets to build on each, and which rig boards run which tests — wired into +`ci_set_matrix.py` and `hil_ci_set_matrix.py` so both providers and the rig filter from one +source. Scoping applies to `pull_request` events only; push, release and `workflow_dispatch` +keep the full matrix. + +**Non-goals:** +- Variant-level or board-level selection below one-board-per-family on the build axis (all + variants of a selected HIL board still build and run). +- Changing `hil_test.py` behaviour. The selector only *composes* existing `-b` / `-bt` args. +- Changing which tests HIL decides to run. The HIL board/test decision is preserved except for + the single rule-7 change called out below. + +## The rule table + +One classification, three outputs. Every rule yields build families, build examples, and HIL +boards/tests. Pairs are unioned **per family** (build) and **per board** (HIL), so a mixed diff +never inflates one axis with another's breadth. + +`DEV` = 33 `examples/device/*`, `HOST` = 9, `DUAL` = 3, `TYPEC` = 1, `ALL` = 46. +`FAM` = the families whose `family.cmake` references the changed path (CMake only — see below). +"roster boards" = boards on `test/hil/{tinyusb,hfp}.json`. + +| # | Changed path | Build families | Build examples | HIL boards → tests | +| --- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------- | ----------------------------------------------------------------- | ---------------------------------------------------------------------------------------------- | +| 1 | `docs/`, `.claude/`, `*.md`, `*.rst`, `LICENSE` | — | — | — | +| 2 | `test/hil/**` | — | — | all boards → all tests | +| 2b | `tools/metrics.py`, `.github/scripts/metrics_*.py` | `ALL` (unchanged — `tinyusb_metrics` runs `metrics.py` as a build target) | `ALL` | — (nothing on the rig runs it) | +| 3 | `src/portable//dcd_*`, `*_device.[ch]` | `FAM` | `DEV`+`DUAL` | `FAM`'s device-role boards → device+dual tests | +| 4 | `src/portable//hcd_*`, `*_host.[ch]` | `FAM` | `HOST`+`DUAL` | `FAM`'s host-role boards → host+dual tests | +| 5 | `src/portable//**` (anything else) | `FAM` | `ALL` | `FAM`'s boards → all their tests | +| 5b | `src/portable//**` where `FAM` is empty | — | — | — (empty resolves to nothing on BOTH axes) | +| 6 | `hw/bsp//**` | that family | `ALL` | that family's boards → all tests (a `boards//` path narrows to that board) | +| 7 | `hw/mcu//**` | `FAM` — empty resolves to nothing (maintainer ruling) | `ALL` | `FAM`'s boards → all tests; empty resolves to nothing (maintainer ruling) ⚠ *see below* | +| 8 | `src/class//*_device.[ch]` | `ALL` | examples enabling `CFG_TUD_` | device-role boards → HIL tests enabling `CFG_TUD_` | +| 9 | `src/class//*_host.[ch]` | `ALL` | examples enabling `CFG_TUH_` | host-role boards → HIL tests enabling `CFG_TUH_` | +| 10 | `src/class//**` (shared header) | `ALL` | either, **plus include-edge classes** | both roles → same, plus include-edge classes | +| 11 | `src/device/**` | `ALL` | `DEV`+`DUAL` | device-role boards → device+dual tests | +| 12 | `src/host/**` | `ALL` | `HOST`+`DUAL` | host-role boards → host+dual tests | +| 13 | `examples///**` | `ALL` | just `` | if `` is a HIL test: all boards → that test; else nothing | +| 14 | `examples/device/board_test/**` | `ALL` | just `board_test` | all boards → all tests (HIL parking firmware) | +| 15 | `examples/build_system/**`, `examples/CMakeLists.txt`, `examples//CMakeLists.txt` | `ALL` | `ALL` | all boards → all tests | +| 16 | `src/common/`, `src/osal/`, `src/tusb.[ch]`, `src/tusb_option.h`, `tools/build*.py`, `tools/cmake/**`, `hw/bsp/{family_support.cmake,board.c,board_api.h,ansi_escape.h}`, `.github/**` | `ALL` | `ALL` | all boards → all tests | +| 16a | `lib//**` | `ALL` | examples whose own `CMakeLists.txt`/`Makefile` names `lib/` | those examples that are HIL tests, on all boards; empty resolves to nothing | +| 16b | `tools/get_deps.py` | families whose `deps_mandatory`/`deps_optional` entries changed | `ALL` | those families' boards → all tests; a logic change, an `'all'` entry, no base content or a changed token naming no family → full | +| 17 | anything unclassified | `ALL` | `ALL` | all boards → all tests (fail-open) | + +**Rule 2 is deliberately asymmetric.** A `test/hil/**` change is invisible to the family matrix +but is exactly what the rig exercises, so it builds nothing and runs everything. + +**Rule 7 is the one HIL-side behaviour change in this design.** Today `hw/mcu/` sits in +`hil_select`'s `_FULL_RE` and forces the full HIL matrix. Since the build axis now resolves +those paths to a family through the same scan, forcing full on the rig is inconsistent. The +path fires rarely — 4 commits in 3 years — so this is low-risk either way; if you would rather +keep the HIL view untouched, rule 7's HIL column becomes "all boards → all tests" and nothing +else in this design changes. + +Rules 8–10 reuse machinery `hil_select` already has — `class_macros`, `_config_enables`, +`class_include_edges` — applied over all 46 examples' `src/tusb_config.h` for the build axis +and over the HIL test list for the HIL axis. The include edges are why an `audio.h` change also +selects the MIDI examples (`midi{,2}_{device,host}.h` include `class/audio/audio.h`) and a +`cdc.h` change the net one. + +### Buildability post-filter (build axis) + +After the pairs are unioned, every `(family, examples)` pair is pruned with +`build_utils.skip_example(example, )` — the same `skip.txt` / `only.txt` +data CMake's `family_filter` uses (40 `skip.txt`, 13 `only.txt` in tree). Examples the family +cannot build are dropped; a family left with none is dropped entirely. + +This is where most of the host-side saving comes from: only 23 of 75 CI families can build +`host/bare_api` at all, and 2 can build `typec/power_delivery`. + +### Measured effect + +GHA `cmake` job, baseline **74 legs / 2494 example-builds**; `hil-build`, baseline **1702 +example-builds** across 37 board-builds. + +| PR shape | Build legs | Build ex-builds | HIL boards | hil-build ex-builds | +| ---------------------------- | ---------: | --------------: | ---------: | ------------------: | +| `dcd_rp2040.c` | 1 | 35 | 2 | 32 | +| `hcd_max3421.c` | 1 | 10 | 7 | 36 | +| `hw/bsp/stm32f4/**` | 1 | 45 | 1 | 15 | +| `dcd_dwc2.c` | 20 | 646 | 10 | 184 | +| `hid_host.c` | 24 | 68 | — | — | +| `msc_host.c` | — | — | 9 | 40 | +| `usbh.c` | 25 | 217 | 10 | 57 | +| `msc_device.c` | 74 | 350 | — | — | +| `cdc_device.c` | 74 | 588 | 27 | 192 | +| `examples/device/cdc_msc/**` | 73 | 73 | 25 | 60 | +| `usbd.c` | 74 | 2297 | 27 | 472 | +| `src/common/**` (full) | 74 | 2494 | 30 | 515 | +| `test/hil/**` only | 0 | 0 | 30 | 515 | + +The full-matrix row is the headline for `hil-build`: even with **no** PR narrowing, per-board +example selection takes it from 1702 to 515. + +### Why "empty means empty" + +Both views answer an empty `FAM` the same way (rule 5b): nothing. The HIL view used to force +the full 30-board rig there, on the theory that an empty result might be a scan miss — but the +build view answered the identical condition with zero families for the same path, so the rig +ran every board to validate a file that nothing compiled. In the build view that theory costs +74 legs, and the +evidence does not support it: of the 28 `src/portable/*/*` directories, **26 resolve to at +least one family**. The two that do not are both real orphans as far as CI is concerned: +`microchip/pic` (only `dcd_pic.c` and a README, with no `hw/bsp/pic` family at all) and +`microchip/pic32mz` (`hw/bsp/pic32mz` has only a `family.mk`, and `pic32mz` is in neither +provider's family list, so no CI job builds it today). A file no CI job compiles cannot be +validated by building anything. + +The safety this gives up is recovered structurally: a unit test asserts every +`src/portable/*/*` and every tracked `hw/mcu/` resolves to ≥1 family, with an explicit +allowlist of known orphans (`microchip/pic`, `microchip/pic32mz`). Adding a port without +wiring a family then fails +pre-commit instead of silently building nothing on every later PR. Same enforcement style as +the existing `test_hil_util.BottomLayer` structural tests. + +Fail-open survives where it belongs: an *unclassified* path or any exception widens to `ALL` on +every axis. + +### Why `hw/mcu/**` is rule 7 and not "full" + +`hw/mcu` is overwhelmingly dependency territory — `tools/get_deps.py` has 87 entries under it, +and those paths are gitignored, so they can never appear in a diff. Only 51 files survive +in-tree, touched 4 times in 3 years, and they resolve through the same scan the ports use: + +| Tracked directory | In `get_deps`? | Resolves to | +| ----------------------------- | ------------------------------------------- | ----------- | +| `hw/mcu/dialog/` (`da1469x`) | **no** — real in-repo MCU support, 21 files | `da1469x` | +| `hw/mcu/nordic/` (`nrf5x`) | beside the `nrfx` dep | `nrf` | +| `hw/mcu/sony/` (`cxd56`) | beside the `spresense-exported-sdk` dep | `cxd56` | +| `hw/mcu/bridgetek/` (`ft9xx`) | beside the `ft90x-sdk` dep | `ft9xx` | + +Rule 7 is therefore not a mechanism of its own — it is rules 3–5's scan pointed at a second +tree, because `src/portable/` and `hw/mcu/` ask the same question. + +Unlike the port rule, an `hw/mcu` path that resolves to no family contributes *nothing* on +either axis (maintainer ruling): if no family's build references it, no build compiles it. The +table above is kept honest by `test_tracked_mcu_vendors_resolve`, which fails pre-commit if a +tracked vendor directory stops resolving. + +### Why `lib/**` is rule 16a and scanned per example + +Its tracked contents (`SEGGER_RTT`, `networking`, `rt-thread`, `embedded-cli`, 22 commits in +3 years) are wired in at `examples/build_system` and per-example `CMakeLists.txt`, not per +family — so the family scan the ports use is the wrong instrument here: it would *wrongly* +narrow `SEGGER_RTT` to the three families that name the path in their `family.cmake`, while +the path is not compiled by any of them by default (it is reached only through `LOGGER=rtt`). +That scan stays applied to `src/portable/` and `hw/mcu/` only. + +Rule 16a asks the per-example question instead (maintainer ruling: only the examples that use +the lib need building): `lib_examples()` reads each example's own `CMakeLists.txt` and +`Makefile` and keeps the ones naming `lib/` at a directory boundary. Every family stays +in play — any of them can build those examples — while the example list collapses: + +| Tracked lib | Examples that build it | HIL tests among them | +| -------------- | ----------------------------------------------------------- | -------------------- | +| `embedded-cli` | `host/msc_file_explorer`, `host/msc_file_explorer_freertos` | both | +| `networking` | `device/net_lwip_webserver` | none (test disabled) | +| `SEGGER_RTT` | — | — | +| `rt-thread` | — | — | + +`SEGGER_RTT` and `rt-thread` resolve to nothing, and "empty means empty" applies: no CI build +compiles them, so there is nothing to validate by building. + +### Why `tools/get_deps.py` is rule 16b + +`deps_mandatory` / `deps_optional` are data: `path -> [url, commit, 'fam1 fam2 ...']`. A commit +bump therefore affects exactly the families listed in that entry, and building the other 70+ is +pure waste. `get_deps_changed_families()` parses both sides of the file with `ast` (never +`exec` — this is PR content), diffs the two dict literals **separately**, and unions the family +tokens of every added, removed or edited entry, from **both** sides (a removed entry has only a +base side; an edited family list must cover the families that lose the dep as well as the ones +that gain it). Separately, because merging the dicts before diffing hides a *move* between +`deps_mandatory` and `deps_optional` — the value is untouched, but mandatory deps are fetched +for every family, so demoting one stops families fetching it. + +It falls open to the full matrix whenever the entries are not the whole answer: + +* anything outside the two dict assignments differs — a logic change to `get_deps` can change + what every family fetches (compared as `ast.dump(..., annotate_fields=False)` of the module + with those two assignments removed, so comments and reformatting alone are not a logic + change); +* an `'all'` entry (every mandatory dep) changed; +* the file will not parse; +* there is no base content: `--diff-file` mode has no git, so no merge-base blob; +* a changed entry carries a family token that names no `hw/bsp/` and is not one of the + eight known aliases. "Changed but unmappable" is not "nothing changed": reading it as the + latter empties the whole build matrix for a dep bump. + +The eight known aliases (`sam3x`, `samd21`, `samd51`, `same5x`, `fc100s`, `spresense`, +`stm32l1`, `stm32l5`) are pinned in `_DEPS_ALIAS_TOKENS` and select nothing. `get_deps` matches +a token against a requested family name verbatim (`f in entry[2].split()`), so these tokens +match nothing there either — four are pre-rename spellings listed beside the current name in +the same entry, two point at a differently-named family dir (`fc100s`→`f1c100s`, +`spresense`→`cxd56`, both unreachable in `get_deps` itself), and two name no family in the tree. +A ninth appearing fails `TestOrphanInvariant`. + +## Component: `tools/ci_select.py` + +`git mv test/hil/helper/hil_select.py tools/ci_select.py` (history preserved). The HIL +classifier is unchanged apart from rule 7; a second, independent build classifier is added +beside it. One diff read, two classifiers, one unit suite. + +``` +python3 tools/ci_select.py --base [--diff-file ] [CONFIG.json ...] +``` + +`configs` becomes `nargs='*'`. With rosters it emits everything it emits today plus the new +keys; with none it emits only the build view, so CircleCI never needs to know HIL exists. + +```json +{ + "full": false, + "boards": {"raspberry_pi_pico": "all"}, + "families": ["rp2040"], + "args": {"tinyusb.json": "-b raspberry_pi_pico"}, + "args_flasher": {"tinyusb.json": {"openocd": "-b raspberry_pi_pico"}}, + "hil_examples": {"raspberry_pi_pico": ["device/cdc_msc", "device/board_test"]}, + "build": { + "full": false, + "families": ["rp2040"], + "family_examples": { + "rp2040": ["device/cdc_msc", "device/hid_composite", "dual/dynamic_switch"] + } + }, + "reasons": ["src/portable/raspberrypi/rp2040/dcd_rp2040.c: port rp2040 -> ..."] +} +``` + +`build.families` is the build family axis. `build.family_examples` maps a family to its example +list; **a family absent from the map builds all its examples**, so the common "narrow families, +all examples" case carries no payload. `build.full` true means no build narrowing at all. + +`hil_examples` is the new HIL build axis: per roster board, the examples `hil-build` must +produce. It is `board_tests(board)` — which the selector already computes — **plus +`device/board_test`**, which `hil_test.py` flashes to park every board at each variant boundary +and at end-of-board teardown (`hil_test.py:1798`, `:1866`). It is emitted even when +`full: true`, because the HIL example universe is 21 of 46 examples regardless of any diff. + +All pre-existing keys keep their exact meaning, so `.github/scripts/hil_ci_set_matrix.py`, the +HIL legs in `build.yml` and `.claude/skills/pre-pr/SKILL.md` need only a path update. + +### Shared helper change + +`port_families(port_dir, repo_root)` generalizes to a path-to-families reference scan over a +second tree (`hw/mcu/`). Its existing **CMake-only** behaviour is kept unchanged and is now the +rule for every axis: it scans `hw/bsp/*/family.cmake` plus the espressif component +`CMakeLists.txt`, and never `family.mk`. + +CMake is the first-class build system; Make follows whatever CMake decides. A family that +CMake does not wire up to a port is not a consumer of that port, and the Make legs on CircleCI +build the same families CMake does. Scanning `family.mk` as well would only ever *widen* the +selection to families CMake never builds, which is coverage nobody asked for — and it would +resolve `microchip/pic32mz` to a family that appears in no CI family list. + +One consequence to keep in view: because HIL and build now share one scan, there is no +per-caller flag, no second cache key, and no way for the two axes to disagree about which +families own a port. + +**Boundary matching.** A directory reference must match at a directory boundary — a trailing +`/` *or* end-of-token — not as a bare substring. Both traps are live: `hw/bsp/nrf/family.cmake` +writes `${TOP}/hw/mcu/nordic/nrf5x` with no trailing slash, while the existing port scan +requires a trailing `/` precisely to stop `microchip/pic` matching `microchip/pic32mz`. + +### Move fallout + +All mechanical, all one-line: + +| File | Change | +| ---------------------------------- | ----------------------------------------------------------------- | +| `tools/ci_select.py` | `sys.path` walk 4 levels → 2 | +| `test/hil/hil_ci.sh` | drop from the scp list (nothing on the rig imports it) | +| `test/hil/test/test_hil_select.py` | rename to `test_ci_select.py`, import path | +| `test/hil/test/test_hil_util.py` | `BottomLayer` stdlib-closure allowlist + module list | +| `.pre-commit-config.yaml` | both hooks (`hil-select-test` → `ci-select-test`, `files:` globs) | +| `.github/workflows/build.yml` | selector path, step name | +| `.claude/skills/pre-pr/SKILL.md` | selector path | + +The test file stays in `test/hil/test/` — it still consumes the rig rosters and `hil_util`. + +The selector gains one non-stdlib-but-local import: `tools/build_utils.skip_example` for the +buildability post-filter. `build_utils` imports only `subprocess`, `pathlib` and `re`, so the +stdlib closure the bare GitHub runner depends on is preserved; `BottomLayer` must be extended +to cover it. + +Because a wrong parents-count already broke this module once (there is a comment in the source +recording it), the moved module gets a guard test asserting its derived repo root contains +`src/` and `hw/bsp/`. + +## Component: `tools/build.py --example` + +`build.py` has no example filter today. `-T/--target` exists and maps to +`cmake --build --target `, but it hard-fails on a target that does not exist, and absent +targets are routine (40 `skip.txt`, 13 `only.txt`). + +New repeatable `-e/--example /`: + +- Default (none given) keeps today's behaviour exactly: `--target all`. +- Given, each board's list is intersected with `build_utils.skip_example(example, board)`, then + passed as one `--target ` per example. Example target names are the directory names and + are unique across all four roles (verified: 46 examples, zero collisions). +- A board whose intersection is empty is reported **skipped**, not failed. +- `--target tinyusb_metrics` must stay last so metrics run after the examples that feed them. +- The espressif path already builds per example via `get_examples` + `skip_example`; it takes + the same filter. + +Both the family matrix and `hil-build` use this one flag. + +## CI wiring + +### `.github/scripts/ci_set_matrix.py` + +Two mutually exclusive optional flags. **Output shape is unchanged** — `{toolchain: [family]}`, +just fewer families. With no flags the output is byte-for-byte today's, so push, release and +`workflow_dispatch` are untouched. + +| Flag | Caller | Behaviour | +| --------------- | -------- | -------------------------------------------------------- | +| `--select JSON` | GHA | consumes the selector JSON the workflow already computes | +| `--base REF` | CircleCI | runs `tools/ci_select.py` itself | + +`build.full` true, or any exception, prints the full matrix with a warning on stderr. + +### `.github/scripts/hil_ci_set_matrix.py` + +Already takes `--select` and already scopes boards. It additionally appends `-e ` per +board from `hil_examples`, so each `hil-build` entry builds only what its board will run plus +`board_test`. When `hil_examples` is absent (hand-runs), it falls back to today's `--target all`. + +### The example map is a side channel, not a matrix entry + +The build example list deliberately does **not** ride inside the family matrix entry string. On +CircleCI the `family` parameter is also passed to `python tools/get_deps.py +<< parameters.family >>` and tested with `if [ << parameters.family >> == "rp2040" ]` — a value +carrying `-e` flags breaks both — and CircleCI matrix parameters form a cartesian product, so a +parallel `example-args` parameter would multiply the jobs rather than zip with them. + +So `build.family_examples` travels as one JSON blob and each build job resolves its own entry: + +- **GHA:** `set-matrix` exposes it as an output; `build_util.yml` gains an optional + `example-map` input (default `''`); a step resolves `-e` flags for `matrix.arg` with `jq`. +- **CircleCI:** `set-matrix` writes `example_map.json` and `persist_to_workspace`s it; the + `build` job gains `attach_workspace` and resolves the same way. + +Consequences of keeping the matrix shape: the metrics artifact name stays `metrics-`, +and CircleCI's generated `config2.yml` does not inflate to one entry per family. `hil-build` +needs none of this — its matrix entries are already compound per-board strings from +`hil_ci_set_matrix.py`, so `-e` flags go straight in. + +### `.github/workflows/build.yml` + +The existing `HIL selection (PR only)` step in `set-matrix` is already gated on +`pull_request` — exactly the gate wanted. It is renamed, repointed at `tools/ci_select.py`, and +its `select` output is threaded into `ci_set_matrix.py --select`, so the filter costs zero extra +selector invocations. + +`build_util.yml`'s `if: inputs.build-args != '[]'` already skips a toolchain leg whose list is +empty, and a partially-skipped matrix aggregating to success is the documented pattern +`hil-build` already relies on. When every leg is empty (a `test/hil`-only PR), the `cmake` job +has nothing to build. Accepted: GitHub treats a skipped job as satisfying a required status +check, and HIL is unaffected because `hil-build` is a separate matrix. `code-metrics` still +runs (`!cancelled()` plus `cmake` success-or-skipped) and posts a "built no families on this +push" marker, so the sticky size comment never shows a stale table from an earlier push. + +### `.circleci/config.yml` + +The `set-matrix` job passes `--base origin/master` when `CIRCLE_PULL_REQUEST` is set, after +`git fetch --no-tags origin master || true`; unfiltered otherwise. CircleCI does not expose the +PR base branch, so `master` is assumed — true for essentially every tinyusb PR, and any ref or +clone problem falls back to the full matrix. + +Two fixes the GHA side does not need: + +- `gen_build_entry` must **skip** a toolchain whose family list is `[]`. An empty matrix + parameter is a hard CircleCI config error, not a skipped job. +- `BUILD_ALIASES` must collect only aliases that were actually generated, or `code-metrics`' + `requires:` names a job that does not exist. + +## Code metrics + +`tools/metrics.py` averages per-file sizes across every build, and the per-family +`metrics-` artifact stores only that average — over whichever examples were built. Both +build axes therefore break the comparison: a 3-family PR against master's 64-family average, +and an 11-example average against master's 46-example one. + +The fix is to make the artifact carry per-example detail and compare the intersection. + +1. **`metrics.py combine --by-example`** additionally writes `metrics_by_example.json`, + `{example: {files: [...]}}`. The example name is the map.json's parent directory + (`///*.map.json`). +2. `examples/CMakeLists.txt`'s `tinyusb_metrics` target emits both files; `build_util.yml` + uploads both under the existing `metrics-` artifact name. +3. `combine` learns to expand a by-example JSON into one data entry per example and an + `--only-examples` filter, so a subset can be averaged on demand. +4. `code-metrics` computes the **intersection of `(family, example)` pairs present on both + sides**, averages each side over exactly those pairs, and compares. Dropped pairs are named + in the PR comment. An empty intersection skips the compare with an explicit note. + +`search_artifacts: true` is required on the base-side download: a docs-only master push +produces no per-family artifacts — which is why `metrics-carry-forward` exists for the +aggregate — so per-family baselines may come from different master runs. That is still a valid +per-family baseline. + +Today's `metrics-tinyusb` aggregate keeps being produced for the unfiltered path, releases and +`metrics-carry-forward`. The filtered path never falls back to it — that is precisely the +mismatched compare this section exists to prevent. `hil-build` uploads no metrics, so its +narrowing does not touch any of this. + +For narrow PRs this is sharper than today: a `dcd_rp2040` PR's size delta stops being diluted +by a 64-family, 46-example average. + +**Size check the plan must run first:** the by-example JSON is ~46× the entries of today's +average. If it proves too large as an artifact, drop per-symbol detail from the by-example file +(sizes only) — symbols are only needed in the aggregate. The plan must also verify that +`dawidd6/action-download-artifact@v11` supports `name_is_regexp`; the fallback is a +`gh run download` loop. + +## Testing + +Extended in `test/hil/test/test_ci_select.py` (stdlib-only, ~0.1 s, already a pre-commit hook +and already gating CI's selector step): + +- One case per rule 1–17, asserting all three outputs. +- Per-family union: a mixed diff (`dcd_rp2040.c` + `cdc_device.c`) gives `rp2040` the device + list and every other family the CDC list — not the cross product of both. +- Include edges: an `audio.h` change selects the MIDI examples; a `cdc.h` change the net one. +- `hil_examples` always contains `device/board_test` for every selected board, including when + `full: true`, and is otherwise exactly `board_tests(board)`. +- `hil_examples` never exceeds the 21-example HIL universe. +- The scan is CMake-only: a port referenced solely from a `family.mk` (`microchip/pic32mz`) + resolves to no family, and no `family.mk` is ever read. +- Boundary matching: `microchip/pic` does not inherit `microchip/pic32mz`'s families, and + `hw/mcu/nordic/nrf5x` resolves despite having no trailing slash at its reference site. +- Buildability post-filter: `typec/power_delivery` prunes to 2 families, `host/bare_api` to 23. +- Structural invariant: every `src/portable/*/*` and every tracked `hw/mcu/` resolves + to ≥1 family, allowlist `{microchip/pic, microchip/pic32mz}`. +- Every name in `build.families` is a real `hw/bsp/`; every example name on either axis is + a real `examples//` directory. +- Repo-root guard for the moved module. +- `ci_set_matrix.py`: no flags → byte-identical to today; `--select` with `build.full` → + identical; `--select` narrow → a subset; malformed `--select` → full plus a warning. +- `hil_ci_set_matrix.py`: no `hil_examples` → today's args byte-for-byte; with it → `-e` flags + appended per board, `board_test` always present. +- `build.py`: `-e` with an example the board skips builds nothing and reports skipped, not + failed; no `-e` still passes `--target all`. + +## Known gaps + +- **CircleCI size comparison.** CircleCI stores only the combined `metrics.json`, so the + intersection compare is unavailable there; when filtered it prints a note and copies + `metrics.md`. Its `metrics_compare.md` is a stored artifact that nothing reads in review — the + PR comment comes from GHA. Making CircleCI store per-example metrics is a follow-up. +- **HIL re-run attempts.** A re-run spec is a subset of the original selection, so the + firmware `hil-build` produced already covers it. This holds only while re-run specs stay + subsets; a future "re-run with extra tests" feature would need `hil-build` re-run too. +- **Membrowse** receives rows for fewer families and fewer examples on filtered PRs. If that + service misbehaves, the escape hatch is keeping the membrowse upload leg unfiltered. +- **`typec/power_delivery`** is reached only through rules 5 and 13 (`src/portable/st/typec` + has neither a `dcd_` nor an `hcd_` prefix, so it selects `ALL` examples on its 5 families, + which the post-filter then prunes to 2). A dedicated typec rule is possible later; the + post-filter already makes it cheap. +- **A `test/hil`-only PR reports `cmake` as skipped** rather than passing. Accepted; + revertible with a one-family floor if branch protection turns out to disagree. + `code-metrics` still runs in that case: with no `cmake-build/*/metrics.json` to + aggregate it writes `_Code-size comparison skipped: PR selection built no families + on this push._` and posts that as the sticky comment, so the size section reflects + THIS push instead of keeping the previous one's table. +- **`microchip/pic32mz` builds nothing.** The scan is CMake-only and `hw/bsp/pic32mz` ships + only a `family.mk`, so a change there selects no family. That matches reality — `pic32mz` is + in neither provider's family list — but it means the port is unbuilt by CI whether or not + this design lands. Giving it a `family.cmake` is the fix, and is out of scope here. +- **Seven bsp families are in no CI toolchain today** (`espressif`, `efm32`, `same7x`, + `cxd56`, `f1c100s`, `pic32mz`, `py32f0`); the intersection drops them, matching current + behaviour. This change does not alter that. -- cgit v1.3.1 From e13eff8d4e757ebe7709a58fce44017b8be5a84d Mon Sep 17 00:00:00 2001 From: hathach Date: Fri, 21 Aug 2026 12:41:47 +0700 Subject: ci: fix nine ways the selection under-selected or mismatched Every one of these dropped coverage silently - the worst failure mode here, because the PR still goes green. Found by review, each reproduced first. Selection rules: * class_macros derived the config macro from the class DIRECTORY, so a change to src/class/midi/midi2_device.c selected the midi_test examples (which do not compile it) and never examples/device/midi2_device (the only one that enables CFG_TUD_MIDI2, and the only one that does). The file's own macro is unioned in where it differs - union, never replace: over-selecting costs a build, under-selecting merges a break. * the ${FAMILY_MCUS} fallback added for espressif fired on any family whose _family_mcus came back empty, and _cmake_sets is if()-blind and keeps the FIRST definition - so mcx/frdm_mcxn947 answered MCXA15, a token six examples' skip.txt names, dropping 12 firmware images CMake builds. Limited now to families that never spell set(FAMILY_MCUS ...) at all. * lib_examples read only an example's top-level CMakeLists.txt/Makefile; host/msc_file_explorer_freertos names lib/embedded-cli in src/CMakeLists.txt and survived by luck. The whole example tree is scanned. (SEGGER_RTT and rt-thread still resolve to nothing: all three references sit inside a LOGGER=rtt guard no CI build sets - the documented ruling, not a miss.) * get_family_boards applied ci_skip_boards/ci_preferred_boards only under GITHUB_ACTIONS/CIRCLECI, so the selector answered differently on a laptop than on a runner; _prune_buildable forces CI semantics. Its one-board pick also abandoned the whole preferred list when entry one could not build the -e set, and asked skip_example without the build's -D tokens. * _config_enables and lib_examples still read with the locale encoding - under LC_ALL=C the selector tracebacked on three tracked tusb_config.h files. The whole selector and its suite run clean there now. Workflows: * the Membrowse Upload step omitted $EX_ARGS, but --one-first now picks the board from the -e set, so it configured a different, empty build dir and uploaded --identical for a board never compiled. It takes $EX_ARGS for the BOARD; the target stays the aggregate, which has no DEPENDS and still records every example. * blanking FAM_REGEX reset only build_filtered, leaving the build scoped while code-metrics took the UNSCOPED branch and diffed a 1-family run against the full averaged baseline. All three drop together now, as CircleCI's fall-open does. * CircleCI's EX_ARGS had no character screen and is used unquoted, and its code-metrics job still exit 1'd on an empty metrics set - which a scoped build makes a legitimate outcome. * a `ci-full` PR label now turns the scoping off for one PR. A selector bug under-selects silently, and without a label the only ways back to a full matrix are accidental. Performance, since the selector gates every other job: family.cmake texts are read once rather than per changed directory (a 6,000-file dep bump re-read 84 files 99,892 times) and _scrape_mcu is cached: 2.2s -> 0.29s there, 0.8s -> 0.33s on a class diff. Tests: a drift guard for hw/bsp families absent from ci_set_matrix.family_list (they select zero legs now, where they used to ride the full matrix); the rule-4 port test asserted a SUBSET, which set() satisfies, so it could not fail on the empty selection it exists to catch; the GITHUB_ENV guard test counted a SUM of two guards. Drops metrics.py's --only-examples, which nothing called, and applies the TOTAL scrub to the by-example branch that skipped it. --- .circleci/config2.yml | 18 +++++- .github/workflows/build.yml | 24 +++++-- .github/workflows/build_util.yml | 16 ++--- .../2026-08-19-ci-build-family-filter-design.md | 16 ++--- test/hil/test/test_ci_metrics.py | 35 +++++++---- test/hil/test/test_ci_select.py | 24 +++++++ tools/build.py | 31 +++++++-- tools/build_utils.py | 14 ++++- tools/ci_select.py | 73 ++++++++++++++++------ tools/metrics.py | 15 +++-- 10 files changed, 199 insertions(+), 67 deletions(-) (limited to 'docs/superpowers/specs') diff --git a/.circleci/config2.yml b/.circleci/config2.yml index 899cbe24a..2e69588ae 100644 --- a/.circleci/config2.yml +++ b/.circleci/config2.yml @@ -125,6 +125,15 @@ commands: # shell-text interpolation (unsafe characters); family is a job # parameter with charset [a-z0-9_], safe to interpolate directly. EX_ARGS=$(printf '%s' "$EXAMPLE_MAP" | jq -r --arg fam "<< parameters.family >>" '(.[$fam] // []) | map("-e " + .) | join(" ")' 2>/dev/null) || EX_ARGS='' + # same screen as build_util.yml's: the values are example dir names from the + # PR checkout and $EX_ARGS is used unquoted below, so a glob metacharacter + # would pathname-expand against the build cwd. Dropping the filter builds + # everything - the safe direction, and what GHA does for the same input. + case "$EX_ARGS" in + *[!-A-Za-z0-9_/\ ]*) + echo "warning: unexpected characters in the example filter - building all examples" + EX_ARGS='' ;; + esac if [ << parameters.toolchain >> == esp-idf ]; then docker run --rm -v $PWD:/project -w /project espressif/idf:v5.5.3 python tools/build.py << parameters.build-args >> --target all $EX_ARGS << parameters.family >> @@ -253,8 +262,13 @@ jobs: if ls /tmp/metrics/*/*.json 1> /dev/null 2>&1; then python tools/metrics.py combine -j -m -f tinyusb/src /tmp/metrics/*/*.json else - echo "No metrics files found" - exit 1 + # A scoped PR can legitimately build no metrics leg at all (every selected + # family empty, or none of them on a metrics toolchain), so this is not an + # error any more - it was, when the matrix was always the full 64 families. + # An empty file keeps store_artifacts and the compare step below honest: + # both would otherwise act on a missing path. + echo "No metrics files found - PR selection built no metrics leg" + echo '{"files": []}' > metrics.json fi - store_artifacts: diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 2ee124cb3..39a4e7afd 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -68,9 +68,14 @@ jobs: with: fetch-depth: 0 + # The `ci-full` PR label turns the scoping off for one PR: no selection file is + # written, so both matrices and every rig job fall back to the unscoped behaviour. + # An escape hatch is the point - a selector bug under-selects SILENTLY, and without + # a label the only routes back to a full matrix are accidental (touch an + # unclassified path, or break the selector badly enough that it falls open). - name: CI selection (PR only) id: hil-select - if: github.event_name == 'pull_request' + if: github.event_name == 'pull_request' && !contains(github.event.pull_request.labels.*.name, 'ci-full') env: BASE_REF: ${{ github.base_ref }} run: | @@ -166,8 +171,6 @@ jobs: fi fi [ -z "$MATRIX_JSON" ] && MATRIX_JSON=$(python .github/scripts/ci_set_matrix.py) - echo "matrix=$MATRIX_JSON" - echo "matrix=$MATRIX_JSON" >> $GITHUB_OUTPUT # Build-axis extras: the per-family example map rides as a side channel # (a value inside matrix entries would break CircleCI's family parameter @@ -188,12 +191,23 @@ jobs: # silently match another family's baseline case "$FAM_REGEX" in *[!-A-Za-z0-9_\|]*) - echo "::warning::unexpected characters in the family list - unscoped metrics" + echo "::warning::unexpected characters in the family list - dropping the scoping" FAM_REGEX='' ;; esac - [ -z "$FAM_REGEX" ] && BUILD_FILTERED='false' + if [ -z "$FAM_REGEX" ]; then + # all three drop together, as CircleCI's fall-open does. Resetting only + # build_filtered leaves the build scoped while code-metrics takes the + # UNSCOPED branch, diffing a 1-family run against the full averaged + # baseline and publishing that as the PR's code-size impact. + BUILD_FILTERED='false' + EXAMPLE_MAP='{}' + MATRIX_JSON=$(python .github/scripts/ci_set_matrix.py) + fi fi fi + # emitted once, after every path that can still change it + echo "matrix=$MATRIX_JSON" + echo "matrix=$MATRIX_JSON" >> $GITHUB_OUTPUT echo "example_map=$EXAMPLE_MAP" >> $GITHUB_OUTPUT echo "build_filtered=$BUILD_FILTERED" >> $GITHUB_OUTPUT echo "build_families_regex=$FAM_REGEX" >> $GITHUB_OUTPUT diff --git a/.github/workflows/build_util.yml b/.github/workflows/build_util.yml index dfbd83ee2..52999616d 100644 --- a/.github/workflows/build_util.yml +++ b/.github/workflows/build_util.yml @@ -126,14 +126,16 @@ jobs: MEMBROWSE_API_KEY: ${{ secrets.MEMBROWSE_API_KEY }} run: | # if code-changed is false --> there is no elf -> membrowse target upload with --identical flag - # Deliberately NOT scoped by $EX_ARGS: -membrowse-upload has no - # DEPENDS (hw/bsp/family_support.cmake), so the aggregate rebuilds nothing - - # it just records every example, reporting the ones with an elf and - # --identical for the rest. Filtering it here would drop the excluded - # examples from the dataset membrowse-comment.yml reports against, instead - # of recording them as unchanged. + # $EX_ARGS is passed for the BOARD it picks, not to scope the targets: + # --one-first now chooses a board that can build the -e set (tools/build.py), + # so omitting it here would configure a DIFFERENT, empty build dir and upload + # --identical for a board that was never compiled. The target list is not + # scoped by it - `examples-membrowse-upload` is not `all`, so it passes + # through as the aggregate, which has no DEPENDS (hw/bsp/family_support.cmake): + # it rebuilds nothing and still records every example, --identical for the + # ones without an elf. BUILD_PY_ARGS="-s ${{ inputs.build-system }} ${{ steps.setup-toolchain.outputs.build_option }} ${{ inputs.build-options }}" - python tools/build.py $BUILD_PY_ARGS --target examples-membrowse-upload -j 1 ${{ matrix.arg }} + python tools/build.py $BUILD_PY_ARGS --target examples-membrowse-upload -j 1 ${{ matrix.arg }} $EX_ARGS shell: bash - name: Upload Artifacts for Metrics diff --git a/docs/superpowers/specs/2026-08-19-ci-build-family-filter-design.md b/docs/superpowers/specs/2026-08-19-ci-build-family-filter-design.md index 9fa358bee..8f77dc50a 100644 --- a/docs/superpowers/specs/2026-08-19-ci-build-family-filter-design.md +++ b/docs/superpowers/specs/2026-08-19-ci-build-family-filter-design.md @@ -211,16 +211,16 @@ It falls open to the full matrix whenever the entries are not the whole answer: * the file will not parse; * there is no base content: `--diff-file` mode has no git, so no merge-base blob; * a changed entry carries a family token that names no `hw/bsp/` and is not one of the - eight known aliases. "Changed but unmappable" is not "nothing changed": reading it as the + known aliases. "Changed but unmappable" is not "nothing changed": reading it as the latter empties the whole build matrix for a dep bump. -The eight known aliases (`sam3x`, `samd21`, `samd51`, `same5x`, `fc100s`, `spresense`, -`stm32l1`, `stm32l5`) are pinned in `_DEPS_ALIAS_TOKENS` and select nothing. `get_deps` matches -a token against a requested family name verbatim (`f in entry[2].split()`), so these tokens -match nothing there either — four are pre-rename spellings listed beside the current name in -the same entry, two point at a differently-named family dir (`fc100s`→`f1c100s`, -`spresense`→`cxd56`, both unreachable in `get_deps` itself), and two name no family in the tree. -A ninth appearing fails `TestOrphanInvariant`. +The six known aliases (`sam3x`, `samd21`, `samd51`, `same5x`, `stm32l1`, `stm32l5`) are +pinned in `_DEPS_ALIAS_TOKENS` and select nothing. `get_deps` matches a token against a +requested family name verbatim (`f in entry[2].split()`), so these tokens match nothing +there either — four are pre-rename spellings listed beside the current name in the same +entry, and two name no family in the tree. (`fc100s` and `spresense` were on this list +until they were corrected in `get_deps.py`; those two were the only ones that left a +real dep unreachable for its own family.) A seventh appearing fails `TestOrphanInvariant`. ## Component: `tools/ci_select.py` diff --git a/test/hil/test/test_ci_metrics.py b/test/hil/test/test_ci_metrics.py index 89d03aaae..6c236e827 100644 --- a/test/hil/test/test_ci_metrics.py +++ b/test/hil/test/test_ci_metrics.py @@ -58,13 +58,16 @@ class TestByExample(unittest.TestCase): '-o', out, os.path.join(td, '*', '*', '*.map.json')], check=True) out2 = os.path.join(td, 'sub') r = subprocess.run([sys.executable, METRICS, 'combine', '-q', '-j', - '--only-examples', 'device/cdc_msc', '-o', out2, out + '_by_example.json'], capture_output=True, text=True) self.assertEqual(r.returncode, 0, r.stderr) sub = json.load(open(out2 + '.json')) names = {f['file'] for f in sub['files']} - self.assertEqual(names, {'usbd.c', 'cdc_device.c'}) # bare_api filtered out + # one data entry per example, not one blob: reading it as an ordinary + # metrics.json would double-count every file + self.assertIn('usbd.c', names) + self.assertIn('cdc_device.c', names) + self.assertNotIn('TOTAL', {n.upper() for n in names}) def test_by_example_expansion_is_keyed_on_the_filename(self): # the '_by_example.json' suffix IS the contract (write_by_example, the CMake @@ -339,9 +342,15 @@ class TestWorkflowSelectionHandOff(unittest.TestCase): # with secrets - and for run_*, flips which rig jobs execute for name in ('EX_ARGS', 'ARTIFACT_TAG'): self.assertIn(f'echo "{name}=', self.util) - self.assertEqual(self.util.count('case "$EX_ARGS" in') + - self.util.count('case "$TAG" in'), 2, - 'both GITHUB_ENV writes must screen their value first') + # per guard, not a sum: `count(a) + count(b) == 2` stays green when one guard is + # deleted and the other duplicated + for guard in ('case "$EX_ARGS" in', 'case "$TAG" in'): + self.assertEqual(self.util.count(guard), 1, + f'{guard}: each GITHUB_ENV write screens its value exactly once') + # CircleCI builds from the same PR-derived map and uses $EX_ARGS unquoted + cci = open(os.path.join(CIRCLECI, 'config2.yml')).read() + self.assertIn('case "$EX_ARGS" in', cci, + 'the CircleCI copy of the example filter needs the same screen') self.assertIn('case "$BUILD_ARGS" in', self.build) self.assertIn('unexpected characters in the " + key', self.build, 'the args_*/run_* emitter must screen each board filter') @@ -429,12 +438,16 @@ class TestWorkflowSelectionHandOff(unittest.TestCase): self.assertEqual(matrix.count('ci_set_matrix: UNSCOPED'), 2, 'every fall-open path must print the marker build.yml greps for') - def test_membrowse_upload_is_not_scoped(self): - # -membrowse-upload has no DEPENDS, so the aggregate rebuilds nothing - - # it records every example, --identical for the ones without an elf. Scoping it - # drops the excluded examples from the dataset instead of marking them unchanged. - upload = self.util[self.util.index('--target examples-membrowse-upload'):] - self.assertNotIn('$EX_ARGS', upload.split('\n')[0]) + def test_membrowse_upload_sees_the_same_board_as_the_build(self): + # $EX_ARGS is passed for the BOARD it selects: --one-first picks a board that can + # build the -e set, so without it membrowse configures a different, empty build + # dir and uploads --identical for a board that was never compiled. It does NOT + # scope the targets - `examples-membrowse-upload` is not `all`, so it passes + # through as the aggregate, which has no DEPENDS and still records every example. + line = [l for l in self.util.splitlines() + if '--target examples-membrowse-upload' in l][0] + self.assertIn('$EX_ARGS', line) + self.assertNotIn('-e ', line.replace('$EX_ARGS', '')) if __name__ == '__main__': diff --git a/test/hil/test/test_ci_select.py b/test/hil/test/test_ci_select.py index 74e5f48e6..031e8e287 100644 --- a/test/hil/test/test_ci_select.py +++ b/test/hil/test/test_ci_select.py @@ -777,6 +777,24 @@ class TestOrphanInvariant(unittest.TestCase): for v in vendors: self.assertTrue(ci_select.mcu_families(v + '/x.c', REPO), f'{v}: resolves to no family') + # hw/bsp families ci_set_matrix's family_list does not map to any toolchain. Before + # scoping these were harmless - the matrix was always every family in family_list, + # so a PR touching one of them still compiled the other 64. Now the selection + # intersects to nothing and every leg skips, so a family landing here by accident is + # a silent hole. espressif is deliberate: its boards are built by hil-build-esp, + # keyed on board name rather than family. + UNBUILT_FAMILIES = {'cxd56', 'efm32', 'espressif', 'f1c100s', 'pic32mz', 'py32f0', + 'same7x'} + + def test_every_bsp_family_is_in_the_ci_matrix(self): + sys.path.insert(0, os.path.join(REPO, '.github/scripts')) + import ci_set_matrix + fams = set(ci_select.all_bsp_families(REPO)) + self.assertEqual(fams - set(ci_set_matrix.family_list), self.UNBUILT_FAMILIES, + 'a hw/bsp family that no toolchain in ci_set_matrix.family_list ' + 'builds: a PR touching only it now selects zero build legs. Wire ' + 'it into family_list, or add it here with a reason.') + def test_every_get_deps_family_token_resolves_or_is_a_known_alias(self): """Same drift guard, dep side. A token naming no hw/bsp dir makes the entry unreachable for its family in get_deps.py itself (`f in entry[2].split()`), and @@ -1132,8 +1150,14 @@ class TestBuildClassifier(unittest.TestCase): # real feather_rp2040_max3421 board) and espressif's component CMakeLists also # references it — so the raw (unpruned) scan legitimately finds both; Task 4's # buildability post-filter is what may later prune either away + # non-empty FIRST: a subset assertion is satisfied by set(), and since ports are + # now empty-means-empty (fail-closed) an unnoticed regression to zero families + # would select no build leg at all and merge an uncompiled HCD + self.assertTrue(s['families'], 'a host-port change must select some family') self.assertLessEqual(set(s['families']), {'espressif', 'rp2040'}) + self.assertTrue(s['family_examples'], 'and must name the examples for them') for exs in s['family_examples'].values(): + self.assertTrue(exs) self.assertFalse(any(e.startswith(('device/', 'typec/')) for e in exs)) def test_port_shared_file_selects_all_examples(self): # rule 5 diff --git a/tools/build.py b/tools/build.py index e7ca1c839..eeefca22d 100755 --- a/tools/build.py +++ b/tools/build.py @@ -299,7 +299,8 @@ def build_boards_list(boards, build_defines, build_system, build_name, build_cfl return ret -def get_family_boards(family, one_random, one_first, examples=None, build_system='cmake'): +def get_family_boards(family, one_random, one_first, examples=None, build_system='cmake', + extra_defines=(), ci=None): """Get list of boards for a family. Args: @@ -314,13 +315,23 @@ def get_family_boards(family, one_random, one_first, examples=None, build_system which every one of those examples skips - and the leg runs to green having compiled nothing and uploaded no metrics. build_system: which skip answer to ask for; the two differ (build_utils) + extra_defines: this build's -D tokens, so a board whose only.txt match comes + from -DMAX3421_HOST=1 is not judged unbuildable here and buildable in + cmake_board + ci: force the ci_skip_boards / ci_preferred_boards lists on or off. Default + None reads the environment, which is right for a build but NOT for a caller + asking what CI would do: ci_select must answer the same on a laptop as on a + runner, or /pre-pr and the code-size skill report a family list CI will not + reproduce. Returns: List of board names """ + if ci is None: + ci = bool(os.getenv('GITHUB_ACTIONS') or os.getenv('CIRCLECI')) skip_list = [] preferred_list = [] - if os.getenv('GITHUB_ACTIONS') or os.getenv('CIRCLECI'): + if ci: skip_list = ci_skip_boards.get(family, []) preferred_list = ci_preferred_boards.get(family, []) @@ -339,9 +350,16 @@ def get_family_boards(family, one_random, one_first, examples=None, build_system # no filter, or nothing in the filter is buildable anywhere: keep today's # answer rather than inventing a different board return examples is None or any( - not build_utils.skip_example(e, board, (), build_system) for e in examples) - - if preferred_list and buildable(preferred_list[0]): + not build_utils.skip_example(e, board, extra_defines, build_system) + for e in examples) + + # the WHOLE preferred list, in order - stopping at entry one would abandon a + # curated list for the raw alphabetical order the moment its first board cannot + # build the filter, which also moves the board the metrics baseline is keyed on + for b in preferred_list: + if buildable(b): + return [b] + if preferred_list and examples is None: return [preferred_list[0]] candidates = [b for b in all_boards if buildable(b)] or all_boards if one_first: @@ -434,7 +452,8 @@ def main(): # get boards from families and append to boards list all_boards = list(boards) for f in all_families: - all_boards.extend(get_family_boards(f, one_random, one_first, examples, build_system)) + all_boards.extend(get_family_boards(f, one_random, one_first, examples, + build_system, tuple(build_defines))) # build all boards result = build_boards_list(all_boards, build_defines, build_system, build_name, build_cflags, build_targets, diff --git a/tools/build_utils.py b/tools/build_utils.py index 2af8fd624..1eeef0269 100755 --- a/tools/build_utils.py +++ b/tools/build_utils.py @@ -141,9 +141,12 @@ def _family_mcus(family_dir, board_dir): board_cmake = pathlib.Path(board_dir) / "board.cmake" out = set() depth = 0 + any_set = False for line in text.splitlines(): line = line.strip() m = _FAMILY_MCUS_RE.match(line) + if m: + any_set = True if m and depth == 0: files = (str(board_cmake), str(fam_cmake)) for tok in m.group(1).split(): @@ -156,16 +159,23 @@ def _family_mcus(family_dir, board_dir): depth += 1 elif re.match(r'endif\s*\(', line): depth = max(0, depth - 1) - if not out: + if not out and not any_set: # FAMILY_MCUS can also be produced rather than set: hw/bsp/espressif derives it # with `string(TOUPPER ${IDF_TARGET} FAMILY_MCUS)`, which _FAMILY_MCUS_RE cannot - # see, leaving espressif's whole cmake answer resting on the IDF_TARGET scrape + # see, leaving espressif's whole cmake answer resting on the IDF_TARGET scrape. + # + # `not any_set` is load-bearing: _cmake_sets is if()-blind and keeps the FIRST + # definition, so on a family that sets FAMILY_MCUS only inside conditionals + # (mcx, nrf) this would leak branch one's value onto every board - mcx/frdm_mcxn947 + # answered MCXA15, which six examples' skip.txt names, dropping 12 firmware + # images CMake actually builds. Those families keep the CFG_TUSB_MCU scrape. val = _cmake_expand('${FAMILY_MCUS}', (str(board_cmake), str(fam_cmake))) if val: out.add(val) return frozenset(out) +@functools.lru_cache(maxsize=None) def _scrape_mcu(family_dir, board_dir, family): """(CFG_TUSB_MCU token of this board, the text it was read from), master's algorithm verbatim: family.mk (family.cmake when there is none) first, falling diff --git a/tools/ci_select.py b/tools/ci_select.py index d253f8c01..cd63899c1 100755 --- a/tools/ci_select.py +++ b/tools/ci_select.py @@ -42,6 +42,7 @@ ALL_TESTS = {'device': device_tests, 'dual': dual_tests, 'host': host_test} # class dir -> config macro suffix exceptions (rule 3); dfu is per-file, handled inline NET_MACROS = ('ECM_RNDIS', 'NCM') + def _read(path: str) -> str: """Read a source file with a fixed encoding. The locale's is not it: several tracked sources carry non-ASCII bytes, and under LC_ALL=C the decode raises UnicodeDecodeError @@ -211,17 +212,25 @@ def path_families(rel_dir: str, repo_root: str) -> set: CI job and resolves to nothing. Boundary = '/', whitespace, quote, paren, brace or end: `${TOP}/hw/mcu/nordic/nrfx` has no trailing slash, while bare 'microchip/pic' must not match '.../microchip/pic32mz/...'.""" - fams = set() - bsp_root = os.path.join(repo_root, 'hw/bsp') pat = re.compile(re.escape(rel_dir) + r'(?=[/\s"\')}]|$)', re.M) - for f in glob.glob(os.path.join(bsp_root, '*/family.cmake')) + \ - glob.glob(os.path.join(bsp_root, '*/components/*/CMakeLists.txt')): + return {fam for fam, text in _family_file_texts(repo_root) if pat.search(text)} + + +@functools.lru_cache(maxsize=None) +def _family_file_texts(repo_root: str) -> tuple: + """((family, text), ...) for every family.cmake and espressif component + CMakeLists.txt, read once. path_families is called per distinct directory in the + diff and its own cache only helps repeats: a 6,000-file hw/mcu dep bump re-read + these 84 files 99,892 times (2.2 s) before this.""" + bsp_root = os.path.join(repo_root, 'hw/bsp') + out = [] + for f in sorted(glob.glob(os.path.join(bsp_root, '*/family.cmake')) + + glob.glob(os.path.join(bsp_root, '*/components/*/CMakeLists.txt'))): try: - if pat.search(_read(f)): - fams.add(os.path.relpath(f, bsp_root).split(os.sep, 1)[0]) + out.append((os.path.relpath(f, bsp_root).split(os.sep, 1)[0], _read(f))) except OSError: pass - return fams + return tuple(out) def port_families(port_dir: str, repo_root: str) -> set: @@ -348,10 +357,14 @@ def class_include_edges(repo_root: str) -> dict: return edges +_CLS_STEM_RE = re.compile(r'(.*?)(?:_(?:device|host))?\.[ch]$') + + def class_macros(cls: str, base: str, prefix: str) -> list: """Config macros that compile a class dir's code, for role prefix TUD/TUH. - `base` refines dfu only (it splits DFU from DFU_RUNTIME per file); pass '' for - a class reached through an include edge, where the widest set is correct.""" + `base` refines dfu (it splits DFU from DFU_RUNTIME per file) and adds the file's + own macro where that differs from the directory's; pass '' for a class reached + through an include edge, where the widest set is correct.""" if cls == 'net': return [f'CFG_{prefix}_{m}' for m in NET_MACROS] if cls == 'dfu': @@ -360,7 +373,18 @@ def class_macros(cls: str, base: str, prefix: str) -> list: if base.startswith('dfu_device') or base.startswith('dfu_host'): return [f'CFG_{prefix}_DFU'] return [f'CFG_{prefix}_DFU', f'CFG_{prefix}_DFU_RUNTIME'] - return [f'CFG_{prefix}_{cls.upper()}'] + out = [f'CFG_{prefix}_{cls.upper()}'] + # A class directory can hold more than one class. src/class/midi ships MIDI 1.0 + # AND MIDI 2.0: midi2_device.c is `#if CFG_TUD_ENABLED && CFG_TUD_MIDI2`, and + # examples/device/midi2_device is the only example that enables it - so the + # directory macro alone selected the midi_test examples, which do not compile the + # changed file, and none of the ones that do. Union, never replace: the file may + # still be pulled in by the directory's own macro, and over-selecting costs a build + # while under-selecting merges a break. + m = _CLS_STEM_RE.match(base) + if m and m.group(1) and m.group(1) != cls: + out.append(f'CFG_{prefix}_{m.group(1).upper()}') + return out # A define is OFF only when its value is a literal zero (0, 00, (0)), optionally @@ -407,7 +431,7 @@ def _class_roles(base: str) -> set: def _config_enables(cfg_path: str, macros) -> bool: try: - with open(cfg_path) as f: + with open(cfg_path, encoding='utf-8', errors='replace') as f: text = f.read() except OSError: return False @@ -435,15 +459,23 @@ def lib_examples(lib_name: str, repo_root: str) -> set: 'lib/net' cannot inherit lib/networking's example). Per-example on purpose: lib/SEGGER_RTT is named by family_support.cmake's - LOGGER=rtt plumbing, which no CI example build turns on, so a family-file scan - would wrongly narrow it to three families instead of answering 'nobody'.""" + LOGGER=rtt plumbing, which no CI example build turns on (all three references - + family_support.cmake, family_support.mk, rp2040/family.cmake - sit inside a + LOGGER=rtt guard), so a family-file scan would wrongly narrow it to three families + instead of answering 'nobody'. + + The whole example TREE is scanned, not just its top-level files: examples/host/ + msc_file_explorer_freertos/src/CMakeLists.txt names lib/embedded-cli, and that + example survived only because its top-level file happens to name it too.""" pat = re.compile(re.escape('lib/' + lib_name) + r'(?=[/\s"\')}]|$)', re.M) out = set() for ex in all_examples(repo_root): - for f in ('CMakeLists.txt', 'Makefile'): + for f in sorted(glob.glob(os.path.join(repo_root, 'examples', ex, '**', '*'), + recursive=True)): + if os.path.basename(f) not in ('CMakeLists.txt', 'Makefile'): + continue try: - with open(os.path.join(repo_root, 'examples', ex, f)) as fh: - text = fh.read() + text = _read(f) except OSError: continue if pat.search(text): @@ -804,7 +836,7 @@ def main(): repo_root = _REPO_ROOT rosters = [] for c in a.configs: - with open(c) as f: + with open(c, encoding='utf-8', errors='replace') as f: rosters.append((c, json.load(f)['boards'])) files = (_read(a.diff_file).splitlines() if a.diff_file @@ -1029,7 +1061,12 @@ def _prune_buildable(fams, fam_ex, repo_root): reasons.append(f'{fam}: family dir gone from tree, dropped') continue try: - boards = build_py.get_family_boards(fam, False, False) + # ci=True unconditionally: this answers "what will CI build", so it must + # not change with GITHUB_ACTIONS/CIRCLECI being set. Locally the lists + # are off by default, and rp2040 would keep feather_rp2040_max3421 - + # the only board satisfying the max3421 only.txt files - giving a + # developer a family list the runner will not reproduce. + boards = build_py.get_family_boards(fam, False, False, ci=True) except OSError as e: # belt and braces: never traceback here reasons.append(f'{fam}: boards unreadable ({e}), dropped') continue diff --git a/tools/metrics.py b/tools/metrics.py index b97b2b206..27c995954 100644 --- a/tools/metrics.py +++ b/tools/metrics.py @@ -83,7 +83,7 @@ def parse_bloaty_csv(csv_text, filters=None): return {"files": files, "TOTAL": total_all} -def combine_files(input_files, filters=None, only_examples=None): +def combine_files(input_files, filters=None): """Combine multiple metrics inputs (bloaty CSV or metrics JSON) into a single data set.""" filters = filters or [] @@ -105,9 +105,11 @@ def combine_files(input_files, filters=None, only_examples=None): # rule and metrics_pair_compare all spell that suffix) - a shape # sniff would silently reroute any coincidentally-shaped JSON. for ex in sorted(json_data): - if only_examples and ex not in only_examples: - continue - sub = {'files': list(json_data[ex]['files'])} + # same TOTAL scrub the shared path below applies: this branch + # `continue`s past it, so do it here or a by-example input keeps + # the fake TOTAL rows an ordinary input has stripped + sub = {'files': [f for f in json_data[ex]['files'] + if str(f.get('file', '')).upper() != 'TOTAL']} if filters: sub['files'] = [f for f in sub['files'] if f.get('path') and any(x in f['path'] for x in filters)] @@ -614,8 +616,7 @@ def render_compare_table(rows, include_sum): def cmd_combine(args): """Handle combine subcommand.""" input_files = expand_files(args.files) - only_examples = set(args.only_examples.split(',')) if args.only_examples else None - all_json_data = combine_files(input_files, args.filters, only_examples=only_examples) + all_json_data = combine_files(input_files, args.filters) json_average = compute_avg(all_json_data) if json_average is None: @@ -673,8 +674,6 @@ def main(argv=None): help='Sort order: size/size- (descending), size+ (ascending), name/name+ (ascending), name- (descending). Default: size-') combine_parser.add_argument('--by-example', dest='by_example', action='store_true', help='Also write _by_example.json: per-example file lists keyed by role/example') - combine_parser.add_argument('--only-examples', dest='only_examples', default='', - help='Comma-separated role/example ids to keep when reading by-example JSON inputs') # Compare subcommand compare_parser = subparsers.add_parser('compare', help='Compare two metrics inputs (bloaty CSV or metrics JSON)') -- cgit v1.3.1