Gave install_riscv.sh the network hardening install.sh already had, and shared it between them - #721
Merged
Merged
Conversation
…nd shared it between them install.sh grew a retry loop, per-command timeouts and a deliberately non-gating apt-get update after this runner pool cost several whole runs: a mirror going silent for two hours, and a Hash Sum mismatch from a third-party repository the project does not even use turning builds red. Those lessons were local to that one file. install_riscv.sh had none of them, and #717 puts it on every pull request's critical path. Under set -e its bare apt-get update was a single point of failure for the whole suite -- the precise case install.sh downgrades to a warning on purpose -- and its two wget calls, each fetching about 500 MB, had no retry and no timeout. Rather than copy the helpers and let them drift again, they move to tx_ci_common.sh and both scripts source it, following the arrangement scripts/tx_windows_common.ps1 already uses on the Windows side. install.sh keeps its behaviour exactly: same APT_OPTIONS, same 120-second TIMEOUT, same three-attempt retry, and the comments explaining each of them travel with the code they explain. Two things are new: - TIMEOUT_LONG, 180 seconds, for a single large download. Sized against the 39 seconds each tarball took on 10 Sep 2026 and deliberately not larger: the install step is capped at ten minutes, and a per-attempt timeout able to swallow that cap would leave the retry loop no turn to take, which is the failure mode the apt comment already records. - fetch(), which verifies a SHA-256 before anything is unpacked. Both digests were taken from the releases API and then checked against the bytes the CDN actually serves. This is not an independent trust root -- expected value and file come from the same host -- but it pins the bytes, so a deleted and re-pushed tag or a replaced asset stops the build instead of being picked up silently. Also verifies qemu-system-riscv32 alongside riscv64. run.sh selects one per architecture, so both are worth failing on here rather than at the first test. Verified locally: retry returns 0 on success and 1 after three attempts; fetch accepts a correct digest and, on a wrong one, fails and removes the partial file; the source line resolves from the repository root, from an absolute path and through a symlink; and both recorded digests match the bytes served for the pinned tag. Assisted-by: Claude Code (Opus 5) <noreply@anthropic.com>
Test Results FreeRTOS3 tests 3 ✅ 0s ⏱️ Results for commit 2fdb171. ♻️ This comment has been updated with latest results. |
Minimum allowed line rate is |
Test Results SMP570 tests 570 ✅ 4m 48s ⏱️ Results for commit 2fdb171. ♻️ This comment has been updated with latest results. |
Minimum allowed line rate is |
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
1 participant
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.
Follow-up to #717, which enables the RISC-V regression suite in CI and so puts
scripts/install_riscv.shon every pull request's critical path. That script did not have the hardeningscripts/install.shgrew for exactly these conditions.The gap
install.shcarries a retry loop, per-command timeouts and anapt-get updatethat deliberately warns rather than gates. Its comments record why: a mirror going silent for over two hours, and a Hash Sum mismatch from a third-party repository this project does not use turning builds red. All of that was local to that one file.install_riscv.shhad none of it. Underset -e, its bareapt-get updatewas a single point of failure for the whole suite — the precise caseinstall.shdowngrades to a warning on purpose — and its twowgetcalls, about 500 MB each, had no retry and no timeout.The change
Rather than copy the helpers and let the two drift again, they move to
scripts/tx_ci_common.shand both scripts source it. This follows the arrangementscripts/tx_windows_common.ps1already uses on the Windows side.install.shkeeps its behaviour exactly — sameAPT_OPTIONS, same 120-secondTIMEOUT, same three-attemptretry— and the comments explaining each of them travel with the code they explain.Two things are new:
TIMEOUT_LONG, 180 seconds, for a single large download. Sized against the 39 seconds each tarball took on 10 Sep 2026, and deliberately not larger: the install step is capped at ten minutes, and a per-attempt timeout able to swallow that cap would leave the retry loop no turn to take — which is the failure mode the existing apt comment already records. Three attempts still do not fit inside ten minutes, so the step timeout stays the outer backstop for a server that is genuinely down; what the retries buy is the transient case, which is the common one.fetch(), which verifies a SHA-256 before anything is unpacked. Worth being straight about what this is and is not: expected value and file come from the same host, so it is not an independent trust root and does not prove the release was not tampered with at source. What it buys is that the bytes are pinned — a deleted and re-pushed tag, or a replaced asset, stops the build instead of being picked up silently.Also verifies
qemu-system-riscv32alongsideriscv64.run.shselects one per architecture, so both are worth failing on in the install step rather than at the first test.Verification
CI here will not exercise
install_riscv.shuntil #717 merges, so this was checked locally instead:retryreturns 0 on success and 1 after three attempts, with the 10/20/30-second backoff.fetchaccepts a correct digest; on a wrong one it fails and removes the partial file, so a bad download cannot be unpacked.sourceline resolves from the repository root (how CI invokes it), from an absolute path, and through a symlink.riscv32-elf73c9a5ad…andriscv64-elf4a66a329…, both matching.Not included
install.shstarts withrm -rf /opt/hostedtoolcacheto free space;install_riscv.shextracts roughly 2.5 GB into/optwithout it. That has not caused a failure — the run on #717 was comfortable — so I have left it out rather than change behaviour on a guess. Worth revisiting if disk ever bites.