fix(runtime): write the module index with unix line endings - #12
Merged
Conversation
An autoexec.bin built on Windows stopped with one "unsafe runtime module directory" per selected module, then "STOP: one or more runtime modules violate their contract". Nothing was applied, on every build, for every user on that platform. The index naming the modules to load is written from Python, which translates each newline to the platform's own unless it is told not to. On Windows that put a carriage return at the end of every line. The orchestrator reads the file with `IFS= read -r`, which strips the LF and keeps the CR, so the directory name it validates ends in a character the name check rejects - correctly, since that directory does not exist. Passing newline="" writes the LFs untranslated. Two tests normalised CRLF out of the built image before searching it, which is what a build carrying this fault produces, so they passed on Windows instead of failing. They now assert the bytes as the player reads them. The regression test also asserts the newline argument in the source: Linux and macOS never perform the translation, so the built image cannot show the fault there, and the unit tests only ever run on ubuntu in CI. Affects 0.5.0 and 0.5.1. Refs #11.
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.
An
autoexec.binbuilt on Windows stops with oneFAILED: unsafe runtime module directoryper selected module, thenSTOP: one or more runtime modules violate their contract. Nothing is applied, on every build, for every user on that platform.Refs #11.
Cause
The index naming the modules to load is written from Python, which translates each newline to the platform's own unless told otherwise. On Windows that appends a carriage return to every line.
mod/autoexec.shreads the file withIFS= read -r, which strips the LF and keeps the CR, so the directory name reaching the check ends in a character it rejects — correctly, since that directory does not exist. Passingnewline=""writes the LFs untranslated.The bracketed names in the reported log break across two lines for the same reason: what sits before the
]is a CR.Why the tests missed it
Two assertions normalised CRLF out of the built image before searching it — which is exactly what a build carrying this fault produces — so they passed on Windows rather than failing. They now assert the bytes as the player reads them.
The new regression test also asserts the
newlineargument in the source. Linux and macOS never perform the translation, so the built image cannot exhibit the fault there, and the unit tests only ever run onubuntu-24.04in CI; a byte-level assertion alone would guard nothing.Scope
Affects 0.5.0 and 0.5.1. The module index arrived in
031e6ca, so 0.4.0 predates it and is unaffected, which matches the report. Builds made on Linux and macOS were never affected.Checks
make test(134 tests) andmake preflightpass locally.Note
Windows-built artifacts are not covered by CI: the packaging matrix builds on Windows but does not run
test_mod_generatorthere. Worth a separate look.