From a0da428f0e2835d751fcb5096cb79497b8f5e8bf Mon Sep 17 00:00:00 2001 From: Jan Krivanek Date: Wed, 6 Mar 2024 14:49:03 +0100 Subject: [PATCH 1/6] Do not print-out version info in terminal logger --- src/MSBuild/XMake.cs | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/MSBuild/XMake.cs b/src/MSBuild/XMake.cs index 3fc3238e3d2..a1656000fd6 100644 --- a/src/MSBuild/XMake.cs +++ b/src/MSBuild/XMake.cs @@ -2455,21 +2455,6 @@ private static bool ProcessCommandLineSwitches( } #endif - bool shouldShowLogo = !commandLineSwitches[CommandLineSwitches.ParameterlessSwitch.NoLogo] && - !commandLineSwitches.IsParameterizedSwitchSet(CommandLineSwitches.ParameterizedSwitch.Preprocess) && - !commandLineSwitches.IsParameterizedSwitchSet(CommandLineSwitches.ParameterizedSwitch.GetProperty) && - !commandLineSwitches.IsParameterizedSwitchSet(CommandLineSwitches.ParameterizedSwitch.GetItem) && - !commandLineSwitches.IsParameterizedSwitchSet(CommandLineSwitches.ParameterizedSwitch.GetTargetResult) && - !commandLineSwitches.IsParameterizedSwitchSet(CommandLineSwitches.ParameterizedSwitch.FeatureAvailability); - - // show copyright message if nologo switch is not set - // NOTE: we heed the nologo switch even if there are switch errors - if (!recursing && shouldShowLogo) - { - DisplayVersionMessage(); - } - - // Idle priority would prevent the build from proceeding as the user does normal actions. // This switch is processed early to capture both the command line case (main node should // also be low priority) and the Visual Studio case in which the main node starts and stays @@ -2684,6 +2669,21 @@ private static bool ProcessCommandLineSwitches( out profilerLogger, out enableProfiler); + bool shouldShowLogo = !commandLineSwitches[CommandLineSwitches.ParameterlessSwitch.NoLogo] && + !commandLineSwitches.IsParameterizedSwitchSet(CommandLineSwitches.ParameterizedSwitch.Preprocess) && + !commandLineSwitches.IsParameterizedSwitchSet(CommandLineSwitches.ParameterizedSwitch.GetProperty) && + !commandLineSwitches.IsParameterizedSwitchSet(CommandLineSwitches.ParameterizedSwitch.GetItem) && + !commandLineSwitches.IsParameterizedSwitchSet(CommandLineSwitches.ParameterizedSwitch.GetTargetResult) && + !commandLineSwitches.IsParameterizedSwitchSet(CommandLineSwitches.ParameterizedSwitch.FeatureAvailability) && + !useTerminalLogger; + + // show copyright message if nologo switch is not set + // NOTE: we heed the nologo switch even if there are switch errors + if (!recursing && shouldShowLogo) + { + DisplayVersionMessage(); + } + // We're finished with defining individual loggers' verbosity at this point, so we don't need to worry about messing them up. if (Traits.Instance.DebugEngine) { From f139605ad9c91ea65bd567506cf9d991793fef31 Mon Sep 17 00:00:00 2001 From: Jan Krivanek Date: Wed, 6 Mar 2024 18:51:14 +0100 Subject: [PATCH 2/6] Frontload the tl check --- src/MSBuild/XMake.cs | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/src/MSBuild/XMake.cs b/src/MSBuild/XMake.cs index a1656000fd6..db480d39baa 100644 --- a/src/MSBuild/XMake.cs +++ b/src/MSBuild/XMake.cs @@ -2455,6 +2455,24 @@ private static bool ProcessCommandLineSwitches( } #endif + bool useTerminalLogger = ProcessTerminalLoggerConfiguration(commandLineSwitches, out string aggregatedTerminalLoggerParameters); + + if (!recursing) + { + bool shouldShowLogo = !commandLineSwitches[CommandLineSwitches.ParameterlessSwitch.NoLogo] && + !commandLineSwitches.IsParameterizedSwitchSet(CommandLineSwitches.ParameterizedSwitch.Preprocess) && + !commandLineSwitches.IsParameterizedSwitchSet(CommandLineSwitches.ParameterizedSwitch.GetProperty) && + !commandLineSwitches.IsParameterizedSwitchSet(CommandLineSwitches.ParameterizedSwitch.GetItem) && + !commandLineSwitches.IsParameterizedSwitchSet(CommandLineSwitches.ParameterizedSwitch.GetTargetResult) && + !commandLineSwitches.IsParameterizedSwitchSet(CommandLineSwitches.ParameterizedSwitch.FeatureAvailability) && + !useTerminalLogger; + + if (shouldShowLogo) + { + DisplayVersionMessage(); + } + } + // Idle priority would prevent the build from proceeding as the user does normal actions. // This switch is processed early to capture both the command line case (main node should // also be low priority) and the Visual Studio case in which the main node starts and stays @@ -2643,8 +2661,6 @@ private static bool ProcessCommandLineSwitches( outputResultsCache = ProcessOutputResultsCache(commandLineSwitches); - bool useTerminalLogger = ProcessTerminalLoggerConfiguration(commandLineSwitches, out string aggregatedTerminalLoggerParameters); - // figure out which loggers are going to listen to build events string[][] groupedFileLoggerParameters = commandLineSwitches.GetFileLoggerParameters(); @@ -2669,21 +2685,6 @@ private static bool ProcessCommandLineSwitches( out profilerLogger, out enableProfiler); - bool shouldShowLogo = !commandLineSwitches[CommandLineSwitches.ParameterlessSwitch.NoLogo] && - !commandLineSwitches.IsParameterizedSwitchSet(CommandLineSwitches.ParameterizedSwitch.Preprocess) && - !commandLineSwitches.IsParameterizedSwitchSet(CommandLineSwitches.ParameterizedSwitch.GetProperty) && - !commandLineSwitches.IsParameterizedSwitchSet(CommandLineSwitches.ParameterizedSwitch.GetItem) && - !commandLineSwitches.IsParameterizedSwitchSet(CommandLineSwitches.ParameterizedSwitch.GetTargetResult) && - !commandLineSwitches.IsParameterizedSwitchSet(CommandLineSwitches.ParameterizedSwitch.FeatureAvailability) && - !useTerminalLogger; - - // show copyright message if nologo switch is not set - // NOTE: we heed the nologo switch even if there are switch errors - if (!recursing && shouldShowLogo) - { - DisplayVersionMessage(); - } - // We're finished with defining individual loggers' verbosity at this point, so we don't need to worry about messing them up. if (Traits.Instance.DebugEngine) { From 5a6c9cd070bcf0f8c1b8d661a56001ba0c1a6389 Mon Sep 17 00:00:00 2001 From: Jan Krivanek Date: Thu, 7 Mar 2024 13:17:30 +0100 Subject: [PATCH 3/6] Add clarifying comment, move logic --- src/MSBuild/XMake.cs | 40 ++++++++++++++++++++++------------------ 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/src/MSBuild/XMake.cs b/src/MSBuild/XMake.cs index db480d39baa..5e80bda9779 100644 --- a/src/MSBuild/XMake.cs +++ b/src/MSBuild/XMake.cs @@ -2456,22 +2456,7 @@ private static bool ProcessCommandLineSwitches( #endif bool useTerminalLogger = ProcessTerminalLoggerConfiguration(commandLineSwitches, out string aggregatedTerminalLoggerParameters); - - if (!recursing) - { - bool shouldShowLogo = !commandLineSwitches[CommandLineSwitches.ParameterlessSwitch.NoLogo] && - !commandLineSwitches.IsParameterizedSwitchSet(CommandLineSwitches.ParameterizedSwitch.Preprocess) && - !commandLineSwitches.IsParameterizedSwitchSet(CommandLineSwitches.ParameterizedSwitch.GetProperty) && - !commandLineSwitches.IsParameterizedSwitchSet(CommandLineSwitches.ParameterizedSwitch.GetItem) && - !commandLineSwitches.IsParameterizedSwitchSet(CommandLineSwitches.ParameterizedSwitch.GetTargetResult) && - !commandLineSwitches.IsParameterizedSwitchSet(CommandLineSwitches.ParameterizedSwitch.FeatureAvailability) && - !useTerminalLogger; - - if (shouldShowLogo) - { - DisplayVersionMessage(); - } - } + DisplayVersionMessageIfNeeded(recursing, useTerminalLogger, commandLineSwitches); // Idle priority would prevent the build from proceeding as the user does normal actions. // This switch is processed early to capture both the command line case (main node should @@ -4466,9 +4451,28 @@ private static void ThrowInvalidToolsVersionInitializationException(IEnumerable< /// /// Displays the application version message/logo. /// - private static void DisplayVersionMessage() + private static void DisplayVersionMessageIfNeeded(bool recursing, bool useTerminalLogger, CommandLineSwitches commandLineSwitches) { - Console.WriteLine(ResourceUtilities.FormatResourceStringStripCodeAndKeyword("MSBuildVersionMessage", ProjectCollection.DisplayVersion, NativeMethods.FrameworkName)); + if (recursing) + { + return; + } + + // Show the versioning information if the user has not disabled it or msbuild is not running in a mode + // where it is not appropriate to show the versioning information (information querying mode that can be plugged into CLI scripts, + // terminal logger mode, where we want to display only the most relevant info, while output is not meant for investigation). + bool shouldShowLogo = !commandLineSwitches[CommandLineSwitches.ParameterlessSwitch.NoLogo] && + !commandLineSwitches.IsParameterizedSwitchSet(CommandLineSwitches.ParameterizedSwitch.Preprocess) && + !commandLineSwitches.IsParameterizedSwitchSet(CommandLineSwitches.ParameterizedSwitch.GetProperty) && + !commandLineSwitches.IsParameterizedSwitchSet(CommandLineSwitches.ParameterizedSwitch.GetItem) && + !commandLineSwitches.IsParameterizedSwitchSet(CommandLineSwitches.ParameterizedSwitch.GetTargetResult) && + !commandLineSwitches.IsParameterizedSwitchSet(CommandLineSwitches.ParameterizedSwitch.FeatureAvailability) && + !useTerminalLogger; + + if (shouldShowLogo) + { + Console.WriteLine(ResourceUtilities.FormatResourceStringStripCodeAndKeyword("MSBuildVersionMessage", ProjectCollection.DisplayVersion, NativeMethods.FrameworkName)); + } } /// From d48554abd4b64f127e2725bf105eff8eff013bf5 Mon Sep 17 00:00:00 2001 From: Jan Krivanek Date: Tue, 12 Mar 2024 17:05:53 +0100 Subject: [PATCH 4/6] Add comment and E2E unit tests --- src/MSBuild.UnitTests/XMake_Tests.cs | 36 ++++++++++++++++++++++++++++ src/MSBuild/XMake.cs | 1 + 2 files changed, 37 insertions(+) diff --git a/src/MSBuild.UnitTests/XMake_Tests.cs b/src/MSBuild.UnitTests/XMake_Tests.cs index 49c3a8792d5..2c3ced849e3 100644 --- a/src/MSBuild.UnitTests/XMake_Tests.cs +++ b/src/MSBuild.UnitTests/XMake_Tests.cs @@ -12,6 +12,7 @@ using System.Runtime.InteropServices; using System.Threading; using Microsoft.Build.CommandLine; +using Microsoft.Build.Evaluation; using Microsoft.Build.Framework; using Microsoft.Build.Logging; using Microsoft.Build.Shared; @@ -2605,6 +2606,41 @@ public override bool Execute() } } + [Theory] + [InlineData("", true)] + [InlineData("/tl:true", false)] + [InlineData("/nologo", false)] + [InlineData("/getProperty:p", false)] + public void EndToEndVersionMessage(string arguments, bool shouldContainVersionMessage) + { + using TestEnvironment testEnvironment = UnitTests.TestEnvironment.Create(); + + string projectContents = ObjectModelHelpers.CleanupFileContents(""" + + + + + """); + + TransientTestProjectWithFiles testProject = testEnvironment.CreateTestProjectWithFiles(projectContents); + + RunnerUtilities.ExecMSBuild($"{arguments} \"{testProject.ProjectFile}\"", out bool success, _output); + success.ShouldBeTrue(); + + string expectedVersionString = + ResourceUtilities.FormatResourceStringStripCodeAndKeyword("MSBuildVersionMessage", + ProjectCollection.DisplayVersion, NativeMethodsShared.FrameworkName); + + if (shouldContainVersionMessage) + { + ((Xunit.Sdk.TestOutputHelper)_output).Output.ShouldContain(expectedVersionString); + } + else + { + ((Xunit.Sdk.TestOutputHelper)_output).Output.ShouldNotContain(expectedVersionString); + } + } + [Theory] [InlineData("/v:diagnostic", MessageImportance.Low)] [InlineData("/v:detailed", MessageImportance.Low)] diff --git a/src/MSBuild/XMake.cs b/src/MSBuild/XMake.cs index 5e80bda9779..7305abef033 100644 --- a/src/MSBuild/XMake.cs +++ b/src/MSBuild/XMake.cs @@ -4461,6 +4461,7 @@ private static void DisplayVersionMessageIfNeeded(bool recursing, bool useTermin // Show the versioning information if the user has not disabled it or msbuild is not running in a mode // where it is not appropriate to show the versioning information (information querying mode that can be plugged into CLI scripts, // terminal logger mode, where we want to display only the most relevant info, while output is not meant for investigation). + // NOTE: response files are not reflected in this check. So enabling TL in response file will lead to version message still being shown. bool shouldShowLogo = !commandLineSwitches[CommandLineSwitches.ParameterlessSwitch.NoLogo] && !commandLineSwitches.IsParameterizedSwitchSet(CommandLineSwitches.ParameterizedSwitch.Preprocess) && !commandLineSwitches.IsParameterizedSwitchSet(CommandLineSwitches.ParameterizedSwitch.GetProperty) && From 5c21f5d8416815a45683c6416838670280bcbbe6 Mon Sep 17 00:00:00 2001 From: Jan Krivanek Date: Wed, 13 Mar 2024 07:47:06 +0100 Subject: [PATCH 5/6] Improve test --- src/MSBuild.UnitTests/XMake_Tests.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/MSBuild.UnitTests/XMake_Tests.cs b/src/MSBuild.UnitTests/XMake_Tests.cs index 2c3ced849e3..4dc5c70b946 100644 --- a/src/MSBuild.UnitTests/XMake_Tests.cs +++ b/src/MSBuild.UnitTests/XMake_Tests.cs @@ -2624,7 +2624,7 @@ public void EndToEndVersionMessage(string arguments, bool shouldContainVersionMe TransientTestProjectWithFiles testProject = testEnvironment.CreateTestProjectWithFiles(projectContents); - RunnerUtilities.ExecMSBuild($"{arguments} \"{testProject.ProjectFile}\"", out bool success, _output); + string output = RunnerUtilities.ExecMSBuild($"{arguments} \"{testProject.ProjectFile}\"", out bool success, _output); success.ShouldBeTrue(); string expectedVersionString = @@ -2633,11 +2633,11 @@ public void EndToEndVersionMessage(string arguments, bool shouldContainVersionMe if (shouldContainVersionMessage) { - ((Xunit.Sdk.TestOutputHelper)_output).Output.ShouldContain(expectedVersionString); + output.ShouldContain(expectedVersionString); } else { - ((Xunit.Sdk.TestOutputHelper)_output).Output.ShouldNotContain(expectedVersionString); + output.ShouldNotContain(expectedVersionString); } } From bce7551d54da3262e450dba2287104dbb9762785 Mon Sep 17 00:00:00 2001 From: Jan Krivanek Date: Wed, 13 Mar 2024 13:32:25 +0100 Subject: [PATCH 6/6] Ensure unrelated test cleanup --- src/MSBuild.UnitTests/XMake_Tests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/MSBuild.UnitTests/XMake_Tests.cs b/src/MSBuild.UnitTests/XMake_Tests.cs index 4dc5c70b946..c309cbde37c 100644 --- a/src/MSBuild.UnitTests/XMake_Tests.cs +++ b/src/MSBuild.UnitTests/XMake_Tests.cs @@ -891,6 +891,7 @@ public void SetConsoleUICulture() // Restore the current UI culture back to the way it was at the beginning of this unit test. thisThread.CurrentUICulture = originalUICulture; + MSBuildApp.SetConsoleUI(); }