dotnet · Forgind · Jun 30, 2022 · Jun 30, 2022 · Jun 30, 2022 · Jun 30, 2022
diff --git a/src/Build.UnitTests/BackEnd/LoggingService_Tests.cs b/src/Build.UnitTests/BackEnd/LoggingService_Tests.cs
@@ -801,18 +801,6 @@ public void TreatWarningsAsErrorWhenAllSpecified(int loggerMode, int nodeId)
             logger.Errors.ShouldHaveSingleItem();
         }
 
-        [Fact]
-        public void VerifyWarningsPromotedToErrorsAreCounted()
-        {
-            ILoggingService ls = LoggingService.CreateLoggingService(LoggerMode.Synchronous, 1);
-            ls.WarningsAsErrors = new HashSet<string>();
-            ls.WarningsAsErrors.Add("FOR123");
-            BuildWarningEventArgs warningArgs = new("abc", "FOR123", "", 0, 0, 0, 0, "warning message", "keyword", "sender");
-            warningArgs.BuildEventContext = new BuildEventContext(1, 2, BuildEventContext.InvalidProjectContextId, BuildEventContext.InvalidProjectContextId, 5, 6);
-            ls.LogBuildEvent(warningArgs);
-            ls.HasBuildSubmissionLoggedErrors(1).ShouldBeTrue();
-        }
-
         /// <summary>
         /// Verifies that a warning is logged as a low importance message when it's warning code is specified.
         /// </summary>

diff --git a/src/Build.UnitTests/BinaryLogger_Tests.cs b/src/Build.UnitTests/BinaryLogger_Tests.cs
@@ -1,13 +1,12 @@
 using System;
 using System.Collections.Generic;
-using System.IO;
 using System.Text;
 
 using Microsoft.Build.BackEnd.Logging;
 using Microsoft.Build.Execution;
 using Microsoft.Build.Framework;
 using Microsoft.Build.Logging;
-using Microsoft.Build.UnitTests.Shared;
+
 using Shouldly;
 using Xunit;
 using Xunit.Abstractions;
@@ -139,45 +138,6 @@ public void BinaryLoggerShouldSupportFilePathExplicitParameter()
             ObjectModelHelpers.BuildProjectExpectSuccess(s_testProject, binaryLogger);
         }
 
-        [Fact]
-        public void UnusedEnvironmentVariablesDoNotAppearInBinaryLog()
-        {
-            using (TestEnvironment env = TestEnvironment.Create())
-            {
-                env.SetEnvironmentVariable("EnvVar1", "itsValue");
-                env.SetEnvironmentVariable("EnvVar2", "value2");
-                env.SetEnvironmentVariable("EnvVar3", "value3");
-                string contents = @"
-<Project DefaultTargets=""PrintEnvVar"">
-
-<PropertyGroup>
-<MyProp1>value</MyProp1>
-<MyProp2>$(EnvVar2)</MyProp2>
-</PropertyGroup>
-
-<Target Name=""PrintEnvVar"">
-<Message Text=""Environment variable EnvVar3 has value $(EnvVar3)"" Importance=""High"" />
-</Target>
-
-</Project>";
-                TransientTestFolder logFolder = env.CreateFolder(createFolder: true);
-                TransientTestFile projectFile = env.CreateFile(logFolder, "myProj.proj", contents);
-                BinaryLogger logger = new();
-                logger.Parameters = _logFile;
-                RunnerUtilities.ExecMSBuild($"{projectFile.Path} -bl:{logger.Parameters}", out bool success);
-                success.ShouldBeTrue();
-                RunnerUtilities.ExecMSBuild($"{logger.Parameters} -flp:logfile={Path.Combine(logFolder.Path, "logFile.log")};verbosity=diagnostic", out success);
-                success.ShouldBeTrue();
-                string text = File.ReadAllText(Path.Combine(logFolder.Path, "logFile.log"));
-                text.ShouldContain("EnvVar2");
-                text.ShouldContain("value2");
-                text.ShouldContain("EnvVar3");
-                text.ShouldContain("value3");
-                text.ShouldNotContain("EnvVar1");
-                text.ShouldNotContain("itsValue");
-            }
-        }
-
         [Fact]
         public void BinaryLoggerShouldNotThrowWhenMetadataCannotBeExpanded()
         {

diff --git a/src/Build.UnitTests/BuildEventArgsSerialization_Tests.cs b/src/Build.UnitTests/BuildEventArgsSerialization_Tests.cs
@@ -24,12 +24,9 @@ public BuildEventArgsSerializationTests()
             _ = ItemGroupLoggingHelper.ItemGroupIncludeLogMessagePrefix;
         }
 
