From c1d088e5802c13dfed6438bfe5b333a616859c8a Mon Sep 17 00:00:00 2001 From: Jan Krivanek Date: Mon, 16 Sep 2024 21:46:05 +0200 Subject: [PATCH] Proto/buildcheck warns promotability (#10668) * Initial version of evaluation time warnings deferring * Transfer the warn as error/msg info via event args * Reflect PR review suggestions * Accomodate buildcheck build errors happening in the main node * Tailor the build error case for buildcheck --- .../BackEnd/MockLoggingService.cs | 26 +++++++ .../Components/Logging/ILoggingService.cs | 14 ++++ .../Components/Logging/LoggingService.cs | 36 ++++++++-- .../Logging/LoggingServiceLogMethods.cs | 37 +++++++++- .../Components/Logging/NodeLoggingContext.cs | 9 ++- .../Logging/ProjectLoggingContext.cs | 72 ++++++++++++++++++- .../RequestBuilder/RequestBuilder.cs | 43 ++++++++--- .../BuildCheckBuildEventHandler.cs | 7 +- .../BuildCheckManagerProvider.cs | 58 +++++++++++++-- .../BuildCheck/Infrastructure/CheckWrapper.cs | 7 +- .../Infrastructure/IBuildCheckManager.cs | 2 +- .../Infrastructure/NullBuildCheckManager.cs | 2 +- src/BuildCheck.UnitTests/EndToEndTests.cs | 35 +++++++++ .../Project1.csproj | 2 + .../BuildCheck/BuildCheckEventArgs.cs | 12 +--- .../BuildCheck/EnumerableExtensions.cs | 30 ++++++++ src/Framework/BuildEventContext.cs | 10 +++ src/Framework/ProjectStartedEventArgs.cs | 54 ++++++++++++++ 18 files changed, 415 insertions(+), 41 deletions(-) diff --git a/src/Build.UnitTests/BackEnd/MockLoggingService.cs b/src/Build.UnitTests/BackEnd/MockLoggingService.cs index e19d7fbec5b..23d3cf093e8 100644 --- a/src/Build.UnitTests/BackEnd/MockLoggingService.cs +++ b/src/Build.UnitTests/BackEnd/MockLoggingService.cs @@ -550,6 +550,32 @@ public BuildEventContext LogProjectStarted( return new BuildEventContext(0, 0, 0, 0); } + public void LogProjectStarted(ProjectStartedEventArgs args) + { } + + public ProjectStartedEventArgs CreateProjectStarted( + BuildEventContext nodeBuildEventContext, + int submissionId, + int configurationId, + BuildEventContext parentBuildEventContext, + string projectFile, + string targetNames, + IEnumerable properties, + IEnumerable items, + int evaluationId = BuildEventContext.InvalidEvaluationId, + int projectContextId = BuildEventContext.InvalidProjectContextId) + { + return new ProjectStartedEventArgs( + configurationId, + message: null, + helpKeyword: null, + projectFile, + targetNames, + properties, + items, + parentBuildEventContext); + } + /// /// Logs a project finished event /// diff --git a/src/Build/BackEnd/Components/Logging/ILoggingService.cs b/src/Build/BackEnd/Components/Logging/ILoggingService.cs index 104dac56f6f..b57ab84ae82 100644 --- a/src/Build/BackEnd/Components/Logging/ILoggingService.cs +++ b/src/Build/BackEnd/Components/Logging/ILoggingService.cs @@ -555,6 +555,20 @@ BuildEventContext LogProjectStarted( int evaluationId = BuildEventContext.InvalidEvaluationId, int projectContextId = BuildEventContext.InvalidProjectContextId); + void LogProjectStarted(ProjectStartedEventArgs args); + + ProjectStartedEventArgs CreateProjectStarted( + BuildEventContext nodeBuildEventContext, + int submissionId, + int configurationId, + BuildEventContext parentBuildEventContext, + string projectFile, + string targetNames, + IEnumerable properties, + IEnumerable items, + int evaluationId = BuildEventContext.InvalidEvaluationId, + int projectContextId = BuildEventContext.InvalidProjectContextId); + /// /// Log that the project has finished /// diff --git a/src/Build/BackEnd/Components/Logging/LoggingService.cs b/src/Build/BackEnd/Components/Logging/LoggingService.cs index 69c729a7cd1..b5ed777d161 100644 --- a/src/Build/BackEnd/Components/Logging/LoggingService.cs +++ b/src/Build/BackEnd/Components/Logging/LoggingService.cs @@ -211,6 +211,11 @@ internal partial class LoggingService : ILoggingService, INodePacketHandler /// private readonly ISet _buildSubmissionIdsThatHaveLoggedErrors = new HashSet(); + /// + /// A list of build submission IDs that have logged errors through buildcheck. If an error is logged outside of a submission, the submission ID is . + /// + private readonly ISet _buildSubmissionIdsThatHaveLoggedBuildcheckErrors = new HashSet(); + /// /// A list of warnings to treat as errors for an associated . If an empty set, all warnings are treated as errors. /// @@ -620,6 +625,11 @@ public bool IncludeEvaluationPropertiesAndItemsInEvaluationFinishedEvent /// true if the build submission logged an errors, otherwise false. public bool HasBuildSubmissionLoggedErrors(int submissionId) { + if (_buildSubmissionIdsThatHaveLoggedBuildcheckErrors.Contains(submissionId)) + { + return true; + } + // Warnings as errors are not tracked if the user did not specify to do so if (WarningsAsErrors == null && _warningsAsErrorsByProject == null) { @@ -730,6 +740,11 @@ public void AddWarningsAsMessages(BuildEventContext buildEventContext, ISetCodes to add private void AddWarningsAsMessagesOrErrors(ref IDictionary> warningsByProject, BuildEventContext buildEventContext, ISet codes) { + if (codes == null) + { + return; + } + lock (_lockObject) { WarningsConfigKey key = GetWarningsConfigKey(buildEventContext); @@ -1638,14 +1653,25 @@ private void RouteBuildEvent(object loggingEvent) if (buildEventArgs is BuildErrorEventArgs errorEvent) { - // Keep track of build submissions that have logged errors. If there is no build context, add BuildEventContext.InvalidSubmissionId. - _buildSubmissionIdsThatHaveLoggedErrors.Add(errorEvent.BuildEventContext?.SubmissionId ?? BuildEventContext.InvalidSubmissionId); + int submissionId = errorEvent.BuildEventContext?.SubmissionId ?? BuildEventContext.InvalidSubmissionId; + + if (buildEventArgs is BuildCheckResultError) + { + _buildSubmissionIdsThatHaveLoggedBuildcheckErrors.Add(submissionId); + } + else + { + // Keep track of build submissions that have logged errors. If there is no build context, add BuildEventContext.InvalidSubmissionId. + _buildSubmissionIdsThatHaveLoggedErrors.Add(submissionId); + } } - if (buildEventArgs is BuildCheckResultError checkResultError) + // If this is BuildCheck-ed build - add the warnings promotability/demotability to the service + if (buildEventArgs is ProjectStartedEventArgs projectStartedEvent && this._componentHost.BuildParameters.IsBuildCheckEnabled) { - // If the specified BuildCheckResultError was issued, an empty ISet signifies that the specified build check warnings should be treated as errors. - AddWarningsAsErrors(checkResultError.BuildEventContext, new HashSet()); + AddWarningsAsErrors(projectStartedEvent.BuildEventContext, projectStartedEvent.WarningsAsErrors); + AddWarningsAsMessages(projectStartedEvent.BuildEventContext, projectStartedEvent.WarningsAsMessages); + AddWarningsNotAsErrors(projectStartedEvent.BuildEventContext, projectStartedEvent.WarningsNotAsErrors); } if (buildEventArgs is ProjectFinishedEventArgs projectFinishedEvent && projectFinishedEvent.BuildEventContext != null) diff --git a/src/Build/BackEnd/Components/Logging/LoggingServiceLogMethods.cs b/src/Build/BackEnd/Components/Logging/LoggingServiceLogMethods.cs index eeded58a88a..c6f711eb8c6 100644 --- a/src/Build/BackEnd/Components/Logging/LoggingServiceLogMethods.cs +++ b/src/Build/BackEnd/Components/Logging/LoggingServiceLogMethods.cs @@ -497,6 +497,39 @@ public BuildEventContext LogProjectStarted( IEnumerable items, int evaluationId = BuildEventContext.InvalidEvaluationId, int projectContextId = BuildEventContext.InvalidProjectContextId) + { + var args = CreateProjectStarted(nodeBuildEventContext, + submissionId, + configurationId, + parentBuildEventContext, + projectFile, + targetNames, + properties, + items, + evaluationId, + projectContextId); + + this.LogProjectStarted(args); + + return args.BuildEventContext; + } + + public void LogProjectStarted(ProjectStartedEventArgs buildEvent) + { + ProcessLoggingEvent(buildEvent); + } + + public ProjectStartedEventArgs CreateProjectStarted( + BuildEventContext nodeBuildEventContext, + int submissionId, + int configurationId, + BuildEventContext parentBuildEventContext, + string projectFile, + string targetNames, + IEnumerable properties, + IEnumerable items, + int evaluationId = BuildEventContext.InvalidEvaluationId, + int projectContextId = BuildEventContext.InvalidProjectContextId) { ErrorUtilities.VerifyThrow(nodeBuildEventContext != null, "Need a nodeBuildEventContext"); @@ -560,9 +593,7 @@ public BuildEventContext LogProjectStarted( buildRequestConfiguration.ToolsVersion); buildEvent.BuildEventContext = projectBuildEventContext; - ProcessLoggingEvent(buildEvent); - - return projectBuildEventContext; + return buildEvent; } /// diff --git a/src/Build/BackEnd/Components/Logging/NodeLoggingContext.cs b/src/Build/BackEnd/Components/Logging/NodeLoggingContext.cs index 5676d9fecf1..e03c8ed13e7 100644 --- a/src/Build/BackEnd/Components/Logging/NodeLoggingContext.cs +++ b/src/Build/BackEnd/Components/Logging/NodeLoggingContext.cs @@ -57,9 +57,16 @@ internal void LogBuildFinished(bool success) /// The build request entry for this project. /// The BuildEventContext to use for this project. internal ProjectLoggingContext LogProjectStarted(BuildRequestEntry requestEntry) + { + (ProjectStartedEventArgs arg, ProjectLoggingContext ctx) = CreateProjectLoggingContext(requestEntry); + LoggingService.LogProjectStarted(arg); + return ctx; + } + + internal (ProjectStartedEventArgs, ProjectLoggingContext) CreateProjectLoggingContext(BuildRequestEntry requestEntry) { ErrorUtilities.VerifyThrow(this.IsValid, "Build not started."); - return new ProjectLoggingContext(this, requestEntry); + return ProjectLoggingContext.CreateLoggingContext(this, requestEntry); } /// diff --git a/src/Build/BackEnd/Components/Logging/ProjectLoggingContext.cs b/src/Build/BackEnd/Components/Logging/ProjectLoggingContext.cs index 06614c42125..69e796772b4 100644 --- a/src/Build/BackEnd/Components/Logging/ProjectLoggingContext.cs +++ b/src/Build/BackEnd/Components/Logging/ProjectLoggingContext.cs @@ -71,6 +71,45 @@ internal ProjectLoggingContext( { } + /// + /// Creates ProjectLoggingContext, without logging ProjectStartedEventArgs as a side effect. + /// The ProjectStartedEventArgs is returned as well - so that it can be later logged explicitly + /// + public static (ProjectStartedEventArgs, ProjectLoggingContext) CreateLoggingContext( + NodeLoggingContext nodeLoggingContext, BuildRequestEntry requestEntry) + { + ProjectStartedEventArgs args = CreateProjectStarted( + nodeLoggingContext, + requestEntry.Request.SubmissionId, + requestEntry.Request.ConfigurationId, + requestEntry.RequestConfiguration.ProjectFullPath, + requestEntry.Request.Targets, + requestEntry.RequestConfiguration.ToolsVersion, + requestEntry.RequestConfiguration.Project.PropertiesToBuildWith, + requestEntry.RequestConfiguration.Project.ItemsToBuildWith, + requestEntry.Request.ParentBuildEventContext, + requestEntry.RequestConfiguration.Project.EvaluationId, + requestEntry.Request.ProjectContextId); + + return (args, new ProjectLoggingContext(nodeLoggingContext, args)); + } + + private ProjectLoggingContext( + NodeLoggingContext nodeLoggingContext, + ProjectStartedEventArgs projectStarted) + : base(nodeLoggingContext, projectStarted.BuildEventContext) + { + _projectFullPath = projectStarted.ProjectFile; + + // No need to log a redundant message in the common case + if (projectStarted.ToolsVersion != "Current") + { + LoggingService.LogComment(this.BuildEventContext, MessageImportance.Low, "ToolsVersionInEffectForBuild", projectStarted.ToolsVersion); + } + + this.IsValid = true; + } + /// /// Constructs a project logging contexts. /// @@ -122,6 +161,37 @@ private static BuildEventContext CreateInitialContext( BuildEventContext parentBuildEventContext, int evaluationId, int projectContextId) + { + ProjectStartedEventArgs args = CreateProjectStarted( + nodeLoggingContext, + submissionId, + configurationId, + projectFullPath, + targets, + toolsVersion, + projectProperties, + projectItems, + parentBuildEventContext, + evaluationId, + projectContextId); + + nodeLoggingContext.LoggingService.LogProjectStarted(args); + + return args.BuildEventContext; + } + + private static ProjectStartedEventArgs CreateProjectStarted( + NodeLoggingContext nodeLoggingContext, + int submissionId, + int configurationId, + string projectFullPath, + List targets, + string toolsVersion, + PropertyDictionary projectProperties, + IItemDictionary projectItems, + BuildEventContext parentBuildEventContext, + int evaluationId, + int projectContextId) { IEnumerable properties = null; IEnumerable items = null; @@ -171,7 +241,7 @@ private static BuildEventContext CreateInitialContext( properties = projectPropertiesToSerialize.Select((ProjectPropertyInstance property) => new DictionaryEntry(property.Name, property.EvaluatedValue)); } - return loggingService.LogProjectStarted( + return loggingService.CreateProjectStarted( nodeLoggingContext.BuildEventContext, submissionId, configurationId, diff --git a/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs b/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs index ccd5e3851e2..5df61cc392b 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Concurrent; using System.Collections.Generic; +using System.Diagnostics; using System.Globalization; using System.IO; using System.Linq; @@ -1105,11 +1106,11 @@ private async Task BuildProject() ErrorUtilities.VerifyThrow(_targetBuilder != null, "Target builder is null"); // We consider this the entrypoint for the project build for purposes of BuildCheck processing - bool isRestoring = _requestEntry.RequestConfiguration.GlobalProperties[MSBuildConstants.MSBuildIsRestoring] is null; + bool isRestoring = _requestEntry.RequestConfiguration.GlobalProperties[MSBuildConstants.MSBuildIsRestoring] is not null; var buildCheckManager = isRestoring - ? (_componentHost.GetComponent(BuildComponentType.BuildCheckManagerProvider) as IBuildCheckManagerProvider)!.Instance - : null; + ? null + : (_componentHost.GetComponent(BuildComponentType.BuildCheckManagerProvider) as IBuildCheckManagerProvider)!.Instance; buildCheckManager?.SetDataSource(BuildCheckDataSource.BuildExecution); @@ -1154,15 +1155,10 @@ private async Task BuildProject() _requestEntry.Request.BuildEventContext); } - _projectLoggingContext = _nodeLoggingContext.LogProjectStarted(_requestEntry); - buildCheckManager?.StartProjectRequest( - _projectLoggingContext.BuildEventContext, - _requestEntry.RequestConfiguration.ProjectFullPath); - + try { - // Now that the project has started, parse a few known properties which indicate warning codes to treat as errors or messages - ConfigureWarningsAsErrorsAndMessages(); + HandleProjectStarted(buildCheckManager); // Make sure to extract known immutable folders from properties and register them for fast up-to-date check ConfigureKnownImmutableFolders(); @@ -1273,6 +1269,31 @@ private void SaveOperatingEnvironment() } } + private void HandleProjectStarted(IBuildCheckManager buildCheckManager) + { + (ProjectStartedEventArgs args, ProjectLoggingContext ctx) = _nodeLoggingContext.CreateProjectLoggingContext(_requestEntry); + + _projectLoggingContext = ctx; + ConfigureWarningsAsErrorsAndMessages(); + ILoggingService loggingService = _projectLoggingContext?.LoggingService; + BuildEventContext projectBuildEventContext = _projectLoggingContext?.BuildEventContext; + + // We can set the warning as errors and messages only after the project logging context has been created (as it creates the new ProjectContextId) + if (buildCheckManager != null && loggingService != null && projectBuildEventContext != null) + { + args.WarningsAsErrors = loggingService.GetWarningsAsErrors(projectBuildEventContext).ToHashSet(StringComparer.OrdinalIgnoreCase); + args.WarningsAsMessages = loggingService.GetWarningsAsMessages(projectBuildEventContext).ToHashSet(StringComparer.OrdinalIgnoreCase); + args.WarningsNotAsErrors = loggingService.GetWarningsNotAsErrors(projectBuildEventContext).ToHashSet(StringComparer.OrdinalIgnoreCase); + } + + // We can log the event only after the warning as errors and messages have been set and added + loggingService?.LogProjectStarted(args); + + buildCheckManager?.StartProjectRequest( + new CheckLoggingContext(_nodeLoggingContext.LoggingService, _projectLoggingContext!.BuildEventContext), + _requestEntry.RequestConfiguration.ProjectFullPath); + } + /// /// Sets the operationg environment to the initial build environment. /// @@ -1409,7 +1430,7 @@ private void ConfigureKnownImmutableFolders() } } - private ISet ParseWarningCodes(string warnings) + private static ISet ParseWarningCodes(string warnings) { if (String.IsNullOrWhiteSpace(warnings)) { diff --git a/src/Build/BuildCheck/Infrastructure/BuildCheckBuildEventHandler.cs b/src/Build/BuildCheck/Infrastructure/BuildCheckBuildEventHandler.cs index 84cdd25ad6d..6e011090046 100644 --- a/src/Build/BuildCheck/Infrastructure/BuildCheckBuildEventHandler.cs +++ b/src/Build/BuildCheck/Infrastructure/BuildCheckBuildEventHandler.cs @@ -36,7 +36,7 @@ internal BuildCheckBuildEventHandler( { typeof(ProjectEvaluationFinishedEventArgs), (BuildEventArgs e) => HandleProjectEvaluationFinishedEvent((ProjectEvaluationFinishedEventArgs)e) }, { typeof(ProjectEvaluationStartedEventArgs), (BuildEventArgs e) => HandleProjectEvaluationStartedEvent((ProjectEvaluationStartedEventArgs)e) }, { typeof(EnvironmentVariableReadEventArgs), (BuildEventArgs e) => HandleEnvironmentVariableReadEvent((EnvironmentVariableReadEventArgs)e) }, - { typeof(ProjectStartedEventArgs), (BuildEventArgs e) => _buildCheckManager.StartProjectRequest(e.BuildEventContext!, ((ProjectStartedEventArgs)e).ProjectFile!) }, + { typeof(ProjectStartedEventArgs), (BuildEventArgs e) => HandleProjectStartedRequest((ProjectStartedEventArgs)e) }, { typeof(ProjectFinishedEventArgs), (BuildEventArgs e) => HandleProjectFinishedRequest((ProjectFinishedEventArgs)e) }, { typeof(BuildCheckTracingEventArgs), (BuildEventArgs e) => HandleBuildCheckTracingEvent((BuildCheckTracingEventArgs)e) }, { typeof(BuildCheckAcquisitionEventArgs), (BuildEventArgs e) => HandleBuildCheckAcquisitionEvent((BuildCheckAcquisitionEventArgs)e) }, @@ -98,6 +98,11 @@ private void HandleProjectEvaluationStartedEvent(ProjectEvaluationStartedEventAr } } + private void HandleProjectStartedRequest(ProjectStartedEventArgs eventArgs) + => _buildCheckManager.StartProjectRequest( + _checkContextFactory.CreateCheckContext(eventArgs.BuildEventContext!), + eventArgs!.ProjectFile!); + private void HandleProjectFinishedRequest(ProjectFinishedEventArgs eventArgs) => _buildCheckManager.EndProjectRequest( _checkContextFactory.CreateCheckContext(eventArgs.BuildEventContext!), diff --git a/src/Build/BuildCheck/Infrastructure/BuildCheckManagerProvider.cs b/src/Build/BuildCheck/Infrastructure/BuildCheckManagerProvider.cs index 7185030e520..13925ad6d5f 100644 --- a/src/Build/BuildCheck/Infrastructure/BuildCheckManagerProvider.cs +++ b/src/Build/BuildCheck/Infrastructure/BuildCheckManagerProvider.cs @@ -61,7 +61,7 @@ public void ShutdownComponent() _instance = null; } - internal sealed class BuildCheckManager : IBuildCheckManager, IBuildEngineDataRouter + internal sealed class BuildCheckManager : IBuildCheckManager, IBuildEngineDataRouter, IResultReporter { private readonly TracingReporter _tracingReporter = new TracingReporter(); private readonly IConfigurationProvider _configurationProvider = new ConfigurationProvider(); @@ -246,7 +246,7 @@ private void SetupSingleCheck(CheckFactoryContext checkFactoryContext, string pr ConfigurationContext configurationContext = ConfigurationContext.FromDataEnumeration(customConfigData, configurations); - wrapper = checkFactoryContext.Initialize(uninitializedCheck, configurationContext); + wrapper = checkFactoryContext.Initialize(uninitializedCheck, this, configurationContext); checkFactoryContext.MaterializedCheck = wrapper; Check check = wrapper.Check; @@ -528,10 +528,53 @@ public void EndProjectEvaluation(BuildEventContext buildEventContext) { } - public void StartProjectRequest(BuildEventContext buildEventContext, string projectFullPath) + public void StartProjectRequest(ICheckContext checkContext, string projectFullPath) { + BuildEventContext buildEventContext = checkContext.BuildEventContext; + // There can be multiple ProjectStarted-ProjectFinished per single configuration project build (each request for different target) _projectsByInstanceId[buildEventContext.ProjectInstanceId] = projectFullPath; + + if (_deferredEvalDiagnostics.TryGetValue(buildEventContext.EvaluationId, out var list)) + { + foreach (BuildEventArgs deferredArgs in list) + { + deferredArgs.BuildEventContext = deferredArgs.BuildEventContext!.WithInstanceIdAndContextId(buildEventContext); + checkContext.DispatchBuildEvent(deferredArgs); + } + list.Clear(); + _deferredEvalDiagnostics.Remove(buildEventContext.EvaluationId); + } + } + + private readonly Dictionary> _deferredEvalDiagnostics = new(); + void IResultReporter.ReportResult(BuildEventArgs eventArgs, ICheckContext checkContext) + { + // If we do not need to decide on promotability/demotability of warnings or we are ready to decide on those + // - we can just dispatch the event. + if ( + // no context - we cannot defer as we'd need eval id to queue it + eventArgs.BuildEventContext == null || + // no eval id - we cannot defer as we'd need eval id to queue it + eventArgs.BuildEventContext.EvaluationId == BuildEventContext.InvalidEvaluationId || + // instance id known - no need to defer + eventArgs.BuildEventContext.ProjectInstanceId != BuildEventContext.InvalidProjectInstanceId || + // it's not a warning - no need to defer + eventArgs is not BuildWarningEventArgs) + { + checkContext.DispatchBuildEvent(eventArgs); + return; + } + + // This is evaluation - so we need to defer it until we know the instance id and context id + + if (!_deferredEvalDiagnostics.TryGetValue(eventArgs.BuildEventContext.EvaluationId, out var list)) + { + list = []; + _deferredEvalDiagnostics[eventArgs.BuildEventContext.EvaluationId] = list; + } + + list.Add(eventArgs); } public void EndProjectRequest( @@ -589,7 +632,7 @@ public Check Factory() return ba; } - public CheckWrapper Initialize(Check ba, ConfigurationContext configContext) + public CheckWrapper Initialize(Check ba, IResultReporter resultReporter, ConfigurationContext configContext) { try { @@ -604,7 +647,7 @@ public CheckWrapper Initialize(Check ba, ConfigurationContext configContext) throw new BuildCheckConfigurationException( $"The Check '{ba.FriendlyName}' failed to initialize: {e.Message}", e); } - return new CheckWrapper(ba); + return new CheckWrapper(ba, resultReporter); } public CheckWrapper? MaterializedCheck { get; set; } @@ -617,3 +660,8 @@ public CheckWrapper Initialize(Check ba, ConfigurationContext configContext) } } } + +internal interface IResultReporter +{ + void ReportResult(BuildEventArgs result, ICheckContext checkContext); +} diff --git a/src/Build/BuildCheck/Infrastructure/CheckWrapper.cs b/src/Build/BuildCheck/Infrastructure/CheckWrapper.cs index 9568709534c..6d2fc868d95 100644 --- a/src/Build/BuildCheck/Infrastructure/CheckWrapper.cs +++ b/src/Build/BuildCheck/Infrastructure/CheckWrapper.cs @@ -39,9 +39,12 @@ internal sealed class CheckWrapper /// private readonly bool _limitReportsNumber = !Traits.Instance.EscapeHatches.DoNotLimitBuildCheckResultsNumber; - public CheckWrapper(Check check) + private readonly IResultReporter _resultReporter; + + public CheckWrapper(Check check, IResultReporter resultReporter) { Check = check; + _resultReporter = resultReporter; _ruleTelemetryData = new BuildCheckRuleTelemetryData[check.SupportedRules.Count]; InitializeTelemetryData(_ruleTelemetryData, check); @@ -157,7 +160,7 @@ internal void ReportResult(BuildCheckResult result, ICheckContext checkContext, _reportsCount++; BuildEventArgs eventArgs = result.ToEventArgs(config.Severity); eventArgs.BuildEventContext = checkContext.BuildEventContext; - checkContext.DispatchBuildEvent(eventArgs); + _resultReporter.ReportResult(eventArgs, checkContext); // Big amount of build check messages may lead to build hang. // See issue https://github.com/dotnet/msbuild/issues/10414 diff --git a/src/Build/BuildCheck/Infrastructure/IBuildCheckManager.cs b/src/Build/BuildCheck/Infrastructure/IBuildCheckManager.cs index 8f125db4551..552c49dac83 100644 --- a/src/Build/BuildCheck/Infrastructure/IBuildCheckManager.cs +++ b/src/Build/BuildCheck/Infrastructure/IBuildCheckManager.cs @@ -79,7 +79,7 @@ void ProcessTaskParameterEventArgs( void EndProjectEvaluation(BuildEventContext buildEventContext); - void StartProjectRequest(BuildEventContext buildEventContext, string projectFullPath); + void StartProjectRequest(ICheckContext checksContext, string projectFullPath); void EndProjectRequest(ICheckContext checksContext, string projectFullPath); diff --git a/src/Build/BuildCheck/Infrastructure/NullBuildCheckManager.cs b/src/Build/BuildCheck/Infrastructure/NullBuildCheckManager.cs index 5f6e5c19b66..294700ef5fc 100644 --- a/src/Build/BuildCheck/Infrastructure/NullBuildCheckManager.cs +++ b/src/Build/BuildCheck/Infrastructure/NullBuildCheckManager.cs @@ -71,7 +71,7 @@ public void EndProjectEvaluation(BuildEventContext buildEventContext) { } - public void StartProjectRequest(BuildEventContext buildEventContext, string projectFullPath) + public void StartProjectRequest(ICheckContext checksContext, string projectFullPath) { } diff --git a/src/BuildCheck.UnitTests/EndToEndTests.cs b/src/BuildCheck.UnitTests/EndToEndTests.cs index dbfc825831c..622609dc5f2 100644 --- a/src/BuildCheck.UnitTests/EndToEndTests.cs +++ b/src/BuildCheck.UnitTests/EndToEndTests.cs @@ -419,6 +419,41 @@ public void NoEnvironmentVariableProperty_Scoping(EvaluationCheckScope scope) } } + [Theory] + [InlineData(true, false)] + [InlineData(false, false)] + [InlineData(false, true)] + public void NoEnvironmentVariableProperty_DeferredProcessing(bool warnAsError, bool warnAsMessage) + { + PrepareSampleProjectsAndConfig( + buildInOutOfProcessNode: true, + out TransientTestFile projectFile, + new List<(string, string)>() { ("BC0103", "warning") }); + + string output = RunnerUtilities.ExecBootstrapedMSBuild( + $"{Path.GetFileName(projectFile.Path)} /m:1 -nr:False -restore -check" + + (warnAsError ? " /p:warn2err=BC0103" : "") + (warnAsMessage ? " /p:warn2msg=BC0103" : ""), out bool success, + false, _env.Output); + + success.ShouldBe(!warnAsError); + + if (warnAsMessage) + { + output.ShouldNotContain("warning BC0103"); + output.ShouldNotContain("error BC0103"); + } + else if (warnAsError) + { + output.ShouldNotContain("warning BC0103"); + output.ShouldContain("error BC0103"); + } + else + { + output.ShouldContain("warning BC0103"); + output.ShouldNotContain("error BC0103"); + } + } + [Theory] [InlineData("CheckCandidate", new[] { "CustomRule1", "CustomRule2" })] [InlineData("CheckCandidateWithMultipleChecksInjected", new[] { "CustomRule1", "CustomRule2", "CustomRule3" }, true)] diff --git a/src/BuildCheck.UnitTests/TestAssets/SampleCheckIntegrationTest/Project1.csproj b/src/BuildCheck.UnitTests/TestAssets/SampleCheckIntegrationTest/Project1.csproj index 81efb8fd795..67b42bf5bc9 100644 --- a/src/BuildCheck.UnitTests/TestAssets/SampleCheckIntegrationTest/Project1.csproj +++ b/src/BuildCheck.UnitTests/TestAssets/SampleCheckIntegrationTest/Project1.csproj @@ -15,6 +15,8 @@ $(TestFromEvaluation) + $(warn2err) + $(warn2msg) diff --git a/src/Framework/BuildCheck/BuildCheckEventArgs.cs b/src/Framework/BuildCheck/BuildCheckEventArgs.cs index b017a05d3fc..adff612cc95 100644 --- a/src/Framework/BuildCheck/BuildCheckEventArgs.cs +++ b/src/Framework/BuildCheck/BuildCheckEventArgs.cs @@ -102,22 +102,14 @@ internal override void CreateFromStream(BinaryReader reader, int version) DiagnosticSeverity defaultSeverity = (DiagnosticSeverity)reader.Read7BitEncodedInt(); int explicitSeveritiesCount = reader.Read7BitEncodedInt(); HashSet explicitSeverities = -#if NETSTANDARD2_0 - new HashSet(); -#else - new HashSet(explicitSeveritiesCount); -#endif + EnumerableExtensions.NewHashSet(explicitSeveritiesCount); for (int j = 0; j < explicitSeveritiesCount; j++) { explicitSeverities.Add((DiagnosticSeverity)reader.Read7BitEncodedInt()); } int projectNamesWhereEnabledCount = reader.Read7BitEncodedInt(); HashSet projectNamesWhereEnabled = -#if NETSTANDARD2_0 - new HashSet(); -#else - new HashSet(projectNamesWhereEnabledCount); -#endif + EnumerableExtensions.NewHashSet(projectNamesWhereEnabledCount); for (int j = 0; j < projectNamesWhereEnabledCount; j++) { projectNamesWhereEnabled.Add(reader.ReadString()); diff --git a/src/Framework/BuildCheck/EnumerableExtensions.cs b/src/Framework/BuildCheck/EnumerableExtensions.cs index 2ec4f958fca..d74136269d9 100644 --- a/src/Framework/BuildCheck/EnumerableExtensions.cs +++ b/src/Framework/BuildCheck/EnumerableExtensions.cs @@ -32,6 +32,36 @@ public static IEnumerable AsSingleItemEnumerable(this T item) yield return item; } + public static HashSet NewHashSet(int capacity) + => NewHashSet(capacity, null); + + public static HashSet NewHashSet(IEqualityComparer equalityComparer) + => NewHashSet(0, equalityComparer); + + public static HashSet NewHashSet(int capacity, IEqualityComparer? equalityComparer) + { +#if NETSTANDARD2_0 + return new HashSet(equalityComparer); +#else + return new HashSet(capacity, equalityComparer); +#endif + } + + public static HashSet? ToHashSet(this ICollection? source, IEqualityComparer? equalityComparer = null) + { + if (source is null) + { + return null; + } + + if (source is HashSet set) + { + return set; + } + + return new HashSet(source, equalityComparer); + } + #if !NET /// /// Returns a read-only wrapper diff --git a/src/Framework/BuildEventContext.cs b/src/Framework/BuildEventContext.cs index 0622bf45896..83a7a1f9330 100644 --- a/src/Framework/BuildEventContext.cs +++ b/src/Framework/BuildEventContext.cs @@ -115,6 +115,16 @@ public BuildEventContext( } #endregion + internal BuildEventContext WithInstanceIdAndContextId(int projectInstanceId, int projectContextId) + { + return new BuildEventContext(_submissionId, _nodeId, _evaluationId, projectInstanceId, projectContextId, + _targetId, _taskId); + } + + internal BuildEventContext WithInstanceIdAndContextId(BuildEventContext other) + { + return WithInstanceIdAndContextId(other.ProjectInstanceId, other.ProjectContextId); + } #region Properties diff --git a/src/Framework/ProjectStartedEventArgs.cs b/src/Framework/ProjectStartedEventArgs.cs index 4636850306a..8dcf4330fb0 100644 --- a/src/Framework/ProjectStartedEventArgs.cs +++ b/src/Framework/ProjectStartedEventArgs.cs @@ -8,6 +8,7 @@ using System.IO; using System.Linq; using System.Runtime.Serialization; +using Microsoft.Build.Experimental.BuildCheck; using Microsoft.Build.Shared; namespace Microsoft.Build.Framework @@ -333,6 +334,14 @@ public IEnumerable? Items } } + // Following 3 properties are intended only for internal transfer - to properly communicate the warn as error/msg + // from the worker node, to the main node - that may be producing the buildcheck diagnostics. + // They are not going to be in a binlog (at least not as of now). + + internal ISet? WarningsAsErrors { get; set; } + internal ISet? WarningsNotAsErrors { get; set; } + internal ISet? WarningsAsMessages { get; set; } + #region CustomSerializationToStream /// @@ -389,6 +398,10 @@ internal override void WriteToStream(BinaryWriter writer) writer.Write((string?)propertyPair.Value ?? ""); } } + + WriteCollection(writer, WarningsAsErrors); + WriteCollection(writer, WarningsNotAsErrors); + WriteCollection(writer, WarningsAsMessages); } /// @@ -457,7 +470,48 @@ internal override void CreateFromStream(BinaryReader reader, int version) properties = dictionaryList; } + + WarningsAsErrors = ReadStringSet(reader); + WarningsNotAsErrors = ReadStringSet(reader); + WarningsAsMessages = ReadStringSet(reader); } + + private static void WriteCollection(BinaryWriter writer, ICollection? collection) + { + if (collection == null) + { + writer.Write((byte)0); + } + else + { + writer.Write((byte)1); + writer.Write(collection.Count); + foreach (string item in collection) + { + writer.Write(item); + } + } + } + + private static ISet? ReadStringSet(BinaryReader reader) + { + if (reader.ReadByte() == 0) + { + return null; + } + else + { + int count = reader.ReadInt32(); + HashSet set = EnumerableExtensions.NewHashSet(count, StringComparer.OrdinalIgnoreCase); + for (int i = 0; i < count; i++) + { + set.Add(reader.ReadString()); + } + + return set; + } + } + #endregion #region SerializationSection