fix(ci): check out the submodule holding the design tri rtl check reads (Closes #2307) - #2308
Merged
Merged
Conversation
Third sequential cause in the cli-tri `build` job. #2303 fixed the yosys ordering; #2305 assigned dry_run_sweep_ok. With both landed the tests report 156 passed / 0 failed and the job reached a step that had never executed once in the workflow's history: ./target/debug/tri rtl check chips/phi --json Error: No such file or directory (os error 2) verdict lines: 0 chips/phi is a gitlink (mode 160000) to gHashTag/tt-trinity-phi. The job's checkout was a bare `uses: actions/checkout@v4` with no `with:` block, so submodules took its default of false and the directory was empty on the runner. The binary was never broken; it had nothing to read. The error is unattributed because top_from_info reads info.yaml with a plain `?` while its sibling declared_sources wraps the identical failure in .with_context() naming the path. submodules: true under the default GITHUB_TOKEN only works for a public submodule -- the token is scoped to this repo, and a private one would have converted a failing step into a failing checkout. All three chips/* submodules are public (phi 941 KB, euler 3,954 KB, gamma 5,365 KB, ~10.2 MB total), and none has a nested .gitmodules, so `true` is the minimal setting and `recursive` would buy nothing. No other workflow in the repo checks out submodules, so the setting comes from the constraint rather than from copied precedent. Not compiled: the authoring machine had ~230 MB free, too little to check out the repo or run cargo. Verified against the pinned gitlink f5456685c3593665153fe2765c85bb1f46ec14c2 via the GitHub contents API -- the commit is reachable in tt-trinity-phi, info.yaml is present and sets top_module "tt_um_trinity_nano", and all 49 declared source_files resolve against the 51 .v files in src/ at that exact commit. So check 1 "sources resolve" passes rather than merely emitting a FAIL line, and the remaining four come from one yosys pass already on PATH from #2303. The N<5 assertion is byte-identical and every other step is unchanged; build was not added to the required contexts. Closes #2307
gHashTag
enabled auto-merge (squash)
August 20, 2026 09:37
Contributor
|
📓 NotebookLM Notebook linked to this PR
This notebook contains session context, decisions, and artifacts for this work. |
Contributor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2307.
Third sequential cause in the
cli-tribuildjob. #2303 fixed the yosys ordering, #2305 assigneddry_run_sweep_ok. With both landed the tests report156 passed; 0 failedand the job reached a step that had never executed once in the workflow's history:chips/phiis a gitlink (mode160000) togHashTag/tt-trinity-phi. The checkout was a bareuses: actions/checkout@v4with nowith:block, sosubmodulestook its default offalseand the directory was empty on the runner. The binary was never broken; it had nothing to read.The change
Plus the comment explaining it. Nothing else in the workflow is touched —
+14/-0.Why this was mechanical, and not a secrets decision
submodules: trueunder the defaultGITHUB_TOKENonly clones a public submodule; the token is scoped to this repository alone. A private submodule would have converted a failing step into a failing checkout, and would have needed a PAT or deploy key — a secrets call, not a mechanical one. All three are public:chips/phigHashTag/tt-trinity-phifalsechips/eulergHashTag/tt-trinity-eulerfalsechips/gammagHashTag/tt-trinity-gammafalsetruepulls all three, ~10.2 MB, which does not move this job's cost. None of the three has a nested.gitmodules, sorecursivewould buy nothing andtrueis the minimal setting that works.No other workflow in this repo checks out submodules, so there was no working configuration to copy — the setting is derived from the constraint rather than from precedent. (
fpga-build.ymlclones prjxray with--recursive, but that is a third-party clone in arun:block, notactions/checkout.)Verification
Not compiled — the authoring machine had ~230 MB free, too little to check out the repo or run cargo. Verified through the GitHub contents API against the pinned gitlink
f5456685c3593665153fe2765c85bb1f46ec14c2, not againstmain, since a checkout resolves the pin and an orphaned pin would fail the clone outright:tt-trinity-phi(dated 2026-05-18);info.yamlis present (3,830 bytes) and setstop_module: "tt_um_trinity_nano", sotop_from_inforesolves;source_files,src/holds 51.vfiles at that commit, and all 49 resolve — parsed with a transcription ofdeclared_sources()rather than by eye.So check 1
sources resolvepasses rather than merely emitting a FAIL line, and the other four verdicts come from a single yosys pass already onPATHfrom #2303. This should produce five passing verdicts, not just five lines.Because
.github/workflows/cli-tri.ymlis inside this workflow's own triggerpaths,buildruns on this PR — so the fix is observable here rather than only after merge.Not in scope
The
N < 5assertion is byte-identical, verified by parsing both YAMLs and comparing the final step'srunblock. The step asserted five verdicts and got zero; that is the guard working.buildwas not added to the required contexts.One thing this does not fix, deliberately:
top_from_infoincli/tri/src/rtl.rsreadsinfo.yamlwith a plain?while its siblingdeclared_sourceswraps the identical failure in.with_context()naming the path. That asymmetry is why a missing submodule surfaced as an unattributedos error 2and cost a diagnosis. It is a separate change from a CI checkout fix.