-        [Theory]
-        [InlineData(true)]
-        [InlineData(false)]
-        public void RoundtripBuildStartedEventArgs(bool serializeAllEnvironmentVariables)
+        [Fact]
+        public void RoundtripBuildStartedEventArgs()
         {
-            Traits.LogAllEnvironmentVariables = serializeAllEnvironmentVariables;
             var args = new BuildStartedEventArgs(
                 "Message",
                 "HelpKeyword",
@@ -44,15 +41,13 @@ public void RoundtripBuildStartedEventArgs(bool serializeAllEnvironmentVariables
                 null,
                 new Dictionary<string, string>
                 {
-                { "SampleName", "SampleValue" }
+                    { "SampleName", "SampleValue" }
                 });
             Roundtrip(args,
-                e => serializeAllEnvironmentVariables ? TranslationHelpers.ToString(e.BuildEnvironment) : null,
+                e => TranslationHelpers.ToString(e.BuildEnvironment),
                 e => e.HelpKeyword,
                 e => e.ThreadId.ToString(),
                 e => e.SenderName);
-
-            Traits.LogAllEnvironmentVariables = false;
         }
 
         [Fact]
@@ -178,17 +173,6 @@ public void RoundtripTaskStartedEventArgs()
                 e => e.ColumnNumber.ToString());
         }
 
