Binlogs Redacting support + Binlogs forward-compatibility reading support#9307
Conversation
rokonec
left a comment
There was a problem hiding this comment.
Request change is only for missing that "backward-compatible-with" field.
Otherwise it looks OK. But I have to admit that I have not time to carefully review full extent of this PR.
|
Is there a design doc or some overview of what the redactor does - it sounds interesting? |
Hi @danmoseley, Design proposal of redactor is located here: https://github.com/JanKrivanek/MSBuildBinlogRedactor/blob/main/docs/DesignProposal.md (ultimate long term wishful plan is here https://github.com/dotnet/msbuild/blob/main/documentation/specs/proposed/security-metadata.md) |
ladipro
left a comment
There was a problem hiding this comment.
I've left comments inline, mostly nits.
src/Build/Logging/BinaryLogger/Postprocessing/TransparentReadStream.cs
Outdated
Show resolved
Hide resolved
src/Build/Logging/BinaryLogger/Postprocessing/IBuildEventArgsReaderNotifications.cs
Outdated
Show resolved
Hide resolved
src/Build/Logging/BinaryLogger/Postprocessing/IBinlogReaderErrors.cs
Outdated
Show resolved
Hide resolved
src/Build/Logging/BinaryLogger/Postprocessing/IBinlogReaderErrors.cs
Outdated
Show resolved
Hide resolved
src/Build/Logging/BinaryLogger/Postprocessing/IBinlogReaderErrors.cs
Outdated
Show resolved
Hide resolved
src/Build/Logging/BinaryLogger/Postprocessing/IBuildEventStringsReader.cs
Outdated
Show resolved
Hide resolved
src/Build/Logging/BinaryLogger/Postprocessing/IBuildFileReader.cs
Outdated
Show resolved
Hide resolved
src/Build/Logging/BinaryLogger/Postprocessing/ArchiveFileEventArgsExtensions.cs
Outdated
Show resolved
Hide resolved
src/Build/Logging/BinaryLogger/Postprocessing/EmbeddedContentKind.cs
Outdated
Show resolved
Hide resolved
292072d to
a71f8b3
Compare
Fixes #9147, #9261 and contributes to #8400
Viewer complement PR: KirillOsenkov/MSBuildStructuredLog#732
Important
This contains all changes from
Context and changes are described in those individual PRs
Feel free to review everything together (this PR), or separately (above 2 PRs) - but not all 3 as you'd be viewing same changes twice
TBD:
localize all stringsDoneI'm contemplating adding support for recognizing over-reading and mismatched reading during events deserializationDone