Build: adjust the BuildSystemDelegate hander #4192
Merged
Conversation
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
compnerd
requested review from
abertelrud,
neonichu,
tomerd and
elsh
as code owners
llbuild does not make guarantees that the output from subcommands are emitted in complete buffers. This shows up particularly well on Windows. clang will emit diagnostics which get read in different chunks. By emitting these eagerly we would print partial messages which would leave the terminal in the improper state due to the message not being complete. With multiple parallel threads writing the messages partially, we would get splits in the message that would yield unreadable messages. Switch to a buffering approach, buffering the entire content until the command exits. At that time we can emit the complete message in a "single" write (which is locked). This ensures that the output is no longer partially emitted and corrects the rendering with the animated emission on Windows.
|
@swift-ci please test |
tomerd
reviewed
Mar 3, 2022
Co-authored-by: tomer doron <[email protected]>
tomerd
reviewed
Mar 3, 2022
|
@swift-ci please smoke test |
|
this seems fine to me. @abertelrud ? |
Absolutely, makes sense to me as well. We should probably audit some of the other callbacks but that can be done as follow-on tasks. Thanks for this fix @compnerd! |
abertelrud
approved these changes
Mar 4, 2022
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.
llbuild does not make guarantees that the output from subcommands are
emitted in complete buffers. This shows up particularly well on
Windows. clang will emit diagnostics which get read in different
chunks. By emitting these eagerly we would print partial messages which
would leave the terminal in the improper state due to the message not
being complete. With multiple parallel threads writing the messages
partially, we would get splits in the message that would yield
unreadable messages.
Switch to a buffering approach, buffering the entire content until the
command exits. At that time we can emit the complete message in a
"single" write (which is locked). This ensures that the output is no
longer partially emitted and corrects the rendering with the animated
emission on Windows.
[One line description of your change]
Motivation:
[Explain here the context, and why you're making that change. What is the problem you're trying to solve.]
Modifications:
[Describe the modifications you've done.]
Result:
[After your change, what will change.]