-        [Fact]
-        public void RoundtripEnvironmentVariableReadEventArgs()
-        {
-            EnvironmentVariableReadEventArgs args = new("VarName", "VarValue");
-            args.BuildEventContext = new BuildEventContext(4, 5, 6, 7);
-            Roundtrip(args,
-                e => e.Message,
-                e => e.EnvironmentVariableName,
-                e => e.BuildEventContext.ToString());
-        }
-
         [Fact]
         public void RoundtripTaskFinishedEventArgs()
         {

diff --git a/src/Build.UnitTests/Evaluation/Evaluator_Tests.cs b/src/Build.UnitTests/Evaluation/Evaluator_Tests.cs
@@ -4522,7 +4522,7 @@ public void VerifyMSBuildLogsAMessageWhenLocalPropertyCannotOverrideValueOfGloba
         [Fact]
         public void VerifyPropertyTrackingLoggingDefault()
         {
-            // Having just environment variables defined should default to nothing being logged except one environment variable read.
+            // Having nothing defined should default to nothing being logged.
             this.VerifyPropertyTrackingLoggingScenario(
                 null,
                 logger =>
@@ -4535,9 +4535,7 @@ public void VerifyPropertyTrackingLoggingDefault()
                     logger
                         .AllBuildEvents
                         .OfType<EnvironmentVariableReadEventArgs>()
-                        .ShouldHaveSingleItem()
-                        .EnvironmentVariableName
-                        .ShouldBe("DEFINED_ENVIRONMENT_VARIABLE2");
+                        .ShouldBeEmpty();
 
                     logger
                         .AllBuildEvents
@@ -4566,9 +4564,7 @@ public void VerifyPropertyTrackingLoggingPropertyReassignment()
                     logger
                         .AllBuildEvents
                         .OfType<EnvironmentVariableReadEventArgs>()
-                        .ShouldHaveSingleItem()
-                        .EnvironmentVariableName
-                        .ShouldBe("DEFINED_ENVIRONMENT_VARIABLE2");
+                        .ShouldBeEmpty();
 
                     logger
                         .AllBuildEvents
@@ -4597,9 +4593,7 @@ public void VerifyPropertyTrackingLoggingNone()
                     logger
                         .AllBuildEvents
                         .OfType<EnvironmentVariableReadEventArgs>()
-                        .ShouldHaveSingleItem()
-                        .EnvironmentVariableName
-                        .ShouldBe("DEFINED_ENVIRONMENT_VARIABLE2");
+                        .ShouldBeEmpty();
 
                     logger
                         .AllBuildEvents
@@ -4628,9 +4622,7 @@ public void VerifyPropertyTrackingLoggingPropertyInitialValue()
                     logger
                         .AllBuildEvents
                         .OfType<EnvironmentVariableReadEventArgs>()
-                        .ShouldHaveSingleItem()
-                        .EnvironmentVariableName
-                        .ShouldBe("DEFINED_ENVIRONMENT_VARIABLE2");
+                        .ShouldBeEmpty();
 
                     logger
                         .AllBuildEvents
@@ -4714,9 +4706,7 @@ public void VerifyPropertyTrackingLoggingUninitializedPropertyRead()
                     logger
                         .AllBuildEvents
                         .OfType<EnvironmentVariableReadEventArgs>()
-                        .ShouldHaveSingleItem()
-                        .EnvironmentVariableName
-                        .ShouldBe("DEFINED_ENVIRONMENT_VARIABLE2");
+                        .ShouldBeEmpty();
 
                     logger
                         .AllBuildEvents

diff --git a/src/Build.UnitTests/Utilities_Tests.cs b/src/Build.UnitTests/Utilities_Tests.cs
@@ -22,7 +22,6 @@
 using System.Collections.Generic;
 using System.IO;
 using Xunit.Abstractions;
-using Shouldly;
 
 #nullable disable
 
@@ -81,41 +80,55 @@ public UtilitiesTestReadOnlyLoad()
         [Fact]
         public void CommentsInPreprocessing()
         {
-            using TestEnvironment env = TestEnvironment.Create();
-            XmlDocumentWithLocation.ClearReadOnlyFlags_UnitTestsOnly();
+            Microsoft.Build.Construction.XmlDocumentWithLocation.ClearReadOnlyFlags_UnitTestsOnly();
 
-            TransientTestFile inputFile = env.CreateFile("tempInput.tmp", ObjectModelHelpers.CleanupFileContents(@"
-<Project DefaultTargets='Build'>
-<Import Project='$(MSBuildToolsPath)\Microsoft.CSharp.targets'/>
-</Project>"));
-            TransientTestFile outputFile = env.CreateFile("tempOutput.tmp");
+            string input = FileUtilities.GetTemporaryFile();
+            string output = FileUtilities.GetTemporaryFile();
 
-            env.SetEnvironmentVariable("MSBUILDLOADALLFILESASWRITEABLE", "1");
+            string _initialLoadFilesWriteable = Environment.GetEnvironmentVariable("MSBUILDLOADALLFILESASWRITEABLE");
+            try
+            {
+                Environment.SetEnvironmentVariable("MSBUILDLOADALLFILESASWRITEABLE", "1");
+
+                string content = ObjectModelHelpers.CleanupFileContents(@"
+<Project DefaultTargets='Build' ToolsVersion='msbuilddefaulttoolsversion' xmlns='msbuildnamespace'>
+  <Import Project='$(MSBuildToolsPath)\Microsoft.CSharp.targets'/>
+</Project>");
+                File.WriteAllText(input, content);
 
 #if FEATURE_GET_COMMANDLINE
-            MSBuildApp.Execute(@"c:\bin\msbuild.exe """ + inputFile.Path +
-                (NativeMethodsShared.IsUnixLike ? @""" -pp:""" : @""" /pp:""") + outputFile.Path + @"""")
-                .ShouldBe(MSBuildApp.ExitType.Success);
+                Assert.Equal(MSBuildApp.ExitType.Success, MSBuildApp.Execute(@"c:\bin\msbuild.exe """ + input +
+                    (NativeMethodsShared.IsUnixLike ? @""" -pp:""" : @""" /pp:""") + output + @""""));
 #else
-            Assert.Equal(
-                MSBuildApp.ExitType.Success,
-                MSBuildApp.Execute(
-                    new[] { @"c:\bin\msbuild.exe", '"' + inputFile.Path + '"',
-                '"' + (NativeMethodsShared.IsUnixLike ? "-pp:" : "/pp:") + outputFile.Path + '"'}));
+                Assert.Equal(
+                    MSBuildApp.ExitType.Success,
+                    MSBuildApp.Execute(
+                        new[] { @"c:\bin\msbuild.exe", '"' + input + '"',
+                    '"' + (NativeMethodsShared.IsUnixLike ? "-pp:" : "/pp:") + output + '"'}));
 #endif
 
-            bool foundDoNotModify = false;
-            foreach (string line in File.ReadLines(outputFile.Path))
-            {
-                line.ShouldNotContain("<!---->", "This is what it will look like if we're loading read/only");
-
-                if (line.Contains("DO NOT MODIFY")) // this is in a comment in our targets
+                bool foundDoNotModify = false;
+                foreach (string line in File.ReadLines(output))
                 {
-                    foundDoNotModify = true;
+                    if (line.Contains("<!---->")) // This is what it will look like if we're loading read/only
+                    {
+                        Assert.True(false);
+                    }
+
+                    if (line.Contains("DO NOT MODIFY")) // this is in a comment in our targets
+                    {
+                        foundDoNotModify = true;
+                    }
                 }
-            }
 
-            foundDoNotModify.ShouldBeTrue();
+                Assert.True(foundDoNotModify);
+            }
+            finally
+            {
+                File.Delete(input);
+                File.Delete(output);
+                Environment.SetEnvironmentVariable("MSBUILDLOADALLFILESASWRITEABLE", _initialLoadFilesWriteable);
+            }
         }
 
         [Fact]

diff --git a/src/Build/BackEnd/Components/Logging/LoggingService.cs b/src/Build/BackEnd/Components/Logging/LoggingService.cs
@@ -8,6 +8,7 @@
 using System.Linq;
 using System.Reflection;
 using System.Threading;
+using System.Threading.Tasks;
 using Microsoft.Build.Framework;
 using Microsoft.Build.Shared;
 using InternalLoggerException = Microsoft.Build.Exceptions.InternalLoggerException;
@@ -1431,8 +1432,17 @@ private void LoggingEventProcessor(object loggingEvent)
         /// </summary>
         private void RouteBuildEvent(object loggingEvent)
         {
-            BuildEventArgs buildEventArgs = loggingEvent as BuildEventArgs ?? (loggingEvent as KeyValuePair<int, BuildEventArgs>?)?.Value;
-            if (buildEventArgs is null)
+            BuildEventArgs buildEventArgs = null;
+
+            if (loggingEvent is BuildEventArgs bea)
+            {
+                buildEventArgs = bea;
+            }
+            else if (loggingEvent is KeyValuePair<int, BuildEventArgs> kvp)
+            {
+                buildEventArgs = kvp.Value;
+            }
+            else
             {
                 ErrorUtilities.ThrowInternalError("Unknown logging item in queue:" + loggingEvent.GetType().FullName);
             }
@@ -1531,7 +1541,7 @@ private void RouteBuildEvent(BuildEventArgs eventArg)
             TryRaiseProjectStartedEvent(eventArg);
 
             // The event has not been through a filter yet. All events must go through a filter before they make it to a logger
-            if (_filterEventSource != null) // Loggers may not be registered
+            if (_filterEventSource != null)   // Loggers may not be registered
             {
                 // Send the event to the filter, the Consume will not return until all of the loggers which have registered to the event have process
                 // them.
@@ -1552,7 +1562,7 @@ private void RouteBuildEvent(BuildEventArgs eventArg)
                         {
                             if (!sink.HaveLoggedBuildStartedEvent)
                             {
-                                sink.Consume(eventArg, pair.Key);
+                                sink.Consume(eventArg, (int)pair.Key);
                             }
 
                             // Reset the HaveLoggedBuildStarted event because no one else will be sending a build started event to any loggers at this time.

diff --git a/src/Build/BackEnd/Components/Logging/LoggingServiceLogMethods.cs b/src/Build/BackEnd/Components/Logging/LoggingServiceLogMethods.cs
@@ -388,9 +388,14 @@ public void LogBuildStarted()
                 message = ResourceUtilities.GetResourceString("BuildStarted");
             }
 
-            IDictionary<string, string> environmentProperties = _componentHost?.BuildParameters?.BuildProcessEnvironment;
+            IDictionary<string, string> environmentProperties = null;
 
-            BuildStartedEventArgs buildEvent = new(message, helpKeyword: null, environmentProperties);
+            if (_componentHost?.BuildParameters != null)
+            {
+                environmentProperties = _componentHost.BuildParameters.BuildProcessEnvironment;
+            }
+
+            BuildStartedEventArgs buildEvent = new BuildStartedEventArgs(message, null /* no help keyword */, environmentProperties);
 
             // Raise the event with the filters
             ProcessLoggingEvent(buildEvent);

diff --git a/src/Build/BackEnd/Components/Logging/ProjectLoggingContext.cs b/src/Build/BackEnd/Components/Logging/ProjectLoggingContext.cs
@@ -8,7 +8,6 @@
 using Microsoft.Build.Execution;
 using Microsoft.Build.Framework;
 using Microsoft.Build.Shared;
-using static Microsoft.Build.Execution.ProjectPropertyInstance;
 using TaskItem = Microsoft.Build.Execution.ProjectItemInstance.TaskItem;
 
 #nullable disable
@@ -72,17 +71,6 @@ internal ProjectLoggingContext(
         {
         }
 
-        private IEnumerable<DictionaryEntry> FilterEnvironmentDerivedProperties(PropertyDictionary<ProjectPropertyInstance> properties)
-        {
-            foreach (ProjectPropertyInstance property in properties)
-            {
-                if (property is not EnvironmentDerivedProjectPropertyInstance)
-                {
-                    yield return new DictionaryEntry(property.Name, property.EvaluatedValue);
-                }
-            }
-        }
-
         /// <summary>
         /// Constructs a project logging contexts.
         /// </summary>
@@ -112,19 +100,7 @@ private ProjectLoggingContext(
                 !LoggingService.IncludeEvaluationPropertiesAndItems &&
                 (!LoggingService.RunningOnRemoteNode || LoggingService.SerializeAllProperties))
             {
-                if (projectProperties is null)
-                {
-                    properties = Enumerable.Empty<DictionaryEntry>();
-                }
-                else if (Traits.LogAllEnvironmentVariables)
-                {
-                    properties = projectProperties.GetCopyOnReadEnumerable(property => new DictionaryEntry(property.Name, property.EvaluatedValue));
-                }
-                else
-                {
-                    properties = FilterEnvironmentDerivedProperties(projectProperties);
-                }
-
+                properties = projectProperties?.GetCopyOnReadEnumerable(property => new DictionaryEntry(property.Name, property.EvaluatedValue)) ?? Enumerable.Empty<DictionaryEntry>();
                 items = projectItems?.GetCopyOnReadEnumerable(item => new DictionaryEntry(item.ItemType, new TaskItem(item))) ?? Enumerable.Empty<DictionaryEntry>();
             }