Skip to content

Wire up the properties reading/writing via loggingContexts#10237

Merged
JanKrivanek merged 29 commits intomainfrom
proto/buildcheck-properties-wiring
Jul 25, 2024
Merged

Wire up the properties reading/writing via loggingContexts#10237
JanKrivanek merged 29 commits intomainfrom
proto/buildcheck-properties-wiring

Conversation

@JanKrivanek
Copy link
Copy Markdown
Member

@JanKrivanek JanKrivanek commented Jun 13, 2024

Contributes towards #9883

Context

This is 2nd from sequence of 3 changesets delivering the "Undefined Property Used" BuildCheck
This changeset wires the property read and write info to the buildcheck. The data are being sent via the LoggingContext->LoggingService->BuildCheck chain - so that we can easily add a reroute that will perform LoggingContext->LoggingService->BuildEventArgs remoting to the central node ->BuildCheck

The actuall check is not part of this changeset - it'll be delivered subsequently

Changes Made

  • Imposing nullable checks on touched code
  • Making sure BuildEventContext is allways initialized in LoggingService
  • Extended the LoggingContecxt and LoggingService internal contract so that they can serve as sinks for build execution produced data
  • Wired the property reads and writes data from engine to LoggingContext
  • Wired the property reads and writes data from LoggingService to BuildCheck
  • Wiring of eventing in BuildCheck

Design documentation

(part of this PR)

https://github.com/dotnet/msbuild/blob/proto/buildcheck-properties-wiring/documentation/specs/proposed/BuildCheck-Architecture.md#sourcing-unexposed-data-from-within-execution

Notes

@f-alizada
Copy link
Copy Markdown
Contributor

I'll need to do another round, over the changes. Thank you!

@JanKrivanek
Copy link
Copy Markdown
Member Author

Btw. there are heavy conflicts with current main - so I'm currently going through a bit of refactoring

JanKrivanek and others added 6 commits July 3, 2024 11:29
changed namespace to Microsoft.Build.Experimental.BuildCheck.Infrastructure for new interfaces
Copy link
Copy Markdown
Member

@surayya-MS surayya-MS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!
When we create the analyzer, we should add to the doc that it won't work for replay mode

@JanKrivanek
Copy link
Copy Markdown
Member Author

@JanKrivanek JanKrivanek requested a review from f-alizada July 8, 2024 05:32
JanKrivanek and others added 2 commits July 8, 2024 16:10
@JanKrivanek JanKrivanek requested a review from AR-May July 8, 2024 14:23
Copy link
Copy Markdown
Member

@AR-May AR-May left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, this PR looks good for me.

@JanKrivanek JanKrivanek enabled auto-merge (squash) July 25, 2024 13:44
@JanKrivanek JanKrivanek disabled auto-merge July 25, 2024 14:44
@JanKrivanek JanKrivanek merged commit bc39ba8 into main Jul 25, 2024
@JanKrivanek JanKrivanek deleted the proto/buildcheck-properties-wiring branch July 25, 2024 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants