diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c665a6c7..89bfb6ce 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -11,9 +11,7 @@ on: # 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 -- -# three superseded runs sat ahead of the current one for over an hour. -# cancel-in-progress because a superseded commit's result is not wanted. +# queued, and both competed for the same scarce windows/macos runners. concurrency: group: ${{ github.workflow }}-${{ github.ref }} cancel-in-progress: true @@ -27,10 +25,8 @@ jobs: matrix: python-version: ["3.10", "3.11", "3.12"] # macos-latest, not macos-13: the macos-13 image is retired, so jobs - # requesting it queue forever and never get a runner. Across four runs - # on this branch the ubuntu-22.04 and windows-2022 jobs all started and - # finished while the three macos-13 jobs sat queued for over two hours. - # Every other workflow in this repo already uses macos-latest. + # 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 @@ -61,7 +57,11 @@ jobs: # module named 'tests'" before checking anything. continue-on-error hid # that the type check was doing no work at all. - name: Type check (mypy) - run: mypy . --ignore-missing-imports --no-strict-optional --exclude '^layers/' + # --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/ @@ -75,8 +75,7 @@ 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: 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. diff --git a/ebuild/build/ninja_backend.py b/ebuild/build/ninja_backend.py index 795c7624..5b68984c 100644 --- a/ebuild/build/ninja_backend.py +++ b/ebuild/build/ninja_backend.py @@ -50,6 +50,25 @@ def _ninja_path(path) -> str: return text.replace("$", "$$").replace(":", "$:").replace(" ", "$ ") +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. @@ -202,8 +221,7 @@ def _write_ninja(self) -> None: link_inputs = obj_files + dep_archives out = str(self.build_dir / target.name) lines.append( - f"build {_ninja_path(out)}: link " - f"{' '.join(_ninja_path(i) for i in 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)}") @@ -221,10 +239,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 {_ninja_path(out)}: ar_rule " - f"{' '.join(_ninja_path(o) for o in 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 @@ -240,10 +255,7 @@ def _write_ninja(self) -> None: for lib in pkg.libraries: libs.append(f"-l{lib}") - lines.append( - f"build {_ninja_path(out)}: link " - f"{' '.join(_ninja_path(o) for o in 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/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/ebuild/test_ninja_backend.py b/tests/ebuild/test_ninja_backend.py index 0d4b63aa..d19dd8e9 100644 --- a/tests/ebuild/test_ninja_backend.py +++ b/tests/ebuild/test_ninja_backend.py @@ -53,6 +53,9 @@ def test_shared_library_links_with_the_platform_shared_flag(tmp_path): NinjaBackend(config, tmp_path / "build", toolchain).generate() ninja_file = (tmp_path / "build" / "build.ninja").read_text(encoding="utf-8") + # 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 diff --git a/tests/unit/test_cad_pipeline.py b/tests/unit/test_cad_pipeline.py new file mode 100644 index 00000000..08860167 --- /dev/null +++ b/tests/unit/test_cad_pipeline.py @@ -0,0 +1,150 @@ +# 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" + # 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(): + """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/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"(? 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 @@ -455,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}") @@ -479,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}/")