search

dotnet / msbuild

The Microsoft Build Engine (MSBuild) is the build platform for .NET and Visual Studio.
https://docs.microsoft.com/visualstudio/msbuild/msbuild
MIT License
5.22k stars 1.35k forks source link

Custom loggers could make default loggers miss information. #9121

Open AR-May opened 1 year ago

AR-May commented 1 year ago

During investigation of #9098 we found a following design problem: If

Then we would miss evaluation items and properties in all the loggers.

Analysis: Despite the name, IncludeEvaluationPropertiesAndItems does not control whether to include or not properties and items to log of the particular logger and rather has a meaning of a logger version. In fact, its presence means that properties and items are included in ProjectEvaluationFinished event for modern loggers and absence that they are included in ProjectStarted event for legacy loggers. So, if one of the attached loggers is legacy ( which is detected by missing IncludeEvaluationPropertiesAndItems), so for backwards compatibility all other modern loggers would not obtain properties in the event they expect, see this commit.

AR-May commented 1 year ago

Team triage: We might consider more proper documentation on writing custom loggers where this would be mentioned. fyi @ghogen

KirillOsenkov commented 1 year ago

Also perhaps if we detect a legacy logger, issue a message at the beginning of the build with some info.

KirillOsenkov commented 1 year ago

Also this page needs to be expanded: https://learn.microsoft.com/en-us/dotnet/api/microsoft.build.framework.ieventsource4.includeevaluationpropertiesanditems?view=msbuild-17-netcore

As well as mention that there's an environment variable MSBUILDLOGPROPERTIESANDITEMSAFTEREVALUATION=1 to force the new mode and ignore legacy loggers.

Here's the relevant code where the flag is read: https://github.com/dotnet/msbuild/blob/c36a54ed3308d1516ffe1a86b9086c42e4ca996f/src/Build/BackEnd/Components/Logging/LoggingService.cs#L547

Also with the legacy behavior, there was a bug where properties and items were not logged from other nodes, so you would only get properties and items on ProjectStartedEventArgs for projects building in the central node.

You can opt in to the new behavior if all loggers call IEventSource4.IncludeEvaluationPropertiesAndItems() or the environment variable is set. With the new behavior properties and items are logged at the evaluation of each project at ProjectEvaluationFinishedEventArgs. This also works regardless of which node the project is evaluated or built.