-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Rendering message about build cancellation (Terminal Logger) #10055
Rendering message about build cancellation (Terminal Logger) #10055
Conversation
Follow-on question - it's always kind of bothered me that a cancelled build is considered |
I like having "cancelled" logging but it's "failed" because it didn't "complete without errors", which makes sense IMO. |
@rainersigwald, you mentioned that you would like to have the "build cancellation event". Could you please elaborate? Would it be just a new event or it will be used also as an indication for loggers to stop (instead of cancellation token)? Token is still IMO expected way how to cancel :) |
I was thinking more or less of a new |
It seems there is no infrastructure for emitting events where the context is overall build (or I didn't looked properly). If no one knows how/where to send the event without node/project/target/task context, then I propose to finish this issue with cancellation token and create new issue tracking the "global events" API. |
^ @rainersigwald, @ladipro, @rokonec, do you agree? |
I would expect the new event to use something analogous to |
Good tip. It allowed me to discover |
@rainersigwald, I believe it also fixes #9898. I'm not able to repro the busy indicator problem with these changes. |
src/Build/BackEnd/Components/Logging/LoggingServiceLogMethods.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Rainer Sigwald <raines@microsoft.com>
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.
I'm voting for special handling the new event only in loggers that wishes to do so
src/Build/BackEnd/Components/Logging/BuildEventArgTransportSink.cs
Outdated
Show resolved
Hide resolved
src/Build/BackEnd/Components/Logging/LoggingServiceLogMethods.cs
Outdated
Show resolved
Hide resolved
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.
Thank you - I like this more simplistic approach
Fixes #8983
Context
When you cancel command-line MSBuild, we send the cancellation signal through the API and also writes status to stdout. But it does it directly in XMake, and TL tends to immediately overwrite that message with status updates, so it's not obvious whether cancellation is in progress.
Changes Made
An event signaling cancellation was added to the new
IEventSource5
. Sources implementing v4 were updated to add support for new event and both terminal logger and console logger(s) are using this event to inform about cancellation.Testing
Manual:
and new unit tests were added.
Notes