fix: restore the package import and reconcile the merged backend behaviour by Kartikey1306 · Pull Request #66 · embeddedos-org/ebuild

Kartikey1306 · 2026-08-28T13:59:11Z

The architectural problem

import ebuild.build.dispatch raises SyntaxError on master. The CLI does not start, and five test modules fail at collection.

Same root cause as the breakage in the sibling repos: overlapping PRs squash-merged on stale bases, with master never re-verified afterwards. Every one of the PRs involved was green on its own branch. What landed is a build tool that cannot be imported, and — once it can be — a default build backend that crashes.

What is broken

`dispatch.py` does not parse

configure() ends with two consecutive else: blocks from an unresolved conflict.

The two blocks also disagree about the exception type, and each has a test suite behind it:

tests/ebuild/test_dispatch.py expects ValueError("Unknown build backend '<name>'")
tests/unit/test_dispatch.py expects RuntimeError matching "ninja"

Both readings are legitimate. An unrecognized backend name is a bad argument; a "ninja" that reaches the dispatcher is a CLI routing failure, since ebuild's own ninja backend is invoked directly and never dispatched here. Rather than pick a winner and delete someone's regression test, this adds UnknownBackendError(ValueError, RuntimeError) with a message that covers both cases, raised from configure(), build() and clean().

"ninja" is also no longer a silent no-op in configure(). That silence is what let ebuild build report "Build completed successfully" without ever running a compiler — the exact failure tests/unit/test_dispatch.py was written to prevent.

The default build backend crashes

NinjaBackend._object_path() was deleted by a cflags refactor while both of its callers survived, so generate() dies with AttributeError. Restored, with its target-namespaced object paths — the thing that stops two targets sharing a source file from claiming the same output and making ninja reject the graph with multiple rules generate ....

This also un-breaks the depfile tests from #48: header edits currently leave stale objects behind and the build silently reports success.

Two incompatible shared-library designs both merged

One PR added a dedicated link_shared ninja rule; another put the platform's shared-object flag into ldflags on the generic link rule. Both landed, with a test each, and the tests contradict — one asserts : link_shared , the other asserts : link plus no -shared anywhere in the file.

The link_shared rule was dead: nothing emitted a build line using it, and it hardcoded -shared, which is wrong on macOS (-dynamiclib) and skipped the -L/-l wiring. Dropped it, and rewrote the test that asserted the dead rule to cover the surviving, platform-correct behaviour.

The Windows matrix has never run — and the backend has never worked there

The test matrix includes windows-2022, where the default shell is PowerShell. The Run test suite step uses backslash line continuations, which PowerShell rejects:

ParserError: Missing expression after unary operator '--'.
   3 |    --cov=ebuild --cov-report=xml --cov-report=term-missing \

So the Windows jobs failed before pytest started, on master and on every branch. Marked shell: bash.

With those jobs actually running, they exposed a real portability defect: NinjaBackend never escaped paths in build statements.

ninja: error: build.ninja:20: expected build command name
build C:\...\main.o: cc main.c
      ^ near here

Ninja splits build statements on unescaped spaces and colons, so a Windows drive-letter colon lands where Ninja expects the separator between outputs and the rule name. Every generated build.ninja 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 deliberately left alone, since escaping them would hand the compiler mangled flags. Four regression tests added, including one asserting each build statement contains exactly one unescaped colon.

Also on Windows: tests/ebuild/test_integration_initramfs_security.py died with WinError 2 before reaching the injection behaviour it exists to check — _create_initramfs() drives find(1) and cpio(1) directly, neither of which exists there. Building a Linux initramfs is not a Windows operation, so those three now skip when the tools are absent, matching how test_ninja_backend.py already skips without a host C compiler.

Finally, mypy . aborted immediately with Duplicate module named "tests" (layers/eosuite/ vendors its own tests/ package). Because the step is continue-on-error, this went unnoticed and the type check had been checking zero files. Excluding layers/ makes it check 81 source files; it stays continue-on-error, so the 11 pre-existing findings are visible without gating the build.

Verification

Check	Result
`pytest tests/`	206 passed
`pytest tests/performance/`	1 passed
ninja escaping	the exact CI error reproduces with the unescaped form and is gone with the escaped one

Before this change: SyntaxError at collection, then 11 failures once patched past it, then 4 more on Windows once that leg could run.

Relationship to open PRs

#65 also fixes the dispatch.py else, and #64 also restores _object_path. Neither reconciles the two contradictory dispatch test suites — they pick one exception type, which leaves the other suite failing — and neither addresses the duplicate shared-library design or the Windows breakage. Happy to rebase onto whichever lands first.

codecov-commenter · 2026-08-28T14:00:51Z

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

srpatcha

Verified — this is the fix master needs

Reproduced the breakage on origin/master first, so this is not a stale CI badge:

$ ebuild new hi && cd hi && ebuild build
  File ".../ebuild/build/dispatch.py", line 133
    else:
    ^^^^
SyntaxError: invalid syntax

$ pytest
2 errors during collection

With this branch merged onto current master: 206 passed, and ebuild build reaches the compiler instead of raising a traceback.

