Bring back ProjectStarted.GlobalProperties (#6342)

Some loggers depended on ProjectStartedEventArgs.GlobalProperties being not null and set. It will take a long time to move them to ProjectEvaluationFinished (needs to bump MSBuild dependency to 16.10). For now log GlobalProperties in both places (ProjectStarted and ProjectEvaluationFinished). Hopefully the deduplication will save us from any significant increase in binlog size. Fixes #6341
dotnet · Apr 13, 2021 · 8d57dfc · 8d57dfc
1 parent 9bcc06c
commit 8d57dfc
Show file tree

Hide file tree

Showing 3 changed files with 9 additions and 14 deletions.
diff --git a/src/Build/BackEnd/Components/Logging/LoggingService.cs b/src/Build/BackEnd/Components/Logging/LoggingService.cs
@@ -1170,10 +1170,11 @@ internal void InjectNonSerializedData(LogMessagePacket loggingPacket)
                 {
                     ErrorUtilities.VerifyThrow(_configCache.Value.HasConfiguration(projectStartedEventArgs.ProjectId), "Cannot find the project configuration while injecting non-serialized data from out-of-proc node.");
                     BuildRequestConfiguration buildRequestConfiguration = _configCache.Value[projectStartedEventArgs.ProjectId];
-                    if (!IncludeEvaluationPropertiesAndItems)
-                    {
-                        s_projectStartedEventArgsGlobalProperties.Value.SetValue(projectStartedEventArgs, buildRequestConfiguration.GlobalProperties.ToDictionary(), null);
-                    }
+
+                    // Always log GlobalProperties on ProjectStarted for compatibility.
+                    // There are loggers that depend on it being not-null and always set.
+                    // See https://github.com/dotnet/msbuild/issues/6341 for details.
+                    s_projectStartedEventArgsGlobalProperties.Value.SetValue(projectStartedEventArgs, buildRequestConfiguration.GlobalProperties.ToDictionary(), index: null);
 
                     s_projectStartedEventArgsToolsVersion.Value.SetValue(projectStartedEventArgs, buildRequestConfiguration.ToolsVersion, null);
                 }

diff --git a/src/Build/BackEnd/Components/Logging/LoggingServiceLogMethods.cs b/src/Build/BackEnd/Components/Logging/LoggingServiceLogMethods.cs
@@ -595,11 +595,9 @@ public BuildEventContext LogProjectStarted(BuildEventContext nodeBuildEventConte
                 ErrorUtilities.VerifyThrow(_configCache.Value.HasConfiguration(projectInstanceId), "Cannot find the project configuration while injecting non-serialized data from out-of-proc node.");
                 var buildRequestConfiguration = _configCache.Value[projectInstanceId];
 
-                IDictionary<string, string> globalProperties = null;
-                if (!IncludeEvaluationPropertiesAndItems)
-                {
-                    globalProperties = buildRequestConfiguration.GlobalProperties.ToDictionary();
-                }
+                // Always log GlobalProperties on ProjectStarted
+                // See https://github.com/dotnet/msbuild/issues/6341 for details
+                IDictionary<string, string> globalProperties = buildRequestConfiguration.GlobalProperties.ToDictionary();
 
                 var buildEvent = new ProjectStartedEventArgs
                     (

diff --git a/src/Build/Evaluation/Evaluator.cs b/src/Build/Evaluation/Evaluator.cs
@@ -785,11 +785,7 @@ private void Evaluate()
 
             if (this._evaluationLoggingContext.LoggingService.IncludeEvaluationPropertiesAndItems)
             {
-                if (_data.GlobalPropertiesDictionary.Count > 0)
-                {
-                    globalProperties = _data.GlobalPropertiesDictionary;
-                }
-
+                globalProperties = _data.GlobalPropertiesDictionary;
                 properties = _data.Properties;
                 items = _data.Items;
             }