Conversation
…o reuse it for binlog replay 2. removed inheritance of EventArgsDispatcher from BinaryLogReplayEventSource, instead use it as a field 3. created BuildCheckEventArgsDispatcher
… AnalysisDisatcherContext; remove commented code
|
Looks like there is a compatibility problem I will change the implementation slightly to make it compatible |
JanKrivanek
left a comment
There was a problem hiding this comment.
Overall looks good!
It feels that it deserves a small diagram or a design description
src/Build/BuildCheck/Infrastructure/BuildCheckBinaryLogReplaySourcerWrapper.cs
Outdated
Show resolved
Hide resolved
src/Build/BuildCheck/Infrastructure/AnalysisContext/AnalysisLoggingContext.cs
Show resolved
Hide resolved
JanKrivanek
left a comment
There was a problem hiding this comment.
Thank you @surayya-MS - to me it appears to be a solid implementation!!
I'd love to see the description of the replay mechanics, together with the diagram as part of the https://github.com/dotnet/msbuild/blob/main/documentation/specs/proposed/BuildCheck-Architecture.md
The new interfaces and classes would probably benefit from brief xml comments (doesn't have to be on each method - but at least for the whole type)
|
One diagram I'd like (not blocking for this PR but I think it'd help) is something like this: graph LR
Real[Live build] -->|EventArgs| mux[???] -->|BuildCheck API| check[BuildCheck<br/><i>#40;doesn't care about<br/>live vs replay#41;</i>]
log[Replayed log] -->|EventArgs| mux
check -->|Check results| out[???]
with those question-mark blocks filled in. |
src/Build/BuildCheck/Infrastructure/AnalysisContext/AnalysisDispatchingContext.cs
Outdated
Show resolved
Hide resolved
src/Build/BuildCheck/Infrastructure/AnalysisContext/AnalysisDispatchingContext.cs
Show resolved
Hide resolved
src/Build/BuildCheck/Infrastructure/AnalysisContext/AnalysisDispatchingContext.cs
Outdated
Show resolved
Hide resolved
src/Build/BuildCheck/Infrastructure/BuildCheckBuildEventHandler.cs
Outdated
Show resolved
Hide resolved
…spatchingContext.cs Co-authored-by: Rainer Sigwald <raines@microsoft.com>
…spatchingContext.cs Co-authored-by: Rainer Sigwald <raines@microsoft.com>
…r.cs Co-authored-by: Rainer Sigwald <raines@microsoft.com>
Co-authored-by: Rainer Sigwald <raines@microsoft.com>
Co-authored-by: Rainer Sigwald <raines@microsoft.com>
|
Events come from BinaryLogReplayEventSource and passed to
The new events from BuildCheck are passed to the mergedEventSource |

Fixes #9760
Refactoring of BuildCheck infrastructure
Currently BuildCheck uses
LoggingServiceandLoggingContextproduced by theAnalyzerLoggingContextFactory.Removed usage of
LoggingServiceinstance and instead usedLoggingContextto log events. This was needed becauseBuildCheckManagercould use theLoggingServiceinstance from the very first build that is no longer available afterwards. The commits:31af242
e47a8ff
Now I needed to decouple from
AnalyzerLoggingContext : LoggingContext. The only usage ofAnalyzerLoggingContextare these methods:LogComment,LogCommentFromText,LogErrorFromTextandLogBuildEvent. So, I created IAnalysisContext with similar methods, and the names start withDispatch:Then created AnalysisLoggingContext : IAnalysis that uses
LoggingServiceand AnalysisLoggingContextFactory similar to the existing AnalyzerLoggingContextFactory.Implementing BuildCheck Replay Mode
Now, I need to be able to produce new
BuildEventArgsand invoke it for the loggers to pick it up. Similar to whatLoggingServicedoes. For that reason, implemented AnalysisDispatchingContext : IAnalysisContext and the factory - AnalysisDispatchingContextFactory.Because the code for the creation of the events was duplicated, I put it all in a helper class EventsCreatorHelper.
Seperated the logic of handling the events from BuildCheckConnectorLogger and put it into BuildCheckBuildEventHandler to reuse it later.
The last part. When replaying binary log with BuildCheck enabled, I create a new IEventSource mergedEventSource. The logic is extracted into a new static class BuildCheckReplayModeConnector
replayEventSourceare passed to themergedEventSourceunchanged.BuildCheckBuildEventHandler.HandleBuildEventto thereplayEventSource.AnyEventRaisedevent, in order to produce new events from BuildCheck.BuildCheckBuildEventHandlerinstance usesmergedEventSourceto invoke new eventsBinaryLogReplayEventSource replaySourceNotes