diff --git a/documentation/wiki/ChangeWaves.md b/documentation/wiki/ChangeWaves.md index c29f35c53b3..09e7ca1394c 100644 --- a/documentation/wiki/ChangeWaves.md +++ b/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) diff --git a/eng/Versions.props b/eng/Versions.props index 26d0e2b84c1..73336557265 100644 --- a/eng/Versions.props +++ b/eng/Versions.props @@ -2,7 +2,7 @@ - 17.12.15release + 17.12.16release 17.11.4 15.1.0.0 preview diff --git a/src/Build.UnitTests/WarningsAsMessagesAndErrors_Tests.cs b/src/Build.UnitTests/WarningsAsMessagesAndErrors_Tests.cs index 6041fbc45ac..db2d9eab3ad 100644 --- a/src/Build.UnitTests/WarningsAsMessagesAndErrors_Tests.cs +++ b/src/Build.UnitTests/WarningsAsMessagesAndErrors_Tests.cs @@ -35,19 +35,6 @@ public void TreatAllWarningsAsErrors() ObjectModelHelpers.BuildProjectExpectSuccess(GetTestProject(treatAllWarningsAsErrors: false)); } - [Fact] - public void TreatAllWarningsAsErrorsNoPrefix() - { - MockLogger logger = ObjectModelHelpers.BuildProjectExpectFailure(GetTestProject(customProperties: new Dictionary - { - {"TreatWarningsAsErrors", "true"}, - })); - - VerifyBuildErrorEvent(logger); - - ObjectModelHelpers.BuildProjectExpectSuccess(GetTestProject(treatAllWarningsAsErrors: false)); - } - /// /// https://github.com/dotnet/msbuild/issues/2667 /// @@ -104,6 +91,22 @@ public void TreatWarningsAsErrorsWhenSpecifiedIndirectly() VerifyBuildErrorEvent(logger); } + [Fact] + public void TreatWarningsAsErrorsWhenSpecifiedThroughAdditiveProperty() + { + MockLogger logger = ObjectModelHelpers.BuildProjectExpectFailure( + GetTestProject( + customProperties: new List> + { + new KeyValuePair("MSBuildWarningsAsErrors", "123"), + new KeyValuePair("MSBuildWarningsAsErrors", $@"$(MSBuildWarningsAsErrors); + {ExpectedEventCode.ToLowerInvariant()}"), + new KeyValuePair("MSBuildWarningsAsErrors", "$(MSBuildWarningsAsErrors);ABC") + })); + + VerifyBuildErrorEvent(logger); + } + [Fact] public void NotTreatWarningsAsErrorsWhenCodeNotSpecified() { @@ -174,99 +177,22 @@ public void TreatWarningsAsMessagesWhenSpecifiedIndirectly() VerifyBuildMessageEvent(logger); } - [Theory] - [InlineData(true)] - [InlineData(false)] - public void TreatWarningsAsMessagesWhenSpecifiedThroughAdditiveProperty(bool usePrefix) - { - string prefix = usePrefix ? "MSBuild" : ""; - MockLogger logger = ObjectModelHelpers.BuildProjectExpectSuccess( - GetTestProject( - customProperties: new List> - { - new KeyValuePair($"{prefix}WarningsAsMessages", "123"), - new KeyValuePair($"{prefix}WarningsAsMessages", $@"$({prefix}WarningsAsMessages); - {ExpectedEventCode.ToLowerInvariant()}"), - new KeyValuePair($"{prefix}WarningsAsMessages", $"$({prefix}WarningsAsMessages);ABC") - })); - - VerifyBuildMessageEvent(logger); - } - [Fact] - /// - /// This is for chaining the properties together via addition. - /// Furthermore it is intended to check if the prefix and no prefix variant interacts properly with each other. - /// - public void TreatWarningsAsMessagesWhenSpecifiedThroughAdditivePropertyCombination() + public void TreatWarningsAsMessagesWhenSpecifiedThroughAdditiveProperty() { MockLogger logger = ObjectModelHelpers.BuildProjectExpectSuccess( GetTestProject( customProperties: new List> { new KeyValuePair("MSBuildWarningsAsMessages", "123"), - new KeyValuePair("WarningsAsMessages", $@"$(MSBuildWarningsAsMessages); + new KeyValuePair("MSBuildWarningsAsMessages", $@"$(MSBuildWarningsAsMessages); {ExpectedEventCode.ToLowerInvariant()}"), - new KeyValuePair("MSBuildWarningsAsMessages", "$(WarningsAsMessages);ABC") + new KeyValuePair("MSBuildWarningsAsMessages", "$(MSBuildWarningsAsMessages);ABC") })); VerifyBuildMessageEvent(logger); } - [Fact] - public void TreatWarningsNotAsErrorsWhenSpecifiedThroughAdditivePropertyCombination() - { - MockLogger logger = ObjectModelHelpers.BuildProjectExpectSuccess( - GetTestProject( - customProperties: new List> - { - new KeyValuePair("MSBuildWarningsNotAsErrors", "123"), - new KeyValuePair("WarningsNotAsErrors", $@"$(MSBuildWarningsNotAsErrors); - {ExpectedEventCode.ToLowerInvariant()}"), - new KeyValuePair("MSBuildWarningsNotAsErrors", "$(WarningsNotAsErrors);ABC") - }), - _output); - - VerifyBuildWarningEvent(logger); - } - - [Theory] - [InlineData(true)] - [InlineData(false)] - public void TreatWarningsAsErrorsWhenSpecifiedThroughAdditiveProperty(bool MSBuildPrefix) - { - string prefix = MSBuildPrefix ? "MSBuild" : ""; - MockLogger logger = ObjectModelHelpers.BuildProjectExpectFailure( - GetTestProject( - customProperties: new List> - { - new KeyValuePair($@"{prefix}WarningsAsErrors", "123"), - new KeyValuePair($@"{prefix}WarningsAsErrors", $@"$({prefix}WarningsAsErrors); - {ExpectedEventCode.ToLowerInvariant()}"), - new KeyValuePair($@"{prefix}WarningsAsErrors", $@"$({prefix}WarningsAsErrors);ABC") - }), - _output); - - VerifyBuildErrorEvent(logger); - } - - [Fact] - public void TreatWarningsAsErrorsWhenSpecifiedThroughAdditivePropertyCombination() - { - MockLogger logger = ObjectModelHelpers.BuildProjectExpectFailure( - GetTestProject( - customProperties: new List> - { - new KeyValuePair("WarningsAsErrors", "123"), - new KeyValuePair("MSBuildWarningsAsErrors", $@"$(WarningsAsErrors); - {ExpectedEventCode.ToLowerInvariant()}"), - new KeyValuePair("WarningsAsErrors", "$(MSBuildWarningsAsErrors);ABC") - }), - _output); - - VerifyBuildErrorEvent(logger); - } - [Fact] public void NotTreatWarningsAsMessagesWhenCodeNotSpecified() { @@ -276,8 +202,7 @@ public void NotTreatWarningsAsMessagesWhenCodeNotSpecified() { new KeyValuePair("MSBuildWarningsAsMessages", "123"), new KeyValuePair("MSBuildWarningsAsMessages", "$(MSBuildWarningsAsMessages);ABC") - }), - _output); + })); VerifyBuildWarningEvent(logger); } @@ -348,33 +273,27 @@ private string GetTestProject(bool? treatAllWarningsAsErrors = null, string warn "; } - [Theory] [InlineData("MSB1235", "MSB1234", "MSB1234", "MSB1234", false)] // Log MSB1234, treat as error via MSBuildWarningsAsErrors [InlineData("MSB1235", "", "MSB1234", "MSB1234", true)] // Log MSB1234, expect MSB1234 as error via MSBuildTreatWarningsAsErrors [InlineData("MSB1234", "MSB1234", "MSB1234", "MSB4181", true)]// Log MSB1234, MSBuildWarningsAsMessages takes priority - [InlineData("MSB1235", "MSB1234", "MSB1234", "MSB1234", false, false)] // Log MSB1234, treat as error via BuildWarningsAsErrors - [InlineData("MSB1235", "", "MSB1234", "MSB1234", true, false)] // Log MSB1234, expect MSB1234 as error via BuildTreatWarningsAsErrors - [InlineData("MSB1234", "MSB1234", "MSB1234", "MSB4181", true, false)]// Log MSB1234, BuildWarningsAsMessages takes priority public void WarningsAsErrorsAndMessages_Tests(string WarningsAsMessages, string WarningsAsErrors, string WarningToLog, string LogShouldContain, - bool allWarningsAreErrors = false, - bool useMSPrefix = true) + bool allWarningsAreErrors = false) { using (TestEnvironment env = TestEnvironment.Create(_output)) { - var prefix = useMSPrefix ? "MSBuild" : ""; TransientTestProjectWithFiles proj = env.CreateTestProjectWithFiles($@" - <{prefix}TreatWarningsAsErrors>{allWarningsAreErrors} - <{prefix}WarningsAsMessages>{WarningsAsMessages} - <{prefix}WarningsAsErrors>{WarningsAsErrors} + {allWarningsAreErrors} + {WarningsAsMessages} + {WarningsAsErrors} @@ -391,83 +310,6 @@ public void WarningsAsErrorsAndMessages_Tests(string WarningsAsMessages, } } - [Theory] - - [InlineData(true)]// Log MSB1234, BuildWarningsNotAsErrors takes priority - [InlineData(false)] - public void WarningsNotAsErrorsAndMessages_Tests(bool useMSPrefix) - { - string Warning = "MSB1235"; - using (TestEnvironment env = TestEnvironment.Create(_output)) - { - string prefix = useMSPrefix ? "MSBuild" : ""; - TransientTestProjectWithFiles proj = env.CreateTestProjectWithFiles($@" - - - <{prefix}TreatWarningsAsErrors>true - <{prefix}WarningsNotAsErrors>{Warning} - - - - - "); - - MockLogger logger = proj.BuildProjectExpectSuccess(); - - logger.WarningCount.ShouldBe(1); - logger.ErrorCount.ShouldBe(0); - - logger.AssertLogContains(Warning); - } - } - - - - [Theory] - [InlineData("TreatWarningsAsErrors", "true", false)] // All warnings are treated as errors - [InlineData("WarningsAsErrors", "MSB1007", false)] - [InlineData("WarningsAsMessages", "MSB1007", false)] - [InlineData("WarningsNotAsErrors", "MSB1007", true)] - [InlineData("WarningsNotAsErrors", "MSB1007", false)] - public void WarningsChangeWaveTest(string property, string propertyData, bool treatWarningsAsErrors) - { - using (TestEnvironment env = TestEnvironment.Create(_output)) - { - string warningCode = "MSB1007"; - string treatWarningsAsErrorsCodeProperty = treatWarningsAsErrors ? "true" : ""; - env.SetEnvironmentVariable("MSBUILDDISABLEFEATURESFROMVERSION", ChangeWaves.Wave17_14.ToString()); - TransientTestProjectWithFiles proj = env.CreateTestProjectWithFiles($@" - - - {treatWarningsAsErrorsCodeProperty} - <{property}>{propertyData} - - - - - "); - if (treatWarningsAsErrors) - { - // Since the "no prefix" variations can't do anything with the change wave disabled, this should always fail. - MockLogger logger = proj.BuildProjectExpectFailure(); - logger.ErrorCount.ShouldBe(1); - logger.AssertLogContains($"error {warningCode}"); - - logger.AssertLogContains(warningCode); - } - else - { - MockLogger logger = proj.BuildProjectExpectSuccess(); - - logger.WarningCount.ShouldBe(1); - logger.AssertLogContains($"warning {warningCode}"); - logger.ErrorCount.ShouldBe(0); - - logger.AssertLogContains(warningCode); - } - } - } - /// /// Item1 and Item2 log warnings and continue, item 3 logs a warn-> error and prevents item 4 from running in the batched build. /// @@ -529,11 +371,8 @@ public void TaskLogsWarningAsError_BatchedBuild() [Theory] [InlineData("MSB1234", false, 1, 1)] [InlineData("MSB0000", true, 0, 2)] - [InlineData("MSB1234", false, 1, 1, false)] - [InlineData("MSB0000", true, 0, 2, false)] - public void TaskReturnsTrue_Tests(string warningsAsErrors, bool treatAllWarningsAsErrors, int warningCountShouldBe, int errorCountShouldBe, bool useMSPrefix = true) + public void TaskReturnsTrue_Tests(string warningsAsErrors, bool treatAllWarningsAsErrors, int warningCountShouldBe, int errorCountShouldBe) { - string prefix = useMSPrefix ? "MSBuild" : ""; using (TestEnvironment env = TestEnvironment.Create(_output)) { TransientTestProjectWithFiles proj = env.CreateTestProjectWithFiles($@" @@ -541,8 +380,8 @@ public void TaskReturnsTrue_Tests(string warningsAsErrors, bool treatAllWarnings - <{prefix}TreatWarningsAsErrors>{treatAllWarningsAsErrors} - <{prefix}WarningsAsErrors>{warningsAsErrors} + {treatAllWarningsAsErrors} + {warningsAsErrors} diff --git a/src/Build/BackEnd/Components/Logging/LoggingService.cs b/src/Build/BackEnd/Components/Logging/LoggingService.cs index cfc289ad30e..b5ed777d161 100644 --- a/src/Build/BackEnd/Components/Logging/LoggingService.cs +++ b/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); diff --git a/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs b/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs index 77f3399d11f..bc364827269 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs @@ -1105,7 +1105,7 @@ private async Task 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 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,17 +1389,14 @@ 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 signals the IEventSourceSink to treat all warnings as errors loggingService.AddWarningsAsErrors(buildEventContext, new HashSet()); } else { - ISet warningsAsErrors = ParseWarningCodes(project.GetEngineRequiredPropertyValue(MSBuildConstants.MSBuildPrefix + MSBuildConstants.WarningsAsErrors), - project.GetEngineRequiredPropertyValue(MSBuildConstants.WarningsAsErrors)); + ISet warningsAsErrors = ParseWarningCodes(project.GetEngineRequiredPropertyValue(MSBuildConstants.WarningsAsErrors)); if (warningsAsErrors?.Count > 0) { @@ -1408,17 +1404,14 @@ private void ConfigureWarningsAsErrorsAndMessages() } } - ISet warningsNotAsErrors = ParseWarningCodes(project.GetEngineRequiredPropertyValue(MSBuildConstants.MSBuildPrefix + MSBuildConstants.WarningsNotAsErrors), - project.GetEngineRequiredPropertyValue(MSBuildConstants.WarningsNotAsErrors)); - + ISet warningsNotAsErrors = ParseWarningCodes(project.GetEngineRequiredPropertyValue(MSBuildConstants.WarningsNotAsErrors)); if (warningsNotAsErrors?.Count > 0) { loggingService.AddWarningsNotAsErrors(buildEventContext, warningsNotAsErrors); } - ISet warningsAsMessages = ParseWarningCodes(project.GetEngineRequiredPropertyValue(MSBuildConstants.MSBuildPrefix + MSBuildConstants.WarningsAsMessages), - project.GetEngineRequiredPropertyValue(MSBuildConstants.WarningsAsMessages)); + ISet warningsAsMessages = ParseWarningCodes(project.GetEngineRequiredPropertyValue(MSBuildConstants.WarningsAsMessages)); if (warningsAsMessages?.Count > 0) { @@ -1436,37 +1429,14 @@ private void ConfigureKnownImmutableFolders() } } - private static ISet ParseWarningCodes(string warnings, string warningsNoPrefix) + private static ISet ParseWarningCodes(string warnings) { - // When this changewave is rotated out and this gets deleted, please consider removing - // the $(NoWarn) - // and the two following lines from the msbuild/src/Tasks/Microsoft.Common.CurrentVersion.targets - if (!ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_14)) - { - warningsNoPrefix = null; - } - - HashSet result1 = null; - if (!String.IsNullOrWhiteSpace(warnings)) - { - result1 = new HashSet(ExpressionShredder.SplitSemiColonSeparatedList(warnings), StringComparer.OrdinalIgnoreCase); - } - HashSet result2 = null; - if (!String.IsNullOrWhiteSpace(warningsNoPrefix)) - { - result2 = new HashSet(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(ExpressionShredder.SplitSemiColonSeparatedList(warnings), StringComparer.OrdinalIgnoreCase); } private sealed class DedicatedThreadsTaskScheduler : TaskScheduler diff --git a/src/Framework/ChangeWaves.cs b/src/Framework/ChangeWaves.cs index 8e58f93835c..1d682c4fc75 100644 --- a/src/Framework/ChangeWaves.cs +++ b/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 }; /// /// Special value indicating that all features behind all Change Waves should be enabled. diff --git a/src/Framework/ProjectStartedEventArgs.cs b/src/Framework/ProjectStartedEventArgs.cs index 9d2bb8345ac..8dcf4330fb0 100644 --- a/src/Framework/ProjectStartedEventArgs.cs +++ b/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? WarningsAsErrors { get; set; } diff --git a/src/Shared/Constants.cs b/src/Shared/Constants.cs index 81d03ed9a0c..e0ac6bae417 100644 --- a/src/Shared/Constants.cs +++ b/src/Shared/Constants.cs @@ -28,30 +28,25 @@ internal static class MSBuildConstants /// internal const string SdksPath = "MSBuildSDKsPath"; - /// - /// The prefix that was originally used. Now extracted out for the purpose of allowing even the non-prefixed variant. - /// - internal const string MSBuildPrefix = "MSBuild"; - /// /// Name of the property that indicates that all warnings should be treated as errors. /// - internal const string TreatWarningsAsErrors = "TreatWarningsAsErrors"; + internal const string TreatWarningsAsErrors = "MSBuildTreatWarningsAsErrors"; /// /// Name of the property that indicates a list of warnings to treat as errors. /// - internal const string WarningsAsErrors = "WarningsAsErrors"; + internal const string WarningsAsErrors = "MSBuildWarningsAsErrors"; /// /// Name of the property that indicates a list of warnings to not treat as errors. /// - internal const string WarningsNotAsErrors = "WarningsNotAsErrors"; + internal const string WarningsNotAsErrors = "MSBuildWarningsNotAsErrors"; /// /// Name of the property that indicates the list of warnings to treat as messages. /// - internal const string WarningsAsMessages = "WarningsAsMessages"; + internal const string WarningsAsMessages = "MSBuildWarningsAsMessages"; /// /// The name of the environment variable that users can specify to override where NuGet assemblies are loaded from in the NuGetSdkResolver. diff --git a/src/UnitTests.Shared/ObjectModelHelpers.cs b/src/UnitTests.Shared/ObjectModelHelpers.cs index 73268e06989..1d560f54315 100644 --- a/src/UnitTests.Shared/ObjectModelHelpers.cs +++ b/src/UnitTests.Shared/ObjectModelHelpers.cs @@ -780,9 +780,9 @@ public static void BuildProjectExpectSuccess( /// /// The project file content in string format. /// The that was used during evaluation and build. - 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; }