From 0a4e36f4964488782ed7b290180bbce2c0a75398 Mon Sep 17 00:00:00 2001 From: Nathan Mytelka Date: Tue, 8 Dec 2020 15:35:14 -0800 Subject: [PATCH 01/10] Do not error if some assemblies were already loaded Changes the check for previously loaded assemblies to only fail if we already registered them via MSBuildLocator. Then adds previously loaded assemblies to the list of loaded assemblies so they aren't loaded again. --- src/MSBuildLocator/MSBuildLocator.cs | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/MSBuildLocator/MSBuildLocator.cs b/src/MSBuildLocator/MSBuildLocator.cs index 9c9c9cd6..44dfc49d 100644 --- a/src/MSBuildLocator/MSBuildLocator.cs +++ b/src/MSBuildLocator/MSBuildLocator.cs @@ -44,14 +44,6 @@ public static class MSBuildLocator /// public static bool IsRegistered => s_registeredHandler != null; - /// - /// Gets a value indicating whether an instance of MSBuild can be registered. - /// - /// - /// If any Microsoft.Build assemblies are already loaded into the current AppDomain, the value will be false. - /// - public static bool CanRegister => !IsRegistered && !LoadedMsBuildAssemblies.Any(); - private static IEnumerable LoadedMsBuildAssemblies => AppDomain.CurrentDomain.GetAssemblies().Where(IsMSBuildAssembly); /// @@ -154,7 +146,7 @@ public static void RegisterMSBuildPath(string msbuildPath) throw new ArgumentException($"Directory \"{msbuildPath}\" does not exist", nameof(msbuildPath)); } - if (!CanRegister) + if (IsRegistered) { var loadedAssemblyList = string.Join(Environment.NewLine, LoadedMsBuildAssemblies.Select(a => a.GetName())); @@ -174,6 +166,10 @@ public static void RegisterMSBuildPath(string msbuildPath) // AssemblyResolve event can fire multiple times for the same assembly, so keep track of what's already been loaded. var loadedAssemblies = new Dictionary(s_msBuildAssemblies.Length); + foreach (Assembly assembly in LoadedMsBuildAssemblies) + { + loadedAssemblies.Add(assembly.GetName().Name, assembly); + } // Saving the handler in a static field so it can be unregistered later. #if NET46 From c04cf55b57044e17fd8668a61979d34869eacbf3 Mon Sep 17 00:00:00 2001 From: Nathan Mytelka Date: Wed, 9 Dec 2020 15:52:57 -0800 Subject: [PATCH 02/10] Load up to all assemblies in the MSBuild folder Don't throw an error if the assembly is already loaded. --- src/MSBuildLocator/MSBuildLocator.cs | 53 ++++++++++++++++------------ 1 file changed, 30 insertions(+), 23 deletions(-) diff --git a/src/MSBuildLocator/MSBuildLocator.cs b/src/MSBuildLocator/MSBuildLocator.cs index 44dfc49d..8692dee7 100644 --- a/src/MSBuildLocator/MSBuildLocator.cs +++ b/src/MSBuildLocator/MSBuildLocator.cs @@ -26,8 +26,6 @@ public static class MSBuildLocator "Microsoft.Build.Framework", "Microsoft.Build.Tasks.Core", "Microsoft.Build.Utilities.Core", - "System.Runtime.CompilerServices.Unsafe", - "System.Numerics.Vectors" }; #if NET46 @@ -44,6 +42,14 @@ public static class MSBuildLocator /// public static bool IsRegistered => s_registeredHandler != null; + /// + /// Gets a value indicating whether an instance of MSBuild can be registered. + /// + /// + /// If any Microsoft.Build assemblies are already loaded into the current AppDomain, the value will be false. + /// + public static bool CanRegister => !IsRegistered && !LoadedMsBuildAssemblies.Any(); + private static IEnumerable LoadedMsBuildAssemblies => AppDomain.CurrentDomain.GetAssemblies().Where(IsMSBuildAssembly); /// @@ -146,7 +152,7 @@ public static void RegisterMSBuildPath(string msbuildPath) throw new ArgumentException($"Directory \"{msbuildPath}\" does not exist", nameof(msbuildPath)); } - if (IsRegistered) + if (!CanRegister) { var loadedAssemblyList = string.Join(Environment.NewLine, LoadedMsBuildAssemblies.Select(a => a.GetName())); @@ -165,11 +171,7 @@ public static void RegisterMSBuildPath(string msbuildPath) } // AssemblyResolve event can fire multiple times for the same assembly, so keep track of what's already been loaded. - var loadedAssemblies = new Dictionary(s_msBuildAssemblies.Length); - foreach (Assembly assembly in LoadedMsBuildAssemblies) - { - loadedAssemblies.Add(assembly.GetName().Name, assembly); - } + var loadedAssemblies = new Dictionary(); // Saving the handler in a static field so it can be unregistered later. #if NET46 @@ -181,7 +183,7 @@ public static void RegisterMSBuildPath(string msbuildPath) AppDomain.CurrentDomain.AssemblyResolve += s_registeredHandler; #else - s_registeredHandler = (assemblyLoadContext, assemblyName) => + s_registeredHandler = (_, assemblyName) => { return TryLoadAssembly(assemblyName); }; @@ -196,27 +198,32 @@ Assembly TryLoadAssembly(AssemblyName assemblyName) // Assembly resolution is not thread-safe. lock (loadedAssemblies) { - Assembly assembly; - if (loadedAssemblies.TryGetValue(assemblyName.FullName, out assembly)) + if (loadedAssemblies.TryGetValue(assemblyName.FullName, out Assembly assembly)) { return assembly; } - if (IsMSBuildAssembly(assemblyName)) + string targetAssembly = Path.Combine(msbuildPath, assemblyName.Name + ".dll"); + if (File.Exists(targetAssembly)) { - var targetAssembly = Path.Combine(msbuildPath, assemblyName.Name + ".dll"); - if (File.Exists(targetAssembly)) + // If the assembly was already loaded, return that. This could theoretically cause version problems, but the user shouldn't be + // trying to load an MSBuild that is incompatible with the version of the assembly they currently have loaded. + Assembly loadedAssembly = AppDomain.CurrentDomain.GetAssemblies().FirstOrDefault(a => a.GetName().Name.Equals(assemblyName.Name)); + if (loadedAssembly != null) + { + loadedAssemblies.Add(assemblyName.FullName, loadedAssembly); + return loadedAssembly; + } + + // Automatically unregister the handler once all supported assemblies have been loaded. + if (Interlocked.Increment(ref numResolvedAssemblies) == s_msBuildAssemblies.Length) { - // Automatically unregister the handler once all supported assemblies have been loaded. - if (Interlocked.Increment(ref numResolvedAssemblies) == s_msBuildAssemblies.Length) - { - Unregister(); - } - - assembly = Assembly.LoadFrom(targetAssembly); - loadedAssemblies.Add(assemblyName.FullName, assembly); - return assembly; + Unregister(); } + + assembly = Assembly.LoadFrom(targetAssembly); + loadedAssemblies.Add(assemblyName.FullName, assembly); + return assembly; } return null; From c5f23891099688eee1397279c7d3a5ebb95d9517 Mon Sep 17 00:00:00 2001 From: Nathan Mytelka Date: Fri, 11 Dec 2020 15:02:27 -0800 Subject: [PATCH 03/10] Remove unnecessary check and early unregister --- samples/BuilderApp/BuilderApp.csproj | 2 +- src/MSBuildLocator/MSBuildLocator.cs | 15 --------------- 2 files changed, 1 insertion(+), 16 deletions(-) diff --git a/samples/BuilderApp/BuilderApp.csproj b/samples/BuilderApp/BuilderApp.csproj index 64952e97..979d02a9 100644 --- a/samples/BuilderApp/BuilderApp.csproj +++ b/samples/BuilderApp/BuilderApp.csproj @@ -2,7 +2,7 @@ Exe - net471;netcoreapp2.1 + net471;netcoreapp3.1 false false diff --git a/src/MSBuildLocator/MSBuildLocator.cs b/src/MSBuildLocator/MSBuildLocator.cs index 8692dee7..58a19614 100644 --- a/src/MSBuildLocator/MSBuildLocator.cs +++ b/src/MSBuildLocator/MSBuildLocator.cs @@ -206,21 +206,6 @@ Assembly TryLoadAssembly(AssemblyName assemblyName) string targetAssembly = Path.Combine(msbuildPath, assemblyName.Name + ".dll"); if (File.Exists(targetAssembly)) { - // If the assembly was already loaded, return that. This could theoretically cause version problems, but the user shouldn't be - // trying to load an MSBuild that is incompatible with the version of the assembly they currently have loaded. - Assembly loadedAssembly = AppDomain.CurrentDomain.GetAssemblies().FirstOrDefault(a => a.GetName().Name.Equals(assemblyName.Name)); - if (loadedAssembly != null) - { - loadedAssemblies.Add(assemblyName.FullName, loadedAssembly); - return loadedAssembly; - } - - // Automatically unregister the handler once all supported assemblies have been loaded. - if (Interlocked.Increment(ref numResolvedAssemblies) == s_msBuildAssemblies.Length) - { - Unregister(); - } - assembly = Assembly.LoadFrom(targetAssembly); loadedAssemblies.Add(assemblyName.FullName, assembly); return assembly; From fdf7063267da2718065b670febb58320be5d052a Mon Sep 17 00:00:00 2001 From: Nathan Mytelka Date: Mon, 14 Dec 2020 09:21:45 -0800 Subject: [PATCH 04/10] Rename register called variable --- src/MSBuildLocator/MSBuildLocator.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/MSBuildLocator/MSBuildLocator.cs b/src/MSBuildLocator/MSBuildLocator.cs index 58a19614..a48093a2 100644 --- a/src/MSBuildLocator/MSBuildLocator.cs +++ b/src/MSBuildLocator/MSBuildLocator.cs @@ -35,7 +35,7 @@ public static class MSBuildLocator #endif // Used to determine when it's time to unregister the registeredHandler. - private static int numResolvedAssemblies; + private static bool registerCalled = false; /// /// Gets a value indicating whether an instance of MSBuild is currently registered. @@ -142,6 +142,7 @@ public static void RegisterInstance(VisualStudioInstance instance) /// public static void RegisterMSBuildPath(string msbuildPath) { + registerCalled = true; if (string.IsNullOrWhiteSpace(msbuildPath)) { throw new ArgumentException("Value may not be null or whitespace", nameof(msbuildPath)); @@ -227,7 +228,7 @@ public static void Unregister() if (!IsRegistered) { var error = $"{typeof(MSBuildLocator)}.{nameof(Unregister)} was called, but no MSBuild instance is registered." + Environment.NewLine; - if (numResolvedAssemblies == 0) + if (!registerCalled) { error += $"Ensure that {nameof(RegisterInstance)}, {nameof(RegisterMSBuildPath)}, or {nameof(RegisterDefaults)} is called before calling this method."; } From 4b152712e881e61f7c4ea5ee168934fe0738c7f0 Mon Sep 17 00:00:00 2001 From: Nathan Mytelka Date: Tue, 15 Dec 2020 21:26:36 -0800 Subject: [PATCH 05/10] Add NuGet location for VS scenarios --- src/MSBuildLocator/MSBuildLocator.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/MSBuildLocator/MSBuildLocator.cs b/src/MSBuildLocator/MSBuildLocator.cs index a48093a2..2ddeb72e 100644 --- a/src/MSBuildLocator/MSBuildLocator.cs +++ b/src/MSBuildLocator/MSBuildLocator.cs @@ -205,7 +205,9 @@ Assembly TryLoadAssembly(AssemblyName assemblyName) } string targetAssembly = Path.Combine(msbuildPath, assemblyName.Name + ".dll"); - if (File.Exists(targetAssembly)) + if (File.Exists(targetAssembly) || File.Exists(targetAssembly = + // Finds and loads NuGet assemblies if msbuildPath is in a VS installation + Path.GetFullPath(Path.Combine(msbuildPath, "..", "..", "..", "Common7", "IDE", "CommonExtensions", "Microsoft", "NuGet", assemblyName.Name + ".dll")))) { assembly = Assembly.LoadFrom(targetAssembly); loadedAssemblies.Add(assemblyName.FullName, assembly); From 21f44cf3db8b32713c9ee76e5f45f1835b0c304d Mon Sep 17 00:00:00 2001 From: Nathan Mytelka Date: Fri, 18 Dec 2020 12:13:20 -0800 Subject: [PATCH 06/10] Separate NuGet check into a separate if statement --- src/MSBuildLocator/MSBuildLocator.cs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/MSBuildLocator/MSBuildLocator.cs b/src/MSBuildLocator/MSBuildLocator.cs index 2ddeb72e..57a33264 100644 --- a/src/MSBuildLocator/MSBuildLocator.cs +++ b/src/MSBuildLocator/MSBuildLocator.cs @@ -205,9 +205,16 @@ Assembly TryLoadAssembly(AssemblyName assemblyName) } string targetAssembly = Path.Combine(msbuildPath, assemblyName.Name + ".dll"); - if (File.Exists(targetAssembly) || File.Exists(targetAssembly = - // Finds and loads NuGet assemblies if msbuildPath is in a VS installation - Path.GetFullPath(Path.Combine(msbuildPath, "..", "..", "..", "Common7", "IDE", "CommonExtensions", "Microsoft", "NuGet", assemblyName.Name + ".dll")))) + if (File.Exists(targetAssembly)) + { + assembly = Assembly.LoadFrom(targetAssembly); + loadedAssemblies.Add(assemblyName.FullName, assembly); + return assembly; + } + + // Finds and loads NuGet assemblies if msbuildPath is in a VS installation + targetAssembly = Path.GetFullPath(Path.Combine(msbuildPath, "..", "..", "..", "Common7", "IDE", "CommonExtensions", "Microsoft", "NuGet", assemblyName.Name + ".dll")); + if (File.Exists(targetAssembly)) { assembly = Assembly.LoadFrom(targetAssembly); loadedAssemblies.Add(assemblyName.FullName, assembly); From eec148427a2fd299f15695f74136dc62f8c1739e Mon Sep 17 00:00:00 2001 From: Nathan Mytelka Date: Tue, 22 Dec 2020 16:01:16 -0800 Subject: [PATCH 07/10] undo bad merge --- samples/BuilderApp/BuilderApp.csproj | 2 +- src/MSBuildLocator/MSBuildLocator.cs | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/samples/BuilderApp/BuilderApp.csproj b/samples/BuilderApp/BuilderApp.csproj index 979d02a9..64952e97 100644 --- a/samples/BuilderApp/BuilderApp.csproj +++ b/samples/BuilderApp/BuilderApp.csproj @@ -2,7 +2,7 @@ Exe - net471;netcoreapp3.1 + net471;netcoreapp2.1 false false diff --git a/src/MSBuildLocator/MSBuildLocator.cs b/src/MSBuildLocator/MSBuildLocator.cs index b240003a..7b8cd2cf 100644 --- a/src/MSBuildLocator/MSBuildLocator.cs +++ b/src/MSBuildLocator/MSBuildLocator.cs @@ -139,7 +139,6 @@ public static void RegisterInstance(VisualStudioInstance instance) /// public static void RegisterMSBuildPath(string msbuildPath) { - registerCalled = true; if (string.IsNullOrWhiteSpace(msbuildPath)) { throw new ArgumentException("Value may not be null or whitespace", nameof(msbuildPath)); From 917d6d8fffa87b27be2a65f6c3cfffb23691510a Mon Sep 17 00:00:00 2001 From: Nathan Mytelka Date: Wed, 23 Dec 2020 17:02:15 -0800 Subject: [PATCH 08/10] Allow providing multiple search paths for MSBuild assemblies --- src/MSBuildLocator/MSBuildLocator.cs | 56 +++++++++++++++++++--------- 1 file changed, 38 insertions(+), 18 deletions(-) diff --git a/src/MSBuildLocator/MSBuildLocator.cs b/src/MSBuildLocator/MSBuildLocator.cs index 7b8cd2cf..37ada2eb 100644 --- a/src/MSBuildLocator/MSBuildLocator.cs +++ b/src/MSBuildLocator/MSBuildLocator.cs @@ -139,14 +139,40 @@ public static void RegisterInstance(VisualStudioInstance instance) /// public static void RegisterMSBuildPath(string msbuildPath) { - if (string.IsNullOrWhiteSpace(msbuildPath)) + RegisterMSBuildPath(new string[] { + msbuildPath +#if NET46 + // Finds and loads NuGet assemblies if msbuildPath is in a VS installation + , Path.GetFullPath(Path.Combine(msbuildPath, "..", "..", "..", "Common7", "IDE", "CommonExtensions", "Microsoft", "NuGet")) +#endif + }); + } + + /// + /// Add assembly resolution for Microsoft.Build core dlls in the current AppDomain from the specified + /// path. + /// + /// + /// Paths to directories containing a deployment of MSBuild binaries. + /// A minimal MSBuild deployment would be the publish result of the Microsoft.Build.Runtime package. + /// + /// In order to restore and build real projects, one needs a deployment that contains the rest of the toolchain (nuget, compilers, etc.). + /// Such deployments can be found in installations such as Visual Studio or dotnet CLI. + /// + public static void RegisterMSBuildPath(string[] msbuildSearchPaths) + { + if (msbuildSearchPaths.Length < 1) + { + throw new ArgumentException("Must provide at least one search path to RegisterMSBuildPath."); + } + if (msbuildSearchPaths.Any(string.IsNullOrWhiteSpace)) { - throw new ArgumentException("Value may not be null or whitespace", nameof(msbuildPath)); + throw new ArgumentException("Value may not be null or whitespace", nameof(msbuildSearchPaths)); } - if (!Directory.Exists(msbuildPath)) + foreach (string path in msbuildSearchPaths.Where(path => !Directory.Exists(path))) { - throw new ArgumentException($"Directory \"{msbuildPath}\" does not exist", nameof(msbuildPath)); + throw new ArgumentException($"Directory \"{path}\" does not exist", nameof(msbuildSearchPaths)); } if (!CanRegister) @@ -202,21 +228,15 @@ Assembly TryLoadAssembly(AssemblyName assemblyName) // Look in the MSBuild folder for any unresolved reference. It may be a dependency // of MSBuild or a task. - string targetAssembly = Path.Combine(msbuildPath, assemblyName.Name + ".dll"); - if (File.Exists(targetAssembly)) + foreach (string msbuildPath in msbuildSearchPaths) { - assembly = Assembly.LoadFrom(targetAssembly); - loadedAssemblies.Add(assemblyName.FullName, assembly); - return assembly; - } - - // Finds and loads NuGet assemblies if msbuildPath is in a VS installation - targetAssembly = Path.GetFullPath(Path.Combine(msbuildPath, "..", "..", "..", "Common7", "IDE", "CommonExtensions", "Microsoft", "NuGet", assemblyName.Name + ".dll")); - if (File.Exists(targetAssembly)) - { - assembly = Assembly.LoadFrom(targetAssembly); - loadedAssemblies.Add(assemblyName.FullName, assembly); - return assembly; + string targetAssembly = Path.Combine(msbuildPath, assemblyName.Name + ".dll"); + if (File.Exists(targetAssembly)) + { + assembly = Assembly.LoadFrom(targetAssembly); + loadedAssemblies.Add(assemblyName.FullName, assembly); + return assembly; + } } return null; From e754f40597196bda4c8d01f4dc45d5aa19b11d6a Mon Sep 17 00:00:00 2001 From: Nathan Mytelka Date: Mon, 4 Jan 2021 09:52:23 -0800 Subject: [PATCH 09/10] Update errors --- src/MSBuildLocator/MSBuildLocator.cs | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/src/MSBuildLocator/MSBuildLocator.cs b/src/MSBuildLocator/MSBuildLocator.cs index 37ada2eb..a2a8b81d 100644 --- a/src/MSBuildLocator/MSBuildLocator.cs +++ b/src/MSBuildLocator/MSBuildLocator.cs @@ -141,7 +141,7 @@ public static void RegisterMSBuildPath(string msbuildPath) { RegisterMSBuildPath(new string[] { msbuildPath -#if NET46 +#if NET46 && !MONO // Finds and loads NuGet assemblies if msbuildPath is in a VS installation , Path.GetFullPath(Path.Combine(msbuildPath, "..", "..", "..", "Common7", "IDE", "CommonExtensions", "Microsoft", "NuGet")) #endif @@ -165,14 +165,24 @@ public static void RegisterMSBuildPath(string[] msbuildSearchPaths) { throw new ArgumentException("Must provide at least one search path to RegisterMSBuildPath."); } - if (msbuildSearchPaths.Any(string.IsNullOrWhiteSpace)) + + List nullOrWhiteSpaceExceptions = new List(); + for (int i = 0; i < msbuildSearchPaths.Length; i++) + { + if (string.IsNullOrWhiteSpace(msbuildSearchPaths[i])) + { + nullOrWhiteSpaceExceptions.Add(new ArgumentException($"Value at position {i+1} may not be null or whitespace", nameof(msbuildSearchPaths))); + } + } + if (nullOrWhiteSpaceExceptions.Count > 0) { - throw new ArgumentException("Value may not be null or whitespace", nameof(msbuildSearchPaths)); + throw new AggregateException("Search paths for MSBuild assemblies cannot be null and must contain non-whitespace characters.", nullOrWhiteSpaceExceptions); } - foreach (string path in msbuildSearchPaths.Where(path => !Directory.Exists(path))) + IEnumerable paths = msbuildSearchPaths.Where(path => !Directory.Exists(path)); + if (paths.FirstOrDefault() == null) { - throw new ArgumentException($"Directory \"{path}\" does not exist", nameof(msbuildSearchPaths)); + throw new AggregateException($"A directory or directories in \"{nameof(msbuildSearchPaths)}\" do not exist", paths.Select(path => new ArgumentException($"Directory \"{path}\" does not exist", nameof(msbuildSearchPaths)))); } if (!CanRegister) From e9e513d1c513086e21e5a69ff9e3cbb6c145e32f Mon Sep 17 00:00:00 2001 From: Forgind Date: Mon, 4 Jan 2021 10:10:18 -0800 Subject: [PATCH 10/10] Update src/MSBuildLocator/MSBuildLocator.cs --- src/MSBuildLocator/MSBuildLocator.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/MSBuildLocator/MSBuildLocator.cs b/src/MSBuildLocator/MSBuildLocator.cs index a2a8b81d..e2fbab52 100644 --- a/src/MSBuildLocator/MSBuildLocator.cs +++ b/src/MSBuildLocator/MSBuildLocator.cs @@ -141,7 +141,7 @@ public static void RegisterMSBuildPath(string msbuildPath) { RegisterMSBuildPath(new string[] { msbuildPath -#if NET46 && !MONO +#if NET46 // Finds and loads NuGet assemblies if msbuildPath is in a VS installation , Path.GetFullPath(Path.Combine(msbuildPath, "..", "..", "..", "Common7", "IDE", "CommonExtensions", "Microsoft", "NuGet")) #endif