From 5b33d8fe1855b718eecd899f461e8c09965fe881 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mariana=20Dematt=C3=A9?= Date: Thu, 21 Mar 2024 15:28:36 +0100 Subject: [PATCH 01/14] First pass on disabling BuildCheck on restore --- .../RequestBuilder/RequestBuilder.cs | 54 +++++++++++++------ 1 file changed, 39 insertions(+), 15 deletions(-) diff --git a/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs b/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs index 5d4938c7b7b..9006ff672fa 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs @@ -1118,10 +1118,21 @@ private void SetProjectCurrentDirectory() /// private async Task BuildProject() { + bool isRestore = false; + var propertyEntry = _requestEntry.RequestConfiguration.GlobalProperties[MSBuildConstants.MSBuildIsRestoring]; + if (propertyEntry != null) + { + isRestore = Convert.ToBoolean(propertyEntry.EvaluatedValue); + } + // We consider this the entrypoint for the project build for purposes of BuildCheck processing + IBuildCheckManager buildCheckManager = null; - var buildCheckManager = (_componentHost.GetComponent(BuildComponentType.BuildCheck) as IBuildCheckManagerProvider)!.Instance; - buildCheckManager.SetDataSource(BuildCheckDataSource.BuildExecution); + if (!isRestore) + { + buildCheckManager = (_componentHost.GetComponent(BuildComponentType.BuildCheck) as IBuildCheckManagerProvider)!.Instance; + buildCheckManager.SetDataSource(BuildCheckDataSource.BuildExecution); + } ErrorUtilities.VerifyThrow(_targetBuilder != null, "Target builder is null"); @@ -1137,10 +1148,13 @@ private async Task BuildProject() // Load the project if (!_requestEntry.RequestConfiguration.IsLoaded) { - buildCheckManager.StartProjectEvaluation( - BuildCheckDataSource.BuildExecution, - _requestEntry.Request.ParentBuildEventContext, - _requestEntry.RequestConfiguration.ProjectFullPath); + if (!isRestore) + { + buildCheckManager.StartProjectEvaluation( + BuildCheckDataSource.BuildExecution, + _requestEntry.Request.ParentBuildEventContext, + _requestEntry.RequestConfiguration.ProjectFullPath); + } _requestEntry.RequestConfiguration.LoadProjectIntoConfiguration( _componentHost, @@ -1162,15 +1176,22 @@ private async Task BuildProject() } finally { - buildCheckManager.EndProjectEvaluation( - BuildCheckDataSource.BuildExecution, - _requestEntry.Request.ParentBuildEventContext); + if (!isRestore) + { + buildCheckManager.EndProjectEvaluation( + BuildCheckDataSource.BuildExecution, + _requestEntry.Request.ParentBuildEventContext); + } } _projectLoggingContext = _nodeLoggingContext.LogProjectStarted(_requestEntry); - buildCheckManager.StartProjectRequest( - BuildCheckDataSource.BuildExecution, - _requestEntry.Request.ParentBuildEventContext); + + if (!isRestore) + { + buildCheckManager.StartProjectRequest( + BuildCheckDataSource.BuildExecution, + _requestEntry.Request.ParentBuildEventContext); + } // Now that the project has started, parse a few known properties which indicate warning codes to treat as errors or messages // @@ -1223,9 +1244,12 @@ private async Task BuildProject() MSBuildEventSource.Log.BuildProjectStop(_requestEntry.RequestConfiguration.ProjectFullPath, string.Join(", ", allTargets)); } - buildCheckManager.EndProjectRequest( - BuildCheckDataSource.BuildExecution, - _requestEntry.Request.ParentBuildEventContext); + if (!isRestore) + { + buildCheckManager.EndProjectRequest( + BuildCheckDataSource.BuildExecution, + _requestEntry.Request.ParentBuildEventContext); + } return result; From 916d82279ba8086abd27bd490be27f72bf85ff99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mariana=20Dematt=C3=A9?= Date: Fri, 22 Mar 2024 13:20:35 +0100 Subject: [PATCH 02/14] BuildCheck doesn't run on restore --- .../RequestBuilder/RequestBuilder.cs | 1 + .../BuildCheckConnectorLogger.cs | 79 ++++++++++--------- .../BuildCheckManagerProvider.cs | 2 + .../Infrastructure/IBuildCheckManager.cs | 2 + .../Infrastructure/NullBuildCheckManager.cs | 2 + 5 files changed, 48 insertions(+), 38 deletions(-) diff --git a/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs b/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs index 9006ff672fa..ab53a1f9a78 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs @@ -1131,6 +1131,7 @@ private async Task BuildProject() if (!isRestore) { buildCheckManager = (_componentHost.GetComponent(BuildComponentType.BuildCheck) as IBuildCheckManagerProvider)!.Instance; + buildCheckManager.isRestore = false; buildCheckManager.SetDataSource(BuildCheckDataSource.BuildExecution); } diff --git a/src/Build/BuildCheck/Infrastructure/BuildCheckConnectorLogger.cs b/src/Build/BuildCheck/Infrastructure/BuildCheckConnectorLogger.cs index b860423748e..5a538f41745 100644 --- a/src/Build/BuildCheck/Infrastructure/BuildCheckConnectorLogger.cs +++ b/src/Build/BuildCheck/Infrastructure/BuildCheckConnectorLogger.cs @@ -26,55 +26,58 @@ public void Initialize(IEventSource eventSource) private void EventSource_AnyEventRaised(object sender, BuildEventArgs e) { - if (e is ProjectEvaluationFinishedEventArgs projectEvaluationFinishedEventArgs) + if (!buildCheckManager.isRestore) { - if (projectEvaluationFinishedEventArgs.ProjectFile?.EndsWith(".metaproj") ?? false) + if (e is ProjectEvaluationFinishedEventArgs projectEvaluationFinishedEventArgs) { - return; - } + if (projectEvaluationFinishedEventArgs.ProjectFile?.EndsWith(".metaproj") ?? false) + { + return; + } - try - { - buildCheckManager.ProcessEvaluationFinishedEventArgs( - loggingContextFactory.CreateLoggingContext(e.BuildEventContext!), - projectEvaluationFinishedEventArgs); + try + { + buildCheckManager.ProcessEvaluationFinishedEventArgs( + loggingContextFactory.CreateLoggingContext(e.BuildEventContext!), + projectEvaluationFinishedEventArgs); + } + catch (Exception exception) + { + Debugger.Launch(); + Console.WriteLine(exception); + throw; + } + + buildCheckManager.EndProjectEvaluation(BuildCheckDataSource.EventArgs, e.BuildEventContext!); } - catch (Exception exception) + else if (e is ProjectEvaluationStartedEventArgs projectEvaluationStartedEventArgs) { - Debugger.Launch(); - Console.WriteLine(exception); - throw; - } + if (projectEvaluationStartedEventArgs.ProjectFile?.EndsWith(".metaproj") ?? false) + { + return; + } - buildCheckManager.EndProjectEvaluation(BuildCheckDataSource.EventArgs, e.BuildEventContext!); - } - else if (e is ProjectEvaluationStartedEventArgs projectEvaluationStartedEventArgs) - { - if (projectEvaluationStartedEventArgs.ProjectFile?.EndsWith(".metaproj") ?? false) + buildCheckManager.StartProjectEvaluation(BuildCheckDataSource.EventArgs, e.BuildEventContext!, + projectEvaluationStartedEventArgs.ProjectFile!); + } + else if (e is ProjectStartedEventArgs projectStartedEvent) { - return; + buildCheckManager.StartProjectRequest(BuildCheckDataSource.EventArgs, e.BuildEventContext!); } - - buildCheckManager.StartProjectEvaluation(BuildCheckDataSource.EventArgs, e.BuildEventContext!, - projectEvaluationStartedEventArgs.ProjectFile!); - } - else if (e is ProjectStartedEventArgs projectStartedEvent) - { - buildCheckManager.StartProjectRequest(BuildCheckDataSource.EventArgs, e.BuildEventContext!); - } - else if (e is ProjectFinishedEventArgs projectFinishedEventArgs) - { - buildCheckManager.EndProjectRequest(BuildCheckDataSource.EventArgs, e.BuildEventContext!); - } - else if (e is BuildCheckEventArgs buildCheckBuildEventArgs) - { - if (buildCheckBuildEventArgs is BuildCheckTracingEventArgs tracingEventArgs) + else if (e is ProjectFinishedEventArgs projectFinishedEventArgs) { - _stats.Merge(tracingEventArgs.TracingData, (span1, span2) => span1 + span2); + buildCheckManager.EndProjectRequest(BuildCheckDataSource.EventArgs, e.BuildEventContext!); } - else if (buildCheckBuildEventArgs is BuildCheckAcquisitionEventArgs acquisitionEventArgs) + else if (e is BuildCheckEventArgs buildCheckBuildEventArgs) { - buildCheckManager.ProcessAnalyzerAcquisition(acquisitionEventArgs.ToAnalyzerAcquisitionData()); + if (buildCheckBuildEventArgs is BuildCheckTracingEventArgs tracingEventArgs) + { + _stats.Merge(tracingEventArgs.TracingData, (span1, span2) => span1 + span2); + } + else if (buildCheckBuildEventArgs is BuildCheckAcquisitionEventArgs acquisitionEventArgs) + { + buildCheckManager.ProcessAnalyzerAcquisition(acquisitionEventArgs.ToAnalyzerAcquisitionData()); + } } } } diff --git a/src/Build/BuildCheck/Infrastructure/BuildCheckManagerProvider.cs b/src/Build/BuildCheck/Infrastructure/BuildCheckManagerProvider.cs index df5385b08ba..e73aef2dbd0 100644 --- a/src/Build/BuildCheck/Infrastructure/BuildCheckManagerProvider.cs +++ b/src/Build/BuildCheck/Infrastructure/BuildCheckManagerProvider.cs @@ -81,6 +81,8 @@ private sealed class BuildCheckManager : IBuildCheckManager private bool IsInProcNode => _enabledDataSources[(int)BuildCheckDataSource.EventArgs] && _enabledDataSources[(int)BuildCheckDataSource.BuildExecution]; + bool IBuildCheckManager.isRestore { get; set; } = true; + /// /// Notifies the manager that the data source will be used - /// so it should register the built-in analyzers for the source if it hasn't been done yet. diff --git a/src/Build/BuildCheck/Infrastructure/IBuildCheckManager.cs b/src/Build/BuildCheck/Infrastructure/IBuildCheckManager.cs index 703d0b6bfa9..e3a6a7b8b70 100644 --- a/src/Build/BuildCheck/Infrastructure/IBuildCheckManager.cs +++ b/src/Build/BuildCheck/Infrastructure/IBuildCheckManager.cs @@ -27,6 +27,8 @@ internal enum BuildCheckDataSource /// internal interface IBuildCheckManager { + bool isRestore { get; set; } + void ProcessEvaluationFinishedEventArgs( IBuildAnalysisLoggingContext buildAnalysisContext, ProjectEvaluationFinishedEventArgs projectEvaluationFinishedEventArgs); diff --git a/src/Build/BuildCheck/Infrastructure/NullBuildCheckManager.cs b/src/Build/BuildCheck/Infrastructure/NullBuildCheckManager.cs index d6685345652..fa5c0334d66 100644 --- a/src/Build/BuildCheck/Infrastructure/NullBuildCheckManager.cs +++ b/src/Build/BuildCheck/Infrastructure/NullBuildCheckManager.cs @@ -15,6 +15,8 @@ namespace Microsoft.Build.BuildCheck.Infrastructure; internal class NullBuildCheckManager : IBuildCheckManager { + public bool isRestore { get => throw new NotImplementedException(); set => throw new NotImplementedException(); } + public void Shutdown() { } public void ProcessEvaluationFinishedEventArgs(IBuildAnalysisLoggingContext buildAnalysisContext, From 8363efc971758137ce851340785da92d609d4d8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mariana=20Dematt=C3=A9?= Date: Tue, 9 Apr 2024 20:09:30 +0200 Subject: [PATCH 03/14] Addressed PR comments --- .../RequestBuilder/RequestBuilder.cs | 11 +-- .../BuildCheckConnectorLogger.cs | 84 ++++++++++--------- 2 files changed, 46 insertions(+), 49 deletions(-) diff --git a/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs b/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs index ab53a1f9a78..f0619f743ec 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs @@ -1126,14 +1126,8 @@ private async Task BuildProject() } // We consider this the entrypoint for the project build for purposes of BuildCheck processing - IBuildCheckManager buildCheckManager = null; - - if (!isRestore) - { - buildCheckManager = (_componentHost.GetComponent(BuildComponentType.BuildCheck) as IBuildCheckManagerProvider)!.Instance; - buildCheckManager.isRestore = false; - buildCheckManager.SetDataSource(BuildCheckDataSource.BuildExecution); - } + IBuildCheckManager buildCheckManager = isRestore ? null : (_componentHost.GetComponent(BuildComponentType.BuildCheck) as IBuildCheckManagerProvider)!.Instance; + buildCheckManager?.SetDataSource(BuildCheckDataSource.BuildExecution); ErrorUtilities.VerifyThrow(_targetBuilder != null, "Target builder is null"); @@ -1183,6 +1177,7 @@ private async Task BuildProject() BuildCheckDataSource.BuildExecution, _requestEntry.Request.ParentBuildEventContext); } + } _projectLoggingContext = _nodeLoggingContext.LogProjectStarted(_requestEntry); diff --git a/src/Build/BuildCheck/Infrastructure/BuildCheckConnectorLogger.cs b/src/Build/BuildCheck/Infrastructure/BuildCheckConnectorLogger.cs index 5a538f41745..0b6b50102f8 100644 --- a/src/Build/BuildCheck/Infrastructure/BuildCheckConnectorLogger.cs +++ b/src/Build/BuildCheck/Infrastructure/BuildCheckConnectorLogger.cs @@ -26,58 +26,60 @@ public void Initialize(IEventSource eventSource) private void EventSource_AnyEventRaised(object sender, BuildEventArgs e) { - if (!buildCheckManager.isRestore) + if (buildCheckManager.isRestore) { - if (e is ProjectEvaluationFinishedEventArgs projectEvaluationFinishedEventArgs) - { - if (projectEvaluationFinishedEventArgs.ProjectFile?.EndsWith(".metaproj") ?? false) - { - return; - } + return; + } - try - { - buildCheckManager.ProcessEvaluationFinishedEventArgs( - loggingContextFactory.CreateLoggingContext(e.BuildEventContext!), - projectEvaluationFinishedEventArgs); - } - catch (Exception exception) - { - Debugger.Launch(); - Console.WriteLine(exception); - throw; - } + if (e is ProjectEvaluationFinishedEventArgs projectEvaluationFinishedEventArgs) + { + if (projectEvaluationFinishedEventArgs.ProjectFile?.EndsWith(".metaproj") ?? false) + { + return; + } - buildCheckManager.EndProjectEvaluation(BuildCheckDataSource.EventArgs, e.BuildEventContext!); + try + { + buildCheckManager.ProcessEvaluationFinishedEventArgs( + loggingContextFactory.CreateLoggingContext(e.BuildEventContext!), + projectEvaluationFinishedEventArgs); } - else if (e is ProjectEvaluationStartedEventArgs projectEvaluationStartedEventArgs) + catch (Exception exception) { - if (projectEvaluationStartedEventArgs.ProjectFile?.EndsWith(".metaproj") ?? false) - { - return; - } - - buildCheckManager.StartProjectEvaluation(BuildCheckDataSource.EventArgs, e.BuildEventContext!, - projectEvaluationStartedEventArgs.ProjectFile!); + Debugger.Launch(); + Console.WriteLine(exception); + throw; } - else if (e is ProjectStartedEventArgs projectStartedEvent) + + buildCheckManager.EndProjectEvaluation(BuildCheckDataSource.EventArgs, e.BuildEventContext!); + } + else if (e is ProjectEvaluationStartedEventArgs projectEvaluationStartedEventArgs) + { + if (projectEvaluationStartedEventArgs.ProjectFile?.EndsWith(".metaproj") ?? false) { - buildCheckManager.StartProjectRequest(BuildCheckDataSource.EventArgs, e.BuildEventContext!); + return; } - else if (e is ProjectFinishedEventArgs projectFinishedEventArgs) + + buildCheckManager.StartProjectEvaluation(BuildCheckDataSource.EventArgs, e.BuildEventContext!, + projectEvaluationStartedEventArgs.ProjectFile!); + } + else if (e is ProjectStartedEventArgs projectStartedEvent) + { + buildCheckManager.StartProjectRequest(BuildCheckDataSource.EventArgs, e.BuildEventContext!); + } + else if (e is ProjectFinishedEventArgs projectFinishedEventArgs) + { + buildCheckManager.EndProjectRequest(BuildCheckDataSource.EventArgs, e.BuildEventContext!); + } + else if (e is BuildCheckEventArgs buildCheckBuildEventArgs) + { + if (buildCheckBuildEventArgs is BuildCheckTracingEventArgs tracingEventArgs) { - buildCheckManager.EndProjectRequest(BuildCheckDataSource.EventArgs, e.BuildEventContext!); + _stats.Merge(tracingEventArgs.TracingData, (span1, span2) => span1 + span2); } - else if (e is BuildCheckEventArgs buildCheckBuildEventArgs) + else if (buildCheckBuildEventArgs is BuildCheckAcquisitionEventArgs acquisitionEventArgs) { - if (buildCheckBuildEventArgs is BuildCheckTracingEventArgs tracingEventArgs) - { - _stats.Merge(tracingEventArgs.TracingData, (span1, span2) => span1 + span2); - } - else if (buildCheckBuildEventArgs is BuildCheckAcquisitionEventArgs acquisitionEventArgs) - { - buildCheckManager.ProcessAnalyzerAcquisition(acquisitionEventArgs.ToAnalyzerAcquisitionData()); - } + buildCheckManager.ProcessAnalyzerAcquisition(acquisitionEventArgs.ToAnalyzerAcquisitionData()); } } } From 7d223bc79dcb033f27e400597af7a890ee727624 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mariana=20Dematt=C3=A9?= Date: Mon, 15 Apr 2024 17:55:54 +0200 Subject: [PATCH 04/14] Modified how logger registers isRestore --- src/Analyzers.UnitTests/EndToEndTests.cs | 85 +++++++++++++++++++ .../BackEnd/MockLoggingService.cs | 2 +- .../Logging/EvaluationLoggingContext.cs | 5 +- .../Components/Logging/ILoggingService.cs | 3 +- .../Logging/LoggingServiceLogMethods.cs | 5 +- .../RequestBuilder/RequestBuilder.cs | 51 ++++------- .../BuildCheckConnectorLogger.cs | 35 ++++++-- src/Build/Evaluation/Evaluator.cs | 3 +- .../ProjectEvaluationStartedEventArgs.cs | 13 +++ src/UnitTests.Shared/RunnerUtilities.cs | 4 +- 10 files changed, 154 insertions(+), 52 deletions(-) diff --git a/src/Analyzers.UnitTests/EndToEndTests.cs b/src/Analyzers.UnitTests/EndToEndTests.cs index dc6bce0563b..33729fa1719 100644 --- a/src/Analyzers.UnitTests/EndToEndTests.cs +++ b/src/Analyzers.UnitTests/EndToEndTests.cs @@ -124,5 +124,90 @@ public void SampleAnalyzerIntegrationTest(bool buildInOutOfProcessNode) // The conflicting outputs warning appears output.ShouldContain("BC0101"); } + + [Fact] + public void skipRestorePhase() + { + string contents = $""" + + + + Exe + net8.0 + enable + enable + + + + Test + + + + + + + + + + + + """; + + string contents2 = $""" + + + + Exe + net8.0 + enable + enable + + + + Test + + + + + + + + + + + + """; + + TransientTestFolder workFolder = _env.CreateFolder(createFolder: true); + TransientTestFile projectFile = _env.CreateFile(workFolder, "FooBar.csproj", contents); + TransientTestFile projectFile2 = _env.CreateFile(workFolder, "FooBar-Copy.csproj", contents2); + + TransientTestFile config = _env.CreateFile(workFolder, "editorconfig.json", + /*lang=json,strict*/ + """ + { + "BC0101": { + "IsEnabled": true, + "Severity": "Error" + }, + "COND0543": { + "IsEnabled": false, + "Severity": "Error", + "EvaluationAnalysisScope": "AnalyzedProjectOnly", + "CustomSwitch": "QWERTY" + }, + "BLA": { + "IsEnabled": false + } + } + """); + + _env.SetCurrentDirectory(Path.GetDirectoryName(projectFile.Path)); + _env.SetEnvironmentVariable("MSBUILDDEBUGONSTART", "1"); + + string output = RunnerUtilities.ExecBootstrapedMSBuild($"{Path.GetFileName(projectFile.Path)} /m:1 -nr:False -restore -analyze", out bool success); + _env.Output.WriteLine(output); + success.ShouldBeTrue(); + } } } diff --git a/src/Build.UnitTests/BackEnd/MockLoggingService.cs b/src/Build.UnitTests/BackEnd/MockLoggingService.cs index b3ceffe4bd5..296546e06be 100644 --- a/src/Build.UnitTests/BackEnd/MockLoggingService.cs +++ b/src/Build.UnitTests/BackEnd/MockLoggingService.cs @@ -496,7 +496,7 @@ public BuildEventContext CreateProjectCacheBuildEventContext(int submissionId, i => new BuildEventContext(0, 0, 0, 0, 0, 0, 0); /// - public void LogProjectEvaluationStarted(BuildEventContext eventContext, string projectFile) + public void LogProjectEvaluationStarted(BuildEventContext eventContext, string projectFile, bool isRestore) { } diff --git a/src/Build/BackEnd/Components/Logging/EvaluationLoggingContext.cs b/src/Build/BackEnd/Components/Logging/EvaluationLoggingContext.cs index 31223745d8b..de476ec3c11 100644 --- a/src/Build/BackEnd/Components/Logging/EvaluationLoggingContext.cs +++ b/src/Build/BackEnd/Components/Logging/EvaluationLoggingContext.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections; +using System.Collections.Generic; using Microsoft.Build.BackEnd.Logging; using Microsoft.Build.Framework; using Microsoft.Build.Framework.Profiler; @@ -27,9 +28,9 @@ public EvaluationLoggingContext(ILoggingService loggingService, BuildEventContex IsValid = true; } - public void LogProjectEvaluationStarted() + public void LogProjectEvaluationStarted(bool isRestore) { - LoggingService.LogProjectEvaluationStarted(BuildEventContext, _projectFile); + LoggingService.LogProjectEvaluationStarted(BuildEventContext, _projectFile, isRestore); } /// diff --git a/src/Build/BackEnd/Components/Logging/ILoggingService.cs b/src/Build/BackEnd/Components/Logging/ILoggingService.cs index 6d4973bc223..d8b8d8ae0af 100644 --- a/src/Build/BackEnd/Components/Logging/ILoggingService.cs +++ b/src/Build/BackEnd/Components/Logging/ILoggingService.cs @@ -485,8 +485,9 @@ MessageImportance MinimumRequiredMessageImportance /// /// The event context to use for logging /// Project file being built + /// Something for now /// The evaluation event context for the project. - void LogProjectEvaluationStarted(BuildEventContext eventContext, string projectFile); + void LogProjectEvaluationStarted(BuildEventContext eventContext, string projectFile, bool isRestore); /// /// Logs that a project evaluation has finished diff --git a/src/Build/BackEnd/Components/Logging/LoggingServiceLogMethods.cs b/src/Build/BackEnd/Components/Logging/LoggingServiceLogMethods.cs index 9ef9a58f17b..4ebd7e54bc6 100644 --- a/src/Build/BackEnd/Components/Logging/LoggingServiceLogMethods.cs +++ b/src/Build/BackEnd/Components/Logging/LoggingServiceLogMethods.cs @@ -444,14 +444,15 @@ public BuildEventContext CreateProjectCacheBuildEventContext( } /// - public void LogProjectEvaluationStarted(BuildEventContext projectEvaluationEventContext, string projectFile) + public void LogProjectEvaluationStarted(BuildEventContext projectEvaluationEventContext, string projectFile, bool isRestore) { ProjectEvaluationStartedEventArgs evaluationEvent = new ProjectEvaluationStartedEventArgs(ResourceUtilities.GetResourceString("EvaluationStarted"), projectFile) { BuildEventContext = projectEvaluationEventContext, - ProjectFile = projectFile + ProjectFile = projectFile, + IsRestore = isRestore }; ProcessLoggingEvent(evaluationEvent); diff --git a/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs b/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs index f0619f743ec..d569309a32f 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs @@ -1118,15 +1118,9 @@ private void SetProjectCurrentDirectory() /// private async Task BuildProject() { - bool isRestore = false; - var propertyEntry = _requestEntry.RequestConfiguration.GlobalProperties[MSBuildConstants.MSBuildIsRestoring]; - if (propertyEntry != null) - { - isRestore = Convert.ToBoolean(propertyEntry.EvaluatedValue); - } - // We consider this the entrypoint for the project build for purposes of BuildCheck processing - IBuildCheckManager buildCheckManager = isRestore ? null : (_componentHost.GetComponent(BuildComponentType.BuildCheck) as IBuildCheckManagerProvider)!.Instance; + var propertyEntry = _requestEntry.RequestConfiguration.GlobalProperties[MSBuildConstants.MSBuildIsRestoring]; + IBuildCheckManager buildCheckManager = propertyEntry is not null ? null : (_componentHost.GetComponent(BuildComponentType.BuildCheck) as IBuildCheckManagerProvider)!.Instance; buildCheckManager?.SetDataSource(BuildCheckDataSource.BuildExecution); ErrorUtilities.VerifyThrow(_targetBuilder != null, "Target builder is null"); @@ -1143,13 +1137,12 @@ private async Task BuildProject() // Load the project if (!_requestEntry.RequestConfiguration.IsLoaded) { - if (!isRestore) - { - buildCheckManager.StartProjectEvaluation( - BuildCheckDataSource.BuildExecution, - _requestEntry.Request.ParentBuildEventContext, - _requestEntry.RequestConfiguration.ProjectFullPath); - } + + buildCheckManager?.StartProjectEvaluation( + BuildCheckDataSource.BuildExecution, + _requestEntry.Request.ParentBuildEventContext, + _requestEntry.RequestConfiguration.ProjectFullPath); + _requestEntry.RequestConfiguration.LoadProjectIntoConfiguration( _componentHost, @@ -1171,23 +1164,16 @@ private async Task BuildProject() } finally { - if (!isRestore) - { - buildCheckManager.EndProjectEvaluation( - BuildCheckDataSource.BuildExecution, - _requestEntry.Request.ParentBuildEventContext); - } - + buildCheckManager?.EndProjectEvaluation( + BuildCheckDataSource.BuildExecution, + _requestEntry.Request.ParentBuildEventContext); } _projectLoggingContext = _nodeLoggingContext.LogProjectStarted(_requestEntry); - if (!isRestore) - { - buildCheckManager.StartProjectRequest( - BuildCheckDataSource.BuildExecution, - _requestEntry.Request.ParentBuildEventContext); - } + buildCheckManager?.StartProjectRequest( + BuildCheckDataSource.BuildExecution, + _requestEntry.Request.ParentBuildEventContext); // Now that the project has started, parse a few known properties which indicate warning codes to treat as errors or messages // @@ -1240,12 +1226,9 @@ private async Task BuildProject() MSBuildEventSource.Log.BuildProjectStop(_requestEntry.RequestConfiguration.ProjectFullPath, string.Join(", ", allTargets)); } - if (!isRestore) - { - buildCheckManager.EndProjectRequest( - BuildCheckDataSource.BuildExecution, - _requestEntry.Request.ParentBuildEventContext); - } + buildCheckManager?.EndProjectRequest( + BuildCheckDataSource.BuildExecution, + _requestEntry.Request.ParentBuildEventContext); return result; diff --git a/src/Build/BuildCheck/Infrastructure/BuildCheckConnectorLogger.cs b/src/Build/BuildCheck/Infrastructure/BuildCheckConnectorLogger.cs index 0b6b50102f8..8b246822403 100644 --- a/src/Build/BuildCheck/Infrastructure/BuildCheckConnectorLogger.cs +++ b/src/Build/BuildCheck/Infrastructure/BuildCheckConnectorLogger.cs @@ -10,6 +10,7 @@ using Microsoft.Build.BuildCheck.Logging; using Microsoft.Build.Experimental.BuildCheck; using Microsoft.Build.Framework; +using Microsoft.Build.Shared; namespace Microsoft.Build.BuildCheck.Infrastructure; internal sealed class BuildCheckConnectorLogger(IBuildAnalysisLoggingContextFactory loggingContextFactory, IBuildCheckManager buildCheckManager) @@ -18,6 +19,8 @@ internal sealed class BuildCheckConnectorLogger(IBuildAnalysisLoggingContextFact public LoggerVerbosity Verbosity { get; set; } public string? Parameters { get; set; } + private bool isRestore = false; + public void Initialize(IEventSource eventSource) { eventSource.AnyEventRaised += EventSource_AnyEventRaised; @@ -26,12 +29,23 @@ public void Initialize(IEventSource eventSource) private void EventSource_AnyEventRaised(object sender, BuildEventArgs e) { - if (buildCheckManager.isRestore) + // NOTE: this event is fired more than one time per project build + if (e is ProjectFinishedEventArgs projectFinishedEventArgs) + { + if (isRestore) + { + isRestore = false; + } + else + { + buildCheckManager.EndProjectRequest(BuildCheckDataSource.EventArgs, e.BuildEventContext!); + } + } + else if (isRestore) { return; } - - if (e is ProjectEvaluationFinishedEventArgs projectEvaluationFinishedEventArgs) + else if (e is ProjectEvaluationFinishedEventArgs projectEvaluationFinishedEventArgs) { if (projectEvaluationFinishedEventArgs.ProjectFile?.EndsWith(".metaproj") ?? false) { @@ -60,17 +74,20 @@ private void EventSource_AnyEventRaised(object sender, BuildEventArgs e) return; } - buildCheckManager.StartProjectEvaluation(BuildCheckDataSource.EventArgs, e.BuildEventContext!, - projectEvaluationStartedEventArgs.ProjectFile!); + if (!projectEvaluationStartedEventArgs.IsRestore) + { + buildCheckManager.StartProjectEvaluation(BuildCheckDataSource.EventArgs, e.BuildEventContext!, + projectEvaluationStartedEventArgs.ProjectFile!); + } + else + { + isRestore = true; + } } else if (e is ProjectStartedEventArgs projectStartedEvent) { buildCheckManager.StartProjectRequest(BuildCheckDataSource.EventArgs, e.BuildEventContext!); } - else if (e is ProjectFinishedEventArgs projectFinishedEventArgs) - { - buildCheckManager.EndProjectRequest(BuildCheckDataSource.EventArgs, e.BuildEventContext!); - } else if (e is BuildCheckEventArgs buildCheckBuildEventArgs) { if (buildCheckBuildEventArgs is BuildCheckTracingEventArgs tracingEventArgs) diff --git a/src/Build/Evaluation/Evaluator.cs b/src/Build/Evaluation/Evaluator.cs index 780d58db6b1..69fcdc0de5d 100644 --- a/src/Build/Evaluation/Evaluator.cs +++ b/src/Build/Evaluation/Evaluator.cs @@ -5,6 +5,7 @@ using System.Collections; using System.Collections.Generic; using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.Globalization; using System.IO; using System.Linq; @@ -626,7 +627,7 @@ private void Evaluate() } } - _evaluationLoggingContext.LogProjectEvaluationStarted(); + _evaluationLoggingContext.LogProjectEvaluationStarted(_data.GlobalPropertiesDictionary[MSBuildConstants.MSBuildIsRestoring] is not null); ErrorUtilities.VerifyThrow(_data.EvaluationId != BuildEventContext.InvalidEvaluationId, "Evaluation should produce an evaluation ID"); diff --git a/src/Framework/ProjectEvaluationStartedEventArgs.cs b/src/Framework/ProjectEvaluationStartedEventArgs.cs index 6d231fe1428..f183083b93a 100644 --- a/src/Framework/ProjectEvaluationStartedEventArgs.cs +++ b/src/Framework/ProjectEvaluationStartedEventArgs.cs @@ -2,6 +2,8 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Collections.Generic; +using System.Net.NetworkInformation; namespace Microsoft.Build.Framework { @@ -26,9 +28,20 @@ public ProjectEvaluationStartedEventArgs(string? message, params object[]? messa { } + public ProjectEvaluationStartedEventArgs(bool isRestore, string? message, params object[]? messageArgs) + : base(message, helpKeyword: null, senderName: null, DateTime.UtcNow, messageArgs) + { + IsRestore = isRestore; + } + /// /// Gets or sets the full path of the project that started evaluation. /// public string? ProjectFile { get; set; } + + /// + /// Gets the set of global properties to be used to evaluate this project. + /// + public bool IsRestore { get; internal set; } } } diff --git a/src/UnitTests.Shared/RunnerUtilities.cs b/src/UnitTests.Shared/RunnerUtilities.cs index 373692d37f5..3605a1247c1 100644 --- a/src/UnitTests.Shared/RunnerUtilities.cs +++ b/src/UnitTests.Shared/RunnerUtilities.cs @@ -157,8 +157,8 @@ public static string RunProcessAndGetOutput(string process, string parameters, o { // Let's not create a unit test for which we need more than 30 sec to execute. // Please consider carefully if you would like to increase the timeout. - p.KillTree(1000); - throw new TimeoutException($"Test failed due to timeout: process {p.Id} is active for more than 30 sec."); + // p.KillTree(1000); + // throw new TimeoutException($"Test failed due to timeout: process {p.Id} is active for more than 30 sec."); } // We need the WaitForExit call without parameters because our processing of output/error streams is not synchronous. From 08b0cd1e7d4173d717205652f8ae1aa6ce80218c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mariana=20Dematt=C3=A9?= Date: Tue, 16 Apr 2024 18:34:01 +0200 Subject: [PATCH 05/14] Fixed merge errors --- eng/cibuild_bootstrapped_msbuild.ps1 | 3 - eng/cibuild_bootstrapped_msbuild.sh | 3 - src/Analyzers.UnitTests/AssemblyInfo.cs | 4 - src/Analyzers.UnitTests/EndToEndTests.cs | 213 ------------------ ...Microsoft.Build.Analyzers.UnitTests.csproj | 74 ------ .../RequestBuilder/RequestBuilder.cs | 7 +- src/Build/BuildCheck/API/BuildAnalyzerRule.cs | 5 - .../BuildCheckConnectorLogger.cs | 4 - .../Infrastructure/BuildCheckContext.cs | 15 -- .../BuildCheckManagerProvider.cs | 3 - .../Infrastructure/BuildEventsProcessor.cs | 2 - .../Infrastructure/NullBuildCheckManager.cs | 2 - src/Build/Evaluation/Evaluator.cs | 3 +- src/BuildCheck.UnitTests/AssemblyInfo.cs | 10 - .../ProjectEvaluationStartedEventArgs.cs | 12 +- .../MSBuild.Bootstrap.csproj | 2 - 16 files changed, 3 insertions(+), 359 deletions(-) delete mode 100644 src/Analyzers.UnitTests/AssemblyInfo.cs delete mode 100644 src/Analyzers.UnitTests/EndToEndTests.cs delete mode 100644 src/Analyzers.UnitTests/Microsoft.Build.Analyzers.UnitTests.csproj diff --git a/eng/cibuild_bootstrapped_msbuild.ps1 b/eng/cibuild_bootstrapped_msbuild.ps1 index a5c0b8d7f10..b6e3c089135 100644 --- a/eng/cibuild_bootstrapped_msbuild.ps1 +++ b/eng/cibuild_bootstrapped_msbuild.ps1 @@ -113,9 +113,6 @@ try { # Opt into performance logging. https://github.com/dotnet/msbuild/issues/5900 $env:DOTNET_PERFLOG_DIR=$PerfLogDir - # Expose stage 1 path so unit tests can find the bootstrapped MSBuild. - $env:MSBUILD_BOOTSTRAPPED_BINDIR=$Stage1BinDir - # When using bootstrapped MSBuild: # - Turn off node reuse (so that bootstrapped MSBuild processes don't stay running and lock files) # - Create bootstrap environment as it's required when also running tests diff --git a/eng/cibuild_bootstrapped_msbuild.sh b/eng/cibuild_bootstrapped_msbuild.sh index 4165de68eba..8edd377ec73 100755 --- a/eng/cibuild_bootstrapped_msbuild.sh +++ b/eng/cibuild_bootstrapped_msbuild.sh @@ -71,9 +71,6 @@ mv $ArtifactsDir $Stage1Dir # Ensure that debug bits fail fast, rather than hanging waiting for a debugger attach. export MSBUILDDONOTLAUNCHDEBUGGER=true -# Expose stage 1 path so unit tests can find the bootstrapped MSBuild. -export MSBUILD_BOOTSTRAPPED_BINDIR="$Stage1Dir/bin" - # Opt into performance logging. export DOTNET_PERFLOG_DIR=$PerfLogDir diff --git a/src/Analyzers.UnitTests/AssemblyInfo.cs b/src/Analyzers.UnitTests/AssemblyInfo.cs deleted file mode 100644 index 3b5d7bbb185..00000000000 --- a/src/Analyzers.UnitTests/AssemblyInfo.cs +++ /dev/null @@ -1,4 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -global using NativeMethodsShared = Microsoft.Build.Framework.NativeMethods; diff --git a/src/Analyzers.UnitTests/EndToEndTests.cs b/src/Analyzers.UnitTests/EndToEndTests.cs deleted file mode 100644 index 33729fa1719..00000000000 --- a/src/Analyzers.UnitTests/EndToEndTests.cs +++ /dev/null @@ -1,213 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System; -using System.Collections.Generic; -using System.IO; -using System.Linq; -using System.Reflection; -using System.Text; -using System.Threading.Tasks; -using Microsoft.Build.UnitTests; -using Microsoft.Build.UnitTests.Shared; -using Shouldly; -using Xunit; -using Xunit.Abstractions; - -namespace Microsoft.Build.Analyzers.UnitTests -{ - public class EndToEndTests : IDisposable - { - private readonly TestEnvironment _env; - public EndToEndTests(ITestOutputHelper output) - { - _env = TestEnvironment.Create(output); - - // this is needed to ensure the binary logger does not pollute the environment - _env.WithEnvironmentInvariant(); - } - - public void Dispose() => _env.Dispose(); - - [Theory] - [InlineData(true)] - [InlineData(false)] - public void SampleAnalyzerIntegrationTest(bool buildInOutOfProcessNode) - { - string contents = $""" - - - - Exe - net8.0 - enable - enable - - - - Test - - - - - - - - - - - - """; - - string contents2 = $""" - - - - Exe - net8.0 - enable - enable - - - - Test - - - - - - - - - - - - """; - TransientTestFolder workFolder = _env.CreateFolder(createFolder: true); - TransientTestFile projectFile = _env.CreateFile(workFolder, "FooBar.csproj", contents); - TransientTestFile projectFile2 = _env.CreateFile(workFolder, "FooBar-Copy.csproj", contents2); - - // var cache = new SimpleProjectRootElementCache(); - // ProjectRootElement xml = ProjectRootElement.OpenProjectOrSolution(projectFile.Path, /*unused*/null, /*unused*/null, cache, false /*Not explicitly loaded - unused*/); - - - TransientTestFile config = _env.CreateFile(workFolder, "editorconfig.json", - /*lang=json,strict*/ - """ - { - "BC0101": { - "IsEnabled": true, - "Severity": "Error" - }, - "COND0543": { - "IsEnabled": false, - "Severity": "Error", - "EvaluationAnalysisScope": "AnalyzedProjectOnly", - "CustomSwitch": "QWERTY" - }, - "BLA": { - "IsEnabled": false - } - } - """); - - // OSX links /var into /private, which makes Path.GetTempPath() return "/var..." but Directory.GetCurrentDirectory return "/private/var...". - // This discrepancy breaks path equality checks in analyzers if we pass to MSBuild full path to the initial project. - // TODO: See if there is a way of fixing it in the engine. - _env.SetCurrentDirectory(Path.GetDirectoryName(projectFile.Path)); - - _env.SetEnvironmentVariable("MSBUILDNOINPROCNODE", buildInOutOfProcessNode ? "1" : "0"); - _env.SetEnvironmentVariable("MSBUILDLOGPROPERTIESANDITEMSAFTEREVALUATION", "1"); - string output = RunnerUtilities.ExecBootstrapedMSBuild($"{Path.GetFileName(projectFile.Path)} /m:1 -nr:False -restore -analyze", out bool success); - _env.Output.WriteLine(output); - success.ShouldBeTrue(); - // The conflicting outputs warning appears - output.ShouldContain("BC0101"); - } - - [Fact] - public void skipRestorePhase() - { - string contents = $""" - - - - Exe - net8.0 - enable - enable - - - - Test - - - - - - - - - - - - """; - - string contents2 = $""" - - - - Exe - net8.0 - enable - enable - - - - Test - - - - - - - - - - - - """; - - TransientTestFolder workFolder = _env.CreateFolder(createFolder: true); - TransientTestFile projectFile = _env.CreateFile(workFolder, "FooBar.csproj", contents); - TransientTestFile projectFile2 = _env.CreateFile(workFolder, "FooBar-Copy.csproj", contents2); - - TransientTestFile config = _env.CreateFile(workFolder, "editorconfig.json", - /*lang=json,strict*/ - """ - { - "BC0101": { - "IsEnabled": true, - "Severity": "Error" - }, - "COND0543": { - "IsEnabled": false, - "Severity": "Error", - "EvaluationAnalysisScope": "AnalyzedProjectOnly", - "CustomSwitch": "QWERTY" - }, - "BLA": { - "IsEnabled": false - } - } - """); - - _env.SetCurrentDirectory(Path.GetDirectoryName(projectFile.Path)); - _env.SetEnvironmentVariable("MSBUILDDEBUGONSTART", "1"); - - string output = RunnerUtilities.ExecBootstrapedMSBuild($"{Path.GetFileName(projectFile.Path)} /m:1 -nr:False -restore -analyze", out bool success); - _env.Output.WriteLine(output); - success.ShouldBeTrue(); - } - } -} diff --git a/src/Analyzers.UnitTests/Microsoft.Build.Analyzers.UnitTests.csproj b/src/Analyzers.UnitTests/Microsoft.Build.Analyzers.UnitTests.csproj deleted file mode 100644 index 1768410a565..00000000000 --- a/src/Analyzers.UnitTests/Microsoft.Build.Analyzers.UnitTests.csproj +++ /dev/null @@ -1,74 +0,0 @@ - - - - - - - - $(LatestDotNetCoreForMSBuild) - $(FullFrameworkTFM);$(TargetFrameworks) - - $(RuntimeOutputPlatformTarget) - false - True - - - - - - - - - - - - - - - - - - - - - Shared\FileUtilities.cs - - - Shared\TempFileUtilities.cs - - - Shared\ErrorUtilities.cs - - - Shared\EscapingUtilities.cs - - - Shared\BuildEnvironmentHelper.cs - - - Shared\ProcessExtensions.cs - - - Shared\ResourceUtilities.cs - - - Shared\ExceptionHandling.cs - - - Shared\FileUtilitiesRegex.cs - - - Shared\AssemblyResources.cs - - - - - - App.config - Designer - - - PreserveNewest - - - diff --git a/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs b/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs index 4e7f1847c75..8c031d8d3af 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs @@ -1104,16 +1104,11 @@ private async Task BuildProject() { // We consider this the entrypoint for the project build for purposes of BuildCheck processing var propertyEntry = _requestEntry.RequestConfiguration.GlobalProperties[MSBuildConstants.MSBuildIsRestoring]; - IBuildCheckManager buildCheckManager = propertyEntry is not null ? null : (_componentHost.GetComponent(BuildComponentType.BuildCheck) as IBuildCheckManagerProvider)!.Instance; + IBuildCheckManager buildCheckManager = propertyEntry is not null ? null : (_componentHost.GetComponent(BuildComponentType.BuildCheckManagerProvider) as IBuildCheckManagerProvider)!.Instance; buildCheckManager?.SetDataSource(BuildCheckDataSource.BuildExecution); ErrorUtilities.VerifyThrow(_targetBuilder != null, "Target builder is null"); - // We consider this the entrypoint for the project build for purposes of BuildCheck processing - - var buildCheckManager = (_componentHost.GetComponent(BuildComponentType.BuildCheckManagerProvider) as IBuildCheckManagerProvider)!.Instance; - buildCheckManager.SetDataSource(BuildCheckDataSource.BuildExecution); - // Make sure it is null before loading the configuration into the request, because if there is a problem // we do not wand to have an invalid projectLoggingContext floating around. Also if this is null the error will be // logged with the node logging context diff --git a/src/Build/BuildCheck/API/BuildAnalyzerRule.cs b/src/Build/BuildCheck/API/BuildAnalyzerRule.cs index e906c2660aa..8b43dad4999 100644 --- a/src/Build/BuildCheck/API/BuildAnalyzerRule.cs +++ b/src/Build/BuildCheck/API/BuildAnalyzerRule.cs @@ -42,11 +42,6 @@ public BuildAnalyzerRule(string id, string title, string description, string mes /// public string Description { get; } - /// - /// TODO: We might turn this into enum, or just remove this. - /// - public string Category { get; } - /// /// Message format that will be used by the actual reports () - those will just supply the actual arguments. /// diff --git a/src/Build/BuildCheck/Infrastructure/BuildCheckConnectorLogger.cs b/src/Build/BuildCheck/Infrastructure/BuildCheckConnectorLogger.cs index decfe5d6adb..48122f75f58 100644 --- a/src/Build/BuildCheck/Infrastructure/BuildCheckConnectorLogger.cs +++ b/src/Build/BuildCheck/Infrastructure/BuildCheckConnectorLogger.cs @@ -89,10 +89,6 @@ private void EventSource_AnyEventRaised(object sender, BuildEventArgs e) { buildCheckManager.StartProjectRequest(BuildCheckDataSource.EventArgs, e.BuildEventContext!); } - else if (e is ProjectFinishedEventArgs projectFinishedEventArgs) - { - buildCheckManager.EndProjectRequest(BuildCheckDataSource.EventArgs, e.BuildEventContext!); - } else if (e is BuildCheckEventArgs buildCheckBuildEventArgs) { if (buildCheckBuildEventArgs is BuildCheckTracingEventArgs tracingEventArgs) diff --git a/src/Build/BuildCheck/Infrastructure/BuildCheckContext.cs b/src/Build/BuildCheck/Infrastructure/BuildCheckContext.cs index 3546a6ab7b8..19a4e3d6967 100644 --- a/src/Build/BuildCheck/Infrastructure/BuildCheckContext.cs +++ b/src/Build/BuildCheck/Infrastructure/BuildCheckContext.cs @@ -9,28 +9,13 @@ namespace Microsoft.Build.BuildCheck.Infrastructure; internal sealed class BuildCheckRegistrationContext(BuildAnalyzerWrapper analyzerWrapper, BuildCheckCentralContext buildCheckCentralContext) : IBuildCheckRegistrationContext { - private int _evaluatedPropertiesActionCount; - private int _parsedItemsActionCount; - public void RegisterEvaluatedPropertiesAction(Action> evaluatedPropertiesAction) { - if (Interlocked.Increment(ref _evaluatedPropertiesActionCount) > 1) - { - throw new BuildCheckConfigurationException( - $"Analyzer '{analyzerWrapper.BuildAnalyzer.FriendlyName}' attempted to call '{nameof(RegisterEvaluatedPropertiesAction)}' multiple times."); - } - buildCheckCentralContext.RegisterEvaluatedPropertiesAction(analyzerWrapper, evaluatedPropertiesAction); } public void RegisterParsedItemsAction(Action> parsedItemsAction) { - if (Interlocked.Increment(ref _parsedItemsActionCount) > 1) - { - throw new BuildCheckConfigurationException( - $"Analyzer '{analyzerWrapper.BuildAnalyzer.FriendlyName}' attempted to call '{nameof(RegisterParsedItemsAction)}' multiple times."); - } - buildCheckCentralContext.RegisterParsedItemsAction(analyzerWrapper, parsedItemsAction); } } diff --git a/src/Build/BuildCheck/Infrastructure/BuildCheckManagerProvider.cs b/src/Build/BuildCheck/Infrastructure/BuildCheckManagerProvider.cs index 7763293dd21..2cc02231114 100644 --- a/src/Build/BuildCheck/Infrastructure/BuildCheckManagerProvider.cs +++ b/src/Build/BuildCheck/Infrastructure/BuildCheckManagerProvider.cs @@ -82,8 +82,6 @@ private sealed class BuildCheckManager : IBuildCheckManager private bool IsInProcNode => _enabledDataSources[(int)BuildCheckDataSource.EventArgs] && _enabledDataSources[(int)BuildCheckDataSource.BuildExecution]; - bool IBuildCheckManager.isRestore { get; set; } = true; - /// /// Notifies the manager that the data source will be used - /// so it should register the built-in analyzers for the source if it hasn't been done yet. @@ -275,7 +273,6 @@ private void SetupAnalyzersForNewProject(string projectFullPath, BuildEventConte _loggingService.LogErrorFromText(buildEventContext, null, null, null, new BuildEventFileInfo(projectFullPath), e.Message); - _loggingService.LogCommentFromText(buildEventContext, MessageImportance.High, $"Dismounting analyzer '{analyzerFactoryContext.FriendlyName}'"); analyzersToRemove.Add(analyzerFactoryContext); } } diff --git a/src/Build/BuildCheck/Infrastructure/BuildEventsProcessor.cs b/src/Build/BuildCheck/Infrastructure/BuildEventsProcessor.cs index c3a3d1c68e8..9514f0a7ca0 100644 --- a/src/Build/BuildCheck/Infrastructure/BuildEventsProcessor.cs +++ b/src/Build/BuildCheck/Infrastructure/BuildEventsProcessor.cs @@ -33,8 +33,6 @@ internal void ProcessEvaluationFinishedEventArgs( AnalyzerLoggingContext buildAnalysisContext, ProjectEvaluationFinishedEventArgs evaluationFinishedEventArgs) { - LoggingContext loggingContext = buildAnalysisContext.ToLoggingContext(); - Dictionary propertiesLookup = new Dictionary(); Internal.Utilities.EnumerateProperties(evaluationFinishedEventArgs.Properties, propertiesLookup, static (dict, kvp) => dict.Add(kvp.Key, kvp.Value)); diff --git a/src/Build/BuildCheck/Infrastructure/NullBuildCheckManager.cs b/src/Build/BuildCheck/Infrastructure/NullBuildCheckManager.cs index 56f5c7bb8f8..00ed2266d09 100644 --- a/src/Build/BuildCheck/Infrastructure/NullBuildCheckManager.cs +++ b/src/Build/BuildCheck/Infrastructure/NullBuildCheckManager.cs @@ -16,8 +16,6 @@ namespace Microsoft.Build.BuildCheck.Infrastructure; internal class NullBuildCheckManager : IBuildCheckManager { - public bool isRestore { get => throw new NotImplementedException(); set => throw new NotImplementedException(); } - public void Shutdown() { } public void ProcessEvaluationFinishedEventArgs(AnalyzerLoggingContext buildAnalysisContext, diff --git a/src/Build/Evaluation/Evaluator.cs b/src/Build/Evaluation/Evaluator.cs index 69fcdc0de5d..a7a4629381c 100644 --- a/src/Build/Evaluation/Evaluator.cs +++ b/src/Build/Evaluation/Evaluator.cs @@ -5,7 +5,6 @@ using System.Collections; using System.Collections.Generic; using System.Diagnostics; -using System.Diagnostics.CodeAnalysis; using System.Globalization; using System.IO; using System.Linq; @@ -627,7 +626,7 @@ private void Evaluate() } } - _evaluationLoggingContext.LogProjectEvaluationStarted(_data.GlobalPropertiesDictionary[MSBuildConstants.MSBuildIsRestoring] is not null); + _evaluationLoggingContext.LogProjectEvaluationStarted(_data.GlobalPropertiesDictionary[MSBuildConstants.MSBuildIsRestoring] is not null); ; ErrorUtilities.VerifyThrow(_data.EvaluationId != BuildEventContext.InvalidEvaluationId, "Evaluation should produce an evaluation ID"); diff --git a/src/BuildCheck.UnitTests/AssemblyInfo.cs b/src/BuildCheck.UnitTests/AssemblyInfo.cs index 5b383e24105..3b5d7bbb185 100644 --- a/src/BuildCheck.UnitTests/AssemblyInfo.cs +++ b/src/BuildCheck.UnitTests/AssemblyInfo.cs @@ -2,13 +2,3 @@ // The .NET Foundation licenses this file to you under the MIT license. global using NativeMethodsShared = Microsoft.Build.Framework.NativeMethods; - -namespace Microsoft.Build.UnitTests.Shared; - -[System.AttributeUsage(System.AttributeTargets.Assembly)] -internal sealed class BootstrapLocationAttribute(string bootstrapRoot, string bootstrapMsbuildBinaryLocation) - : System.Attribute -{ - public string BootstrapRoot { get; } = bootstrapRoot; - public string BootstrapMsbuildBinaryLocation { get; } = bootstrapMsbuildBinaryLocation; -} diff --git a/src/Framework/ProjectEvaluationStartedEventArgs.cs b/src/Framework/ProjectEvaluationStartedEventArgs.cs index f183083b93a..b6c2f098d59 100644 --- a/src/Framework/ProjectEvaluationStartedEventArgs.cs +++ b/src/Framework/ProjectEvaluationStartedEventArgs.cs @@ -2,8 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; -using System.Collections.Generic; -using System.Net.NetworkInformation; namespace Microsoft.Build.Framework { @@ -28,20 +26,12 @@ public ProjectEvaluationStartedEventArgs(string? message, params object[]? messa { } - public ProjectEvaluationStartedEventArgs(bool isRestore, string? message, params object[]? messageArgs) - : base(message, helpKeyword: null, senderName: null, DateTime.UtcNow, messageArgs) - { - IsRestore = isRestore; - } - /// /// Gets or sets the full path of the project that started evaluation. /// public string? ProjectFile { get; set; } - /// - /// Gets the set of global properties to be used to evaluate this project. - /// + public bool IsRestore { get; internal set; } } } diff --git a/src/MSBuild.Bootstrap/MSBuild.Bootstrap.csproj b/src/MSBuild.Bootstrap/MSBuild.Bootstrap.csproj index d1a614d9805..8a2a558e452 100644 --- a/src/MSBuild.Bootstrap/MSBuild.Bootstrap.csproj +++ b/src/MSBuild.Bootstrap/MSBuild.Bootstrap.csproj @@ -2,8 +2,6 @@ - - $(RuntimeOutputTargetFrameworks) From 288adff49d18ca8524b410efb81a13da1b7014e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mariana=20Dematt=C3=A9?= Date: Tue, 16 Apr 2024 18:40:14 +0200 Subject: [PATCH 06/14] Clean merge v2 --- src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs b/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs index 8c031d8d3af..fb1fbcd50d0 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs @@ -1102,13 +1102,13 @@ private void SetProjectCurrentDirectory() /// 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 var propertyEntry = _requestEntry.RequestConfiguration.GlobalProperties[MSBuildConstants.MSBuildIsRestoring]; IBuildCheckManager buildCheckManager = propertyEntry is not null ? null : (_componentHost.GetComponent(BuildComponentType.BuildCheckManagerProvider) as IBuildCheckManagerProvider)!.Instance; buildCheckManager?.SetDataSource(BuildCheckDataSource.BuildExecution); - ErrorUtilities.VerifyThrow(_targetBuilder != null, "Target builder is null"); - // Make sure it is null before loading the configuration into the request, because if there is a problem // we do not wand to have an invalid projectLoggingContext floating around. Also if this is null the error will be // logged with the node logging context From e3ea5ba9183e5db6d633a1e7e6fa1dce6387fb2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mariana=20Dematt=C3=A9?= Date: Thu, 18 Apr 2024 18:47:31 +0200 Subject: [PATCH 07/14] Updated existing tests --- eng/BootStrapMSBuild.props | 21 -------- eng/BootStrapMsBuild.props | 21 -------- .../BuildCheckConnectorLogger.cs | 49 ++++++++++--------- src/BuildCheck.UnitTests/EndToEndTests.cs | 9 ++-- 4 files changed, 31 insertions(+), 69 deletions(-) delete mode 100644 eng/BootStrapMSBuild.props delete mode 100644 eng/BootStrapMsBuild.props diff --git a/eng/BootStrapMSBuild.props b/eng/BootStrapMSBuild.props deleted file mode 100644 index e70bcb3489d..00000000000 --- a/eng/BootStrapMSBuild.props +++ /dev/null @@ -1,21 +0,0 @@ - - - - - - $(ArtifactsBinDir)bootstrap\ - $(BootstrapDestination)$(Platform)\ - $(BootstrapDestination)$(TargetFramework.ToLowerInvariant())\MSBuild\ - - - - $(BootstrapDestination)$(TargetMSBuildToolsVersion)\Bin - - - - $(BootstrapDestination) - - diff --git a/eng/BootStrapMsBuild.props b/eng/BootStrapMsBuild.props deleted file mode 100644 index 858cf76ac54..00000000000 --- a/eng/BootStrapMsBuild.props +++ /dev/null @@ -1,21 +0,0 @@ - - - - - - $(ArtifactsBinDir)bootstrap\ - $(BootstrapDestination)$(Platform)\ - $(BootstrapDestination)$(TargetFramework.ToLowerInvariant())\MSBuild\ - - - - $(BootstrapDestination)$(TargetMSBuildToolsVersion)\Bin - - - - $(BootstrapDestination) - - diff --git a/src/Build/BuildCheck/Infrastructure/BuildCheckConnectorLogger.cs b/src/Build/BuildCheck/Infrastructure/BuildCheckConnectorLogger.cs index 48122f75f58..e414db7ece0 100644 --- a/src/Build/BuildCheck/Infrastructure/BuildCheckConnectorLogger.cs +++ b/src/Build/BuildCheck/Infrastructure/BuildCheckConnectorLogger.cs @@ -25,11 +25,16 @@ public void Initialize(IEventSource eventSource) { eventSource.AnyEventRaised += EventSource_AnyEventRaised; eventSource.BuildFinished += EventSource_BuildFinished; + + + if (eventSource is IEventSource4 eventSource4) + { + eventSource4.IncludeEvaluationPropertiesAndItems(); + } } private void EventSource_AnyEventRaised(object sender, BuildEventArgs e) { - // NOTE: this event is fired more than one time per project build if (e is ProjectFinishedEventArgs projectFinishedEventArgs) { if (isRestore) @@ -45,6 +50,24 @@ private void EventSource_AnyEventRaised(object sender, BuildEventArgs e) { return; } + else if (e is ProjectEvaluationStartedEventArgs projectEvaluationStartedEventArgs) + { + // Skip autogenerated transient projects (as those are not user projects to be analyzed) + if (projectEvaluationStartedEventArgs.ProjectFile?.EndsWith(".metaproj") ?? false) + { + return; + } + + if (!projectEvaluationStartedEventArgs.IsRestore) + { + buildCheckManager.StartProjectEvaluation(BuildCheckDataSource.EventArgs, e.BuildEventContext!, + projectEvaluationStartedEventArgs.ProjectFile!); + } + else + { + isRestore = true; + } + } else if (e is ProjectEvaluationFinishedEventArgs projectEvaluationFinishedEventArgs) { if (projectEvaluationFinishedEventArgs.ProjectFile?.EndsWith(".metaproj") ?? false) @@ -54,9 +77,9 @@ private void EventSource_AnyEventRaised(object sender, BuildEventArgs e) try { - buildCheckManager.ProcessEvaluationFinishedEventArgs( - loggingContextFactory.CreateLoggingContext(e.BuildEventContext!), - projectEvaluationFinishedEventArgs); + buildCheckManager.ProcessEvaluationFinishedEventArgs( + loggingContextFactory.CreateLoggingContext(e.BuildEventContext!), + projectEvaluationFinishedEventArgs); } catch (Exception exception) { @@ -67,24 +90,6 @@ private void EventSource_AnyEventRaised(object sender, BuildEventArgs e) buildCheckManager.EndProjectEvaluation(BuildCheckDataSource.EventArgs, e.BuildEventContext!); } - else if (e is ProjectEvaluationStartedEventArgs projectEvaluationStartedEventArgs) - { - // Skip autogenerated transient projects (as those are not user projects to be analyzed) - if (projectEvaluationStartedEventArgs.ProjectFile?.EndsWith(".metaproj") ?? false) - { - return; - } - - if (!projectEvaluationStartedEventArgs.IsRestore) - { - buildCheckManager.StartProjectEvaluation(BuildCheckDataSource.EventArgs, e.BuildEventContext!, - projectEvaluationStartedEventArgs.ProjectFile!); - } - else - { - isRestore = true; - } - } else if (e is ProjectStartedEventArgs projectStartedEvent) { buildCheckManager.StartProjectRequest(BuildCheckDataSource.EventArgs, e.BuildEventContext!); diff --git a/src/BuildCheck.UnitTests/EndToEndTests.cs b/src/BuildCheck.UnitTests/EndToEndTests.cs index a0007d2c103..2a93cc62419 100644 --- a/src/BuildCheck.UnitTests/EndToEndTests.cs +++ b/src/BuildCheck.UnitTests/EndToEndTests.cs @@ -36,7 +36,7 @@ public EndToEndTests(ITestOutputHelper output) public void SampleAnalyzerIntegrationTest(bool buildInOutOfProcessNode, bool analysisRequested) { string contents = $""" - + Exe @@ -52,10 +52,6 @@ public void SampleAnalyzerIntegrationTest(bool buildInOutOfProcessNode, bool ana - - - - """; @@ -84,9 +80,12 @@ public void SampleAnalyzerIntegrationTest(bool buildInOutOfProcessNode, bool ana """; + + string content3 = "Console.WriteLine(\"Hello, World!\");\r\n"; TransientTestFolder workFolder = _env.CreateFolder(createFolder: true); TransientTestFile projectFile = _env.CreateFile(workFolder, "FooBar.csproj", contents); TransientTestFile projectFile2 = _env.CreateFile(workFolder, "FooBar-Copy.csproj", contents2); + TransientTestFile projectFile3 = _env.CreateFile(workFolder, "Program.cs", content3); // var cache = new SimpleProjectRootElementCache(); // ProjectRootElement xml = ProjectRootElement.OpenProjectOrSolution(projectFile.Path, /*unused*/null, /*unused*/null, cache, false /*Not explicitly loaded - unused*/); From f19e200e8eaa46134986c01258b84eb06e771f01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mariana=20Dematt=C3=A9?= Date: Thu, 18 Apr 2024 19:17:40 +0200 Subject: [PATCH 08/14] Added test --- eng/BootStrapMsBuild.props | 21 ++++++ src/BuildCheck.UnitTests/EndToEndTests.cs | 82 ++++++++++++++--------- 2 files changed, 71 insertions(+), 32 deletions(-) create mode 100644 eng/BootStrapMsBuild.props diff --git a/eng/BootStrapMsBuild.props b/eng/BootStrapMsBuild.props new file mode 100644 index 00000000000..858cf76ac54 --- /dev/null +++ b/eng/BootStrapMsBuild.props @@ -0,0 +1,21 @@ + + + + + + $(ArtifactsBinDir)bootstrap\ + $(BootstrapDestination)$(Platform)\ + $(BootstrapDestination)$(TargetFramework.ToLowerInvariant())\MSBuild\ + + + + $(BootstrapDestination)$(TargetMSBuildToolsVersion)\Bin + + + + $(BootstrapDestination) + + diff --git a/src/BuildCheck.UnitTests/EndToEndTests.cs b/src/BuildCheck.UnitTests/EndToEndTests.cs index 2a93cc62419..4e947ea3222 100644 --- a/src/BuildCheck.UnitTests/EndToEndTests.cs +++ b/src/BuildCheck.UnitTests/EndToEndTests.cs @@ -35,7 +35,40 @@ public EndToEndTests(ITestOutputHelper output) [InlineData(false, false)] public void SampleAnalyzerIntegrationTest(bool buildInOutOfProcessNode, bool analysisRequested) { - string contents = $""" + TransientTestFile projectFile = SetupTestFiles(); + _env.SetEnvironmentVariable("MSBUILDNOINPROCNODE", buildInOutOfProcessNode ? "1" : "0"); + _env.SetEnvironmentVariable("MSBUILDLOGPROPERTIESANDITEMSAFTEREVALUATION", "1"); + string output = RunnerUtilities.ExecBootstrapedMSBuild( + $"{Path.GetFileName(projectFile.Path)} /m:1 -nr:False -restore" + + (analysisRequested ? " -analyze" : string.Empty), out bool success); + _env.Output.WriteLine(output); + success.ShouldBeTrue(); + // The conflicting outputs warning appears - but only if analysis was requested + if (analysisRequested) + { + output.ShouldContain("BC0101"); + } + else + { + output.ShouldNotContain("BC0101"); + } + } + + [Fact] + public void NoRunOnRestore() + { + TransientTestFile projectFile = SetupTestFiles(); + string output = RunnerUtilities.ExecBootstrapedMSBuild( + $"{Path.GetFileName(projectFile.Path)} /m:1 -nr:False -analyze -t:restore", out bool success); + _env.Output.WriteLine(output); + success.ShouldBeTrue(); + output.ShouldNotContain("BC0101"); + } + + private TransientTestFile SetupTestFiles() + { + { + string contents = $""" @@ -56,7 +89,7 @@ public void SampleAnalyzerIntegrationTest(bool buildInOutOfProcessNode, bool ana """; - string contents2 = $""" + string contents2 = $""" @@ -81,19 +114,19 @@ public void SampleAnalyzerIntegrationTest(bool buildInOutOfProcessNode, bool ana """; - string content3 = "Console.WriteLine(\"Hello, World!\");\r\n"; - TransientTestFolder workFolder = _env.CreateFolder(createFolder: true); - TransientTestFile projectFile = _env.CreateFile(workFolder, "FooBar.csproj", contents); - TransientTestFile projectFile2 = _env.CreateFile(workFolder, "FooBar-Copy.csproj", contents2); - TransientTestFile projectFile3 = _env.CreateFile(workFolder, "Program.cs", content3); + string content3 = "Console.WriteLine(\"Hello, World!\");\r\n"; + TransientTestFolder workFolder = _env.CreateFolder(createFolder: true); + TransientTestFile projectFile = _env.CreateFile(workFolder, "FooBar.csproj", contents); + TransientTestFile projectFile2 = _env.CreateFile(workFolder, "FooBar-Copy.csproj", contents2); + TransientTestFile projectFile3 = _env.CreateFile(workFolder, "Program.cs", content3); - // var cache = new SimpleProjectRootElementCache(); - // ProjectRootElement xml = ProjectRootElement.OpenProjectOrSolution(projectFile.Path, /*unused*/null, /*unused*/null, cache, false /*Not explicitly loaded - unused*/); + // var cache = new SimpleProjectRootElementCache(); + // ProjectRootElement xml = ProjectRootElement.OpenProjectOrSolution(projectFile.Path, /*unused*/null, /*unused*/null, cache, false /*Not explicitly loaded - unused*/); - TransientTestFile config = _env.CreateFile(workFolder, "editorconfig.json", - /*lang=json,strict*/ - """ + TransientTestFile config = _env.CreateFile(workFolder, "editorconfig.json", + /*lang=json,strict*/ + """ { "BC0101": { "IsEnabled": true, @@ -111,26 +144,11 @@ public void SampleAnalyzerIntegrationTest(bool buildInOutOfProcessNode, bool ana } """); - // OSX links /var into /private, which makes Path.GetTempPath() return "/var..." but Directory.GetCurrentDirectory return "/private/var...". - // This discrepancy breaks path equality checks in analyzers if we pass to MSBuild full path to the initial project. - // See if there is a way of fixing it in the engine - tracked: https://github.com/orgs/dotnet/projects/373/views/1?pane=issue&itemId=55702688. - _env.SetCurrentDirectory(Path.GetDirectoryName(projectFile.Path)); - - _env.SetEnvironmentVariable("MSBUILDNOINPROCNODE", buildInOutOfProcessNode ? "1" : "0"); - _env.SetEnvironmentVariable("MSBUILDLOGPROPERTIESANDITEMSAFTEREVALUATION", "1"); - string output = RunnerUtilities.ExecBootstrapedMSBuild( - $"{Path.GetFileName(projectFile.Path)} /m:1 -nr:False -restore" + - (analysisRequested ? " -analyze" : string.Empty), out bool success); - _env.Output.WriteLine(output); - success.ShouldBeTrue(); - // The conflicting outputs warning appears - but only if analysis was requested - if (analysisRequested) - { - output.ShouldContain("BC0101"); - } - else - { - output.ShouldNotContain("BC0101"); + // OSX links /var into /private, which makes Path.GetTempPath() return "/var..." but Directory.GetCurrentDirectory return "/private/var...". + // This discrepancy breaks path equality checks in analyzers if we pass to MSBuild full path to the initial project. + // See if there is a way of fixing it in the engine - tracked: https://github.com/orgs/dotnet/projects/373/views/1?pane=issue&itemId=55702688. + _env.SetCurrentDirectory(Path.GetDirectoryName(projectFile.Path)); + return (projectFile); } } } From 15c61c8c343a3c8855257389bdcca3f518b7ed11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mariana=20Dematt=C3=A9?= Date: Fri, 19 Apr 2024 16:33:00 +0200 Subject: [PATCH 09/14] Fix failing tests --- src/BuildCheck.UnitTests/EndToEndTests.cs | 27 +++++++++-------------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/src/BuildCheck.UnitTests/EndToEndTests.cs b/src/BuildCheck.UnitTests/EndToEndTests.cs index 4e947ea3222..52033611699 100644 --- a/src/BuildCheck.UnitTests/EndToEndTests.cs +++ b/src/BuildCheck.UnitTests/EndToEndTests.cs @@ -39,7 +39,7 @@ public void SampleAnalyzerIntegrationTest(bool buildInOutOfProcessNode, bool ana _env.SetEnvironmentVariable("MSBUILDNOINPROCNODE", buildInOutOfProcessNode ? "1" : "0"); _env.SetEnvironmentVariable("MSBUILDLOGPROPERTIESANDITEMSAFTEREVALUATION", "1"); string output = RunnerUtilities.ExecBootstrapedMSBuild( - $"{Path.GetFileName(projectFile.Path)} /m:1 -nr:False -restore" + + $"{Path.GetFileName(projectFile.Path)} /m:1 -nr:False -restore -p:BuildProjectReferences=false" + (analysisRequested ? " -analyze" : string.Empty), out bool success); _env.Output.WriteLine(output); success.ShouldBeTrue(); @@ -69,23 +69,22 @@ private TransientTestFile SetupTestFiles() { { string contents = $""" - - + - Exe - net8.0 - enable - enable + Exe + net8.0 + enable + enable - + - Test + Test - + - + - + """; @@ -106,10 +105,6 @@ private TransientTestFile SetupTestFiles() - - - - """; From b047dd461bc5497f177359aa635175aeb9460b50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mariana=20Dematt=C3=A9?= Date: Fri, 19 Apr 2024 16:59:30 +0200 Subject: [PATCH 10/14] Clean up merge, one again --- .../Infrastructure/BuildCheckConnectorLogger.cs | 16 ++++------------ .../BuildAnalysisLoggingContextExtensions.cs | 15 --------------- .../Logging/IBuildAnalysisLoggingContext.cs | 7 ------- 3 files changed, 4 insertions(+), 34 deletions(-) delete mode 100644 src/Build/BuildCheck/Logging/BuildAnalysisLoggingContextExtensions.cs delete mode 100644 src/Build/BuildCheck/Logging/IBuildAnalysisLoggingContext.cs diff --git a/src/Build/BuildCheck/Infrastructure/BuildCheckConnectorLogger.cs b/src/Build/BuildCheck/Infrastructure/BuildCheckConnectorLogger.cs index e414db7ece0..b423403b9b8 100644 --- a/src/Build/BuildCheck/Infrastructure/BuildCheckConnectorLogger.cs +++ b/src/Build/BuildCheck/Infrastructure/BuildCheckConnectorLogger.cs @@ -75,18 +75,10 @@ private void EventSource_AnyEventRaised(object sender, BuildEventArgs e) return; } - try - { - buildCheckManager.ProcessEvaluationFinishedEventArgs( - loggingContextFactory.CreateLoggingContext(e.BuildEventContext!), - projectEvaluationFinishedEventArgs); - } - catch (Exception exception) - { - Debugger.Launch(); - Console.WriteLine(exception); - throw; - } + + buildCheckManager.ProcessEvaluationFinishedEventArgs( + loggingContextFactory.CreateLoggingContext(e.BuildEventContext!), + projectEvaluationFinishedEventArgs); buildCheckManager.EndProjectEvaluation(BuildCheckDataSource.EventArgs, e.BuildEventContext!); } diff --git a/src/Build/BuildCheck/Logging/BuildAnalysisLoggingContextExtensions.cs b/src/Build/BuildCheck/Logging/BuildAnalysisLoggingContextExtensions.cs deleted file mode 100644 index 4951fd7e3c6..00000000000 --- a/src/Build/BuildCheck/Logging/BuildAnalysisLoggingContextExtensions.cs +++ /dev/null @@ -1,15 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System; -using Microsoft.Build.BackEnd.Logging; -using Microsoft.Build.Experimental.BuildCheck; - -namespace Microsoft.Build.BuildCheck.Logging; - -internal static class BuildAnalysisLoggingContextExtensions -{ - public static LoggingContext ToLoggingContext(this IBuildAnalysisLoggingContext loggingContext) => - loggingContext as AnalyzerLoggingContext ?? - throw new InvalidOperationException("The logging context is not an AnalyzerLoggingContext"); -} diff --git a/src/Build/BuildCheck/Logging/IBuildAnalysisLoggingContext.cs b/src/Build/BuildCheck/Logging/IBuildAnalysisLoggingContext.cs deleted file mode 100644 index c7433a14eb9..00000000000 --- a/src/Build/BuildCheck/Logging/IBuildAnalysisLoggingContext.cs +++ /dev/null @@ -1,7 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -namespace Microsoft.Build.Experimental.BuildCheck; - -public interface IBuildAnalysisLoggingContext -{ } From a014fdb0b82cefc55606672d1e8cf71bf001561c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mariana=20Dematt=C3=A9?= Date: Mon, 22 Apr 2024 16:41:20 +0200 Subject: [PATCH 11/14] changed case on test file --- src/BuildCheck.UnitTests/EndToEndTests.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/BuildCheck.UnitTests/EndToEndTests.cs b/src/BuildCheck.UnitTests/EndToEndTests.cs index 52033611699..ac564e8b615 100644 --- a/src/BuildCheck.UnitTests/EndToEndTests.cs +++ b/src/BuildCheck.UnitTests/EndToEndTests.cs @@ -82,7 +82,7 @@ private TransientTestFile SetupTestFiles() - + @@ -109,11 +109,9 @@ private TransientTestFile SetupTestFiles() """; - string content3 = "Console.WriteLine(\"Hello, World!\");\r\n"; TransientTestFolder workFolder = _env.CreateFolder(createFolder: true); TransientTestFile projectFile = _env.CreateFile(workFolder, "FooBar.csproj", contents); TransientTestFile projectFile2 = _env.CreateFile(workFolder, "FooBar-Copy.csproj", contents2); - TransientTestFile projectFile3 = _env.CreateFile(workFolder, "Program.cs", content3); // var cache = new SimpleProjectRootElementCache(); // ProjectRootElement xml = ProjectRootElement.OpenProjectOrSolution(projectFile.Path, /*unused*/null, /*unused*/null, cache, false /*Not explicitly loaded - unused*/); From f0c8c1964682153fcfa30db428ed616222c0c7d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mariana=20Dematt=C3=A9?= Date: Thu, 25 Apr 2024 17:08:42 +0200 Subject: [PATCH 12/14] Reqorked on ConnectorLogger due changes --- .../BuildCheckConnectorLogger.cs | 36 ++++++++++++++++--- src/BuildCheck.UnitTests/EndToEndTests.cs | 2 +- 2 files changed, 33 insertions(+), 5 deletions(-) diff --git a/src/Build/BuildCheck/Infrastructure/BuildCheckConnectorLogger.cs b/src/Build/BuildCheck/Infrastructure/BuildCheckConnectorLogger.cs index 1e420ebcc03..ee0ab28bf8a 100644 --- a/src/Build/BuildCheck/Infrastructure/BuildCheckConnectorLogger.cs +++ b/src/Build/BuildCheck/Infrastructure/BuildCheckConnectorLogger.cs @@ -49,10 +49,13 @@ public void Shutdown() { } - } - private void HandleProjectEvaluationFinishedEvent(ProjectEvaluationFinishedEventArgs eventArgs) { + if (isRestore) + { + return; + } + if (!IsMetaProjFile(eventArgs.ProjectFile)) { _buildCheckManager.ProcessEvaluationFinishedEventArgs( @@ -65,12 +68,30 @@ private void HandleProjectEvaluationFinishedEvent(ProjectEvaluationFinishedEvent private void HandleProjectEvaluationStartedEvent(ProjectEvaluationStartedEventArgs eventArgs) { + if (eventArgs.IsRestore) + { + isRestore = true; + return; + } + if (!IsMetaProjFile(eventArgs.ProjectFile)) { _buildCheckManager.StartProjectEvaluation(BuildCheckDataSource.EventArgs, eventArgs.BuildEventContext!, eventArgs.ProjectFile!); } } + private void HandleProjectFinishedEvent(ProjectFinishedEventArgs projectFinishedEventArgs) + { + if (isRestore) + { + isRestore = false; + } + else + { + _buildCheckManager.EndProjectRequest(BuildCheckDataSource.EventArgs, projectFinishedEventArgs.BuildEventContext!); + } + } + private bool IsMetaProjFile(string? projectFile) => !string.IsNullOrEmpty(projectFile) && projectFile!.EndsWith(".metaproj", StringComparison.OrdinalIgnoreCase); private void EventSource_AnyEventRaised(object sender, BuildEventArgs e) @@ -105,8 +126,15 @@ private void EventSource_BuildFinished(object sender, BuildFinishedEventArgs e) { { typeof(ProjectEvaluationFinishedEventArgs), (BuildEventArgs e) => HandleProjectEvaluationFinishedEvent((ProjectEvaluationFinishedEventArgs) e) }, { typeof(ProjectEvaluationStartedEventArgs), (BuildEventArgs e) => HandleProjectEvaluationStartedEvent((ProjectEvaluationStartedEventArgs) e) }, - { typeof(ProjectStartedEventArgs), (BuildEventArgs e) => _buildCheckManager.StartProjectRequest(BuildCheckDataSource.EventArgs, e.BuildEventContext!) }, - { typeof(ProjectFinishedEventArgs), (BuildEventArgs e) => _buildCheckManager.EndProjectRequest(BuildCheckDataSource.EventArgs, e.BuildEventContext!) }, + { typeof(ProjectStartedEventArgs), (BuildEventArgs e) => + { + if (!isRestore) + { + _buildCheckManager.StartProjectRequest(BuildCheckDataSource.EventArgs, e.BuildEventContext!); + } + } + }, + { typeof(ProjectFinishedEventArgs), (BuildEventArgs e) => HandleProjectFinishedEvent((ProjectFinishedEventArgs) e) }, { typeof(BuildCheckTracingEventArgs), (BuildEventArgs e) => _stats.Merge(((BuildCheckTracingEventArgs)e).TracingData, (span1, span2) => span1 + span2) }, { typeof(BuildCheckAcquisitionEventArgs), (BuildEventArgs e) => _buildCheckManager.ProcessAnalyzerAcquisition(((BuildCheckAcquisitionEventArgs)e).ToAnalyzerAcquisitionData(), e.BuildEventContext!) }, }; diff --git a/src/BuildCheck.UnitTests/EndToEndTests.cs b/src/BuildCheck.UnitTests/EndToEndTests.cs index 86ae9d6fb46..85f5f495d4b 100644 --- a/src/BuildCheck.UnitTests/EndToEndTests.cs +++ b/src/BuildCheck.UnitTests/EndToEndTests.cs @@ -39,7 +39,7 @@ public void SampleAnalyzerIntegrationTest(bool buildInOutOfProcessNode, bool ana _env.SetEnvironmentVariable("MSBUILDNOINPROCNODE", buildInOutOfProcessNode ? "1" : "0"); _env.SetEnvironmentVariable("MSBUILDLOGPROPERTIESANDITEMSAFTEREVALUATION", "1"); string output = RunnerUtilities.ExecBootstrapedMSBuild( - $"{Path.GetFileName(projectFile.Path)} /m:1 -nr:False -restore" + + $"{Path.GetFileName(projectFile.Path)} /m:1 /p:BuildProjectReferences=false -nr:False -restore" + (analysisRequested ? " -analyze" : string.Empty), out bool success, false, _env.Output); _env.Output.WriteLine(output); success.ShouldBeTrue(); From 5626302e27714563aece2819b60e0e1b0e4b0efd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mariana=20Dematt=C3=A9?= Date: Mon, 29 Apr 2024 15:38:36 +0200 Subject: [PATCH 13/14] Changed isRestore reset to project evaluation started --- .../BuildCheckConnectorLogger.cs | 28 +++++++++++-------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/src/Build/BuildCheck/Infrastructure/BuildCheckConnectorLogger.cs b/src/Build/BuildCheck/Infrastructure/BuildCheckConnectorLogger.cs index ee0ab28bf8a..2281228db8a 100644 --- a/src/Build/BuildCheck/Infrastructure/BuildCheckConnectorLogger.cs +++ b/src/Build/BuildCheck/Infrastructure/BuildCheckConnectorLogger.cs @@ -73,6 +73,10 @@ private void HandleProjectEvaluationStartedEvent(ProjectEvaluationStartedEventAr isRestore = true; return; } + if (isRestore) + { + isRestore = false; + } if (!IsMetaProjFile(eventArgs.ProjectFile)) { @@ -82,14 +86,7 @@ private void HandleProjectEvaluationStartedEvent(ProjectEvaluationStartedEventAr private void HandleProjectFinishedEvent(ProjectFinishedEventArgs projectFinishedEventArgs) { - if (isRestore) - { - isRestore = false; - } - else - { - _buildCheckManager.EndProjectRequest(BuildCheckDataSource.EventArgs, projectFinishedEventArgs.BuildEventContext!); - } + _buildCheckManager.EndProjectRequest(BuildCheckDataSource.EventArgs, projectFinishedEventArgs.BuildEventContext!); } private bool IsMetaProjFile(string? projectFile) => !string.IsNullOrEmpty(projectFile) && projectFile!.EndsWith(".metaproj", StringComparison.OrdinalIgnoreCase); @@ -128,13 +125,20 @@ private void EventSource_BuildFinished(object sender, BuildFinishedEventArgs e) { typeof(ProjectEvaluationStartedEventArgs), (BuildEventArgs e) => HandleProjectEvaluationStartedEvent((ProjectEvaluationStartedEventArgs) e) }, { typeof(ProjectStartedEventArgs), (BuildEventArgs e) => { - if (!isRestore) - { + if (!isRestore) + { _buildCheckManager.StartProjectRequest(BuildCheckDataSource.EventArgs, e.BuildEventContext!); - } + } + } + }, + { typeof(ProjectFinishedEventArgs), (BuildEventArgs e) => + { + if (!isRestore) + { + _buildCheckManager.EndProjectRequest(BuildCheckDataSource.EventArgs, e.BuildEventContext!); + } } }, - { typeof(ProjectFinishedEventArgs), (BuildEventArgs e) => HandleProjectFinishedEvent((ProjectFinishedEventArgs) e) }, { typeof(BuildCheckTracingEventArgs), (BuildEventArgs e) => _stats.Merge(((BuildCheckTracingEventArgs)e).TracingData, (span1, span2) => span1 + span2) }, { typeof(BuildCheckAcquisitionEventArgs), (BuildEventArgs e) => _buildCheckManager.ProcessAnalyzerAcquisition(((BuildCheckAcquisitionEventArgs)e).ToAnalyzerAcquisitionData(), e.BuildEventContext!) }, }; From 39cadf82878cfa9896840d59bcdf0bed54df1d91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mariana=20Dematt=C3=A9?= Date: Mon, 29 Apr 2024 16:21:30 +0200 Subject: [PATCH 14/14] Added documentation --- src/Build/BackEnd/Components/Logging/ILoggingService.cs | 2 +- .../BuildCheck/Infrastructure/BuildCheckConnectorLogger.cs | 5 ----- src/Framework/ProjectEvaluationStartedEventArgs.cs | 4 +++- 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/Build/BackEnd/Components/Logging/ILoggingService.cs b/src/Build/BackEnd/Components/Logging/ILoggingService.cs index bec766c3138..e8138d35a4e 100644 --- a/src/Build/BackEnd/Components/Logging/ILoggingService.cs +++ b/src/Build/BackEnd/Components/Logging/ILoggingService.cs @@ -485,7 +485,7 @@ MessageImportance MinimumRequiredMessageImportance /// /// The event context to use for logging /// Project file being built - /// Something for now + /// If the project is currently in restore phase /// The evaluation event context for the project. void LogProjectEvaluationStarted(BuildEventContext eventContext, string projectFile, bool isRestore); diff --git a/src/Build/BuildCheck/Infrastructure/BuildCheckConnectorLogger.cs b/src/Build/BuildCheck/Infrastructure/BuildCheckConnectorLogger.cs index 2281228db8a..02e3b9cf78c 100644 --- a/src/Build/BuildCheck/Infrastructure/BuildCheckConnectorLogger.cs +++ b/src/Build/BuildCheck/Infrastructure/BuildCheckConnectorLogger.cs @@ -84,11 +84,6 @@ private void HandleProjectEvaluationStartedEvent(ProjectEvaluationStartedEventAr } } - private void HandleProjectFinishedEvent(ProjectFinishedEventArgs projectFinishedEventArgs) - { - _buildCheckManager.EndProjectRequest(BuildCheckDataSource.EventArgs, projectFinishedEventArgs.BuildEventContext!); - } - private bool IsMetaProjFile(string? projectFile) => !string.IsNullOrEmpty(projectFile) && projectFile!.EndsWith(".metaproj", StringComparison.OrdinalIgnoreCase); private void EventSource_AnyEventRaised(object sender, BuildEventArgs e) diff --git a/src/Framework/ProjectEvaluationStartedEventArgs.cs b/src/Framework/ProjectEvaluationStartedEventArgs.cs index b6c2f098d59..06bb2ceb994 100644 --- a/src/Framework/ProjectEvaluationStartedEventArgs.cs +++ b/src/Framework/ProjectEvaluationStartedEventArgs.cs @@ -31,7 +31,9 @@ public ProjectEvaluationStartedEventArgs(string? message, params object[]? messa /// public string? ProjectFile { get; set; } - + /// + /// Gets or sets is the project is currently on restore phase. + /// public bool IsRestore { get; internal set; } } }