I had independently written the same repair (#70) before seeing this. Yours came first and is the one I would take; I will close mine. Checking them against each other, this branch covers three of the four defects I found:


Two consecutive `else:` in `configure()`	fixed
`"ninja"` listed among backends needing no configure — so `configure("ninja")` silently succeeded	fixed
`NinjaBackend._object_path` called twice, defined nowhere	fixed
ninja invoked as `python -m ninja`	still present

The one left

ebuild/build/dispatch.py:244   [sys.executable, "-m", "ninja", "-C", ...]
ebuild/cli/commands.py:690     [sys.executable, "-m", "ninja", "-f", ...]

commands.py:690 is the build path, so this is the more interesting of the two. It works today only because pyproject.toml declares ninja>=1.11, which puts the PyPI wheel in the environment. But it means a machine with a perfectly good ninja on PATH is ignored, and the moment that wheel is absent — a system-package install, a distro build, a container that pip-installed with --no-deps — the build fails with No module named ninja rather than anything a developer can act on.

A three-line helper covers it:

def ninja_command() -> list:
    exe = shutil.which("ninja")
    return [exe] if exe else [sys.executable, "-m", "ninja"]

Happy for that to be a follow-up rather than growing this PR — the SyntaxError is the urgent half and I do not want to hold it. Say the word and I will open it against your branch or after it lands.

Verification

Merged onto origin/master locally; pytest 206 passed; ebuild new + ebuild build walked by hand from a clean project.

srpatcha · 2026-08-29T22:43:35Z

Verified merge order for the ebuild backlog

master cannot collect its own test suite (2 errors in 0.25s), so I applied every open PR in sequence onto origin/master locally and ran pytest after each step.

#	PR	after merging
1	#66	206 passed — from a suite that could not be collected
2	#67	223
3	#68	234
4	#69	234
5	#73	243
6	#76	247
7	#63	247
8	#71	275
9	#72	—
10	#74	—

#71, #72 and #74 stack in that order; the last of them brings the suite to 316 when applied on top of the merged base.

#66 goes first, and supersedes my #70

Three PRs fix the same SyntaxError in ebuild/build/dispatch.py — #66, #70 and #75. I wrote #70 before seeing #66; #66 came first and is the better change, so I have marked mine superseded rather than competing with it.

What I rebased

#64, #70, #71, #72 and #74 all conflicted in dispatch.py and ninja_backend.py — not because they disagree with #66, but because they were branched on top of my own repair of the same files.

None of #71, #72 or #74 actually touches dispatch.py. Rebased onto plain master, they are independent of #66 and merge clean.

That rebase also fixed something worth flagging. The footprint commit had normalised 1,814 CRLF line endings in ebuild/cli/commands.py, so its diff read as 1,903 changed lines:

raw:          1903  1814  ebuild/cli/commands.py
ignoring CR:    89     0  ebuild/cli/commands.py

The real change is 89 lines. commands.py has mixed CRLF/LF endings, and a whole-file rewrite silently normalises them — worth knowing before anyone else edits that file with a script.

Still to resolve

#65 and #75 conflict with #66 in dispatch.py. Both are alternate fixes to the same defect; I have commented on each with what is uniquely theirs and worth keeping after a rebase.

#64 conflicts more deeply and needs its own pass.

Verification

Every row above is pytest run on the merged tree, in that order, on 29 Aug 2026.

The rebase resolution spliced test() into the middle of clean(), leaving clean()'s body orphaned under a second `def clean` and dropping the module-level TestOutcome dataclass with its two parser helpers entirely. The result imported far enough to collect and then failed: ImportError: cannot import name 'TestOutcome' from 'ebuild.build.dispatch' and before that, six of master's own clean() tests failed with NameError: name 'dry_run' is not defined because the shadowing duplicate did not take the keyword. This is the same defect this session has been repairing elsewhere — two versions of one function spliced by a merge that nothing recompiled — and it is worth naming rather than quietly fixing, because I introduced it by resolving a conflict with "keep both sides" without rebuilding afterwards. That is exactly how eos's sync.c ended up referencing fields no version of the file ever defined at once. clean() and test() are now separate again and the dataclass is back. Verified against the branch's base rather than a stale master: base (#66) 205 passed, 1 skipped this branch 240 passed, 1 skipped new failures none ebuild setup / new / configure / build / test / flash / monitor all present ebuild test on a real CMake project -> 1 passed, 0 failed exit 0 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

AshrafAhmed9 · 2026-08-30T22:44:08Z

@srpatcha suggested I flag this here. I hit the same -shared / -dynamiclib bug independently in #86 and went the other way, correcting the rule's flag instead of removing the rule. Their review makes the case that deleting it is the better shape, which I agree with, so I've closed mine in favour of this.

Nothing needed from you, just noting it for the changelog mention they offered.

Kartikey1306 · 2026-08-31T05:29:15Z

@srpatcha — thank you for the merge-order analysis, and for marking #70 superseded rather than competing. Since then master moved and this PR went CONFLICTING, so I have rebuilt it on current master and dropped everything master now does better.

Dropped (master's versions are in, and are the right calls):

the dispatch.py SyntaxError repair, and my UnknownBackendError(ValueError, RuntimeError). Master resolved it as RuntimeError and updated tests/ebuild/test_dispatch.py to match, so both suites now agree — the dual-inheritance trick I used to satisfy two contradictory suites is no longer needed.
NinjaBackend._object_path, restored on master.
my removal of the link_shared rule. Master kept the rule and moved the flag behind _shared_flag(), which is the better shape: -dynamiclib on darwin, -shared elsewhere, with the -L/-l wiring intact. I was wrong to delete it.

What is left is what master still does not have — now 4 files instead of the previous sprawl:

build.ninja is invalid on Windows. Ninja splits build statements on unescaped spaces and colons, so a drive-letter colon lands where the outputs/rule separator belongs: ninja: error: expected build command name. _ninja_path() escapes $, : and in build-statement paths only — variable values are read to end of line and must not be escaped. Four regression tests.
test_shared_library_uses_shared_link_rule fails on master today — on macOS. It asserts the literal "-shared" while _shared_flag() correctly returns -dynamiclib. The implementation is right; the test was not.
The Windows leg of the test matrix has never run — backslash continuations under PowerShell. shell: bash.
macos-13 is a retired image, so those jobs queue until they time out.
mypy . checks nothing — it aborts on Duplicate module named "tests" (layers/eosuite vendors its own), hidden by continue-on-error. Excluding layers/ makes it check 84 files.
No concurrency group, alone among this repo's workflows.

pytest: 292 passed (287 on master + the fixed shared-flag test + 4 new).

Re: the CRLF finding in commands.py — that matches something I hit in eBoot, where a scripted whole-file rewrite turned a 40-line change into 312. Worth a .gitattributes entry rather than relying on everyone remembering.

This branch is based on master, whose ci.yml still runs the test step under PowerShell on windows-2022 (the backslash continuations are a ParserError there, so pytest never starts) and still pins the retired macos-13 image (those jobs queue forever). Both Test (Python 3.x) failures on this PR were that, not the packages change. The fixes are byte-identical to the ones in embeddedos-org#66 -- same hunks, so whichever PR merges first, the other's copy merges cleanly. Taken as one file copy rather than re-derived, to keep them from drifting. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…-org#66 With macos-latest actually running (the previous commit), this test would fail there: it asserts the literal "-shared" while _shared_flag() correctly returns "-dynamiclib" on darwin. Identical hunk to embeddedos-org#66's fix, so the two PRs merge cleanly in either order. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Rebuilt on current master. Master has since fixed the dispatch.py SyntaxError and restored NinjaBackend._object_path, so embeddedos-org#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) <noreply@anthropic.com>

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) <noreply@anthropic.com>

…em 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 embeddedos-org#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) <noreply@anthropic.com>

PackageRecipe.validate() accepts any non-empty version string, but the registry ordered versions with sorted(versions, key=lambda v: [int(x) for x in v.split('.')]) which raises ValueError for anything that is not dotted integers. Real recipes are full of those: a leading v (littlefs and FreeRTOS both publish their tags that way -- the recipe example in the book uses `tag: V10.5.1`), pre-release tags like 3.6.0-rc1, and build metadata like 1.3.1+patch2. The blast radius is wider than the odd package itself. The key was duplicated across get(), list_packages() and list_all_versions(), and * get(name) with no version scans every version of that package, so one such recipe breaks that package entirely; * list_packages() scans every package, and PackageResolver builds its "package not found in registry. Available: ..." message from it -- so a single unusual recipe anywhere in the registry turns an ordinary missing-package error into a ValueError traceback. Replace the three copies with one version_sort_key(). Ordering: a leading v/V is ignored; all-digit components compare numerically so 1.10.0 still sorts above 1.9.0; any other component compares as text and ranks below a numeric one; a -/+ suffix ranks below the same version without one, so 3.6.0-rc1 < 3.6.0. The order is total and never raises, which is the property that matters here -- one unusual recipe must not decide whether lookup works for the packages around it. Deliberately not a full PEP 440 / semver implementation. That would mean a dependency or a lot more code for a comparison the recipe format does not specify; this defines the rules it does need and documents them. Tests: tests/ebuild/test_package_registry.py grows from 1 case to 12, covering v-prefixes, pre-releases, build metadata, date-stamped and non-numeric versions, one odd version among good ones, and a totality check over the key. Nine of the eleven behavioural cases fail against the unfixed registry.py (checked by running them against it); the other two are the pre-existing numeric-order test and "2024.06", which parsed as ints before. Docs: the ordering rules are now in docs/book/book.md section 12.5. Note on CI: master currently fails for reasons unrelated to this change -- ebuild/build/dispatch.py has a duplicated `else:` from a merge, so the module does not parse (PRs embeddedos-org#65/embeddedos-org#66 address it), and ninja_backend.py is missing _object_path. This branch leaves the failure set exactly as it found it: 26 failed / 162 passed before and after, identical lists. Verified: pytest tests/ebuild/test_package_registry.py -> 12 passed. Verified: ruff check --select=E,F,W --ignore=E501 and mypy --ignore-missing-imports on both changed files -> clean.

@srpatcha

Answers findings 1 and 2 from the review on embeddedos-org#103. Finding 2 (Medium) -- `CI Gate` gates ci.yml and nothing else, while the PR body's closing instruction ("Settings -> Branches -> master -> Require status checks -> add `CI Gate`. One name") read as the complete fix for embeddedos-org#87. It is the complete fix for ci.yml. Six other workflows produce pull-request checks here, and the rot-guard was hardcoded to ci.yml so it could not see any of them: a job added to codeql.yml or simulation-test.yml was uncovered and nothing failed. Adds two tests over every workflow with a `pull_request` trigger: - test_every_pull_request_workflow_is_accounted_for -- each must be in REQUIRED_CHECKS (gated, with the display name a maintainer requires) or in NO_GATE with a reason about the workflow itself. "It has no gate yet" is explicitly not a reason, since that is the gap being made visible. - test_gated_workflows_really_have_their_gate -- every name in REQUIRED_CHECKS resolves to a job that exists and displays under that name, so the required set cannot point at a status that never arrives. REQUIRED_CHECKS is the answer to "what does a maintainer actually require", which one name was never going to be. The six NO_GATE entries carry their reasons; simulation-test.yml's says plainly that its gate still fails open on part of its `needs`, which is why requiring it today would assert more than it checks. Finding 1 (High), the run log, is answered on the thread: the log in my 2026-09-01 comment is from eBoot, not from this repository -- those are eBoot's job names and `build-arm` is a key `jq` could never produce here. I posted the same comment on three gate PRs without adjusting it. Correction posted; re-running against this workflow for the real log. Verified: pytest tests/unit/test_ci_gate.py 8 passed pytest tests/ 1 failed, 565 passed, 3 skipped discrimination: dropping an ungated pull_request workflow into .github/workflows/ gives AssertionError: these workflows produce pull-request checks but are neither gated nor excused: ['zz-probe.yml'] so the guard catches the class it was written for. The one failure is PRE-EXISTING and not caused by this change -- confirmed by checking out origin/master and reproducing it there: test_build_dir_resolution.py::test_end_to_end_build_from_outside_produces_the_binary No module named ninja It is the `[sys.executable, "-m", "ninja"]` invocation @srpatcha raised on embeddedos-org#66: the build ignores a perfectly good `ninja` on PATH and fails when the PyPI wheel is absent. CI installs that wheel, so CI does not see it. Their offer to open the `shutil.which("ninja")` follow-up is still open and still worth taking. Refs embeddedos-org#103, embeddedos-org#87, embeddedos-org#66

…and let the path be exercised Answers the review on embeddedos-org#106. Finding 1 (Medium) -- `CIBW_ARCHS_MACOS: "x86_64 arm64"` was inert and the reason given for dropping macos-13 rested on wheels that have never existed. ebuild is pure Python: pyproject.toml declares setuptools.build_meta with no ext-modules, no cmdclass and no setup.py, so cibuildwheel cannot complete on any leg. All four fell through to `python -m build --wheel` and produced the same ebuild-X.Y.Z-py3-none-any.whl, which download-artifact --merge-multiple then collapsed back into one. macos-13's wheels were that same universal file. Dropped the env line and corrected the comment. Finding 2 (Medium) -- the `||` fallback was a fail-open build step, and this PR's `shell: bash` is what makes it reachable on Windows. It does not merely swallow a failure; it substitutes a different artefact and reports success, and `skip-existing: true` on the publish step means the substitution would not even collide with anything. Replaced with an explicit choice: detect whether the project builds a C extension, then run cibuildwheel or `python -m build` accordingly, with `set -euo pipefail` so a real failure fails. The step's name now means what it says. Verified both branches of that detection: against this repo it reports has_ext = False and takes the `python -m build` path; with a setup.py present it reports True. So the day a C extension lands the step switches by itself, and a genuine compile failure is a failure. Finding 4 (Low) -- added `workflow_dispatch`, with `pypi` and `release` gated on `github.event_name == 'push'`. The workflow's only trigger was a version tag, so the first execution of any change to it was a real release -- for a path whose last 15 runs all failed. A maintainer can now run validate + cibuildwheel + cross-compile on demand and watch the wheels land as artifacts without spending a tag, and a manual run cannot publish. Finding 3 (Low) is a PR-body correction and is handled there: the run history is worse than the body said -- the last 15 runs are failure or cancelled, back to v1.0.2 on 2026-04-28, and v1.0.2 failed in five minutes, which is neither the macOS queue timeout nor the shell syntax error. The two causes fixed here are the current ones, not the whole history, and the next tag is a test of these two fixes rather than proof the release path is healthy. Verified: yaml.safe_load of the result parses; jobs are ['validate', 'cibuildwheel', 'cross-compile', 'pypi', 'release'] triggers are ['push', 'workflow_dispatch'] pypi.if and release.if are both github.event_name == 'push' no `||` remains in the Build wheels step; `set -euo pipefail` is present pytest tests/ 1 failed, 565 passed, 3 skipped The one failure is PRE-EXISTING on origin/master and unrelated: the `python -m ninja` invocation from embeddedos-org#66's open follow-up. test_ci_gate.py does not exist on this branch (it lives in embeddedos-org#103), so it was NOT RUN here. NOT RUN: the workflow itself. It is tag- and dispatch-only, so this PR's CI cannot execute it -- which is exactly what finding 4 is about. The workflow_dispatch trigger only becomes usable once this is on master. Refs embeddedos-org#106

…and let the path be exercised Answers the review on embeddedos-org#106. Finding 1 (Medium) -- `CIBW_ARCHS_MACOS: "x86_64 arm64"` was inert and the reason given for dropping macos-13 rested on wheels that have never existed. ebuild is pure Python: pyproject.toml declares setuptools.build_meta with no ext-modules, no cmdclass and no setup.py, so cibuildwheel cannot complete on any leg. All four fell through to `python -m build --wheel` and produced the same ebuild-X.Y.Z-py3-none-any.whl, which download-artifact --merge-multiple then collapsed back into one. macos-13's wheels were that same universal file. Dropped the env line and corrected the comment. Finding 2 (Medium) -- the `||` fallback was a fail-open build step, and this PR's `shell: bash` is what makes it reachable on Windows. It does not merely swallow a failure; it substitutes a different artefact and reports success, and `skip-existing: true` on the publish step means the substitution would not even collide with anything. Replaced with an explicit choice: detect whether the project builds a C extension, then run cibuildwheel or `python -m build` accordingly, with `set -euo pipefail` so a real failure fails. The step's name now means what it says. Verified both branches of that detection: against this repo it reports has_ext = False and takes the `python -m build` path; with a setup.py present it reports True. So the day a C extension lands the step switches by itself, and a genuine compile failure is a failure. Finding 4 (Low) -- added `workflow_dispatch`, with `pypi` and `release` gated on `github.event_name == 'push'`. The workflow's only trigger was a version tag, so the first execution of any change to it was a real release -- for a path whose last 15 runs all failed. A maintainer can now run validate + cibuildwheel + cross-compile on demand and watch the wheels land as artifacts without spending a tag, and a manual run cannot publish. Finding 3 (Low) is a PR-body correction and is handled there: the run history is worse than the body said -- the last 15 runs are failure or cancelled, back to v1.0.2 on 2026-04-28, and v1.0.2 failed in five minutes, which is neither the macOS queue timeout nor the shell syntax error. The two causes fixed here are the current ones, not the whole history, and the next tag is a test of these two fixes rather than proof the release path is healthy. Verified: yaml.safe_load of the result parses; jobs are ['validate', 'cibuildwheel', 'cross-compile', 'pypi', 'release'] triggers are ['push', 'workflow_dispatch'] pypi.if and release.if are both github.event_name == 'push' no `||` remains in the Build wheels step; `set -euo pipefail` is present pytest tests/ 1 failed, 557 passed, 3 skipped The one failure is PRE-EXISTING on origin/master and unrelated: the `python -m ninja` invocation from embeddedos-org#66's open follow-up. test_ci_gate.py does not exist on this branch (it lives in embeddedos-org#103), so it was NOT RUN here. NOT RUN: the workflow itself. It is tag- and dispatch-only, so this PR's CI cannot execute it -- which is exactly what finding 4 is about. The workflow_dispatch trigger only becomes usable once this is on master. Refs embeddedos-org#106

…ather than the parse Answers the second review on embeddedos-org#106. Finding 1 is a defect I introduced in the previous commit. Finding 1 (High) -- `workflow_dispatch:` was inserted between the two tag patterns, so `- "v*.*.*-*"` became its value instead of a second entry under `push.tags`: {"push": {"tags": ["v*.*.*"]}, "workflow_dispatch": ["v*.*.*-*"]} Two consequences. Pre-release tags stopped triggering the workflow at all -- .github/STANDARDS.md names -rc.N as the org's recognised pre-release suffix and the release job handles it explicitly with `--prerelease`, so that code became unreachable. And `workflow_dispatch` takes null or a mapping; a sequence is not valid in GitHub's schema, so the workflow may not load at all -- the exact inverse of this PR's purpose. The verification in my last commit was `yaml.safe_load(...)` plus a print of the job list. That parses this file perfectly well. **Loading is not verifying**: the shape was wrong, not the syntax, and nothing I ran looked at the shape. So tests/unit/test_release_workflow.py asserts the structure: push.tags is exactly the two patterns; workflow_dispatch is null or a mapping; both publishing jobs are gated on `github.event_name == 'push'` so a manual run cannot release. Confirmed it catches what shipped. Reintroducing the exact breakage: safe_load parses fine test_push_tags_are_exactly_the_patterns_we_mean FAILS test_workflow_dispatch_is_a_null_key_not_a_value FAILS Finding 2 (Medium) -- `mv wheelhouse/*.whl dist/ 2>/dev/null || true` in the publish job. If no wheel had been produced -- every cibuildwheel leg red, the artifact upload skipped, the glob matching nothing -- dist/ held the sdist alone and PyPI was published with no wheels, silently, with `skip-existing: true` meaning it would not even collide. Now `set -euo pipefail`, a nullglob array, and an explicit failure when the array is empty. Two further tests pin that: staging cannot swallow its own failure, and no build or publish step in the workflow ends a line with `|| true`. Those tests read shell with comments stripped, because a comment explaining why a `|| true` was removed must not itself read as one. That is not hypothetical -- it failed on my own comment first. Finding 3 (Low) -- the macos-13 comment still described the `|| python -m build` fallback that the previous commit replaced with an explicit builder choice. Corrected. Finding 4 (Low) is CI timing, not a code change: four checks were pending when the review bundle was cut. Verified: pytest tests/unit/test_release_workflow.py 5 passed pytest tests/ 1 failed, 562 passed, 3 skipped The one failure is PRE-EXISTING on origin/master and unrelated: the `python -m ninja` invocation from embeddedos-org#66's open follow-up. parsed triggers are now {"push": {"tags": ["v*.*.*", "v*.*.*-*"]}, "workflow_dispatch": None} NOT RUN: the workflow itself, still. It is tag- and dispatch-only. Refs embeddedos-org#106

… it prevents Answers finding 1 from the second review on embeddedos-org#104. A matrix job with no `name:` was reported as a check-name collision that cannot happen. `_expanded_names()` fell back to `job.get("name", job_id)` and substituted nothing into it, so an unnamed two-leg matrix yielded `["smoke", "smoke"]`. GitHub generates distinct names for exactly this case -- `smoke (ubuntu-22.04)`, `smoke (macos-latest)` -- so the legs never share a name. Asserting a duplicate GitHub prevents sends the reader looking for a bug that is not there, which is worse than not checking. `_expanded_names()` now takes `has_explicit_name` and, when there is none, expands to the default form with the matrix values in it. `test_a_matrix_job_names_every_dimension_it_varies` skips an unnamed job for the same reason: the rule is about a `name:` that varies less than its matrix does, and no `name:` at all already satisfies it. Reproduced the reviewer's probe. An unnamed two-leg `os` matrix wired into the gate: before this commit 2 failed (test_every_check_name_is_unique and test_a_matrix_job_names_every_dimension) after 10 passed And the guard still catches the real thing -- reverting the `name:` template on `test` gives `2 failed` with `assert not ['os']`, unchanged. Finding 2 (Low) is scope rather than a code change: this rule is enforced in ebuild only, while eos and eBoot have the same matrix jobs. Carrying test_ci_gate.py across is worth doing and belongs with the gate PRs there (eos#121, eBoot#90) rather than here, where it would be a third copy landing before the first two have merged. Noted in the PR body. Verified: pytest tests/unit/test_ci_gate.py 10 passed pytest tests/ 1 failed, 567 passed, 3 skipped The one failure is PRE-EXISTING on origin/master and unrelated -- the `python -m ninja` invocation from embeddedos-org#66's open follow-up. Refs embeddedos-org#104

…po-wide Answers the overnight review on embeddedos-org#104. Finding 1 (Medium) -- nightly.yml's full-test-suite was `name: Full Test Suite` over three Python legs: three check runs, one name, the identical defect this PR fixes in ci.yml. The reviewer expanded every matrix in all 15 workflows with this PR's own helper and found it to be the only remaining offender. Renamed now, while nothing refers to the old name -- the PR's own timing argument, taken. Finding 3 (Low) -- uniqueness was checked within one workflow file, but branch protection matches check-run names repo-wide. test_check_names_are_unique_across_every_workflow builds the name set over every pull_request-triggered workflow plus the schedule-only ones (their names land in the same namespace) and asserts uniqueness over the union. Discrimination, executed: reverting the nightly rename makes the new test fail on its own -- these check names are produced more than once across the repository's workflows: ['Full Test Suite'] -- and it passes again on the fixed tree. Finding 1's class now fails loudly wherever it recurs, which is what makes it a class fix rather than a rename. Finding 2 (Low) -- the docstring claimed release.yml and eosim-sanity.yml used the include:-only form and were the live blind spot. Expansion showed neither does; nightly.yml was the real one and the docstring did not mention it. It now records what was actually found: the repo is clean, and the cross-file test is what keeps it that way. Finding 4 (Low) -- the codecov flags change (ebuild-py<ver> -> ebuild-py<ver>-<os>) was in the diff but not the body. Disclosed on the thread with its one real consequence: codecov.yml has no flag_management or carryforward and comment.layout includes flags, so PR comments will list 9 new flags with the previous 3 absent until history rolls over. Verified: pytest tests/unit/test_ci_gate.py 11 passed pytest tests/ 1 failed, 568 passed, 3 skipped (the one failure is the pre-existing python -m ninja environment gap from embeddedos-org#66's open follow-up, reproduced on origin/master) Refs embeddedos-org#104

@srpatcha

* ci: add one job branch protection can require #87 and eos#92 both stay open on the same point: `required_status_checks` is null on master, so nothing builds the merge result before it becomes master. Marking a check required is a settings change, but there is no name here worth pointing it at. `test` is a 3x3 matrix, so it reports as nine generated names that change whenever the matrix does, and a required name that stops appearing blocks every pull request until someone edits the protection rule. `release` is skipped on every pull request, and a skipped required check never reports at all. `ci-gate` is one job, one stable name, that succeeds only if every other job that runs on a pull request succeeded. It carries `if: always()` -- without it the gate is skipped when an earlier job fails, and the pull request would wait for a status that never arrives rather than showing a red X. Any non-success result fails it, `skipped` included: a job that did not run did not verify anything. tests/unit/test_ci_gate.py fails if a job is added to the workflow without being wired into the gate, which is the way this would rot. Verified by mutation -- dropping a job from `needs`, removing `if: always()`, renaming the gate, adding an unwired job, weakening the result check to `== "failure"`, and deleting the gate are each caught. 567 tests pass. * ci: make the gate's coverage checkable, not just its wiring Answers findings 1 and 2 from the review on #103. Finding 2 (Medium) -- `CI Gate` gates ci.yml and nothing else, while the PR body's closing instruction ("Settings -> Branches -> master -> Require status checks -> add `CI Gate`. One name") read as the complete fix for #87. It is the complete fix for ci.yml. Six other workflows produce pull-request checks here, and the rot-guard was hardcoded to ci.yml so it could not see any of them: a job added to codeql.yml or simulation-test.yml was uncovered and nothing failed. Adds two tests over every workflow with a `pull_request` trigger: - test_every_pull_request_workflow_is_accounted_for -- each must be in REQUIRED_CHECKS (gated, with the display name a maintainer requires) or in NO_GATE with a reason about the workflow itself. "It has no gate yet" is explicitly not a reason, since that is the gap being made visible. - test_gated_workflows_really_have_their_gate -- every name in REQUIRED_CHECKS resolves to a job that exists and displays under that name, so the required set cannot point at a status that never arrives. REQUIRED_CHECKS is the answer to "what does a maintainer actually require", which one name was never going to be. The six NO_GATE entries carry their reasons; simulation-test.yml's says plainly that its gate still fails open on part of its `needs`, which is why requiring it today would assert more than it checks. Finding 1 (High), the run log, is answered on the thread: the log in my 2026-09-01 comment is from eBoot, not from this repository -- those are eBoot's job names and `build-arm` is a key `jq` could never produce here. I posted the same comment on three gate PRs without adjusting it. Correction posted; re-running against this workflow for the real log. Verified: pytest tests/unit/test_ci_gate.py 8 passed pytest tests/ 1 failed, 565 passed, 3 skipped discrimination: dropping an ungated pull_request workflow into .github/workflows/ gives AssertionError: these workflows produce pull-request checks but are neither gated nor excused: ['zz-probe.yml'] so the guard catches the class it was written for. The one failure is PRE-EXISTING and not caused by this change -- confirmed by checking out origin/master and reproducing it there: test_build_dir_resolution.py::test_end_to_end_build_from_outside_produces_the_binary No module named ninja It is the `[sys.executable, "-m", "ninja"]` invocation @srpatcha raised on #66: the build ignores a perfectly good `ninja` on PATH and fails when the PyPI wheel is absent. CI installs that wheel, so CI does not see it. Their offer to open the `shutil.which("ninja")` follow-up is still open and still worth taking. Refs #103, #87, #66

… in bash (#106) * ci(release): stop requesting a retired runner, and run the build step in bash No ebuild release has ever published. All six tags -- v1.2.0 through v1.6.0 -- ran for exactly one day and failed, and Publish to PyPI was skipped every time because it needs a job that never finished. Two causes, both already fixed in ci.yml and never carried across. macos-13 is retired. A job requesting it is never assigned a runner and sits queued until GitHub kills it at 24h: cibuildwheel (macos-13) cancelled runner NEVER ASSIGNED Publish to PyPI (OIDC) skipped Create Release skipped Dropped rather than swapped for another image: macos-14 already succeeds, and CIBW_ARCHS_MACOS="x86_64 arm64" makes it produce the Intel wheels macos-13 was there for. The Build wheels step is bash -- a `||` fallback with trailing-backslash continuations -- and the matrix includes windows-latest, where the default shell is PowerShell. That job failed in every run for that reason alone: cibuildwheel (windows-latest) failure step "Build wheels" ci.yml carries the same `shell: bash` fix for the same matrix, with the same comment; release.yml never got it. Verified: no macos-13 remains in any workflow, and release.yml parses. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * ci(release): choose the wheel builder instead of catching a failure, and let the path be exercised Answers the review on #106. Finding 1 (Medium) -- `CIBW_ARCHS_MACOS: "x86_64 arm64"` was inert and the reason given for dropping macos-13 rested on wheels that have never existed. ebuild is pure Python: pyproject.toml declares setuptools.build_meta with no ext-modules, no cmdclass and no setup.py, so cibuildwheel cannot complete on any leg. All four fell through to `python -m build --wheel` and produced the same ebuild-X.Y.Z-py3-none-any.whl, which download-artifact --merge-multiple then collapsed back into one. macos-13's wheels were that same universal file. Dropped the env line and corrected the comment. Finding 2 (Medium) -- the `||` fallback was a fail-open build step, and this PR's `shell: bash` is what makes it reachable on Windows. It does not merely swallow a failure; it substitutes a different artefact and reports success, and `skip-existing: true` on the publish step means the substitution would not even collide with anything. Replaced with an explicit choice: detect whether the project builds a C extension, then run cibuildwheel or `python -m build` accordingly, with `set -euo pipefail` so a real failure fails. The step's name now means what it says. Verified both branches of that detection: against this repo it reports has_ext = False and takes the `python -m build` path; with a setup.py present it reports True. So the day a C extension lands the step switches by itself, and a genuine compile failure is a failure. Finding 4 (Low) -- added `workflow_dispatch`, with `pypi` and `release` gated on `github.event_name == 'push'`. The workflow's only trigger was a version tag, so the first execution of any change to it was a real release -- for a path whose last 15 runs all failed. A maintainer can now run validate + cibuildwheel + cross-compile on demand and watch the wheels land as artifacts without spending a tag, and a manual run cannot publish. Finding 3 (Low) is a PR-body correction and is handled there: the run history is worse than the body said -- the last 15 runs are failure or cancelled, back to v1.0.2 on 2026-04-28, and v1.0.2 failed in five minutes, which is neither the macOS queue timeout nor the shell syntax error. The two causes fixed here are the current ones, not the whole history, and the next tag is a test of these two fixes rather than proof the release path is healthy. Verified: yaml.safe_load of the result parses; jobs are ['validate', 'cibuildwheel', 'cross-compile', 'pypi', 'release'] triggers are ['push', 'workflow_dispatch'] pypi.if and release.if are both github.event_name == 'push' no `||` remains in the Build wheels step; `set -euo pipefail` is present pytest tests/ 1 failed, 557 passed, 3 skipped The one failure is PRE-EXISTING on origin/master and unrelated: the `python -m ninja` invocation from #66's open follow-up. test_ci_gate.py does not exist on this branch (it lives in #103), so it was NOT RUN here. NOT RUN: the workflow itself. It is tag- and dispatch-only, so this PR's CI cannot execute it -- which is exactly what finding 4 is about. The workflow_dispatch trigger only becomes usable once this is on master. Refs #106 * ci(release): repair the trigger block I broke, and assert the shape rather than the parse Answers the second review on #106. Finding 1 is a defect I introduced in the previous commit. Finding 1 (High) -- `workflow_dispatch:` was inserted between the two tag patterns, so `- "v*.*.*-*"` became its value instead of a second entry under `push.tags`: {"push": {"tags": ["v*.*.*"]}, "workflow_dispatch": ["v*.*.*-*"]} Two consequences. Pre-release tags stopped triggering the workflow at all -- .github/STANDARDS.md names -rc.N as the org's recognised pre-release suffix and the release job handles it explicitly with `--prerelease`, so that code became unreachable. And `workflow_dispatch` takes null or a mapping; a sequence is not valid in GitHub's schema, so the workflow may not load at all -- the exact inverse of this PR's purpose. The verification in my last commit was `yaml.safe_load(...)` plus a print of the job list. That parses this file perfectly well. **Loading is not verifying**: the shape was wrong, not the syntax, and nothing I ran looked at the shape. So tests/unit/test_release_workflow.py asserts the structure: push.tags is exactly the two patterns; workflow_dispatch is null or a mapping; both publishing jobs are gated on `github.event_name == 'push'` so a manual run cannot release. Confirmed it catches what shipped. Reintroducing the exact breakage: safe_load parses fine test_push_tags_are_exactly_the_patterns_we_mean FAILS test_workflow_dispatch_is_a_null_key_not_a_value FAILS Finding 2 (Medium) -- `mv wheelhouse/*.whl dist/ 2>/dev/null || true` in the publish job. If no wheel had been produced -- every cibuildwheel leg red, the artifact upload skipped, the glob matching nothing -- dist/ held the sdist alone and PyPI was published with no wheels, silently, with `skip-existing: true` meaning it would not even collide. Now `set -euo pipefail`, a nullglob array, and an explicit failure when the array is empty. Two further tests pin that: staging cannot swallow its own failure, and no build or publish step in the workflow ends a line with `|| true`. Those tests read shell with comments stripped, because a comment explaining why a `|| true` was removed must not itself read as one. That is not hypothetical -- it failed on my own comment first. Finding 3 (Low) -- the macos-13 comment still described the `|| python -m build` fallback that the previous commit replaced with an explicit builder choice. Corrected. Finding 4 (Low) is CI timing, not a code change: four checks were pending when the review bundle was cut. Verified: pytest tests/unit/test_release_workflow.py 5 passed pytest tests/ 1 failed, 562 passed, 3 skipped The one failure is PRE-EXISTING on origin/master and unrelated: the `python -m ninja` invocation from #66's open follow-up. parsed triggers are now {"push": {"tags": ["v*.*.*", "v*.*.*-*"]}, "workflow_dispatch": None} NOT RUN: the workflow itself, still. It is tag- and dispatch-only. Refs #106 --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Srikanth Patchava <srikanth.patchava@outlook.com>

@srpatcha

* ci: add one job branch protection can require #87 and eos#92 both stay open on the same point: `required_status_checks` is null on master, so nothing builds the merge result before it becomes master. Marking a check required is a settings change, but there is no name here worth pointing it at. `test` is a 3x3 matrix, so it reports as nine generated names that change whenever the matrix does, and a required name that stops appearing blocks every pull request until someone edits the protection rule. `release` is skipped on every pull request, and a skipped required check never reports at all. `ci-gate` is one job, one stable name, that succeeds only if every other job that runs on a pull request succeeded. It carries `if: always()` -- without it the gate is skipped when an earlier job fails, and the pull request would wait for a status that never arrives rather than showing a red X. Any non-success result fails it, `skipped` included: a job that did not run did not verify anything. tests/unit/test_ci_gate.py fails if a job is added to the workflow without being wired into the gate, which is the way this would rot. Verified by mutation -- dropping a job from `needs`, removing `if: always()`, renaming the gate, adding an unwired job, weakening the result check to `== "failure"`, and deleting the gate are each caught. 567 tests pass. * ci: make the gate's coverage checkable, not just its wiring Answers findings 1 and 2 from the review on #103. Finding 2 (Medium) -- `CI Gate` gates ci.yml and nothing else, while the PR body's closing instruction ("Settings -> Branches -> master -> Require status checks -> add `CI Gate`. One name") read as the complete fix for #87. It is the complete fix for ci.yml. Six other workflows produce pull-request checks here, and the rot-guard was hardcoded to ci.yml so it could not see any of them: a job added to codeql.yml or simulation-test.yml was uncovered and nothing failed. Adds two tests over every workflow with a `pull_request` trigger: - test_every_pull_request_workflow_is_accounted_for -- each must be in REQUIRED_CHECKS (gated, with the display name a maintainer requires) or in NO_GATE with a reason about the workflow itself. "It has no gate yet" is explicitly not a reason, since that is the gap being made visible. - test_gated_workflows_really_have_their_gate -- every name in REQUIRED_CHECKS resolves to a job that exists and displays under that name, so the required set cannot point at a status that never arrives. REQUIRED_CHECKS is the answer to "what does a maintainer actually require", which one name was never going to be. The six NO_GATE entries carry their reasons; simulation-test.yml's says plainly that its gate still fails open on part of its `needs`, which is why requiring it today would assert more than it checks. Finding 1 (High), the run log, is answered on the thread: the log in my 2026-09-01 comment is from eBoot, not from this repository -- those are eBoot's job names and `build-arm` is a key `jq` could never produce here. I posted the same comment on three gate PRs without adjusting it. Correction posted; re-running against this workflow for the real log. Verified: pytest tests/unit/test_ci_gate.py 8 passed pytest tests/ 1 failed, 565 passed, 3 skipped discrimination: dropping an ungated pull_request workflow into .github/workflows/ gives AssertionError: these workflows produce pull-request checks but are neither gated nor excused: ['zz-probe.yml'] so the guard catches the class it was written for. The one failure is PRE-EXISTING and not caused by this change -- confirmed by checking out origin/master and reproducing it there: test_build_dir_resolution.py::test_end_to_end_build_from_outside_produces_the_binary No module named ninja It is the `[sys.executable, "-m", "ninja"]` invocation @srpatcha raised on #66: the build ignores a perfectly good `ninja` on PATH and fails when the PyPI wheel is absent. CI installs that wheel, so CI does not see it. Their offer to open the `shutil.which("ninja")` follow-up is still open and still worth taking. Refs #103, #87, #66 * ci: give each matrix leg its own check name The test job's `name:` interpolated the Python version but not the OS, so all three legs of each version reported under one name. Measured on #103's head: $ gh api repos/embeddedos-org/ebuild/commits/<sha>/check-runs \ --jq '.check_runs[].name' | sort | uniq -c 3 Test (Python 3.12) 3 Test (Python 3.11) 3 Test (Python 3.10) Nine check runs, three names. Two consequences, one for people and one for branch protection: A Windows-only failure is indistinguishable from the ubuntu and macOS legs in the checks list -- all three rows read "Test (Python 3.10)" and you have to open them to find which failed. That is not hypothetical here: the Windows leg did not run at all until `shell: bash` was added, and once it did it surfaced failures the other two legs never saw. And a required status check names a check. With three check runs behind one name, a rule naming it cannot say which leg it means. Adding `${{ matrix.os }}` makes all nine names distinct. Doing it now, before #103's `CI Gate` gives anyone a reason to start pinning check names: `required_status_checks` is null today, so nothing anywhere refers to these names -- `grep` finds the template itself and nothing else. Renaming nine checks is cheap exactly while no rule depends on them, and expensive afterwards. Two tests added to tests/unit/test_ci_gate.py: check names must be unique across the whole workflow, and a matrix job must name every dimension it varies. Verified by mutation -- reverting the template to its previous form, adding a third matrix dimension without naming it, and removing `os` from the name while leaving it in the matrix are each caught (2 failed of 8 in every case). 569 tests pass. * ci: model include/exclude in the matrix guard, and name both dimensions in the flag too Answers the review on #104. Finding 1 (Medium) -- _expanded_names() built the cartesian product of list dimensions and skipped `include`/`exclude` entirely. The comment justifying that was right as far as it went, but the consequence was that a matrix expressed *entirely* through `include:` has no list dimensions, so the function returned one name for however many legs the job really has. The reviewer's probe -- a `lint` job with a two-entry `include:` and a static `name: Lint` -- passed the whole suite, which is the exact defect this PR exists to catch, in the form the guard could not see. Not hypothetical: release.yml:67 and eosim-sanity.yml:83 are both `matrix: include:` with no list dimensions today, so the blind spot goes live as soon as the guard is pointed at them -- which #103's REQUIRED_CHECKS table now makes a natural next step. `include` entries that refine an existing combination update it; entries that introduce new values add a leg. `exclude` removes combinations rather than being ignored, which also closes the mirror problem the finding notes -- a combination that never runs could otherwise be reported as a duplicate. Finding 2 (Low) -- the Codecov flag four lines below the name template had the identical defect: `ebuild-py${{ matrix.python-version }}` interpolates the Python version and not the OS, so after this PR nine legs reported under nine check names and three coverage flags. Fixed rather than commented, since the PR's own stated rule ("a matrix job names every dimension it varies") covers it and merging three OS legs into one flag was the leftover of the bug, not a decision. Finding 3 (Low) is a PR-body correction and is handled there: the "nothing refers to these names" grep covered three extensions in one repository and cannot support the word *nothing*. `required_status_checks` being null is the load-bearing half, and that half is solid. Verified: pytest tests/unit/test_ci_gate.py 10 passed pytest tests/ 1 failed, 567 passed, 3 skipped discrimination: with an include:-only `lint` job wired into the gate, AssertionError: these check names are produced more than once: ['Lint'] from test_every_check_name_is_unique. Before this commit the same probe passed. The one failure is PRE-EXISTING on origin/master and unrelated -- the `python -m ninja` invocation from #66's open follow-up; see the note on #103's commit. Refs #104 * test(ci): model GitHub's auto-generated matrix names, not a collision it prevents Answers finding 1 from the second review on #104. A matrix job with no `name:` was reported as a check-name collision that cannot happen. `_expanded_names()` fell back to `job.get("name", job_id)` and substituted nothing into it, so an unnamed two-leg matrix yielded `["smoke", "smoke"]`. GitHub generates distinct names for exactly this case -- `smoke (ubuntu-22.04)`, `smoke (macos-latest)` -- so the legs never share a name. Asserting a duplicate GitHub prevents sends the reader looking for a bug that is not there, which is worse than not checking. `_expanded_names()` now takes `has_explicit_name` and, when there is none, expands to the default form with the matrix values in it. `test_a_matrix_job_names_every_dimension_it_varies` skips an unnamed job for the same reason: the rule is about a `name:` that varies less than its matrix does, and no `name:` at all already satisfies it. Reproduced the reviewer's probe. An unnamed two-leg `os` matrix wired into the gate: before this commit 2 failed (test_every_check_name_is_unique and test_a_matrix_job_names_every_dimension) after 10 passed And the guard still catches the real thing -- reverting the `name:` template on `test` gives `2 failed` with `assert not ['os']`, unchanged. Finding 2 (Low) is scope rather than a code change: this rule is enforced in ebuild only, while eos and eBoot have the same matrix jobs. Carrying test_ci_gate.py across is worth doing and belongs with the gate PRs there (eos#121, eBoot#90) rather than here, where it would be a third copy landing before the first two have merged. Noted in the PR body. Verified: pytest tests/unit/test_ci_gate.py 10 passed pytest tests/ 1 failed, 567 passed, 3 skipped The one failure is PRE-EXISTING on origin/master and unrelated -- the `python -m ninja` invocation from #66's open follow-up. Refs #104 * ci: rename the last static-named matrix, and check name uniqueness repo-wide Answers the overnight review on #104. Finding 1 (Medium) -- nightly.yml's full-test-suite was `name: Full Test Suite` over three Python legs: three check runs, one name, the identical defect this PR fixes in ci.yml. The reviewer expanded every matrix in all 15 workflows with this PR's own helper and found it to be the only remaining offender. Renamed now, while nothing refers to the old name -- the PR's own timing argument, taken. Finding 3 (Low) -- uniqueness was checked within one workflow file, but branch protection matches check-run names repo-wide. test_check_names_are_unique_across_every_workflow builds the name set over every pull_request-triggered workflow plus the schedule-only ones (their names land in the same namespace) and asserts uniqueness over the union. Discrimination, executed: reverting the nightly rename makes the new test fail on its own -- these check names are produced more than once across the repository's workflows: ['Full Test Suite'] -- and it passes again on the fixed tree. Finding 1's class now fails loudly wherever it recurs, which is what makes it a class fix rather than a rename. Finding 2 (Low) -- the docstring claimed release.yml and eosim-sanity.yml used the include:-only form and were the live blind spot. Expansion showed neither does; nightly.yml was the real one and the docstring did not mention it. It now records what was actually found: the repo is clean, and the cross-file test is what keeps it that way. Finding 4 (Low) -- the codecov flags change (ebuild-py<ver> -> ebuild-py<ver>-<os>) was in the diff but not the body. Disclosed on the thread with its one real consequence: codecov.yml has no flag_management or carryforward and comment.layout includes flags, so PR comments will list 9 new flags with the previous 3 absent until history rolls over. Verified: pytest tests/unit/test_ci_gate.py 11 passed pytest tests/ 1 failed, 568 passed, 3 skipped (the one failure is the pre-existing python -m ninja environment gap from #66's open follow-up, reproduced on origin/master) Refs #104 --------- Co-authored-by: Srikanth Patchava <srikanth.patchava@outlook.com>

…and let the path be exercised Answers the review on embeddedos-org#106. Finding 1 (Medium) -- `CIBW_ARCHS_MACOS: "x86_64 arm64"` was inert and the reason given for dropping macos-13 rested on wheels that have never existed. ebuild is pure Python: pyproject.toml declares setuptools.build_meta with no ext-modules, no cmdclass and no setup.py, so cibuildwheel cannot complete on any leg. All four fell through to `python -m build --wheel` and produced the same ebuild-X.Y.Z-py3-none-any.whl, which download-artifact --merge-multiple then collapsed back into one. macos-13's wheels were that same universal file. Dropped the env line and corrected the comment. Finding 2 (Medium) -- the `||` fallback was a fail-open build step, and this PR's `shell: bash` is what makes it reachable on Windows. It does not merely swallow a failure; it substitutes a different artefact and reports success, and `skip-existing: true` on the publish step means the substitution would not even collide with anything. Replaced with an explicit choice: detect whether the project builds a C extension, then run cibuildwheel or `python -m build` accordingly, with `set -euo pipefail` so a real failure fails. The step's name now means what it says. Verified both branches of that detection: against this repo it reports has_ext = False and takes the `python -m build` path; with a setup.py present it reports True. So the day a C extension lands the step switches by itself, and a genuine compile failure is a failure. Finding 4 (Low) -- added `workflow_dispatch`, with `pypi` and `release` gated on `github.event_name == 'push'`. The workflow's only trigger was a version tag, so the first execution of any change to it was a real release -- for a path whose last 15 runs all failed. A maintainer can now run validate + cibuildwheel + cross-compile on demand and watch the wheels land as artifacts without spending a tag, and a manual run cannot publish. Finding 3 (Low) is a PR-body correction and is handled there: the run history is worse than the body said -- the last 15 runs are failure or cancelled, back to v1.0.2 on 2026-04-28, and v1.0.2 failed in five minutes, which is neither the macOS queue timeout nor the shell syntax error. The two causes fixed here are the current ones, not the whole history, and the next tag is a test of these two fixes rather than proof the release path is healthy. Verified: yaml.safe_load of the result parses; jobs are ['validate', 'cibuildwheel', 'cross-compile', 'pypi', 'release'] triggers are ['push', 'workflow_dispatch'] pypi.if and release.if are both github.event_name == 'push' no `||` remains in the Build wheels step; `set -euo pipefail` is present pytest tests/ 1 failed, 557 passed, 3 skipped The one failure is PRE-EXISTING on origin/master and unrelated: the `python -m ninja` invocation from embeddedos-org#66's open follow-up. test_ci_gate.py does not exist on this branch (it lives in embeddedos-org#103), so it was NOT RUN here. NOT RUN: the workflow itself. It is tag- and dispatch-only, so this PR's CI cannot execute it -- which is exactly what finding 4 is about. The workflow_dispatch trigger only becomes usable once this is on master. Refs embeddedos-org#106

…ather than the parse Answers the second review on embeddedos-org#106. Finding 1 is a defect I introduced in the previous commit. Finding 1 (High) -- `workflow_dispatch:` was inserted between the two tag patterns, so `- "v*.*.*-*"` became its value instead of a second entry under `push.tags`: {"push": {"tags": ["v*.*.*"]}, "workflow_dispatch": ["v*.*.*-*"]} Two consequences. Pre-release tags stopped triggering the workflow at all -- .github/STANDARDS.md names -rc.N as the org's recognised pre-release suffix and the release job handles it explicitly with `--prerelease`, so that code became unreachable. And `workflow_dispatch` takes null or a mapping; a sequence is not valid in GitHub's schema, so the workflow may not load at all -- the exact inverse of this PR's purpose. The verification in my last commit was `yaml.safe_load(...)` plus a print of the job list. That parses this file perfectly well. **Loading is not verifying**: the shape was wrong, not the syntax, and nothing I ran looked at the shape. So tests/unit/test_release_workflow.py asserts the structure: push.tags is exactly the two patterns; workflow_dispatch is null or a mapping; both publishing jobs are gated on `github.event_name == 'push'` so a manual run cannot release. Confirmed it catches what shipped. Reintroducing the exact breakage: safe_load parses fine test_push_tags_are_exactly_the_patterns_we_mean FAILS test_workflow_dispatch_is_a_null_key_not_a_value FAILS Finding 2 (Medium) -- `mv wheelhouse/*.whl dist/ 2>/dev/null || true` in the publish job. If no wheel had been produced -- every cibuildwheel leg red, the artifact upload skipped, the glob matching nothing -- dist/ held the sdist alone and PyPI was published with no wheels, silently, with `skip-existing: true` meaning it would not even collide. Now `set -euo pipefail`, a nullglob array, and an explicit failure when the array is empty. Two further tests pin that: staging cannot swallow its own failure, and no build or publish step in the workflow ends a line with `|| true`. Those tests read shell with comments stripped, because a comment explaining why a `|| true` was removed must not itself read as one. That is not hypothetical -- it failed on my own comment first. Finding 3 (Low) -- the macos-13 comment still described the `|| python -m build` fallback that the previous commit replaced with an explicit builder choice. Corrected. Finding 4 (Low) is CI timing, not a code change: four checks were pending when the review bundle was cut. Verified: pytest tests/unit/test_release_workflow.py 5 passed pytest tests/ 1 failed, 562 passed, 3 skipped The one failure is PRE-EXISTING on origin/master and unrelated: the `python -m ninja` invocation from embeddedos-org#66's open follow-up. parsed triggers are now {"push": {"tags": ["v*.*.*", "v*.*.*-*"]}, "workflow_dispatch": None} NOT RUN: the workflow itself, still. It is tag- and dispatch-only. Refs embeddedos-org#106

Rebuilt on current master. Master has since fixed the dispatch.py SyntaxError and restored NinjaBackend._object_path, so embeddedos-org#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).

Kartikey1306 requested review from hshanmug12, maheshmunnangi and srpatcha as code owners August 28, 2026 13:59

Kartikey1306 mentioned this pull request Aug 28, 2026

test(packages): make the fetcher's fixture archive reproducible #67

Merged

Harshit-Vashisth mentioned this pull request Aug 28, 2026

fix(packages): stop a non-numeric version taking down registry lookup #68

Merged

srpatcha previously approved these changes Aug 29, 2026

View reviewed changes

This was referenced Aug 29, 2026

Fix/fix graph node names that can become unterminated #69

Merged

fix: prevent false success for unsupported system backend #65

Merged

config: reject a non-list packages section instead of dropping it #73

Merged

Kartikey1306 dismissed srpatcha’s stale review via 31ffd98 August 31, 2026 04:15

Kartikey1306 force-pushed the fix/restore-master-build-and-tests branch from 14928ab to 31ffd98 Compare August 31, 2026 04:15

This was referenced Sep 1, 2026

fix(cad): parse whole s-expression blocks, and ship a board to run them on #94

Merged

ci: pin EoSim to a tag that exists #95

Merged

Kartikey1306 and others added 2 commits September 1, 2026 15:07

Kartikey1306 force-pushed the fix/restore-master-build-and-tests branch from 3916f19 to 4a5ee23 Compare September 1, 2026 09:38

srpatcha merged commit 4a5ee23 into embeddedos-org:master Sep 1, 2026
28 checks passed

srpatcha mentioned this pull request Sep 14, 2026

ci: grade only open pull requests against the linked-issue policy #144

Merged

Navigation Menu

Repository navigation

fix: restore the package import and reconcile the merged backend behaviour - #66

fix: restore the package import and reconcile the merged backend behaviour#66
srpatcha merged 2 commits into
embeddedos-org:masterfrom
Kartikey1306:fix/restore-master-build-and-tests

Kartikey1306 commented Aug 28, 2026 •

edited

Loading

Uh oh!

codecov-commenter commented Aug 28, 2026 •

edited

Loading

Uh oh!

srpatcha left a comment

Uh oh!

srpatcha commented Aug 29, 2026

Uh oh!

AshrafAhmed9 commented Aug 30, 2026

Uh oh!

Kartikey1306 commented Aug 31, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

Kartikey1306 commented Aug 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

The architectural problem

What is broken

dispatch.py does not parse

The default build backend crashes

Two incompatible shared-library designs both merged

The Windows matrix has never run — and the backend has never worked there

Verification

Relationship to open PRs

Uh oh!

codecov-commenter commented Aug 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

srpatcha left a comment

Choose a reason for hiding this comment

Verified — this is the fix master needs

The one left

Verification

Uh oh!

srpatcha commented Aug 29, 2026

Verified merge order for the ebuild backlog

#66 goes first, and supersedes my #70

What I rebased

Still to resolve

Verification

Uh oh!

AshrafAhmed9 commented Aug 30, 2026

Uh oh!

Kartikey1306 commented Aug 31, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Kartikey1306 commented Aug 28, 2026 •

edited

Loading

`dispatch.py` does not parse

codecov-commenter commented Aug 28, 2026 •

edited

Loading