From 525eebc0f31f8c770ad7b1636866aa2a15850e41 Mon Sep 17 00:00:00 2001 From: Ben Villalobos <4691428+BenVillalobos@users.noreply.github.com> Date: Fri, 25 Feb 2022 13:15:25 -0800 Subject: [PATCH 1/2] Expand architecture customization for usingtasks Use RuntimeInformation.ProcessArchitecture in GetCurrentMSBuildArchitecture. Refactor ToolsDirectory computation BuildEnvironmentHelper goes through a complicated process to compute the paths to the 32-bit and 64-bit tools folders. Refactored the logic to expose a new ToolsDirectoryRoot that is "the base folder" and simplify construction of the more-specific ones to append to the root. Don't distinguish tools directory root in handshake Node communication improvements --- .../Communications/NodeEndpointOutOfProc.cs | 2 +- .../Communications/NodeProviderOutOfProc.cs | 4 +- .../NodeProviderOutOfProcTaskHost.cs | 32 +++++++++- src/Shared/BuildEnvironmentHelper.cs | 62 +++++++++++-------- src/Shared/CommunicationsUtilities.cs | 34 ++++++---- src/Shared/XMakeAttributes.cs | 24 ++++++- 6 files changed, 114 insertions(+), 44 deletions(-) diff --git a/src/Build/BackEnd/Components/Communications/NodeEndpointOutOfProc.cs b/src/Build/BackEnd/Components/Communications/NodeEndpointOutOfProc.cs index 45303ac6d30..dfff3884413 100644 --- a/src/Build/BackEnd/Components/Communications/NodeEndpointOutOfProc.cs +++ b/src/Build/BackEnd/Components/Communications/NodeEndpointOutOfProc.cs @@ -54,7 +54,7 @@ internal NodeEndpointOutOfProc( /// protected override Handshake GetHandshake() { - return new Handshake(CommunicationsUtilities.GetHandshakeOptions(taskHost: false, is64Bit: EnvironmentUtilities.Is64BitProcess, nodeReuse: _enableReuse, lowPriority: _lowPriority)); + return new Handshake(CommunicationsUtilities.GetHandshakeOptions(taskHost: false, architectureFlagToSet: XMakeAttributes.GetCurrentMSBuildArchitecture(), nodeReuse: _enableReuse, lowPriority: _lowPriority)); } #region Structs diff --git a/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProc.cs b/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProc.cs index 2430e450cac..747ab889643 100644 --- a/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProc.cs +++ b/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProc.cs @@ -69,7 +69,7 @@ public int AvailableNodes internal static Handshake GetHandshake(bool enableNodeReuse, bool enableLowPriority) { CommunicationsUtilities.Trace("MSBUILDNODEHANDSHAKESALT=\"{0}\", msbuildDirectory=\"{1}\", enableNodeReuse={2}, enableLowPriority={3}", Traits.MSBuildNodeHandshakeSalt, BuildEnvironmentHelper.Instance.MSBuildToolsDirectory32, enableNodeReuse, enableLowPriority); - return new Handshake(CommunicationsUtilities.GetHandshakeOptions(taskHost: false, nodeReuse: enableNodeReuse, lowPriority: enableLowPriority, is64Bit: EnvironmentUtilities.Is64BitProcess)); + return new Handshake(CommunicationsUtilities.GetHandshakeOptions(taskHost: false, architectureFlagToSet: XMakeAttributes.GetCurrentMSBuildArchitecture(), nodeReuse: enableNodeReuse, lowPriority: enableLowPriority)); } /// @@ -94,7 +94,7 @@ public bool CreateNode(int nodeId, INodePacketFactory factory, NodeConfiguration // Make it here. CommunicationsUtilities.Trace("Starting to acquire a new or existing node to establish node ID {0}...", nodeId); - Handshake hostHandshake = new Handshake(CommunicationsUtilities.GetHandshakeOptions(taskHost: false, nodeReuse: ComponentHost.BuildParameters.EnableNodeReuse, lowPriority: ComponentHost.BuildParameters.LowPriority, is64Bit: EnvironmentUtilities.Is64BitProcess)); + Handshake hostHandshake = new Handshake(CommunicationsUtilities.GetHandshakeOptions(taskHost: false, architectureFlagToSet: XMakeAttributes.GetCurrentMSBuildArchitecture(), nodeReuse: ComponentHost.BuildParameters.EnableNodeReuse, lowPriority: ComponentHost.BuildParameters.LowPriority)); NodeContext context = GetNode(null, commandLineArgs, nodeId, factory, hostHandshake, NodeContextTerminated); if (context != null) diff --git a/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcTaskHost.cs b/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcTaskHost.cs index 7ac00b41d4d..635f34728b6 100644 --- a/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcTaskHost.cs +++ b/src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcTaskHost.cs @@ -37,6 +37,11 @@ internal class NodeProviderOutOfProcTaskHost : NodeProviderOutOfProcBase, INodeP /// private static string s_baseTaskHostPath64; + /// + /// Store the 64-bit path for MSBuild / MSBuildTaskHost so that we don't have to keep recalculating it. + /// + private static string s_baseTaskHostPathArm64; + /// /// Store the path for the 32-bit MSBuildTaskHost so that we don't have to keep re-calculating it. /// @@ -57,6 +62,11 @@ internal class NodeProviderOutOfProcTaskHost : NodeProviderOutOfProcBase, INodeP /// private static string s_pathToX64Clr4; + /// + /// Store the path for the 64-bit MSBuild so that we don't have to keep re-calculating it. + /// + private static string s_pathToArm64Clr4; + /// /// Name for MSBuild.exe /// @@ -353,8 +363,10 @@ internal static void ClearCachedTaskHostPaths() s_pathToX32Clr4 = null; s_pathToX64Clr2 = null; s_pathToX64Clr4 = null; + s_pathToArm64Clr4 = null; s_baseTaskHostPath = null; s_baseTaskHostPath64 = null; + s_baseTaskHostPathArm64 = null; } /// @@ -392,13 +404,20 @@ internal static string GetTaskHostNameFromHostContext(HandshakeOptions hostConte internal static string GetMSBuildLocationFromHostContext(HandshakeOptions hostContext) { string toolName = GetTaskHostNameFromHostContext(hostContext); - string toolPath; + string toolPath = null; s_baseTaskHostPath = BuildEnvironmentHelper.Instance.MSBuildToolsDirectory32; s_baseTaskHostPath64 = BuildEnvironmentHelper.Instance.MSBuildToolsDirectory64; + s_baseTaskHostPathArm64 = BuildEnvironmentHelper.Instance.MSBuildToolsDirectoryArm64; + ErrorUtilities.VerifyThrowInternalErrorUnreachable((hostContext & HandshakeOptions.TaskHost) == HandshakeOptions.TaskHost); - if ((hostContext & HandshakeOptions.X64) == HandshakeOptions.X64 && (hostContext & HandshakeOptions.CLR2) == HandshakeOptions.CLR2) + if ((hostContext & HandshakeOptions.Arm64) == HandshakeOptions.Arm64 && (hostContext & HandshakeOptions.CLR2) == HandshakeOptions.CLR2) + { + // Unsupported, throw. + ErrorUtilities.ThrowInternalError("ARM64 CLR2 task hosts are not supported."); + } + else if ((hostContext & HandshakeOptions.X64) == HandshakeOptions.X64 && (hostContext & HandshakeOptions.CLR2) == HandshakeOptions.CLR2) { if (s_pathToX64Clr2 == null) { @@ -434,6 +453,15 @@ internal static string GetMSBuildLocationFromHostContext(HandshakeOptions hostCo toolPath = s_pathToX64Clr4; } + else if ((hostContext & HandshakeOptions.Arm64) == HandshakeOptions.Arm64) + { + if (s_pathToArm64Clr4 == null) + { + s_pathToArm64Clr4 = s_baseTaskHostPathArm64; + } + + toolPath = s_pathToArm64Clr4; + } else { if (s_pathToX32Clr4 == null) diff --git a/src/Shared/BuildEnvironmentHelper.cs b/src/Shared/BuildEnvironmentHelper.cs index 2862be2a9d1..0bf416196af 100644 --- a/src/Shared/BuildEnvironmentHelper.cs +++ b/src/Shared/BuildEnvironmentHelper.cs @@ -8,6 +8,7 @@ using System.Collections.Generic; using System.Text.RegularExpressions; +using Microsoft.Build.Framework; using Microsoft.Build.Shared.FileSystem; using System.Reflection; @@ -531,42 +532,38 @@ public BuildEnvironment(BuildEnvironmentMode mode, string currentMSBuildExePath, if (mode == BuildEnvironmentMode.None || currentMSBuildExeFile == null || currentToolsDirectory == null) return; - // Check to see if our current folder is 'amd64' - bool runningInAmd64 = string.Equals(currentToolsDirectory.Name, "amd64", StringComparison.OrdinalIgnoreCase); - bool runningInARM64 = string.Equals(currentToolsDirectory.Name, "arm64", StringComparison.OrdinalIgnoreCase); - var msBuildExeName = currentMSBuildExeFile.Name; - var folderAbove = currentToolsDirectory.Parent?.FullName; - if (folderAbove != null) + if (mode == BuildEnvironmentMode.VisualStudio) + { + // In Visual Studio, the entry-point MSBuild.exe is often from an arch-specific subfolder + MSBuildToolsDirectoryRoot = NativeMethodsShared.ProcessorArchitecture switch + { + NativeMethodsShared.ProcessorArchitectures.X86 => CurrentMSBuildToolsDirectory, + NativeMethodsShared.ProcessorArchitectures.X64 or NativeMethodsShared.ProcessorArchitectures.ARM64 + => currentToolsDirectory.Parent?.FullName, + _ => throw new InternalErrorException("Unknown processor architecture " + NativeMethodsShared.ProcessorArchitecture), + }; + } + else + { + // In the .NET SDK, there's one copy of MSBuild.dll and it's in the root folder. + MSBuildToolsDirectoryRoot = CurrentMSBuildToolsDirectory; + } + + if (mode == BuildEnvironmentMode.VisualStudio && MSBuildToolsDirectoryRoot != null) { // Calculate potential paths to other architecture MSBuild.exe - var potentialAmd64FromX86 = FileUtilities.CombinePaths(CurrentMSBuildToolsDirectory, "amd64", msBuildExeName); - var potentialARM64FromX86 = FileUtilities.CombinePaths(CurrentMSBuildToolsDirectory, "arm64", msBuildExeName); - var potentialX86FromAmd64 = Path.Combine(folderAbove, msBuildExeName); + var potentialAmd64FromX86 = FileUtilities.CombinePaths(MSBuildToolsDirectoryRoot, "amd64", msBuildExeName); + var potentialARM64FromX86 = FileUtilities.CombinePaths(MSBuildToolsDirectoryRoot, "arm64", msBuildExeName); // Check for existence of an MSBuild file. Note this is not necessary in a VS installation where we always want to // assume the correct layout. var existsCheck = mode == BuildEnvironmentMode.VisualStudio ? new Func(_ => true) : File.Exists; - if ((runningInARM64 || runningInAmd64) && existsCheck(potentialX86FromAmd64)) - { - MSBuildToolsDirectory32 = folderAbove; - MSBuildToolsDirectory64 = CurrentMSBuildToolsDirectory; - } - else if (!runningInAmd64 && !runningInARM64) - { - MSBuildToolsDirectory32 = CurrentMSBuildToolsDirectory; - - if (existsCheck(potentialARM64FromX86) && NativeMethodsShared.ProcessorArchitecture == Framework.NativeMethods.ProcessorArchitectures.ARM64) - { - MSBuildToolsDirectory64 = Path.Combine(CurrentMSBuildToolsDirectory, "arm64"); - } - else if (existsCheck(potentialAmd64FromX86)) - { - MSBuildToolsDirectory64 = Path.Combine(CurrentMSBuildToolsDirectory, "amd64"); - } - } + MSBuildToolsDirectory32 = MSBuildToolsDirectoryRoot; + MSBuildToolsDirectory64 = Path.Combine(MSBuildToolsDirectoryRoot, "amd64"); + MSBuildToolsDirectoryArm64 = File.Exists(potentialARM64FromX86) ? Path.Combine(MSBuildToolsDirectoryRoot, "arm64") : null; } MSBuildExtensionsPath = mode == BuildEnvironmentMode.VisualStudio @@ -586,6 +583,11 @@ public BuildEnvironment(BuildEnvironmentMode mode, string currentMSBuildExePath, /// internal bool RunningInVisualStudio { get; } + /// + /// Path to the root of the MSBuild folder (in VS scenarios, MSBuild\Current\bin). + /// + internal string MSBuildToolsDirectoryRoot { get; } + /// /// Path to the MSBuild 32-bit tools directory. /// @@ -596,6 +598,12 @@ public BuildEnvironment(BuildEnvironmentMode mode, string currentMSBuildExePath, /// internal string MSBuildToolsDirectory64 { get; } + /// + /// Path to the ARM64 tools directory. + /// if ARM64 tools are not installed. + /// + internal string MSBuildToolsDirectoryArm64 { get; } + /// /// Path to the Sdks folder for this MSBuild instance. /// diff --git a/src/Shared/CommunicationsUtilities.cs b/src/Shared/CommunicationsUtilities.cs index 2d0a774c6a0..9fdf1e22306 100644 --- a/src/Shared/CommunicationsUtilities.cs +++ b/src/Shared/CommunicationsUtilities.cs @@ -68,6 +68,11 @@ internal enum HandshakeOptions /// Using the .NET Core/.NET 5.0+ runtime /// NET = 64, + + /// + /// ARM64 process + /// + Arm64 = 128, } internal readonly struct Handshake @@ -90,9 +95,9 @@ internal Handshake(HandshakeOptions nodeType) CommunicationsUtilities.Trace("Building handshake for node type {0}, (version {1}): options {2}.", nodeType, handshakeVersion, options); string handshakeSalt = Environment.GetEnvironmentVariable("MSBUILDNODEHANDSHAKESALT"); - CommunicationsUtilities.Trace("Handshake salt is \"{0}\"", handshakeSalt); - string toolsDirectory = (nodeType & HandshakeOptions.X64) == HandshakeOptions.X64 ? BuildEnvironmentHelper.Instance.MSBuildToolsDirectory64 : BuildEnvironmentHelper.Instance.MSBuildToolsDirectory32; - CommunicationsUtilities.Trace("Tools directory is \"{0}\"", toolsDirectory); + CommunicationsUtilities.Trace("Handshake salt is " + handshakeSalt); + string toolsDirectory = BuildEnvironmentHelper.Instance.MSBuildToolsDirectoryRoot; + CommunicationsUtilities.Trace("Tools directory root is " + toolsDirectory); salt = CommunicationsUtilities.GetHashCode(handshakeSalt + toolsDirectory); Version fileVersion = new Version(FileVersionInfo.GetVersionInfo(Assembly.GetExecutingAssembly().Location).FileVersion); fileVersionMajor = fileVersion.Major; @@ -483,22 +488,22 @@ internal static async Task ReadAsync(Stream stream, byte[] buffer, int byte /// /// Given the appropriate information, return the equivalent HandshakeOptions. /// - internal static HandshakeOptions GetHandshakeOptions(bool taskHost, bool is64Bit = false, bool nodeReuse = false, bool lowPriority = false, IDictionary taskHostParameters = null) + internal static HandshakeOptions GetHandshakeOptions(bool taskHost, string architectureFlagToSet = null, bool nodeReuse = false, bool lowPriority = false, IDictionary taskHostParameters = null) { HandshakeOptions context = taskHost ? HandshakeOptions.TaskHost : HandshakeOptions.None; int clrVersion = 0; - // We don't know about the TaskHost. Figure it out. + // We don't know about the TaskHost. if (taskHost) { - // Take the current TaskHost context + // No parameters given, default to current if (taskHostParameters == null) { clrVersion = typeof(bool).GetTypeInfo().Assembly.GetName().Version.Major; - is64Bit = XMakeAttributes.GetCurrentMSBuildArchitecture().Equals(XMakeAttributes.MSBuildArchitectureValues.x64); + architectureFlagToSet = XMakeAttributes.GetCurrentMSBuildArchitecture(); } - else + else // Figure out flags based on parameters given { ErrorUtilities.VerifyThrow(taskHostParameters.TryGetValue(XMakeAttributes.runtime, out string runtimeVersion), "Should always have an explicit runtime when we call this method."); ErrorUtilities.VerifyThrow(taskHostParameters.TryGetValue(XMakeAttributes.architecture, out string architecture), "Should always have an explicit architecture when we call this method."); @@ -520,13 +525,20 @@ internal static HandshakeOptions GetHandshakeOptions(bool taskHost, bool is64Bit ErrorUtilities.ThrowInternalErrorUnreachable(); } - is64Bit = architecture.Equals(XMakeAttributes.MSBuildArchitectureValues.x64); + architectureFlagToSet = architecture; } } - if (is64Bit) + if (!string.IsNullOrEmpty(architectureFlagToSet)) { - context |= HandshakeOptions.X64; + if (architectureFlagToSet.Equals(XMakeAttributes.MSBuildArchitectureValues.x64, StringComparison.OrdinalIgnoreCase)) + { + context |= HandshakeOptions.X64; + } + else if (architectureFlagToSet.Equals(XMakeAttributes.MSBuildArchitectureValues.arm64, StringComparison.OrdinalIgnoreCase)) + { + context |= HandshakeOptions.Arm64; + } } switch (clrVersion) diff --git a/src/Shared/XMakeAttributes.cs b/src/Shared/XMakeAttributes.cs index c7a3e3752fc..7a4d501e09c 100644 --- a/src/Shared/XMakeAttributes.cs +++ b/src/Shared/XMakeAttributes.cs @@ -3,6 +3,9 @@ using System; using System.Collections.Generic; +#if !CLR2COMPATIBILITY +using System.Runtime.InteropServices; +#endif using System.Runtime.CompilerServices; #nullable disable @@ -88,6 +91,7 @@ internal struct MSBuildArchitectureValues { internal const string x86 = "x86"; internal const string x64 = "x64"; + internal const string arm64 = "arm64"; internal const string currentArchitecture = "CurrentArchitecture"; internal const string any = "*"; } @@ -106,7 +110,7 @@ internal struct MSBuildArchitectureValues private static readonly HashSet ValidMSBuildRuntimeValues = new HashSet(StringComparer.OrdinalIgnoreCase) { MSBuildRuntimeValues.clr2, MSBuildRuntimeValues.clr4, MSBuildRuntimeValues.currentRuntime, MSBuildRuntimeValues.net, MSBuildRuntimeValues.any }; - private static readonly HashSet ValidMSBuildArchitectureValues = new HashSet(StringComparer.OrdinalIgnoreCase) { MSBuildArchitectureValues.x86, MSBuildArchitectureValues.x64, MSBuildArchitectureValues.currentArchitecture, MSBuildArchitectureValues.any }; + private static readonly HashSet ValidMSBuildArchitectureValues = new HashSet(StringComparer.OrdinalIgnoreCase) { MSBuildArchitectureValues.x86, MSBuildArchitectureValues.x64, MSBuildArchitectureValues.arm64, MSBuildArchitectureValues.currentArchitecture, MSBuildArchitectureValues.any }; /// /// Returns true if and only if the specified attribute is one of the attributes that the engine specifically recognizes @@ -429,7 +433,25 @@ internal static bool TryMergeArchitectureValues(string architectureA, string arc /// internal static string GetCurrentMSBuildArchitecture() { +#if !CLR2COMPATIBILITY + string currentArchitecture = string.Empty; + switch (RuntimeInformation.ProcessArchitecture) + { + case Architecture.X86: + currentArchitecture = MSBuildArchitectureValues.x86; + break; + case Architecture.X64: + currentArchitecture = MSBuildArchitectureValues.x64; + break; + case Architecture.Arm64: + currentArchitecture = MSBuildArchitectureValues.arm64; + break; + default: + throw new PlatformNotSupportedException(string.Format("{0} is not a supported architecture.", RuntimeInformation.ProcessArchitecture)); + } +#else string currentArchitecture = (IntPtr.Size == sizeof(Int64)) ? MSBuildArchitectureValues.x64 : MSBuildArchitectureValues.x86; +#endif return currentArchitecture; } From e03abacba6955ff397382cb4a72d64b8a1580f51 Mon Sep 17 00:00:00 2001 From: Ben Villalobos <4691428+BenVillalobos@users.noreply.github.com> Date: Tue, 19 Apr 2022 14:13:59 -0700 Subject: [PATCH 2/2] Skip tests that fail under new process detection logic --- src/Build.UnitTests/BuildEnvironmentHelper_Tests.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Build.UnitTests/BuildEnvironmentHelper_Tests.cs b/src/Build.UnitTests/BuildEnvironmentHelper_Tests.cs index f1e11354289..023b16eb686 100644 --- a/src/Build.UnitTests/BuildEnvironmentHelper_Tests.cs +++ b/src/Build.UnitTests/BuildEnvironmentHelper_Tests.cs @@ -369,6 +369,7 @@ public void BuildEnvironmentFindsAmd64() } [Fact] + [ActiveIssue("https://github.com/dotnet/msbuild/issues/7552", TargetFrameworkMonikers.Any)] [PlatformSpecific(TestPlatforms.Windows)] public void BuildEnvironmentFindsAmd64RunningInAmd64NoVS() { @@ -386,6 +387,7 @@ public void BuildEnvironmentFindsAmd64RunningInAmd64NoVS() } [Fact] + [ActiveIssue("https://github.com/dotnet/msbuild/issues/7552", TargetFrameworkMonikers.Any)] [PlatformSpecific(TestPlatforms.Windows)] public void BuildEnvironmentFindsAmd64NoVS() { @@ -401,6 +403,7 @@ public void BuildEnvironmentFindsAmd64NoVS() } [Fact] + [ActiveIssue("https://github.com/dotnet/msbuild/issues/7552", TargetFrameworkMonikers.Any)] [SkipOnTargetFramework(TargetFrameworkMonikers.Netcoreapp, "No Visual Studio install for netcore")] [PlatformSpecific(TestPlatforms.Windows)] public void BuildEnvironmentFindsAmd64RunningInAmd64()