diff --git a/src/Build.UnitTests/BackEnd/BuildRequestEntry_Tests.cs b/src/Build.UnitTests/BackEnd/BuildRequestEntry_Tests.cs index e2bb6680df1..b994f4ceb2c 100644 --- a/src/Build.UnitTests/BackEnd/BuildRequestEntry_Tests.cs +++ b/src/Build.UnitTests/BackEnd/BuildRequestEntry_Tests.cs @@ -11,8 +11,6 @@ using Microsoft.Build.Unittest; using Xunit; -#nullable disable - namespace Microsoft.Build.UnitTests.BackEnd { public class BuildRequestEntry_Tests @@ -23,7 +21,7 @@ public class BuildRequestEntry_Tests public void TestConstructorGood() { BuildRequest request = CreateNewBuildRequest(1, Array.Empty()); - BuildRequestData data = new BuildRequestData("foo", new Dictionary(), "foo", Array.Empty(), null); + BuildRequestData data = new BuildRequestData("foo", new Dictionary(), "foo", Array.Empty(), null); BuildRequestConfiguration config = new BuildRequestConfiguration(1, data, "2.0"); BuildRequestEntry entry = new BuildRequestEntry(request, config); @@ -36,7 +34,7 @@ public void TestConstructorBad() { Assert.Throws(() => { - BuildRequestEntry entry = new BuildRequestEntry(null, null); + BuildRequestEntry entry = new BuildRequestEntry(null!, null!); }); } [Fact] @@ -44,7 +42,7 @@ public void TestSimpleStateProgression() { // Start in Ready BuildRequest request = CreateNewBuildRequest(1, new string[1] { "foo" }); - BuildRequestConfiguration config = new BuildRequestConfiguration(1, new BuildRequestData("foo", new Dictionary(), "foo", Array.Empty(), null), "2.0"); + BuildRequestConfiguration config = new BuildRequestConfiguration(1, new BuildRequestData("foo", new Dictionary(), "foo", Array.Empty(), null), "2.0"); BuildRequestEntry entry = new BuildRequestEntry(request, config); Assert.Equal(BuildRequestEntryState.Ready, entry.State); Assert.Equal(entry.Request, request); @@ -92,7 +90,7 @@ public void TestSimpleStateProgression() public void TestResolveConfiguration() { BuildRequest request = CreateNewBuildRequest(1, new string[1] { "foo" }); - BuildRequestData data1 = new BuildRequestData("foo", new Dictionary(), "foo", Array.Empty(), null); + BuildRequestData data1 = new BuildRequestData("foo", new Dictionary(), "foo", Array.Empty(), null); BuildRequestConfiguration config = new BuildRequestConfiguration(1, data1, "2.0"); BuildRequestEntry entry = new BuildRequestEntry(request, config); @@ -114,7 +112,7 @@ public void TestResolveConfiguration() public void TestMultipleWaitingRequests() { BuildRequest request = CreateNewBuildRequest(1, new string[1] { "foo" }); - BuildRequestData data1 = new BuildRequestData("foo", new Dictionary(), "foo", Array.Empty(), null); + BuildRequestData data1 = new BuildRequestData("foo", new Dictionary(), "foo", Array.Empty(), null); BuildRequestConfiguration config = new BuildRequestConfiguration(1, data1, "2.0"); BuildRequestEntry entry = new BuildRequestEntry(request, config); @@ -144,7 +142,7 @@ public void TestMultipleWaitingRequests() public void TestMixedWaitingRequests() { BuildRequest request = CreateNewBuildRequest(1, new string[1] { "foo" }); - BuildRequestConfiguration config = new BuildRequestConfiguration(1, new BuildRequestData("foo", new Dictionary(), "foo", Array.Empty(), null), "2.0"); + BuildRequestConfiguration config = new BuildRequestConfiguration(1, new BuildRequestData("foo", new Dictionary(), "foo", Array.Empty(), null), "2.0"); BuildRequestEntry entry = new BuildRequestEntry(request, config); Assert.Equal(BuildRequestEntryState.Ready, entry.State); @@ -181,7 +179,7 @@ public void TestNoReadyToWaiting() Assert.Throws(() => { BuildRequest request = CreateNewBuildRequest(1, new string[1] { "foo" }); - BuildRequestData data1 = new BuildRequestData("foo", new Dictionary(), "foo", Array.Empty(), null); + BuildRequestData data1 = new BuildRequestData("foo", new Dictionary(), "foo", Array.Empty(), null); BuildRequestConfiguration config = new BuildRequestConfiguration(1, data1, "2.0"); BuildRequestEntry entry = new BuildRequestEntry(request, config); Assert.Equal(BuildRequestEntryState.Ready, entry.State); @@ -197,7 +195,7 @@ public void TestNoReadyToComplete() Assert.Throws(() => { BuildRequest request = CreateNewBuildRequest(1, new string[1] { "foo" }); - BuildRequestData data1 = new BuildRequestData("foo", new Dictionary(), "foo", Array.Empty(), null); + BuildRequestData data1 = new BuildRequestData("foo", new Dictionary(), "foo", Array.Empty(), null); BuildRequestConfiguration config = new BuildRequestConfiguration(1, data1, "2.0"); BuildRequestEntry entry = new BuildRequestEntry(request, config); Assert.Equal(BuildRequestEntryState.Ready, entry.State); @@ -214,7 +212,7 @@ public void TestNoWaitingToComplete() Assert.Throws(() => { BuildRequest request = CreateNewBuildRequest(1, new string[1] { "foo" }); - BuildRequestData data1 = new BuildRequestData("foo", new Dictionary(), "foo", Array.Empty(), null); + BuildRequestData data1 = new BuildRequestData("foo", new Dictionary(), "foo", Array.Empty(), null); BuildRequestConfiguration config = new BuildRequestConfiguration(1, data1, "2.0"); BuildRequestEntry entry = new BuildRequestEntry(request, config); Assert.Equal(BuildRequestEntryState.Ready, entry.State); @@ -238,7 +236,7 @@ public void TestNoCompleteToWaiting() Assert.Throws(() => { BuildRequest request = CreateNewBuildRequest(1, new string[1] { "foo" }); - BuildRequestConfiguration config = new BuildRequestConfiguration(1, new BuildRequestData("foo", new Dictionary(), "foo", Array.Empty(), null), "2.0"); + BuildRequestConfiguration config = new BuildRequestConfiguration(1, new BuildRequestData("foo", new Dictionary(), "foo", Array.Empty(), null), "2.0"); BuildRequestEntry entry = new BuildRequestEntry(request, config); Assert.Equal(BuildRequestEntryState.Ready, entry.State); @@ -258,7 +256,7 @@ public void TestNoCompleteToWaiting() public void TestResultsWithNoMatch1() { BuildRequest request = CreateNewBuildRequest(1, new string[1] { "foo" }); - BuildRequestConfiguration config = new BuildRequestConfiguration(1, new BuildRequestData("foo", new Dictionary(), "foo", Array.Empty(), null), "2.0"); + BuildRequestConfiguration config = new BuildRequestConfiguration(1, new BuildRequestData("foo", new Dictionary(), "foo", Array.Empty(), null), "2.0"); BuildRequestEntry entry = new BuildRequestEntry(request, config); Assert.Equal(BuildRequestEntryState.Ready, entry.State); diff --git a/src/Build.UnitTests/BackEnd/BuildResult_Tests.cs b/src/Build.UnitTests/BackEnd/BuildResult_Tests.cs index 02c1179e863..dec50951300 100644 --- a/src/Build.UnitTests/BackEnd/BuildResult_Tests.cs +++ b/src/Build.UnitTests/BackEnd/BuildResult_Tests.cs @@ -14,8 +14,6 @@ using Xunit; using TaskItem = Microsoft.Build.Execution.ProjectItemInstance.TaskItem; -#nullable disable - namespace Microsoft.Build.UnitTests.BackEnd { public class BuildResult_Tests @@ -39,14 +37,14 @@ public void Clone() { BuildRequest request = CreateNewBuildRequest(1, Array.Empty()); BuildResult result1 = new BuildResult(request); - result1.ResultsByTarget.Add("FOO", BuildResultUtilities.GetEmptySucceedingTargetResult()); - Assert.True(result1.ResultsByTarget.ContainsKey("foo")); // test comparer + result1.ResultsByTarget?.Add("FOO", BuildResultUtilities.GetEmptySucceedingTargetResult()); + Assert.True(result1.ResultsByTarget?.ContainsKey("foo")); // test comparer BuildResult result2 = result1.Clone(); - result1.ResultsByTarget.Add("BAR", BuildResultUtilities.GetEmptySucceedingTargetResult()); - Assert.True(result1.ResultsByTarget.ContainsKey("foo")); // test comparer - Assert.True(result1.ResultsByTarget.ContainsKey("bar")); + result1.ResultsByTarget?.Add("BAR", BuildResultUtilities.GetEmptySucceedingTargetResult()); + Assert.True(result1.ResultsByTarget?.ContainsKey("foo")); // test comparer + Assert.True(result1.ResultsByTarget?.ContainsKey("bar")); Assert.Equal(result1.SubmissionId, result2.SubmissionId); Assert.Equal(result1.ConfigurationId, result2.ConfigurationId); @@ -54,16 +52,16 @@ public void Clone() Assert.Equal(result1.ParentGlobalRequestId, result2.ParentGlobalRequestId); Assert.Equal(result1.NodeRequestId, result2.NodeRequestId); Assert.Equal(result1.CircularDependency, result2.CircularDependency); - Assert.Equal(result1.ResultsByTarget["foo"], result2.ResultsByTarget["foo"]); + Assert.Equal(result1.ResultsByTarget?["foo"], result2.ResultsByTarget?["foo"]); Assert.Equal(result1.OverallResult, result2.OverallResult); } [Fact] public void TestConstructorBad() { - Assert.Throws(() => + Assert.Throws(() => { - BuildResult result = new BuildResult(null); + BuildResult result = new BuildResult(null!); }); } [Fact] @@ -163,7 +161,7 @@ public void TestAddResultsInvalid1() { BuildRequest request = CreateNewBuildRequest(1, Array.Empty()); BuildResult result = new BuildResult(request); - result.AddResultsForTarget(null, BuildResultUtilities.GetEmptySucceedingTargetResult()); + result.AddResultsForTarget(null!, BuildResultUtilities.GetEmptySucceedingTargetResult()); }); } @@ -174,7 +172,7 @@ public void TestAddResultsInvalid2() { BuildRequest request = CreateNewBuildRequest(1, Array.Empty()); BuildResult result = new BuildResult(request); - result.AddResultsForTarget("foo", null); + result.AddResultsForTarget("foo", null!); }); } @@ -185,7 +183,7 @@ public void TestAddResultsInvalid3() { BuildRequest request = CreateNewBuildRequest(1, Array.Empty()); BuildResult result = new BuildResult(request); - result.AddResultsForTarget(null, BuildResultUtilities.GetEmptySucceedingTargetResult()); + result.AddResultsForTarget(null!, BuildResultUtilities.GetEmptySucceedingTargetResult()); }); } [Fact] @@ -222,7 +220,7 @@ public void TestMergeResultsBad1() BuildResult result = new BuildResult(request); result.AddResultsForTarget("foo", BuildResultUtilities.GetEmptySucceedingTargetResult()); - result.MergeResults(null); + result.MergeResults(null!); }); } @@ -258,24 +256,25 @@ public void TestEnumerator() { BuildRequest request = CreateNewBuildRequest(1, Array.Empty()); BuildResult result = new BuildResult(request); - int countFound = 0; - foreach (KeyValuePair resultPair in result.ResultsByTarget) - { - countFound++; - } + int countFound = result.ResultsByTarget?.Count ?? 0; Assert.Equal(0, countFound); result.AddResultsForTarget("foo", BuildResultUtilities.GetEmptySucceedingTargetResult()); bool foundFoo = false; countFound = 0; - foreach (KeyValuePair resultPair in result.ResultsByTarget) + if (result.ResultsByTarget != null) { - if (resultPair.Key == "foo") + foreach (KeyValuePair resultPair in result.ResultsByTarget) { - foundFoo = true; + if (resultPair.Key == "foo") + { + foundFoo = true; + } + + countFound++; } - countFound++; } + Assert.Equal(1, countFound); Assert.True(foundFoo); @@ -283,20 +282,26 @@ public void TestEnumerator() foundFoo = false; bool foundBar = false; countFound = 0; - foreach (KeyValuePair resultPair in result.ResultsByTarget) + if (result.ResultsByTarget != null) { - if (resultPair.Key == "foo") - { - Assert.False(foundFoo); - foundFoo = true; - } - if (resultPair.Key == "bar") + foreach (KeyValuePair resultPair in result.ResultsByTarget) { - Assert.False(foundBar); - foundBar = true; + if (resultPair.Key == "foo") + { + Assert.False(foundFoo); + foundFoo = true; + } + + if (resultPair.Key == "bar") + { + Assert.False(foundBar); + foundBar = true; + } + + countFound++; } - countFound++; } + Assert.Equal(2, countFound); Assert.True(foundFoo); Assert.True(foundBar); @@ -322,12 +327,12 @@ public void TestTranslation() ((ITranslatable)result).Translate(TranslationHelpers.GetWriteTranslator()); INodePacket packet = BuildResult.FactoryForDeserialization(TranslationHelpers.GetReadTranslator()); - BuildResult deserializedResult = packet as BuildResult; + BuildResult deserializedResult = (packet as BuildResult)!; Assert.Equal(result.ConfigurationId, deserializedResult.ConfigurationId); Assert.True(TranslationHelpers.CompareCollections(result.DefaultTargets, deserializedResult.DefaultTargets, StringComparer.Ordinal)); Assert.True(TranslationHelpers.CompareExceptions(result.Exception, deserializedResult.Exception, out string diffReason), diffReason); - Assert.Equal(result.Exception.Message, deserializedResult.Exception.Message); + Assert.Equal(result.Exception?.Message, deserializedResult.Exception?.Message); Assert.Equal(result.GlobalRequestId, deserializedResult.GlobalRequestId); Assert.True(TranslationHelpers.CompareCollections(result.InitialTargets, deserializedResult.InitialTargets, StringComparer.Ordinal)); Assert.Equal(result.NodeRequestId, deserializedResult.NodeRequestId); diff --git a/src/Build.UnitTests/BackEnd/MockHost.cs b/src/Build.UnitTests/BackEnd/MockHost.cs index 1ff79810e54..b4c1306c1b4 100644 --- a/src/Build.UnitTests/BackEnd/MockHost.cs +++ b/src/Build.UnitTests/BackEnd/MockHost.cs @@ -205,6 +205,9 @@ public IBuildComponent GetComponent(BuildComponentType type) }; } + public TComponent GetComponent(BuildComponentType type) where TComponent : IBuildComponent + => (TComponent) GetComponent(type); + /// /// Register a new build component factory with the host. /// @@ -224,25 +227,5 @@ public INodePacket DeserializePacket(NodePacketType type, byte[] serializedPacke } #endregion - - #region IBuildComponent Members - - /// - /// Initialize this component using the provided host - /// - public void InitializeComponent(IBuildComponentHost host) - { - throw new NotImplementedException(); - } - - /// - /// Clean up any state - /// - public void ShutdownComponent() - { - throw new NotImplementedException(); - } - - #endregion } } diff --git a/src/Build.UnitTests/BackEnd/MockLoggingService.cs b/src/Build.UnitTests/BackEnd/MockLoggingService.cs index a60e662d532..65c535fc0f8 100644 --- a/src/Build.UnitTests/BackEnd/MockLoggingService.cs +++ b/src/Build.UnitTests/BackEnd/MockLoggingService.cs @@ -4,6 +4,7 @@ using System; using System.Collections; using System.Collections.Generic; +using Microsoft.Build.BackEnd; using Microsoft.Build.BackEnd.Logging; using Microsoft.Build.Framework; using Microsoft.Build.Framework.Profiler; @@ -643,6 +644,9 @@ public void LogIncludeFile(BuildEventContext buildEventContext, string filePath) throw new NotImplementedException(); } + public void InitializeComponent(IBuildComponentHost host) => throw new NotImplementedException(); + public void ShutdownComponent() => throw new NotImplementedException(); + #endregion } } diff --git a/src/Build.UnitTests/BackEnd/NodeEndpointInProc_Tests.cs b/src/Build.UnitTests/BackEnd/NodeEndpointInProc_Tests.cs index 3d29df4b00a..32f6b0e3093 100644 --- a/src/Build.UnitTests/BackEnd/NodeEndpointInProc_Tests.cs +++ b/src/Build.UnitTests/BackEnd/NodeEndpointInProc_Tests.cs @@ -81,6 +81,8 @@ public IBuildComponent GetComponent(BuildComponentType type) throw new NotImplementedException(); } + public TComponent GetComponent(BuildComponentType type) where TComponent : IBuildComponent => throw new NotImplementedException("Not expected to be used."); + public void RegisterFactory(BuildComponentType type, BuildComponentFactoryDelegate factory) { } diff --git a/src/Build.UnitTests/BackEnd/TargetBuilder_Tests.cs b/src/Build.UnitTests/BackEnd/TargetBuilder_Tests.cs index 8807b4c2979..a25f2a134e5 100644 --- a/src/Build.UnitTests/BackEnd/TargetBuilder_Tests.cs +++ b/src/Build.UnitTests/BackEnd/TargetBuilder_Tests.cs @@ -1825,6 +1825,9 @@ public IBuildComponent GetComponent(BuildComponentType type) }; } + public TComponent GetComponent(BuildComponentType type) where TComponent : IBuildComponent + => (TComponent)GetComponent(type); + /// /// Registers a component factory /// @@ -1833,27 +1836,6 @@ public void RegisterFactory(BuildComponentType type, BuildComponentFactoryDelega } #endregion - - #region IBuildComponent Members - - /// - /// Sets the component host - /// - /// The component host - public void InitializeComponent(IBuildComponentHost host) - { - throw new NotImplementedException(); - } - - /// - /// Shuts down the component - /// - public void ShutdownComponent() - { - throw new NotImplementedException(); - } - - #endregion } } } diff --git a/src/Build.UnitTests/BackEnd/TargetEntry_Tests.cs b/src/Build.UnitTests/BackEnd/TargetEntry_Tests.cs index 3dea23f8e46..b2cb7cd17bd 100644 --- a/src/Build.UnitTests/BackEnd/TargetEntry_Tests.cs +++ b/src/Build.UnitTests/BackEnd/TargetEntry_Tests.cs @@ -1327,6 +1327,9 @@ public IBuildComponent GetComponent(BuildComponentType type) }; } + public TComponent GetComponent(BuildComponentType type) where TComponent : IBuildComponent + => (TComponent)GetComponent(type); + /// /// Register a component factory. /// @@ -1335,27 +1338,6 @@ public void RegisterFactory(BuildComponentType type, BuildComponentFactoryDelega } #endregion - - #region IBuildComponent Members - - /// - /// Sets the component host - /// - /// The component host - public void InitializeComponent(IBuildComponentHost host) - { - throw new NotImplementedException(); - } - - /// - /// Shuts down the component - /// - public void ShutdownComponent() - { - throw new NotImplementedException(); - } - - #endregion } } } diff --git a/src/Build.UnitTests/BackEnd/TaskBuilder_Tests.cs b/src/Build.UnitTests/BackEnd/TaskBuilder_Tests.cs index c4a3d2bda19..68a8dea7eb0 100644 --- a/src/Build.UnitTests/BackEnd/TaskBuilder_Tests.cs +++ b/src/Build.UnitTests/BackEnd/TaskBuilder_Tests.cs @@ -1384,6 +1384,9 @@ public IBuildComponent GetComponent(BuildComponentType type) }; } + public TComponent GetComponent(BuildComponentType type) where TComponent : IBuildComponent + => (TComponent)GetComponent(type); + /// /// Register a component factory. /// @@ -1392,27 +1395,6 @@ public void RegisterFactory(BuildComponentType type, BuildComponentFactoryDelega } #endregion - - #region IBuildComponent Members - - /// - /// Sets the component host - /// - /// The component host - public void InitializeComponent(IBuildComponentHost host) - { - throw new NotImplementedException(); - } - - /// - /// Shuts down the component - /// - public void ShutdownComponent() - { - throw new NotImplementedException(); - } - - #endregion } } } diff --git a/src/Build.UnitTests/ProjectCache/ProjectCacheTests.cs b/src/Build.UnitTests/ProjectCache/ProjectCacheTests.cs index 201b74d310b..022ed16ed2c 100644 --- a/src/Build.UnitTests/ProjectCache/ProjectCacheTests.cs +++ b/src/Build.UnitTests/ProjectCache/ProjectCacheTests.cs @@ -843,7 +843,7 @@ private void AssertBuildResultForCacheHit( // If it's not a cache result by proxy targets then the cache constructed the target results by hand and only the real target result // exists in the BuildResult. - var targetResult = buildResult.ResultsByTarget["Build"]; + var targetResult = buildResult.ResultsByTarget!["Build"]; targetResult.Items.ShouldHaveSingleItem(); var itemResult = targetResult.Items.First(); diff --git a/src/Build/BackEnd/BuildManager/BuildManager.cs b/src/Build/BackEnd/BuildManager/BuildManager.cs index 476f5de6793..dc7303f4d25 100644 --- a/src/Build/BackEnd/BuildManager/BuildManager.cs +++ b/src/Build/BackEnd/BuildManager/BuildManager.cs @@ -40,8 +40,6 @@ using ForwardingLoggerRecord = Microsoft.Build.Logging.ForwardingLoggerRecord; using LoggerDescription = Microsoft.Build.Logging.LoggerDescription; -#nullable disable - namespace Microsoft.Build.Execution { /// @@ -68,7 +66,7 @@ public class BuildManager : INodePacketHandler, IBuildComponentHost, IDisposable /// /// The singleton instance for the BuildManager. /// - private static BuildManager s_singletonInstance; + private static BuildManager? s_singletonInstance; /// /// The next build id; @@ -85,37 +83,37 @@ public class BuildManager : INodePacketHandler, IBuildComponentHost, IDisposable /// /// The cache for build request configurations. /// - private IConfigCache _configCache; + private IConfigCache? _configCache; /// /// The cache for build results. /// - private IResultsCache _resultsCache; + private IResultsCache? _resultsCache; /// /// The object responsible for creating and managing nodes. /// - private INodeManager _nodeManager; + private INodeManager? _nodeManager; /// /// The object responsible for creating and managing task host nodes. /// - private INodeManager _taskHostNodeManager; + private INodeManager? _taskHostNodeManager; /// /// The object which determines which projects to build, and where. /// - private IScheduler _scheduler; + private IScheduler? _scheduler; /// /// The node configuration to use for spawning new nodes. /// - private NodeConfiguration _nodeConfiguration; + private NodeConfiguration? _nodeConfiguration; /// /// Any exception which occurs on a logging thread will go here. /// - private ExceptionDispatchInfo _threadException; + private ExceptionDispatchInfo? _threadException; /// /// Set of active nodes in the system. @@ -125,7 +123,7 @@ public class BuildManager : INodePacketHandler, IBuildComponentHost, IDisposable /// /// Event signalled when all nodes have shutdown. /// - private AutoResetEvent _noNodesActiveEvent; + private AutoResetEvent? _noNodesActiveEvent; /// /// Mapping of nodes to the configurations they know about. @@ -140,7 +138,7 @@ public class BuildManager : INodePacketHandler, IBuildComponentHost, IDisposable /// /// CancellationTokenSource to use for async operations. This will be cancelled when we are shutting down to cancel any async operations. /// - private CancellationTokenSource _executionCancellationTokenSource; + private CancellationTokenSource? _executionCancellationTokenSource; /// /// The current state of the BuildManager. @@ -155,7 +153,7 @@ public class BuildManager : INodePacketHandler, IBuildComponentHost, IDisposable /// /// The parameters with which the build was started. /// - private BuildParameters _buildParameters; + private BuildParameters? _buildParameters; /// /// The current pending and active submissions. @@ -163,20 +161,12 @@ public class BuildManager : INodePacketHandler, IBuildComponentHost, IDisposable /// /// { submissionId, BuildSubmission } /// - private readonly Dictionary _buildSubmissions; - - /// - /// The current pending and active graph build submissions. - /// - /// - /// { submissionId, GraphBuildSubmission } - /// - private readonly Dictionary _graphBuildSubmissions; + private readonly Dictionary _buildSubmissions; /// /// Event signalled when all build submissions are complete. /// - private AutoResetEvent _noActiveSubmissionsEvent; + private AutoResetEvent? _noActiveSubmissionsEvent; /// /// The overall success of the build. @@ -243,7 +233,7 @@ public class BuildManager : INodePacketHandler, IBuildComponentHost, IDisposable /// /// The worker queue. /// - private ActionBlock _workQueue; + private ActionBlock? _workQueue; /// /// Flag indicating we have disposed. @@ -258,15 +248,15 @@ public class BuildManager : INodePacketHandler, IBuildComponentHost, IDisposable /// /// Messages to be logged /// - private IEnumerable _deferredBuildMessages; + private IEnumerable? _deferredBuildMessages; /// /// Build telemetry to be send when this build ends. /// Could be null /// - private BuildTelemetry _buildTelemetry; + private BuildTelemetry? _buildTelemetry; - private ProjectCacheService _projectCacheService; + private ProjectCacheService? _projectCacheService; private bool _hasProjectCacheServiceInitializedVsScenario; @@ -300,8 +290,7 @@ public BuildManager(string hostName) ErrorUtilities.VerifyThrowArgumentNull(hostName, nameof(hostName)); _hostName = hostName; _buildManagerState = BuildManagerState.Idle; - _buildSubmissions = new Dictionary(); - _graphBuildSubmissions = new Dictionary(); + _buildSubmissions = new Dictionary(); _noActiveSubmissionsEvent = new AutoResetEvent(true); _activeNodes = new HashSet(); _noNodesActiveEvent = new AutoResetEvent(true); @@ -375,13 +364,13 @@ public static BuildManager DefaultBuildManager /// /// Retrieves a hosted instance for resolving SDKs. /// - private ISdkResolverService SdkResolverService => (this as IBuildComponentHost).GetComponent(BuildComponentType.SdkResolverService) as ISdkResolverService; + private ISdkResolverService SdkResolverService => ((this as IBuildComponentHost).GetComponent(BuildComponentType.SdkResolverService) as ISdkResolverService)!; /// /// Retrieves the logging service associated with a particular build /// /// The logging service. - ILoggingService IBuildComponentHost.LoggingService => _componentFactories.GetComponent(BuildComponentType.LoggingService) as ILoggingService; + ILoggingService IBuildComponentHost.LoggingService => _componentFactories.GetComponent(BuildComponentType.LoggingService); /// /// Retrieves the name of the component host. @@ -392,7 +381,7 @@ public static BuildManager DefaultBuildManager /// Retrieves the build parameters associated with this build. /// /// The build parameters. - BuildParameters IBuildComponentHost.BuildParameters => _buildParameters; + BuildParameters? IBuildComponentHost.BuildParameters => _buildParameters; /// /// Retrieves the LegacyThreadingData associated with a particular build manager @@ -408,7 +397,7 @@ public readonly struct DeferredBuildMessage public string Text { get; } - public string FilePath { get; } + public string? FilePath { get; } public DeferredBuildMessage(string text, MessageImportance importance) { @@ -470,7 +459,7 @@ public void BeginBuild(BuildParameters parameters) if (NativeMethodsShared.IsWindows || parameters.LowPriority) { ProcessPriorityClass priority = parameters.LowPriority ? ProcessPriorityClass.BelowNormal : ProcessPriorityClass.Normal; - IEnumerable processes = _nodeManager?.GetProcesses(); + IEnumerable? processes = _nodeManager?.GetProcesses(); if (processes is not null) { foreach (Process p in processes) @@ -547,7 +536,7 @@ public void BeginBuild(BuildParameters parameters) // Initialize additional build parameters. _buildParameters.BuildId = GetNextBuildId(); - if (_buildParameters.UsesCachedResults() && parameters.ProjectIsolationMode == ProjectIsolationMode.False) + if (_buildParameters.UsesCachedResults() && _buildParameters.ProjectIsolationMode == ProjectIsolationMode.False) { // If input or output caches are used and the project isolation mode is set to // ProjectIsolationMode.False, then set it to ProjectIsolationMode.True. The explicit @@ -583,7 +572,7 @@ public void BeginBuild(BuildParameters parameters) InitializeCaches(); #if FEATURE_REPORTFILEACCESSES - var fileAccessManager = ((IBuildComponentHost)this).GetComponent(BuildComponentType.FileAccessManager) as IFileAccessManager; + var fileAccessManager = ((IBuildComponentHost)this).GetComponent(BuildComponentType.FileAccessManager); #endif _projectCacheService = new ProjectCacheService( @@ -592,13 +581,13 @@ public void BeginBuild(BuildParameters parameters) #if FEATURE_REPORTFILEACCESSES fileAccessManager, #endif - _configCache, + _configCache!, _buildParameters.ProjectCacheDescriptor); - _taskHostNodeManager = ((IBuildComponentHost)this).GetComponent(BuildComponentType.TaskHostNodeManager) as INodeManager; - _scheduler = ((IBuildComponentHost)this).GetComponent(BuildComponentType.Scheduler) as IScheduler; + _taskHostNodeManager = ((IBuildComponentHost)this).GetComponent(BuildComponentType.TaskHostNodeManager); + _scheduler = ((IBuildComponentHost)this).GetComponent(BuildComponentType.Scheduler); - _nodeManager.RegisterPacketHandler(NodePacketType.BuildRequestBlocker, BuildRequestBlocker.FactoryForDeserialization, this); + _nodeManager!.RegisterPacketHandler(NodePacketType.BuildRequestBlocker, BuildRequestBlocker.FactoryForDeserialization, this); _nodeManager.RegisterPacketHandler(NodePacketType.BuildRequestConfiguration, BuildRequestConfiguration.FactoryForDeserialization, this); _nodeManager.RegisterPacketHandler(NodePacketType.BuildRequestConfigurationResponse, BuildRequestConfigurationResponse.FactoryForDeserialization, this); _nodeManager.RegisterPacketHandler(NodePacketType.BuildResult, BuildResult.FactoryForDeserialization, this); @@ -622,8 +611,8 @@ public void BeginBuild(BuildParameters parameters) _buildManagerState = BuildManagerState.Building; - _noActiveSubmissionsEvent.Set(); - _noNodesActiveEvent.Set(); + _noActiveSubmissionsEvent!.Set(); + _noNodesActiveEvent!.Set(); } ILoggingService InitializeLoggingService() @@ -635,7 +624,7 @@ ILoggingService InitializeLoggingService() _buildParameters.WarningsNotAsErrors, _buildParameters.WarningsAsMessages); - _nodeManager.RegisterPacketHandler(NodePacketType.LogMessage, LogMessagePacket.FactoryForDeserialization, loggingService as INodePacketHandler); + _nodeManager!.RegisterPacketHandler(NodePacketType.LogMessage, LogMessagePacket.FactoryForDeserialization, loggingService as INodePacketHandler); try { @@ -687,10 +676,10 @@ void InitializeCaches() ReuseOldCaches(_buildParameters.InputResultsCacheFiles); } - _configCache = ((IBuildComponentHost)this).GetComponent(BuildComponentType.ConfigCache) as IConfigCache; - _resultsCache = ((IBuildComponentHost)this).GetComponent(BuildComponentType.ResultsCache) as IResultsCache; + _configCache = ((IBuildComponentHost)this).GetComponent(BuildComponentType.ConfigCache); + _resultsCache = ((IBuildComponentHost)this).GetComponent(BuildComponentType.ResultsCache); - if (!usesInputCaches && (_buildParameters.ResetCaches || _configCache.IsConfigCacheSizeLargerThanThreshold())) + if (!usesInputCaches && (_buildParameters.ResetCaches || _configCache!.IsConfigCacheSizeLargerThanThreshold())) { ResetCaches(); } @@ -698,18 +687,18 @@ void InitializeCaches() { if (!usesInputCaches) { - List configurationsCleared = _configCache.ClearNonExplicitlyLoadedConfigurations(); + List configurationsCleared = _configCache!.ClearNonExplicitlyLoadedConfigurations(); if (configurationsCleared != null) { foreach (int configurationId in configurationsCleared) { - _resultsCache.ClearResultsForConfiguration(configurationId); + _resultsCache!.ClearResultsForConfiguration(configurationId); } } } - foreach (var config in _configCache) + foreach (var config in _configCache!) { config.ResultsNodeId = Scheduler.InvalidNodeId; } @@ -733,7 +722,7 @@ private void EnableDetouredNodeLauncher() ErrorUtilities.VerifyThrowInvalidOperation(NativeMethodsShared.ProcessorArchitecture == NativeMethodsShared.ProcessorArchitectures.X64, "ReportFileAccessesX64Only"); // To properly report file access, we need to disable the in-proc node which won't be detoured. - _buildParameters.DisableInProcNode = true; + _buildParameters!.DisableInProcNode = true; // Node reuse must be disabled as future builds will not be able to listen to events raised by detours. _buildParameters.EnableNodeReuse = false; @@ -764,7 +753,7 @@ private static void AttachDebugger() case "2": // Sometimes easier to attach rather than deal with JIT prompt Process currentProcess = Process.GetCurrentProcess(); - Console.WriteLine($"Waiting for debugger to attach ({currentProcess.MainModule.FileName} PID {currentProcess.Id}). Press enter to continue..."); + Console.WriteLine($"Waiting for debugger to attach ({currentProcess.MainModule!.FileName} PID {currentProcess.Id}). Press enter to continue..."); Console.ReadLine(); break; } @@ -790,7 +779,7 @@ private void CancelAllSubmissions(bool async) ? _buildParameters.UICulture : CultureInfo.CurrentUICulture; - void Callback(object state) + void Callback(object? state) { lock (_syncLock) { @@ -809,21 +798,15 @@ void Callback(object state) _overallBuildSuccess = false; - foreach (BuildSubmission submission in _buildSubmissions.Values) - { - if (submission.BuildRequest != null) - { - BuildResult result = new BuildResult(submission.BuildRequest, new BuildAbortedException()); - _resultsCache.AddResult(result); - submission.CompleteResults(result); - } - } - - foreach (GraphBuildSubmission submission in _graphBuildSubmissions.Values) + foreach (BuildSubmissionBase submission in _buildSubmissions.Values) { if (submission.IsStarted) { - submission.CompleteResults(new GraphBuildResult(submission.SubmissionId, new BuildAbortedException())); + BuildResultBase buildResult = submission.CompleteResultsWithException(new BuildAbortedException()); + if (buildResult is BuildResult result) + { + _resultsCache!.AddResult(result); + } } } @@ -845,12 +828,12 @@ public void ResetCaches() ErrorIfState(BuildManagerState.WaitingForBuildToComplete, "WaitingForEndOfBuild"); ErrorIfState(BuildManagerState.Building, "BuildInProgress"); - _configCache = ((IBuildComponentHost)this).GetComponent(BuildComponentType.ConfigCache) as IConfigCache; - _resultsCache = ((IBuildComponentHost)this).GetComponent(BuildComponentType.ResultsCache) as IResultsCache; - _resultsCache.ClearResults(); + _configCache = ((IBuildComponentHost)this).GetComponent(BuildComponentType.ConfigCache); + _resultsCache = ((IBuildComponentHost)this).GetComponent(BuildComponentType.ResultsCache); + _resultsCache!.ClearResults(); // This call clears out the directory. - _configCache.ClearConfigurations(); + _configCache!.ClearConfigurations(); _buildParameters?.ProjectRootElementCache.DiscardImplicitReferences(); } @@ -867,12 +850,12 @@ public ProjectInstance GetProjectInstanceForBuild(Project project) lock (_syncLock) { _configCache = ((IBuildComponentHost)this).GetComponent(BuildComponentType.ConfigCache) as IConfigCache; - BuildRequestConfiguration configuration = _configCache.GetMatchingConfiguration( + BuildRequestConfiguration configuration = _configCache!.GetMatchingConfiguration( new ConfigurationMetadata(project), (config, loadProject) => CreateConfiguration(project, config), loadProject: true); ErrorUtilities.VerifyThrow(configuration.Project != null, "Configuration should have been loaded."); - return configuration.Project; + return configuration.Project!; } } @@ -882,27 +865,7 @@ public ProjectInstance GetProjectInstanceForBuild(Project project) /// /// Thrown if StartBuild has not been called or if EndBuild has been called. public BuildSubmission PendBuildRequest(BuildRequestData requestData) - { - lock (_syncLock) - { - ErrorUtilities.VerifyThrowArgumentNull(requestData, nameof(requestData)); - ErrorIfState(BuildManagerState.WaitingForBuildToComplete, "WaitingForEndOfBuild"); - ErrorIfState(BuildManagerState.Idle, "NoBuildInProgress"); - VerifyStateInternal(BuildManagerState.Building); - - var newSubmission = new BuildSubmission(this, GetNextSubmissionId(), requestData, _buildParameters.LegacyThreadingSemantics); - - if (_buildTelemetry != null) - { - _buildTelemetry.Project ??= requestData.ProjectFullPath; - _buildTelemetry.Target ??= string.Join(",", requestData.TargetNames); - } - - _buildSubmissions.Add(newSubmission.SubmissionId, newSubmission); - _noActiveSubmissionsEvent.Reset(); - return newSubmission; - } - } + => (BuildSubmission) PendBuildRequest(requestData); /// /// Submits a graph build request to the current build but does not start it immediately. Allows the user to @@ -910,6 +873,17 @@ public BuildSubmission PendBuildRequest(BuildRequestData requestData) /// /// Thrown if StartBuild has not been called or if EndBuild has been called. public GraphBuildSubmission PendBuildRequest(GraphBuildRequestData requestData) + => (GraphBuildSubmission) PendBuildRequest(requestData); + + /// + /// Submits a build request to the current build but does not start it immediately. Allows the user to + /// perform asynchronous execution or access the submission ID prior to executing the request. + /// + /// Thrown if StartBuild has not been called or if EndBuild has been called. + private BuildSubmissionBase PendBuildRequest( + TRequestData requestData) + where TRequestData : BuildRequestData + where TResultData : BuildResultBase { lock (_syncLock) { @@ -918,39 +892,41 @@ public GraphBuildSubmission PendBuildRequest(GraphBuildRequestData requestData) ErrorIfState(BuildManagerState.Idle, "NoBuildInProgress"); VerifyStateInternal(BuildManagerState.Building); - var newSubmission = new GraphBuildSubmission(this, GetNextSubmissionId(), requestData); + var newSubmission = requestData.CreateSubmission(this, GetNextSubmissionId(), requestData, + _buildParameters!.LegacyThreadingSemantics); if (_buildTelemetry != null) { // Project graph can have multiple entry points, for purposes of identifying event for same build project, // we believe that including only one entry point will provide enough precision. - _buildTelemetry.Project ??= requestData.ProjectGraphEntryPoints?.FirstOrDefault().ProjectFile; + _buildTelemetry.Project ??= requestData.EntryProjectsFullPath.FirstOrDefault(); _buildTelemetry.Target ??= string.Join(",", requestData.TargetNames); } - _graphBuildSubmissions.Add(newSubmission.SubmissionId, newSubmission); - _noActiveSubmissionsEvent.Reset(); + _buildSubmissions.Add(newSubmission.SubmissionId, newSubmission); + _noActiveSubmissionsEvent!.Reset(); return newSubmission; } } + private TResultData BuildRequest(TRequestData requestData) + where TRequestData : BuildRequestData + where TResultData : BuildResultBase + => PendBuildRequest(requestData).Execute(); + /// /// Convenience method. Submits a build request and blocks until the results are available. /// /// Thrown if StartBuild has not been called or if EndBuild has been called. public BuildResult BuildRequest(BuildRequestData requestData) - { - BuildSubmission submission = PendBuildRequest(requestData); - BuildResult result = submission.Execute(); - - return result; - } + => BuildRequest(requestData); /// /// Convenience method. Submits a graph build request and blocks until the results are available. /// /// Thrown if StartBuild has not been called or if EndBuild has been called. - public GraphBuildResult BuildRequest(GraphBuildRequestData requestData) => PendBuildRequest(requestData).Execute(); + public GraphBuildResult BuildRequest(GraphBuildRequestData requestData) + => BuildRequest(requestData); /// /// Signals that no more build requests are expected (or allowed) and the BuildManager may clean up. @@ -977,36 +953,30 @@ public void EndBuild() lock (_syncLock) { // If there are any submissions which never started, remove them now. - var submissionsToCheck = new List(_buildSubmissions.Values); - foreach (BuildSubmission submission in submissionsToCheck) - { - CheckSubmissionCompletenessAndRemove(submission); - } - - var graphSubmissionsToCheck = new List(_graphBuildSubmissions.Values); - foreach (GraphBuildSubmission submission in graphSubmissionsToCheck) + var submissionsToCheck = new List(_buildSubmissions.Values); + foreach (BuildSubmissionBase submission in submissionsToCheck) { CheckSubmissionCompletenessAndRemove(submission); } } - _noActiveSubmissionsEvent.WaitOne(); + _noActiveSubmissionsEvent!.WaitOne(); ShutdownConnectedNodes(false /* normal termination */); - _noNodesActiveEvent.WaitOne(); + _noNodesActiveEvent!.WaitOne(); // Wait for all of the actions in the work queue to drain. // _workQueue.Completion.Wait() could throw here if there was an unhandled exception in the work queue, // but the top level exception handler there should catch everything and have forwarded it to the // OnThreadException method in this class already. - _workQueue.Complete(); + _workQueue!.Complete(); _workQueue.Completion.Wait(); - Task projectCacheDispose = _projectCacheService.DisposeAsync().AsTask(); + Task projectCacheDispose = _projectCacheService!.DisposeAsync().AsTask(); - ErrorUtilities.VerifyThrow(_buildSubmissions.Count == 0 && _graphBuildSubmissions.Count == 0, "All submissions not yet complete."); + ErrorUtilities.VerifyThrow(_buildSubmissions.Count == 0, "All submissions not yet complete."); ErrorUtilities.VerifyThrow(_activeNodes.Count == 0, "All nodes not yet shut down."); - if (_buildParameters.UsesOutputCache()) + if (_buildParameters!.UsesOutputCache()) { SerializeCaches(); } @@ -1020,7 +990,7 @@ public void EndBuild() foreach (KeyValuePair projectStartedEvent in _projectStartedEvents) { - BuildResult result = _resultsCache.GetResultsForConfiguration(projectStartedEvent.Value.BuildEventContext.ProjectInstanceId); + BuildResult result = _resultsCache!.GetResultsForConfiguration(projectStartedEvent.Value.BuildEventContext!.ProjectInstanceId); // It's valid to have a mismatched project started event IFF that particular // project had some sort of unhandled exception. If there is no result, we @@ -1040,7 +1010,7 @@ public void EndBuild() if (_buildParameters.DiscardBuildResults) { - _resultsCache.ClearResults(); + _resultsCache!.ClearResults(); } } catch (Exception e) @@ -1058,7 +1028,7 @@ public void EndBuild() { try { - ILoggingService loggingService = ((IBuildComponentHost)this).LoggingService; + ILoggingService? loggingService = ((IBuildComponentHost)this).LoggingService; if (loggingService != null) { @@ -1079,7 +1049,7 @@ public void EndBuild() _buildTelemetry.DisplayVersion = ProjectCollection.DisplayVersion; _buildTelemetry.FrameworkName = NativeMethodsShared.FrameworkName; - string host = null; + string? host = null; if (BuildEnvironmentState.s_runningInVisualStudio) { host = "VS"; @@ -1104,7 +1074,7 @@ public void EndBuild() } finally { - if (_buildParameters.LegacyThreadingSemantics) + if (_buildParameters!.LegacyThreadingSemantics) { _legacyThreadingData.MainThreadSubmissionId = -1; } @@ -1141,13 +1111,15 @@ void SerializeCaches() /// Convenience method. Submits a lone build request and blocks until results are available. /// /// Thrown if a build is already in progress. - public BuildResult Build(BuildParameters parameters, BuildRequestData requestData) + private TResultData Build(BuildParameters parameters, TRequestData requestData) + where TRequestData : BuildRequestData + where TResultData : BuildResultBase { - BuildResult result; + TResultData result; BeginBuild(parameters); try { - result = BuildRequest(requestData); + result = BuildRequest(requestData); if (result.Exception == null && _threadException != null) { result.Exception = _threadException.SourceException; @@ -1162,30 +1134,19 @@ public BuildResult Build(BuildParameters parameters, BuildRequestData requestDat return result; } + /// + /// Convenience method. Submits a lone build request and blocks until results are available. + /// + /// Thrown if a build is already in progress. + public BuildResult Build(BuildParameters parameters, BuildRequestData requestData) + => Build(parameters, requestData); + /// /// Convenience method. Submits a lone graph build request and blocks until results are available. /// /// Thrown if a build is already in progress. public GraphBuildResult Build(BuildParameters parameters, GraphBuildRequestData requestData) - { - GraphBuildResult result; - BeginBuild(parameters); - try - { - result = BuildRequest(requestData); - if (result.Exception == null && _threadException != null) - { - result.Exception = _threadException.SourceException; - _threadException = null; - } - } - finally - { - EndBuild(); - } - - return result; - } + => Build(parameters, requestData); /// /// Shuts down all idle MSBuild nodes on the machine @@ -1217,7 +1178,7 @@ public void Dispose() /// The packet. void INodePacketHandler.PacketReceived(int node, INodePacket packet) { - _workQueue.Post(() => ProcessPacket(node, packet)); + _workQueue!.Post(() => ProcessPacket(node, packet)); } #endregion @@ -1249,6 +1210,11 @@ IBuildComponent IBuildComponentHost.GetComponent(BuildComponentType type) return _componentFactories.GetComponent(type); } + TComponent IBuildComponentHost.GetComponent(BuildComponentType type) + { + return _componentFactories.GetComponent(type); + } + #endregion /// @@ -1256,38 +1222,40 @@ IBuildComponent IBuildComponentHost.GetComponent(BuildComponentType type) /// [SuppressMessage("Microsoft.Design", "CA1031:DoNotCatchGeneralExceptionTypes", Justification = "Standard ExpectedException pattern used")] [SuppressMessage("Microsoft.Maintainability", "CA1506:AvoidExcessiveClassCoupling", Justification = "Complex class might need refactoring to separate scheduling elements from submission elements.")] - internal void ExecuteSubmission(BuildSubmission submission, bool allowMainThreadBuild) + private void ExecuteSubmission(BuildSubmission submission, bool allowMainThreadBuild) { ErrorUtilities.VerifyThrowArgumentNull(submission, nameof(submission)); ErrorUtilities.VerifyThrow(!submission.IsCompleted, "Submission already complete."); - BuildRequestConfiguration resolvedConfiguration = null; + BuildRequestConfiguration? resolvedConfiguration = null; bool shuttingDown = false; try { lock (_syncLock) { - ProjectInstance projectInstance = submission.BuildRequestData.ProjectInstance; + submission.IsStarted = true; + + ProjectInstance? projectInstance = submission.BuildRequestData.ProjectInstance; if (projectInstance != null) { if (_acquiredProjectRootElementCacheFromProjectInstance) { ErrorUtilities.VerifyThrowArgument( - _buildParameters.ProjectRootElementCache == projectInstance.ProjectRootElementCache, + _buildParameters!.ProjectRootElementCache == projectInstance.ProjectRootElementCache, "OM_BuildSubmissionsMultipleProjectCollections"); } else { - _buildParameters.ProjectRootElementCache = projectInstance.ProjectRootElementCache; + _buildParameters!.ProjectRootElementCache = projectInstance.ProjectRootElementCache; _acquiredProjectRootElementCacheFromProjectInstance = true; } } - else if (_buildParameters.ProjectRootElementCache == null) + else if (_buildParameters!.ProjectRootElementCache == null) { // Create our own cache; if we subsequently get a build submission with a project instance attached, // we'll dump our cache and use that one. - _buildParameters.ProjectRootElementCache = + _buildParameters!.ProjectRootElementCache = new ProjectRootElementCache(false /* do not automatically reload from disk */); } @@ -1302,20 +1270,20 @@ internal void ExecuteSubmission(BuildSubmission submission, bool allowMainThread // If we have already named this instance when it was submitted previously during this build, use the same // name so that we get the same configuration (and thus don't cause it to rebuild.) - if (!_unnamedProjectInstanceToNames.TryGetValue(submission.BuildRequestData.ProjectInstance, out var tempName)) + if (!_unnamedProjectInstanceToNames.TryGetValue(submission.BuildRequestData.ProjectInstance!, out var tempName)) { tempName = "Unnamed_" + _nextUnnamedProjectId++; - _unnamedProjectInstanceToNames[submission.BuildRequestData.ProjectInstance] = tempName; + _unnamedProjectInstanceToNames[submission.BuildRequestData.ProjectInstance!] = tempName; } submission.BuildRequestData.ProjectFullPath = Path.Combine( - submission.BuildRequestData.ProjectInstance.GetProperty(ReservedPropertyNames.projectDirectory).EvaluatedValue, + submission.BuildRequestData.ProjectInstance!.GetProperty(ReservedPropertyNames.projectDirectory)!.EvaluatedValue, tempName); } // Create/Retrieve a configuration for each request var buildRequestConfiguration = new BuildRequestConfiguration(submission.BuildRequestData, _buildParameters.DefaultToolsVersion); - var matchingConfiguration = _configCache.GetMatchingConfiguration(buildRequestConfiguration); + var matchingConfiguration = _configCache!.GetMatchingConfiguration(buildRequestConfiguration); resolvedConfiguration = ResolveConfiguration( buildRequestConfiguration, matchingConfiguration, @@ -1334,14 +1302,14 @@ internal void ExecuteSubmission(BuildSubmission submission, bool allowMainThread // Only initialize once as it should be the same for all projects. _hasProjectCacheServiceInitializedVsScenario = true; - _projectCacheService.InitializePluginsForVsScenario( + _projectCacheService!.InitializePluginsForVsScenario( ProjectCacheDescriptors.Values, resolvedConfiguration, submission.BuildRequestData.TargetNames, - _executionCancellationTokenSource.Token); + _executionCancellationTokenSource!.Token); } - if (_projectCacheService.ShouldUseCache(resolvedConfiguration)) + if (_projectCacheService!.ShouldUseCache(resolvedConfiguration)) { IssueCacheRequestForBuildSubmission(new CacheRequest(submission, resolvedConfiguration)); } @@ -1372,7 +1340,7 @@ internal void ExecuteSubmission(BuildSubmission submission, bool allowMainThread { ErrorUtilities.VerifyThrow(resolvedConfiguration is not null, "Cannot call project cache without having BuildRequestConfiguration"); // We were already canceled! - CompleteSubmissionWithException(submission, resolvedConfiguration, new BuildAbortedException()); + CompleteSubmissionWithException(submission, resolvedConfiguration!, new BuildAbortedException()); } } @@ -1383,11 +1351,11 @@ private void IssueCacheRequestForBuildSubmission(CacheRequest cacheRequest) { Debug.Assert(Monitor.IsEntered(_syncLock)); - _workQueue.Post(() => + _workQueue!.Post(() => { try { - _projectCacheService.PostCacheRequest(cacheRequest, _executionCancellationTokenSource.Token); + _projectCacheService!.PostCacheRequest(cacheRequest, _executionCancellationTokenSource!.Token); } catch (Exception e) { @@ -1396,16 +1364,36 @@ private void IssueCacheRequestForBuildSubmission(CacheRequest cacheRequest) }); } + internal void ExecuteSubmission( + BuildSubmissionBase submission, bool allowMainThreadBuild) + where TRequestData : BuildRequestDataBase + where TResultData : BuildResultBase + { + // TODO: here we should add BuildRequestStarted https://github.com/dotnet/msbuild/issues/10145 + // BuildEventContext buildEventContext = new BuildEventContext(submission.SubmissionId, 1, BuildEventContext.InvalidProjectInstanceId, BuildEventContext.InvalidProjectContextId, BuildEventContext.InvalidTargetId, BuildEventContext.InvalidTaskId); + // ((IBuildComponentHost)this).LoggingService.LogBuildEvent() + + + if (submission is BuildSubmission buildSubmission) + { + ExecuteSubmission(buildSubmission, allowMainThreadBuild); + } + else if (submission is GraphBuildSubmission graphBuildSubmission) + { + ExecuteSubmission(graphBuildSubmission); + } + } + /// /// This method adds the graph build request in the specified submission to the set of requests being handled by the scheduler. /// - internal void ExecuteSubmission(GraphBuildSubmission submission) + private void ExecuteSubmission(GraphBuildSubmission submission) { - lock (_syncLock) - { - VerifyStateInternal(BuildManagerState.Building); + VerifyStateInternal(BuildManagerState.Building); - try + try + { + lock (_syncLock) { submission.IsStarted = true; @@ -1431,15 +1419,16 @@ internal void ExecuteSubmission(GraphBuildSubmission submission) HandleSubmissionException(submission, ex); } }, - _executionCancellationTokenSource.Token, + _executionCancellationTokenSource!.Token, TaskCreationOptions.LongRunning, TaskScheduler.Default); } - catch (Exception ex) when (!ExceptionHandling.IsCriticalException(ex)) - { - HandleSubmissionException(submission, ex); - throw; - } + } + // The handling of submission exception needs to be done outside of the lock + catch (Exception ex) when (!ExceptionHandling.IsCriticalException(ex)) + { + HandleSubmissionException(submission, ex); + throw; } } @@ -1489,7 +1478,7 @@ private void LoadSolutionIntoConfiguration(BuildRequestConfiguration config, Bui GetNewConfigurationId(), instances[i]) { ExplicitlyLoaded = config.ExplicitlyLoaded }; - if (_configCache.GetMatchingConfiguration(newConfig) == null) + if (_configCache!.GetMatchingConfiguration(newConfig) == null) { _configCache.AddConfiguration(newConfig); } @@ -1507,7 +1496,7 @@ private static int GetNextBuildId() /// /// Creates and optionally populates a new configuration. /// - private BuildRequestConfiguration CreateConfiguration(Project project, BuildRequestConfiguration existingConfiguration) + private BuildRequestConfiguration CreateConfiguration(Project project, BuildRequestConfiguration? existingConfiguration) { ProjectInstance newInstance = project.CreateProjectInstance(); @@ -1536,7 +1525,7 @@ private void ProcessWorkQueue(Action action) try { - if (!Equals(CultureInfo.CurrentCulture, _buildParameters.Culture)) + if (!Equals(CultureInfo.CurrentCulture, _buildParameters!.Culture)) { CultureInfo.CurrentCulture = _buildParameters.Culture; } @@ -1658,73 +1647,10 @@ private void CompleteSubmissionWithException(BuildSubmission submission, BuildRe /// /// To avoid deadlock possibility, this method MUST NOT be called inside of 'lock (_syncLock)' /// - private void HandleSubmissionException(BuildSubmission submission, Exception ex) + private void HandleSubmissionException(BuildSubmissionBase submission, Exception ex) { Debug.Assert(!Monitor.IsEntered(_syncLock)); - if (ex is AggregateException ae && ae.InnerExceptions.Count == 1) - { - ex = ae.InnerExceptions.First(); - } - if (ex is InvalidProjectFileException projectException) - { - if (!projectException.HasBeenLogged) - { - BuildEventContext buildEventContext = new BuildEventContext(submission.SubmissionId, 1, BuildEventContext.InvalidProjectInstanceId, BuildEventContext.InvalidProjectContextId, BuildEventContext.InvalidTargetId, BuildEventContext.InvalidTaskId); - ((IBuildComponentHost)this).LoggingService.LogInvalidProjectFileError(buildEventContext, projectException); - projectException.HasBeenLogged = true; - } - } - - bool submissionNeedsCompletion; - lock (_syncLock) - { - // BuildRequest may be null if the submission fails early on. - submissionNeedsCompletion = submission.BuildRequest != null; - if (submissionNeedsCompletion) - { - var result = new BuildResult(submission.BuildRequest, ex); - submission.CompleteResults(result); - } - } - - if (submissionNeedsCompletion) - { - WaitForAllLoggingServiceEventsToBeProcessed(); - } - - lock (_syncLock) - { - if (submissionNeedsCompletion) - { - submission.CompleteLogging(); - } - - _overallBuildSuccess = false; - CheckSubmissionCompletenessAndRemove(submission); - } - } - - /// - /// Waits to drain all events of logging service. - /// This method shall be used carefully because during draining, LoggingService will block all incoming events. - /// - /// - /// To avoid deadlock possibility, this method MUST NOT be called inside of 'lock (_syncLock)' - /// - private void WaitForAllLoggingServiceEventsToBeProcessed() - { - // this has to be called out of the lock (_syncLock) - // because processing events can callback to 'this' instance and cause deadlock - Debug.Assert(!Monitor.IsEntered(_syncLock)); - ((LoggingService)((IBuildComponentHost)this).LoggingService).WaitForLoggingToProcessEvents(); - } - - /// - /// Deals with exceptions that may be thrown as a result of ExecuteSubmission. - /// - private void HandleSubmissionException(GraphBuildSubmission submission, Exception ex) - { if (ex is AggregateException ae) { // If there's exactly 1, just flatten it @@ -1734,7 +1660,7 @@ private void HandleSubmissionException(GraphBuildSubmission submission, Exceptio } else { - // Log each InvalidProjectFileException encountered during ProjectGraph creation + // Log each InvalidProjectFileException encountered foreach (Exception innerException in ae.InnerExceptions) { if (innerException is InvalidProjectFileException innerProjectException) @@ -1755,11 +1681,27 @@ private void HandleSubmissionException(GraphBuildSubmission submission, Exceptio LogInvalidProjectFileError(new InvalidProjectFileException(ex.Message, ex)); } + bool submissionNeedsCompletion; lock (_syncLock) { - if (submission.IsStarted) + // BuildRequest may be null if the submission fails early on. + submissionNeedsCompletion = submission.IsStarted; + if (submissionNeedsCompletion) { - submission.CompleteResults(new GraphBuildResult(submission.SubmissionId, ex)); + submission.CompleteResultsWithException(ex); + } + } + + if (submissionNeedsCompletion) + { + WaitForAllLoggingServiceEventsToBeProcessed(); + } + + lock (_syncLock) + { + if (submissionNeedsCompletion) + { + submission.CompleteLogging(); } _overallBuildSuccess = false; @@ -1777,6 +1719,21 @@ void LogInvalidProjectFileError(InvalidProjectFileException projectException) } } + /// + /// Waits to drain all events of logging service. + /// This method shall be used carefully because during draining, LoggingService will block all incoming events. + /// + /// + /// To avoid deadlock possibility, this method MUST NOT be called inside of 'lock (_syncLock)' + /// + private void WaitForAllLoggingServiceEventsToBeProcessed() + { + // this has to be called out of the lock (_syncLock) + // because processing events can callback to 'this' instance and cause deadlock + Debug.Assert(!Monitor.IsEntered(_syncLock)); + ((LoggingService)((IBuildComponentHost)this).LoggingService).WaitForLoggingToProcessEvents(); + } + private static void AddBuildRequestToSubmission(BuildSubmission submission, int configurationId, int projectContextId = BuildEventContext.InvalidProjectContextId) { submission.BuildRequest = new BuildRequest( @@ -1815,7 +1772,7 @@ private static void AddProxyBuildRequestToSubmission( /// private void IssueBuildRequestForBuildSubmission(BuildSubmission submission, BuildRequestConfiguration configuration, bool allowMainThreadBuild = false) { - _workQueue.Post( + _workQueue!.Post( () => { try @@ -1844,7 +1801,7 @@ void IssueBuildSubmissionToSchedulerImpl(BuildSubmission submission, bool allowM throw new BuildAbortedException(); } - if (allowMainThreadBuild && _buildParameters.LegacyThreadingSemantics) + if (allowMainThreadBuild && _buildParameters!.LegacyThreadingSemantics) { if (_legacyThreadingData.MainThreadSubmissionId == -1) { @@ -1890,7 +1847,7 @@ void IssueBuildSubmissionToSchedulerImpl(BuildSubmission submission, bool allowM lock (_syncLock) { submission.CompleteLogging(); - ReportResultsToSubmission(new BuildResult(submission.BuildRequest, ex)); + ReportResultsToSubmission(new BuildResult(submission.BuildRequest!, ex)); _overallBuildSuccess = false; } } @@ -1917,7 +1874,7 @@ private void ExecuteGraphBuildScheduler(GraphBuildSubmission submission) ProjectCollection.GlobalProjectCollection, (path, properties, collection) => { - ProjectLoadSettings projectLoadSettings = _buildParameters.ProjectLoadSettings; + ProjectLoadSettings projectLoadSettings = _buildParameters!.ProjectLoadSettings; if (submission.BuildRequestData.Flags.HasFlag(BuildRequestDataFlags.IgnoreMissingEmptyAndInvalidImports)) { projectLoadSettings |= ProjectLoadSettings.IgnoreMissingImports | ProjectLoadSettings.IgnoreInvalidImports | ProjectLoadSettings.IgnoreEmptyImports; @@ -1955,11 +1912,11 @@ private void ExecuteGraphBuildScheduler(GraphBuildSubmission submission) projectGraph.ConstructionMetrics.NodeCount, projectGraph.ConstructionMetrics.EdgeCount)); - Dictionary resultsPerNode = null; + Dictionary? resultsPerNode = null; if (submission.BuildRequestData.GraphBuildOptions.Build) { - _projectCacheService.InitializePluginsForGraph(projectGraph, submission.BuildRequestData.TargetNames, _executionCancellationTokenSource.Token); + _projectCacheService!.InitializePluginsForGraph(projectGraph, submission.BuildRequestData.TargetNames, _executionCancellationTokenSource!.Token); IReadOnlyDictionary> targetsPerNode = projectGraph.GetTargetLists(submission.BuildRequestData.TargetNames); @@ -1985,12 +1942,12 @@ private void ExecuteGraphBuildScheduler(GraphBuildSubmission submission) "Exceptions only get set when the graph submission gets completed with an exception in OnThreadException. That should not happen during graph builds."); // The overall submission is complete, so report it as complete - ReportResultsToSubmission( + ReportResultsToSubmission( new GraphBuildResult( submission.SubmissionId, new ReadOnlyDictionary(resultsPerNode ?? new Dictionary()))); - static void DumpGraph(ProjectGraph graph, IReadOnlyDictionary> targetList = null) + static void DumpGraph(ProjectGraph graph, IReadOnlyDictionary>? targetList = null) { if (Traits.Instance.DebugEngine is false) { @@ -2013,9 +1970,9 @@ private Dictionary BuildGraph( var blockedNodes = new HashSet(projectGraph.ProjectNodes); var finishedNodes = new HashSet(projectGraph.ProjectNodes.Count); - var buildingNodes = new Dictionary(); + var buildingNodes = new Dictionary(); var resultsPerNode = new Dictionary(projectGraph.ProjectNodes.Count); - ExceptionDispatchInfo submissionException = null; + ExceptionDispatchInfo? submissionException = null; while (blockedNodes.Count > 0 || buildingNodes.Count > 0) { @@ -2064,7 +2021,7 @@ private Dictionary BuildGraph( { lock (graphBuildStateLock) { - if (submissionException == null && finishedBuildSubmission.BuildResult.Exception != null) + if (submissionException == null && finishedBuildSubmission.BuildResult?.Exception != null) { // Preserve the original stack. submissionException = ExceptionDispatchInfo.Capture(finishedBuildSubmission.BuildResult.Exception); @@ -2075,7 +2032,7 @@ private Dictionary BuildGraph( finishedNodes.Add(finishedNode); buildingNodes.Remove(finishedBuildSubmission); - resultsPerNode.Add(finishedNode, finishedBuildSubmission.BuildResult); + resultsPerNode.Add(finishedNode, finishedBuildSubmission.BuildResult!); } waitHandle.Set(); @@ -2096,17 +2053,17 @@ private void ShutdownConnectedNodes(bool abort) lock (_syncLock) { _shuttingDown = true; - _executionCancellationTokenSource.Cancel(); + _executionCancellationTokenSource!.Cancel(); // If we are aborting, we will NOT reuse the nodes because their state may be compromised by attempts to shut down while the build is in-progress. - _nodeManager.ShutdownConnectedNodes(!abort && _buildParameters.EnableNodeReuse); + _nodeManager!.ShutdownConnectedNodes(!abort && _buildParameters!.EnableNodeReuse); // if we are aborting, the task host will hear about it in time through the task building infrastructure; // so only shut down the task host nodes if we're shutting down tidily (in which case, it is assumed that all // tasks are finished building and thus that there's no risk of a race between the two shutdown pathways). if (!abort) { - _taskHostNodeManager.ShutdownConnectedNodes(_buildParameters.EnableNodeReuse); + _taskHostNodeManager!.ShutdownConnectedNodes(_buildParameters!.EnableNodeReuse); } } } @@ -2154,23 +2111,22 @@ private void VerifyStateInternal(BuildManagerState requiredState) /// private void Reset() { - _nodeManager.UnregisterPacketHandler(NodePacketType.BuildRequestBlocker); - _nodeManager.UnregisterPacketHandler(NodePacketType.BuildRequestConfiguration); - _nodeManager.UnregisterPacketHandler(NodePacketType.BuildRequestConfigurationResponse); - _nodeManager.UnregisterPacketHandler(NodePacketType.BuildResult); - _nodeManager.UnregisterPacketHandler(NodePacketType.NodeShutdown); + _nodeManager?.UnregisterPacketHandler(NodePacketType.BuildRequestBlocker); + _nodeManager?.UnregisterPacketHandler(NodePacketType.BuildRequestConfiguration); + _nodeManager?.UnregisterPacketHandler(NodePacketType.BuildRequestConfigurationResponse); + _nodeManager?.UnregisterPacketHandler(NodePacketType.BuildResult); + _nodeManager?.UnregisterPacketHandler(NodePacketType.NodeShutdown); - _nodeManager.ClearPerBuildState(); + _nodeManager?.ClearPerBuildState(); _nodeManager = null; _shuttingDown = false; - _executionCancellationTokenSource.Dispose(); + _executionCancellationTokenSource?.Dispose(); _executionCancellationTokenSource = null; _nodeConfiguration = null; _buildSubmissions.Clear(); - _graphBuildSubmissions.Clear(); - _scheduler.Reset(); + _scheduler?.Reset(); _scheduler = null; _workQueue = null; _projectCacheService = null; @@ -2195,7 +2151,7 @@ private void Reset() // Optionally clear out the cache. This has the advantage of releasing memory, // but the disadvantage of causing the next build to repeat the load and parse. // We'll experiment here and ship with the best default. - _buildParameters.ProjectRootElementCache.Clear(); + _buildParameters?.ProjectRootElementCache.Clear(); } } @@ -2221,11 +2177,11 @@ private int GetNewConfigurationId() /// /// Finds a matching configuration in the cache and returns it, or stores the configuration passed in. /// - private BuildRequestConfiguration ResolveConfiguration(BuildRequestConfiguration unresolvedConfiguration, BuildRequestConfiguration matchingConfigurationFromCache, bool replaceProjectInstance) + private BuildRequestConfiguration ResolveConfiguration(BuildRequestConfiguration unresolvedConfiguration, BuildRequestConfiguration? matchingConfigurationFromCache, bool replaceProjectInstance) { Debug.Assert(Monitor.IsEntered(_syncLock)); - BuildRequestConfiguration resolvedConfiguration = matchingConfigurationFromCache ?? _configCache.GetMatchingConfiguration(unresolvedConfiguration); + BuildRequestConfiguration resolvedConfiguration = matchingConfigurationFromCache ?? _configCache!.GetMatchingConfiguration(unresolvedConfiguration); if (resolvedConfiguration == null) { resolvedConfiguration = AddNewConfiguration(unresolvedConfiguration); @@ -2259,16 +2215,16 @@ private void ReplaceExistingProjectInstance(BuildRequestConfiguration newConfigu Debug.Assert(Monitor.IsEntered(_syncLock)); existingConfiguration.Project = newConfiguration.Project; - _resultsCache.ClearResultsForConfiguration(existingConfiguration.ConfigurationId); + _resultsCache!.ClearResultsForConfiguration(existingConfiguration.ConfigurationId); } private BuildRequestConfiguration AddNewConfiguration(BuildRequestConfiguration unresolvedConfiguration) { Debug.Assert(Monitor.IsEntered(_syncLock)); - var newConfigurationId = _scheduler.GetConfigurationIdFromPlan(unresolvedConfiguration.ProjectFullPath); + var newConfigurationId = _scheduler!.GetConfigurationIdFromPlan(unresolvedConfiguration.ProjectFullPath); - if (_configCache.HasConfiguration(newConfigurationId) || (newConfigurationId == BuildRequestConfiguration.InvalidConfigurationId)) + if (_configCache!.HasConfiguration(newConfigurationId) || (newConfigurationId == BuildRequestConfiguration.InvalidConfigurationId)) { // There is already a configuration like this one or one didn't exist in a plan, so generate a new ID. newConfigurationId = GetNewConfigurationId(); @@ -2283,7 +2239,7 @@ private BuildRequestConfiguration AddNewConfiguration(BuildRequestConfiguration internal void PostCacheResult(CacheRequest cacheRequest, CacheResult cacheResult, int projectContextId) { - _workQueue.Post(() => + _workQueue!.Post(() => { if (cacheResult.Exception is not null) { @@ -2322,16 +2278,16 @@ void HandleCacheResult() // There must be a build request for the results, so fake one. AddBuildRequestToSubmission(submission, configuration.ConfigurationId, projectContextId); - var result = new BuildResult(submission.BuildRequest); + var result = new BuildResult(submission.BuildRequest!); - foreach (var cacheResult in cacheResult.BuildResult.ResultsByTarget) + foreach (var cacheResultInner in cacheResult.BuildResult?.ResultsByTarget ?? Enumerable.Empty>()) { - result.AddResultsForTarget(cacheResult.Key, cacheResult.Value); + result.AddResultsForTarget(cacheResultInner.Key, cacheResultInner.Value); } - _resultsCache.AddResult(result); + _resultsCache!.AddResult(result); submission.CompleteLogging(); - ReportResultsToSubmission(result); + ReportResultsToSubmission(result); } } catch (Exception e) @@ -2352,7 +2308,7 @@ private void HandleNewRequest(int node, BuildRequestBlocker blocker) { foreach (BuildRequest request in blocker.BuildRequests) { - BuildRequestConfiguration config = _configCache[request.ConfigurationId]; + BuildRequestConfiguration config = _configCache![request.ConfigurationId]; if (FileUtilities.IsSolutionFilename(config.ProjectFullPath)) { try @@ -2362,7 +2318,7 @@ private void HandleNewRequest(int node, BuildRequestBlocker blocker) catch (InvalidProjectFileException e) { // Throw the error in the cache. The Scheduler will pick it up and return the results correctly. - _resultsCache.AddResult(new BuildResult(request, e)); + _resultsCache!.AddResult(new BuildResult(request, e)); if (node == Scheduler.VirtualNode) { throw; @@ -2372,7 +2328,7 @@ private void HandleNewRequest(int node, BuildRequestBlocker blocker) } } - IEnumerable response = _scheduler.ReportRequestBlocked(node, blocker); + IEnumerable response = _scheduler!.ReportRequestBlocked(node, blocker); PerformSchedulingActions(response); } @@ -2387,17 +2343,17 @@ private void HandleResourceRequest(int node, ResourceRequest request) { // Resource request requires a response and may be blocking. Our continuation is effectively a callback // to be called once at least one core becomes available. - _scheduler.RequestCores(request.GlobalRequestId, request.NumCores, request.IsBlocking).ContinueWith((Task task) => + _scheduler!.RequestCores(request.GlobalRequestId, request.NumCores, request.IsBlocking).ContinueWith((Task task) => { var response = new ResourceResponse(request.GlobalRequestId, task.Result); - _nodeManager.SendData(node, response); + _nodeManager!.SendData(node, response); }, TaskContinuationOptions.ExecuteSynchronously); } else { // Resource release is a one-way call, no response is expected. We release the cores as instructed // and kick the scheduler because there may be work waiting for cores to become available. - IEnumerable response = _scheduler.ReleaseCores(request.GlobalRequestId, request.NumCores); + IEnumerable response = _scheduler!.ReleaseCores(request.GlobalRequestId, request.NumCores); PerformSchedulingActions(response); } } @@ -2413,7 +2369,7 @@ private void HandleConfigurationRequest(int node, BuildRequestConfiguration unre var response = new BuildRequestConfigurationResponse(unresolvedConfiguration.ConfigurationId, resolvedConfiguration.ConfigurationId, resolvedConfiguration.ResultsNodeId); - if (!_nodeIdToKnownConfigurations.TryGetValue(node, out HashSet configurationsOnNode)) + if (!_nodeIdToKnownConfigurations.TryGetValue(node, out HashSet? configurationsOnNode)) { configurationsOnNode = new HashSet(); _nodeIdToKnownConfigurations[node] = configurationsOnNode; @@ -2421,7 +2377,7 @@ private void HandleConfigurationRequest(int node, BuildRequestConfiguration unre configurationsOnNode.Add(resolvedConfiguration.ConfigurationId); - _nodeManager.SendData(node, response); + _nodeManager!.SendData(node, response); } /// @@ -2430,7 +2386,7 @@ private void HandleConfigurationRequest(int node, BuildRequestConfiguration unre private void HandleResult(int node, BuildResult result) { // Update cache with the default, initial, and project targets, as needed. - BuildRequestConfiguration configuration = _configCache[result.ConfigurationId]; + BuildRequestConfiguration configuration = _configCache![result.ConfigurationId]; if (result.DefaultTargets != null) { // If the result has Default, Initial, and project targets, we populate the configuration cache with them if it @@ -2447,7 +2403,7 @@ private void HandleResult(int node, BuildResult result) // handled here. This intentionally mirrors the behavior for cache requests, as it doesn't make sense to // report for projects which aren't going to be requested. Ideally, *any* request could be handled, but that // would require moving the cache service interactions to the Scheduler. - if (_buildSubmissions.TryGetValue(result.SubmissionId, out BuildSubmission buildSubmission)) + if (_buildSubmissions.TryGetValue(result.SubmissionId, out BuildSubmissionBase? buildSubmissionBase) && buildSubmissionBase is BuildSubmission buildSubmission) { // The result may be associated with the build submission due to it being the submission which // caused the build, but not the actual request which was originally used with the build submission. @@ -2455,14 +2411,14 @@ private void HandleResult(int node, BuildResult result) // isn't what we're looking for. Ensure only the actual submission's request is considered. if (buildSubmission.BuildRequest != null && buildSubmission.BuildRequest.ConfigurationId == configuration.ConfigurationId - && _projectCacheService.ShouldUseCache(configuration)) + && _projectCacheService!.ShouldUseCache(configuration)) { - BuildEventContext buildEventContext = _projectStartedEvents.TryGetValue(result.SubmissionId, out BuildEventArgs buildEventArgs) - ? buildEventArgs.BuildEventContext + BuildEventContext buildEventContext = _projectStartedEvents.TryGetValue(result.SubmissionId, out BuildEventArgs? buildEventArgs) + ? buildEventArgs.BuildEventContext! : new BuildEventContext(result.SubmissionId, node, configuration.Project?.EvaluationId ?? BuildEventContext.InvalidEvaluationId, configuration.ConfigurationId, BuildEventContext.InvalidProjectContextId, BuildEventContext.InvalidTargetId, BuildEventContext.InvalidTaskId); try { - _projectCacheService.HandleBuildResultAsync(configuration, result, buildEventContext, _executionCancellationTokenSource.Token).Wait(); + _projectCacheService.HandleBuildResultAsync(configuration, result, buildEventContext, _executionCancellationTokenSource!.Token).Wait(); } catch (AggregateException ex) when (ex.InnerExceptions.All(inner => inner is OperationCanceledException)) { @@ -2475,7 +2431,7 @@ private void HandleResult(int node, BuildResult result) } } - IEnumerable response = _scheduler.ReportResult(node, result); + IEnumerable response = _scheduler!.ReportResult(node, result); PerformSchedulingActions(response); } @@ -2487,7 +2443,7 @@ private void HandleNodeShutdown(int node, NodeShutdown shutdownPacket) Debug.Assert(Monitor.IsEntered(_syncLock)); _shuttingDown = true; - _executionCancellationTokenSource.Cancel(); + _executionCancellationTokenSource?.Cancel(); ErrorUtilities.VerifyThrow(_activeNodes.Contains(node), "Unexpected shutdown from node {0} which shouldn't exist.", node); _activeNodes.Remove(node); @@ -2495,47 +2451,44 @@ private void HandleNodeShutdown(int node, NodeShutdown shutdownPacket) { if (shutdownPacket.Reason == NodeShutdownReason.ConnectionFailed) { - ILoggingService loggingService = ((IBuildComponentHost)this).GetComponent(BuildComponentType.LoggingService) as ILoggingService; - foreach (BuildSubmission submission in _buildSubmissions.Values) - { - BuildEventContext buildEventContext = new BuildEventContext(submission.SubmissionId, BuildEventContext.InvalidNodeId, BuildEventContext.InvalidProjectInstanceId, BuildEventContext.InvalidProjectContextId, BuildEventContext.InvalidTargetId, BuildEventContext.InvalidTaskId); - string exception = ExceptionHandling.ReadAnyExceptionFromFile(_instantiationTimeUtc); - loggingService.LogError(buildEventContext, new BuildEventFileInfo(String.Empty) /* no project file */, "ChildExitedPrematurely", node, ExceptionHandling.DebugDumpPath, exception); - } - - foreach (GraphBuildSubmission submission in _graphBuildSubmissions.Values) + ILoggingService loggingService = ((IBuildComponentHost)this).GetComponent(BuildComponentType.LoggingService); + foreach (BuildSubmissionBase submission in _buildSubmissions.Values) { BuildEventContext buildEventContext = new BuildEventContext(submission.SubmissionId, BuildEventContext.InvalidNodeId, BuildEventContext.InvalidProjectInstanceId, BuildEventContext.InvalidProjectContextId, BuildEventContext.InvalidTargetId, BuildEventContext.InvalidTaskId); string exception = ExceptionHandling.ReadAnyExceptionFromFile(_instantiationTimeUtc); - loggingService.LogError(buildEventContext, new BuildEventFileInfo(String.Empty) /* no project file */, "ChildExitedPrematurely", node, ExceptionHandling.DebugDumpPath, exception); + loggingService?.LogError(buildEventContext, new BuildEventFileInfo(string.Empty) /* no project file */, "ChildExitedPrematurely", node, ExceptionHandling.DebugDumpPath, exception); } } - else if (shutdownPacket.Reason == NodeShutdownReason.Error && _buildSubmissions.Values.Count == 0 && _graphBuildSubmissions.Values.Count == 0) + else if (shutdownPacket.Reason == NodeShutdownReason.Error && _buildSubmissions.Values.Count == 0) { // We have no submissions to attach any exceptions to, lets just log it here. if (shutdownPacket.Exception != null) { - ILoggingService loggingService = ((IBuildComponentHost)this).GetComponent(BuildComponentType.LoggingService) as ILoggingService; - loggingService.LogError(BuildEventContext.Invalid, new BuildEventFileInfo(String.Empty) /* no project file */, "ChildExitedPrematurely", node, ExceptionHandling.DebugDumpPath, shutdownPacket.Exception.ToString()); + ILoggingService loggingService = ((IBuildComponentHost)this).GetComponent(BuildComponentType.LoggingService); + loggingService?.LogError(BuildEventContext.Invalid, new BuildEventFileInfo(string.Empty) /* no project file */, "ChildExitedPrematurely", node, ExceptionHandling.DebugDumpPath, shutdownPacket.Exception.ToString()); OnThreadException(shutdownPacket.Exception); } } - _nodeManager.ShutdownConnectedNodes(_buildParameters.EnableNodeReuse); - _taskHostNodeManager.ShutdownConnectedNodes(_buildParameters.EnableNodeReuse); + _nodeManager!.ShutdownConnectedNodes(_buildParameters!.EnableNodeReuse); + _taskHostNodeManager!.ShutdownConnectedNodes(_buildParameters.EnableNodeReuse); - foreach (BuildSubmission submission in _buildSubmissions.Values) + foreach (BuildSubmissionBase submission in _buildSubmissions.Values) { // The submission has not started - if (submission.BuildRequest == null) + if (!submission.IsStarted) { continue; } - _resultsCache.AddResult(new BuildResult(submission.BuildRequest, shutdownPacket.Exception ?? new BuildAbortedException())); + if (submission is BuildSubmission buildSubmission && buildSubmission.BuildRequest != null) + { + _resultsCache!.AddResult(new BuildResult(buildSubmission.BuildRequest, + shutdownPacket.Exception ?? new BuildAbortedException())); + } } - _scheduler.ReportBuildAborted(node); + _scheduler!.ReportBuildAborted(node); } CheckForActiveNodesAndCleanUpSubmissions(); @@ -2549,7 +2502,7 @@ private void HandleNodeShutdown(int node, NodeShutdown shutdownPacket) private void HandleFileAccessReport(int nodeId, FileAccessReport fileAccessReport) { #if FEATURE_REPORTFILEACCESSES - if (_buildParameters.ReportFileAccesses) + if (_buildParameters!.ReportFileAccesses) { ((FileAccessManager)((IBuildComponentHost)this).GetComponent(BuildComponentType.FileAccessManager)).ReportFileAccess(fileAccessReport.FileAccessData, nodeId); } @@ -2564,7 +2517,7 @@ private void HandleFileAccessReport(int nodeId, FileAccessReport fileAccessRepor private void HandleProcessReport(int nodeId, ProcessReport processReport) { #if FEATURE_REPORTFILEACCESSES - if (_buildParameters.ReportFileAccesses) + if (_buildParameters!.ReportFileAccesses) { ((FileAccessManager)((IBuildComponentHost)this).GetComponent(BuildComponentType.FileAccessManager)).ReportProcess(processReport.ProcessData, nodeId); } @@ -2583,22 +2536,19 @@ private void CheckForActiveNodesAndCleanUpSubmissions() if (_activeNodes.Count == 0) { - var submissions = new List(_buildSubmissions.Values); - foreach (BuildSubmission submission in submissions) + var submissions = new List(_buildSubmissions.Values); + foreach (BuildSubmissionBase submission in submissions) { // The submission has not started do not add it to the results cache - if (submission.BuildRequest == null) + if (!submission.IsStarted) { continue; } - // UNDONE: (stability) It might be best to trigger the logging service to shut down here, - // since the full build is complete. This would allow us to ensure all logging messages have been - // drained and all submissions can complete their logging requirements. - BuildResult result = _resultsCache.GetResultsForConfiguration(submission.BuildRequest.ConfigurationId) ?? - new BuildResult(submission.BuildRequest, new BuildAbortedException()); - - submission.CompleteResults(result); + if (!CompleteSubmissionFromCache(submission)) + { + submission.CompleteResultsWithException(new BuildAbortedException()); + } // If we never received a project started event, consider logging complete anyhow, since the nodes have // shut down. @@ -2607,21 +2557,23 @@ private void CheckForActiveNodesAndCleanUpSubmissions() CheckSubmissionCompletenessAndRemove(submission); } - var graphSubmissions = new List(_graphBuildSubmissions.Values); - foreach (GraphBuildSubmission submission in graphSubmissions) - { - if (submission.IsStarted) - { - continue; - } - - submission.CompleteResults(new GraphBuildResult(submission.SubmissionId, new BuildAbortedException())); + _noNodesActiveEvent?.Set(); + } + } - CheckSubmissionCompletenessAndRemove(submission); + private bool CompleteSubmissionFromCache(BuildSubmissionBase submissionBase) + { + if (submissionBase is BuildSubmission submission) + { + BuildResult? result = submission.BuildRequest == null ? null : _resultsCache?.GetResultsForConfiguration(submission.BuildRequest.ConfigurationId); + if (result != null) + { + submission.CompleteResults(result); + return true; } - - _noNodesActiveEvent.Set(); } + + return false; } /// @@ -2639,22 +2591,22 @@ private void PerformSchedulingActions(IEnumerable responses) break; case ScheduleActionType.SubmissionComplete: - if (_buildParameters.DetailedSummary) + if (_buildParameters!.DetailedSummary) { - _scheduler.WriteDetailedSummary(response.BuildResult.SubmissionId); + _scheduler!.WriteDetailedSummary(response.BuildResult.SubmissionId); } - ReportResultsToSubmission(response.BuildResult); + ReportResultsToSubmission(response.BuildResult); break; case ScheduleActionType.CircularDependency: case ScheduleActionType.ResumeExecution: case ScheduleActionType.ReportResults: - _nodeManager.SendData(response.NodeId, response.Unblocker); + _nodeManager!.SendData(response.NodeId, response.Unblocker); break; case ScheduleActionType.CreateNode: - IList newNodes = _nodeManager.CreateNodes(GetNodeConfiguration(), response.RequiredNodeType, response.NumberOfNodesToCreate); + IList newNodes = _nodeManager!.CreateNodes(GetNodeConfiguration(), response.RequiredNodeType, response.NumberOfNodesToCreate); if (newNodes?.Count != response.NumberOfNodesToCreate || newNodes.Any(n => n == null)) { @@ -2666,11 +2618,11 @@ private void PerformSchedulingActions(IEnumerable responses) foreach (var node in newNodes) { - _noNodesActiveEvent.Reset(); + _noNodesActiveEvent?.Reset(); _activeNodes.Add(node.NodeId); } - IEnumerable newResponses = _scheduler.ReportNodesCreated(newNodes); + IEnumerable newResponses = _scheduler!.ReportNodesCreated(newNodes); PerformSchedulingActions(newResponses); break; @@ -2683,15 +2635,15 @@ private void PerformSchedulingActions(IEnumerable responses) // of which nodes have had configurations specifically assigned to them for building. However, a node may // have created a configuration based on a build request it needs to wait on. In this // case we need not send the configuration since it will already have been mapped earlier. - if (!_nodeIdToKnownConfigurations.TryGetValue(response.NodeId, out HashSet configurationsOnNode) || + if (!_nodeIdToKnownConfigurations.TryGetValue(response.NodeId, out HashSet? configurationsOnNode) || !configurationsOnNode.Contains(response.BuildRequest.ConfigurationId)) { - IConfigCache configCache = _componentFactories.GetComponent(BuildComponentType.ConfigCache) as IConfigCache; - _nodeManager.SendData(response.NodeId, configCache[response.BuildRequest.ConfigurationId]); + IConfigCache configCache = _componentFactories.GetComponent(BuildComponentType.ConfigCache); + _nodeManager!.SendData(response.NodeId, configCache[response.BuildRequest.ConfigurationId]); } } - _nodeManager.SendData(response.NodeId, response.BuildRequest); + _nodeManager!.SendData(response.NodeId, response.BuildRequest); break; default: @@ -2701,15 +2653,15 @@ private void PerformSchedulingActions(IEnumerable responses) } } - /// - /// Completes a submission using the specified overall results. - /// - private void ReportResultsToSubmission(BuildResult result) + internal void ReportResultsToSubmission(TResultData result) + where TRequestData : BuildRequestDataBase + where TResultData : BuildResultBase { lock (_syncLock) { // The build submission has not already been completed. - if (_buildSubmissions.TryGetValue(result.SubmissionId, out BuildSubmission submission)) + if (_buildSubmissions.TryGetValue(result.SubmissionId, out BuildSubmissionBase? submissionBase) && + submissionBase is BuildSubmissionBase submission) { /* If the request failed because we caught an exception from the loggers, we can assume we will receive no more logging messages for * this submission, therefore set the logging as complete. InternalLoggerExceptions are unhandled exceptions from the logger. If the logger author does @@ -2720,75 +2672,36 @@ private void ReportResultsToSubmission(BuildResult result) * * If any other exception happened and logging is not completed, then go ahead and complete it now since this is the last place to do it. * Otherwise the submission would remain uncompleted, potentially causing hangs (EndBuild waiting on all BuildSubmissions, users waiting on BuildSubmission, or expecting a callback, etc) - */ + */ if (!submission.LoggingCompleted && result.Exception != null) { submission.CompleteLogging(); } submission.CompleteResults(result); - CheckSubmissionCompletenessAndRemove(submission); } } } - /// - /// Completes a submission using the specified overall results. - /// - private void ReportResultsToSubmission(GraphBuildResult result) - { - lock (_syncLock) - { - // The build submission has not already been completed. - if (_graphBuildSubmissions.TryGetValue(result.SubmissionId, out GraphBuildSubmission submission)) - { - submission.CompleteResults(result); - - CheckSubmissionCompletenessAndRemove(submission); - } - } - } - - /// - /// Determines if the submission is fully completed. - /// - private void CheckSubmissionCompletenessAndRemove(BuildSubmission submission) - { - lock (_syncLock) - { - // If the submission has completed or never started, remove it. - if (submission.IsCompleted || submission.BuildRequest == null) - { - _overallBuildSuccess &= (submission.BuildResult?.OverallResult == BuildResultCode.Success); - _buildSubmissions.Remove(submission.SubmissionId); - - // Clear all cached SDKs for the submission - SdkResolverService.ClearCache(submission.SubmissionId); - } - - CheckAllSubmissionsComplete(submission.BuildRequestData?.Flags); - } - } - /// /// Determines if the submission is fully completed. /// - private void CheckSubmissionCompletenessAndRemove(GraphBuildSubmission submission) + private void CheckSubmissionCompletenessAndRemove(BuildSubmissionBase submission) { lock (_syncLock) { // If the submission has completed or never started, remove it. if (submission.IsCompleted || !submission.IsStarted) { - _overallBuildSuccess &= submission.BuildResult?.OverallResult == BuildResultCode.Success; - _graphBuildSubmissions.Remove(submission.SubmissionId); + _overallBuildSuccess &= (submission.BuildResultBase?.OverallResult == BuildResultCode.Success); + _buildSubmissions.Remove(submission.SubmissionId); // Clear all cached SDKs for the submission SdkResolverService.ClearCache(submission.SubmissionId); } - CheckAllSubmissionsComplete(submission.BuildRequestData?.Flags); + CheckAllSubmissionsComplete(submission.BuildRequestDataBase.Flags); } } @@ -2796,7 +2709,7 @@ private void CheckAllSubmissionsComplete(BuildRequestDataFlags? flags) { Debug.Assert(Monitor.IsEntered(_syncLock)); - if (_buildSubmissions.Count == 0 && _graphBuildSubmissions.Count == 0) + if (_buildSubmissions.Count == 0) { if (flags.HasValue && flags.Value.HasFlag(BuildRequestDataFlags.ClearCachesAfterBuild)) { @@ -2811,7 +2724,7 @@ private void CheckAllSubmissionsComplete(BuildRequestDataFlags? flags) #endif } - _noActiveSubmissionsEvent.Set(); + _noActiveSubmissionsEvent?.Set(); } } @@ -2825,7 +2738,7 @@ private NodeConfiguration GetNodeConfiguration() if (_nodeConfiguration == null) { // Get the remote loggers - ILoggingService loggingService = ((IBuildComponentHost)this).GetComponent(BuildComponentType.LoggingService) as ILoggingService; + ILoggingService loggingService = ((IBuildComponentHost)this).GetComponent(BuildComponentType.LoggingService); var remoteLoggers = new List(loggingService.LoggerDescriptions); _nodeConfiguration = new NodeConfiguration( @@ -2862,40 +2775,30 @@ private void OnThreadException(Exception e) } _threadException = ExceptionDispatchInfo.Capture(e); - var submissions = new List(_buildSubmissions.Values); - foreach (BuildSubmission submission in submissions) + var submissions = new List(_buildSubmissions.Values); + foreach (BuildSubmissionBase submission in submissions) { // Submission has not started - if (submission.BuildRequest == null) + if (!submission.IsStarted) { continue; } // Attach the exception to this submission if it does not already have an exception associated with it - if (!submission.IsCompleted && submission.BuildResult != null && submission.BuildResult.Exception == null) + if (!submission.IsCompleted && submission.BuildResultBase != null && submission.BuildResultBase.Exception == null) { - submission.BuildResult.Exception = e; + submission.BuildResultBase.Exception = e; } submission.CompleteLogging(); - submission.CompleteResults(new BuildResult(submission.BuildRequest, e)); - - CheckSubmissionCompletenessAndRemove(submission); - } - var graphSubmissions = new List(_graphBuildSubmissions.Values); - foreach (GraphBuildSubmission submission in graphSubmissions) - { - if (!submission.IsStarted) + if (submission.BuildResultBase != null) { - continue; + submission.CheckForCompletion(); } - - // Attach the exception to this submission if it does not already have an exception associated with it - if (!submission.IsCompleted && submission.BuildResult != null && submission.BuildResult.Exception == null) + else { - submission.BuildResult.Exception = e; + submission.CompleteResultsWithException(e); } - submission.CompleteResults(submission.BuildResult ?? new GraphBuildResult(submission.SubmissionId, e)); CheckSubmissionCompletenessAndRemove(submission); } @@ -2908,7 +2811,7 @@ private void OnThreadException(Exception e) /// private void OnLoggingThreadException(Exception e) { - _workQueue.Post(() => OnThreadException(e)); + _workQueue!.Post(() => OnThreadException(e)); } /// @@ -2916,13 +2819,13 @@ private void OnLoggingThreadException(Exception e) /// private void OnProjectFinished(object sender, ProjectFinishedEventArgs e) { - _workQueue.Post(() => + _workQueue!.Post(() => { lock (_syncLock) { - if (_projectStartedEvents.TryGetValue(e.BuildEventContext.SubmissionId, out var originalArgs)) + if (_projectStartedEvents.TryGetValue(e.BuildEventContext!.SubmissionId, out var originalArgs)) { - if (originalArgs.BuildEventContext.Equals(e.BuildEventContext)) + if (originalArgs.BuildEventContext!.Equals(e.BuildEventContext)) { _projectStartedEvents.Remove(e.BuildEventContext.SubmissionId); if (_buildSubmissions.TryGetValue(e.BuildEventContext.SubmissionId, out var submission)) @@ -2941,11 +2844,11 @@ private void OnProjectFinished(object sender, ProjectFinishedEventArgs e) /// private void OnProjectStarted(object sender, ProjectStartedEventArgs e) { - _workQueue.Post(() => + _workQueue!.Post(() => { lock (_syncLock) { - if (!_projectStartedEvents.ContainsKey(e.BuildEventContext.SubmissionId)) + if (!_projectStartedEvents.ContainsKey(e.BuildEventContext!.SubmissionId)) { _projectStartedEvents[e.BuildEventContext.SubmissionId] = e; } @@ -2967,15 +2870,15 @@ internal void EnableBuildCheck() /// Creates a logging service around the specified set of loggers. /// private ILoggingService CreateLoggingService( - IEnumerable loggers, - IEnumerable forwardingLoggers, + IEnumerable? loggers, + IEnumerable? forwardingLoggers, ISet warningsAsErrors, ISet warningsNotAsErrors, ISet warningsAsMessages) { Debug.Assert(Monitor.IsEntered(_syncLock)); - int cpuCount = _buildParameters.MaxNodeCount; + int cpuCount = _buildParameters!.MaxNodeCount; LoggerMode loggerMode = cpuCount == 1 && _buildParameters.UseSynchronousLogging ? LoggerMode.Synchronous @@ -3054,11 +2957,11 @@ private ILoggingService CreateLoggingService( // We need to register SOME logger if we don't have any. This ensures the out of proc nodes will still send us message, // ensuring we receive project started and finished events. - static List ProcessForwardingLoggers(IEnumerable forwarders) + static List ProcessForwardingLoggers(IEnumerable? forwarders) { Type configurableLoggerType = typeof(ConfigurableForwardingLogger); string engineAssemblyName = configurableLoggerType.GetTypeInfo().Assembly.GetName().FullName; - string configurableLoggerName = configurableLoggerType.FullName; + string configurableLoggerName = configurableLoggerType.FullName!; if (forwarders == null) { @@ -3077,7 +2980,7 @@ static List ProcessForwardingLoggers(IEnumerable - l.ForwardingLoggerDescription.Name.Contains(typeof(CentralForwardingLogger).FullName) + l.ForwardingLoggerDescription.Name.Contains(typeof(CentralForwardingLogger).FullName!) || (l.ForwardingLoggerDescription.Name.Contains(configurableLoggerName) && @@ -3092,7 +2995,7 @@ static List ProcessForwardingLoggers(IEnumerable + ForwardingLoggerRecord? configurableLogger = result.FirstOrDefault(l => l.ForwardingLoggerDescription.Name.Contains(configurableLoggerName)); // If there is not - we need to add our own. @@ -3122,7 +3025,7 @@ ForwardingLoggerRecord CreateMinimalForwarder() } } - private static void LogDeferredMessages(ILoggingService loggingService, IEnumerable deferredBuildMessages) + private static void LogDeferredMessages(ILoggingService loggingService, IEnumerable? deferredBuildMessages) { if (deferredBuildMessages == null) { @@ -3147,7 +3050,7 @@ private static void LogDeferredMessages(ILoggingService loggingService, IEnumera /// The instance-type of packet being expected private static I ExpectPacketType(INodePacket packet, NodePacketType expectedType) where I : class, INodePacket { - I castPacket = packet as I; + I? castPacket = packet as I; // PERF: Not using VerifyThrow here to avoid boxing of expectedType. if (castPacket == null) @@ -3155,13 +3058,13 @@ private static I ExpectPacketType(INodePacket packet, NodePacketType expected ErrorUtilities.ThrowInternalError("Incorrect packet type: {0} should have been {1}", packet.Type, expectedType); } - return castPacket; + return castPacket!; } /// /// Shutdown the logging service /// - private void ShutdownLoggingService(ILoggingService loggingService) + private void ShutdownLoggingService(ILoggingService? loggingService) { try { @@ -3177,7 +3080,7 @@ private void ShutdownLoggingService(ILoggingService loggingService) { // Even if an exception is thrown, we want to make sure we null out the logging service so that // we don't try to shut it down again in some other cleanup code. - _componentFactories.ReplaceFactory(BuildComponentType.LoggingService, (IBuildComponent)null); + _componentFactories.ReplaceFactory(BuildComponentType.LoggingService, (IBuildComponent?)null); } } @@ -3322,7 +3225,7 @@ private void CancelAndMarkAsFailure() // CancelAllSubmissions also ends up setting _shuttingDown and _overallBuildSuccess but it does so in a separate thread to avoid deadlocks. // This might cause a race with the first builds which might miss the shutdown update and succeed instead of fail. _shuttingDown = true; - _executionCancellationTokenSource.Cancel(); + _executionCancellationTokenSource?.Cancel(); _overallBuildSuccess = false; } @@ -3345,7 +3248,7 @@ public LoggerVerbosity Verbosity /// /// The logger parameters. /// - public string Parameters + public string? Parameters { get => String.Empty; set { } diff --git a/src/Build/BackEnd/BuildManager/BuildRequestData.cs b/src/Build/BackEnd/BuildManager/BuildRequestData.cs index b8ae83411b2..510498ed694 100644 --- a/src/Build/BackEnd/BuildManager/BuildRequestData.cs +++ b/src/Build/BackEnd/BuildManager/BuildRequestData.cs @@ -2,13 +2,15 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Collections; using System.Collections.Generic; +using System.Collections.Immutable; +using System.Linq; using Microsoft.Build.Collections; using Microsoft.Build.Evaluation; +using Microsoft.Build.Experimental.BuildCheck; using Microsoft.Build.Shared; -#nullable disable - namespace Microsoft.Build.Execution { /// @@ -87,9 +89,9 @@ public enum BuildRequestDataFlags } /// - /// BuildRequestData encapsulates all of the data needed to submit a build request. + /// BuildRequestData encapsulates all the data needed to submit a build request. /// - public class BuildRequestData + public class BuildRequestData : BuildRequestData { /// /// Constructs a BuildRequestData for build requests based on project instances. @@ -119,7 +121,7 @@ public BuildRequestData(ProjectInstance projectInstance, string[] targetsToBuild /// The targets to build. /// The host services to use, if any. May be null. /// Flags controlling this build request. - public BuildRequestData(ProjectInstance projectInstance, string[] targetsToBuild, HostServices hostServices, BuildRequestDataFlags flags) + public BuildRequestData(ProjectInstance projectInstance, string[] targetsToBuild, HostServices? hostServices, BuildRequestDataFlags flags) : this(projectInstance, targetsToBuild, hostServices, flags, null) { } @@ -132,8 +134,8 @@ public BuildRequestData(ProjectInstance projectInstance, string[] targetsToBuild /// The host services to use, if any. May be null. /// Flags controlling this build request. /// The list of properties whose values should be transferred from the project to any out-of-proc node. - public BuildRequestData(ProjectInstance projectInstance, string[] targetsToBuild, HostServices hostServices, BuildRequestDataFlags flags, IEnumerable propertiesToTransfer) - : this(targetsToBuild, hostServices, flags) + public BuildRequestData(ProjectInstance projectInstance, string[] targetsToBuild, HostServices? hostServices, BuildRequestDataFlags flags, IEnumerable? propertiesToTransfer) + : this(targetsToBuild, hostServices, flags, projectInstance.FullPath) { ErrorUtilities.VerifyThrowArgumentNull(projectInstance, nameof(projectInstance)); @@ -144,7 +146,6 @@ public BuildRequestData(ProjectInstance projectInstance, string[] targetsToBuild ProjectInstance = projectInstance; - ProjectFullPath = projectInstance.FullPath; GlobalPropertiesDictionary = projectInstance.GlobalPropertiesDictionary; ExplicitlySpecifiedToolsVersion = projectInstance.ExplicitToolsVersion; if (propertiesToTransfer != null) @@ -162,7 +163,7 @@ public BuildRequestData(ProjectInstance projectInstance, string[] targetsToBuild /// Flags controlling this build request. /// The list of properties whose values should be transferred from the project to any out-of-proc node. /// A describing properties, items, and metadata that should be returned. Requires setting . - public BuildRequestData(ProjectInstance projectInstance, string[] targetsToBuild, HostServices hostServices, BuildRequestDataFlags flags, IEnumerable propertiesToTransfer, RequestedProjectState requestedProjectState) + public BuildRequestData(ProjectInstance projectInstance, string[] targetsToBuild, HostServices? hostServices, BuildRequestDataFlags flags, IEnumerable? propertiesToTransfer, RequestedProjectState requestedProjectState) : this(projectInstance, targetsToBuild, hostServices, flags, propertiesToTransfer) { ErrorUtilities.VerifyThrowArgumentNull(requestedProjectState, nameof(requestedProjectState)); @@ -179,7 +180,7 @@ public BuildRequestData(ProjectInstance projectInstance, string[] targetsToBuild /// The tools version to use for the build. May be null. /// The targets to build. /// The host services to use. May be null. - public BuildRequestData(string projectFullPath, IDictionary globalProperties, string toolsVersion, string[] targetsToBuild, HostServices hostServices) + public BuildRequestData(string projectFullPath, IDictionary globalProperties, string? toolsVersion, string[] targetsToBuild, HostServices? hostServices) : this(projectFullPath, globalProperties, toolsVersion, targetsToBuild, hostServices, BuildRequestDataFlags.None) { } @@ -194,8 +195,8 @@ public BuildRequestData(string projectFullPath, IDictionary glob /// The host services to use. May be null. /// The to use. /// A describing properties, items, and metadata that should be returned. Requires setting . - public BuildRequestData(string projectFullPath, IDictionary globalProperties, - string toolsVersion, string[] targetsToBuild, HostServices hostServices, BuildRequestDataFlags flags, + public BuildRequestData(string projectFullPath, IDictionary globalProperties, + string? toolsVersion, string[] targetsToBuild, HostServices? hostServices, BuildRequestDataFlags flags, RequestedProjectState requestedProjectState) : this(projectFullPath, globalProperties, toolsVersion, targetsToBuild, hostServices, flags) { @@ -213,15 +214,14 @@ public BuildRequestData(string projectFullPath, IDictionary glob /// The targets to build. /// The host services to use. May be null. /// The to use. - public BuildRequestData(string projectFullPath, IDictionary globalProperties, string toolsVersion, string[] targetsToBuild, HostServices hostServices, BuildRequestDataFlags flags) - : this(targetsToBuild, hostServices, flags) + public BuildRequestData(string projectFullPath, IDictionary globalProperties, string? toolsVersion, string[] targetsToBuild, HostServices? hostServices, BuildRequestDataFlags flags) + : this(targetsToBuild, hostServices, flags, FileUtilities.NormalizePath(projectFullPath)!) { ErrorUtilities.VerifyThrowArgumentLength(projectFullPath, nameof(projectFullPath)); ErrorUtilities.VerifyThrowArgumentNull(globalProperties, nameof(globalProperties)); - ProjectFullPath = FileUtilities.NormalizePath(projectFullPath); GlobalPropertiesDictionary = new PropertyDictionary(globalProperties.Count); - foreach (KeyValuePair propertyPair in globalProperties) + foreach (KeyValuePair propertyPair in globalProperties) { GlobalPropertiesDictionary.Set(ProjectPropertyInstance.Create(propertyPair.Key, propertyPair.Value)); } @@ -232,13 +232,10 @@ public BuildRequestData(string projectFullPath, IDictionary glob /// /// Common constructor. /// - private BuildRequestData(string[] targetsToBuild, HostServices hostServices, BuildRequestDataFlags flags) + private BuildRequestData(string[] targetsToBuild, HostServices? hostServices, BuildRequestDataFlags flags, string projectFullPath) + : base(targetsToBuild, flags, hostServices) { - ErrorUtilities.VerifyThrowArgumentNull(targetsToBuild, nameof(targetsToBuild)); - - HostServices = hostServices; - TargetNames = new List(targetsToBuild); - Flags = flags; + ProjectFullPath = projectFullPath; } /// @@ -246,7 +243,7 @@ private BuildRequestData(string[] targetsToBuild, HostServices hostServices, Bui /// May be null. /// /// The project instance. - public ProjectInstance ProjectInstance + public ProjectInstance? ProjectInstance { get; } @@ -255,16 +252,12 @@ public ProjectInstance ProjectInstance /// The project file to be built. public string ProjectFullPath { get; internal set; } - /// - /// The name of the targets to build. - /// - /// An array of targets in the project to be built. - public ICollection TargetNames { get; } + internal override BuildSubmissionBase CreateSubmission(BuildManager buildManager, + int submissionId, BuildRequestData requestData, + bool legacyThreadingSemantics) => + new BuildSubmission(buildManager, submissionId, requestData, legacyThreadingSemantics); - /// - /// Extra flags for this BuildRequest. - /// - public BuildRequestDataFlags Flags { get; } + public override IEnumerable EntryProjectsFullPath => ProjectFullPath.AsSingleItemEnumerable(); /// /// The global properties to use. @@ -274,26 +267,23 @@ public ProjectInstance ProjectInstance (ICollection)ReadOnlyEmptyCollection.Instance : new ReadOnlyCollection(GlobalPropertiesDictionary); - /// - /// The explicitly requested tools version to use. - /// - public string ExplicitlySpecifiedToolsVersion { get; } + public override bool IsGraphRequest => false; /// - /// Gets the HostServices object for this request. + /// The explicitly requested tools version to use. /// - public HostServices HostServices { get; } + public string? ExplicitlySpecifiedToolsVersion { get; } /// /// Returns a list of properties to transfer out of proc for the build. /// - public IEnumerable PropertiesToTransfer { get; } + public IEnumerable? PropertiesToTransfer { get; } /// /// Returns the properties, items, and metadata that will be returned /// by this build. /// - public RequestedProjectState RequestedProjectState { get; } + public RequestedProjectState? RequestedProjectState { get; } /// /// Whether the tools version used originated from an explicit specification, @@ -304,6 +294,12 @@ public ProjectInstance ProjectInstance /// /// Returns the global properties as a dictionary. /// - internal PropertyDictionary GlobalPropertiesDictionary { get; } + internal PropertyDictionary? GlobalPropertiesDictionary { get; } + + private IReadOnlyDictionary? _globalPropertiesLookup; + + /// + public override IReadOnlyDictionary GlobalPropertiesLookup => _globalPropertiesLookup ??= + Execution.GlobalPropertiesLookup.ToGlobalPropertiesLookup(GlobalPropertiesDictionary); } } diff --git a/src/Build/BackEnd/BuildManager/BuildRequestDataBase.cs b/src/Build/BackEnd/BuildManager/BuildRequestDataBase.cs new file mode 100644 index 00000000000..c31381c083d --- /dev/null +++ b/src/Build/BackEnd/BuildManager/BuildRequestDataBase.cs @@ -0,0 +1,67 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; +using Microsoft.Build.Shared; + +namespace Microsoft.Build.Execution +{ + public abstract class BuildRequestDataBase + { + protected BuildRequestDataBase( + ICollection targetNames, + BuildRequestDataFlags flags, + HostServices? hostServices) + { + ErrorUtilities.VerifyThrowArgumentNull(targetNames, nameof(targetNames)); + foreach (string targetName in targetNames) + { + ErrorUtilities.VerifyThrowArgumentNull(targetName, "target"); + } + + TargetNames = new List(targetNames); + Flags = flags; + HostServices = hostServices; + } + + public abstract IEnumerable EntryProjectsFullPath { get; } + + /// + /// The name of the targets to build. + /// + /// An array of targets in the project to be built. + public ICollection TargetNames { get; protected set; } + + /// + /// Extra flags for this BuildRequest. + /// + public BuildRequestDataFlags Flags { get; protected set; } + + /// + /// Gets the global properties to use for this entry point. + /// + public abstract IReadOnlyDictionary GlobalPropertiesLookup { get; } + + public abstract bool IsGraphRequest { get; } + + /// + /// Gets the HostServices object for this request. + /// + public HostServices? HostServices { get; } + } + + public abstract class BuildRequestData : BuildRequestDataBase + where TRequestData : BuildRequestData + where TResultData : BuildResultBase + { + protected BuildRequestData( + ICollection targetNames, + BuildRequestDataFlags flags, + HostServices? hostServices) + : base(targetNames, flags, hostServices) + { } + + internal abstract BuildSubmissionBase CreateSubmission( + BuildManager buildManager, int submissionId, TRequestData requestData, bool legacyThreadingSemantics); + } +} diff --git a/src/Build/BackEnd/BuildManager/BuildSubmission.cs b/src/Build/BackEnd/BuildManager/BuildSubmission.cs index ca2147dfae2..77def07e7bb 100644 --- a/src/Build/BackEnd/BuildManager/BuildSubmission.cs +++ b/src/Build/BackEnd/BuildManager/BuildSubmission.cs @@ -7,8 +7,6 @@ using Microsoft.Build.BackEnd; using Microsoft.Build.Shared; -#nullable disable - namespace Microsoft.Build.Execution { /// @@ -17,104 +15,170 @@ namespace Microsoft.Build.Execution /// /// When this delegate is invoked, the WaitHandle on the BuildSubmission will have been be signalled and the OverallBuildResult will be valid. /// - public delegate void BuildSubmissionCompleteCallback(BuildSubmission submission); - - /// - /// A BuildSubmission represents an build request which has been submitted to the BuildManager for processing. It may be used to - /// execute synchronous or asynchronous build requests and provides access to the results upon completion. - /// - /// - /// This class is thread-safe. - /// - public class BuildSubmission + internal delegate void BuildSubmissionCompleteCallbackInternal( + BuildSubmissionBase submission) + where TRequestData : BuildRequestDataBase + where TResultData : BuildResultBase; + + public abstract class BuildSubmissionBase : BuildSubmissionBase + where TRequestData : BuildRequestDataBase + where TResultData : BuildResultBase { /// /// The callback to invoke when the submission is complete. /// - private BuildSubmissionCompleteCallback _completionCallback; + private BuildSubmissionCompleteCallbackInternal? _completionCallback; /// - /// The completion event. + /// Constructor /// - private readonly ManualResetEvent _completionEvent; + protected internal BuildSubmissionBase(BuildManager buildManager, int submissionId, TRequestData requestData) + : base(buildManager, submissionId) + { + ErrorUtilities.VerifyThrowArgumentNull(requestData, nameof(requestData)); + BuildRequestData = requestData; + } + + // + // Unfortunately covariant overrides are not available for .NET 472, + // so we have to use two set of properties for derived classes. + internal override BuildResultBase? BuildResultBase => BuildResult; + internal override BuildRequestDataBase BuildRequestDataBase => BuildRequestData; /// - /// Flag indicating if logging is done. + /// The results of the build per graph node. Valid only after WaitHandle has become signalled. /// - internal bool LoggingCompleted { get; private set; } + public TResultData? BuildResult { get; set; } /// - /// True if it has been invoked + /// The BuildRequestData being used for this submission. /// - private int _completionInvoked; + internal TRequestData BuildRequestData { get; } /// - /// Flag indicating whether synchronous wait should support legacy threading semantics. + /// Starts the request and blocks until results are available. /// - private readonly bool _legacyThreadingSemantics; + /// The request has already been started or is already complete. + public abstract TResultData Execute(); + + private protected void ExecuteAsync( + BuildSubmissionCompleteCallbackInternal? callback, + object? context, + bool allowMainThreadBuild) + { + ErrorUtilities.VerifyThrowInvalidOperation(!IsCompleted, "SubmissionAlreadyComplete"); + _completionCallback = callback; + AsyncContext = context; + BuildManager.ExecuteSubmission(this, allowMainThreadBuild); + } /// - /// Constructor + /// Sets the event signaling that the build is complete. /// - internal BuildSubmission(BuildManager buildManager, int submissionId, BuildRequestData requestData, bool legacyThreadingSemantics) + internal void CompleteResults(TResultData result) { - ErrorUtilities.VerifyThrowArgumentNull(buildManager, nameof(buildManager)); - ErrorUtilities.VerifyThrowArgumentNull(requestData, nameof(requestData)); + ErrorUtilities.VerifyThrowArgumentNull(result, nameof(result)); + ErrorUtilities.VerifyThrow(result.SubmissionId == SubmissionId, + "GraphBuildResult's submission id doesn't match GraphBuildSubmission's"); - BuildManager = buildManager; - SubmissionId = submissionId; - BuildRequestData = requestData; - _completionEvent = new ManualResetEvent(false); - LoggingCompleted = false; - _completionInvoked = 0; - _legacyThreadingSemantics = legacyThreadingSemantics; + BuildResult ??= result; + + CheckForCompletion(); } - /// - /// The BuildManager with which this submission is associated. - /// - public BuildManager BuildManager { get; } + protected internal abstract TResultData CreateFailedResult(Exception exception); - /// - /// An ID uniquely identifying this request from among other submissions within the same build. - /// - public int SubmissionId { get; } + internal override BuildResultBase CompleteResultsWithException(Exception exception) + => CompleteResults(exception); - /// - /// The asynchronous context provided to , if any. - /// - public Object AsyncContext { get; private set; } + private TResultData CompleteResults(Exception exception) + { + TResultData result = CreateFailedResult(exception); + CompleteResults(result); + return result; + } /// - /// A which will be signalled when the build is complete. Valid after or returns, otherwise null. + /// Determines if we are completely done with this submission and can complete it so the user may access results. /// - public WaitHandle WaitHandle => _completionEvent; + protected internal override void CheckForCompletion() + { + if (BuildResult != null && LoggingCompleted) + { + bool hasCompleted = (Interlocked.Exchange(ref CompletionInvoked, 1) == 1); + if (!hasCompleted) + { + OnCompletition(); - /// - /// Returns true if this submission is complete. - /// - public bool IsCompleted => WaitHandle.WaitOne(new TimeSpan(0)); + CompletionEvent.Set(); + if (_completionCallback != null) + { + void Callback(object? state) + { + _completionCallback(this); + } + + ThreadPoolExtensions.QueueThreadPoolWorkItemWithCulture(Callback, CultureInfo.CurrentCulture, CultureInfo.CurrentUICulture); + } + } + } + } + } + + + /// + /// A callback used to receive notification that a build has completed. + /// + /// + /// When this delegate is invoked, the WaitHandle on the BuildSubmission will have been be signalled and the OverallBuildResult will be valid. + /// + public delegate void BuildSubmissionCompleteCallback(BuildSubmission submission); + + /// + /// A BuildSubmission represents a build request which has been submitted to the BuildManager for processing. It may be used to + /// execute synchronous or asynchronous build requests and provides access to the results upon completion. + /// + /// + /// This class is thread-safe. + /// + public class BuildSubmission : BuildSubmissionBase + { /// - /// The result of the build. Valid only after WaitHandle has become signalled. + /// Flag indicating whether synchronous wait should support legacy threading semantics. /// - public BuildResult BuildResult { get; set; } + private readonly bool _legacyThreadingSemantics; /// - /// The BuildRequestData being used for this submission. + /// The build request for execution. /// - internal BuildRequestData BuildRequestData { get; } + internal BuildRequest? BuildRequest { get; set; } + + internal BuildSubmission(BuildManager buildManager, int submissionId, BuildRequestData requestData, bool legacyThreadingSemantics) + : base(buildManager, submissionId, requestData) + { + _legacyThreadingSemantics = legacyThreadingSemantics; + } /// - /// The build request for execution. + /// Starts the request asynchronously and immediately returns control to the caller. /// - internal BuildRequest BuildRequest { get; set; } + /// The request has already been started or is already complete. + public void ExecuteAsync(BuildSubmissionCompleteCallback? callback, object? context) + { + void Clb(BuildSubmissionBase submission) + { + callback?.Invoke((BuildSubmission)submission); + } + + ExecuteAsync(Clb, context, allowMainThreadBuild: false); + } /// /// Starts the request and blocks until results are available. /// /// The request has already been started or is already complete. - public BuildResult Execute() + public override BuildResult Execute() { LegacyThreadingData legacyThreadingData = ((IBuildComponentHost)BuildManager).LegacyThreadingData; legacyThreadingData.RegisterSubmissionForLegacyThread(SubmissionId); @@ -131,89 +195,28 @@ public BuildResult Execute() legacyThreadingData.UnregisterSubmissionForLegacyThread(SubmissionId); - return BuildResult; - } - - /// - /// Starts the request asynchronously and immediately returns control to the caller. - /// - /// The request has already been started or is already complete. - public void ExecuteAsync(BuildSubmissionCompleteCallback callback, object context) - { - ExecuteAsync(callback, context, false); - } - - /// - /// Sets the event signaling that the build is complete. - /// - internal void CompleteResults(BuildResult result) - { - ErrorUtilities.VerifyThrowArgumentNull(result, nameof(result)); - - // We verify that we got results from the same configuration, but not necessarily the same request, because we are - // rather flexible in how users are allowed to submit multiple requests for the same configuration. In this case, the - // request id of the result will match the first request, even though it will contain results for all requests (including - // this one.) - ErrorUtilities.VerifyThrow(result.ConfigurationId == BuildRequest.ConfigurationId, "BuildResult doesn't match BuildRequest configuration"); + ErrorUtilities.VerifyThrow(BuildResult != null, + "BuildResult is not populated after Execute is done."); - if (BuildResult == null) - { - BuildResult = result; - } - - CheckForCompletion(); + return BuildResult!; } - /// - /// Indicates that all logging events for this submission are complete. - /// - internal void CompleteLogging() + protected internal override BuildResult CreateFailedResult(Exception exception) { - LoggingCompleted = true; - CheckForCompletion(); + ErrorUtilities.VerifyThrow(BuildRequest != null, + "BuildRequest is not populated while reporting failed result."); + return new(BuildRequest!, exception); } + - /// - /// Starts the request asynchronously and immediately returns control to the caller. - /// - /// The request has already been started or is already complete. - private void ExecuteAsync(BuildSubmissionCompleteCallback callback, object context, bool allowMainThreadBuild) + protected internal override void OnCompletition() { - ErrorUtilities.VerifyThrowInvalidOperation(!IsCompleted, "SubmissionAlreadyComplete"); - _completionCallback = callback; - AsyncContext = context; - BuildManager.ExecuteSubmission(this, allowMainThreadBuild); - } - - /// - /// Determines if we are completely done with this submission and can complete it so the user may access results. - /// - private void CheckForCompletion() - { - if (BuildResult != null && LoggingCompleted) + // Did this submission have warnings elevated to errors? If so, mark it as + // failed even though it succeeded (with warnings--but they're errors). + if (BuildResult != null && + ((IBuildComponentHost)BuildManager).LoggingService.HasBuildSubmissionLoggedErrors(BuildResult.SubmissionId)) { - bool hasCompleted = (Interlocked.Exchange(ref _completionInvoked, 1) == 1); - if (!hasCompleted) - { - // Did this submission have warnings elevated to errors? If so, mark it as - // failed even though it succeeded (with warnings--but they're errors). - if (((IBuildComponentHost)BuildManager).LoggingService.HasBuildSubmissionLoggedErrors(BuildResult.SubmissionId)) - { - BuildResult.SetOverallResult(overallResult: false); - } - - _completionEvent.Set(); - - if (_completionCallback != null) - { - void Callback(object state) - { - _completionCallback(this); - } - - ThreadPoolExtensions.QueueThreadPoolWorkItemWithCulture(Callback, CultureInfo.CurrentCulture, CultureInfo.CurrentUICulture); - } - } + BuildResult.SetOverallResult(overallResult: false); } } } diff --git a/src/Build/BackEnd/BuildManager/BuildSubmissionBase.cs b/src/Build/BackEnd/BuildManager/BuildSubmissionBase.cs new file mode 100644 index 00000000000..ba6f45afc09 --- /dev/null +++ b/src/Build/BackEnd/BuildManager/BuildSubmissionBase.cs @@ -0,0 +1,96 @@ +// 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.Linq; +using System.Text; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.Build.Shared; + +namespace Microsoft.Build.Execution +{ + public abstract class BuildSubmissionBase + { + /// + /// The completion event. + /// + protected readonly ManualResetEvent CompletionEvent; + + /// + /// Flag indicating if logging is done. + /// + internal bool LoggingCompleted { get; private set; } + + /// + /// True if it has been invoked + /// + protected int CompletionInvoked; + + // + // Unfortunately covariant overrides are not available for .NET 472, + // so we have to use two set of properties for derived classes. + internal abstract BuildRequestDataBase BuildRequestDataBase { get; } + + internal abstract BuildResultBase? BuildResultBase { get; } + + /// + /// Constructor + /// + protected internal BuildSubmissionBase(BuildManager buildManager, int submissionId) + { + ErrorUtilities.VerifyThrowArgumentNull(buildManager, nameof(buildManager)); + + BuildManager = buildManager; + SubmissionId = submissionId; + CompletionEvent = new ManualResetEvent(false); + LoggingCompleted = false; + CompletionInvoked = 0; + } + + /// + /// The BuildManager with which this submission is associated. + /// + public BuildManager BuildManager { get; } + + /// + /// An ID uniquely identifying this request from among other submissions within the same build. + /// + public int SubmissionId { get; } + + /// + /// The asynchronous context provided to , if any. + /// + public object? AsyncContext { get; protected set; } + + /// + /// A which will be signalled when the build is complete. Valid after or returns, otherwise null. + /// + public WaitHandle WaitHandle => CompletionEvent; + + /// + /// Returns true if this submission is complete. + /// + public bool IsCompleted => WaitHandle.WaitOne(new TimeSpan(0)); + + /// + /// Whether the build has started. + /// + internal bool IsStarted { get; set; } + + /// + /// Indicates that all logging events for this submission are complete. + /// + internal void CompleteLogging() + { + LoggingCompleted = true; + CheckForCompletion(); + } + + protected internal virtual void OnCompletition() { } + protected internal abstract void CheckForCompletion(); + + internal abstract BuildResultBase CompleteResultsWithException(Exception exception); + } +} diff --git a/src/Build/BackEnd/BuildManager/GlobalPropertiesLookup.cs b/src/Build/BackEnd/BuildManager/GlobalPropertiesLookup.cs new file mode 100644 index 00000000000..1cf11aa2068 --- /dev/null +++ b/src/Build/BackEnd/BuildManager/GlobalPropertiesLookup.cs @@ -0,0 +1,64 @@ +// 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; +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Linq; +using System.Text; +using System.Threading.Tasks; +using Microsoft.Build.Collections; + +namespace Microsoft.Build.Execution +{ + internal class GlobalPropertiesLookup : IReadOnlyDictionary + { + internal static IReadOnlyDictionary ToGlobalPropertiesLookup( + PropertyDictionary? backing) + { + if (backing == null) + { + return ImmutableDictionary.Empty; + } + + return new GlobalPropertiesLookup(backing); + } + + private GlobalPropertiesLookup(IDictionary backingProperties) + { + _backingProperties = backingProperties; + } + + private readonly IDictionary _backingProperties; + + public IEnumerator> GetEnumerator() + => _backingProperties + .Select(p => new KeyValuePair(p.Key, ExtractEscapedValue(p.Value))) + .GetEnumerator(); + + IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); + + public int Count => _backingProperties.Count; + public bool ContainsKey(string key) => _backingProperties.ContainsKey(key); + + public bool TryGetValue(string key, out string? value) + { + if (_backingProperties.TryGetValue(key, out var property)) + { + value = ExtractEscapedValue(property); + return true; + } + + value = null; + return false; + } + + public string? this[string key] => ExtractEscapedValue(_backingProperties[key]); + + public IEnumerable Keys => _backingProperties.Keys; + public IEnumerable Values => _backingProperties.Values.Select(ExtractEscapedValue); + + private static string? ExtractEscapedValue(ProjectPropertyInstance property) => ((IValued)property).EscapedValue; + } +} diff --git a/src/Build/BackEnd/Components/BuildComponentFactoryCollection.cs b/src/Build/BackEnd/Components/BuildComponentFactoryCollection.cs index dd833490899..7262bb0d329 100644 --- a/src/Build/BackEnd/Components/BuildComponentFactoryCollection.cs +++ b/src/Build/BackEnd/Components/BuildComponentFactoryCollection.cs @@ -162,6 +162,11 @@ public IBuildComponent GetComponent(BuildComponentType type) return componentEntry.GetInstance(_host); } + internal TComponent GetComponent(BuildComponentType type) where TComponent : IBuildComponent + { + return (TComponent)GetComponent(type); + } + /// /// A helper class wrapping build components. /// diff --git a/src/Build/BackEnd/Components/FileAccesses/FileAccessManager.cs b/src/Build/BackEnd/Components/FileAccesses/FileAccessManager.cs index 6943ff208e0..de28c100d1d 100644 --- a/src/Build/BackEnd/Components/FileAccesses/FileAccessManager.cs +++ b/src/Build/BackEnd/Components/FileAccesses/FileAccessManager.cs @@ -14,7 +14,7 @@ namespace Microsoft.Build.FileAccesses { - internal sealed class FileAccessManager : IFileAccessManager, IBuildComponent + internal sealed class FileAccessManager : IFileAccessManager { private record Handlers(Action FileAccessHander, Action ProcessHandler); diff --git a/src/Build/BackEnd/Components/FileAccesses/IFileAccessManager.cs b/src/Build/BackEnd/Components/FileAccesses/IFileAccessManager.cs index dd3b7685053..14c38c0b590 100644 --- a/src/Build/BackEnd/Components/FileAccesses/IFileAccessManager.cs +++ b/src/Build/BackEnd/Components/FileAccesses/IFileAccessManager.cs @@ -9,7 +9,7 @@ namespace Microsoft.Build.FileAccesses { - internal interface IFileAccessManager + internal interface IFileAccessManager : IBuildComponent { void ReportFileAccess(FileAccessData fileAccessData, int nodeId); diff --git a/src/Build/BackEnd/Components/FileAccesses/OutOfProcNodeFileAccessManager.cs b/src/Build/BackEnd/Components/FileAccesses/OutOfProcNodeFileAccessManager.cs index e211f674789..c1f7df47679 100644 --- a/src/Build/BackEnd/Components/FileAccesses/OutOfProcNodeFileAccessManager.cs +++ b/src/Build/BackEnd/Components/FileAccesses/OutOfProcNodeFileAccessManager.cs @@ -13,7 +13,7 @@ namespace Microsoft.Build.FileAccesses /// /// Reports file accesses and process data to the in-proc node. /// - internal sealed class OutOfProcNodeFileAccessManager : IFileAccessManager, IBuildComponent + internal sealed class OutOfProcNodeFileAccessManager : IFileAccessManager { /// /// The to report file accesses and process diff --git a/src/Build/BackEnd/Components/IBuildComponentHost.cs b/src/Build/BackEnd/Components/IBuildComponentHost.cs index 8b2ded4d251..7983229f5b2 100644 --- a/src/Build/BackEnd/Components/IBuildComponentHost.cs +++ b/src/Build/BackEnd/Components/IBuildComponentHost.cs @@ -195,6 +195,15 @@ internal interface IBuildComponentHost /// The component IBuildComponent GetComponent(BuildComponentType type); + /// + /// Gets an instance of the specified component type from the host. + /// + /// + /// The component type to be retrieved + /// The component + TComponent GetComponent(BuildComponentType type) + where TComponent : IBuildComponent; + #endregion } } diff --git a/src/Build/BackEnd/Components/Logging/ILoggingService.cs b/src/Build/BackEnd/Components/Logging/ILoggingService.cs index 583f2b7d99b..1bb38687ca3 100644 --- a/src/Build/BackEnd/Components/Logging/ILoggingService.cs +++ b/src/Build/BackEnd/Components/Logging/ILoggingService.cs @@ -27,7 +27,7 @@ namespace Microsoft.Build.BackEnd.Logging /// Interface representing logging services in the build system. /// Implementations should be thread-safe. /// - internal interface ILoggingService + internal interface ILoggingService : IBuildComponent { #region Events /// diff --git a/src/Build/BackEnd/Components/Logging/LoggingService.cs b/src/Build/BackEnd/Components/Logging/LoggingService.cs index a70ab4f9fb4..0c7866e53a9 100644 --- a/src/Build/BackEnd/Components/Logging/LoggingService.cs +++ b/src/Build/BackEnd/Components/Logging/LoggingService.cs @@ -69,7 +69,7 @@ internal enum LoggingServiceState /// /// Logging services is used as a helper class to assist logging messages in getting to the correct loggers. /// - internal partial class LoggingService : ILoggingService, INodePacketHandler, IBuildComponent + internal partial class LoggingService : ILoggingService, INodePacketHandler { /// /// The default maximum size for the logging event queue. diff --git a/src/Build/BackEnd/Components/ProjectCache/ProjectCacheService.cs b/src/Build/BackEnd/Components/ProjectCache/ProjectCacheService.cs index e53d28292d1..d573f4bdc74 100644 --- a/src/Build/BackEnd/Components/ProjectCache/ProjectCacheService.cs +++ b/src/Build/BackEnd/Components/ProjectCache/ProjectCacheService.cs @@ -454,7 +454,7 @@ public void PostCacheRequest(CacheRequest cacheRequest, CancellationToken cancel BuildRequestData buildRequest = new BuildRequestData( cacheRequest.Configuration.Project, - cacheRequest.Submission.BuildRequestData.TargetNames.ToArray()); + cacheRequest.Submission.BuildRequestData?.TargetNames.ToArray() ?? Array.Empty()); BuildEventContext buildEventContext = _loggingService.CreateProjectCacheBuildEventContext( cacheRequest.Submission.SubmissionId, evaluationId: cacheRequest.Configuration.Project.EvaluationId, @@ -477,13 +477,16 @@ public void PostCacheRequest(CacheRequest cacheRequest, CancellationToken cancel void EvaluateProjectIfNecessary(BuildSubmission submission, BuildRequestConfiguration configuration) { + ErrorUtilities.VerifyThrow(submission.BuildRequestData != null, + "Submission BuildRequestData is not populated."); + lock (configuration) { if (!configuration.IsLoaded) { configuration.LoadProjectIntoConfiguration( _buildManager, - submission.BuildRequestData.Flags, + submission.BuildRequestData!.Flags, submission.SubmissionId, Scheduler.InProcNodeId); @@ -519,7 +522,7 @@ private async Task GetCacheResultAsync(BuildRequestData buildReques HashSet queriedCaches = new(ProjectCacheDescriptorEqualityComparer.Instance); CacheResult? cacheResult = null; - foreach (ProjectCacheDescriptor projectCacheDescriptor in GetProjectCacheDescriptors(buildRequest.ProjectInstance)) + foreach (ProjectCacheDescriptor projectCacheDescriptor in GetProjectCacheDescriptors(buildRequest.ProjectInstance!)) { // Ensure each unique plugin is only queried once if (!queriedCaches.Add(projectCacheDescriptor)) @@ -583,7 +586,7 @@ private async Task GetCacheResultAsync(BuildRequestData buildReques // TODO: This should be indented by the console logger. That requires making these log events structured. if (!buildRequestConfiguration.IsTraversal) { - _loggingService.LogComment(buildEventContext, MessageImportance.High, "ProjectCacheHitWithOutputs", buildRequest.ProjectInstance.GetPropertyValue(ReservedPropertyNames.projectName)); + _loggingService.LogComment(buildEventContext, MessageImportance.High, "ProjectCacheHitWithOutputs", buildRequest.ProjectInstance!.GetPropertyValue(ReservedPropertyNames.projectName)); } break; @@ -734,7 +737,7 @@ public async Task HandleBuildResultAsync( IReadOnlyDictionary globalProperties = GetGlobalProperties(requestConfiguration); - List targets = buildResult.ResultsByTarget.Keys.ToList(); + List targets = buildResult.ResultsByTarget?.Keys.ToList() ?? new(); string? targetNames = string.Join(", ", targets); FileAccessContext fileAccessContext = new(requestConfiguration.ProjectFullPath, globalProperties, targets); diff --git a/src/Build/BackEnd/Node/OutOfProcNode.cs b/src/Build/BackEnd/Node/OutOfProcNode.cs index 51ac31c32dd..3d7c50595d4 100644 --- a/src/Build/BackEnd/Node/OutOfProcNode.cs +++ b/src/Build/BackEnd/Node/OutOfProcNode.cs @@ -303,6 +303,9 @@ IBuildComponent IBuildComponentHost.GetComponent(BuildComponentType type) return _componentFactories.GetComponent(type); } + TComponent IBuildComponentHost.GetComponent(BuildComponentType type) + => (TComponent)((IBuildComponentHost)this).GetComponent(type); + #endregion #region INodePacketFactory Members diff --git a/src/Build/BackEnd/Shared/BuildResult.cs b/src/Build/BackEnd/Shared/BuildResult.cs index 208fa2e7b9a..56458bc630c 100644 --- a/src/Build/BackEnd/Shared/BuildResult.cs +++ b/src/Build/BackEnd/Shared/BuildResult.cs @@ -10,8 +10,6 @@ using Microsoft.Build.Shared; using Microsoft.Build.Shared.FileSystem; -#nullable disable - namespace Microsoft.Build.Execution { /// @@ -33,7 +31,7 @@ public enum BuildResultCode /// /// Contains the current results for all of the targets which have produced results for a particular configuration. /// - public class BuildResult : INodePacket, IBuildResults + public class BuildResult : BuildResultBase, INodePacket, IBuildResults { /// /// The submission with which this result is associated. @@ -64,13 +62,13 @@ public class BuildResult : INodePacket, IBuildResults /// The first build request to generate results for a configuration will set this so that future /// requests may be properly satisfied from the cache. /// - private List _initialTargets; + private List? _initialTargets; /// /// The first build request to generate results for a configuration will set this so that future /// requests may be properly satisfied from the cache. /// - private List _defaultTargets; + private List? _defaultTargets; /// /// The set of results for each target. @@ -87,7 +85,7 @@ public class BuildResult : INodePacket, IBuildResults /// Note that this can be set if the request itself fails, or if it receives /// an exception from a target or task. /// - private Exception _requestException; + private Exception? _requestException; /// /// The overall result calculated in the constructor. @@ -98,13 +96,13 @@ public class BuildResult : INodePacket, IBuildResults /// Snapshot of the environment from the configuration this results comes from. /// This should only be populated when the configuration for this result is moved between nodes. /// - private Dictionary _savedEnvironmentVariables; + private Dictionary? _savedEnvironmentVariables; /// /// Snapshot of the current directory from the configuration this result comes from. /// This should only be populated when the configuration for this result is moved between nodes. /// - private string _savedCurrentDirectory; + private string? _savedCurrentDirectory; /// /// state after the build. This is only provided if @@ -114,22 +112,23 @@ public class BuildResult : INodePacket, IBuildResults /// be used to retrieve , and /// from it. No other operation is guaranteed to be supported. /// - private ProjectInstance _projectStateAfterBuild; + private ProjectInstance? _projectStateAfterBuild; /// /// The flags provide additional control over the build results and may affect the cached value. /// private BuildRequestDataFlags _buildRequestDataFlags; - private string _schedulerInducedError; + private string? _schedulerInducedError; - private HashSet _projectTargets; + private HashSet? _projectTargets; /// /// Constructor for serialization. /// public BuildResult() { + _resultsByTarget = CreateTargetResultDictionary(1); } /// @@ -146,7 +145,7 @@ internal BuildResult(BuildRequest request) /// /// The build request to which these results should be associated. /// The exception, if any. - internal BuildResult(BuildRequest request, Exception exception) + internal BuildResult(BuildRequest request, Exception? exception) : this(request, null, exception) { } @@ -189,7 +188,7 @@ internal BuildResult(BuildResult existingResults, string[] targetNames) /// The build request with which these results should be associated. /// The existing results, if any. /// The exception, if any - internal BuildResult(BuildRequest request, BuildResult existingResults, Exception exception) + internal BuildResult(BuildRequest request, BuildResult? existingResults, Exception? exception) : this(request, existingResults, null, exception) { } @@ -201,9 +200,8 @@ internal BuildResult(BuildRequest request, BuildResult existingResults, Exceptio /// The existing results, if any. /// The list of target names that are the subset of results that should be returned. /// The exception, if any - internal BuildResult(BuildRequest request, BuildResult existingResults, string[] targetNames, Exception exception) + internal BuildResult(BuildRequest request, BuildResult? existingResults, string[]? targetNames, Exception? exception) { - ErrorUtilities.VerifyThrow(request != null, "Must specify a request."); _submissionId = request.SubmissionId; _configurationId = request.ConfigurationId; _globalRequestId = request.GlobalRequestId; @@ -270,12 +268,13 @@ internal BuildResult(BuildResult result, int submissionId, int configurationId, private BuildResult(ITranslator translator) { ((ITranslatable)this).Translate(translator); + _resultsByTarget ??= CreateTargetResultDictionary(1); } /// /// Returns the submission id. /// - public int SubmissionId + public override int SubmissionId { [DebuggerStepThrough] get @@ -325,7 +324,7 @@ public int NodeRequestId /// /// Returns the exception generated while this result was run, if any. /// - public Exception Exception + public override Exception? Exception { [DebuggerStepThrough] get @@ -339,7 +338,7 @@ internal set /// /// Returns a flag indicating if a circular dependency was detected. /// - public bool CircularDependency + public override bool CircularDependency { [DebuggerStepThrough] get @@ -349,7 +348,7 @@ public bool CircularDependency /// /// Returns the overall result for this result set. /// - public BuildResultCode OverallResult + public override BuildResultCode OverallResult { get { @@ -358,7 +357,7 @@ public BuildResultCode OverallResult return BuildResultCode.Failure; } - foreach (KeyValuePair result in _resultsByTarget) + foreach (KeyValuePair result in _resultsByTarget ?? Enumerable.Empty>()) { if ((result.Value.ResultCode == TargetResultCode.Failure && !result.Value.TargetFailureDoesntCauseBuildFailure) || result.Value.AfterTargetsHaveFailed) @@ -386,7 +385,7 @@ public IDictionary ResultsByTarget /// be used to retrieve , and /// from it. Any other operation is not guaranteed to be supported. /// - public ProjectInstance ProjectStateAfterBuild + public ProjectInstance? ProjectStateAfterBuild { get => _projectStateAfterBuild; set => _projectStateAfterBuild = value; @@ -411,7 +410,7 @@ NodePacketType INodePacket.Type /// /// Holds a snapshot of the environment at the time we blocked. /// - Dictionary IBuildResults.SavedEnvironmentVariables + Dictionary? IBuildResults.SavedEnvironmentVariables { get => _savedEnvironmentVariables; @@ -421,7 +420,7 @@ Dictionary IBuildResults.SavedEnvironmentVariables /// /// Holds a snapshot of the current working directory at the time we blocked. /// - string IBuildResults.SavedCurrentDirectory + string? IBuildResults.SavedCurrentDirectory { get => _savedCurrentDirectory; @@ -431,7 +430,7 @@ string IBuildResults.SavedCurrentDirectory /// /// Returns the initial targets for the configuration which requested these results. /// - internal List InitialTargets + internal List? InitialTargets { [DebuggerStepThrough] get @@ -445,7 +444,7 @@ internal List InitialTargets /// /// Returns the default targets for the configuration which requested these results. /// - internal List DefaultTargets + internal List? DefaultTargets { [DebuggerStepThrough] get @@ -459,7 +458,7 @@ internal List DefaultTargets /// /// The defined targets for the project associated with this build result. /// - internal HashSet ProjectTargets + internal HashSet? ProjectTargets { [DebuggerStepThrough] get => _projectTargets; @@ -471,7 +470,7 @@ internal HashSet ProjectTargets /// Container used to transport errors from the scheduler (issued while computing a build result) /// to the TaskHost that has the proper logging context (project id, target id, task id, file location) /// - internal string SchedulerInducedError + internal string? SchedulerInducedError { get => _schedulerInducedError; set => _schedulerInducedError = value; @@ -488,7 +487,7 @@ public ITargetResult this[string target] { [DebuggerStepThrough] get - { return _resultsByTarget[target]; } + { return _resultsByTarget![target]; } } /// @@ -506,7 +505,7 @@ public void AddResultsForTarget(string target, TargetResult result) _resultsByTarget ??= CreateTargetResultDictionary(1); } - if (_resultsByTarget.TryGetValue(target, out TargetResult targetResult)) + if (_resultsByTarget.TryGetValue(target, out TargetResult? targetResult)) { ErrorUtilities.VerifyThrow(targetResult.ResultCode == TargetResultCode.Skipped, "Items already exist for target {0}.", target); } @@ -524,11 +523,11 @@ internal void KeepSpecificTargetResults(IReadOnlyCollection targetsToKee targetsToKeep.Count > 0, $"{nameof(targetsToKeep)} should contain at least one target."); - foreach (string target in _resultsByTarget.Keys) + foreach (string target in _resultsByTarget?.Keys ?? Enumerable.Empty()) { if (!targetsToKeep.Contains(target)) { - _ = _resultsByTarget.TryRemove(target, out _); + _ = _resultsByTarget!.TryRemove(target, out _); } } } @@ -549,7 +548,7 @@ public void MergeResults(BuildResult results) } // Merge in the results - foreach (KeyValuePair targetResult in results._resultsByTarget) + foreach (KeyValuePair targetResult in results._resultsByTarget ?? Enumerable.Empty>()) { // NOTE: I believe that because we only allow results for a given target to be produced and cached once for a given configuration, // we can never receive conflicting results for that target, since the cache and build request manager would always return the @@ -559,7 +558,7 @@ public void MergeResults(BuildResult results) // ErrorUtilities.VerifyThrow(!HasResultsForTarget(targetResult.Key), "Results already exist"); // Copy the new results in. - _resultsByTarget[targetResult.Key] = targetResult.Value; + _resultsByTarget![targetResult.Key] = targetResult.Value; } // If there is an exception and we did not previously have one, add it in. @@ -573,7 +572,7 @@ public void MergeResults(BuildResult results) /// True if results exist, false otherwise. public bool HasResultsForTarget(string target) { - return _resultsByTarget.ContainsKey(target); + return _resultsByTarget?.ContainsKey(target) ?? false; } #region INodePacket Members @@ -617,7 +616,7 @@ internal static BuildResult FactoryForDeserialization(ITranslator translator) /// internal void CacheIfPossible() { - foreach (KeyValuePair targetResultPair in _resultsByTarget) + foreach (KeyValuePair targetResultPair in _resultsByTarget ?? Enumerable.Empty>()) { targetResultPair.Value.CacheItems(ConfigurationId, targetResultPair.Key); } @@ -648,9 +647,7 @@ internal BuildResult Clone() _parentGlobalRequestId = _parentGlobalRequestId, _nodeRequestId = _nodeRequestId, _requestException = _requestException, - _resultsByTarget = new ConcurrentDictionary( - _resultsByTarget, - StringComparer.OrdinalIgnoreCase), + _resultsByTarget = new ConcurrentDictionary(_resultsByTarget, StringComparer.OrdinalIgnoreCase), _baseOverallResult = OverallResult == BuildResultCode.Success, _circularDependency = _circularDependency }; @@ -685,7 +682,7 @@ private static ConcurrentDictionary CreateTargetResultDict foreach (string target in targetNames) { - if (existingResults.ResultsByTarget.TryGetValue(target, out TargetResult targetResult)) + if (existingResults.ResultsByTarget?.TryGetValue(target, out TargetResult? targetResult) ?? false) { resultsByTarget[target] = targetResult; } diff --git a/src/Build/BackEnd/Shared/BuildResultBase.cs b/src/Build/BackEnd/Shared/BuildResultBase.cs new file mode 100644 index 00000000000..f144bcb778c --- /dev/null +++ b/src/Build/BackEnd/Shared/BuildResultBase.cs @@ -0,0 +1,30 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; + +namespace Microsoft.Build.Execution +{ + public abstract class BuildResultBase + { + /// + /// Returns the submission id. + /// + public abstract int SubmissionId { get; } + + /// + /// Returns a flag indicating if a circular dependency was detected. + /// + public abstract bool CircularDependency { get; } + + /// + /// Returns the exception generated while this result was run, if any. + /// + public abstract Exception? Exception { get; internal set; } + + /// + /// Returns the overall result for this result set. + /// + public abstract BuildResultCode OverallResult { get; } + } +} diff --git a/src/Build/BuildCheck/Utilities/EnumerableExtensions.cs b/src/Build/BuildCheck/Utilities/EnumerableExtensions.cs index 96efc8ff2fd..f89f72b0e2c 100644 --- a/src/Build/BuildCheck/Utilities/EnumerableExtensions.cs +++ b/src/Build/BuildCheck/Utilities/EnumerableExtensions.cs @@ -3,9 +3,7 @@ using System; using System.Collections.Generic; -using System.Linq; -using System.Text; -using System.Threading.Tasks; +using System.Collections.ObjectModel; namespace Microsoft.Build.Experimental.BuildCheck; @@ -23,6 +21,26 @@ public static string ToCsvString(this IEnumerable? source, bool useSpace = return source == null ? "" : string.Join(useSpace ? ", " : ",", source); } + /// + /// Returns the item as an enumerable with single item. + /// + /// + /// + /// + public static IEnumerable AsSingleItemEnumerable(this T item) + { + yield return item; + } + +#if !NET + /// + /// Returns a read-only wrapper + /// for the current dictionary. + /// + public static ReadOnlyDictionary AsReadOnly(this IDictionary dictionary) + => new(dictionary); +#endif + /// /// Adds a content of given dictionary to current dictionary. /// diff --git a/src/Build/Graph/GraphBuildRequestData.cs b/src/Build/Graph/GraphBuildRequestData.cs index c09a587b106..7a173627c99 100644 --- a/src/Build/Graph/GraphBuildRequestData.cs +++ b/src/Build/Graph/GraphBuildRequestData.cs @@ -2,11 +2,12 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Generic; +using System.Collections.Immutable; +using System.Linq; using Microsoft.Build.Execution; +using Microsoft.Build.Experimental.BuildCheck; using Microsoft.Build.Shared; -#nullable disable - namespace Microsoft.Build.Graph { public record GraphBuildOptions @@ -20,7 +21,7 @@ public record GraphBuildOptions /// /// GraphBuildRequestData encapsulates all of the data needed to submit a graph build request. /// - public sealed class GraphBuildRequestData + public sealed class GraphBuildRequestData : BuildRequestData { /// /// Constructs a GraphBuildRequestData for build requests based on a project graph. @@ -38,7 +39,7 @@ public GraphBuildRequestData(ProjectGraph projectGraph, ICollection targ /// The graph to build. /// The targets to build. /// The host services to use, if any. May be null. - public GraphBuildRequestData(ProjectGraph projectGraph, ICollection targetsToBuild, HostServices hostServices) + public GraphBuildRequestData(ProjectGraph projectGraph, ICollection targetsToBuild, HostServices? hostServices) : this(projectGraph, targetsToBuild, hostServices, BuildRequestDataFlags.None) { } @@ -50,7 +51,7 @@ public GraphBuildRequestData(ProjectGraph projectGraph, ICollection targ /// The targets to build. /// The host services to use, if any. May be null. /// Flags controlling this build request. - public GraphBuildRequestData(ProjectGraph projectGraph, ICollection targetsToBuild, HostServices hostServices, BuildRequestDataFlags flags) + public GraphBuildRequestData(ProjectGraph projectGraph, ICollection targetsToBuild, HostServices? hostServices, BuildRequestDataFlags flags) : this(targetsToBuild, hostServices, flags) { ErrorUtilities.VerifyThrowArgumentNull(projectGraph, nameof(projectGraph)); @@ -65,7 +66,7 @@ public GraphBuildRequestData(ProjectGraph projectGraph, ICollection targ /// The global properties which should be used during evaluation of the project. Cannot be null. /// The targets to build. /// The host services to use. May be null. - public GraphBuildRequestData(string projectFullPath, IDictionary globalProperties, ICollection targetsToBuild, HostServices hostServices) + public GraphBuildRequestData(string projectFullPath, IDictionary globalProperties, ICollection targetsToBuild, HostServices? hostServices) : this(new ProjectGraphEntryPoint(projectFullPath, globalProperties).AsEnumerable(), targetsToBuild, hostServices, BuildRequestDataFlags.None) { } @@ -78,7 +79,7 @@ public GraphBuildRequestData(string projectFullPath, IDictionary /// The targets to build. /// The host services to use. May be null. /// The to use. - public GraphBuildRequestData(string projectFullPath, IDictionary globalProperties, ICollection targetsToBuild, HostServices hostServices, BuildRequestDataFlags flags) + public GraphBuildRequestData(string projectFullPath, IDictionary globalProperties, ICollection targetsToBuild, HostServices? hostServices, BuildRequestDataFlags flags) : this(new ProjectGraphEntryPoint(projectFullPath, globalProperties).AsEnumerable(), targetsToBuild, hostServices, flags) { } @@ -99,7 +100,7 @@ public GraphBuildRequestData(ProjectGraphEntryPoint projectGraphEntryPoint, ICol /// The entry point to use in the build. /// The targets to build. /// The host services to use, if any. May be null. - public GraphBuildRequestData(ProjectGraphEntryPoint projectGraphEntryPoint, ICollection targetsToBuild, HostServices hostServices) + public GraphBuildRequestData(ProjectGraphEntryPoint projectGraphEntryPoint, ICollection targetsToBuild, HostServices? hostServices) : this(projectGraphEntryPoint.AsEnumerable(), targetsToBuild, hostServices, BuildRequestDataFlags.None) { } @@ -111,7 +112,7 @@ public GraphBuildRequestData(ProjectGraphEntryPoint projectGraphEntryPoint, ICol /// The targets to build. /// The host services to use, if any. May be null. /// Flags controlling this build request. - public GraphBuildRequestData(ProjectGraphEntryPoint projectGraphEntryPoint, ICollection targetsToBuild, HostServices hostServices, BuildRequestDataFlags flags) + public GraphBuildRequestData(ProjectGraphEntryPoint projectGraphEntryPoint, ICollection targetsToBuild, HostServices? hostServices, BuildRequestDataFlags flags) : this(projectGraphEntryPoint.AsEnumerable(), targetsToBuild, hostServices, flags) { } @@ -132,7 +133,7 @@ public GraphBuildRequestData(IEnumerable projectGraphEnt /// The entry points to use in the build. /// The targets to build. /// The host services to use, if any. May be null. - public GraphBuildRequestData(IEnumerable projectGraphEntryPoints, ICollection targetsToBuild, HostServices hostServices) + public GraphBuildRequestData(IEnumerable projectGraphEntryPoints, ICollection targetsToBuild, HostServices? hostServices) : this(projectGraphEntryPoints, targetsToBuild, hostServices, BuildRequestDataFlags.None) { } @@ -144,7 +145,7 @@ public GraphBuildRequestData(IEnumerable projectGraphEnt /// The targets to build. /// The host services to use, if any. May be null. /// Flags controlling this build request. - public GraphBuildRequestData(IEnumerable projectGraphEntryPoints, ICollection targetsToBuild, HostServices hostServices, BuildRequestDataFlags flags) + public GraphBuildRequestData(IEnumerable projectGraphEntryPoints, ICollection targetsToBuild, HostServices? hostServices, BuildRequestDataFlags flags) : this(targetsToBuild, hostServices, flags) { ErrorUtilities.VerifyThrowArgumentNull(projectGraphEntryPoints, nameof(projectGraphEntryPoints)); @@ -152,27 +153,20 @@ public GraphBuildRequestData(IEnumerable projectGraphEnt ProjectGraphEntryPoints = projectGraphEntryPoints; } - public GraphBuildRequestData(IEnumerable projectGraphEntryPoints, ICollection targetsToBuild, HostServices hostServices, BuildRequestDataFlags flags, GraphBuildOptions graphBuildOptions) + public GraphBuildRequestData(IEnumerable projectGraphEntryPoints, ICollection targetsToBuild, HostServices? hostServices, BuildRequestDataFlags flags, GraphBuildOptions graphBuildOptions) : this(targetsToBuild, hostServices, flags, graphBuildOptions) { ErrorUtilities.VerifyThrowArgumentNull(projectGraphEntryPoints, nameof(projectGraphEntryPoints)); - ProjectGraphEntryPoints = projectGraphEntryPoints; + ProjectGraphEntryPoints = projectGraphEntryPoints.ToList(); } /// /// Common constructor. /// - private GraphBuildRequestData(ICollection targetsToBuild, HostServices hostServices, BuildRequestDataFlags flags, GraphBuildOptions graphBuildOptions = null) + private GraphBuildRequestData(ICollection targetsToBuild, HostServices? hostServices, BuildRequestDataFlags flags, GraphBuildOptions? graphBuildOptions = null) + : base(targetsToBuild, flags, hostServices) { - ErrorUtilities.VerifyThrowArgumentNull(targetsToBuild, nameof(targetsToBuild)); - foreach (string targetName in targetsToBuild) - { - ErrorUtilities.VerifyThrowArgumentNull(targetName, "target"); - } - - HostServices = hostServices; - TargetNames = new List(targetsToBuild); Flags = flags; GraphBuildOptions = graphBuildOptions ?? new GraphBuildOptions(); } @@ -182,34 +176,64 @@ private GraphBuildRequestData(ICollection targetsToBuild, HostServices h /// May be null. /// /// The project graph. - public ProjectGraph ProjectGraph { get; } + public ProjectGraph? ProjectGraph { get; } /// /// The project graph entry points. /// May be null. /// /// The project graph entry points. - public IEnumerable ProjectGraphEntryPoints { get; } + public IEnumerable? ProjectGraphEntryPoints { get; } - /// - /// The name of the targets to build. - /// - /// An array of targets in the project to be built. - public ICollection TargetNames { get; } + internal override BuildSubmissionBase CreateSubmission(BuildManager buildManager, int submissionId, GraphBuildRequestData requestData, + bool legacyThreadingSemantics) => + new GraphBuildSubmission(buildManager, submissionId, requestData); - /// - /// Extra flags for this BuildRequest. - /// - public BuildRequestDataFlags Flags { get; } + public override IEnumerable EntryProjectsFullPath + { + get + { + if (ProjectGraph != null) + { + foreach (ProjectGraphNode entryPoint in ProjectGraph.EntryPointNodes) + { + yield return entryPoint.ProjectInstance.FullPath; + } + } + else if (ProjectGraphEntryPoints != null) + { + foreach (ProjectGraphEntryPoint entryPoint in ProjectGraphEntryPoints) + { + yield return entryPoint.ProjectFile; + } + } + } + } + + public override IReadOnlyDictionary GlobalPropertiesLookup + { + get + { + ProjectGraphNode? node = ProjectGraph?.EntryPointNodes.FirstOrDefault(); + if (node != null) + { + return node.ProjectInstance.GlobalProperties.AsReadOnly(); + } + + ProjectGraphEntryPoint? entryPoint = ProjectGraphEntryPoints?.FirstOrDefault(); + if (entryPoint != null) + { + return entryPoint.Value.GlobalProperties.AsReadOnly(); + } + + return ImmutableDictionary.Empty; + } + } + public override bool IsGraphRequest => true; /// /// Options for how the graph should be built. /// public GraphBuildOptions GraphBuildOptions { get; } - - /// - /// Gets the HostServices object for this request. - /// - public HostServices HostServices { get; } } } diff --git a/src/Build/Graph/GraphBuildResult.cs b/src/Build/Graph/GraphBuildResult.cs index 6df04a0c899..c13ea8ce105 100644 --- a/src/Build/Graph/GraphBuildResult.cs +++ b/src/Build/Graph/GraphBuildResult.cs @@ -3,14 +3,13 @@ using System; using System.Collections.Generic; +using System.Collections.Immutable; using Microsoft.Build.Exceptions; using Microsoft.Build.Execution; -#nullable disable - namespace Microsoft.Build.Graph { - public sealed class GraphBuildResult + public sealed class GraphBuildResult : BuildResultBase { /// /// Constructor creates a build result with results for each graph node. @@ -32,27 +31,28 @@ internal GraphBuildResult(int submissionId, Exception exception) { SubmissionId = submissionId; Exception = exception; + ResultsByNode = ImmutableDictionary.Empty; } /// /// Returns the submission id. /// - public int SubmissionId { get; } + public override int SubmissionId { get; } /// /// Returns a flag indicating if a circular dependency was detected. /// - public bool CircularDependency => Exception is CircularDependencyException; + public override bool CircularDependency => Exception is CircularDependencyException; /// /// Returns the exception generated while this result was run, if any. /// - public Exception Exception { get; internal set; } + public override Exception? Exception { get; internal set; } /// /// Returns the overall result for this result set. /// - public BuildResultCode OverallResult + public override BuildResultCode OverallResult { get { diff --git a/src/Build/Graph/GraphBuildSubmission.cs b/src/Build/Graph/GraphBuildSubmission.cs index 9c00b5bb0e4..6e45cc0231d 100644 --- a/src/Build/Graph/GraphBuildSubmission.cs +++ b/src/Build/Graph/GraphBuildSubmission.cs @@ -4,11 +4,10 @@ using System; using System.Globalization; using System.Threading; +using Microsoft.Build.BackEnd; using Microsoft.Build.Execution; using Microsoft.Build.Shared; -#nullable disable - namespace Microsoft.Build.Graph { /// @@ -26,126 +25,44 @@ namespace Microsoft.Build.Graph /// /// This class is thread-safe. /// - public class GraphBuildSubmission + public class GraphBuildSubmission : BuildSubmissionBase { - /// - /// The callback to invoke when the submission is complete. - /// - private GraphBuildSubmissionCompleteCallback _completionCallback; - - /// - /// The completion event. - /// - private readonly ManualResetEvent _completionEvent; - - /// - /// True if it has been invoked - /// - private int _completionInvoked; - - /// - /// Constructor - /// - internal GraphBuildSubmission(BuildManager buildManager, int submissionId, GraphBuildRequestData requestData) + internal GraphBuildSubmission(BuildManager buildManager, int submissionId, GraphBuildRequestData requestData) : + base(buildManager, submissionId, requestData) { - ErrorUtilities.VerifyThrowArgumentNull(buildManager, nameof(buildManager)); - ErrorUtilities.VerifyThrowArgumentNull(requestData, nameof(requestData)); - - BuildManager = buildManager; - SubmissionId = submissionId; - BuildRequestData = requestData; - _completionEvent = new ManualResetEvent(false); - _completionInvoked = 0; + CompleteLogging(); } /// - /// The BuildManager with which this submission is associated. - /// - public BuildManager BuildManager { get; } - - /// - /// An ID uniquely identifying this request from among other submissions within the same build. - /// - public int SubmissionId { get; } - - /// - /// The asynchronous context provided to , if any. - /// - public Object AsyncContext { get; private set; } - - /// - /// A which will be signalled when the build is complete. Valid after or returns, otherwise null. - /// - public WaitHandle WaitHandle => _completionEvent; - - /// - /// Returns true if this submission is complete. - /// - public bool IsCompleted => WaitHandle.WaitOne(new TimeSpan(0)); - - /// - /// The results of the build per graph node. Valid only after WaitHandle has become signalled. - /// - public GraphBuildResult BuildResult { get; internal set; } - - /// - /// The BuildRequestData being used for this submission. + /// Starts the request asynchronously and immediately returns control to the caller. /// - internal GraphBuildRequestData BuildRequestData { get; } + /// The request has already been started or is already complete. + public void ExecuteAsync(GraphBuildSubmissionCompleteCallback? callback, object? context) + { + void Clb(BuildSubmissionBase submission) + { + callback?.Invoke((GraphBuildSubmission)submission); + } - /// - /// Whether the graph build has started. - /// - internal bool IsStarted { get; set; } + ExecuteAsync(Clb, context, allowMainThreadBuild: false); + } /// /// Starts the request and blocks until results are available. /// /// The request has already been started or is already complete. - public GraphBuildResult Execute() + public override GraphBuildResult Execute() { ExecuteAsync(null, null); WaitHandle.WaitOne(); - return BuildResult; - } + ErrorUtilities.VerifyThrow(BuildResult != null, + "BuildResult is not populated after Execute is done."); - /// - /// Starts the request asynchronously and immediately returns control to the caller. - /// - /// The request has already been started or is already complete. - public void ExecuteAsync(GraphBuildSubmissionCompleteCallback callback, object context) - { - ErrorUtilities.VerifyThrowInvalidOperation(!IsCompleted, "SubmissionAlreadyComplete"); - _completionCallback = callback; - AsyncContext = context; - BuildManager.ExecuteSubmission(this); + return BuildResult!; } - /// - /// Sets the event signaling that the build is complete. - /// - internal void CompleteResults(GraphBuildResult result) - { - ErrorUtilities.VerifyThrowArgumentNull(result, nameof(result)); - ErrorUtilities.VerifyThrow(result.SubmissionId == SubmissionId, "GraphBuildResult's submission id doesn't match GraphBuildSubmission's"); - - bool hasCompleted = (Interlocked.Exchange(ref _completionInvoked, 1) == 1); - if (!hasCompleted) - { - BuildResult = result; - _completionEvent.Set(); - - if (_completionCallback != null) - { - void Callback(object state) - { - _completionCallback(this); - } - - ThreadPoolExtensions.QueueThreadPoolWorkItemWithCulture(Callback, CultureInfo.CurrentCulture, CultureInfo.CurrentUICulture); - } - } - } + protected internal override GraphBuildResult CreateFailedResult(Exception exception) + => new(SubmissionId, exception); } } diff --git a/src/Build/Microsoft.Build.csproj b/src/Build/Microsoft.Build.csproj index 363a6330ddd..88962e732ae 100644 --- a/src/Build/Microsoft.Build.csproj +++ b/src/Build/Microsoft.Build.csproj @@ -132,8 +132,11 @@ + + + @@ -156,6 +159,7 @@ + diff --git a/src/BuildCheck.UnitTests/TaskInvocationAnalysisDataTests.cs b/src/BuildCheck.UnitTests/TaskInvocationAnalysisDataTests.cs index e0600fc657c..b6210248c0d 100644 --- a/src/BuildCheck.UnitTests/TaskInvocationAnalysisDataTests.cs +++ b/src/BuildCheck.UnitTests/TaskInvocationAnalysisDataTests.cs @@ -81,7 +81,7 @@ private void BuildProject(string taskInvocation) using (var buildManager = new BuildManager()) { - var request = new BuildRequestData(testProject.ProjectFile, new Dictionary(), MSBuildConstants.CurrentToolsVersion, [], null, BuildRequestDataFlags.None); + var request = new BuildRequestData(testProject.ProjectFile, new Dictionary(), MSBuildConstants.CurrentToolsVersion, [], null, BuildRequestDataFlags.None); var parameters = new BuildParameters { LogTaskInputs = true, diff --git a/src/MSBuild/JsonOutputFormatter.cs b/src/MSBuild/JsonOutputFormatter.cs index 095f0761386..cdc166849ca 100644 --- a/src/MSBuild/JsonOutputFormatter.cs +++ b/src/MSBuild/JsonOutputFormatter.cs @@ -132,7 +132,7 @@ internal void AddTargetResultsInJsonFormat(string[] targetNames, BuildResult res JsonObject targetResultsNode = new(); foreach (string targetName in targetNames) { - TargetResult targetResult = result.ResultsByTarget[targetName]; + TargetResult targetResult = result.ResultsByTarget![targetName]; JsonObject targetResults = new(); targetResults["Result"] = targetResult.ResultCode.ToString(); JsonArray outputArray = new();