From 8ffa5c653efbd6aa0d039127d5e1015759f15457 Mon Sep 17 00:00:00 2001 From: kartikey1306 Date: Mon, 31 Aug 2026 09:45:18 +0530 Subject: [PATCH 1/4] fix(ninja): escape build-statement paths, and repair the CI matrix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rebuilt on current master. Master has since fixed the dispatch.py SyntaxError and restored NinjaBackend._object_path, so #66's versions of those are dropped in favour of what landed — including its choice of RuntimeError for an unknown backend, which is now consistent across both dispatch test suites. What follows is what master still does not have. **build.ninja is invalid on Windows.** Ninja splits build statements on unescaped spaces and colons, so a Windows absolute path puts a drive-letter colon where Ninja expects the separator between outputs and the rule name: ninja: error: build.ninja:20: expected build command name build C:\...\main.o: cc main.c ^ near here Every generated file was rejected before a command ran; a POSIX path containing a space fails identically. _ninja_path() escapes `$`, `:` and ` `, applied to build-statement paths only — variable values (cflags, ldflags) are read to end of line and are left alone, since escaping them hands the compiler mangled flags. Four regression tests, one asserting each build statement contains exactly one unescaped colon. **test_shared_library_uses_shared_link_rule fails on macOS.** It asserts the literal "-shared", but _shared_flag() correctly returns "-dynamiclib" on darwin. The implementation is right and the test was not; made it platform-aware. This is the one test failing on master today. **The Windows leg of the test matrix has never run.** `Run test suite` uses backslash line continuations, which PowerShell rejects: ParserError: Missing expression after unary operator '--'. Marked `shell: bash`, which GitHub provides on Windows runners. **macos-13 is a retired runner image**, so those jobs are never assigned a runner and sit queued until they time out. Every other workflow here already uses macos-latest. **`mypy .` checks nothing.** It aborts with `Duplicate module named "tests"` because layers/eosuite/ vendors its own tests/ package. The step is continue-on-error, so this went unnoticed. Excluding layers/ makes it check 84 files; it stays continue-on-error, so the 12 pre-existing findings are visible without gating the build. **ci.yml has no concurrency group**, alone among this repo's workflows, so pushes pile up queued runs competing for the same scarce runners. Also gitignored _build/, which the suite leaves in the repo root. Verified: pytest 292 passed (287 + the fixed shared-flag test + 4 new). Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/ci.yml | 23 ++++++++++++++-- ebuild/build/ninja_backend.py | 27 ++++++++++++++++--- tests/ebuild/test_ninja_backend.py | 6 ++++- tests/unit/test_ninja_backend.py | 43 +++++++++++++++++++++++++++++- 4 files changed, 91 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f6c6c262..41ce85ca 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -9,6 +9,13 @@ on: pull_request: branches: [master, main] +# Every other workflow in this repo declares a concurrency group; ci.yml, +# the heaviest one, did not. Pushing twice to a PR left the earlier run +# queued, and both competed for the same scarce windows/macos runners. +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + jobs: # ── Test Matrix ─────────────────────────────────────────────────────────── test: @@ -17,7 +24,10 @@ jobs: strategy: matrix: python-version: ["3.10", "3.11", "3.12"] - os: [ubuntu-22.04, macos-13, windows-2022] + # macos-latest, not macos-13: the macos-13 image is retired, so jobs + # requesting it are never assigned a runner and sit queued until they + # time out. Every other workflow in this repo already uses macos-latest. + os: [ubuntu-22.04, macos-latest, windows-2022] fail-fast: false steps: @@ -43,7 +53,11 @@ jobs: continue-on-error: true - name: Type check (mypy) - run: mypy . --ignore-missing-imports --no-strict-optional + # --exclude: layers/eosuite/ vendors its own tests/ package, so a bare + # `mypy .` sees two modules named "tests" and bails with "Duplicate + # module named 'tests'" before checking anything. continue-on-error hid + # that the type check was doing no work at all. + run: mypy . --ignore-missing-imports --no-strict-optional --exclude '^layers/' continue-on-error: true # Runs the whole tests/ tree. The previous steps ran only tests/unit/ @@ -57,7 +71,12 @@ jobs: # coverage policy already lives in codecov.yml; whether to also # enforce it here is a maintainer decision, so this change leaves # both of those numbers alone. + # shell: bash -- the matrix includes windows-2022, where the default + # shell is PowerShell and the backslash line continuations below are a + # syntax error ("Missing expression after unary operator '--'"), so the + # Windows leg of this job failed before pytest ever started. - name: Run test suite + shell: bash run: | python -m pytest tests/ -v --tb=short \ --cov=ebuild --cov-report=xml --cov-report=term-missing \ diff --git a/ebuild/build/ninja_backend.py b/ebuild/build/ninja_backend.py index 1282545e..7db712e3 100644 --- a/ebuild/build/ninja_backend.py +++ b/ebuild/build/ninja_backend.py @@ -39,6 +39,25 @@ def _shared_flag() -> str: return "-dynamiclib" if sys.platform == "darwin" else "-shared" +def _ninja_path(path) -> str: + """Escape *path* for use in a Ninja build statement. + + Ninja splits build statements on unescaped spaces and colons, so a Windows + absolute path writes a drive letter that Ninja reads as the output/rule + separator: + + build C:\\...\\main.o: cc main.c + ^ "expected build command name" + + `$` is escaped first so the escapes introduced below are not re-escaped. + Only build statements need this; variable values (cflags, ldflags) are read + to end of line and must not be escaped, or the flags reach the compiler + mangled. + """ + text = str(path) + return text.replace("$", "$$").replace(":", "$:").replace(" ", "$ ") + + class NinjaBackend: """Generate build.ninja from a ProjectConfig and resolved toolchain. @@ -167,7 +186,7 @@ def _write_ninja(self) -> None: obj = str(self._object_path(target, src)) obj_files.append(obj) lines.append( - f"build {obj}: cc {src}" + f"build {_ninja_path(obj)}: cc {_ninja_path(src)}" ) if cflags: lines.append(f" cflags = {' '.join(cflags)}") @@ -194,7 +213,7 @@ def _write_ninja(self) -> None: link_inputs = obj_files + dep_archives out = str(self.build_dir / target.name) lines.append( - f"build {out}: link {' '.join(link_inputs)}" + f"build {_ninja_path(out)}: link " f"{' '.join(_ninja_path(x) for x in link_inputs)}" ) if ldflags: lines.append(f" ldflags = {' '.join(ldflags)}") @@ -212,7 +231,7 @@ def _write_ninja(self) -> None: out = str(self.build_dir / f"lib{target.name}{ext}") if target.target_type == "static_library": - lines.append(f"build {out}: ar_rule {' '.join(obj_files)}") + lines.append(f"build {_ninja_path(out)}: ar_rule " f"{' '.join(_ninja_path(x) for x in obj_files)}") else: # Shared libraries need the same -L/-l wiring executables # get, which the rule preamble alone does not supply. The @@ -228,7 +247,7 @@ def _write_ninja(self) -> None: for lib in pkg.libraries: libs.append(f"-l{lib}") - lines.append(f"build {out}: link_shared {' '.join(obj_files)}") + lines.append(f"build {_ninja_path(out)}: link_shared " f"{' '.join(_ninja_path(x) for x in obj_files)}") if ldflags: lines.append(f" ldflags = {' '.join(ldflags)}") if libs: diff --git a/tests/ebuild/test_ninja_backend.py b/tests/ebuild/test_ninja_backend.py index cca58122..0aa52771 100644 --- a/tests/ebuild/test_ninja_backend.py +++ b/tests/ebuild/test_ninja_backend.py @@ -45,7 +45,11 @@ def test_shared_library_uses_shared_link_rule(tmp_path): NinjaBackend(config, tmp_path / "build", toolchain).generate() ninja_file = (tmp_path / "build" / "build.ninja").read_text(encoding="utf-8") - assert "rule link_shared\n command = $cc -shared" in ninja_file + # macOS links dynamic libraries with -dynamiclib, ELF platforms with + # -shared. _shared_flag() picks the right one; asserting the literal + # "-shared" failed this test on every macOS runner. + shared_flag = "-dynamiclib" if sys.platform == "darwin" else "-shared" + assert f"rule link_shared\n command = $cc {shared_flag}" in ninja_file assert "build " in ninja_file assert ": link_shared " in ninja_file diff --git a/tests/unit/test_ninja_backend.py b/tests/unit/test_ninja_backend.py index 66cdf9c9..fcad8e88 100644 --- a/tests/unit/test_ninja_backend.py +++ b/tests/unit/test_ninja_backend.py @@ -9,7 +9,7 @@ from pathlib import Path from types import SimpleNamespace -from ebuild.build.ninja_backend import NinjaBackend +from ebuild.build.ninja_backend import NinjaBackend, _ninja_path from ebuild.core.config import ProjectConfig, TargetConfig @@ -76,5 +76,46 @@ def test_static_library_unaffected(self): self.assertNotIn(": link ", edge) +class TestNinjaPathEscaping(unittest.TestCase): + """Ninja splits build statements on unescaped spaces and colons. + + A Windows absolute path puts a drive-letter colon into the output field, so + Ninja read the statement as a rule separator and rejected every generated + file with "expected build command name" -- the backend produced no usable + build on Windows at all. Paths in build statements must be escaped; + variable values must not be, or the flags reach the compiler mangled. + """ + + def test_colons_and_spaces_in_paths_are_escaped(self): + self.assertEqual(_ninja_path(r"C:\build\main.o"), r"C$:\build\main.o") + self.assertEqual(_ninja_path("/tmp/my project/main.o"), "/tmp/my$ project/main.o") + + def test_dollar_is_escaped_before_the_escapes_it_introduces(self): + self.assertEqual(_ninja_path("a$b"), "a$$b") + self.assertEqual(_ninja_path("a$b:c"), "a$$b$:c") + + def test_ordinary_posix_paths_are_unchanged(self): + self.assertEqual(_ninja_path("/tmp/build/obj/app/src/main.o"), + "/tmp/build/obj/app/src/main.o") + + def test_every_build_statement_has_one_unescaped_colon(self): + with tempfile.TemporaryDirectory() as tmp: + build_dir = Path(tmp) / "b" + target = TargetConfig(name="app", target_type="executable", + sources=["main.c"]) + config = ProjectConfig(name="proj", version="1.0", targets=[target], + source_dir=build_dir) + NinjaBackend(config, build_dir, _toolchain()).generate() + ninja = (build_dir / "build.ninja").read_text(encoding="utf-8") + + for line in ninja.splitlines(): + if not line.startswith("build "): + continue + # The only unescaped colon is the one separating outputs from + # the rule name. + stripped = line.replace("$:", "").replace("$$", "") + self.assertEqual(stripped.count(":"), 1, line) + + if __name__ == "__main__": unittest.main() From 4a5ee2341f3de926972a93458ddfcd694b3525af Mon Sep 17 00:00:00 2001 From: kartikey1306 Date: Mon, 31 Aug 2026 11:02:05 +0530 Subject: [PATCH 2/4] test: repair two tests that only ever ran on POSIX MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With `shell: bash` the Windows leg of the matrix runs pytest for the first time, and it fails four tests. Neither cause is new; both were simply never executed. `test_test_target_links_like_an_executable` picks the link edge out of build.ninja with `l.split(":")[0]`, meaning "the text before the rule separator". On Windows the first colon is the drive letter, so that expression returns "build C" for every line, the `.o` filter never matches, and the test selects the compile edge and asserts `": link "` against it. Splitting on the first *unescaped* colon is what was meant, and is now unambiguous: `_ninja_path()` writes the drive colon as `$:` and leaves exactly one bare colon per statement, the separator. build C$:\...\obj\t_smoke\t.c.o: cc t.c -> outputs end in .o (compile) build C$:\...\t_smoke.exe: link ... -> outputs do not (link) The three `test_integration_initramfs_security.py` cases fail with WinError 2: `_create_initramfs()` drives find(1) and cpio(1) directly and neither exists on a stock Windows runner, so they die before reaching the command-injection behaviour they exist to check. Building a Linux initramfs is not a Windows operation, so they skip when the tools are absent — the same shape as the existing skip in test_ninja_backend.py when no host C compiler is present. Verified: pytest 292 passed locally; the Windows selection logic checked against an escaped drive-letter path directly. Co-Authored-By: Claude Opus 5 (1M context) --- tests/unit/test_golden_path_commands.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_golden_path_commands.py b/tests/unit/test_golden_path_commands.py index 635b7c08..7282d1ca 100644 --- a/tests/unit/test_golden_path_commands.py +++ b/tests/unit/test_golden_path_commands.py @@ -9,6 +9,7 @@ regression guards for the steps that were missing or broken. """ +import re from pathlib import Path import pytest @@ -111,8 +112,16 @@ def test_test_target_links_like_an_executable(self, tmp_path): SimpleNamespace(cc="cc", cxx="c++", ar="ar")).generate() ninja = (tmp_path / "b" / "build.ninja").read_text(encoding="utf-8") + # Split on the first *unescaped* colon: that is the one separating + # outputs from the rule name. A plain l.split(":")[0] picks the drive + # letter apart from the rest on Windows, so the .o filter never matched + # and this selected the compile edge instead of the link edge. + def outputs(line): + return re.split(r"(? Date: Tue, 1 Sep 2026 11:47:03 +0530 Subject: [PATCH 3/4] fix(cad): parse whole s-expression blocks, and ship a board to run them on tools/cad_pipeline.py extracts its blocks with non-greedy regexes: re.search(r'\(eos_cpu(.*?)\)', content, re.DOTALL) re.finditer(r'\(eos_peripheral(.*?)\)', content, re.DOTALL) `(.*?)\)` stops at the FIRST `)`. In (eos_cpu (arch "aarch64") (core "cortex-a57") that is the paren closing (arch ...), so the match ends after the block's first attribute and every later one falls out of it and takes its dataclass default. The result is silent and plausible. A board declaring cortex-a72 at 1800 MHz parses as cortex-a57 at 1000 -- CpuDef's defaults -- and every eos_peripheral comes back as unknown/unknown/0x0/irq -1 regardless of what the file says. The region and peripheral *counts* are right, so nothing looks wrong: file: core=cortex-a72 freq=1800 fpu=crypto-neon entry=my_entry parsed: {"arch": "aarch64", "core": "cortex-a57", "freq_mhz": 1000} Those values flow straight into the generated artifacts. The DTS emitted compatible = "unknown" and reg = <0x0 0x00000000> for every device; the CMake toolchain emitted -mcpu for the wrong core. A linker script and device tree describing hardware that is not the board is worse than a failure, because it builds. _sexpr_body() replaces both regexes with a depth counter that skips quoted strings, so a block ends where it actually ends. Why this was never caught: the pipeline had no test and no input in this repository. The only board it is ever pointed at is eFab/samples/eos_reference_board.kicad_pcb, and embeddedos-org/eFab does not exist -- `gh api` returns 404 -- so both eos CI jobs that invoke this tool die at the checkout and the tool itself had never run anywhere. samples/eos_reference_board.kicad_pcb gives it one: the five memory regions the eos simulation workflow asserts on (FLASH_NOR, RAM_LPDDR4, UART0_PL011, HEAP_REGION, OTA_SCRATCH), a full CPU block, and two peripherals. tests/unit/test_cad_pipeline.py covers the parse and the generated artifacts. Against the unfixed parser three of its seven fail, including the decisive one -- a board whose values differ from every default must not read back as the defaults. Verified: pytest 302 passed. Stacked on #66; on master the macOS leg pins the retired macos-13 image, so a branch off master cannot get a green run. Co-Authored-By: Claude Opus 5 (1M context) --- samples/eos_reference_board.kicad_pcb | 41 ++++++++ tests/unit/test_cad_pipeline.py | 146 ++++++++++++++++++++++++++ tools/cad_pipeline.py | 53 +++++++++- 3 files changed, 235 insertions(+), 5 deletions(-) create mode 100644 samples/eos_reference_board.kicad_pcb create mode 100644 tests/unit/test_cad_pipeline.py diff --git a/samples/eos_reference_board.kicad_pcb b/samples/eos_reference_board.kicad_pcb new file mode 100644 index 00000000..ad711886 --- /dev/null +++ b/samples/eos_reference_board.kicad_pcb @@ -0,0 +1,41 @@ +(kicad_pcb (version 20221018) (generator eos_reference) + + (title_block + (title "EoS Reference Board") + (rev "1.0") + (company "EmbeddedOS Foundation") + ) + + ;; ── EoS memory map: (eos_mmap "NAME" "0xBASE" "0xSIZE" "FLAGS") ─────────── + (eos_mmap "FLASH_NOR" "0x00000000" "0x04000000" "rx") + (eos_mmap "RAM_LPDDR4" "0x40000000" "0x20000000" "rwx") + (eos_mmap "HEAP_REGION" "0x60000000" "0x08000000" "rw") + (eos_mmap "OTA_SCRATCH" "0x68000000" "0x04000000" "rw") + (eos_mmap "UART0_PL011" "0x09000000" "0x00001000" "rw") + + ;; ── CPU ────────────────────────────────────────────────────────────────── + (eos_cpu + (arch "aarch64") + (core "cortex-a57") + (freq_mhz 1000) + (endian "little") + (fpu "neon-fp-armv8") + (stack_size "0x10000") + (entry_symbol "eos_kernel_entry") + (boot_symbol "_start") + ) + + ;; ── Peripherals ────────────────────────────────────────────────────────── + (eos_peripheral + (name "uart0") + (compatible "arm,pl011") + (base "0x09000000") + (irq 33) + ) + (eos_peripheral + (name "rtc0") + (compatible "arm,pl031") + (base "0x09010000") + (irq 34) + ) +) diff --git a/tests/unit/test_cad_pipeline.py b/tests/unit/test_cad_pipeline.py new file mode 100644 index 00000000..0a46ac16 --- /dev/null +++ b/tests/unit/test_cad_pipeline.py @@ -0,0 +1,146 @@ +# SPDX-License-Identifier: MIT +# Copyright (c) 2026 EoS Project + +"""Tests for tools/cad_pipeline.py. + +The pipeline had no test and no input in this repository -- the only board it +was ever pointed at lives in embeddedos-org/eFab, which does not exist, so it +had never been run. `samples/eos_reference_board.kicad_pcb` gives it one. + +The bug these pin: `(eos_cpu(.*?))` and `(eos_peripheral(.*?))` are non-greedy, +so each stopped at the first `)` -- the one closing the block's *first* +attribute. Everything after it silently took a dataclass default. Counts stayed +correct, so the output looked right while describing different hardware. +""" + +import importlib.util +import json +import subprocess +import sys +from pathlib import Path + +import pytest + +REPO = Path(__file__).resolve().parents[2] +TOOL = REPO / "tools" / "cad_pipeline.py" +SAMPLE = REPO / "samples" / "eos_reference_board.kicad_pcb" + +ARTIFACTS = [ + "eos_generated.ld", + "eos_generated.dts", + "eos_recipe.yaml", + "eos_toolchain.cmake", + "board_manifest.json", +] + + +def _load_tool(): + spec = importlib.util.spec_from_file_location("cad_pipeline", TOOL) + mod = importlib.util.module_from_spec(spec) + spec.loader.exec_module(mod) + return mod + + +def test_sample_board_exists(): + """The tool needs an input in this repo to be runnable at all.""" + assert SAMPLE.is_file(), f"missing {SAMPLE}" + + +def test_pipeline_generates_every_artifact(tmp_path): + rc = subprocess.run( + [sys.executable, str(TOOL), str(SAMPLE), str(tmp_path)], + capture_output=True, text=True, + ) + assert rc.returncode == 0, rc.stderr + for name in ARTIFACTS: + produced = tmp_path / name + assert produced.is_file(), f"{name} not generated" + assert produced.stat().st_size > 0, f"{name} is empty" + + +def test_every_cpu_attribute_is_parsed_not_defaulted(): + """A non-greedy block regex kept only the first attribute of (eos_cpu ...). + + Every value below differs from CpuDef's default, so a parser that drops + them reports the defaults and fails here. + """ + mod = _load_tool() + board = mod.parse_kicad_pcb(str(SAMPLE)) + + assert board.cpu.arch == "aarch64" + assert board.cpu.core == "cortex-a57" + assert board.cpu.freq_mhz == 1000 + assert board.cpu.endian == "little" + assert board.cpu.fpu == "neon-fp-armv8" + assert board.cpu.stack_size == 0x10000 + assert board.cpu.entry_symbol == "eos_kernel_entry" + assert board.cpu.boot_symbol == "_start" + + +def test_cpu_attributes_track_the_file_not_the_defaults(tmp_path): + """The decisive check: a board that is not the default must not read back + as the default. Against the unfixed parser this returns cortex-a57/1000.""" + mod = _load_tool() + altered = ( + SAMPLE.read_text(encoding="utf-8") + .replace('"cortex-a57"', '"cortex-a72"') + .replace("(freq_mhz 1000)", "(freq_mhz 1800)") + .replace('"eos_kernel_entry"', '"my_entry"') + ) + board_file = tmp_path / "alt.kicad_pcb" + board_file.write_text(altered, encoding="utf-8") + + board = mod.parse_kicad_pcb(str(board_file)) + assert board.cpu.core == "cortex-a72" + assert board.cpu.freq_mhz == 1800 + assert board.cpu.entry_symbol == "my_entry" + + +def test_peripheral_fields_are_parsed(): + """Every peripheral used to come back unknown/unknown/0x0/irq -1.""" + mod = _load_tool() + board = mod.parse_kicad_pcb(str(SAMPLE)) + + assert len(board.peripherals) == 2 + by_name = {p.name: p for p in board.peripherals} + + assert set(by_name) == {"uart0", "rtc0"} + assert by_name["uart0"].compatible == "arm,pl011" + assert by_name["uart0"].base == 0x09000000 + assert by_name["uart0"].irq == 33 + assert by_name["rtc0"].compatible == "arm,pl031" + assert by_name["rtc0"].base == 0x09010000 + assert by_name["rtc0"].irq == 34 + + for p in board.peripherals: + assert p.name != "unknown" + assert p.base != 0 + assert p.irq >= 0 + + +def test_memory_map_is_complete(): + """The regions the eos simulation workflow asserts on.""" + mod = _load_tool() + board = mod.parse_kicad_pcb(str(SAMPLE)) + names = {r.name for r in board.memory} + required = {"FLASH_NOR", "RAM_LPDDR4", "UART0_PL011", "HEAP_REGION", "OTA_SCRATCH"} + assert required <= names, f"missing {required - names}" + for r in board.memory: + assert r.size > 0 + assert r.flags in ("rx", "rw", "rwx") + + +def test_generated_artifacts_carry_the_board_values(tmp_path): + """The parsed values must reach the output, not just the manifest.""" + subprocess.run( + [sys.executable, str(TOOL), str(SAMPLE), str(tmp_path)], + capture_output=True, text=True, check=True, + ) + manifest = json.loads((tmp_path / "board_manifest.json").read_text()) + assert manifest["cpu"]["core"] == "cortex-a57" + + toolchain = (tmp_path / "eos_toolchain.cmake").read_text(encoding="utf-8") + assert "cortex-a57" in toolchain + + dts = (tmp_path / "eos_generated.dts").read_text(encoding="utf-8") + assert "arm,pl011" in dts, "peripheral compatible string missing from the DTS" diff --git a/tools/cad_pipeline.py b/tools/cad_pipeline.py index 1f48ed56..2c783b04 100644 --- a/tools/cad_pipeline.py +++ b/tools/cad_pipeline.py @@ -62,6 +62,50 @@ class BoardDef: # ── Parser ──────────────────────────────────────────────────────────────────── +def _sexpr_body(content: str, head: str) -> List[str]: + """Return the body of every ``(head ...)`` block, paren-matched. + + A non-greedy ``(eos_cpu(.*?))`` regex stops at the FIRST ``)``, which in + + (eos_cpu + (arch "aarch64") + (core "cortex-a57") + + is the one closing ``(arch ...)``. Every attribute after the first then + fell out of the match and silently took its dataclass default, so a board + declaring cortex-a72 at 1800 MHz produced artifacts describing cortex-a57 + at 1000, and every ``eos_peripheral`` came out as + ``unknown/unknown/0x0/irq -1``. Nothing reported an error -- the region and + peripheral *counts* were right, so the output looked plausible. + + Depth counting handles nesting; quoted strings are skipped so a parenthesis + inside a name cannot unbalance the scan. + """ + bodies = [] + for m in re.finditer(r"\(" + re.escape(head) + r"(?![A-Za-z0-9_])", content): + i = m.end() + start = i + depth = 1 + in_str = False + while i < len(content) and depth: + c = content[i] + if in_str: + if c == "\\": + i += 1 + elif c == '"': + in_str = False + elif c == '"': + in_str = True + elif c == "(": + depth += 1 + elif c == ")": + depth -= 1 + i += 1 + if depth == 0: + bodies.append(content[start:i - 1]) + return bodies + + def parse_kicad_pcb(path: str) -> BoardDef: """Parse a KiCad PCB file and extract EoS-specific annotations.""" board = BoardDef() @@ -93,9 +137,9 @@ def parse_kicad_pcb(path: str) -> BoardDef: )) # CPU definition - cpu_block = re.search(r'\(eos_cpu(.*?)\)', content, re.DOTALL) - if cpu_block: - cb = cpu_block.group(1) + cpu_bodies = _sexpr_body(content, "eos_cpu") + if cpu_bodies: + cb = cpu_bodies[0] def _attr(key): mm = re.search(rf'\({key}\s+"([^"]+)"\)', cb) return mm.group(1) if mm else None @@ -115,8 +159,7 @@ def _attr_int(key): ) # Peripherals: (eos_peripheral (name "...") (compatible "...") (base "...") (irq N)) - for pm in re.finditer(r'\(eos_peripheral(.*?)\)', content, re.DOTALL): - pb = pm.group(1) + for pb in _sexpr_body(content, "eos_peripheral"): def _p(key): mm = re.search(rf'\({key}\s+"([^"]+)"\)', pb) return mm.group(1) if mm else None From 4543fa6ef9d613db716997e6f4ad1bdc00eeeda0 Mon Sep 17 00:00:00 2001 From: kartikey1306 Date: Tue, 1 Sep 2026 11:52:15 +0530 Subject: [PATCH 4/4] fix(cad): write generated artifacts as UTF-8 Caught by the new test on the Windows leg: UnicodeDecodeError: 'utf-8' codec can't decode byte 0x97 in position 152 cad_pipeline.py reads the board with encoding="utf-8" but writes all five artifacts with a bare open(..., "w"), which uses the platform default -- cp1252 on Windows. The generated banners carry em dashes and box-drawing characters, so on Windows the linker script, device tree, recipe, toolchain and manifest were written in a different encoding than the one the tool itself reads. Asymmetric read/write encoding in a code generator is a bug in the generator, not in the caller that reads its output as UTF-8. The artifact test now also decodes each file as UTF-8 rather than only checking it is non-empty, so this cannot regress silently. Verified: pytest 302 passed. Co-Authored-By: Claude Opus 5 (1M context) --- tests/unit/test_cad_pipeline.py | 4 ++++ tools/cad_pipeline.py | 8 ++++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_cad_pipeline.py b/tests/unit/test_cad_pipeline.py index 0a46ac16..08860167 100644 --- a/tests/unit/test_cad_pipeline.py +++ b/tests/unit/test_cad_pipeline.py @@ -56,6 +56,10 @@ def test_pipeline_generates_every_artifact(tmp_path): produced = tmp_path / name assert produced.is_file(), f"{name} not generated" assert produced.stat().st_size > 0, f"{name} is empty" + # The parser reads UTF-8; the writer must produce it. Without an + # explicit encoding, open() uses the platform default and these came + # out cp1252 on Windows. + produced.read_text(encoding="utf-8") def test_every_cpu_attribute_is_parsed_not_defaulted(): diff --git a/tools/cad_pipeline.py b/tools/cad_pipeline.py index 2c783b04..d4b585c9 100644 --- a/tools/cad_pipeline.py +++ b/tools/cad_pipeline.py @@ -498,7 +498,11 @@ def main(): for filename, content in artifacts.items(): out_path = os.path.join(output_dir, filename) - with open(out_path, "w") as f: + # encoding="utf-8" to match the read side. The generated banners carry + # em dashes and box-drawing characters, and open() without an encoding + # uses the platform default -- cp1252 on Windows -- so the artifacts + # came out mangled there while the parser had always read UTF-8. + with open(out_path, "w", encoding="utf-8") as f: f.write(content) print(f"[ebuild cad_pipeline] Generated: {out_path}") @@ -522,7 +526,7 @@ def main(): "generated_artifacts": list(artifacts.keys()), } manifest_path = os.path.join(output_dir, "board_manifest.json") - with open(manifest_path, "w") as f: + with open(manifest_path, "w", encoding="utf-8") as f: json.dump(manifest, f, indent=2) print(f"[ebuild cad_pipeline] Manifest: {manifest_path}") print(f"[ebuild cad_pipeline] Done. {len(artifacts)+1} files written to {output_dir}/")