Make 'run_cmd' operate without universal newlines#462
Merged
Conversation
Contributor
Author
|
OK, I'm not sure if this was an issue beforehand, but it is definitely an issue now: If we start to get output that looks like: Then Python exits with |
Contributor
Author
|
OK, no, this issue already existed beforehand: logSo this is fine then, though we should probably fix it. |
In 'text' mode (an alias for universal_newlines), Python converts all \r\n sequences into just \n, under the expectation that on Linux it is not necessary to explicit emit a carriage return (and that this would be consistent with other platforms where it does similar). However, this is not always true: if `stty -onlcr` is set, then newline (\n) does not automatically emit a carriage return (\r). Our machine queue scripts (linked below) explicitly set `-onlcr`, so a \r is also needed for \n to behave as expected. But with universal newlines on, the \r emitted by the other end of `mq.sh` ends up being stripped. https://github.com/seL4/machine_queue/blob/master/scripts/remote#L16 We turn off 'text'/'universal_newlines' mode, leaving the output in raw binary (which also necessitates disabling line buffering as python does not support that, and so we disable all buffering entirely); then we decode() to text as necessary for the lines return-value, and print binary to our stdout. Signed-off-by: Julia Vassiliki <julia.vassiliki@unsw.edu.au>
midnightveil
force-pushed
the
julia/onlcr-handling
branch
from
893e433 to
98943f0
Compare
Indanz
approved these changes
May 22, 2026
Contributor
Indanz
left a comment
Indanz
left a comment
There was a problem hiding this comment.
Have you tried not setting onlcr too or only this work-around?
Contributor
Author
I haven't messed with the machine queue scripts, no. I don't know why the scripts even invoke stty tbh, I thought that SSH forwards the PTY ioctls across and conserver on the remote end does make those work. So I'm wary to change it given I don't know why it is like it is. |
lsf37
approved these changes
May 26, 2026
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
3 participants
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.
In 'text' mode (an alias for universal_newlines), Python converts all \r\n sequences into just \n, under the expectation that on Linux it is not necessary to explicit emit a carriage return (and that this would be consistent with other platforms where it does similar).
However, this is not always true: if
stty -onlcris set, then newline (\n) does not automatically emit a carriage return (\r). Our machine queue scripts (linked below) explicitly set-onlcr, so a \r is also needed for \n to behave as expected. But with universal newlines on, the \r emitted by the other end ofmq.shends up being stripped.https://github.com/seL4/machine_queue/blob/master/scripts/remote#L16
We turn off 'text'/'universal_newlines' mode, leaving the output in raw binary (which also necessitates disabling line buffering as python does not support that, and so we disable all buffering entirely); then we decode() to text as necessary for the lines return-value, and print binary to our stdout.
This is necessary to make the local CI runner I added for microkit to emit output that does not perform the staircase output when \r is not being emitted.