Revert "Revert backport pr 10942 to vs17.12" (#11096)

* Revert "Revert backport pr 10942 to vs17.12 (#11088)"

This reverts commit bc692d0.

* Update Versions.props

Author: SimaTian

documentation/wiki/ChangeWaves.md

@@ -23,6 +23,9 @@ A wave of features is set to "rotate out" (i.e. become standard functionality) t
 
 ## Current Rotation of Change Waves
 
+### 17.14
+- [TreatWarningsAsErrors, WarningsAsMessages, WarningsAsErrors, WarningsNotAsErrors are now supported on the engine side of MSBuild](https://github.com/dotnet/msbuild/pull/10942)
+
 ### 17.12
 - [Log TaskParameterEvent for scalar parameters](https://github.com/dotnet/msbuild/pull/9908)
 - [Convert.ToString during a property evaluation uses the InvariantCulture for all types](https://github.com/dotnet/msbuild/pull/9874)

eng/Versions.props

@@ -2,7 +2,7 @@
 <!-- Copyright (c) .NET Foundation and contributors. All rights reserved. Licensed under the MIT license. See License.txt in the project root for full license information. -->
 <Project>
   <PropertyGroup>
-    <VersionPrefix>17.12.16</VersionPrefix><DotNetFinalVersionKind>release</DotNetFinalVersionKind>
+    <VersionPrefix>17.12.17</VersionPrefix><DotNetFinalVersionKind>release</DotNetFinalVersionKind>
     <PackageValidationBaselineVersion>17.11.4</PackageValidationBaselineVersion>
     <AssemblyVersion>15.1.0.0</AssemblyVersion>
     <PreReleaseVersionLabel>preview</PreReleaseVersionLabel>

src/Build.UnitTests/WarningsAsMessagesAndErrors_Tests.cs

@@ -1666,8 +1666,8 @@ private void RouteBuildEvent(object loggingEvent)
                 }
             }
 
-            // If this is BuildCheck-ed build - add the warnings promotability/demotability to the service
-            if (buildEventArgs is ProjectStartedEventArgs projectStartedEvent && this._componentHost.BuildParameters.IsBuildCheckEnabled)
+            // Respect warning-promotion properties from the remote project
+            if (buildEventArgs is ProjectStartedEventArgs projectStartedEvent)
             {
                 AddWarningsAsErrors(projectStartedEvent.BuildEventContext, projectStartedEvent.WarningsAsErrors);
                 AddWarningsAsMessages(projectStartedEvent.BuildEventContext, projectStartedEvent.WarningsAsMessages);

src/Build/BackEnd/Components/Logging/LoggingService.cs

@@ -1105,7 +1105,7 @@ private async Task<BuildResult> BuildProject()
         {
             ErrorUtilities.VerifyThrow(_targetBuilder != null, "Target builder is null");
 
-            // We consider this the entrypoint for the project build for purposes of BuildCheck processing 
+            // We consider this the entrypoint for the project build for purposes of BuildCheck processing
             bool isRestoring = _requestEntry.RequestConfiguration.GlobalProperties[MSBuildConstants.MSBuildIsRestoring] is not null;
 
             var buildCheckManager = isRestoring
@@ -1155,6 +1155,7 @@ private async Task<BuildResult> BuildProject()
                     _requestEntry.Request.BuildEventContext);
             }
 
+
             try
             {
                 HandleProjectStarted(buildCheckManager);
@@ -1278,7 +1279,7 @@ private void HandleProjectStarted(IBuildCheckManager buildCheckManager)
             BuildEventContext projectBuildEventContext = _projectLoggingContext?.BuildEventContext;
 
             // We can set the warning as errors and messages only after the project logging context has been created (as it creates the new ProjectContextId)
-            if (buildCheckManager != null && loggingService != null && projectBuildEventContext != null)
+            if (loggingService != null && projectBuildEventContext != null)
             {
                 args.WarningsAsErrors = loggingService.GetWarningsAsErrors(projectBuildEventContext).ToHashSet(StringComparer.OrdinalIgnoreCase);
                 args.WarningsAsMessages = loggingService.GetWarningsAsMessages(projectBuildEventContext).ToHashSet(StringComparer.OrdinalIgnoreCase);
@@ -1389,29 +1390,35 @@ private void ConfigureWarningsAsErrorsAndMessages()
             // Ensure everything that is required is available at this time
             if (project != null && buildEventContext != null && loggingService != null && buildEventContext.ProjectInstanceId != BuildEventContext.InvalidProjectInstanceId)
             {
-                if (String.Equals(project.GetEngineRequiredPropertyValue(MSBuildConstants.TreatWarningsAsErrors)?.Trim(), "true", StringComparison.OrdinalIgnoreCase))
+                if (String.Equals(project.GetEngineRequiredPropertyValue(MSBuildConstants.MSBuildPrefix + MSBuildConstants.TreatWarningsAsErrors)?.Trim(), "true", StringComparison.OrdinalIgnoreCase) ||
+                    (String.Equals(project.GetEngineRequiredPropertyValue(MSBuildConstants.TreatWarningsAsErrors)?.Trim(), "true", StringComparison.OrdinalIgnoreCase) &&
+                     ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_14)))
                 {
                     // If <MSBuildTreatWarningsAsErrors was specified then an empty ISet<string> signals the IEventSourceSink to treat all warnings as errors
                     loggingService.AddWarningsAsErrors(buildEventContext, new HashSet<string>());
                 }
                 else
                 {
-                    ISet<string> warningsAsErrors = ParseWarningCodes(project.GetEngineRequiredPropertyValue(MSBuildConstants.WarningsAsErrors));
+                    ISet<string> warningsAsErrors = ParseWarningCodes(project.GetEngineRequiredPropertyValue(MSBuildConstants.MSBuildPrefix + MSBuildConstants.WarningsAsErrors),
+                                                                      project.GetEngineRequiredPropertyValue(MSBuildConstants.WarningsAsErrors));
 
                     if (warningsAsErrors?.Count > 0)
                     {
                         loggingService.AddWarningsAsErrors(buildEventContext, warningsAsErrors);
                     }
                 }
 
-                ISet<string> warningsNotAsErrors = ParseWarningCodes(project.GetEngineRequiredPropertyValue(MSBuildConstants.WarningsNotAsErrors));
+                ISet<string> warningsNotAsErrors = ParseWarningCodes(project.GetEngineRequiredPropertyValue(MSBuildConstants.MSBuildPrefix + MSBuildConstants.WarningsNotAsErrors),
+                                                                     project.GetEngineRequiredPropertyValue(MSBuildConstants.WarningsNotAsErrors));
+
 
                 if (warningsNotAsErrors?.Count > 0)
                 {
                     loggingService.AddWarningsNotAsErrors(buildEventContext, warningsNotAsErrors);
                 }
 
-                ISet<string> warningsAsMessages = ParseWarningCodes(project.GetEngineRequiredPropertyValue(MSBuildConstants.WarningsAsMessages));
+                ISet<string> warningsAsMessages = ParseWarningCodes(project.GetEngineRequiredPropertyValue(MSBuildConstants.MSBuildPrefix + MSBuildConstants.WarningsAsMessages),
+                                                                    project.GetEngineRequiredPropertyValue(MSBuildConstants.WarningsAsMessages));
 
                 if (warningsAsMessages?.Count > 0)
                 {
@@ -1429,14 +1436,37 @@ private void ConfigureKnownImmutableFolders()
             }
         }
 
-        private static ISet<string> ParseWarningCodes(string warnings)
+        private static ISet<string> ParseWarningCodes(string warnings, string warningsNoPrefix)
         {
-            if (String.IsNullOrWhiteSpace(warnings))
+            // When this changewave is rotated out and this gets deleted, please consider removing
+            // the <MSBuildWarningsAsMessages Condition="'$(MSBuildWarningsAsMessages)'==''">$(NoWarn)</MSBuildWarningsAsMessages>
+            // and the two following lines from the msbuild/src/Tasks/Microsoft.Common.CurrentVersion.targets
+            if (!ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_14))
+            {
+                warningsNoPrefix = null;
+            }
+
+            HashSet<string> result1 = null;
+            if (!String.IsNullOrWhiteSpace(warnings))
+            {
+                result1 = new HashSet<string>(ExpressionShredder.SplitSemiColonSeparatedList(warnings), StringComparer.OrdinalIgnoreCase);
+            }
+            HashSet<string> result2 = null;
+            if (!String.IsNullOrWhiteSpace(warningsNoPrefix))
+            {
+                result2 = new HashSet<string>(ExpressionShredder.SplitSemiColonSeparatedList(warningsNoPrefix), StringComparer.OrdinalIgnoreCase);
+            }
+
+            if (result1 != null)
             {
-                return null;
+                if (result2 != null)
+                {
+                    result1.UnionWith(result2);
+                }
+                return result1;
             }
 
-            return new HashSet<string>(ExpressionShredder.SplitSemiColonSeparatedList(warnings), StringComparer.OrdinalIgnoreCase);
+            return result2;
         }
 
         private sealed class DedicatedThreadsTaskScheduler : TaskScheduler

src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs

@@ -27,7 +27,8 @@ internal static class ChangeWaves
     {
         internal static readonly Version Wave17_10 = new Version(17, 10);
         internal static readonly Version Wave17_12 = new Version(17, 12);
-        internal static readonly Version[] AllWaves = { Wave17_10, Wave17_12 };
+        internal static readonly Version Wave17_14 = new Version(17, 14);
+        internal static readonly Version[] AllWaves = { Wave17_10, Wave17_12, Wave17_14 };
 
         /// <summary>
         /// Special value indicating that all features behind all Change Waves should be enabled.

src/Framework/ChangeWaves.cs

@@ -335,7 +335,7 @@ public IEnumerable? Items
         }
 
         // Following 3 properties are intended only for internal transfer - to properly communicate the warn as error/msg
-        //  from the worker node, to the main node - that may be producing the buildcheck diagnostics.
+        //  from the worker node, to the main node.
         // They are not going to be in a binlog (at least not as of now).
 
         internal ISet<string>? WarningsAsErrors { get; set; }

src/Framework/ProjectStartedEventArgs.cs

@@ -28,25 +28,30 @@ internal static class MSBuildConstants
         /// </summary>
         internal const string SdksPath = "MSBuildSDKsPath";
 
+        /// <summary>
+        ///  The prefix that was originally used. Now extracted out for the purpose of allowing even the non-prefixed variant.
+        /// </summary>
+        internal const string MSBuildPrefix = "MSBuild";
+
         /// <summary>
         /// Name of the property that indicates that all warnings should be treated as errors.
         /// </summary>
-        internal const string TreatWarningsAsErrors = "MSBuildTreatWarningsAsErrors";
+        internal const string TreatWarningsAsErrors = "TreatWarningsAsErrors";
 
         /// <summary>
         /// Name of the property that indicates a list of warnings to treat as errors.
         /// </summary>
-        internal const string WarningsAsErrors = "MSBuildWarningsAsErrors";
+        internal const string WarningsAsErrors = "WarningsAsErrors";
 
         /// <summary>
         /// Name of the property that indicates a list of warnings to not treat as errors.
         /// </summary>
-        internal const string WarningsNotAsErrors = "MSBuildWarningsNotAsErrors";
+        internal const string WarningsNotAsErrors = "WarningsNotAsErrors";
 
         /// <summary>
         /// Name of the property that indicates the list of warnings to treat as messages.
         /// </summary>
-        internal const string WarningsAsMessages = "MSBuildWarningsAsMessages";
+        internal const string WarningsAsMessages = "WarningsAsMessages";
 
         /// <summary>
         /// The name of the environment variable that users can specify to override where NuGet assemblies are loaded from in the NuGetSdkResolver.

src/Shared/Constants.cs

@@ -780,9 +780,9 @@ public static void BuildProjectExpectSuccess(
         /// </summary>
         /// <param name="projectContents">The project file content in string format.</param>
         /// <returns>The <see cref="MockLogger"/> that was used during evaluation and build.</returns>
-        public static MockLogger BuildProjectExpectFailure(string projectContents)
+        public static MockLogger BuildProjectExpectFailure(string projectContents, ITestOutputHelper testOutputHelper = null)
         {
-            MockLogger logger = new MockLogger();
+            MockLogger logger = new MockLogger(testOutputHelper);
             BuildProjectExpectFailure(projectContents, logger);
             return logger;
         }