From b452d73538534f49d5bf9e68819d5283aa030320 Mon Sep 17 00:00:00 2001 From: YuliiaKovalova Date: Tue, 22 Aug 2023 19:50:59 +0200 Subject: [PATCH] Fix review comment - remove extra method, cleanup and fix bug with dotnet.exe name comparison Remove extra spacing in Microsoft.Build.Locator.csproj Remove extra changes in Microsoft.Build.Locator.csproj --- .github/workflows/pull-request.yml | 1 - samples/BuilderApp/BuilderApp.csproj | 2 +- .../Microsoft.Build.Locator.Tests.csproj | 2 +- src/MSBuildLocator/DotNetSdkLocationHelper.cs | 102 ++++++------------ .../Microsoft.Build.Locator.csproj | 5 +- 5 files changed, 40 insertions(+), 72 deletions(-) diff --git a/.github/workflows/pull-request.yml b/.github/workflows/pull-request.yml index b978f021..9fd2fef6 100644 --- a/.github/workflows/pull-request.yml +++ b/.github/workflows/pull-request.yml @@ -21,7 +21,6 @@ jobs: uses: actions/setup-dotnet@v1 with: dotnet-version: | - 3.1.x 6.0.x - name: Restore run: dotnet restore -bl:restore.binlog diff --git a/samples/BuilderApp/BuilderApp.csproj b/samples/BuilderApp/BuilderApp.csproj index 6b53d89d..9035d7de 100644 --- a/samples/BuilderApp/BuilderApp.csproj +++ b/samples/BuilderApp/BuilderApp.csproj @@ -2,7 +2,7 @@ Exe - net472;net7.0 + net472;net6.0 false false diff --git a/src/MSBuildLocator.Tests/Microsoft.Build.Locator.Tests.csproj b/src/MSBuildLocator.Tests/Microsoft.Build.Locator.Tests.csproj index 50d067a3..4114e127 100644 --- a/src/MSBuildLocator.Tests/Microsoft.Build.Locator.Tests.csproj +++ b/src/MSBuildLocator.Tests/Microsoft.Build.Locator.Tests.csproj @@ -1,7 +1,7 @@ - net472;net7.0 + net472;net6.0 false true ..\MSBuildLocator\key.snk diff --git a/src/MSBuildLocator/DotNetSdkLocationHelper.cs b/src/MSBuildLocator/DotNetSdkLocationHelper.cs index 5fb3152c..ef4b9d8b 100644 --- a/src/MSBuildLocator/DotNetSdkLocationHelper.cs +++ b/src/MSBuildLocator/DotNetSdkLocationHelper.cs @@ -22,6 +22,8 @@ internal static class DotNetSdkLocationHelper private static readonly string ExeName = IsWindows ? "dotnet.exe" : "dotnet"; private static readonly string? DotnetPath = ResolveDotnetPath(); + static DotNetSdkLocationHelper() => LoadHostFxr(); + public static VisualStudioInstance? GetInstance(string dotNetSdkPath) { if (string.IsNullOrWhiteSpace(dotNetSdkPath) || !File.Exists(Path.Combine(dotNetSdkPath, "Microsoft.Build.dll"))) @@ -69,8 +71,6 @@ public static IEnumerable GetInstances(string workingDirec private static IEnumerable GetDotNetBasePaths(string workingDirectory) { - LoadHostFxr(); - string? bestSDK = GetSdkFromGlobalSettings(workingDirectory); if (!string.IsNullOrEmpty(bestSDK)) yield return bestSDK; @@ -99,9 +99,9 @@ private static void LoadHostFxr() private static IntPtr HostFxrResolver(Assembly assembly, string libraryName) { - var hostFxrRegex = new Regex("(lib)?hostfxr.dylib", RegexOptions.IgnoreCase); + var hostFxrLibName = "libhostfxr.dylib"; - if (!hostFxrRegex.IsMatch(libraryName) || string.IsNullOrEmpty(DotnetPath)) + if (!hostFxrLibName.Equals(libraryName, StringComparison.Ordinal) || string.IsNullOrEmpty(DotnetPath)) return IntPtr.Zero; var hostFxrRoot = Path.Combine(DotnetPath, "host", "fxr"); @@ -115,7 +115,7 @@ private static IntPtr HostFxrResolver(Assembly assembly, string libraryName) if (hostFxrAssemblyDirectory != null) { var hostfxrAssembly = Directory.GetFiles(hostFxrAssemblyDirectory) - .Where(filePath => hostFxrRegex.IsMatch(Path.GetFileName(filePath))) + .Where(filePath => hostFxrLibName.Equals(libraryName, StringComparison.Ordinal)) .FirstOrDefault(); if (hostfxrAssembly != null) @@ -157,16 +157,15 @@ private static string ResolveDotnetPath() { string? dotnetExePath = GetCurrentProcessPath(); var isRunFromDotnetExecutable = !string.IsNullOrEmpty(dotnetExePath) - && Path.GetFileNameWithoutExtension(dotnetExePath).Equals(ExeName, StringComparison.InvariantCultureIgnoreCase); + && Path.GetFileName(dotnetExePath).Equals(ExeName, StringComparison.InvariantCultureIgnoreCase); if (isRunFromDotnetExecutable) dotnetPath = Path.GetDirectoryName(dotnetExePath); else { - dotnetPath = GetDotnetPathFromDefaultPaths(); - - if (string.IsNullOrEmpty(dotnetPath)) - dotnetPath = GetDotnetPathFromPATH(); + dotnetPath = FindDotnetPathFromEnvVariable("DOTNET_HOST_PATH") + ?? FindDotnetPathFromEnvVariable("DOTNET_MSBUILD_SDK_RESOLVER_CLI_DIR") + ?? GetDotnetPathFromPATH(); } } @@ -176,61 +175,26 @@ private static string ResolveDotnetPath() return dotnetPath; } - private static void LoadHostFxr(string dotnetPath) - { - var isOSX = RuntimeInformation.IsOSPlatform(OSPlatform.OSX); - if (isOSX) - { - var hostFxrRoot = Path.Combine(dotnetPath, "host", "fxr"); - if (Directory.Exists(hostFxrRoot)) - { - // Agreed to load hostfxr from the highest version - var hostFxrAssemblyDirectory = Directory.GetDirectories(hostFxrRoot) - .OrderByDescending(d => d) - .FirstOrDefault(); - - if (hostFxrAssemblyDirectory != null) - { - Regex regex = new Regex("(lib)?hostfxr.dylib", RegexOptions.IgnoreCase); - - var matchingAssembly = Directory.GetFiles(hostFxrAssemblyDirectory) - .Where(filePath => regex.IsMatch(Path.GetFileName(filePath))) - .FirstOrDefault(); - - if (matchingAssembly != null) - { - var isHostFxrLoaded = AppDomain.CurrentDomain.GetAssemblies().Any(assembly => - string.Equals(assembly.GetName().Name, matchingAssembly, StringComparison.OrdinalIgnoreCase)); - - if (!isHostFxrLoaded) - Assembly.Load(matchingAssembly); - } - } - } - } - } - private static string? GetDotnetPathFromROOT() { // 32-bit architecture has (x86) suffix string envVarName = (IntPtr.Size == 4) ? "DOTNET_ROOT(x86)" : "DOTNET_ROOT"; - var dotnetPath = FindDotnetPathFromEnvVariable(envVarName, ExeName); + var dotnetPath = FindDotnetPathFromEnvVariable(envVarName); return dotnetPath; } - private static string? GetDotnetPathFromDefaultPaths() + private static string? GetDotnetPathFromHOST() { - var dotnetPath = FindDotnetPathFromEnvVariable("DOTNET_HOST_PATH", ExeName); + var dotnetPath = FindDotnetPathFromEnvVariable("DOTNET_HOST_PATH"); if (dotnetPath == null) { - dotnetPath ??= FindDotnetPathFromEnvVariable("DOTNET_MSBUILD_SDK_RESOLVER_CLI_DIR", ExeName); + dotnetPath ??= FindDotnetPathFromEnvVariable("DOTNET_MSBUILD_SDK_RESOLVER_CLI_DIR"); } return dotnetPath; } - private static string? GetCurrentProcessPath() { string? processPath = null; @@ -254,18 +218,11 @@ private static void LoadHostFxr(string dotnetPath) var paths = Environment.GetEnvironmentVariable("PATH")?.Split(Path.PathSeparator) ?? Array.Empty(); foreach (string dir in paths) { - string filePath = Path.Combine(dir, ExeName); - if (File.Exists(filePath)) - { - if (!IsWindows) - { - filePath = realpath(filePath) ?? filePath; - if (!File.Exists(filePath)) - continue; - } - - dotnetPath = dir; - } + string? filePath = ValidatePath(dir); + if (string.IsNullOrEmpty(filePath)) + continue; + + dotnetPath = dir; } return dotnetPath; @@ -298,14 +255,25 @@ private static string[] GetAllAvailableSDKs() return result; } - private static string? FindDotnetPathFromEnvVariable(string environmentVariable, string exeName) + private static string? FindDotnetPathFromEnvVariable(string environmentVariable) { - string? dotnet_root = Environment.GetEnvironmentVariable(environmentVariable); - if (!string.IsNullOrEmpty(dotnet_root)) + string? dotnetPath = Environment.GetEnvironmentVariable(environmentVariable); + + return string.IsNullOrEmpty(dotnetPath) ? null : ValidatePath(dotnetPath); + } + + private static string? ValidatePath(string dotnetPath) + { + string fullPathToDotnetFromRoot = Path.Combine(dotnetPath, ExeName); + if (File.Exists(fullPathToDotnetFromRoot)) { - string fullPathToDotnetFromRoot = Path.Combine(dotnet_root, exeName); - if (File.Exists(fullPathToDotnetFromRoot)) - return realpath(fullPathToDotnetFromRoot) ?? fullPathToDotnetFromRoot; + if (!IsWindows) + { + fullPathToDotnetFromRoot = realpath(fullPathToDotnetFromRoot) ?? fullPathToDotnetFromRoot; + return File.Exists(fullPathToDotnetFromRoot) ? Path.GetDirectoryName(fullPathToDotnetFromRoot) : null; + } + + return dotnetPath; } return null; diff --git a/src/MSBuildLocator/Microsoft.Build.Locator.csproj b/src/MSBuildLocator/Microsoft.Build.Locator.csproj index 82f734c6..056f7ca5 100644 --- a/src/MSBuildLocator/Microsoft.Build.Locator.csproj +++ b/src/MSBuildLocator/Microsoft.Build.Locator.csproj @@ -2,10 +2,10 @@ Library - net46;net7.0 + net46;net6.0 full - false + false Microsoft.Build.Locator Microsoft.Build.Locator true @@ -36,6 +36,7 @@ build\ + Microsoft400