From 0a1c3e2916821719bc8aac542929d7c3914c8732 Mon Sep 17 00:00:00 2001 From: Jan Krivanek Date: Fri, 26 Jul 2024 08:45:57 +0200 Subject: [PATCH] Revert Emit eval props if requested by any sink (#10243) --- documentation/wiki/ChangeWaves.md | 1 - .../BackEnd/BuildManager_Tests.cs | 59 ++++-------- .../BackEnd/MockLoggingService.cs | 19 ++-- src/Build.UnitTests/ConsoleLogger_Tests.cs | 2 +- .../BackEnd/BuildManager/BuildManager.cs | 3 +- .../Components/Logging/ILoggingService.cs | 18 +--- .../Components/Logging/LoggingService.cs | 91 ++++++------------- .../Logging/ProjectLoggingContext.cs | 4 +- .../BackEnd/Node/LoggingNodeConfiguration.cs | 15 +-- src/Build/BackEnd/Node/OutOfProcNode.cs | 7 +- src/Build/Evaluation/Evaluator.cs | 2 +- .../ProjectStartedEventArgs_Tests.cs | 24 +++++ src/Framework/ProjectStartedEventArgs.cs | 13 +-- src/Shared/UnitTests/TestAssemblyInfo.cs | 6 -- src/UnitTests.Shared/MockLogger.cs | 5 - 15 files changed, 93 insertions(+), 176 deletions(-) diff --git a/documentation/wiki/ChangeWaves.md b/documentation/wiki/ChangeWaves.md index 9aff19e9e15..c30d03cea0f 100644 --- a/documentation/wiki/ChangeWaves.md +++ b/documentation/wiki/ChangeWaves.md @@ -28,7 +28,6 @@ A wave of features is set to "rotate out" (i.e. become standard functionality) t - [Convert.ToString during a property evaluation uses the InvariantCulture for all types](https://github.com/dotnet/msbuild/pull/9874) - [Fix oversharing of build results in ResultsCache](https://github.com/dotnet/msbuild/pull/9987) - [Add ParameterName and PropertyName to TaskParameterEventArgs](https://github.com/dotnet/msbuild/pull/10130) -- [Emit eval props if requested by any sink](https://github.com/dotnet/msbuild/pull/10243) ### 17.10 - [AppDomain configuration is serialized without using BinFmt](https://github.com/dotnet/msbuild/pull/9320) - feature can be opted out only if [BinaryFormatter](https://learn.microsoft.com/en-us/dotnet/api/system.runtime.serialization.formatters.binary.binaryformatter) is allowed at runtime by editing `MSBuild.runtimeconfig.json` diff --git a/src/Build.UnitTests/BackEnd/BuildManager_Tests.cs b/src/Build.UnitTests/BackEnd/BuildManager_Tests.cs index c5c90d7a549..faf9f3f8ccf 100644 --- a/src/Build.UnitTests/BackEnd/BuildManager_Tests.cs +++ b/src/Build.UnitTests/BackEnd/BuildManager_Tests.cs @@ -82,7 +82,7 @@ public BuildManager_Tests(ITestOutputHelper output) EnableNodeReuse = false }; _buildManager = new BuildManager(); - _projectCollection = new ProjectCollection(globalProperties: null, _parameters.Loggers, ToolsetDefinitionLocations.Default); + _projectCollection = new ProjectCollection(); _env = TestEnvironment.Create(output); _inProcEnvCheckTransientEnvironmentVariable = _env.SetEnvironmentVariable("MSBUILDINPROCENVCHECK", "1"); @@ -137,8 +137,8 @@ public void SimpleBuild() _logger.AssertLogContains("[success]"); Assert.Single(_logger.ProjectStartedEvents); - ProjectEvaluationFinishedEventArgs evalFinishedEvent = _logger.EvaluationFinishedEvents[0]; - Dictionary properties = ExtractProjectStartedPropertyList(evalFinishedEvent.Properties); + ProjectStartedEventArgs projectStartedEvent = _logger.ProjectStartedEvents[0]; + Dictionary properties = ExtractProjectStartedPropertyList(projectStartedEvent.Properties); Assert.True(properties.TryGetValue("InitialProperty1", out string propertyValue)); Assert.Equal("InitialProperty1", propertyValue); @@ -254,8 +254,8 @@ public void SimpleGraphBuild() _logger.AssertLogContains("[success]"); _logger.ProjectStartedEvents.Count.ShouldBe(1); - ProjectEvaluationFinishedEventArgs evalFinishedEvent = _logger.EvaluationFinishedEvents[0]; - Dictionary properties = ExtractProjectStartedPropertyList(evalFinishedEvent.Properties); + ProjectStartedEventArgs projectStartedEvent = _logger.ProjectStartedEvents[0]; + Dictionary properties = ExtractProjectStartedPropertyList(projectStartedEvent.Properties); properties.TryGetValue("InitialProperty1", out string propertyValue).ShouldBeTrue(); propertyValue.ShouldBe("InitialProperty1", StringCompareShould.IgnoreCase); @@ -571,8 +571,8 @@ public void InProcForwardPropertiesFromChild() _logger.AssertLogContains("[success]"); Assert.Single(_logger.ProjectStartedEvents); - ProjectEvaluationFinishedEventArgs evalFinishedEvent = _logger.EvaluationFinishedEvents[0]; - Dictionary properties = ExtractProjectStartedPropertyList(evalFinishedEvent.Properties); + ProjectStartedEventArgs projectStartedEvent = _logger.ProjectStartedEvents[0]; + Dictionary properties = ExtractProjectStartedPropertyList(projectStartedEvent.Properties); Assert.True(properties.TryGetValue("InitialProperty1", out string propertyValue)); Assert.Equal("InitialProperty1", propertyValue); @@ -611,8 +611,8 @@ public void InProcMsBuildForwardAllPropertiesFromChild() _logger.AssertLogContains("[success]"); Assert.Single(_logger.ProjectStartedEvents); - ProjectEvaluationFinishedEventArgs evalFinishedEvent = _logger.EvaluationFinishedEvents[0]; - Dictionary properties = ExtractProjectStartedPropertyList(evalFinishedEvent.Properties); + ProjectStartedEventArgs projectStartedEvent = _logger.ProjectStartedEvents[0]; + Dictionary properties = ExtractProjectStartedPropertyList(projectStartedEvent.Properties); Assert.True(properties.TryGetValue("InitialProperty1", out string propertyValue)); Assert.Equal("InitialProperty1", propertyValue); @@ -655,8 +655,8 @@ public void MsBuildForwardAllPropertiesFromChildLaunchChildNode() _logger.AssertLogContains("[success]"); Assert.Single(_logger.ProjectStartedEvents); - ProjectEvaluationFinishedEventArgs evalFinishedEvent = _logger.EvaluationFinishedEvents[0]; - Dictionary properties = ExtractProjectStartedPropertyList(evalFinishedEvent.Properties); + ProjectStartedEventArgs projectStartedEvent = _logger.ProjectStartedEvents[0]; + Dictionary properties = ExtractProjectStartedPropertyList(projectStartedEvent.Properties); Assert.True(properties.TryGetValue("InitialProperty1", out string propertyValue)); Assert.Equal("InitialProperty1", propertyValue); @@ -704,15 +704,7 @@ public void OutOfProcNodeForwardCertainproperties() var data = new BuildRequestData(project.FullPath, new Dictionary(), MSBuildDefaultToolsVersion, Array.Empty(), null); - // We need to recreate build parameters to ensure proper capturing of newly set environment variables - BuildParameters parameters = new BuildParameters - { - ShutdownInProcNodeOnBuildFinish = true, - Loggers = new ILogger[] { _logger }, - EnableNodeReuse = false - }; - - BuildResult result = _buildManager.Build(parameters, data); + BuildResult result = _buildManager.Build(_parameters, data); Assert.Equal(BuildResultCode.Success, result.OverallResult); _logger.AssertLogContains("[success]"); Assert.Single(_logger.ProjectStartedEvents); @@ -768,21 +760,11 @@ public void OutOfProcNodeForwardCertainpropertiesAlsoGetResultsFromCache() _env.SetEnvironmentVariable("MsBuildForwardPropertiesFromChild", "InitialProperty3;IAMNOTREAL"); _env.SetEnvironmentVariable("MSBUILDNOINPROCNODE", "1"); - _env.SetEnvironmentVariable("MSBUILDLOGPROPERTIESANDITEMSAFTEREVALUATION", "0"); - var project = CreateProject(contents, null, _projectCollection, false); var data = new BuildRequestData(project.FullPath, new Dictionary(), MSBuildDefaultToolsVersion, Array.Empty(), null); - // We need to recreate build parameters to ensure proper capturing of newly set environment variables - BuildParameters parameters = new BuildParameters - { - ShutdownInProcNodeOnBuildFinish = true, - Loggers = new ILogger[] { _logger }, - EnableNodeReuse = false - }; - - BuildResult result = _buildManager.Build(parameters, data); + BuildResult result = _buildManager.Build(_parameters, data); Assert.Equal(BuildResultCode.Success, result.OverallResult); _logger.AssertLogContains("[success]"); Assert.Equal(3, _logger.ProjectStartedEvents.Count); @@ -803,8 +785,7 @@ public void OutOfProcNodeForwardCertainpropertiesAlsoGetResultsFromCache() Assert.Equal("InitialProperty3", propertyValue); projectStartedEvent = _logger.ProjectStartedEvents[2]; - properties = ExtractProjectStartedPropertyList(projectStartedEvent.Properties); - (properties == null || properties.Count == 0).ShouldBeTrue(); + Assert.Null(projectStartedEvent.Properties); } /// @@ -841,7 +822,7 @@ public void ForwardNoPropertiesLaunchChildNode() ProjectStartedEventArgs projectStartedEvent = _logger.ProjectStartedEvents[0]; Dictionary properties = ExtractProjectStartedPropertyList(projectStartedEvent.Properties); - (properties == null || properties.Count == 0).ShouldBeTrue(); + Assert.Null(properties); } /// @@ -938,7 +919,7 @@ public void ForwardNoPropertiesLaunchChildNodeDefault() ProjectStartedEventArgs projectStartedEvent = _logger.ProjectStartedEvents[0]; Dictionary properties = ExtractProjectStartedPropertyList(projectStartedEvent.Properties); - (properties == null || properties.Count == 0).ShouldBeTrue(); + Assert.Null(properties); } /// @@ -3494,11 +3475,9 @@ private static string BuildAndCheckCache(BuildManager localBuildManager, IEnumer /// private static Dictionary ExtractProjectStartedPropertyList(IEnumerable properties) { - Dictionary propertiesLookup = new Dictionary(StringComparer.OrdinalIgnoreCase); - Internal.Utilities.EnumerateProperties(properties, propertiesLookup, - static (dict, kvp) => dict.Add(kvp.Key, kvp.Value)); - - return propertiesLookup; + // Gather a sorted list of all the properties. + return properties?.Cast() + .ToDictionary(prop => (string)prop.Key, prop => (string)prop.Value, StringComparer.OrdinalIgnoreCase); } /// diff --git a/src/Build.UnitTests/BackEnd/MockLoggingService.cs b/src/Build.UnitTests/BackEnd/MockLoggingService.cs index d1e4654f648..cbc0a2d02c3 100644 --- a/src/Build.UnitTests/BackEnd/MockLoggingService.cs +++ b/src/Build.UnitTests/BackEnd/MockLoggingService.cs @@ -222,21 +222,14 @@ public bool IncludeEvaluationProfile set { } } - /// - public void SetIncludeEvaluationPropertiesAndItemsInEvents(bool inProjectStartedEvent, - bool inEvaluationFinishedEvent) - { } - - /// - public bool IncludeEvaluationPropertiesAndItemsInProjectStartedEvent - { - get => false; - } - - /// - public bool IncludeEvaluationPropertiesAndItemsInEvaluationFinishedEvent + /// + /// Log properties and items on ProjectEvaluationFinishedEventArgs + /// instead of ProjectStartedEventArgs. + /// + public bool IncludeEvaluationPropertiesAndItems { get => false; + set { } } /// diff --git a/src/Build.UnitTests/ConsoleLogger_Tests.cs b/src/Build.UnitTests/ConsoleLogger_Tests.cs index c1fdc67f6a5..10859bb9ce5 100644 --- a/src/Build.UnitTests/ConsoleLogger_Tests.cs +++ b/src/Build.UnitTests/ConsoleLogger_Tests.cs @@ -277,7 +277,7 @@ public void ErrorMessageWithMultiplePropertiesInMessage(bool includeEvaluationPr if (includeEvaluationPropertiesAndItems) { - pc.Collection.LoggingService.SetIncludeEvaluationPropertiesAndItemsInEvents(inProjectStartedEvent: false, inEvaluationFinishedEvent: true); + pc.Collection.LoggingService.IncludeEvaluationPropertiesAndItems = true; } var project = env.CreateTestProjectWithFiles(@" diff --git a/src/Build/BackEnd/BuildManager/BuildManager.cs b/src/Build/BackEnd/BuildManager/BuildManager.cs index 9d2ddcbac8c..38922b2de85 100644 --- a/src/Build/BackEnd/BuildManager/BuildManager.cs +++ b/src/Build/BackEnd/BuildManager/BuildManager.cs @@ -2757,8 +2757,7 @@ private NodeConfiguration GetNodeConfiguration() , new LoggingNodeConfiguration( loggingService.IncludeEvaluationMetaprojects, loggingService.IncludeEvaluationProfile, - loggingService.IncludeEvaluationPropertiesAndItemsInProjectStartedEvent, - loggingService.IncludeEvaluationPropertiesAndItemsInEvaluationFinishedEvent, + loggingService.IncludeEvaluationPropertiesAndItems, loggingService.IncludeTaskInputs)); } diff --git a/src/Build/BackEnd/Components/Logging/ILoggingService.cs b/src/Build/BackEnd/Components/Logging/ILoggingService.cs index 104dac56f6f..ecbf7b8026b 100644 --- a/src/Build/BackEnd/Components/Logging/ILoggingService.cs +++ b/src/Build/BackEnd/Components/Logging/ILoggingService.cs @@ -206,24 +206,12 @@ bool IncludeEvaluationProfile /// /// Should properties and items be logged on - /// or/and ? + /// instead of ? /// - void SetIncludeEvaluationPropertiesAndItemsInEvents(bool inProjectStartedEvent, bool inEvaluationFinishedEvent); - - /// - /// Indicates whether properties and items should be logged on . - /// - bool IncludeEvaluationPropertiesAndItemsInProjectStartedEvent - { - get; - } - - /// - /// Indicates whether properties and items should be logged on . - /// - bool IncludeEvaluationPropertiesAndItemsInEvaluationFinishedEvent + bool IncludeEvaluationPropertiesAndItems { get; + set; } /// diff --git a/src/Build/BackEnd/Components/Logging/LoggingService.cs b/src/Build/BackEnd/Components/Logging/LoggingService.cs index d00b7d90a1d..aa34bd3cafa 100644 --- a/src/Build/BackEnd/Components/Logging/LoggingService.cs +++ b/src/Build/BackEnd/Components/Logging/LoggingService.cs @@ -201,6 +201,12 @@ internal partial class LoggingService : ILoggingService, INodePacketHandler /// private bool? _includeEvaluationProfile; + /// + /// Whether properties and items should be logged on + /// instead of . + /// + private bool? _includeEvaluationPropertiesAndItems; + /// /// Whether to include task inputs in task events. /// @@ -540,77 +546,33 @@ public bool IncludeTaskInputs set => _includeTaskInputs = value; } - /// - public void SetIncludeEvaluationPropertiesAndItemsInEvents(bool inProjectStartedEvent, bool inEvaluationFinishedEvent) - { - _evalDataBehaviorSet = true; - IncludeEvaluationPropertiesAndItemsInEvaluationFinishedEvent = inEvaluationFinishedEvent; - IncludeEvaluationPropertiesAndItemsInProjectStartedEvent = inProjectStartedEvent; - } - - private bool _evalDataBehaviorSet; - private bool _includeEvaluationPropertiesAndItemsInProjectStartedEvent; - private bool _includeEvaluationPropertiesAndItemsInEvaluationFinishedEvent; - private void InferEvalDataBehavior() + /// + /// Should properties and items be logged on + /// instead of ? + /// + public bool IncludeEvaluationPropertiesAndItems { - if (_evalDataBehaviorSet) - { - return; - } - // Set this right away - to prevent SO exception in case of any future refactoring - // that would refer to the IncludeEvaluation... properties here - _evalDataBehaviorSet = true; - - bool? escapeHatch = Traits.Instance.EscapeHatches.LogPropertiesAndItemsAfterEvaluation; - if (escapeHatch.HasValue) - { - IncludeEvaluationPropertiesAndItemsInEvaluationFinishedEvent = escapeHatch.Value; - IncludeEvaluationPropertiesAndItemsInProjectStartedEvent = !escapeHatch.Value; - } - else + get { - var sinks = _eventSinkDictionary.Values.OfType().ToList(); - - if (ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_12)) - { - // If any logger requested the data - we need to emit them - IncludeEvaluationPropertiesAndItemsInEvaluationFinishedEvent = - sinks.Any(sink => sink.IncludeEvaluationPropertiesAndItems); - // If any logger didn't request the data - hence it's likely legacy logger - // - we need to populate the data in legacy way - IncludeEvaluationPropertiesAndItemsInProjectStartedEvent = - sinks.Any(sink => !sink.IncludeEvaluationPropertiesAndItems); - } - else + if (_includeEvaluationPropertiesAndItems == null) { - bool allSinksIncludeEvalData = sinks.Any() && sinks.All(sink => sink.IncludeEvaluationPropertiesAndItems); - - IncludeEvaluationPropertiesAndItemsInEvaluationFinishedEvent = allSinksIncludeEvalData; - IncludeEvaluationPropertiesAndItemsInProjectStartedEvent = !allSinksIncludeEvalData; + var escapeHatch = Traits.Instance.EscapeHatches.LogPropertiesAndItemsAfterEvaluation; + if (escapeHatch.HasValue) + { + _includeEvaluationPropertiesAndItems = escapeHatch.Value; + } + else + { + var sinks = _eventSinkDictionary.Values.OfType(); + // .All() on an empty list defaults to true, we want to default to false + _includeEvaluationPropertiesAndItems = sinks.Any() && sinks.All(sink => sink.IncludeEvaluationPropertiesAndItems); + } } - } - } - /// - public bool IncludeEvaluationPropertiesAndItemsInProjectStartedEvent - { - get - { - InferEvalDataBehavior(); - return _includeEvaluationPropertiesAndItemsInProjectStartedEvent; + return _includeEvaluationPropertiesAndItems ?? false; } - private set => _includeEvaluationPropertiesAndItemsInProjectStartedEvent = value; - } - /// - public bool IncludeEvaluationPropertiesAndItemsInEvaluationFinishedEvent - { - get - { - InferEvalDataBehavior(); - return _includeEvaluationPropertiesAndItemsInEvaluationFinishedEvent; - } - private set => _includeEvaluationPropertiesAndItemsInEvaluationFinishedEvent = value; + set => _includeEvaluationPropertiesAndItems = value; } /// @@ -652,7 +614,6 @@ public ICollection GetWarningsNotAsErrors(BuildEventContext context) return GetWarningsForProject(context, _warningsNotAsErrorsByProject, WarningsNotAsErrors); } - /// /// Returns a collection of warnings to be demoted to messages for the specified build context. /// diff --git a/src/Build/BackEnd/Components/Logging/ProjectLoggingContext.cs b/src/Build/BackEnd/Components/Logging/ProjectLoggingContext.cs index 06614c42125..49a3cd48fb7 100644 --- a/src/Build/BackEnd/Components/Logging/ProjectLoggingContext.cs +++ b/src/Build/BackEnd/Components/Logging/ProjectLoggingContext.cs @@ -132,7 +132,7 @@ private static BuildEventContext CreateInitialContext( // If we are only logging critical events lets not pass back the items or properties if (!loggingService.OnlyLogCriticalEvents && - loggingService.IncludeEvaluationPropertiesAndItemsInProjectStartedEvent && + !loggingService.IncludeEvaluationPropertiesAndItems && (!loggingService.RunningOnRemoteNode || loggingService.SerializeAllProperties)) { if (projectProperties is null) @@ -152,7 +152,7 @@ private static BuildEventContext CreateInitialContext( } if (projectProperties != null && - loggingService.IncludeEvaluationPropertiesAndItemsInProjectStartedEvent && + !loggingService.IncludeEvaluationPropertiesAndItems && propertiesToSerialize?.Length > 0 && !loggingService.SerializeAllProperties) { diff --git a/src/Build/BackEnd/Node/LoggingNodeConfiguration.cs b/src/Build/BackEnd/Node/LoggingNodeConfiguration.cs index 8c6315338da..1bae5efa98b 100644 --- a/src/Build/BackEnd/Node/LoggingNodeConfiguration.cs +++ b/src/Build/BackEnd/Node/LoggingNodeConfiguration.cs @@ -9,14 +9,12 @@ internal sealed class LoggingNodeConfiguration : ITranslatable { private bool _includeEvaluationMetaprojects; private bool _includeEvaluationProfiles; - private bool _includeEvaluationPropertiesAndItemsInProjectStartedEvent; - private bool _includeEvaluationPropertiesAndItemsInEvaluationFinishedEvent; + private bool _includeEvaluationPropertiesAndItems; private bool _includeTaskInputs; public bool IncludeEvaluationMetaprojects => _includeEvaluationMetaprojects; public bool IncludeEvaluationProfiles => _includeEvaluationProfiles; - public bool IncludeEvaluationPropertiesAndItemsInProjectStartedEvent => _includeEvaluationPropertiesAndItemsInProjectStartedEvent; - public bool IncludeEvaluationPropertiesAndItemsInEvaluationFinishedEvent => _includeEvaluationPropertiesAndItemsInEvaluationFinishedEvent; + public bool IncludeEvaluationPropertiesAndItems => _includeEvaluationPropertiesAndItems; public bool IncludeTaskInputs => _includeTaskInputs; public LoggingNodeConfiguration() @@ -26,14 +24,12 @@ public LoggingNodeConfiguration() public LoggingNodeConfiguration( bool includeEvaluationMetaprojects, bool includeEvaluationProfiles, - bool includeEvaluationPropertiesAndItemsInProjectStartedEvent, - bool includeEvaluationPropertiesAndItemsInEvaluationFinishedEvent, + bool includeEvaluationPropertiesAndItems, bool includeTaskInputs) { _includeEvaluationMetaprojects = includeEvaluationMetaprojects; _includeEvaluationProfiles = includeEvaluationProfiles; - _includeEvaluationPropertiesAndItemsInProjectStartedEvent = includeEvaluationPropertiesAndItemsInProjectStartedEvent; - _includeEvaluationPropertiesAndItemsInEvaluationFinishedEvent = includeEvaluationPropertiesAndItemsInEvaluationFinishedEvent; + _includeEvaluationPropertiesAndItems = includeEvaluationPropertiesAndItems; _includeTaskInputs = includeTaskInputs; } @@ -41,8 +37,7 @@ void ITranslatable.Translate(ITranslator translator) { translator.Translate(ref _includeEvaluationMetaprojects); translator.Translate(ref _includeEvaluationProfiles); - translator.Translate(ref _includeEvaluationPropertiesAndItemsInProjectStartedEvent); - translator.Translate(ref _includeEvaluationPropertiesAndItemsInEvaluationFinishedEvent); + translator.Translate(ref _includeEvaluationPropertiesAndItems); translator.Translate(ref _includeTaskInputs); } } diff --git a/src/Build/BackEnd/Node/OutOfProcNode.cs b/src/Build/BackEnd/Node/OutOfProcNode.cs index f0fd2ca4af5..3d7c50595d4 100644 --- a/src/Build/BackEnd/Node/OutOfProcNode.cs +++ b/src/Build/BackEnd/Node/OutOfProcNode.cs @@ -781,12 +781,9 @@ private void HandleNodeConfiguration(NodeConfiguration configuration) _loggingService.IncludeTaskInputs = true; } - if (configuration.LoggingNodeConfiguration.IncludeEvaluationPropertiesAndItemsInEvaluationFinishedEvent) + if (configuration.LoggingNodeConfiguration.IncludeEvaluationPropertiesAndItems) { - _loggingService.SetIncludeEvaluationPropertiesAndItemsInEvents( - configuration.LoggingNodeConfiguration.IncludeEvaluationPropertiesAndItemsInProjectStartedEvent, - configuration.LoggingNodeConfiguration - .IncludeEvaluationPropertiesAndItemsInEvaluationFinishedEvent); + _loggingService.IncludeEvaluationPropertiesAndItems = true; } try diff --git a/src/Build/Evaluation/Evaluator.cs b/src/Build/Evaluation/Evaluator.cs index 3e67f786ac8..d4d0e961465 100644 --- a/src/Build/Evaluation/Evaluator.cs +++ b/src/Build/Evaluation/Evaluator.cs @@ -343,7 +343,7 @@ internal static void Evaluate( IEnumerable properties = null; IEnumerable items = null; - if (evaluator._evaluationLoggingContext.LoggingService.IncludeEvaluationPropertiesAndItemsInEvaluationFinishedEvent) + if (evaluator._evaluationLoggingContext.LoggingService.IncludeEvaluationPropertiesAndItems) { globalProperties = evaluator._data.GlobalPropertiesDictionary; properties = Traits.LogAllEnvironmentVariables ? evaluator._data.Properties : evaluator.FilterOutEnvironmentDerivedProperties(evaluator._data.Properties); diff --git a/src/Framework.UnitTests/ProjectStartedEventArgs_Tests.cs b/src/Framework.UnitTests/ProjectStartedEventArgs_Tests.cs index 7eb7895b2df..67bacf49a74 100644 --- a/src/Framework.UnitTests/ProjectStartedEventArgs_Tests.cs +++ b/src/Framework.UnitTests/ProjectStartedEventArgs_Tests.cs @@ -50,6 +50,30 @@ public void EventArgsCtors() projectStartedEvent = new ProjectStartedEventArgs(1, null, null, null, null, null, null, null, DateTime.Now); } + /// + /// Verify different Items and properties are not taken into account in the equals comparison. They should + /// not be considered as part of the equals evaluation + /// + [Fact] + public void ItemsAndPropertiesDifferentEquals() + { + ArrayList itemsList = new ArrayList(); + ArrayList propertiesList = new ArrayList(); + ProjectStartedEventArgs differentItemsAndProperties = new ProjectStartedEventArgs( + s_baseProjectStartedEvent.ProjectId, + s_baseProjectStartedEvent.Message, + s_baseProjectStartedEvent.HelpKeyword, + s_baseProjectStartedEvent.ProjectFile, + s_baseProjectStartedEvent.TargetNames, + propertiesList, + itemsList, + s_baseProjectStartedEvent.ParentProjectBuildEventContext, + s_baseProjectStartedEvent.Timestamp); + + s_baseProjectStartedEvent.Properties.ShouldNotBe(propertiesList); + s_baseProjectStartedEvent.Items.ShouldNotBe(itemsList); + } + /// /// Create a derived class so that we can test the default constructor in order to increase code coverage and /// verify this code path does not cause any exceptions. diff --git a/src/Framework/ProjectStartedEventArgs.cs b/src/Framework/ProjectStartedEventArgs.cs index 4636850306a..cc9d14af45a 100644 --- a/src/Framework/ProjectStartedEventArgs.cs +++ b/src/Framework/ProjectStartedEventArgs.cs @@ -4,7 +4,6 @@ using System; using System.Collections; using System.Collections.Generic; -using System.Collections.Immutable; using System.IO; using System.Linq; using System.Runtime.Serialization; @@ -250,9 +249,7 @@ public IDictionary? GlobalProperties { get { - return globalProperties ?? (ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_12) - ? ImmutableDictionary.Empty - : null); + return globalProperties; } internal set @@ -301,9 +298,7 @@ public IEnumerable? Properties // up the live list of properties from the loaded project, which is stored in the configuration as well. // By doing this, we no longer need to transmit properties using this message because they've already // been transmitted as part of the BuildRequestConfiguration. - return properties ?? (ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_12) - ? Enumerable.Empty() - : null); + return properties; } } @@ -327,9 +322,7 @@ public IEnumerable? Items // case, this access is to the live list. For the central logger in the multi-proc case, the main node // has likely not loaded this project, and therefore the live items would not be available to them, which is // the same as the current functionality. - return items ?? (ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_12) - ? Enumerable.Empty() - : null); + return items; } } diff --git a/src/Shared/UnitTests/TestAssemblyInfo.cs b/src/Shared/UnitTests/TestAssemblyInfo.cs index afd777eef97..08353749def 100644 --- a/src/Shared/UnitTests/TestAssemblyInfo.cs +++ b/src/Shared/UnitTests/TestAssemblyInfo.cs @@ -41,12 +41,6 @@ public MSBuildTestAssemblyFixture() var runningTestsField = testInfoType.GetField("s_runningTests", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Public | System.Reflection.BindingFlags.Static); runningTestsField.SetValue(null, true); - // Set the field in BuildEnvironmentState - as it might have been already preintialized by the data preparation of data driven tests - testInfoType = frameworkAssembly.GetType("Microsoft.Build.Framework.BuildEnvironmentState"); - runningTestsField = testInfoType.GetField("s_runningTests", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Public | System.Reflection.BindingFlags.Static); - runningTestsField.SetValue(null, true); - - // Note: build error files will be initialized in test environments for particular tests, also we don't have output to report error files into anyway... _testEnvironment = TestEnvironment.Create(output: null, ignoreBuildErrorFiles: true); diff --git a/src/UnitTests.Shared/MockLogger.cs b/src/UnitTests.Shared/MockLogger.cs index b530cc538f0..782cef74d41 100644 --- a/src/UnitTests.Shared/MockLogger.cs +++ b/src/UnitTests.Shared/MockLogger.cs @@ -213,11 +213,6 @@ public void Initialize(IEventSource eventSource) { _reportTelemetry = true; } - - if (eventSource is IEventSource4 eventSource4) - { - eventSource4.IncludeEvaluationPropertiesAndItems(); - } } ///