From 078e520926e73d7a015f8b5b592065bdc6d7cbf5 Mon Sep 17 00:00:00 2001 From: Michal Pavlik Date: Wed, 29 Jun 2022 11:46:38 +0200 Subject: [PATCH 1/2] Fixed some commented issues --- .../BackEnd/RedirectConsoleWriter_Tests.cs | 15 +++++++------ src/Build/BackEnd/Client/MSBuildClient.cs | 21 ++++++++++--------- .../BackEnd/Client/MSBuildClientPacketPump.cs | 2 +- src/Build/BackEnd/Node/OutOfProcServerNode.cs | 14 ------------- src/Framework/MSBuildEventSource.cs | 6 ++++++ src/MSBuild/XMake.cs | 2 +- src/Shared/CommunicationsUtilities.cs | 11 ---------- src/Shared/NodeEndpointOutOfProcBase.cs | 1 - 8 files changed, 28 insertions(+), 44 deletions(-) diff --git a/src/Build.UnitTests/BackEnd/RedirectConsoleWriter_Tests.cs b/src/Build.UnitTests/BackEnd/RedirectConsoleWriter_Tests.cs index 121ea908677..cf5cd0b2823 100644 --- a/src/Build.UnitTests/BackEnd/RedirectConsoleWriter_Tests.cs +++ b/src/Build.UnitTests/BackEnd/RedirectConsoleWriter_Tests.cs @@ -3,9 +3,11 @@ // using System; +using System.IO; using System.Text; using System.Threading.Tasks; using Microsoft.Build.Experimental; +using Shouldly; using Xunit; namespace Microsoft.Build.Engine.UnitTests.BackEnd @@ -16,14 +18,15 @@ public class RedirectConsoleWriter_Tests public async Task EmitConsoleMessages() { StringBuilder sb = new StringBuilder(); - var writer = OutOfProcServerNode.RedirectConsoleWriter.Create(text => sb.Append(text)); - writer.WriteLine("Line 1"); - await Task.Delay(300); - writer.Write("Line 2"); - writer.Dispose(); + using (TextWriter writer = OutOfProcServerNode.RedirectConsoleWriter.Create(text => sb.Append(text))) + { + writer.WriteLine("Line 1"); + await Task.Delay(300); + writer.Write("Line 2"); + } - Assert.Equal($"Line 1{Environment.NewLine}Line 2", sb.ToString()); + sb.ToString().ShouldBe($"Line 1{Environment.NewLine}Line 2"); } } } diff --git a/src/Build/BackEnd/Client/MSBuildClient.cs b/src/Build/BackEnd/Client/MSBuildClient.cs index 273cfd69f84..9b231b2eb31 100644 --- a/src/Build/BackEnd/Client/MSBuildClient.cs +++ b/src/Build/BackEnd/Client/MSBuildClient.cs @@ -133,6 +133,7 @@ public MSBuildClientExitResult Execute(string commandLine, CancellationToken can CommunicationsUtilities.Trace("Server was not running. Starting server now."); if (!TryLaunchServer()) { + _exitResult.MSBuildClientExitType = MSBuildClientExitType.LaunchError; return _exitResult; } } @@ -371,11 +372,11 @@ private bool TryLaunchServer() NodeLauncher nodeLauncher = new NodeLauncher(); CommunicationsUtilities.Trace("Starting Server..."); Process msbuildProcess = nodeLauncher.Start(_msbuildLocation, string.Join(" ", msBuildServerOptions)); - CommunicationsUtilities.Trace("Server started with PID: {0}", msbuildProcess?.Id); + CommunicationsUtilities.Trace($"Server started with PID: {msbuildProcess?.Id}"); } catch (Exception ex) { - CommunicationsUtilities.Trace("Failed to launch the msbuild server: {0}", ex); + CommunicationsUtilities.Trace($"Failed to launch the msbuild server: {ex}"); _exitResult.MSBuildClientExitType = MSBuildClientExitType.LaunchError; return false; } @@ -406,7 +407,7 @@ private ServerNodeBuildCommand GetServerNodeBuildCommand(string commandLine) } // We remove env variable used to invoke MSBuild server as that might be equal to 1, so we do not get an infinite recursion here. - envVars[Traits.UseMSBuildServerEnvVarName] = "0"; + envVars.Remove(Traits.UseMSBuildServerEnvVarName); return new ServerNodeBuildCommand( commandLine, @@ -437,8 +438,8 @@ private void HandleCancellation() /// private void HandlePacketPumpError(MSBuildClientPacketPump packetPump) { - CommunicationsUtilities.Trace("MSBuild client error: packet pump unexpectedly shut down: {0}", packetPump.PacketPumpException); - throw packetPump.PacketPumpException ?? new Exception("Packet pump unexpectedly shut down"); + CommunicationsUtilities.Trace($"MSBuild client error: packet pump unexpectedly shut down: {packetPump.PacketPumpException}"); + throw packetPump.PacketPumpException ?? new InternalErrorException("Packet pump unexpectedly shut down"); } /// @@ -478,7 +479,7 @@ private void HandleServerNodeConsoleWrite(ServerNodeConsoleWrite consoleWrite) private void HandleServerNodeBuildResult(ServerNodeBuildResult response) { - CommunicationsUtilities.Trace("Build response received: exit code {0}, exit type '{1}'", response.ExitCode, response.ExitType); + CommunicationsUtilities.Trace($"Build response received: exit code {response.ExitCode}, exit type '{response.ExitType}'"); _exitResult.MSBuildClientExitType = MSBuildClientExitType.Success; _exitResult.MSBuildAppExitTypeString = response.ExitType; _buildFinished = true; @@ -497,14 +498,14 @@ private bool TryConnectToServer(int timeout) int[] handshakeComponents = _handshake.RetrieveHandshakeComponents(); for (int i = 0; i < handshakeComponents.Length; i++) { - CommunicationsUtilities.Trace("Writing handshake part {0} ({1}) to pipe {2}", i, handshakeComponents[i], _pipeName); + CommunicationsUtilities.Trace($"Writing handshake part {i} ({handshakeComponents[i]}) to pipe {_pipeName}"); _nodeStream.WriteIntForHandshake(handshakeComponents[i]); } // This indicates that we have finished all the parts of our handshake; hopefully the endpoint has as well. _nodeStream.WriteEndOfHandshakeSignal(); - CommunicationsUtilities.Trace("Reading handshake from pipe {0}", _pipeName); + CommunicationsUtilities.Trace($"Reading handshake from pipe {_pipeName}"); #if NETCOREAPP2_1_OR_GREATER || MONO _nodeStream.ReadEndOfHandshakeSignal(false, 1000); @@ -512,11 +513,11 @@ private bool TryConnectToServer(int timeout) _nodeStream.ReadEndOfHandshakeSignal(false); #endif - CommunicationsUtilities.Trace("Successfully connected to pipe {0}...!", _pipeName); + CommunicationsUtilities.Trace($"Successfully connected to pipe {_pipeName}...!"); } catch (Exception ex) { - CommunicationsUtilities.Trace("Failed to connect to server: {0}", ex); + CommunicationsUtilities.Trace($"Failed to connect to server: {ex}"); _exitResult.MSBuildClientExitType = MSBuildClientExitType.ConnectionError; return false; } diff --git a/src/Build/BackEnd/Client/MSBuildClientPacketPump.cs b/src/Build/BackEnd/Client/MSBuildClientPacketPump.cs index b2c82c88ab6..b59c5a79e27 100644 --- a/src/Build/BackEnd/Client/MSBuildClientPacketPump.cs +++ b/src/Build/BackEnd/Client/MSBuildClientPacketPump.cs @@ -273,7 +273,7 @@ private void RunReadLoop(Stream localStream, ManualResetEvent localPacketPumpShu #if FEATURE_APM result = localStream.BeginRead(headerByte, 0, headerByte.Length, null, null); #else - readTask = CommunicationsUtilities.ReadAsync(localStream, headerByte, headerByte.Length); + readTask = CommunicationsUtilities.ReadAsync(localStream, headerByte, headerByte.Length); #endif } } diff --git a/src/Build/BackEnd/Node/OutOfProcServerNode.cs b/src/Build/BackEnd/Node/OutOfProcServerNode.cs index 0053b91705b..a338b2cb078 100644 --- a/src/Build/BackEnd/Node/OutOfProcServerNode.cs +++ b/src/Build/BackEnd/Node/OutOfProcServerNode.cs @@ -2,7 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; -using System.Collections.Generic; using System.Collections.Concurrent; using System.IO; using System.Threading; @@ -57,18 +56,11 @@ public sealed class OutOfProcServerNode : INode, INodePacketFactory, INodePacket /// private Exception? _shutdownException = null; - /// - /// Flag indicating if we should debug communications or not. - /// - private readonly bool _debugCommunications; - private string _serverBusyMutexName = default!; public OutOfProcServerNode(Func buildFunction) { _buildFunction = buildFunction; - new Dictionary(); - _debugCommunications = (Environment.GetEnvironmentVariable("MSBUILDDEBUGCOMM") == "1"); _receivedPackets = new ConcurrentQueue(); _packetReceivedEvent = new AutoResetEvent(false); @@ -243,12 +235,6 @@ private void OnLinkStatusChanged(INodeEndpoint endpoint, LinkStatus status) _shutdownEvent.Set(); break; - case LinkStatus.Inactive: - break; - - case LinkStatus.Active: - break; - default: break; } diff --git a/src/Framework/MSBuildEventSource.cs b/src/Framework/MSBuildEventSource.cs index 708f5f6a31b..afeb32973ad 100644 --- a/src/Framework/MSBuildEventSource.cs +++ b/src/Framework/MSBuildEventSource.cs @@ -53,6 +53,12 @@ public void ApplyLazyItemOperationsStart(string itemType) WriteEvent(1, itemType); } + [Event(1, Keywords = Keywords.All)] + public void ApplyLazyItemOperationsStart2(string itemType) + { + WriteEvent(1, itemType); + } + /// The type of the item being mutated. [Event(2, Keywords = Keywords.All)] public void ApplyLazyItemOperationsStop(string itemType) diff --git a/src/MSBuild/XMake.cs b/src/MSBuild/XMake.cs index 373e4a58aa3..5457f298aba 100644 --- a/src/MSBuild/XMake.cs +++ b/src/MSBuild/XMake.cs @@ -2697,8 +2697,8 @@ private static void StartLocalNode(CommandLineSwitches commandLineSwitches, bool QuotingUtilities.SplitUnquoted(commandLine).ToArray() #endif ); - exitCode = exitType == ExitType.Success ? 0 : 1; } + exitCode = exitType == ExitType.Success ? 0 : 1; return (exitCode, exitType.ToString()); diff --git a/src/Shared/CommunicationsUtilities.cs b/src/Shared/CommunicationsUtilities.cs index 50e34e2bc2a..4f3c09b3be3 100644 --- a/src/Shared/CommunicationsUtilities.cs +++ b/src/Shared/CommunicationsUtilities.cs @@ -148,17 +148,6 @@ internal ServerNodeHandshake(HandshakeOptions nodeType) { } - /// - /// Compute stable hash as integer - /// - private static int ComputeHandshakeHash(string fromString) - { - using var sha = SHA256.Create(); - var bytes = sha.ComputeHash(Encoding.UTF8.GetBytes(fromString)); - - return BitConverter.ToInt32(bytes, 0); - } - public override int[] RetrieveHandshakeComponents() { return new int[] diff --git a/src/Shared/NodeEndpointOutOfProcBase.cs b/src/Shared/NodeEndpointOutOfProcBase.cs index 0be21ce32c0..4c5a3357063 100644 --- a/src/Shared/NodeEndpointOutOfProcBase.cs +++ b/src/Shared/NodeEndpointOutOfProcBase.cs @@ -14,7 +14,6 @@ using Microsoft.Build.Shared; #if FEATURE_SECURITY_PERMISSIONS || FEATURE_PIPE_SECURITY using System.Security.AccessControl; -using System.Linq; #endif #if FEATURE_PIPE_SECURITY && FEATURE_NAMED_PIPE_SECURITY_CONSTRUCTOR using System.Security.Principal; From 7945c3817480b73599317dfbf33eb6b508321d30 Mon Sep 17 00:00:00 2001 From: Michal Pavlik Date: Wed, 29 Jun 2022 15:48:22 +0200 Subject: [PATCH 2/2] Removed event --- src/Framework/MSBuildEventSource.cs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/Framework/MSBuildEventSource.cs b/src/Framework/MSBuildEventSource.cs index afeb32973ad..708f5f6a31b 100644 --- a/src/Framework/MSBuildEventSource.cs +++ b/src/Framework/MSBuildEventSource.cs @@ -53,12 +53,6 @@ public void ApplyLazyItemOperationsStart(string itemType) WriteEvent(1, itemType); } - [Event(1, Keywords = Keywords.All)] - public void ApplyLazyItemOperationsStart2(string itemType) - { - WriteEvent(1, itemType); - } - /// The type of the item being mutated. [Event(2, Keywords = Keywords.All)] public void ApplyLazyItemOperationsStop(string itemType)