fixes #8958 TerminalLogger in .NET 8.0.100-preview.6 issues audible alerts on iTerm2 (#9060)

vlada-shubina · web-flow · commit ce59c700bf33 · 2023-07-28T11:01:13.000+08:00
fixes #8958 Context TerminalLogger in .NET 8.0.100-preview.6 issues audible alerts on iTerm2 Iterm2 treats ;9 code differently from ConEmu: https://iterm2.com/documentation-escape-codes.html Changes Made Disabled progress reporting on Mac Testing Tested manually on Mac Notes
diff --git a/.editorconfig b/.editorconfig
@@ -15,6 +15,9 @@ trim_trailing_whitespace = true
 [project.json]
 indent_size = 2
 
+[*.txt]
+insert_final_newline = false
+
 # C# files
 [*.cs]
 # New line preferences
diff --git a/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.DisplayNodesOverwritesWithNewTargetFramework.Linux.verified.txt b/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.DisplayNodesOverwritesWithNewTargetFramework.Linux.verified.txt
@@ -2,4 +2,4 @@
   project [36;1mtfName[m Build (0.0s)
 [?25h[?25l[2F
   project [36;1mtf2[m Build (0.0s)[K
-[?25h
+[?25h
diff --git a/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.DisplayNodesOverwritesWithNewTargetFramework.OSX.verified.txt b/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.DisplayNodesOverwritesWithNewTargetFramework.OSX.verified.txt
@@ -0,0 +1,5 @@
+﻿[?25l[1F
+  project [36;1mtfName[m Build (0.0s)
+[?25h[?25l[2F
+  project [36;1mtf2[m Build (0.0s)[K
+[?25h
diff --git a/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.DisplayNodesOverwritesWithNewTargetFramework.Windows.verified.txt b/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.DisplayNodesOverwritesWithNewTargetFramework.Windows.verified.txt
@@ -0,0 +1,5 @@
+﻿]9;4;3;\[?25l[1F
+  project [36;1mtfName[m Build (0.0s)
+[?25h[?25l[2F
+  project [36;1mtf2[m Build (0.0s)[K
+[?25h
diff --git a/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.DisplayNodesShowsCurrent.Linux.verified.txt b/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.DisplayNodesShowsCurrent.Linux.verified.txt
@@ -1,3 +1,3 @@
 ﻿]9;4;3;\[?25l[1F
   project Build (0.0s)
-[?25h
+[?25h
diff --git a/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.DisplayNodesShowsCurrent.OSX.verified.txt b/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.DisplayNodesShowsCurrent.OSX.verified.txt
@@ -0,0 +1,3 @@
+﻿[?25l[1F
+  project Build (0.0s)
+[?25h
diff --git a/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.DisplayNodesShowsCurrent.Windows.verified.txt b/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.DisplayNodesShowsCurrent.Windows.verified.txt
@@ -0,0 +1,3 @@
+﻿]9;4;3;\[?25l[1F
+  project Build (0.0s)
+[?25h
diff --git a/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintBuildSummary_Failed.Linux.verified.txt b/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintBuildSummary_Failed.Linux.verified.txt
@@ -1,4 +1,4 @@
 ﻿]9;4;3;\[?25l[1F
 [?25h
 Build [31;1mfailed[m in 0.0s
-]9;4;0;\
+]9;4;0;\
diff --git a/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintBuildSummary_Failed.OSX.verified.txt b/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintBuildSummary_Failed.OSX.verified.txt
@@ -0,0 +1,3 @@
+﻿[?25l[1F
+[?25h
+Build [31;1mfailed[m in 0.0s
diff --git a/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintBuildSummary_Failed.Windows.verified.txt b/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintBuildSummary_Failed.Windows.verified.txt
@@ -0,0 +1,4 @@
+﻿]9;4;3;\[?25l[1F
+[?25h
+Build [31;1mfailed[m in 0.0s
+]9;4;0;\
diff --git a/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintBuildSummary_FailedWithErrors.Linux.verified.txt b/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintBuildSummary_FailedWithErrors.Linux.verified.txt
@@ -3,4 +3,4 @@
 [?25l[1F
 [?25h
 Build [31;1mfailed with errors[m in 0.0s
-]9;4;0;\
+]9;4;0;\
diff --git a/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintBuildSummary_FailedWithErrors.OSX.verified.txt b/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintBuildSummary_FailedWithErrors.OSX.verified.txt
@@ -0,0 +1,5 @@
+﻿  project [31;1mfailed with errors[m (0.0s)
+[31;1m    ❌︎[7D[6C MSBUILD : error : Error![m
+[?25l[1F
+[?25h
+Build [31;1mfailed with errors[m in 0.0s
diff --git a/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintBuildSummary_FailedWithErrors.Windows.verified.txt b/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintBuildSummary_FailedWithErrors.Windows.verified.txt
@@ -0,0 +1,6 @@
+﻿]9;4;3;\  project [31;1mfailed with errors[m (0.0s)
+[31;1m    ❌︎[7D[6C MSBUILD : error : Error![m
+[?25l[1F
+[?25h
+Build [31;1mfailed with errors[m in 0.0s
+]9;4;0;\
diff --git a/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintBuildSummary_SucceededWithWarnings.Linux.verified.txt b/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintBuildSummary_SucceededWithWarnings.Linux.verified.txt
@@ -3,4 +3,4 @@
 [?25l[1F
 [?25h
 Build [33;1msucceeded with warnings[m in 0.0s
-]9;4;0;\
+]9;4;0;\
diff --git a/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintBuildSummary_SucceededWithWarnings.OSX.verified.txt b/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintBuildSummary_SucceededWithWarnings.OSX.verified.txt
@@ -0,0 +1,5 @@
+﻿  project [33;1msucceeded with warnings[m (0.0s)
+[33;1m    ⚠︎[7D[6C MSBUILD : warning : Warning![m
+[?25l[1F
+[?25h
+Build [33;1msucceeded with warnings[m in 0.0s
diff --git a/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintBuildSummary_SucceededWithWarnings.Windows.verified.txt b/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintBuildSummary_SucceededWithWarnings.Windows.verified.txt
@@ -0,0 +1,6 @@
+﻿]9;4;3;\  project [33;1msucceeded with warnings[m (0.0s)
+[33;1m    ⚠︎[7D[6C MSBUILD : warning : Warning![m
+[?25l[1F
+[?25h
+Build [33;1msucceeded with warnings[m in 0.0s
+]9;4;0;\
diff --git a/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintsBuildSummary_Succeeded.Linux.verified.txt b/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintsBuildSummary_Succeeded.Linux.verified.txt
@@ -1,4 +1,4 @@
 ﻿]9;4;3;\[?25l[1F
 [?25h
 Build [32;1msucceeded[m in 0.0s
-]9;4;0;\
+]9;4;0;\
diff --git a/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintsBuildSummary_Succeeded.OSX.verified.txt b/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintsBuildSummary_Succeeded.OSX.verified.txt
@@ -0,0 +1,3 @@
+﻿[?25l[1F
+[?25h
+Build [32;1msucceeded[m in 0.0s
diff --git a/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintsBuildSummary_Succeeded.Windows.verified.txt b/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintsBuildSummary_Succeeded.Windows.verified.txt
@@ -0,0 +1,4 @@
+﻿]9;4;3;\[?25l[1F
+[?25h
+Build [32;1msucceeded[m in 0.0s
+]9;4;0;\
diff --git a/src/MSBuild.UnitTests/TerminalLogger_Tests.cs b/src/MSBuild.UnitTests/TerminalLogger_Tests.cs
@@ -212,7 +212,7 @@ public Task PrintsBuildSummary_Succeeded()
         {
             InvokeLoggerCallbacksForSimpleProject(succeeded: true, () => { });
 
-            return Verify(_outputWriter.ToString(), _settings);
+            return Verify(_outputWriter.ToString(), _settings).UniqueForOSPlatform();
         }
 
         [Fact]
@@ -223,14 +223,14 @@ public Task PrintBuildSummary_SucceededWithWarnings()
                 WarningRaised?.Invoke(_eventSender, MakeWarningEventArgs("Warning!"));
             });
 
-            return Verify(_outputWriter.ToString(), _settings);
+            return Verify(_outputWriter.ToString(), _settings).UniqueForOSPlatform();
         }
 
         [Fact]
         public Task PrintBuildSummary_Failed()
         {
             InvokeLoggerCallbacksForSimpleProject(succeeded: false, () => { });
-            return Verify(_outputWriter.ToString(), _settings);
+            return Verify(_outputWriter.ToString(), _settings).UniqueForOSPlatform();
         }
 
         [Fact]
@@ -241,7 +241,7 @@ public Task PrintBuildSummary_FailedWithErrors()
                ErrorRaised?.Invoke(_eventSender, MakeErrorEventArgs("Error!"));
            });
 
-           return Verify(_outputWriter.ToString(), _settings);
+            return Verify(_outputWriter.ToString(), _settings).UniqueForOSPlatform();
         }
 
         #endregion
@@ -253,7 +253,7 @@ public void DisplayNodesShowsCurrent()
             {
                 _terminallogger.DisplayNodes();
 
-                await Verify(_outputWriter.ToString(), _settings);
+                await Verify(_outputWriter.ToString(), _settings).UniqueForOSPlatform();
             });
         }
 
@@ -283,7 +283,7 @@ public async Task DisplayNodesOverwritesWithNewTargetFramework()
 
             _terminallogger.DisplayNodes();
 
-            await Verify(_outputWriter.ToString(), _settings);
+            await Verify(_outputWriter.ToString(), _settings).UniqueForOSPlatform();
         }
     }
 }
diff --git a/src/MSBuild/TerminalLogger/AnsiCodes.cs b/src/MSBuild/TerminalLogger/AnsiCodes.cs
@@ -95,13 +95,23 @@ internal static class AnsiCodes
     public const string ShowCursor = "\x1b[?25h";
 
     /// <summary>
-    /// Set progress state to a busy spinner.
+    /// Set progress state to a busy spinner. <br/>
+    /// Note: this code works only on ConEmu terminals, and conflicts with push a notification code on iTerm2.
     /// </summary>
+    /// <remarks>
+    /// <see href="https://conemu.github.io/en/AnsiEscapeCodes.html#ConEmu_specific_OSC">ConEmu specific OSC codes.</see><br/>
+    /// <see href="https://iterm2.com/documentation-escape-codes.html">iTerm2 proprietary escape codes.</see>
+    /// </remarks>
     public const string SetProgressIndeterminate = "\x1b]9;4;3;\x1b\\";
 
     /// <summary>
-    /// Remove progress state, restoring taskbar status to normal.
+    /// Remove progress state, restoring taskbar status to normal. <br/>
+    /// Note: this code works only on ConEmu terminals, and conflicts with push a notification code on iTerm2.
     /// </summary>
+    /// <remarks>
+    /// <see href="https://conemu.github.io/en/AnsiEscapeCodes.html#ConEmu_specific_OSC">ConEmu specific OSC codes.</see><br/>
+    /// <see href="https://iterm2.com/documentation-escape-codes.html">iTerm2 proprietary escape codes.</see>
+    /// </remarks>
     public const string RemoveProgress = "\x1b]9;4;0;\x1b\\";
 
     public static string Colorize(string? s, TerminalColor color)
diff --git a/src/MSBuild/TerminalLogger/ITerminal.cs b/src/MSBuild/TerminalLogger/ITerminal.cs
@@ -20,6 +20,11 @@ internal interface ITerminal : IDisposable
     /// </summary>
     int Height { get; }
 
+    /// <summary>
+    /// <see langword="true"/> if the terminal emulator supports progress reporting.
+    /// </summary>
+    bool SupportsProgressReporting { get; }
+
     /// <summary>
     /// Starts buffering the text passed via the <c>Write*</c> methods.
     /// </summary>
diff --git a/src/MSBuild/TerminalLogger/Terminal.cs b/src/MSBuild/TerminalLogger/Terminal.cs
@@ -3,6 +3,7 @@
 
 using System;
 using System.IO;
+using System.Runtime.InteropServices;
 using System.Text;
 #if NETFRAMEWORK
 using Microsoft.Build.Shared;
@@ -62,6 +63,12 @@ public int Width
         }
     }
 
+    /// <inheritdoc/>
+    /// <remarks>
+    /// https://github.com/dotnet/msbuild/issues/8958: iTerm2 treats ;9 code to post a notification instead, so disable progress reporting on Mac.
+    /// </remarks>
+    public bool SupportsProgressReporting { get; } = !RuntimeInformation.IsOSPlatform(OSPlatform.OSX);
+
     public Terminal()
     {
         _originalOutputEncoding = Console.OutputEncoding;
diff --git a/src/MSBuild/TerminalLogger/TerminalLogger.cs b/src/MSBuild/TerminalLogger/TerminalLogger.cs
@@ -9,6 +9,7 @@
 using System.Threading;
 using Microsoft.Build.Framework;
 using Microsoft.Build.Shared;
+using System.Runtime.InteropServices;
 #if NETFRAMEWORK
 using Microsoft.IO;
 #else
@@ -235,7 +236,10 @@ private void BuildStarted(object sender, BuildStartedEventArgs e)
 
         _buildStartTime = e.Timestamp;
 
-        Terminal.Write(AnsiCodes.SetProgressIndeterminate);
+        if (Terminal.SupportsProgressReporting)
+        {
+            Terminal.Write(AnsiCodes.SetProgressIndeterminate);
+        }
     }
 
     /// <summary>
@@ -270,7 +274,11 @@ private void BuildFinished(object sender, BuildFinishedEventArgs e)
         }
         finally
         {
-            Terminal.Write(AnsiCodes.RemoveProgress);
+            if (Terminal.SupportsProgressReporting)
+            {
+                Terminal.Write(AnsiCodes.RemoveProgress);
+            }
+
             Terminal.EndUpdate();
         }
 

-Original file line number
+Diff line change
   project [36;1mtfName[m Build (0.0s)
 [?25h[?25l[2F
   project [36;1mtf2[m Build (0.0s)[K
 -[?25h
 +[?25h
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+[?25l[1F`
	`2`	`+ project Build (0.0s)`
	`3`	`+[?25h`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+]9;4;3;\[?25l[1F`
	`2`	`+ project Build (0.0s)`
	`3`	`+[?25h`