-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
[BuildCheck] Add OM and infra for tracking task invocations #10100
Conversation
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.
This looks good - thank you!!
The only objection I have is about the expressing of the location info.
Btw. - don't you want to extend the TaskParameterEventArgs
as part of this PR? If not - would be nice to have addition PR for that - possibly merged prior this one.
Thank you for the quick review.
The fact that assigning task outputs to properties is currently logged as text messages will need to be addressed in a careful and viewer-friendly way. Let me think about it a bit. I would prefer to do it in a separate PR without blocking this one. It should not be an issue for the double writes analyzer, for example. |
FYI @KirillOsenkov - just a heads up that another change is comming ;-) But I bet that move from textual to structured info is allways appreciated. |
Do we have a sample analyzer that would exercise this? It's always better to design APIs driven by a real-world use case. Would love to see how this API can be used at the consumer side. |
True that we don't currently have an analyzer that would need this. It is motivated by completeness - when something is interested in task parameters, it feels natural to provide both inputs and outputs. The current model follows the MSBuild syntax: The viewer renders |
Here's what I'm planning to do to address the missing output parameter name issue: https://github.com/ladipro/msbuild/pull/1/files |
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!!
This reverts commit 261e4bb.
Contributes to #9881
Context
Some build checks will want to analyze tasks executed during build. This PR is adding support for these by introducing several new types and implementing processing of task-related logger events in
BuildEventsProcessor
.Changes Made
RegisterTaskInvocationAction
for analyzers to call to subscribe to task events.TaskInvocationAnalysisData
as a unit of reporting task events to analyzers.TaskStartedEventArgs
,TaskFinishedEventArgs
, andTaskParameterEventArgs
toTaskInvocationAnalysisData
.Testing
New unit tests.