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

* Revert "[vs17.12] Consistently respect unprefixed Warning-as-error/message/warning properties (#11007)"

This reverts commit 56cc2a0.

* Revert "Always respect warning-as-error properties"

This reverts commit ae660b7.

* Update Versions.props

* remove newline

* Update Versions.props

documentation/wiki/ChangeWaves.md

@@ -23,9 +23,6 @@ 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.15</VersionPrefix><DotNetFinalVersionKind>release</DotNetFinalVersionKind>
+    <VersionPrefix>17.12.16</VersionPrefix><DotNetFinalVersionKind>release</DotNetFinalVersionKind>
     <PackageValidationBaselineVersion>17.11.4</PackageValidationBaselineVersion>
     <AssemblyVersion>15.1.0.0</AssemblyVersion>
     <PreReleaseVersionLabel>preview</PreReleaseVersionLabel>

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

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

src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.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,7 +1155,6 @@ private async Task<BuildResult> BuildProject()
                     _requestEntry.Request.BuildEventContext);
             }
 
-
             try
             {
                 HandleProjectStarted(buildCheckManager);
@@ -1279,7 +1278,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 (loggingService != null && projectBuildEventContext != null)
+            if (buildCheckManager != null && loggingService != null && projectBuildEventContext != null)
             {
                 args.WarningsAsErrors = loggingService.GetWarningsAsErrors(projectBuildEventContext).ToHashSet(StringComparer.OrdinalIgnoreCase);
                 args.WarningsAsMessages = loggingService.GetWarningsAsMessages(projectBuildEventContext).ToHashSet(StringComparer.OrdinalIgnoreCase);
@@ -1390,35 +1389,29 @@ 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.MSBuildPrefix + MSBuildConstants.TreatWarningsAsErrors)?.Trim(), "true", StringComparison.OrdinalIgnoreCase) ||
-                    (String.Equals(project.GetEngineRequiredPropertyValue(MSBuildConstants.TreatWarningsAsErrors)?.Trim(), "true", StringComparison.OrdinalIgnoreCase) &&
-                     ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_14)))
+                if (String.Equals(project.GetEngineRequiredPropertyValue(MSBuildConstants.TreatWarningsAsErrors)?.Trim(), "true", StringComparison.OrdinalIgnoreCase))
                 {
                     // 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.MSBuildPrefix + MSBuildConstants.WarningsAsErrors),
-                                                                      project.GetEngineRequiredPropertyValue(MSBuildConstants.WarningsAsErrors));
+                    ISet<string> warningsAsErrors = ParseWarningCodes(project.GetEngineRequiredPropertyValue(MSBuildConstants.WarningsAsErrors));
 
                     if (warningsAsErrors?.Count > 0)
                     {
                         loggingService.AddWarningsAsErrors(buildEventContext, warningsAsErrors);
                     }
                 }
 
-                ISet<string> warningsNotAsErrors = ParseWarningCodes(project.GetEngineRequiredPropertyValue(MSBuildConstants.MSBuildPrefix + MSBuildConstants.WarningsNotAsErrors),
-                                                                     project.GetEngineRequiredPropertyValue(MSBuildConstants.WarningsNotAsErrors));
-
+                ISet<string> warningsNotAsErrors = ParseWarningCodes(project.GetEngineRequiredPropertyValue(MSBuildConstants.WarningsNotAsErrors));
 
                 if (warningsNotAsErrors?.Count > 0)
                 {
                     loggingService.AddWarningsNotAsErrors(buildEventContext, warningsNotAsErrors);
                 }
 
-                ISet<string> warningsAsMessages = ParseWarningCodes(project.GetEngineRequiredPropertyValue(MSBuildConstants.MSBuildPrefix + MSBuildConstants.WarningsAsMessages),
-                                                                    project.GetEngineRequiredPropertyValue(MSBuildConstants.WarningsAsMessages));
+                ISet<string> warningsAsMessages = ParseWarningCodes(project.GetEngineRequiredPropertyValue(MSBuildConstants.WarningsAsMessages));
 
                 if (warningsAsMessages?.Count > 0)
                 {
@@ -1436,37 +1429,14 @@ private void ConfigureKnownImmutableFolders()
             }
         }
 
-        private static ISet<string> ParseWarningCodes(string warnings, string warningsNoPrefix)
+        private static ISet<string> ParseWarningCodes(string 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)
+            if (String.IsNullOrWhiteSpace(warnings))
             {
-                if (result2 != null)
-                {
-                    result1.UnionWith(result2);
-                }
-                return result1;
+                return null;
             }
 
-            return result2;
+            return new HashSet<string>(ExpressionShredder.SplitSemiColonSeparatedList(warnings), StringComparer.OrdinalIgnoreCase);
         }
 
         private sealed class DedicatedThreadsTaskScheduler : TaskScheduler

src/Framework/ChangeWaves.cs

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

src/Framework/ProjectStartedEventArgs.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.
+        //  from the worker node, to the main node - that may be producing the buildcheck diagnostics.
         // They are not going to be in a binlog (at least not as of now).
 
         internal ISet<string>? WarningsAsErrors { get; set; }

src/Shared/Constants.cs

@@ -28,30 +28,25 @@ 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 = "TreatWarningsAsErrors";
+        internal const string TreatWarningsAsErrors = "MSBuildTreatWarningsAsErrors";
 
         /// <summary>
         /// Name of the property that indicates a list of warnings to treat as errors.
         /// </summary>
-        internal const string WarningsAsErrors = "WarningsAsErrors";
+        internal const string WarningsAsErrors = "MSBuildWarningsAsErrors";
 
         /// <summary>
         /// Name of the property that indicates a list of warnings to not treat as errors.
         /// </summary>
-        internal const string WarningsNotAsErrors = "WarningsNotAsErrors";
+        internal const string WarningsNotAsErrors = "MSBuildWarningsNotAsErrors";
 
         /// <summary>
         /// Name of the property that indicates the list of warnings to treat as messages.
         /// </summary>
-        internal const string WarningsAsMessages = "WarningsAsMessages";
+        internal const string WarningsAsMessages = "MSBuildWarningsAsMessages";
 
         /// <summary>
         /// The name of the environment variable that users can specify to override where NuGet assemblies are loaded from in the NuGetSdkResolver.

src/UnitTests.Shared/ObjectModelHelpers.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, ITestOutputHelper testOutputHelper = null)
+        public static MockLogger BuildProjectExpectFailure(string projectContents)
         {
-            MockLogger logger = new MockLogger(testOutputHelper);
+            MockLogger logger = new MockLogger();
             BuildProjectExpectFailure(projectContents, logger);
             return logger;
         }