-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Fix test output streaming on Windows #9191
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
base: main
Are you sure you want to change the base?
Conversation
@swift-ci test |
95a8c1c
to
1226c88
Compare
@swift-ci test |
@swift-ci test Windows |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yay! Yes, the only proper way to read stdout/stderr on Windows is multi-threaded. I do wonder if creating dedicated threads for this instead of using the Task thread pool would be safer.
This is fine, it's all non-blocking through Dispatch. |
1226c88
to
39c51cf
Compare
@swift-ci test |
39c51cf
to
c2dd0c0
Compare
@swift-ci test |
@swift-ci please test windows |
c2dd0c0
to
3483f59
Compare
@swift-ci test |
@swift-ci test Windows |
while !Task.isCancelled { | ||
let chunk = try await readChunk(upToLength: 4096) | ||
if chunk.isEmpty { | ||
return nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be a continuation (e.g. cont.finish() here)? I'm not sure what the implications are of returning nil are for clients of this stream.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
returning nil here is intended to terminate the stream, this initializer doesn't use a continuation
3483f59
to
f153b91
Compare
@swift-ci test |
@swift-ci test Windows |
f153b91
to
7c818d8
Compare
@swift-ci test |
@swift-ci test Windows |
The AsyncProcess implementation on Windows was effectively buffering all output until process exit. Vendor over some utilities from Swift Build to stream the output, and attempt to avoid the bug addressed in #8047 (review)