From 04d0f71984117b8c72349f4584bd9e26a37b129c Mon Sep 17 00:00:00 2001 From: hathach Date: Fri, 21 Aug 2026 11:07:27 +0700 Subject: ci: scope the build matrix and the HIL run to what a PR affects Every PR built all 74 legs (2494 example builds on GHA cmake alone) and flashed all 30 rig boards, whatever it touched. One classifier now walks the PR diff twice and answers three questions: which families to build, which examples per family, and which boards run which tests. Fail-open throughout - anything no rule classifies, any exception, any unusable output falls back to the full matrix, and a master push always builds everything. test/hil/helper/hil_select.py moves to tools/ci_select.py: it is no longer HIL-only, and tools/ is where the build side can import it. test_hil_select.py follows it as test_ci_select.py. Rules (docs/superpowers/specs/2026-08-19-ci-build-family-filter-design.md holds the full table): a port selects the families whose family.cmake references it, and its role - a dcd change skips host examples and vice versa; a class selects only the examples whose tusb_config.h enables its CFG_TU[DH]_ macro, following cross-class includes; an example selects itself; hw/bsp selects its family or board; hw/mcu and lib select whoever references them. CMake is the reference for all of it - make follows whatever cmake decides, family.mk is never scanned. Empty means empty (maintainer ruling): a rule that classifies a path to nothing selects nothing. Ports no family references, classes no config enables, libs no example builds and hw/mcu paths that resolve nowhere are all real - nothing compiles them, so nothing can validate them, and the master-push build is the net. Structural tests pin each such case with an explicit allowlist, so the day one stops being empty it fails pre-commit instead of silently narrowing CI. Per-example builds: build.py grows a repeatable -e, resolved against the targets CMake actually registered and batched into one `cmake --build --target a b c`. build_utils mirrors CMake's family_filter (the whole FAMILY_MCUS list, ${...} and string(TOUPPER ...) resolved) for the cmake side, while the make side keeps master's algorithm verbatim - the two build systems answer differently and a shared answer breaks lpc54's make link. hil-build gains this even on a full selection: 1702 example builds become 515. Transport: the selection travels as a file, never an argv or env var - a mass-sweep diff selects 261 KB against a 128 KiB exec limit, and E2BIG would fail the step before its own fallback could run. CircleCI carries the example map inside the generated config (pipeline parameters cap at 512 chars), swapped into the parameter defaults by sentinel match, and drops the scoping wholesale if that rewrite fails. Every PR-derived value written to $GITHUB_ENV/$GITHUB_OUTPUT is character-screened. Code metrics follow the scoping: metrics.py emits per-example totals, and metrics_pair_compare compares the (board, example) pairs present on both sides instead of a scoped run against a full-matrix average. The selector's own suite gates it in both providers: a selector that exits 0 with valid-but-wrong JSON is the one failure fail-open cannot catch, so a red suite means the full matrix. --- tools/ci_select.py | 1084 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 1084 insertions(+) create mode 100755 tools/ci_select.py (limited to 'tools/ci_select.py') diff --git a/tools/ci_select.py b/tools/ci_select.py new file mode 100755 index 000000000..d253f8c01 --- /dev/null +++ b/tools/ci_select.py @@ -0,0 +1,1084 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: MIT +"""PR-diff -> CI selection: which rig boards and which tests a change can affect. + +Lives in tools/ so it can serve both HIL selection and, from Task 3, build-family +selection. Stdlib-only (runs on bare CI runners; imports hil_util for the example +rosters, never hil_test/pyserial — test_hil_util.BottomLayer enforces the stdlib +closure). Fail-open: any file no rule classifies forces the full matrix. See +docs/superpowers/specs/2026-07-29-hil-pr-scoped-selection-design.md and +docs/superpowers/specs/2026-08-19-ci-build-family-filter-design.md. + +JSON: full, boards (name -> 'all' | [tests]), families (bsp families the diff +touches, including ones with no rig board - build-only consumers such as /pre-pr +sample from these), args (hil_test.py args per config) and args_flasher (the same +args split by each board's flasher, for CI legs that split one rig by flasher). +""" +import argparse +import ast +import contextlib +import functools +import glob +import io +import json +import os +import re +import subprocess +import sys + +# tools/ -> repo root is ONE level up. Guarded by TestModuleMove.test_repo_root_guard: +# a wrong parent count here silently re-points every repo-relative glob (it happened +# at the helper/ move). +_REPO_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +sys.path.insert(0, os.path.join(_REPO_ROOT, 'test', 'hil')) # for `from helper...` +from helper.hil_util import device_tests, dual_tests, host_test + +sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) # tools/, for build helpers +import build_utils +import build as build_py + +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 + - a ValueError, which every `except OSError` fail-open below would let through as a + traceback instead of a full matrix.""" + with open(path, encoding='utf-8', errors='replace') as f: + return f.read() + + +_NONCODE_RE = re.compile( + r'^(docs/|\.claude/|.*\.(md|rst)$|LICENSE)') +# Build-size metrics tooling. HIL axis ONLY: nothing on the rig runs any of it, and +# without this rule these paths are unclassified, so a metrics-only PR booked an +# exclusive full 30-board sweep to validate a script no board executes. +# The BUILD axis deliberately keeps its full-matrix answer: `tinyusb_metrics` runs +# tools/metrics.py as a build target (examples/CMakeLists.txt), and build_util.yml adds +# `--target tinyusb_metrics` to every metrics leg - a break in it fails the build, so a +# build has to exercise it. +_METRICS_RE = re.compile( + r'^(tools/metrics[^/]*\.py$|\.github/scripts/metrics_[^/]*\.py$)') +_FULL_RE = re.compile( + r'^(src/common/|src/osal/|src/tusb\.c$|src/tusb\.h$|src/tusb_option\.h$|' + r'test/hil/|\.github/workflows/build.*\.yml$|\.github/actions/|\.github/scripts/|' + r'tools/build\.py$|tools/cmake/|' + r'hw/bsp/(family_support\.cmake|board_api\.h|board\.c|ansi_escape\.h)$|' + r'examples/build_system/|examples/CMakeLists\.txt$|' + # board_test is HIL infrastructure, not a test: hil_test.py flashes it to park + # every board (variant boundary + end-of-board teardown), so every board depends on it + r'examples/device/board_test/)') + +# --no-renames: with rename detection git reports only a rename's destination, so code +# moved out of an HIL-relevant path would be classified by its new path alone +GIT_DIFF_ARGV = ['git', 'diff', '--no-renames', '--name-only'] + + +def test_role(test: str) -> str: + return test.split('/', 1)[0] # 'device' | 'dual' | 'host' + + +def board_roles(board: dict) -> set: + t = board.get('tests', {}) + roles = set() + if t.get('device'): + roles.add('device') + if t.get('host'): + roles.add('host') + if t.get('dual'): + roles.update(('device', 'host')) + for only in t.get('only', []): + r = test_role(only) + roles.update(('device', 'host') if r == 'dual' else (r,)) + return roles + + +def board_tests(board: dict) -> list: + """Every test this board would run today (mirrors hil_test.test_board's default).""" + t = board.get('tests', {}) + if 'only' in t: + run = list(t['only']) + else: + run = [] + if t.get('device'): + run += device_tests + if t.get('dual'): + run += dual_tests + if t.get('host'): + run += host_test + return [x for x in run if x not in t.get('skip', [])] + + +# cached: called per changed file x roster board, and the tree doesn't change mid-run +@functools.lru_cache(maxsize=None) +def board_family(board_name: str, repo_root: str): + hits = glob.glob(os.path.join(repo_root, 'hw/bsp/*/boards', board_name)) + return os.path.basename(os.path.dirname(os.path.dirname(hits[0]))) if hits else None + + +# `if (OPTION STREQUAL "1")` guards in family_support.cmake, and the option tokens +# a roster entry passes to the build (NAME=VALUE / -DNAME=VALUE) +_CM_IF_RE = re.compile(r'if\s*\(') +_CM_ELSE_RE = re.compile(r'else(if)?\s*\(') +_CM_ENDIF_RE = re.compile(r'endif\s*\(') +_CM_OPT_RE = re.compile(r'if\s*\(\s*\$?\{?([A-Za-z_]\w*)\}?\s+STREQUAL\s+"?1"?\s*\)') +_CM_PORT_RE = re.compile(r'src/portable/((?:[^/\s]+/)?[^/\s]+)/') +_FALSY = ('', '0', 'off', 'false', 'no') + + +@functools.lru_cache(maxsize=None) +def port_option_gates(repo_root: str) -> dict: + """port dir -> build options that compile it regardless of the board's family + file, e.g. {'analog/max3421': {'MAX3421_HOST'}} from family_support.cmake.""" + gates = {} + try: + text = _read(os.path.join(repo_root, 'hw/bsp/family_support.cmake')) + except OSError: + return gates + stack = [] # one entry per open if(): its option, or None + for line in text.splitlines(): + line = line.strip() + if _CM_IF_RE.match(line): + m = _CM_OPT_RE.match(line) + stack.append(m.group(1) if m else None) + elif _CM_ELSE_RE.match(line): + if stack: + stack[-1] = None # the guard doesn't hold in this branch + elif _CM_ENDIF_RE.match(line): + if stack: + stack.pop() + opts = {o for o in stack if o} + m = _CM_PORT_RE.search(line) + if opts and m: + gates.setdefault(m.group(1), set()).update(opts) + return gates + + +_CM_SET_RE = re.compile(r'set\s*\(\s*([A-Za-z_]\w*)\s+([^)\s]+)\s*\)') + + +# cached: called per changed portable file x roster board +@functools.lru_cache(maxsize=None) +def bsp_board_options(board_name: str, repo_root: str) -> frozenset: + """Build options a board turns on in its own BSP: `set( )` in + hw/bsp//boards//board.cmake, e.g. MAX3421_HOST on the espressif + and rp2040 max3421 boards. CMake only - HIL CI builds nothing with Make, so a + board.mk-only option (e.g. nrf5340dk's MAX3421_HOST) compiles no port here.""" + fam = board_family(board_name, repo_root) + if not fam: + return frozenset() + path = os.path.join(repo_root, 'hw/bsp', fam, 'boards', board_name, 'board.cmake') + try: + text = _read(path) + except OSError: + return frozenset() + out = set() + for line in text.splitlines(): + line = line.strip() + if line.startswith('#'): + continue + m = _CM_SET_RE.match(line) + if m and m.group(2).strip('"').lower() not in _FALSY: + out.add(m.group(1)) + return frozenset(out) + + +def board_options(board: dict, repo_root: str) -> set: + """Build options a board has truthy: the roster entry's build.args plus each + variant's defines (NAME=VALUE) and raw CFLAGS (-DNAME=VALUE), plus whatever its + own board.cmake sets (a board can enable a gated port without the roster saying so).""" + toks = list(board.get('build', {}).get('args', [])) + for v in board.get('variant', []): + toks += list(v.get('defines', [])) + toks += v.get('flags', '').split() + out = set(bsp_board_options(board['name'], repo_root)) + for t in toks: + name, _, val = (t[2:] if t.startswith('-D') else t).partition('=') + if name and val.strip().strip('"').lower() not in _FALSY: + out.add(name.strip()) + return out + + +@functools.lru_cache(maxsize=None) +def path_families(rel_dir: str, repo_root: str) -> set: + """Board families whose family.cmake (or espressif component CMakeLists) + references rel_dir at a directory boundary. CMake only, on every axis: CMake + is the first-class build system and Make follows it, so family.mk is never + read - a port wired up in family.mk alone (microchip/pic32mz) is built by no + 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')): + try: + if pat.search(_read(f)): + fams.add(os.path.relpath(f, bsp_root).split(os.sep, 1)[0]) + except OSError: + pass + return fams + + +def port_families(port_dir: str, repo_root: str) -> set: + # 'portable/', not 'src/portable/': family.cmake always spells the full literal + # path ('${TOP}/src/portable/...'), but espressif's component CMakeLists.txt + # assigns 'src' into a ${tusb_src} variable first (`${tusb_src}/portable/...`), + # so a leading 'src/' in the needle would never match there and silently drop + # espressif boards (see TestRealRosterPortFamilies). + return path_families('portable/' + port_dir, repo_root) + + +def mcu_families(path: str, repo_root: str) -> set: + """Families referencing a changed hw/mcu path: longest resolving dir prefix, + hw/mcu///... down to hw/mcu/.""" + parts = path.split('/') + for n in range(len(parts) - 1, 2, -1): + fams = path_families('/'.join(parts[:n]), repo_root) + if fams: + return fams + return set() + + +GET_DEPS_PATH = 'tools/get_deps.py' +_DEPS_DICTS = ('deps_mandatory', 'deps_optional') + + +def _deps_split(text: str): + """(module dump with the two dep-dict assigns removed, {dict name: entries}). + Parsed with ast, never exec'd: this runs on PR content.""" + mod = ast.parse(text) + dicts, rest = {}, [] + for node in mod.body: + if (isinstance(node, ast.Assign) and len(node.targets) == 1 and + isinstance(node.targets[0], ast.Name) and + node.targets[0].id in _DEPS_DICTS and isinstance(node.value, ast.Dict)): + dicts[node.targets[0].id] = ast.literal_eval(node.value) + else: + rest.append(node) + mod.body = rest + # annotate_fields=False keeps the dump readable-length; line numbers are not + # included unless asked for, so reformatting alone never reads as a logic change + return ast.dump(mod, annotate_fields=False), dicts + + +# Family tokens in tools/get_deps.py that name no hw/bsp directory. get_deps matches a +# token against a requested family name verbatim (`f in deps_optional[d][2].split()`), +# so a token like these matches nothing - a stale spelling in get_deps.py, not a +# selector bug, and out of scope to change here. Pinned so that any OTHER unresolvable +# token (real drift) falls open to the full matrix instead of silently selecting +# nothing, and so TestOrphanInvariant fails the day one is fixed or a new one appears. +# sam3x, samd21, samd51, same5x -> pre-rename spellings, listed alongside the current +# samd2x_l2x / samd5x_e5x / same7x in the same entry +# stm32l1, stm32l5 -> no hw/bsp family in the tree at all +_DEPS_ALIAS_TOKENS = frozenset({'sam3x', 'samd21', 'samd51', 'same5x', + 'stm32l1', 'stm32l5'}) + + +def get_deps_changed_families(base_text: str, head_text: str, repo_root: str): + """Families whose tools/get_deps.py dep entries changed between two versions of + the file, or None meaning 'cannot tell - use the full matrix'. + + None on: anything outside deps_mandatory/deps_optional differing (a logic change + to get_deps affects every family), a mandatory `'all'` entry changing, a token + that resolves to no family and is not a known alias, or text that will not parse. + Callers with no base content at all - `--diff-file` mode has no git and therefore + no merge-base blob - pass None themselves. + + An entry that is added, removed or edited contributes the family tokens of BOTH + sides (a removed entry has only a base side). The two dicts are diffed SEPARATELY: + merging them first would hide a move between deps_mandatory and deps_optional, + which changes which families fetch the dep even though the value is untouched.""" + try: + base_rest, base_d = _deps_split(base_text) + head_rest, head_d = _deps_split(head_text) + except (SyntaxError, ValueError, TypeError): + return None + if base_rest != head_rest: + return None + toks = set() + for name in _DEPS_DICTS: + base_x, head_x = base_d.get(name, {}), head_d.get(name, {}) + for key in set(base_x) | set(head_x): + if base_x.get(key) == head_x.get(key): + continue + for entry in (base_x.get(key), head_x.get(key)): + if entry and len(entry) > 2: + toks.update(str(entry[2]).split()) + if 'all' in toks: + return None + fams = set(all_bsp_families(repo_root)) + if toks - fams - _DEPS_ALIAS_TOKENS: + # a changed entry we cannot map to a family. "changed but unmappable" is NOT + # "nothing changed": reading it as the latter empties the entire build matrix + # for a dep bump, so fall open instead + return None + return toks & fams + + +_CLS_INC_RE = re.compile(r'#\s*include\s*[<"]class/([^/"<>]+)/([^"<>]+)[">]') + + +@functools.lru_cache(maxsize=None) +def class_include_edges(repo_root: str) -> dict: + """'/
' -> the other class dirs that include it. A class header + pulled in by a second class ships in every firmware enabling that second class: + src/class/midi/midi{,2}_{device,host}.h include class/audio/audio.h, and + net_device.h includes class/cdc/cdc.h. The class rule derives macros from the + directory name alone, so without this edge a change to the included header + selects only its own class's examples - and on a board that skips those (e.g. + metro_m4_express skips audio_test_freertos), nothing at all. + + Derived from the actual #include lines rather than a hand-written table so it + cannot rot when a class picks up or drops a cross-class include.""" + edges = {} + for f in sorted(glob.glob(os.path.join(repo_root, 'src/class/*/*.[ch]'))): + cls = os.path.basename(os.path.dirname(f)) + try: + text = _read(f) + except OSError: + continue + for inc_cls, inc_hdr in _CLS_INC_RE.findall(text): + if inc_cls != cls: + edges.setdefault(f'{inc_cls}/{inc_hdr}', set()).add(cls) + return edges + + +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.""" + if cls == 'net': + return [f'CFG_{prefix}_{m}' for m in NET_MACROS] + if cls == 'dfu': + if base.startswith('dfu_rt'): + return [f'CFG_{prefix}_DFU_RUNTIME'] + 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()}'] + + +# A define is OFF only when its value is a literal zero (0, 00, (0)), optionally +# followed by a comment. Anything else counts as ON - including a value this cannot +# evaluate, e.g. `#define CFG_TUH_MIDI CFG_TUH_DEVICE_MAX` (examples/host/midi_rx). +# Fail-open: reading such a define as OFF made midi_host.c select zero families and +# let a compile break merge green. +# +# A macro defined more than once is ON if ANY of its defines is non-zero, because +# the preprocessor branches are not evaluated here: uac2_speaker_fb defines +# CFG_TUD_HID 1 under `#if CFG_AUDIO_DEBUG` and 0 in the #else, and the default +# build (CFG_AUDIO_DEBUG defaults to 1) compiles the HID class in. Deciding on the +# LAST/only match found made that example invisible to CFG_TUD_HID changes. +_DEF_VALUE = r'^[ \t]*#[ \t]*define[ \t]+{}[ \t]+(\S[^\n]*?)[ \t]*$' +_DEF_ZERO_VALUE = re.compile(r'\(?\s*0+\s*\)?\s*(?://.*|/\*.*)?') + + +# Shared rule-recognition primitives. The two classifiers walk the same diff with +# different answers, but they must RECOGNISE the same things: one copy each, so a +# new naming convention cannot land in one walk and be missed by the other. +_PORT_PATH_RE = re.compile(r'src/portable/((?:[^/]+/)?[^/]+)/') + + +def _port_roles(base: str) -> set: + """Which USB role a src/portable file serves, from its name: dcd_*/ *_device is + the device-controller side, hcd_*/ *_host the host side, anything else (shared + headers, glue) both.""" + if re.match(r'(dcd_|.*_device)', base): + return {'device'} + if re.match(r'(hcd_|.*_host)', base): + return {'host'} + return {'device', 'host'} + + +def _class_roles(base: str) -> set: + """Same question for a src/class file: _device.[ch] / _host.[ch], + else both - the class's shared header ships in either role.""" + if re.search(r'_device\.[ch]$', base): + return {'device'} + if re.search(r'_host\.[ch]$', base): + return {'host'} + return {'device', 'host'} + + +def _config_enables(cfg_path: str, macros) -> bool: + try: + with open(cfg_path) as f: + text = f.read() + except OSError: + return False + for m in macros: + for value in re.findall(_DEF_VALUE.format(m), text, re.M): + if not _DEF_ZERO_VALUE.fullmatch(value): + return True + return False + + +def examples_enabling(pool, macros, repo_root: str) -> set: + """The 'role/name' entries of `pool` whose src/tusb_config.h turns any of + `macros` on. The pool differs per classifier (HIL test lists vs every example), + the question does not.""" + return {ex for ex in pool + if _config_enables(os.path.join(repo_root, 'examples', ex, 'src', + 'tusb_config.h'), macros)} + + +# cached: called per changed lib file, and the tree doesn't change mid-run +@functools.lru_cache(maxsize=None) +def lib_examples(lib_name: str, repo_root: str) -> set: + """Examples whose OWN examples///{CMakeLists.txt,Makefile} references + lib/ at a directory boundary (same boundary rule as path_families, so + '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'.""" + 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'): + try: + with open(os.path.join(repo_root, 'examples', ex, f)) as fh: + text = fh.read() + except OSError: + continue + if pat.search(text): + out.add(ex) + break + return out + + +def roster_only_tests(all_boards) -> set: + """Test paths that only appear in a roster board's tests.only list (e.g. + espressif boards), not in the shared device/dual/host_test lists.""" + out = set() + for b in all_boards: + out.update(b.get('tests', {}).get('only', [])) + return out + + +def class_examples(macros, role: str, repo_root: str, extra_tests: set) -> set: + """Tests (from role's + dual lists, plus roster-only-list tests of that role) + whose example config enables any macro.""" + return examples_enabling(role_tests({role}, extra_tests), macros, repo_root) + + +def role_tests(roles: set, extras: set) -> set: + """Every test for the given role(s): each role's own list + dual tests, + plus roster-only-list tests (extras) matching those roles or 'dual'.""" + pool = set(dual_tests) + for r in roles: + pool |= set(ALL_TESTS[r]) + pool |= {t for t in extras if test_role(t) in roles or test_role(t) == 'dual'} + return pool + + +class _Sel: + """Accumulates contributions. board->set(tests) plus 'all-board' markers.""" + def __init__(self): + self.full = False + self.by_board = {} # name -> set of tests, or 'all' + self.roles = set() # roles touched by any contribution + self.families = set() # bsp families touched (incl. off-rig ones: build-only consumers) + self.reasons = [] + + def add(self, boards, tests, reason): + """tests: 'all' or iterable of test paths.""" + self.reasons.append(reason) + for b in boards: + cur = self.by_board.get(b) + if tests == 'all' or cur == 'all': + self.by_board[b] = 'all' + else: + self.by_board[b] = (cur or set()) | set(tests) + + def force_full(self, reason): + self.full = True + self.reasons.append(reason) + + +def _classify_one(path, repo_root, roster_boards, extras: set, s: _Sel, + get_deps_families=None): + base = os.path.basename(path) + if _NONCODE_RE.match(path): + s.reasons.append(f'{path}: non-code, no contribution') + return + if _METRICS_RE.match(path): + s.reasons.append(f'{path}: build-size metrics tooling, no HIL contribution') + return + if _FULL_RE.match(path): + s.force_full(f'{path}: core/infra -> full matrix') + return + + if path == GET_DEPS_PATH: + if get_deps_families is None: + s.force_full(f'{path}: dep changes not resolvable -> full matrix') + return + if not get_deps_families: + s.reasons.append(f'{path}: no dep entry changed, no contribution') + return + fams = sorted(get_deps_families) + s.families.update(fams) + boards = [b['name'] for b in roster_boards + if board_family(b['name'], repo_root) in get_deps_families] + s.roles.update(('device', 'host')) + s.add(boards, 'all', f'{path}: dep entries changed -> families {fams} -> ' + f'boards {boards}') + return + + m = _PORT_PATH_RE.match(path) + if m: + port = m.group(1) + roles = _port_roles(base) + fams = port_families(port, repo_root) + if not fams: + # empty means empty (maintainer ruling), same reading as hw/mcu and as the + # build walk: no family's build references this port, so nothing compiles it + # and there is nothing to run. Forcing the full 30-board rig here bought no + # coverage at all - the build side selected zero families for the same path. + # Live for src/portable/template and the two microchip pic ports; + # TestPortFamiliesCoverage is the drift guard for a port that stops resolving. + s.reasons.append(f'{path}: port {port} maps to no board family, no contribution') + return + s.families.update(fams) + # a board can also pull the port in through a build option (e.g. MAX3421_HOST=1 + # from the roster on metro_m4_express, or from its own board.cmake), which its + # family file never names + gates = port_option_gates(repo_root).get(port, set()) + boards = [b['name'] for b in roster_boards + if (board_family(b['name'], repo_root) in fams or + (gates and board_options(b, repo_root) & gates)) and (board_roles(b) & roles)] + tests = role_tests(roles, extras) + s.roles.update(roles) + why = f'{path}: port {port} -> families {sorted(fams)}' + if gates: + why += f' + option {sorted(gates)}' + s.add(boards, tests, f'{why} -> boards {boards} ({"/".join(sorted(roles))})') + return + + m = re.match(r'src/class/([^/]+)/', path) + if m: + cls = m.group(1) + roles = _class_roles(base) + # this file's own class, plus any class whose headers include it + via = sorted(class_include_edges(repo_root).get(f'{cls}/{base}', ())) + + def macros(prefix): + return (class_macros(cls, base, prefix) + + [m2 for c in via for m2 in class_macros(c, '', prefix)]) + tests = set() + if 'device' in roles: + tests |= class_examples(macros('TUD'), 'device', repo_root, extras) + if 'host' in roles: + tests |= class_examples(macros('TUH'), 'host', repo_root, extras) + boards = [b['name'] for b in roster_boards if board_roles(b) & roles] + s.roles.update(roles) + why = f'{path}: class {cls}' + (f' (+ included by {via})' if via else '') + s.add(boards, tests, f'{why} -> {sorted(tests)} ({"/".join(sorted(roles))})') + return + + m = re.match(r'src/(device|host)/', path) + if m: + role = m.group(1) + boards = [b['name'] for b in roster_boards if role in board_roles(b)] + s.roles.add(role) + s.add(boards, role_tests({role}, extras), f'{path}: core {role} stack -> all {role} tests') + return + + m = re.match(r'hw/bsp/([^/]+)/(?:boards/([^/]+)/)?', path) + if m: + fam, brd = m.group(1), m.group(2) + s.families.add(fam) + if brd: + boards = [b['name'] for b in roster_boards if b['name'] == brd] + why = f'{path}: bsp board {brd}' + else: + boards = [b['name'] for b in roster_boards + if board_family(b['name'], repo_root) == fam] + why = f'{path}: bsp family {fam}' + s.roles.update(('device', 'host')) + s.add(boards, 'all', f'{why} -> boards {boards}') + return + + if re.match(r'hw/mcu/', path): + fams = mcu_families(path, repo_root) + if not fams: + # empty means empty (maintainer ruling): if no family's build references + # the path, no build consumes the change - there is nothing to compile, + # so there is nothing to run either. TestOrphanInvariant's + # test_tracked_mcu_vendors_resolve is the drift guard: a real vendor dir + # that stops resolving fails pre-commit instead of silently vanishing + s.reasons.append(f'{path}: hw/mcu path resolves to no family, no contribution') + return + s.families.update(fams) + boards = [b['name'] for b in roster_boards + if board_family(b['name'], repo_root) in fams] + s.roles.update(('device', 'host')) + s.add(boards, 'all', f'{path}: mcu dir -> families {sorted(fams)} -> boards {boards}') + return + + m = re.match(r'lib/([^/]+)/', path) + if m: + lib = m.group(1) + # only the tests whose example builds the lib, and only those the rig runs + tests = {e for e in lib_examples(lib, repo_root) + if any(e in pool for pool in ALL_TESTS.values()) or e in extras} + if not tests: + s.reasons.append(f'{path}: lib {lib} used by no HIL test, no contribution') + return + roles = set() + for test in tests: + r = test_role(test) + roles.update(('device', 'host') if r == 'dual' else (r,)) + boards = [b['name'] for b in roster_boards] + s.roles.update(roles) + s.add(boards, sorted(tests), f'{path}: lib {lib} -> {sorted(tests)} on all boards') + return + + m = _BUILD_EX_RE.match(path) + if m: + if m.group(1) not in _HIL_EX_ROLES: + # examples/typec: the build matrix compiles it, nothing on the rig runs it + s.reasons.append(f'{path}: {m.group(1)} example, no HIL contribution') + return + test = f'{m.group(1)}/{m.group(2)}' + known = any(test in pool for pool in ALL_TESTS.values()) or test in extras + if known: + boards = [b['name'] for b in roster_boards] + role = test_role(test) + s.roles.update(('device', 'host') if role == 'dual' else (role,)) + s.add(boards, [test], f'{path}: example -> {test} on all boards') + else: + s.reasons.append(f'{path}: example not in HIL lists, no contribution') + return + + s.force_full(f'{path}: unclassified -> full matrix') + + +def classify(changed_files, repo_root, rosters, get_deps_families=None): + all_boards = [] + seen = set() + for _, boards in rosters: + for b in boards: + if b['name'] not in seen: + seen.add(b['name']) + all_boards.append(b) + + extras = roster_only_tests(all_boards) + s = _Sel() + # no early exit once full: keep classifying so `families` still reports every + # family the diff touches (build-only consumers need it). Nothing after the first + # force_full can change full/boards/args - the full branch below ignores by_board. + for path in changed_files: + _classify_one(path, repo_root, all_boards, extras, s, get_deps_families) + + if s.full: + return {'full': True, 'boards': {b['name']: 'all' for b in all_boards}, + 'families': sorted(s.families), 'reasons': s.reasons} + + # role pruning: single-role selections drop the other role's tests and boards + by_name = {b['name']: b for b in all_boards} + out = {} + for name, tests in s.by_board.items(): + allowed = board_tests(by_name[name]) + if tests == 'all': + kept = list(allowed) + else: + kept = [t for t in allowed if t in tests] + if s.roles and s.roles != {'device', 'host'}: + role = next(iter(s.roles)) + kept = [t for t in kept if test_role(t) in (role, 'dual')] + if kept: + out[name] = 'all' if set(kept) == set(allowed) else sorted(kept) + return {'full': False, 'boards': out, 'families': sorted(s.families), + 'reasons': s.reasons} + + +def _board_args(name, chosen) -> list: + parts = [f'-b {name}'] + if chosen != 'all': + parts.append(f'-bt {name}:{",".join(chosen)}') + return parts + + +def hil_examples(sel, rosters): + """{board: examples hil-build must produce}: the board's selected tests plus + device/board_test, which hil_test.py flashes to park at every variant + boundary and at end-of-board teardown. Emitted for full selections too - the + HIL example universe is a fraction of the tree regardless of the diff.""" + by_name = {} + for _, boards in rosters: + for b in boards: + by_name.setdefault(b['name'], []).append(b) + if sel['full']: + chosen = {n: 'all' for n in by_name} + else: + chosen = sel['boards'] + out = {} + for name, tests in chosen.items(): + if tests == 'all': + # a board named by two rosters (rig migration, or shared between rigs) + # may run different tests on each: union them. Superset firmware costs a + # build; a missing image fails the run on whichever rig lost the toss. + run = set().union(*(board_tests(b) for b in by_name[name])) + else: + run = set(tests) + out[name] = sorted(run | {'device/board_test'}) + return out + + +def selection_args(sel, rosters): + """hil_test.py args per config. Empty means either 'full matrix' or 'nothing + selected' - callers must read sel['full'] to tell them apart.""" + args = {} + for cfg_path, boards in rosters: + parts = [] + if not sel['full']: + for b in boards: + chosen = sel['boards'].get(b['name']) + if chosen is not None: + parts += _board_args(b['name'], chosen) + args[os.path.basename(cfg_path)] = ' '.join(parts) + return args + + +def selection_args_by_flasher(sel, rosters): + """{config: {flasher name: args}}. CI runs one rig as several jobs split by + flasher (esptool vs the rest); each must gate on its own subset, otherwise the + other leg runs a filter matching zero boards and reports a vacuous green.""" + out = {} + for cfg_path, boards in rosters: + per = {} + if not sel['full']: + for b in boards: + chosen = sel['boards'].get(b['name']) + if chosen is None: + continue + per.setdefault(b.get('flasher', {}).get('name', ''), []).extend( + _board_args(b['name'], chosen)) + out[os.path.basename(cfg_path)] = {f: ' '.join(p) for f, p in per.items()} + return out + + +def merge_base(base, repo_root): + return subprocess.run(['git', 'merge-base', 'HEAD', base], cwd=repo_root, + capture_output=True, text=True, check=True).stdout.strip() + + +def git_show(spec, repo_root): + return subprocess.run(['git', 'show', spec], cwd=repo_root, + capture_output=True, text=True, check=True).stdout + + +def changed_files_from_git(base, repo_root): + diff = subprocess.run(GIT_DIFF_ARGV + [f'{merge_base(base, repo_root)}..HEAD'], + cwd=repo_root, capture_output=True, text=True, check=True).stdout + return [l for l in diff.splitlines() if l.strip()] + + +def get_deps_families_from_git(base, repo_root): + """The changed dep entries' families for a --base run, or None (-> full matrix) + if git cannot produce both sides of tools/get_deps.py.""" + try: + mb = merge_base(base, repo_root) + return get_deps_changed_families(git_show(f'{mb}:{GET_DEPS_PATH}', repo_root), + git_show(f'HEAD:{GET_DEPS_PATH}', repo_root), + repo_root) + except (subprocess.CalledProcessError, OSError) as e: + print(f'ci_select: {GET_DEPS_PATH}: base content unreadable ({e})', file=sys.stderr) + return None + + +def main(): + ap = argparse.ArgumentParser(description=__doc__) + g = ap.add_mutually_exclusive_group(required=True) + g.add_argument('--base', help='git ref to diff against (merge-base..HEAD)') + g.add_argument('--diff-file', help='newline-separated changed-file list') + ap.add_argument('configs', nargs='*', help='rig roster JSON file(s); omit for the build view alone') + a = ap.parse_args() + + repo_root = _REPO_ROOT + rosters = [] + for c in a.configs: + with open(c) as f: + rosters.append((c, json.load(f)['boards'])) + + files = (_read(a.diff_file).splitlines() if a.diff_file + else changed_files_from_git(a.base, repo_root)) + files = [f for f in files if f.strip()] + + # --diff-file has no git and so no base content: the rule falls open to full + gd = (get_deps_families_from_git(a.base, repo_root) + if a.base and GET_DEPS_PATH in files else None) + + s = classify(files, repo_root, rosters, gd) + s['args'] = selection_args(s, rosters) + s['args_flasher'] = selection_args_by_flasher(s, rosters) + if rosters: + s['hil_examples'] = hil_examples(s, rosters) + s['build'] = classify_build(files, repo_root, gd) + for r in s['build']['reasons']: + print(f'ci_select[build]: {r}', file=sys.stderr) + for r in s['reasons']: + print(f'ci_select: {r}', file=sys.stderr) + print(json.dumps(s)) + + +# ------------------------------------------------------------- +# Build-axis classifier (spec rule table, docs/superpowers/specs/ +# 2026-08-19-ci-build-family-filter-design.md). Independent of the HIL +# classifier: same diff, second walk, its own fail-open. +# ------------------------------------------------------------- +# Both walks recognise an example path with the SAME regex, so a role can never be +# known to one walk and unclassified (-> full matrix) to the other. What differs is the +# answer: the rig runs device/host/dual tests, while the build matrix also compiles +# examples/typec, which nothing on the rig runs. +_EX_ROLES = ('device', 'dual', 'host', 'typec') +_HIL_EX_ROLES = ('device', 'host', 'dual') +_BUILD_EX_RE = re.compile(r'examples/(%s)/([^/]+)/' % '|'.join(_EX_ROLES)) + + +@functools.lru_cache(maxsize=None) +def all_examples(repo_root: str) -> tuple: + """Every examples// with a CMakeLists.txt, as 'role/name'.""" + out = [] + for role in _EX_ROLES: + for d in sorted(glob.glob(os.path.join(repo_root, 'examples', role, '*/'))): + if os.path.isfile(os.path.join(d, 'CMakeLists.txt')): + out.append(f'{role}/{os.path.basename(d.rstrip(os.sep))}') + return tuple(out) + + +def role_examples(repo_root: str, roles) -> set: + want = set(roles) + return {e for e in all_examples(repo_root) if e.split('/', 1)[0] in want} + + +@functools.lru_cache(maxsize=None) +def all_bsp_families(repo_root: str) -> tuple: + return tuple(sorted(d for d in os.listdir(os.path.join(repo_root, 'hw/bsp')) + if os.path.isdir(os.path.join(repo_root, 'hw/bsp', d)))) + + +def _build_class_examples(cls: str, base: str, roles: set, repo_root: str) -> set: + """Examples (all 46, not the HIL lists) whose tusb_config.h enables the class's + macros for the given roles, plus classes that #include the changed header.""" + via = sorted(class_include_edges(repo_root).get(f'{cls}/{base}', ())) + out = set() + for prefix, role in (('TUD', 'device'), ('TUH', 'host')): + if role not in roles: + continue + macros = class_macros(cls, base, prefix) + \ + [m for c in via for m in class_macros(c, '', prefix)] + out |= examples_enabling(all_examples(repo_root), macros, repo_root) + return out + + +class _BSel: + """family -> set(examples) | 'all', unioned per family.""" + def __init__(self): + self.full = False + self.fam_ex = {} + self.reasons = [] + + def add(self, fams, examples, reason): + self.reasons.append(reason) + for f in fams: + cur = self.fam_ex.get(f) + if examples == 'all' or cur == 'all': + self.fam_ex[f] = 'all' + else: + self.fam_ex[f] = (cur or set()) | set(examples) + + def force_full(self, reason): + self.full = True + self.reasons.append(reason) + + +def _classify_build_one(path, repo_root, s: _BSel, get_deps_families=None): + base = os.path.basename(path) + if _NONCODE_RE.match(path): # rule 1 + return + if re.match(r'test/hil/', path): # rule 2 + s.reasons.append(f'{path}: HIL harness, no build contribution') + return + if path == GET_DEPS_PATH: # get_deps rule + if get_deps_families is None: + s.force_full(f'{path}: dep changes not resolvable -> full build matrix') + return + if not get_deps_families: + s.reasons.append(f'{path}: no dep entry changed, no contribution') + return + fams = sorted(get_deps_families) + s.add(fams, 'all', f'{path}: dep entries changed -> families {fams}') + return + m = _PORT_PATH_RE.match(path) + if m: # rules 3-5 + port = m.group(1) + fams = port_families(port, repo_root) + roles = _port_roles(base) + exs = 'all' if roles == {'device', 'host'} else \ + role_examples(repo_root, tuple(roles) + ('dual',)) + s.add(fams, exs, f'{path}: port {port} -> families {sorted(fams)}') + return + if re.match(r'hw/bsp/[^/]+/', path): # rule 6 + fam = path.split('/')[2] + s.add({fam}, 'all', f'{path}: bsp family {fam}') + return + if re.match(r'hw/mcu/', path): # rule 7 + fams = mcu_families(path, repo_root) + if not fams: + # empty means empty, same reading as the HIL walk: no family's build + # references the path, so no build compiles it + s.reasons.append(f'{path}: hw/mcu path resolves to no family, no contribution') + return + s.add(fams, 'all', f'{path}: mcu -> families {sorted(fams)}') + return + m = re.match(r'src/class/([^/]+)/', path) + if m: # rules 8-10 + cls = m.group(1) + roles = _class_roles(base) + exs = _build_class_examples(cls, base, roles, repo_root) + if not exs: + # Empty means empty - maintainer decision. No example config enables this + # class, so no build exercises it and + # nothing is selected. The file IS still parsed by every full build + # (src/CMakeLists.txt, src/tinyusb.mk list class sources unconditionally, + # the CFG_ guard sits inside), so a break outside the guard surfaces on the + # next master push - the accepted safety net. + s.reasons.append(f'{path}: class {cls} enabled by no example config, ' + f'no contribution') + return + s.add(all_bsp_families(repo_root), exs, + f'{path}: class {cls} -> {sorted(exs)}') + return + m = re.match(r'src/(device|host)/', path) + if m: # rules 11-12 + role = m.group(1) + s.add(all_bsp_families(repo_root), role_examples(repo_root, (role, 'dual')), + f'{path}: core {role} stack') + return + m = _BUILD_EX_RE.match(path) + if m: # rules 13-14 + ex = f'{m.group(1)}/{m.group(2)}' + if ex in all_examples(repo_root): + s.add(all_bsp_families(repo_root), {ex}, f'{path}: example {ex}') + else: + # a deleted example builds nothing; removing it from the role + # CMakeLists (rule 15) is what forces the full matrix + s.reasons.append(f'{path}: not an example dir, no build contribution') + return + m = re.match(r'lib/([^/]+)/', path) + if m: # lib rule + lib = m.group(1) + exs = lib_examples(lib, repo_root) + if not exs: + # empty means empty: no example's build pulls this lib in, so no build + # compiles it (lib/SEGGER_RTT is only reached through LOGGER=rtt, which + # no CI build sets) + s.reasons.append(f'{path}: lib {lib} built by no example, no contribution') + return + s.add(all_bsp_families(repo_root), exs, f'{path}: lib {lib} -> {sorted(exs)}') + return + s.force_full(f'{path}: unclassified -> full build matrix') # rules 15-17 + + +@contextlib.contextmanager +def _in_repo(repo_root): + """build_utils/build.py use repo-relative paths; scope a chdir around them. + get_family_boards also prints on an empty family - swallow stdout so the + selector's machine-read JSON stays clean (diagnostics belong on stderr).""" + old = os.getcwd() + os.chdir(repo_root) + try: + with contextlib.redirect_stdout(io.StringIO()): + yield + finally: + os.chdir(old) + + +def _prune_buildable(fams, fam_ex, repo_root): + """Intersect each family's selection with what the family can build at all + (build_utils.skip_example - the same skip.txt/only.txt data CMake's + family_filter reads). + + ANY board of the family counts, not just the one GHA's --one-first picks: + CircleCI's cmake legs build every board of a family, so an example gated to a + single board (only.txt board:mimxrt1060_evk) would otherwise lose ALL compile + coverage exactly when a PR touches it. get_family_boards(.., False, False) is + that full list, with the same CI skip lists the build jobs apply. + + EITHER build system counts too. This one list gates CircleCI's make legs as well + as its cmake ones, and the two answer different questions (build_utils.skip_example): + examples/device/dfu carries `mcu:BCM2835` in skip.txt, which the cmake FAMILY_MCUS + union applies to every broadcom_64bit board while the make scrape applies it to + none - asking cmake alone drops the only aarch64-gcc family in the matrix and + `build-make-aarch64-gcc` stops compiling dfu at all.""" + out_fams, out_ex, reasons = [], {}, [] + allex = list(all_examples(repo_root)) + with _in_repo(repo_root): + for fam in fams: + if not os.path.isdir(os.path.join(repo_root, 'hw/bsp', fam, 'boards')): + # a PR that deletes or renames hw/bsp/ still names it in the + # diff (rule 6); the family builds nothing now, and get_family_boards + # would raise FileNotFoundError out of the whole selector + reasons.append(f'{fam}: family dir gone from tree, dropped') + continue + try: + boards = build_py.get_family_boards(fam, False, False) + except OSError as e: # belt and braces: never traceback here + reasons.append(f'{fam}: boards unreadable ({e}), dropped') + continue + if not boards: + out_fams.append(fam) # unknown layout: keep unfiltered + continue + # what this family's build path can even see, asked the same way for + # every family. build.py's espressif branch builds get_examples('espressif') + # only (the *_freertos examples plus a short extra list); keeping the family + # for anything else spins up CI's most expensive leg to skip every example + # it was given. Identical to the unfiltered list on all 81 other families. + pool = set(build_py.get_examples(fam)) + try: + buildable = [e for e in allex if e in pool and + any(not build_utils.skip_example(e, b) or + not build_utils.skip_example(e, b, (), 'make') + for b in boards)] + except OSError as e: + # a family mid-bring-up (boards/ but no family.cmake/family.mk yet) + # reads as unbuildable to the scrape; keep it rather than tracebacking + # out of the selector and losing the scoping for the whole PR + reasons.append(f'{fam}: mcu scrape unreadable ({e}), kept unfiltered') + out_fams.append(fam) + continue + want = fam_ex.get(fam) + have = set(buildable) + kept = buildable if want is None else [e for e in want if e in have] + if not kept: + continue # this diff builds nothing for this family + out_fams.append(fam) + if set(kept) != set(buildable): + out_ex[fam] = kept + return out_fams, out_ex, reasons + + +def classify_build(changed_files, repo_root, get_deps_families=None): + s = _BSel() + for p in changed_files: + _classify_build_one(p, repo_root, s, get_deps_families) + if s.full: + return {'full': True, 'families': list(all_bsp_families(repo_root)), + 'family_examples': {}, 'reasons': s.reasons} + fams = sorted(s.fam_ex) + fam_ex = {f: sorted(e) for f, e in s.fam_ex.items() if e != 'all'} + fams, fam_ex, pruned = _prune_buildable(fams, fam_ex, repo_root) + s.reasons += pruned + return {'full': False, 'families': fams, 'family_examples': fam_ex, + 'reasons': s.reasons} + + +if __name__ == '__main__': + main() -- 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 'tools/ci_select.py') 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 From a408a8e9af4a043202f79a2b8e20d229093148e5 Mon Sep 17 00:00:00 2001 From: hathach Date: Fri, 21 Aug 2026 14:23:40 +0700 Subject: hil: express a board's always-on defines as a variant, dropping build.args The roster had two ways to pass a cmake -D to a board's build: `build.args`, applied to every variant, and `variant[].defines`, applied to one. They did the same thing, and only metro_m4_express used the first - for MAX3421_HOST=1, which is what makes it the one rig board that compiles hcd_max3421.c. A board whose define is always on now carries a single variant named after itself, which is exactly the shape `board.get('variant') or [{'name': name, 'flags': ''}]` already synthesises everywhere - so the build dir, the HIL report row and the variant-boundary handling are unchanged. raspberry_pi_pico has used that shape for its flags all along. Removes the BuildCfg type and the parallel code path from all four consumers: hil_test.build_board, hil_pool_check's two builders, hil_ci_set_matrix and ci_select.board_options. Verified: the hil-build matrix entry is byte-identical (`-b metro_m4_express -DMAX3421_HOST=1`), hil_test's build command is unchanged, ci_select still selects the board for a max3421 diff with MAX3421_HOST in its options, and a real build of dual/host_info_to_device_cdc and host/cdc_msc_hid on that board still compiles hcd_max3421.c. --- .github/scripts/hil_ci_set_matrix.py | 2 -- test/hil/helper/hil_pool_check.py | 4 +--- test/hil/hil_test.py | 14 ++++---------- test/hil/test/test_ci_select.py | 10 +++++----- test/hil/tinyusb.json | 13 ++++++++----- tools/ci_select.py | 10 ++++++---- 6 files changed, 24 insertions(+), 29 deletions(-) (limited to 'tools/ci_select.py') diff --git a/.github/scripts/hil_ci_set_matrix.py b/.github/scripts/hil_ci_set_matrix.py index 396c4175a..bf50061dd 100644 --- a/.github/scripts/hil_ci_set_matrix.py +++ b/.github/scripts/hil_ci_set_matrix.py @@ -103,8 +103,6 @@ def main(): f'hil-build-esp jobs in .github/workflows/build.yml') build_board = f'-b {name}' - if 'build' in board and 'args' in board['build']: - build_board += ' ' + ' '.join(f'-D{a}' for a in board['build']['args']) # PR selection: build only the examples this board will run (its test # list plus device/board_test, the parking firmware) - tools/build.py -e. diff --git a/test/hil/helper/hil_pool_check.py b/test/hil/helper/hil_pool_check.py index d926bbe3d..179a417ed 100644 --- a/test/hil/helper/hil_pool_check.py +++ b/test/hil/helper/hil_pool_check.py @@ -433,7 +433,7 @@ def build_example(board: dict, variant: str, example: str) -> int: cmd = ['idf.py', '-C', f'examples/{example}', '-B', f'cmake-build/cmake-build-{vcfg["name"]}/{example}', '-G', 'Ninja', f'-DBOARD={name}', 'build'] - for d in board.get('build', {}).get('args', []) + vcfg.get('defines', []): + for d in vcfg.get('defines', []): cmd.insert(-1, f'-D{d}') if vcfg.get('flags'): cmd.insert(-1, f'-DCFLAGS_CLI={vcfg["flags"]}') @@ -446,8 +446,6 @@ def build_example(board: dict, variant: str, example: str) -> int: cmd = [sys.executable, str(hil_util.TINYUSB_ROOT / 'tools' / 'build.py'), '-b', name, '-T', Path(example).name, '-j', str(max(1, (os.cpu_count() or _jobs) // _jobs))] - for d in board.get('build', {}).get('args', []): - cmd += ['-D', d] if vcfg['name'] != name: cmd += ['--build-name', vcfg['name']] for d in vcfg.get('defines', []): diff --git a/test/hil/hil_test.py b/test/hil/hil_test.py index 174251343..fcd7c7e6f 100755 --- a/test/hil/hil_test.py +++ b/test/hil/hil_test.py @@ -194,10 +194,6 @@ class TestsCfg(TypedDict, total=False): dev_attached: list[AttachedDevCfg] -class BuildCfg(TypedDict, total=False): - args: list[str] - - class VariantCfg(TypedDict, total=False): name: str # build dir (cmake-build-) and HIL report row flags: str # raw CFLAGS, e.g. "-DCFG_TUD_DWC2_DMA_ENABLE=1" @@ -209,7 +205,9 @@ class Board(TypedDict): uid: str tests: TestsCfg flasher: FlasherCfg - build: NotRequired[BuildCfg] + # every build knob lives here, including a board's always-on defines: a board that + # needs one carries a single variant named after itself (metro_m4_express / + # MAX3421_HOST=1), which is exactly what the `or [...]` default below synthesises variant: NotRequired[list[VariantCfg]] toolchain: NotRequired[str] # CI build bucket override, e.g. "riscv-gcc" (consumed by hil_ci_set_matrix.py) @@ -1670,21 +1668,17 @@ def test_example(board: Board, variant: str, example: str) -> tuple[int, str, st def build_board(board: Board) -> tuple[str, int]: """Build firmware for this board via tools/build.py. - Honors board config's variant list and build.args defines. + Honors board config's variant list (name, defines, flags). Output goes to cmake-build/cmake-build-/ (tools/build.py layout). Unbounded on purpose: --build is a local convenience (no CI workflow passes it), so the developer watching the build is the timeout.""" name = board['name'] - bcfg = cast(BuildCfg, board.get('build', {})) - extra_defs = bcfg.get('args', []) variants = board.get('variant') or [{'name': name, 'flags': ''}] failed = 0 for v in variants: cmd = [sys.executable, str(hil_util.TINYUSB_ROOT / 'tools' / 'build.py'), '-b', name] - for d in extra_defs: - cmd += ['-D', d] if v['name'] != name: cmd += ['--build-name', v['name']] for d in v.get('defines', []): diff --git a/test/hil/test/test_ci_select.py b/test/hil/test/test_ci_select.py index 031e8e287..f5c64ac1c 100644 --- a/test/hil/test/test_ci_select.py +++ b/test/hil/test/test_ci_select.py @@ -329,7 +329,7 @@ class TestOptionGatedPort(unittest.TestCase): # host-side option board (max3421 as host controller), off any max3421 family OPT_ROSTER = [('test/hil/opt.json', [ {'name': 'fake_dual_board', 'uid': 'o1', 'flasher': {'name': 'jlink'}, - 'build': {'args': ['MAX3421_HOST=1']}, + 'variant': [{'name': 'fake_dual_board', 'defines': ['MAX3421_HOST=1']}], 'tests': {'device': True, 'host': False, 'dual': True}}, {'name': 'fake_host_board', 'uid': 'o2', 'flasher': {'name': 'jlink'}, 'variant': [{'name': 'fake_host_board', 'flags': '-DMAX3421_HOST=1'}], @@ -346,10 +346,10 @@ class TestOptionGatedPort(unittest.TestCase): for board in boards: self.assertIn(board, s['boards']) - def test_option_selects_via_args_defines_and_flags(self): + def test_option_selects_via_defines_and_flags(self): s = ci_select.classify(['src/portable/analog/max3421/hcd_max3421.c'], REPO, self.OPT_ROSTER) self.assertFalse(s['full']) - self.assertIn('fake_dual_board', s['boards']) # build.args + self.assertIn('fake_dual_board', s['boards']) # variant defines self.assertIn('fake_host_board', s['boards']) # variant flags self.assertNotIn('fake_off_board', s['boards']) # variant defines, but =0 @@ -1762,7 +1762,7 @@ class TestBuildPyExampleFilter(unittest.TestCase): {'tinyusb_metrics', 'cdc_msc', 'cdc_msc-membrowse-upload'}) def test_build_defines_reach_the_example_filter(self): - # metro_m4_express gets MAX3421_HOST=1 from the roster build args, never + # metro_m4_express gets MAX3421_HOST=1 from its roster variant, never # from its BSP: without threading them through, -e drops the rig's only # MAX3421 dual firmware that --target all used to build self.assertIsNone(self.build.resolve_example_target_groups( @@ -1934,7 +1934,7 @@ class TestSkipExampleMirrorsFamilyFilter(unittest.TestCase): def test_build_define_enables_max3421_only_list(self): # family_support.cmake:940 appends MAX3421 to FAMILY_MCUS when # MAX3421_HOST=1; on metro_m4_express that define comes from the roster - # build args, so skip_example has to be told about it + # variant defines, so skip_example has to be told about it ex = 'dual/host_info_to_device_cdc' self.assertTrue(self.build_utils.skip_example(ex, 'metro_m4_express')) self.assertFalse(self.build_utils.skip_example(ex, 'metro_m4_express', diff --git a/test/hil/tinyusb.json b/test/hil/tinyusb.json index 6f552f126..8fd4683a4 100644 --- a/test/hil/tinyusb.json +++ b/test/hil/tinyusb.json @@ -157,11 +157,14 @@ { "name": "metro_m4_express", "uid": "9995AD485337433231202020FF100A34", - "build": { - "args": [ - "MAX3421_HOST=1" - ] - }, + "variant": [ + { + "name": "metro_m4_express", + "defines": [ + "MAX3421_HOST=1" + ] + } + ], "tests": { "device": true, "host": false, diff --git a/tools/ci_select.py b/tools/ci_select.py index cd63899c1..ced3bbbc0 100755 --- a/tools/ci_select.py +++ b/tools/ci_select.py @@ -188,10 +188,12 @@ def bsp_board_options(board_name: str, repo_root: str) -> frozenset: def board_options(board: dict, repo_root: str) -> set: - """Build options a board has truthy: the roster entry's build.args plus each - variant's defines (NAME=VALUE) and raw CFLAGS (-DNAME=VALUE), plus whatever its - own board.cmake sets (a board can enable a gated port without the roster saying so).""" - toks = list(board.get('build', {}).get('args', [])) + """Build options a board has truthy: each variant's defines (NAME=VALUE) and raw + CFLAGS (-DNAME=VALUE), plus whatever its own board.cmake sets (a board can enable a + gated port without the roster saying so). A board whose option is always on carries + a single variant named after itself - metro_m4_express and MAX3421_HOST=1, which is + what makes it the one rig board that compiles hcd_max3421.c.""" + toks = [] for v in board.get('variant', []): toks += list(v.get('defines', [])) toks += v.get('flags', '').split() -- cgit v1.3.1 From 03a329eeda09e073d7de9be84df55d65392e4013 Mon Sep 17 00:00:00 2001 From: hathach Date: Fri, 21 Aug 2026 22:17:51 +0700 Subject: ci_select: classify the 254 files that were reaching rule 17 Rule 17 (unclassified -> full on both axes) is the fail-open net for paths nobody anticipated, and it must stay that way: a wrong `full` costs runner minutes and is visible in the run, a wrong `empty` costs a merged regression and is invisible. But nothing in the tree should REACH it, and 254 tracked files did. The cost was real. PR #3842 changed a skill, a README and .gitignore; .gitignore matched no rule, so both axes went full and 74 cmake legs span up runners to do checkout + toolchain + get_deps before skipping the build, plus the whole 30-board rig. Three changes, none of which touch rule 17 itself: 1. _META_RE - repo metadata and tooling no Build step reads: .gitignore, .gitattributes, .clang-format, .codespellrc, .pre-commit-config.yaml, .readthedocs.yaml, .PVS-Studio/, .idea/, sonar-project.properties, the packaging manifests, CMakePresets, udev rules, test/{fuzz,unit-test} (their own jobs build those), the non-build .github/ files, and the tools/*.py scripts no build invokes. Deliberately NOT included, and still full: .circleci/**, .github/workflows/build*.yml, .github/actions/**, .github/scripts/**. The line is "does a Build step read this", not "is it source". 2. Rules 15 and 16 now match what they already claimed. Row 15 names examples//CMakeLists.txt and the regex never had it; row 16 says tools/build*.py but anchored tools/build\.py$. Both got the right answer only because rule 17 caught them on the way past. Also names their siblings - family_support.mk, family_rules.mk, src/CMakeLists.txt, src/tinyusb.mk - and .circleci/**, which generates the whole CircleCI matrix and was in no row at all. 3. src/typec/** gets row 12b. It is listed unconditionally by both build systems but its body is `#if CFG_TUC_ENABLED`, which only examples/typec/power_delivery sets - the same shape as the class rule, so the same answer: the examples that enable it (stm32g4 and stm32u5 after the buildability prune), and nothing on the rig, which runs no typec test. It was force-fulling 82 families and all 30 boards. TestNoTrackedFileIsUnclassified walks every tracked file and asserts none reaches rule 17, on both axes - 254 -> 0. Verified it fails when a new unclassified path appears. That turns 17 into what it should be: unreachable for anything in the tree, so it fires only for genuinely new shapes, and the author is told to write the row rather than letting the fall-through pick an answer for them. test_full_paths used sonar-project.properties as its stand-in for "unclassified"; that is now metadata, so the case moved to the new test_repo_metadata_is_not_a_build_input, with test_the_build_machinery_is_still_full pinning the other side of the line. --- .../2026-08-19-ci-build-family-filter-design.md | 48 ++++++------- test/hil/test/test_ci_select.py | 62 ++++++++++++++++- tools/ci_select.py | 79 ++++++++++++++++++++-- 3 files changed, 160 insertions(+), 29 deletions(-) (limited to 'tools/ci_select.py') 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 8f77dc50a..b10f5b4ae 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 @@ -46,29 +46,31 @@ never inflates one axis with another's breadth. `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) | +| # | Changed path | Build families | Build examples | HIL boards → tests | +| --- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------- | ----------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------- | +| 1 | `docs/`, `.claude/`, `*.md`, `*.rst`, `LICENSE` | — | — | — | +| 1b | `.gitignore`, `.clang-format`, `.idea/**`, `test/{fuzz,unit-test}/**`, non-build `.github/**`, packaging manifests | — | — | — | +| 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 | +| 12b | `src/typec/**` | `ALL` | examples enabling `CFG_TUC_ENABLED` | — (no rig board runs a typec test) | +| 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,build_utils,ci_select}.py`, `tools/cmake/**`, `src/CMakeLists.txt`, `src/tinyusb.mk`, `hw/bsp/{family_support.{cmake,mk},family_rules.mk,zephyr_board_aliases.cmake,board.c,board_api.h,ansi_escape.h}`, `.github/**`, `.circleci/**` | `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 (no tracked file reaches this — TestNoTrackedFileIsUnclassified) | `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. diff --git a/test/hil/test/test_ci_select.py b/test/hil/test/test_ci_select.py index 8f1841531..66b20b2e4 100644 --- a/test/hil/test/test_ci_select.py +++ b/test/hil/test/test_ci_select.py @@ -840,6 +840,45 @@ class TestRostersDoNotOverlap(unittest.TestCase): seen[b['name']] = b.get('tests') +class TestNoTrackedFileIsUnclassified(unittest.TestCase): + """Rule 17 (unclassified -> full on both axes) is the fail-open net for paths nobody + anticipated. It must stay that way - a wrong `full` costs runner minutes and is + visible in the run, a wrong `empty` costs a merged regression and is invisible - but + nothing in the tree should REACH it. Every tracked file is classified by a rule, so + 17 fires only for genuinely new shapes, and this test is what tells the author to + write the row instead of letting the fall-through pick an answer for them. + + Before this guard, 254 tracked files reached 17: .gitignore took a docs-only PR to + 74 cmake legs and the whole rig, while examples//CMakeLists.txt got the RIGHT + answer from the wrong rule - row 15 names it, the regex never matched it.""" + + def _unclassified(self, axis): + import subprocess as sp + r = sp.run(['git', 'ls-files'], cwd=REPO, capture_output=True, text=True) + if r.returncode != 0: + self.skipTest('not a git checkout') + files = r.stdout.split() + self.assertGreater(len(files), 1000, 'suspiciously few tracked files') + out = [] + for f in files: + s = (ci_select.classify_build([f], REPO) if axis == 'build' + else ci_select.classify([f], REPO, real_rosters())) + if any('unclassified' in why for why in s['reasons']): + out.append(f) + return out + + def test_build_axis(self): + left = self._unclassified('build') + self.assertEqual(left, [], f'{len(left)} tracked files fall through to rule 17 on ' + f'the build axis, e.g. {left[:5]} - classify them, or ' + f'add the pattern to _META_RE if no build reads them') + + def test_hil_axis(self): + left = self._unclassified('hil') + self.assertEqual(left, [], f'{len(left)} tracked files fall through to rule 17 on ' + f'the HIL axis, e.g. {left[:5]}') + + class TestLibRule(unittest.TestCase): """lib/** is not a full-matrix path: only the examples that build the lib need it.""" @@ -1241,7 +1280,28 @@ class TestBuildClassifier(unittest.TestCase): 'tools/build.py', 'tools/cmake/cpu/cortex-m4.cmake', 'examples/CMakeLists.txt', 'examples/device/CMakeLists.txt', 'examples/build_system/cmake/cpu.cmake', '.github/workflows/build.yml', - 'sonar-project.properties', 'some/unknown/path.c'): + '.circleci/config.yml', 'src/CMakeLists.txt', 'src/tinyusb.mk', + 'hw/bsp/family_support.mk', 'tools/build_utils.py', + 'some/unknown/path.c'): + self.assertTrue(self.b([p])['full'], p) + + def test_repo_metadata_is_not_a_build_input(self): + # these used to reach `full` through rule 17: a PR touching only .gitignore and a + # README created 74 cmake legs and booked the whole rig. No Build step reads them. + for p in ('sonar-project.properties', '.gitignore', '.gitattributes', + '.clang-format', '.idea/misc.xml', 'version.yml', 'library.json', + 'examples/CMakePresets.json', 'test/fuzz/fuzz.cc', + 'test/unit-test/project.yml', '.github/workflows/pr_comment.yml', + 'tools/gen_doc.py'): + s = self.b([p]) + self.assertFalse(s['full'], p) + self.assertEqual(s['families'], [], p) + + def test_the_build_machinery_is_still_full(self): + # the other side of the same line: these DECIDE what gets built + for p in ('.circleci/config.yml', '.github/workflows/build.yml', + '.github/scripts/ci_set_matrix.py', 'tools/ci_select.py', + 'tools/build_utils.py', 'tools/metrics.py'): self.assertTrue(self.b([p])['full'], p) def test_mixed_diff_unions_per_family(self): diff --git a/tools/ci_select.py b/tools/ci_select.py index ced3bbbc0..cf5a0da5b 100755 --- a/tools/ci_select.py +++ b/tools/ci_select.py @@ -54,6 +54,34 @@ def _read(path: str) -> str: _NONCODE_RE = re.compile( r'^(docs/|\.claude/|.*\.(md|rst)$|LICENSE)') +# Repo metadata and tooling that no CI build reads. Enumerated rather than left to +# rule 17, which widens BOTH axes: a PR touching only .gitignore and a README was +# creating 74 cmake legs (each a runner doing checkout + toolchain + get_deps before +# skipping the build) and booking the whole 30-board rig. +# +# Deliberately NOT here, and still full: .circleci/**, .github/workflows/build*.yml, +# .github/actions/**, .github/scripts/** - those decide what gets built. The line is +# "does any Build step read this file", not "is it source". +# +# test/{fuzz,unit-test} have their own jobs (cifuzz.yml, the unit-test pre-commit hook +# and workflow); the Build matrix never compiles them, and test/hil is rule 2. +_META_RE = re.compile( + r'^(' + r'\.(gitignore|gitattributes|clang-format|codespellrc|readthedocs\.yaml)$|' + r'\.pre-commit-config\.yaml$|\.PVS-Studio/|\.idea/|\.vscode/|' + r'sonar-project\.properties$|library\.json$|pkg\.yml$|repository\.yml$|' + r'version\.yml$|SConscript$|' + r'.*CMakePresets\.json$|hw/bsp/BoardPresets\.json$|examples/west\.yml$|' + r'.*/[0-9]+-tinyusb[^/]*\.rules$|tools/usb_drivers/|tools/codespell/|' + r'test/(fuzz|unit-test)/|' + # .github, minus the build machinery named in _FULL_RE + r'\.github/(FUNDING\.yml|labeler\.yml|membrowse_pr_message\.j2|ISSUE_TEMPLATE/|' + r'workflows/(cifuzz|claude|claude-code-review|labeler|membrowse-comment|' + r'membrowse-onboard|pr_comment|pre-commit|static_analysis|trigger)\.yml$)|' + # tools/ scripts no build invokes (tools/build*.py and metrics are handled above) + r'tools/(build_doc|check_example_pids|file2carray|gen_doc|gen_presets|iar_gen|' + r'make_release|mksunxi|pcapng_to_corpus)\.py$|tools/iar_template\.ipcf$' + r')') # Build-size metrics tooling. HIL axis ONLY: nothing on the rig runs any of it, and # without this rule these paths are unclassified, so a metrics-only PR booked an # exclusive full 30-board sweep to validate a script no board executes. @@ -66,9 +94,20 @@ _METRICS_RE = re.compile( _FULL_RE = re.compile( r'^(src/common/|src/osal/|src/tusb\.c$|src/tusb\.h$|src/tusb_option\.h$|' r'test/hil/|\.github/workflows/build.*\.yml$|\.github/actions/|\.github/scripts/|' - r'tools/build\.py$|tools/cmake/|' - r'hw/bsp/(family_support\.cmake|board_api\.h|board\.c|ansi_escape\.h)$|' + # generates the whole CircleCI matrix, same authority as .github/** + r'\.circleci/|' + # rule 16 says `tools/build*.py`; name the two siblings the glob implies. Both + # decide what gets built, so neither can be trusted to narrow its own change. + r'tools/(build|build_utils|ci_select)\.py$|tools/cmake/|' + # the make twins of family_support.cmake are the same authority for the make legs + r'hw/bsp/(family_support\.(cmake|mk)|family_rules\.mk|zephyr_board_aliases\.cmake|' + r'board_api\.h|board\.c|ansi_escape\.h)$|' + # rule 15 lists examples//CMakeLists.txt - it registers every target in that + # role, so it was only ever reaching `full` through rule 17's fall-through r'examples/build_system/|examples/CMakeLists\.txt$|' + r'examples/[^/]+/CMakeLists\.txt$|' + # every firmware compiles these unconditionally (src/CMakeLists.txt, src/tinyusb.mk) + r'src/CMakeLists\.txt$|src/tinyusb\.mk$|' # board_test is HIL infrastructure, not a test: hil_test.py flashes it to park # every board (variant boundary + end-of-board teardown), so every board depends on it r'examples/device/board_test/)') @@ -538,7 +577,7 @@ class _Sel: def _classify_one(path, repo_root, roster_boards, extras: set, s: _Sel, get_deps_families=None): base = os.path.basename(path) - if _NONCODE_RE.match(path): + if _NONCODE_RE.match(path) or _META_RE.match(path): s.reasons.append(f'{path}: non-code, no contribution') return if _METRICS_RE.match(path): @@ -673,6 +712,11 @@ def _classify_one(path, repo_root, roster_boards, extras: set, s: _Sel, s.add(boards, sorted(tests), f'{path}: lib {lib} -> {sorted(tests)} on all boards') return + if re.match(r'src/typec/', path): + # only examples/typec enables CFG_TUC_ENABLED, and no rig board runs a typec + # test (see _HIL_EX_ROLES) - so the build axis covers it and the rig cannot + s.reasons.append(f'{path}: typec, no HIL contribution') + return m = _BUILD_EX_RE.match(path) if m: if m.group(1) not in _HIL_EX_ROLES: @@ -935,7 +979,8 @@ class _BSel: def _classify_build_one(path, repo_root, s: _BSel, get_deps_families=None): base = os.path.basename(path) - if _NONCODE_RE.match(path): # rule 1 + if _NONCODE_RE.match(path) or _META_RE.match(path): # rule 1 + s.reasons.append(f'{path}: non-code, no build contribution') return if re.match(r'test/hil/', path): # rule 2 s.reasons.append(f'{path}: HIL harness, no build contribution') @@ -1006,6 +1051,18 @@ def _classify_build_one(path, repo_root, s: _BSel, get_deps_families=None): # CMakeLists (rule 15) is what forces the full matrix s.reasons.append(f'{path}: not an example dir, no build contribution') return + if re.match(r'src/typec/', path): # rule 12b + # listed unconditionally by src/CMakeLists.txt and src/tinyusb.mk, but the whole + # body is `#if CFG_TUC_ENABLED` - so it is PARSED by every build and COMPILED + # only for examples that enable it. Same shape as the class rule, same answer: + # the examples whose tusb_config.h turns it on, and empty means empty. + exs = examples_enabling(role_examples(repo_root, ('typec',)), + ('CFG_TUC_ENABLED',), repo_root) + if not exs: + s.reasons.append(f'{path}: typec enabled by no example config, no contribution') + return + s.add(all_bsp_families(repo_root), exs, f'{path}: typec -> {sorted(exs)}') + return m = re.match(r'lib/([^/]+)/', path) if m: # lib rule lib = m.group(1) @@ -1018,7 +1075,19 @@ def _classify_build_one(path, repo_root, s: _BSel, get_deps_families=None): return s.add(all_bsp_families(repo_root), exs, f'{path}: lib {lib} -> {sorted(exs)}') return - s.force_full(f'{path}: unclassified -> full build matrix') # rules 15-17 + if _METRICS_RE.match(path): + # HIL-suppressed above; on this axis they stay full - tools/metrics.py runs as + # the `tinyusb_metrics` build target, so a break in it fails the build + s.force_full(f'{path}: metrics tooling runs in the build -> full build matrix') + return + if _FULL_RE.match(path): # rules 15-16 + # attribution, not behaviour: these already reached `full` through the + # fall-through below. Naming them means a future narrowing of rule 17 cannot + # silently change what they do. Deliberately last, so every earlier rule keeps + # priority - examples/device/board_test is rule 14 (just board_test), not ALL. + s.force_full(f'{path}: core/infra -> full build matrix') + return + s.force_full(f'{path}: unclassified -> full build matrix') # rule 17 @contextlib.contextmanager -- cgit v1.3.1 From 050595d64f9f130783853a2342eb1114d32199e8 Mon Sep 17 00:00:00 2001 From: hathach Date: Sat, 22 Aug 2026 23:10:14 +0700 Subject: ci_select: address Copilot review - anchor _META_RE, cover rule 12b Anchor the .github file alternatives. FUNDING.yml, labeler.yml and membrowse_pr_message.j2 sat inside a group whose only `$` belonged to the workflows/ branch, so they matched as prefixes: .github/labeler.yml.bak and .github/FUNDING.yml.old were classified as metadata and would have selected nothing. No such file exists today - the workflows/ alternative was already anchored and ISSUE_TEMPLATE/ is a directory prefix on purpose. Rule 12b had no test of its own: TestNoTrackedFileIsUnclassified only proved src/typec no longer reaches rule 17, not that the answer is right. TestTypecRule pins it - non-full, every selected example under typec/, all four src/typec files answering alike, no rig board, and the set derived from CFG_TUC_ENABLED rather than hardcoded, so it follows a new typec example on its own. Verified all four fail with rule 12b removed. --- test/hil/test/test_ci_select.py | 42 +++++++++++++++++++++++++++++++++++++++++ tools/ci_select.py | 2 +- 2 files changed, 43 insertions(+), 1 deletion(-) (limited to 'tools/ci_select.py') diff --git a/test/hil/test/test_ci_select.py b/test/hil/test/test_ci_select.py index 9b2c61ef2..a19392bde 100644 --- a/test/hil/test/test_ci_select.py +++ b/test/hil/test/test_ci_select.py @@ -840,6 +840,48 @@ class TestRostersDoNotOverlap(unittest.TestCase): seen[b['name']] = b.get('tests') +class TestTypecRule(unittest.TestCase): + """Rule 12b. src/typec/usbc.c is listed unconditionally by src/CMakeLists.txt and + src/tinyusb.mk, but its whole body is `#if CFG_TUC_ENABLED`, which only + examples/typec/power_delivery sets - so it is parsed by every build and compiled by + one. Same shape as the class rule, same answer. Before this rule it matched nothing + and force-fulled 82 families and all 30 rig boards.""" + + def test_build_axis_selects_only_the_typec_examples(self): + s = ci_select.classify_build(['src/typec/usbc.c'], REPO) + self.assertFalse(s['full']) + self.assertTrue(s['families'], 'typec must be compiled somewhere') + self.assertTrue(s['family_examples'], 'and the examples must be named') + for fam, exs in s['family_examples'].items(): + self.assertTrue(exs, fam) + for e in exs: + self.assertTrue(e.startswith('typec/'), f'{fam}: {e} is not a typec example') + + def test_every_typec_file_answers_the_same(self): + for f in ('src/typec/usbc.c', 'src/typec/usbc.h', 'src/typec/tcd.h', + 'src/typec/pd_types.h'): + s = ci_select.classify_build([f], REPO) + self.assertFalse(s['full'], f) + self.assertTrue(s['families'], f) + + def test_no_rig_board_runs_typec(self): + # typec is not a HIL role, so the rig cannot exercise it whatever it selects + s = sel(['src/typec/usbc.c']) + self.assertFalse(s['full']) + self.assertEqual(s['boards'], {}) + + def test_it_tracks_the_enabling_config_rather_than_a_hardcoded_list(self): + # the answer must come from CFG_TUC_ENABLED in the example configs, so it + # follows a new typec example (or an old one switched off) on its own + want = ci_select.examples_enabling( + ci_select.role_examples(REPO, ('typec',)), ('CFG_TUC_ENABLED',), REPO) + self.assertTrue(want, 'no example enables CFG_TUC_ENABLED - rule 12b is dead') + got = set() + for exs in ci_select.classify_build(['src/typec/usbc.c'], REPO)['family_examples'].values(): + got |= set(exs) + self.assertEqual(got, want) + + class TestCachesAreKeyedOnTheTree(unittest.TestCase): """build_utils caches on repo-RELATIVE paths while ci_select._in_repo() chdirs between trees, so the cwd has to be part of every cache key. Without it a second diff --git a/tools/ci_select.py b/tools/ci_select.py index cf5a0da5b..89a0d214c 100755 --- a/tools/ci_select.py +++ b/tools/ci_select.py @@ -75,7 +75,7 @@ _META_RE = re.compile( r'.*/[0-9]+-tinyusb[^/]*\.rules$|tools/usb_drivers/|tools/codespell/|' r'test/(fuzz|unit-test)/|' # .github, minus the build machinery named in _FULL_RE - r'\.github/(FUNDING\.yml|labeler\.yml|membrowse_pr_message\.j2|ISSUE_TEMPLATE/|' + r'\.github/(FUNDING\.yml$|labeler\.yml$|membrowse_pr_message\.j2$|ISSUE_TEMPLATE/|' r'workflows/(cifuzz|claude|claude-code-review|labeler|membrowse-comment|' r'membrowse-onboard|pr_comment|pre-commit|static_analysis|trigger)\.yml$)|' # tools/ scripts no build invokes (tools/build*.py and metrics are handled above) -- cgit v1.3.1 From da255b1d2db10b8f31332a779b2a526f579acee1 Mon Sep 17 00:00:00 2001 From: Ha Thach Date: Tue, 25 Aug 2026 09:46:42 +0700 Subject: ci: an empty selection must build nothing, plus selector follow-ups (#3845) ci: an empty selection must build nothing, plus selector follow-ups A PR whose build axis legitimately selected nothing rebuilt everything. build.yml reads .build.families twice - as a |-joined regex, and implicitly as "is anything selected" - but tested only -z "$FAMILY_REGEX", which an empty list and a charset-rejected one both satisfy while meaning opposite things. ci_set_matrix had already returned the correct all-empty matrix; the fall-open branch discarded it. #3842 and #3840 each spent 74 cmake legs on it. Branch on the two cases instead, rename FAM_* to FAMILY_*, and cover the block with a test that extracts it from build.yml and executes it - it had no test at all, which is how this shipped through two merges. Follow-ups to the same machinery: glob.escape the repo root at five sites, so a checkout path containing [ or * stops failing closed; drop the ci-full label, read after the matrix was already computed and so never functional; delete 13 mcu:MKL25ZXX / mcu:SAME5X skip tokens matching no board; carry the rule table in the module docstring, guarded against drift; and pin six selection behaviours a mutation pass proved untested. Cut the selector's cost 1.8x (26.0s -> 14.6s) with 0 divergences over 260 paths, and stop scoping the membrowse upload by the PR example filter. --- .github/scripts/ci_set_matrix.py | 8 +- .github/workflows/build.yml | 35 ++-- .github/workflows/build_util.yml | 13 +- docs/reference/hil_boards.md | 2 +- .../2026-08-19-ci-build-family-filter-design.md | 13 ++ examples/device/audio_4_channel_mic/skip.txt | 1 - .../device/audio_4_channel_mic_freertos/skip.txt | 1 - examples/device/audio_test/skip.txt | 1 - examples/device/audio_test_freertos/skip.txt | 1 - examples/device/audio_test_multi_rate/skip.txt | 1 - examples/device/cdc_msc_freertos/skip.txt | 1 - examples/device/cdc_uac2/skip.txt | 1 - examples/device/hid_composite_freertos/skip.txt | 1 - examples/device/midi_test_freertos/skip.txt | 1 - examples/device/msc_dual_lun/skip.txt | 1 - examples/device/uac2_headset/skip.txt | 1 - examples/device/uac2_speaker_fb/skip.txt | 1 - test/hil/test/test_ci_metrics.py | 126 ++++++++++++- test/hil/test/test_ci_select.py | 204 ++++++++++++++++++++- tools/build.py | 4 +- tools/ci_select.py | 130 ++++++++++--- 21 files changed, 466 insertions(+), 81 deletions(-) (limited to 'tools/ci_select.py') diff --git a/.github/scripts/ci_set_matrix.py b/.github/scripts/ci_set_matrix.py index 79f466893..409e6dbc1 100755 --- a/.github/scripts/ci_set_matrix.py +++ b/.github/scripts/ci_set_matrix.py @@ -131,7 +131,13 @@ def set_matrix_json(select=None): # a family this file does not list builds on no toolchain, so it contributes no # leg. hw/bsp holds several CI has never built (efm32, py32f0, same7x, ...) plus # espressif, whose boards hil-build-esp builds by name. - unbuilt = sorted(f for f in sel_fams if f not in family_list) + # espressif is not a gap: its examples need the ESP-IDF environment + # (CLAUDE.md: `. "$IDF_PATH/export.sh"` before any build), which the cmake legs + # do not have - that is why it is commented out of family_list above. Its + # coverage comes from hil-build-esp, which builds those boards BY NAME in an IDF + # container, so an espressif-only PR is already validated and falling open to the + # full matrix would add 74 legs, none of which can compile espressif. + unbuilt = sorted(f for f in sel_fams if f not in family_list and f != 'espressif') if unbuilt and not any(matrix.values()): # NONE of the selected families is buildable here, so every leg would skip # and the PR would go green from a build job that ran no compiler. That is diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 39a4e7afd..c26fe5cf8 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -68,14 +68,9 @@ 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' && !contains(github.event.pull_request.labels.*.name, 'ci-full') + if: github.event_name == 'pull_request' env: BASE_REF: ${{ github.base_ref }} run: | @@ -179,29 +174,43 @@ jobs: # treats false like null, so .build.full is compared explicitly. EXAMPLE_MAP='{}' BUILD_FILTERED='false' - FAM_REGEX='' + FAMILY_REGEX='' if [ -n "$BUILD_SELECT_FILE" ]; then EXAMPLE_MAP=$(jq -c '.build.family_examples // {}' "$BUILD_SELECT_FILE") || EXAMPLE_MAP='{}' BUILD_FILTERED=$(jq -r 'if (.build? | type) == "object" and .build.full == false then "true" else "false" end' "$BUILD_SELECT_FILE") || BUILD_FILTERED='false' if [ "$BUILD_FILTERED" = "true" ]; then - FAM_REGEX=$(jq -r '.build.families | join("|")' "$BUILD_SELECT_FILE") || FAM_REGEX='' + FAMILY_COUNT=$(jq -r '.build.families | length' "$BUILD_SELECT_FILE") || FAMILY_COUNT=0 + FAMILY_REGEX=$(jq -r '.build.families | join("|")' "$BUILD_SELECT_FILE") || FAMILY_REGEX='' # family names come from hw/bsp dir names, which rule 6 reads straight out # of the PR's diff path - and this is interpolated raw into a # `name_is_regexp` artifact pattern, so a regex metacharacter there would # silently match another family's baseline - case "$FAM_REGEX" in + FAMILY_REJECTED=0 + case "$FAMILY_REGEX" in *[!-A-Za-z0-9_\|]*) echo "::warning::unexpected characters in the family list - dropping the scoping" - FAM_REGEX='' ;; + FAMILY_REGEX=''; FAMILY_REJECTED=1 ;; esac - if [ -z "$FAM_REGEX" ]; then - # all three drop together, as CircleCI's fall-open does. Resetting only + # An EMPTY families list and a REJECTED one both leave FAMILY_REGEX empty and + # mean opposite things, so branch on which happened. Testing `-z` alone sent + # every nothing-selected PR down the fall-open path: a docs/.gitignore diff + # (#3842) and a test/hil-only diff (#3840) each rebuilt all 74 cmake legs + # after the selector had correctly chosen none. + if [ "$FAMILY_REJECTED" = "1" ]; then + # unusable: fall open, and all three drop together. 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) + elif [ "$FAMILY_COUNT" = "0" ]; then + # legitimate nothing-selected. MATRIX_JSON already holds the all-empty + # matrix ci_set_matrix produced from this selection - keep it, so every + # leg skips. Nothing is built, so there is nothing to compare a baseline + # against: build_filtered goes false to keep code-metrics off the scoped + # path, and EXAMPLE_MAP stays '{}' (family_examples is empty anyway). + BUILD_FILTERED='false' fi fi fi @@ -210,7 +219,7 @@ jobs: 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 + echo "build_families_regex=$FAMILY_REGEX" >> $GITHUB_OUTPUT # HIL matrix (merged from tinyusb + hifiphile configs), scoped on PRs. # Scoping is best-effort too: fall back to the unscoped (full) matrix. diff --git a/.github/workflows/build_util.yml b/.github/workflows/build_util.yml index 52999616d..407ed1e71 100644 --- a/.github/workflows/build_util.yml +++ b/.github/workflows/build_util.yml @@ -126,16 +126,11 @@ jobs: MEMBROWSE_API_KEY: ${{ secrets.MEMBROWSE_API_KEY }} run: | # if code-changed is false --> there is no elf -> membrowse target upload with --identical flag - # $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. + # deliberately unscoped by $EX_ARGS: keeps the size history on a stable board + # per family, at the cost of an --identical-only upload where that board is not + # the one the Build step picked (test_ci_metrics pins which families those are) 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 }} $EX_ARGS + python tools/build.py $BUILD_PY_ARGS --target examples-membrowse-upload -j 1 ${{ matrix.arg }} shell: bash - name: Upload Artifacts for Metrics diff --git a/docs/reference/hil_boards.md b/docs/reference/hil_boards.md index e8f364646..678f7f0ed 100644 --- a/docs/reference/hil_boards.md +++ b/docs/reference/hil_boards.md @@ -12,7 +12,7 @@ | espressif_s3_devkitm | device, host | esptool | espressif_s3_devkitm, espressif_s3_devkitm-DMA | Use TS3USB30 mux to test both device and host | | feather_nrf52840_express | device | jlink | | | | max32666fthr | device | openocd | | | -| metro_m4_express | device, dual | jlink | | pl23x; audio_test_freertos skipped: samd51 iso-IN capture fails (arecord EIO) | +| metro_m4_express | device, dual | jlink | metro_m4_express | pl23x; audio_test_freertos skipped: samd51 iso-IN capture fails (arecord EIO) | | lpcxpresso11u37 | device | jlink | | | | lpcxpresso55s28 | device | jlink | | | | ra4m1_ek | device | jlink | | | 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 b10f5b4ae..524568aeb 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 @@ -146,6 +146,19 @@ 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. +### A class no example enables selects nothing + +`src/class/bth` is the live instance: no example's `tusb_config.h` sets `CFG_TUD_BTH`, so +rules 8-10 resolve to no examples and a bth-only PR builds nothing and runs nothing. That is +the empty-means-empty ruling applied to classes, and it is deliberate — nothing compiles the +file, so nothing can validate it, and the master-push build is the net. + +Worth stating plainly because the exposure changed: GHA used to rebuild everything for such +a PR by accident, through the empty-`families` bug in `build.yml`. With that fixed, both +providers now correctly build nothing, so `tud_bt_*` can be broken by a green PR. +`TestClassesWithNoEnablingExample` pins the set to `{bth}` so a second class cannot enter +this state unnoticed. + ### Why `hw/mcu/**` is rule 7 and not "full" `hw/mcu` is overwhelmingly dependency territory — `tools/get_deps.py` has 87 entries under it, diff --git a/examples/device/audio_4_channel_mic/skip.txt b/examples/device/audio_4_channel_mic/skip.txt index 3ca433c08..e5e74cd60 100644 --- a/examples/device/audio_4_channel_mic/skip.txt +++ b/examples/device/audio_4_channel_mic/skip.txt @@ -1,5 +1,4 @@ mcu:SAMD11 -mcu:SAME5X mcu:SAMG family:broadcom_64bit family:espressif diff --git a/examples/device/audio_4_channel_mic_freertos/skip.txt b/examples/device/audio_4_channel_mic_freertos/skip.txt index 1fd6b4b8a..cfde51051 100644 --- a/examples/device/audio_4_channel_mic_freertos/skip.txt +++ b/examples/device/audio_4_channel_mic_freertos/skip.txt @@ -7,7 +7,6 @@ mcu:CXD56 mcu:F1C100S mcu:GD32VF103 mcu:MCXA15 -mcu:MKL25ZXX mcu:MSP430x5xx mcu:FT90X mcu:SAMD11 diff --git a/examples/device/audio_test/skip.txt b/examples/device/audio_test/skip.txt index 42394bb11..862c91c6f 100644 --- a/examples/device/audio_test/skip.txt +++ b/examples/device/audio_test/skip.txt @@ -1,5 +1,4 @@ mcu:SAMD11 -mcu:SAME5X mcu:SAMG family:espressif mcu:CH583 diff --git a/examples/device/audio_test_freertos/skip.txt b/examples/device/audio_test_freertos/skip.txt index 660bacd25..3d8d43286 100644 --- a/examples/device/audio_test_freertos/skip.txt +++ b/examples/device/audio_test_freertos/skip.txt @@ -7,7 +7,6 @@ mcu:CXD56 mcu:F1C100S mcu:GD32VF103 mcu:MCXA15 -mcu:MKL25ZXX mcu:MSP430x5xx mcu:FT90X mcu:SAMD11 diff --git a/examples/device/audio_test_multi_rate/skip.txt b/examples/device/audio_test_multi_rate/skip.txt index 42394bb11..862c91c6f 100644 --- a/examples/device/audio_test_multi_rate/skip.txt +++ b/examples/device/audio_test_multi_rate/skip.txt @@ -1,5 +1,4 @@ mcu:SAMD11 -mcu:SAME5X mcu:SAMG family:espressif mcu:CH583 diff --git a/examples/device/cdc_msc_freertos/skip.txt b/examples/device/cdc_msc_freertos/skip.txt index 48781de84..095e350c9 100644 --- a/examples/device/cdc_msc_freertos/skip.txt +++ b/examples/device/cdc_msc_freertos/skip.txt @@ -7,7 +7,6 @@ mcu:CXD56 mcu:F1C100S mcu:GD32VF103 mcu:MCXA15 -mcu:MKL25ZXX mcu:MSP430x5xx mcu:FT90X mcu:SAMD11 diff --git a/examples/device/cdc_uac2/skip.txt b/examples/device/cdc_uac2/skip.txt index db1d5b80b..3159cb176 100644 --- a/examples/device/cdc_uac2/skip.txt +++ b/examples/device/cdc_uac2/skip.txt @@ -2,7 +2,6 @@ mcu:LPC11UXX mcu:LPC13XX mcu:NUC121 mcu:SAMD11 -mcu:SAME5X mcu:SAMG board:stm32l052dap52 family:espressif diff --git a/examples/device/hid_composite_freertos/skip.txt b/examples/device/hid_composite_freertos/skip.txt index 97d8e168b..0e8415d3b 100644 --- a/examples/device/hid_composite_freertos/skip.txt +++ b/examples/device/hid_composite_freertos/skip.txt @@ -7,7 +7,6 @@ mcu:CXD56 mcu:F1C100S mcu:GD32VF103 mcu:MCXA15 -mcu:MKL25ZXX mcu:MSP430x5xx mcu:FT90X mcu:SAMD11 diff --git a/examples/device/midi_test_freertos/skip.txt b/examples/device/midi_test_freertos/skip.txt index 97d8e168b..0e8415d3b 100644 --- a/examples/device/midi_test_freertos/skip.txt +++ b/examples/device/midi_test_freertos/skip.txt @@ -7,7 +7,6 @@ mcu:CXD56 mcu:F1C100S mcu:GD32VF103 mcu:MCXA15 -mcu:MKL25ZXX mcu:MSP430x5xx mcu:FT90X mcu:SAMD11 diff --git a/examples/device/msc_dual_lun/skip.txt b/examples/device/msc_dual_lun/skip.txt index a9e3a99b1..833fd072c 100644 --- a/examples/device/msc_dual_lun/skip.txt +++ b/examples/device/msc_dual_lun/skip.txt @@ -1,3 +1,2 @@ mcu:SAMD11 -mcu:MKL25ZXX family:espressif diff --git a/examples/device/uac2_headset/skip.txt b/examples/device/uac2_headset/skip.txt index db1d5b80b..3159cb176 100644 --- a/examples/device/uac2_headset/skip.txt +++ b/examples/device/uac2_headset/skip.txt @@ -2,7 +2,6 @@ mcu:LPC11UXX mcu:LPC13XX mcu:NUC121 mcu:SAMD11 -mcu:SAME5X mcu:SAMG board:stm32l052dap52 family:espressif diff --git a/examples/device/uac2_speaker_fb/skip.txt b/examples/device/uac2_speaker_fb/skip.txt index 0c7339c65..88df3e549 100644 --- a/examples/device/uac2_speaker_fb/skip.txt +++ b/examples/device/uac2_speaker_fb/skip.txt @@ -2,7 +2,6 @@ mcu:LPC11UXX mcu:LPC13XX mcu:NUC121 mcu:SAMD11 -mcu:SAME5X mcu:SAMG board:stm32l052dap52 family:broadcom_64bit diff --git a/test/hil/test/test_ci_metrics.py b/test/hil/test/test_ci_metrics.py index a76b6e3a0..aac251824 100644 --- a/test/hil/test/test_ci_metrics.py +++ b/test/hil/test/test_ci_metrics.py @@ -447,16 +447,126 @@ class TestWorkflowSelectionHandOff(unittest.TestCase): self.assertIn('UNSCOPED', flat[max(0, i - 200):i], 'a fall-open path without the marker build.yml greps for') - 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. + def _run_extras_block(self, sel): + """Extract the build-extras shell block from build.yml and run it for real. + Nothing else exercises it, which is why the empty/rejected conflation shipped.""" + import re as _re, shlex, subprocess, tempfile, json as _json + repo = os.path.dirname(CIRCLECI) + i = self.build.index("EXAMPLE_MAP='{}'\n BUILD_FILTERED='false'") + i = self.build.rindex('\n', 0, i) + 1 + j = self.build.index(' echo "matrix=$MATRIX_JSON"', i) + block = _re.sub(r'^ {10}', '', self.build[i:j], flags=_re.M) + with tempfile.TemporaryDirectory() as d: + selp = os.path.join(d, 'sel.json') + with open(selp, 'w') as fh: + _json.dump(sel, fh) + matrix = subprocess.run( + [sys.executable, os.path.join(repo, '.github/scripts/ci_set_matrix.py'), + '--select-file', selp], capture_output=True, text=True, cwd=repo).stdout.strip() + self.assertTrue(matrix, 'ci_set_matrix produced nothing') + sh = os.path.join(d, 'probe.sh') + with open(sh, 'w') as fh: + # shlex.quote, not hand-rolled quoting: a TMPDIR with a space in it + # made this fail for a reason that had nothing to do with the block + fh.write('BUILD_SELECT_FILE=' + shlex.quote(selp) + '\n') + fh.write('MATRIX_JSON=' + shlex.quote(matrix) + '\n') + fh.write(block) + # sentinel + newline separated: the block itself writes ::warning:: to + # stdout, and '|' would collide with the regex's own separator + fh.write('\nprintf "@@R@@\\n%s\\n%s\\n%s" "$MATRIX_JSON" "$BUILD_FILTERED" "$FAMILY_REGEX"\n') + r = subprocess.run(['bash', sh], capture_output=True, text=True, cwd=repo) + self.assertEqual(r.returncode, 0, r.stderr) + mj, filtered, regex = r.stdout.split('@@R@@\n', 1)[1].split('\n', 2) + return sum(len(v) for v in _json.loads(mj).values()), filtered, regex + + def test_an_empty_family_list_is_not_treated_as_unusable(self): + """.build.families is read twice - as a count and as a `|`-joined regex. An EMPTY + list and one REJECTED by the charset guard both leave the regex empty and mean + opposite things, so the block has to branch on which happened. + + Testing `-z "$FAMILY_REGEX"` alone sent every nothing-selected PR down the + fall-open path and discarded the correct all-empty matrix: #3842 (docs + + .gitignore) and #3840 (test/hil only) each rebuilt all 74 cmake legs after the + selector had correctly chosen none.""" + legs, filtered, regex = self._run_extras_block( + {'build': {'full': False, 'families': [], 'family_examples': {}}}) + self.assertEqual(legs, 0, 'an empty families list must keep the all-empty matrix') + self.assertEqual(filtered, 'false', 'nothing was built, so nothing to compare') + self.assertEqual(regex, '') + + def test_a_real_family_list_stays_scoped(self): + legs, filtered, regex = self._run_extras_block( + {'build': {'full': False, 'families': ['stm32f4', 'rp2040'], + 'family_examples': {}}}) + self.assertGreater(legs, 0) + self.assertEqual(filtered, 'true') + self.assertEqual(regex, 'stm32f4|rp2040') + + def test_a_regex_metacharacter_in_a_family_name_falls_open(self): + # the name is interpolated raw into a name_is_regexp artifact pattern, so a + # metacharacter would match another family's baseline - reject and widen + legs, filtered, regex = self._run_extras_block( + {'build': {'full': False, 'families': ['stm32f4.*'], 'family_examples': {}}}) + self.assertGreater(legs, 100, 'a rejected family list must fall open to full') + self.assertEqual(filtered, 'false') + self.assertEqual(regex, '') + + def test_membrowse_upload_is_not_scoped_by_the_pr_filter(self): + # by decision, the upload runs unfiltered so the size history stays keyed on the + # family's preferred board whatever the PR touched. $EX_ARGS would not have + # scoped the targets either way - `examples-membrowse-upload` is not `all`, so + # resolve_example_target_groups passes it through as the aggregate - but it DID + # move the board, because --one-first picks one that can build the -e set. + # + # The accepted cost: on a family whose preferred board cannot build that set, + # the upload lands on a board the Build step never compiled and every example + # goes up --identical. test_the_upload_board_can_diverge_from_the_built_board + # keeps that consequence measured rather than assumed. 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', '')) + self.assertNotIn('$EX_ARGS', line) + self.assertNotIn('-e ', line) + + def test_the_upload_board_can_diverge_from_the_built_board(self): + """Pins the SIZE of what the removal gave up, so it cannot grow unnoticed. + + --one-first with no -e returns preferred_list[0]; with one it returns the first + preferred board that can build it. Where those differ, the Membrowse Upload step + configures a build dir the Build step never wrote.""" + sys.path.insert(0, os.path.join(REPO, 'tools')) + import build as build_py + roles = ('device', 'host', 'dual') + exs = sorted(f'{r}/{n}' for r in roles + for n in os.listdir(os.path.join(REPO, 'examples', r)) + if os.path.isdir(os.path.join(REPO, 'examples', r, n))) + fams = sorted(d for d in os.listdir(os.path.join(REPO, 'hw/bsp')) + if os.path.isdir(os.path.join(REPO, 'hw/bsp', d, 'boards'))) + cwd = os.getcwd() + os.chdir(REPO) + try: + diverging = set() + for fam in fams: + try: + base = build_py.get_family_boards(fam, False, True, None, 'cmake', ()) + except Exception: + continue + if not base: + continue + for e in exs: + try: + one = build_py.get_family_boards(fam, False, True, [e], 'cmake', ()) + except Exception: + continue + if one and one[0] != base[0]: + diverging.add(fam) + break + finally: + os.chdir(cwd) + self.assertEqual(diverging, {'imxrt', 'lpc11', 'lpc18', 'lpc54', 'mcx', 'rp2040', + 'rx', 'samd11', 'stm32l0', 'stm32l4', 'tm4c'}, + 'the set of families whose membrowse upload can land on an ' + 'uncompiled board changed; re-check whether dropping $EX_ARGS ' + 'from the upload step is still the right trade') if __name__ == '__main__': diff --git a/test/hil/test/test_ci_select.py b/test/hil/test/test_ci_select.py index a19392bde..dc10f769a 100644 --- a/test/hil/test/test_ci_select.py +++ b/test/hil/test/test_ci_select.py @@ -302,7 +302,11 @@ class TestArgsEmission(unittest.TestCase): out = j.loads(r.stdout) self.assertFalse(out['full']) self.assertIn('tinyusb.json', out['args']) - self.assertTrue(any('cdc_device' in line for line in out['reasons'])) + # reasons are a stderr diagnostic, deliberately NOT in the payload: they were + # 97% of a 9.8 MB JSON on a dep bump, and every consumer re-parses that file + self.assertNotIn('reasons', out, 'reasons must not ride in the machine-read JSON') + self.assertNotIn('reasons', out['build']) + self.assertIn('cdc_device', r.stderr) # A core-class diff must select boards THROUGH THE CLI: the in-process tests # inject their own repo root, so only this subprocess path catches a broken # repo_root derivation -- which once made every repo-relative glob match @@ -938,6 +942,181 @@ class TestClassesWithNoEnablingExample(unittest.TestCase): 'both axes, so nothing compiles it until the next master push') +class TestExampleMapOmitsFullFamilies(unittest.TestCase): + """A family whose selection is ALREADY everything it can build carries no -e list. + + Sixth of the same shape as the class below, found the same way: a perf rewrite of + _prune_buildable dropped the `set(kept) != set(buildable)` test and all 216 tests + stayed green. The build outcome is identical either way -- build.py applies the same + skip_example the pruner just did -- so nothing compiled differently and only the + payload grew (22 families x 33 examples on one dcd_dwc2.c diff). That is exactly the + kind of drift no build failure ever reports.""" + + def test_a_device_only_port_diff_still_omits_families_it_cannot_narrow(self): + # dcd_dwc2.c selects device+dual examples only, but a family whose host examples + # are all unbuildable anyway ends up wanting its entire buildable set + b = ci_select.classify_build(['src/portable/synopsys/dwc2/dcd_dwc2.c'], REPO) + self.assertFalse(b['full']) + self.assertTrue(b['families']) + omitted = [f for f in b['families'] if f not in b['family_examples']] + self.assertTrue(omitted, 'no family omitted its -e list; the "already everything ' + 'this family builds" case stopped being detected') + for fam in omitted: + self.assertNotIn(fam, b['family_examples']) + + def test_a_family_that_can_build_more_than_the_diff_wants_keeps_its_list(self): + # the other direction: one example selects itself and nothing else, so every + # family it lands on must carry an explicit -e or CI builds all 46 + b = ci_select.classify_build(['examples/device/cdc_msc/src/main.c'], REPO) + self.assertFalse(b['full']) + for fam in b['families']: + self.assertEqual(b['family_examples'].get(fam), ['device/cdc_msc'], fam) + + +class TestSelectionBehavioursThatHadNoTest(unittest.TestCase): + """Five behaviours a reviewer's mutation pass proved were unpinned: break each one + and the whole suite stayed green. Each test here fails against its mutant. + + They are grouped because they share a shape - every one is a small expression whose + removal silently NARROWS the selection, which is the failure direction that merges a + regression rather than wasting a runner.""" + + def test_build_defines_reach_the_prefilter(self): + # mutant: `defines = ()` in build.py's build_boards_list. metro_m4_express gets + # MAX3421_HOST=1 from its roster variant, never from its BSP, so without the + # defines the -e prefilter drops the rig's only MAX3421 firmware and hil-tinyusb + # has nothing to flash. + import build as build_py, build_utils, inspect + src = inspect.getsource(build_py.build_boards_list) + self.assertIn('defines = tuple(sorted(build_defines))', src, + 'the -D tokens must reach cmake_board/skip_example') + old = os.getcwd() + os.chdir(REPO) + try: + ex, board = 'dual/host_info_to_device_cdc', 'metro_m4_express' + self.assertTrue(build_utils.skip_example(ex, board), + 'without the define this example is correctly skipped') + self.assertFalse(build_utils.skip_example(ex, board, ('MAX3421_HOST=1',)), + 'with it, it must build - that is what the roster passes') + finally: + os.chdir(old) + + def test_one_first_prefers_a_board_that_can_build_the_filter(self): + # mutant: buildable() -> True, i.e. back to all_boards[0]. lpc54's first board + # skips every msc_file_explorer example, so the leg would compile nothing. + import build as build_py + old_env, old = os.environ.get('GITHUB_ACTIONS'), os.getcwd() + os.environ['GITHUB_ACTIONS'] = 'true' + os.chdir(REPO) + try: + unfiltered = build_py.get_family_boards('lpc54', False, True) + filtered = build_py.get_family_boards('lpc54', False, True, + ['host/msc_file_explorer']) + self.assertEqual(unfiltered, ['lpcxpresso54114'], 'unfiltered pick must not move') + self.assertNotEqual(filtered, unfiltered, + 'the -e pick must avoid a board that skips the whole filter') + import build_utils + self.assertFalse(build_utils.skip_example('host/msc_file_explorer', filtered[0]), + f'{filtered[0]} must actually build the filtered example') + finally: + os.chdir(old) + if old_env is None: + os.environ.pop('GITHUB_ACTIONS', None) + else: + os.environ['GITHUB_ACTIONS'] = old_env + + def test_a_class_file_selects_its_own_macro_not_just_the_directory(self): + # mutant: delete the _CLS_STEM_RE block. src/class/midi holds MIDI 1.0 AND 2.0; + # examples/device/midi2_device is the only example enabling CFG_TUD_MIDI2 and the + # only one that compiles midi2_device.c, but the directory macro alone misses it. + got = ci_select._build_class_examples('midi', 'midi2_device.c', {'device'}, REPO) + self.assertIn('device/midi2_device', got, + 'a midi2 change must select the example that compiles it') + host = ci_select._build_class_examples('midi', 'midi2_host.c', {'host'}, REPO) + self.assertIn('host/midi2_host', host) + # and the plain midi files must NOT drag midi2 in + plain = ci_select._build_class_examples('midi', 'midi_device.c', {'device'}, REPO) + self.assertNotIn('device/midi2_device', plain) + + def test_a_port_change_selects_the_dual_examples(self): + # mutant: drop `+ ('dual',)`. A dcd/hcd change must build the dual examples - + # they exercise both stacks on one board, so a dwc2 break lands there first. + s = ci_select.classify_build(['src/portable/synopsys/dwc2/dcd_dwc2.c'], REPO) + duals = {e for exs in s['family_examples'].values() for e in exs + if e.startswith('dual/')} + self.assertTrue(duals, 'a dcd change selected no dual example') + + def test_the_selector_answers_the_same_with_and_without_ci_env(self): + # mutant: drop ci=True from _prune_buildable. ci_skip_boards/ci_preferred_boards + # only apply when GITHUB_ACTIONS/CIRCLECI is set, so without the pin a laptop and + # a runner disagree - and /pre-pr would report a family list CI will not build. + files = ['examples/host/cdc_msc_hid_freertos/src/main.c'] + old = os.environ.get('GITHUB_ACTIONS') + os.environ.pop('GITHUB_ACTIONS', None) + try: + local = ci_select.classify_build(files, REPO)['families'] + os.environ['GITHUB_ACTIONS'] = 'true' + import importlib + importlib.reload(ci_select) + runner = ci_select.classify_build(files, REPO)['families'] + finally: + if old is None: + os.environ.pop('GITHUB_ACTIONS', None) + else: + os.environ['GITHUB_ACTIONS'] = old + import importlib + importlib.reload(ci_select) + self.assertEqual(local, runner, 'the selector must not depend on the CI env vars') + + +class TestRuleTableIsCarbonOfTheSpec(unittest.TestCase): + """ci_select's module docstring carries the rule table so a reader landing in the + code does not have to open the spec to learn what rule 6 is. Both are maintained by + hand, so this pins them cell-for-cell: edit one without the other and this fails. + + It also pins the table against the CODE - every rule id the docstring claims must + appear as a `# rule N` marker on a branch of _classify_build_one, so a row cannot be + documented without a branch, or a branch renumbered without the table.""" + + @staticmethod + def _rows(text): + import re as _re + out = [] + for l in text.splitlines(): + if not l.startswith('| '): + continue + c = [x.strip() for x in l.strip().strip('|').split('|')] + if len(c) == 5 and _re.fullmatch(r'\d+[a-z]?', c[0]): + out.append(c) + return out + + def test_docstring_table_matches_the_spec(self): + spec = open(os.path.join( + REPO, 'docs/superpowers/specs/2026-08-19-ci-build-family-filter-design.md')).read() + doc, spec_rows = self._rows(ci_select.__doc__), self._rows(spec) + self.assertTrue(spec_rows, 'no rule table found in the spec') + self.assertEqual([r[0] for r in doc], [r[0] for r in spec_rows], + 'rule ids differ between ci_select.__doc__ and the spec') + for d, s in zip(doc, spec_rows): + self.assertEqual(d, s, f'rule {d[0]} differs between the docstring and the spec') + + def test_every_documented_rule_has_a_branch(self): + import re as _re + src = open(os.path.join(REPO, 'tools/ci_select.py')).read() + marked = set() + # handles `# rule 6`, `# rules 1, 1b` and `# rules 8-10` + for m in _re.finditer(r'#\s*rules?\s+([0-9a-z, -]+)', src): + for tok in _re.split(r',\s*', m.group(1).strip()): + rng = _re.fullmatch(r'(\d+)\s*-\s*(\d+)', tok.strip()) + if rng: + marked.update(str(n) for n in range(int(rng.group(1)), int(rng.group(2)) + 1)) + elif _re.fullmatch(r'\d+[a-z]?', tok.strip()): + marked.add(tok.strip()) + documented = {r[0] for r in self._rows(ci_select.__doc__)} + missing = sorted(documented - marked, key=lambda s: (int(_re.match(r'\d+', s).group()), s)) + self.assertEqual(missing, [], f'documented rules with no `# rule N` branch marker: {missing}') + + class TestNoTrackedFileIsUnclassified(unittest.TestCase): """Rule 17 (unclassified -> full on both axes) is the fail-open net for paths nobody anticipated. It must stay that way - a wrong `full` costs runner minutes and is @@ -1477,10 +1656,17 @@ class TestBuildPostFilter(unittest.TestCase): # the accepted net for a break outside its #if guard). src/class/bth is the # live instance of this state today; TestClassesWithNoEnablingExample pins the # whole set, so a new one cannot appear unnoticed. - s = ci_select.classify_build(['src/class/vendor/vendor_host.c'], REPO) + # src/class/bth/bth_device.c, a file that EXISTS: the old assertion named + # src/class/vendor/vendor_host.c, deleted by the same branch, so any made-up + # path reached the same branch and the test passed vacuously. + real = os.path.join(REPO, 'src/class/bth/bth_device.c') + self.assertTrue(os.path.isfile(real), 'the case needs a file that exists') + s = ci_select.classify_build(['src/class/bth/bth_device.c'], REPO) self.assertFalse(s['full']) self.assertEqual(s['families'], []) self.assertTrue(any('no contribution' in r for r in s['reasons']), s['reasons']) + # and the reason must name the class, not just any empty answer + self.assertTrue(any('bth' in r for r in s['reasons']), s['reasons']) def test_class_source_with_examples_still_scopes(self): s = ci_select.classify_build(['src/class/cdc/cdc_device.c'], REPO) @@ -2040,14 +2226,14 @@ class TestMcuTokensResolve(unittest.TestCase): # produce, or a rename nobody followed through. `family:samd21` was one of these # until the nine examples/host/*/only.txt files were corrected to samd2x_l2x. # - # The `mcu:` entries are NOT all harmless. MIMXRT10XX/MIMXRT11XX and LPC177X_8X sit - # beside a live token in the same file, so they gate nothing either way. MKL25ZXX - # (device/msc_dual_lun) and SAME5X (device/audio_test) do not: those skips are dead, - # and both examples are built today on the boards their skip file meant to exclude - - # successfully, which is why nobody noticed. Correcting them REMOVES working build - # coverage, so it is a maintainer call, not a drive-by fix. + # The remaining `mcu:` entries sit beside a live token in the same file, so they gate + # nothing either way. MKL25ZXX (7 files) and SAME5X (1) were dead too, but unlike + # these they were the ONLY token for their board - the examples were already being + # built on the very boards those lines meant to exclude. Dropping them is a no-op for + # the build (verified per example) and was chosen over re-pointing, which would have + # removed working coverage. UNREACHABLE_TOKENS = { - 'mcu': {'LPC177X_8X', 'MIMXRT10XX', 'MIMXRT11XX', 'MKL25ZXX', 'SAME5X', 'STM32U3'}, + 'mcu': {'LPC177X_8X', 'MIMXRT10XX', 'MIMXRT11XX', 'STM32U3'}, 'family': set(), 'board': set(), } diff --git a/tools/build.py b/tools/build.py index eeefca22d..0bb366e3d 100755 --- a/tools/build.py +++ b/tools/build.py @@ -356,11 +356,11 @@ def get_family_boards(family, one_random, one_first, examples=None, build_system # 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 + # the whole preferred list, in order. Unreachable-when-unfiltered: with + # examples is None, buildable() is True and the loop returns on entry one. 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: return [candidates[0]] diff --git a/tools/ci_select.py b/tools/ci_select.py index 89a0d214c..53fbcd3a1 100755 --- a/tools/ci_select.py +++ b/tools/ci_select.py @@ -13,6 +13,38 @@ JSON: full, boards (name -> 'all' | [tests]), families (bsp families the diff touches, including ones with no rig board - build-only consumers such as /pre-pr sample from these), args (hil_test.py args per config) and args_flasher (the same args split by each board's flasher, for CI legs that split one rig by flasher). + +THE RULE TABLE. First match wins; answers union per family (build) and per board +(HIL). A CARBON COPY of the table in the design spec above - edit both, or +TestRuleTableIsCarbonOfTheSpec fails. `FAM` = the families whose family.cmake +references the changed path (CMake only; make follows it). `DEV`/`HOST`/`DUAL`/ +`TYPEC`/`ALL` are the example role sets. The Build families column is PRE-PRUNE: +_prune_buildable then intersects each family with what it can actually build. + +| # | Changed path | Build families | Build examples | HIL boards → tests | +| 1 | `docs/`, `.claude/`, `*.md`, `*.rst`, `LICENSE` | — | — | — | +| 1b | `.gitignore`, `.clang-format`, `.idea/**`, `test/{fuzz,unit-test}/**`, non-build `.github/**`, packaging manifests | — | — | — | +| 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 | +| 12b | `src/typec/**` | `ALL` | examples enabling `CFG_TUC_ENABLED` | — (no rig board runs a typec test) | +| 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,build_utils,ci_select}.py`, `tools/cmake/**`, `src/CMakeLists.txt`, `src/tinyusb.mk`, `hw/bsp/{family_support.{cmake,mk},family_rules.mk,zephyr_board_aliases.cmake,board.c,board_api.h,ansi_escape.h}`, `.github/**`, `.circleci/**` | `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 (no tracked file reaches this — TestNoTrackedFileIsUnclassified) | `ALL` | `ALL` | all boards → all tests (fail-open) | """ import argparse import ast @@ -53,7 +85,10 @@ def _read(path: str) -> str: _NONCODE_RE = re.compile( - r'^(docs/|\.claude/|.*\.(md|rst)$|LICENSE)') + # LICENSE is anchored and LICENSES/ named separately: a bare `LICENSE` alternative + # also swallowed anything merely STARTING with it (a future LICENSE_extra.c), + # which is the silent-under-selection direction + r'^(docs/|\.claude/|.*\.(md|rst)$|LICENSE$|LICENSES/)') # Repo metadata and tooling that no CI build reads. Enumerated rather than left to # rule 17, which widens BOTH axes: a PR touching only .gitignore and a README was # creating 74 cmake legs (each a runner doing checkout + toolchain + get_deps before @@ -152,10 +187,20 @@ def board_tests(board: dict) -> list: return [x for x in run if x not in t.get('skip', [])] + +def _rg(repo_root: str, *parts: str) -> str: + """A glob pattern rooted at repo_root, with the ROOT escaped and the parts left as + patterns. The root is a filesystem path, not a pattern: a checkout at + /w/pr[1]/tinyusb (a worktree named after a PR, a CI workspace with brackets) makes + an unescaped '[1]' a character class that matches nothing, and every lookup below + then resolves to zero - families=0 instead of 30, i.e. the selector fails CLOSED + and the whole matrix compiles nothing while reporting green.""" + return os.path.join(glob.escape(repo_root), *parts) + # cached: called per changed file x roster board, and the tree doesn't change mid-run @functools.lru_cache(maxsize=None) def board_family(board_name: str, repo_root: str): - hits = glob.glob(os.path.join(repo_root, 'hw/bsp/*/boards', board_name)) + hits = glob.glob(_rg(repo_root, 'hw/bsp/*/boards', board_name)) return os.path.basename(os.path.dirname(os.path.dirname(hits[0]))) if hits else None @@ -263,10 +308,10 @@ def _family_file_texts(repo_root: str) -> tuple: 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') + bsp_root = os.path.join(repo_root, 'hw/bsp') # escaped by _rg below out = [] - for f in sorted(glob.glob(os.path.join(bsp_root, '*/family.cmake')) + - glob.glob(os.path.join(bsp_root, '*/components/*/CMakeLists.txt'))): + for f in sorted(glob.glob(_rg(bsp_root, '*/family.cmake')) + + glob.glob(_rg(bsp_root, '*/components/*/CMakeLists.txt'))): try: out.append((os.path.relpath(f, bsp_root).split(os.sep, 1)[0], _read(f))) except OSError: @@ -386,7 +431,7 @@ def class_include_edges(repo_root: str) -> dict: Derived from the actual #include lines rather than a hand-written table so it cannot rot when a class picks up or drops a cross-class include.""" edges = {} - for f in sorted(glob.glob(os.path.join(repo_root, 'src/class/*/*.[ch]'))): + for f in sorted(glob.glob(_rg(repo_root, 'src/class/*/*.[ch]'))): cls = os.path.basename(os.path.dirname(f)) try: text = _read(f) @@ -470,11 +515,23 @@ def _class_roles(base: str) -> set: return {'device', 'host'} -def _config_enables(cfg_path: str, macros) -> bool: +@functools.lru_cache(maxsize=None) +def _config_text(cfg_path: str) -> str: + """An example's tusb_config.h, read once. Every class path re-asks the same 46 + configs on both axes, so the reads go up with the diff: 4,240 of the same 46 files + for a diff touching all of src/class (0.48s -> 0.13s), and they cannot change + mid-run. Cached here rather than on _config_enables so the macros argument stays an + ordinary list at every call site.""" try: with open(cfg_path, encoding='utf-8', errors='replace') as f: - text = f.read() + return f.read() except OSError: + return '' + + +def _config_enables(cfg_path: str, macros) -> bool: + text = _config_text(cfg_path) + if not text: return False for m in macros: for value in re.findall(_DEF_VALUE.format(m), text, re.M): @@ -511,10 +568,13 @@ def lib_examples(lib_name: str, repo_root: str) -> set: pat = re.compile(re.escape('lib/' + lib_name) + r'(?=[/\s"\')}]|$)', re.M) out = set() for ex in all_examples(repo_root): - for f in sorted(glob.glob(os.path.join(repo_root, 'examples', ex, '**', '*'), + # the two filenames directly: '**/*' enumerated 489 entries per lib against a + # clean tree to use 107, and grows without bound once `make BOARD=... all` has + # written examples///_build/ - which is where /pre-pr runs + for f in sorted(glob.glob(_rg(repo_root, 'examples', ex, '**', 'CMakeLists.txt'), + recursive=True) + + glob.glob(_rg(repo_root, 'examples', ex, '**', 'Makefile'), recursive=True)): - if os.path.basename(f) not in ('CMakeLists.txt', 'Makefile'): - continue try: text = _read(f) except OSError: @@ -580,7 +640,7 @@ def _classify_one(path, repo_root, roster_boards, extras: set, s: _Sel, if _NONCODE_RE.match(path) or _META_RE.match(path): s.reasons.append(f'{path}: non-code, no contribution') return - if _METRICS_RE.match(path): + if _METRICS_RE.match(path): # rule 2b s.reasons.append(f'{path}: build-size metrics tooling, no HIL contribution') return if _FULL_RE.match(path): @@ -903,7 +963,13 @@ def main(): print(f'ci_select[build]: {r}', file=sys.stderr) for r in s['reasons']: print(f'ci_select: {r}', file=sys.stderr) - print(json.dumps(s)) + # reasons go to stderr ONLY - they are a human diagnostic and no consumer reads them + # back. They are also ~97% of the payload (a whole-tree diff: 453 KB -> 12 KB), which + # build.yml re-parses with ci_set_matrix, hil_ci_set_matrix, an inline python and + # three jq calls. The in-process dicts still carry them, for the log and the tests. + out = {k: v for k, v in s.items() if k != 'reasons'} + out['build'] = {k: v for k, v in s['build'].items() if k != 'reasons'} + print(json.dumps(out)) # ------------------------------------------------------------- @@ -925,7 +991,7 @@ def all_examples(repo_root: str) -> tuple: """Every examples// with a CMakeLists.txt, as 'role/name'.""" out = [] for role in _EX_ROLES: - for d in sorted(glob.glob(os.path.join(repo_root, 'examples', role, '*/'))): + for d in sorted(glob.glob(_rg(repo_root, 'examples', role, '*/'))): if os.path.isfile(os.path.join(d, 'CMakeLists.txt')): out.append(f'{role}/{os.path.basename(d.rstrip(os.sep))}') return tuple(out) @@ -979,13 +1045,13 @@ class _BSel: def _classify_build_one(path, repo_root, s: _BSel, get_deps_families=None): base = os.path.basename(path) - if _NONCODE_RE.match(path) or _META_RE.match(path): # rule 1 + if _NONCODE_RE.match(path) or _META_RE.match(path): # rules 1, 1b s.reasons.append(f'{path}: non-code, no build contribution') return if re.match(r'test/hil/', path): # rule 2 s.reasons.append(f'{path}: HIL harness, no build contribution') return - if path == GET_DEPS_PATH: # get_deps rule + if path == GET_DEPS_PATH: # rule 16b if get_deps_families is None: s.force_full(f'{path}: dep changes not resolvable -> full build matrix') return @@ -1002,6 +1068,7 @@ def _classify_build_one(path, repo_root, s: _BSel, get_deps_families=None): roles = _port_roles(base) exs = 'all' if roles == {'device', 'host'} else \ role_examples(repo_root, tuple(roles) + ('dual',)) + # rule 5b: fams empty -> s.add iterates nothing -> no contribution s.add(fams, exs, f'{path}: port {port} -> families {sorted(fams)}') return if re.match(r'hw/bsp/[^/]+/', path): # rule 6 @@ -1064,7 +1131,7 @@ def _classify_build_one(path, repo_root, s: _BSel, get_deps_families=None): s.add(all_bsp_families(repo_root), exs, f'{path}: typec -> {sorted(exs)}') return m = re.match(r'lib/([^/]+)/', path) - if m: # lib rule + if m: # rule 16a lib = m.group(1) exs = lib_examples(lib, repo_root) if not exs: @@ -1150,11 +1217,25 @@ def _prune_buildable(fams, fam_ex, repo_root): # for anything else spins up CI's most expensive leg to skip every example # it was given. Identical to the unfiltered list on all 81 other families. pool = set(build_py.get_examples(fam)) + + # asked per example instead of materialising the family's whole buildable + # list: skip_example is by far the hottest call in the selector, and every + # question below short-circuits (one cdc_device.c diff: 6,883 calls -> 1,889) + def can_build(ex): + # EITHER build system: this one list gates CircleCI's make legs too, and + # the two answer differently (build_utils.skip_example) + return ex in pool and any( + not build_utils.skip_example(ex, b) or + not build_utils.skip_example(ex, b, (), 'make') for b in boards) + + want = fam_ex.get(fam) try: - buildable = [e for e in allex if e in pool and - any(not build_utils.skip_example(e, b) or - not build_utils.skip_example(e, b, (), 'make') - for b in boards)] + if want is None: + kept = None if any(can_build(e) for e in allex) else [] + else: + kept = [e for e in want if can_build(e)] + if kept and not any(can_build(e) for e in allex if e not in want): + kept = None # already everything the family can build except OSError as e: # a family mid-bring-up (boards/ but no family.cmake/family.mk yet) # reads as unbuildable to the scrape; keep it rather than tracebacking @@ -1162,13 +1243,10 @@ def _prune_buildable(fams, fam_ex, repo_root): reasons.append(f'{fam}: mcu scrape unreadable ({e}), kept unfiltered') out_fams.append(fam) continue - want = fam_ex.get(fam) - have = set(buildable) - kept = buildable if want is None else [e for e in want if e in have] - if not kept: + if kept == []: continue # this diff builds nothing for this family out_fams.append(fam) - if set(kept) != set(buildable): + if kept is not None: out_ex[fam] = kept return out_fams, out_ex, reasons -- cgit v1.3.1 From b610ff039bafa1040c19d6a11cb04adcb22936e5 Mon Sep 17 00:00:00 2001 From: Ha Thach Date: Tue, 25 Aug 2026 10:35:45 +0700 Subject: ci_select: fix the membrowse test's env dependence, and stop HIL unit tests taking the rig (#3846) test_the_upload_board_can_diverge_from_the_built_board called get_family_boards without ci=True, so it pinned the developer's set, not the runner's: the CI skip lists move the one-first pick on three families. It held locally and went red on its first CI run. Pass ci=True, as _prune_buildable already does, and pin the runner's twelve. Rule 2 is a bare test/hil/ prefix, so the harness's own unit tests booked the full 27-board rig for diffs that cannot reach it. Carve test/hil/test/** out to rule 1b, beside test/{fuzz,unit-test}/**; the harness itself is untouched. A test pins that directory's file list, so anything added there that the rig does read fails rather than silently skipping hardware. Rule table updated in the spec and its carbon in the docstring. --- .../2026-08-19-ci-build-family-filter-design.md | 10 ++++- test/hil/test/test_ci_metrics.py | 18 ++++++--- test/hil/test/test_ci_select.py | 47 ++++++++++++++++++++++ tools/ci_select.py | 10 +++-- 4 files changed, 75 insertions(+), 10 deletions(-) (limited to 'tools/ci_select.py') 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 524568aeb..799c83c23 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 @@ -49,8 +49,8 @@ never inflates one axis with another's breadth. | # | Changed path | Build families | Build examples | HIL boards → tests | | --- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------- | ----------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------- | | 1 | `docs/`, `.claude/`, `*.md`, `*.rst`, `LICENSE` | — | — | — | -| 1b | `.gitignore`, `.clang-format`, `.idea/**`, `test/{fuzz,unit-test}/**`, non-build `.github/**`, packaging manifests | — | — | — | -| 2 | `test/hil/**` | — | — | all boards → all tests | +| 1b | `.gitignore`, `.clang-format`, `.idea/**`, `test/{fuzz,unit-test}/**`, `test/hil/test/**`, non-build `.github/**`, packaging manifests | — | — | — | +| 2 | `test/hil/**` (not `test/hil/test/**`) | — | — | 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 | @@ -75,6 +75,12 @@ never inflates one axis with another's breadth. **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. +`test/hil/test/**` is carved out to rule 1b: it holds the harness's own unit tests, which +nothing on the rig runs (pre-commit does, and `build.yml` runs `test_ci_select.py` as the +gate before trusting a selection). A bare `test/hil/` prefix was booking the full 27-board +rig for diffs that cannot reach it. The carve-out is a claim about that directory's +contents, so a test pins its file list: add anything the rig reads and it fails. + **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 diff --git a/test/hil/test/test_ci_metrics.py b/test/hil/test/test_ci_metrics.py index aac251824..6f1511913 100644 --- a/test/hil/test/test_ci_metrics.py +++ b/test/hil/test/test_ci_metrics.py @@ -532,7 +532,12 @@ class TestWorkflowSelectionHandOff(unittest.TestCase): --one-first with no -e returns preferred_list[0]; with one it returns the first preferred board that can build it. Where those differ, the Membrowse Upload step - configures a build dir the Build step never wrote.""" + configures a build dir the Build step never wrote. + + ci=True unconditionally, as _prune_buildable does and for the same reason: the + answer must be the runner's, not the developer's. The CI skip lists are off by + default locally, which moves the pick on three families - this test asserted the + local set and went red on its first CI run.""" sys.path.insert(0, os.path.join(REPO, 'tools')) import build as build_py roles = ('device', 'host', 'dual') @@ -547,14 +552,16 @@ class TestWorkflowSelectionHandOff(unittest.TestCase): diverging = set() for fam in fams: try: - base = build_py.get_family_boards(fam, False, True, None, 'cmake', ()) + base = build_py.get_family_boards(fam, False, True, None, 'cmake', + (), ci=True) except Exception: continue if not base: continue for e in exs: try: - one = build_py.get_family_boards(fam, False, True, [e], 'cmake', ()) + one = build_py.get_family_boards(fam, False, True, [e], 'cmake', + (), ci=True) except Exception: continue if one and one[0] != base[0]: @@ -562,8 +569,9 @@ class TestWorkflowSelectionHandOff(unittest.TestCase): break finally: os.chdir(cwd) - self.assertEqual(diverging, {'imxrt', 'lpc11', 'lpc18', 'lpc54', 'mcx', 'rp2040', - 'rx', 'samd11', 'stm32l0', 'stm32l4', 'tm4c'}, + self.assertEqual(diverging, {'imxrt', 'lpc11', 'lpc18', 'lpc54', 'mcx', 'rx', + 'samd11', 'samd2x_l2x', 'samd5x_e5x', 'stm32l0', + 'stm32l4', 'tm4c'}, 'the set of families whose membrowse upload can land on an ' 'uncompiled board changed; re-check whether dropping $EX_ARGS ' 'from the upload step is still the right trade') diff --git a/test/hil/test/test_ci_select.py b/test/hil/test/test_ci_select.py index dc10f769a..c34bccd1f 100644 --- a/test/hil/test/test_ci_select.py +++ b/test/hil/test/test_ci_select.py @@ -942,6 +942,53 @@ class TestClassesWithNoEnablingExample(unittest.TestCase): 'both axes, so nothing compiles it until the next master push') +class TestTheHarnessTestsAreNotTheHarness(unittest.TestCase): + """test/hil/test/ selects nothing; test/hil/ itself still selects everything. + + Rule 2 is a bare `test/hil/` prefix, so the harness's own unit tests were booking + the full 27-board rig - ~11 minutes of exclusive hardware for a diff that cannot + reach it. Nothing on the rig runs them: pre-commit does, and build.yml runs + test_ci_select.py as the gate before trusting a selection at all. + + The carve-out is only safe while that directory holds nothing rig-affecting, which + is what the second test pins.""" + + def test_the_harness_own_tests_select_nothing_on_either_axis(self): + for p in ('test/hil/test/test_ci_select.py', 'test/hil/test/test_ci_metrics.py', + 'test/hil/test/test_hil_bounded.py', 'test/hil/test/stubs/pymtp.py'): + s = ci_select.classify([p], REPO, ROSTERS) + self.assertFalse(s['full'], p) + self.assertFalse(s['boards'], p) + b = ci_select.classify_build([p], REPO) + self.assertFalse(b['full'], p) + self.assertFalse(b['families'], p) + + def test_the_harness_itself_still_takes_the_whole_rig(self): + # the thing rule 2 exists for: these decide what the rig does, so they cannot be + # trusted to narrow their own blast radius + for p in ('test/hil/hil_test.py', 'test/hil/tinyusb.json', + 'test/hil/helper/hil_ci_set_matrix.py'): + s = ci_select.classify([p], REPO, ROSTERS) + self.assertTrue(s['full'], f'{p} must still force the full rig') + + def test_nothing_rig_affecting_has_moved_into_the_carve_out(self): + """The carve-out is a claim about that directory's contents; pin them. + + A new file there that the rig DOES read would silently stop selecting the rig. + Listing them costs one line per file and makes that a failing test instead.""" + out = subprocess.run(['git', 'ls-files', 'test/hil/test'], cwd=REPO, + capture_output=True, text=True, check=True) + self.assertEqual(sorted(out.stdout.split()), [ + 'test/hil/test/stubs/pymtp.py', + 'test/hil/test/test_ci_metrics.py', + 'test/hil/test/test_ci_select.py', + 'test/hil/test/test_hil_bounded.py', + 'test/hil/test/test_hil_health.py', + 'test/hil/test/test_hil_util.py', + ], 'test/hil/test/ gained or lost a file; it is carved out of rule 2, so confirm ' + 'the rig still does not read anything in there before updating this list') + + class TestExampleMapOmitsFullFamilies(unittest.TestCase): """A family whose selection is ALREADY everything it can build carries no -e list. diff --git a/tools/ci_select.py b/tools/ci_select.py index 53fbcd3a1..ca9d54c27 100755 --- a/tools/ci_select.py +++ b/tools/ci_select.py @@ -23,8 +23,8 @@ _prune_buildable then intersects each family with what it can actually build. | # | Changed path | Build families | Build examples | HIL boards → tests | | 1 | `docs/`, `.claude/`, `*.md`, `*.rst`, `LICENSE` | — | — | — | -| 1b | `.gitignore`, `.clang-format`, `.idea/**`, `test/{fuzz,unit-test}/**`, non-build `.github/**`, packaging manifests | — | — | — | -| 2 | `test/hil/**` | — | — | all boards → all tests | +| 1b | `.gitignore`, `.clang-format`, `.idea/**`, `test/{fuzz,unit-test}/**`, `test/hil/test/**`, non-build `.github/**`, packaging manifests | — | — | — | +| 2 | `test/hil/**` (not `test/hil/test/**`) | — | — | 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 | @@ -108,7 +108,11 @@ _META_RE = re.compile( r'version\.yml$|SConscript$|' r'.*CMakePresets\.json$|hw/bsp/BoardPresets\.json$|examples/west\.yml$|' r'.*/[0-9]+-tinyusb[^/]*\.rules$|tools/usb_drivers/|tools/codespell/|' - r'test/(fuzz|unit-test)/|' + # test/hil/test/ holds the harness's own unit tests, not the harness: nothing on + # the rig runs them (pre-commit does, and build.yml runs test_ci_select.py as the + # gate before trusting a selection), so they cannot change what the rig does. + # The harness itself stays under _FULL_RE's test/hil/ prefix. + r'test/(fuzz|unit-test)/|test/hil/test/|' # .github, minus the build machinery named in _FULL_RE r'\.github/(FUNDING\.yml$|labeler\.yml$|membrowse_pr_message\.j2$|ISSUE_TEMPLATE/|' r'workflows/(cifuzz|claude|claude-code-review|labeler|membrowse-comment|' -- cgit v1.3.1