Repository navigation
firmware: make build-schedules.sh portable to Linux - #175
Merged
Merged
Conversation
AI-assisted: Claude Opus 5
Merged
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.
build-schedules.shcannot work on Linux today, and it fails in the worst wayavailable: silently, with plausible-looking output.
Line 78 used
sed -i '' -E. BSD sed reads the empty string as the backupsuffix; GNU sed reads it as the sed script and then treats the real expression
as a filename. It prints "can't read ..." to stderr, edits nothing, and exits
0. Under
set -euo pipefailthat is not caught, so the run continues, buildsthe image with whatever
schedule_appalready pointed at, and files it underthe requested name. Asking for
tinygets you a file called03app_gateway_net-tiny.hexcontaining thehugeschedule.That is worse than a crash because nothing downstream can detect it. Every
image links all four schedules in and only the compile-time pointer differs,
so once the hex exists there is no way to inspect which schedule it holds. The
error surfaces as a gateway quietly running the wrong slotframe.
The in-place edit now goes through a scratch file and is written back with
cat, which both seds accept and which preserves the inode and permissions ofmain.c- that matters because the script's EXIT trap restores the filebyte-for-byte so local uncommitted edits survive a build.
shasumis likewisereplaced by a helper that prefers
sha256sumwhen it exists, since that iscoreutils and
shasumis the perl tool macOS ships; neither is presenteverywhere.
Verified on Ubuntu 22.04 with GNU sed 4.8: the helper flips
&schedule_hugeto
&schedule_tinycorrectly, the checksum helper takes thesha256sumbranch, the script reaches the compiler invocation, and the EXIT trap leaves
main.cunmodified afterwards. Not verified on macOS, where the previousbehaviour was already correct and the scratch-file form is the portable
spelling of the same edit.
One related rough edge left alone deliberately:
SEGGER_DIRstill defaults to/opt/segger, which is a common Linux location but not a universal one, andis not where the SES packages install by default. That is a configurable
default with an env override rather than a correctness bug, and changing it
would affect macOS users who rely on the current value, so it seemed worth
raising separately rather than bundling here.