From 78c4ff6c30b1b8f485cfddd7e707dcee7e4b4ee5 Mon Sep 17 00:00:00 2001 From: Igor Pecovnik Date: Sat, 2 May 2026 20:47:02 +0200 Subject: [PATCH] validate-board-config: follow `source ${SRC}/config/boards/` inheritance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Validate changed board configs` (PR #9747 CI run 25258999953) failed on the three Ayn-Odin2 derivative boards with: ERROR: config/boards/ayn-odin2mini.csc: BOARDFAMILY: required, ... ERROR: config/boards/ayn-odin2mini.csc: KERNEL_TARGET: required, ... (same pair on ayn-odin2portal.csc and ayn-thor.csc) The validator's header explicitly says it does NOT source the file ("boards have side-effecty function bodies"), and the regex `parse_assignments` only sees top-level `KEY=value` lines. Boards that consist of one `source "${SRC}/config/boards/.csc"` line plus a handful of overrides (BOARD_NAME, BOARD_VENDOR, BOARD_MAINTAINER, ARCH) inherit BOARDFAMILY / KERNEL_TARGET / KERNEL_TEST_TARGET from the parent, so the validator saw them as missing and erred out. The errors are pre-existing — they would fire on any future change to those .csc files. PR #9747 just made them visible by being the first PR to touch an inheriting board since the validator landed. Add `collect_inherited_assignments`: when a top-level `source` line points at another file under config/boards/ (matched by a regex anchored to start-of-line so `source` calls inside function bodies — which are always indented — aren't followed), parse that file's top-level assignments and lay them behind the child's own. Child's explicit values still win via dict.setdefault. Recursion is guarded by a visited set keyed on resolved paths, so a self-source or a parent-source-child cycle terminates instead of looping. Missing source targets are silently skipped — the main required-field checks will still flag any field that ends up unset after the merge, so a typo'd source path doesn't mask a real gap. Verified against the five PR-changed files: 6 errors → 0 errors, 4 warnings → 1 warning (the unmaintained-board warning on aml-t95z-plus.tvb, which is unrelated to inheritance and was already there). Self-contained boards (musepipro.conf etc.) produce identical output to before this change. --- tools/validate-board-config.py | 64 ++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/tools/validate-board-config.py b/tools/validate-board-config.py index 22fdb841fe82..424f45f4322a 100755 --- a/tools/validate-board-config.py +++ b/tools/validate-board-config.py @@ -42,6 +42,16 @@ re.MULTILINE, ) +# Top-level `source ${SRC}/config/boards/.csc` (or .conf/.tvb/.wip). +# Anchored to start-of-line so a `source` call inside a function body +# (which is always indented) doesn't get followed. Captures the relative +# path under config/boards/ so the validator can resolve it next to the +# child file being validated. +_SOURCE_RE = re.compile( + r'^source\s+["\']?\$\{?SRC\}?/config/boards/([^"\'\s]+\.(?:conf|csc|tvb|wip))["\']?', + re.MULTILINE, +) + @dataclass @@ -83,6 +93,56 @@ def parse_assignments(text: str) -> dict[str, str]: return out +def collect_inherited_assignments(path: Path, _visited: set[Path] | None = None) -> dict[str, str]: + """Resolve fields a board inherits via `source ${SRC}/config/boards/`. + + Some boards (e.g. ayn-odin2{mini,portal}.csc, ayn-thor.csc) consist + of one `source` line pointing at a base .csc plus a handful of + overrides for the fields they actually change. Fields the child + doesn't redeclare — BOARDFAMILY, KERNEL_TARGET, KERNEL_TEST_TARGET + in the typical case — live in the sourced parent. + + Returns a flat dict of effective fields the parent chain provides, + in the same shape parse_assignments() returns. Caller merges this + behind the child's own dict so the child's explicit values still + win. + + Cycles (a sources b sources a) are guarded by the _visited set; + missing or unresolvable source targets are silently skipped — the + main validator will still flag any field that ends up unset. + """ + if _visited is None: + _visited = set() + try: + resolved = path.resolve() + except OSError: + return {} + if resolved in _visited: + return {} + _visited.add(resolved) + + text = path.read_text(errors="replace") + inherited: dict[str, str] = {} + for m in _SOURCE_RE.finditer(text): + sourced = path.parent / m.group(1) + if not sourced.is_file(): + continue + # Recurse first so transitive parents are merged behind the + # immediate parent's fields. Within a single chain, nearest + # parent's value should win over a more distant ancestor's, + # which falls out naturally from setdefault's "skip if set". + ancestors = collect_inherited_assignments(sourced, _visited) + parent_fields = parse_assignments(sourced.read_text(errors="replace")) + # Parent's own assignments win over its ancestors; merge in + # that order, then those become the inheritance pool the + # caller will lay behind the child. + for k, v in parent_fields.items(): + inherited.setdefault(k, v) + for k, v in ancestors.items(): + inherited.setdefault(k, v) + return inherited + + def validate(path: Path) -> list[Finding]: ext = path.suffix.lstrip(".") if ext == "eos": @@ -90,6 +150,10 @@ def validate(path: Path) -> list[Finding]: text = path.read_text(errors="replace") fields = parse_assignments(text) + # Layer fields the child inherits via `source ${SRC}/config/boards/<...>` + # behind its own — child's explicit value wins, parent fills the gaps. + for k, v in collect_inherited_assignments(path).items(): + fields.setdefault(k, v) findings: list[Finding] = [] fname = str(path)