-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[utils] Make update-checkout output more readable #41714
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
In Python 3, subprocess output is read as binary data by default, which isn’t what we want. Instead of reading process output as byte strings, then manually decoding them into strings, simply pass `text=True` to functions in the `subprocess` module, so that we get properly decoded strings right out the box. This fixes places that forget to do the decoding step — most especially, the `update-checkout` script. That script prints Git output as byte strings, which leads to unreadable results. Additionally, in shell.run, use the same pipe for capturing both stdout and stderr. The distinction is pretty pointless in this use case; however, keeping the two channels separate means that we lose the original ordering of printed messages, which does matter.
@swift-ci test |
Ah, I didn't know Python 2.7 was still a thing on the Linux bots 😬
|
@swift-ci test |
1 similar comment
@swift-ci test |
Before:
After:
Stdout and stderr are still garbled; I think the process pool isn't opening new pipes for each process. |
@swift-ci test |
407e0ba
to
a11b8d9
Compare
@swift-ci test |
1 similar comment
@swift-ci test |
…ed line with the repo name
e182dce
to
4f9cf1d
Compare
@swift-ci test |
And now it looks like this:
Things are in order, and interleaved messages are neatly labeled with their corresponding repository name. |
Thanks @lorentey! |
It would be also nice if we stop printing all env variable with command line output. |
@shahmishal Fix at #41778 |
In Python 3, subprocess output is read as binary data by default, which isn’t what we want.
Instead of reading process output as byte strings, then manually decoding them into strings, simply pass
text=True
to functions in thesubprocess
module, so that we get properly decoded strings right out the box.This fixes places that forget to do the decoding step — most especially, the
update-checkout
script. That script prints Git output as byte strings, which leads to unreadable results.Additionally, in shell.run, use the same pipe for capturing both stdout and stderr. The distinction is pretty pointless in this use case; however, keeping the two channels separate means that we lose the original ordering of printed messages, which does matter.