New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Abstracting Assembly Resolver #913
Changes from 3 commits
bb79193
4e988cf
0633aa2
36cf8af
e8b23dc
20e7e1d
b8f6709
0bd1dea
f386114
f7e0196
a041828
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,16 +10,16 @@ namespace Microsoft.VisualStudio.TestPlatform.Common.ExtensionFramework | |
using System.Reflection; | ||
using System.Text.RegularExpressions; | ||
|
||
#if NET46 | ||
using System.Threading; | ||
#endif | ||
|
||
using Microsoft.VisualStudio.TestPlatform.Common.ExtensionFramework.Utilities; | ||
using Microsoft.VisualStudio.TestPlatform.Common.Utilities; | ||
using Microsoft.VisualStudio.TestPlatform.ObjectModel; | ||
using Microsoft.VisualStudio.TestPlatform.PlatformAbstractions; | ||
using Microsoft.VisualStudio.TestPlatform.PlatformAbstractions.Interfaces; | ||
using Microsoft.VisualStudio.TestPlatform.Utilities.Helpers; | ||
using Microsoft.VisualStudio.TestPlatform.Utilities.Helpers.Interfaces; | ||
#if NET46 | ||
using System.Threading; | ||
#else | ||
using System.Runtime.Loader; | ||
#endif | ||
|
||
/// <summary> | ||
/// The test plugin cache. | ||
|
@@ -29,7 +29,6 @@ public class TestPluginCache | |
{ | ||
#region Private Members | ||
|
||
private readonly Dictionary<string, Assembly> resolvedAssemblies; | ||
private readonly IFileHelper fileHelper; | ||
|
||
/// <summary> | ||
|
@@ -45,7 +44,7 @@ public class TestPluginCache | |
/// <summary> | ||
/// Assembly resolver used to resolve the additional extensions | ||
/// </summary> | ||
private AssemblyResolver assemblyResolver; | ||
private IAssemblyResolver assemblyResolver; | ||
|
||
/// <summary> | ||
/// Lock for extensions update | ||
|
@@ -68,7 +67,6 @@ public class TestPluginCache | |
/// </param> | ||
protected TestPluginCache(IFileHelper fileHelper) | ||
{ | ||
this.resolvedAssemblies = new Dictionary<string, Assembly>(); | ||
this.pathToExtensions = null; | ||
this.loadOnlyWellKnownExtensions = false; | ||
this.lockForExtensionsUpdate = new object(); | ||
|
@@ -158,11 +156,8 @@ public bool LoadOnlyWellKnownExtensions | |
// and that succeeds. | ||
// Because of this assembly failure, below domain.CreateInstanceAndUnwrap() call fails with error | ||
// "Unable to cast transparent proxy to type 'Microsoft.VisualStudio.TestPlatform.Core.TestPluginsFramework.TestPluginDiscoverer" | ||
#if NET46 | ||
AppDomain.CurrentDomain.AssemblyResolve += new ResolveEventHandler(CurrentDomainAssemblyResolve); | ||
#else | ||
AssemblyLoadContext.Default.Resolving += this.CurrentDomainAssemblyResolve; | ||
#endif | ||
this.assemblyResolver.SetCurrentDomainAssemblyResolve(); | ||
|
||
try | ||
{ | ||
EqtTrace.Verbose("TestPluginCache: Discovering the extensions using extension path."); | ||
|
@@ -218,17 +213,7 @@ public bool LoadOnlyWellKnownExtensions | |
} | ||
finally | ||
{ | ||
#if NET46 | ||
AppDomain.CurrentDomain.AssemblyResolve -= new ResolveEventHandler(CurrentDomainAssemblyResolve); | ||
#else | ||
AssemblyLoadContext.Default.Resolving -= this.CurrentDomainAssemblyResolve; | ||
#endif | ||
|
||
// clear the assemblies | ||
lock (this.resolvedAssemblies) | ||
{ | ||
this.resolvedAssemblies?.Clear(); | ||
} | ||
this.assemblyResolver.RemoveCurrentDomainAssemblyResolve(); | ||
} | ||
|
||
return pluginInfos; | ||
|
@@ -274,7 +259,6 @@ public void UpdateExtensions(IEnumerable<string> additionalExtensionsPath, bool | |
// directory. The path to nuget directory is automatically setup for CLR to resolve. | ||
// Test platform tries to load every extension by assembly name. If it is not resolved, we don't | ||
// an error. | ||
|
||
if (this.pathToExtensions != null) | ||
{ | ||
extensions.AddRange(this.pathToExtensions); | ||
|
@@ -316,6 +300,7 @@ internal IEnumerable<string> DefaultExtensionPaths | |
{ | ||
return this.defaultExtensionPaths; | ||
} | ||
|
||
set | ||
{ | ||
if (value != null) | ||
|
@@ -501,55 +486,14 @@ private void SetupAssemblyResolver(string extensionAssembly) | |
// Add assembly resolver which can resolve the extensions from the specified directory. | ||
if (this.assemblyResolver == null) | ||
{ | ||
this.assemblyResolver = new AssemblyResolver(resolutionPaths); | ||
this.assemblyResolver = new AssemblyResolver(resolutionPaths, EqtTrace.PlatformTrace); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. AssemblyResolver just requires a way to |
||
} | ||
else | ||
{ | ||
this.assemblyResolver.AddSearchDirectories(resolutionPaths); | ||
} | ||
} | ||
|
||
#if NET46 | ||
private Assembly CurrentDomainAssemblyResolve(object sender, ResolveEventArgs args) | ||
#else | ||
private Assembly CurrentDomainAssemblyResolve(AssemblyLoadContext loadContext, AssemblyName args) | ||
#endif | ||
{ | ||
var assemblyName = new AssemblyName(args.Name); | ||
|
||
Assembly assembly = null; | ||
lock (this.resolvedAssemblies) | ||
{ | ||
try | ||
{ | ||
EqtTrace.Verbose("CurrentDomain_AssemblyResolve: Resolving assembly '{0}'.", args.Name); | ||
|
||
if (this.resolvedAssemblies.TryGetValue(args.Name, out assembly)) | ||
{ | ||
return assembly; | ||
} | ||
|
||
// Put it in the resolved assembly so that if below Assembly.Load call | ||
// triggers another assembly resolution, then we dont end up in stack overflow | ||
this.resolvedAssemblies[args.Name] = null; | ||
|
||
assembly = Assembly.Load(assemblyName); | ||
|
||
// Replace the value with the loaded assembly | ||
this.resolvedAssemblies[args.Name] = assembly; | ||
|
||
return assembly; | ||
} | ||
finally | ||
{ | ||
if (null == assembly) | ||
{ | ||
EqtTrace.Verbose("CurrentDomainAssemblyResolve: Failed to resolve assembly '{0}'.", args.Name); | ||
} | ||
} | ||
} | ||
} | ||
|
||
/// <summary> | ||
/// Log the extensions | ||
/// </summary> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -64,6 +64,14 @@ public static PlatformTraceLevel TraceLevel | |
} | ||
#endif | ||
|
||
public static IPlatformEqtTrace PlatformTrace | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't need to provide this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. resolved |
||
{ | ||
get | ||
{ | ||
return traceImpl; | ||
} | ||
} | ||
|
||
public static string LogFile | ||
{ | ||
get | ||
|
@@ -126,18 +134,23 @@ public static bool IsWarningEnabled | |
/// Initializes the verbose tracing with custom log file | ||
/// And overrides if any trace is set before | ||
/// </summary> | ||
/// <param name="customLogFile">A custom log file for trace messages.</param> | ||
/// <param name="customLogFile"> | ||
/// A custom log file for trace messages. | ||
/// </param> | ||
/// <returns> | ||
/// The <see cref="bool"/>. | ||
/// </returns> | ||
public static bool InitializeVerboseTrace(string customLogFile) | ||
{ | ||
if(!traceImpl.InitializeVerboseTrace(customLogFile)) | ||
if (!traceImpl.InitializeVerboseTrace(customLogFile)) | ||
{ | ||
ErrorOnInitialization = PlatformEqtTrace.ErrorOnInitialization; | ||
return false; | ||
} | ||
|
||
return true; | ||
} | ||
|
||
|
||
/// <summary> | ||
/// Prints an error message and prompts with a Debug dialog | ||
/// </summary> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
// Copyright (c) Microsoft Corporation. All rights reserved. | ||
// Licensed under the MIT license. See LICENSE file in the project root for full license information. | ||
|
||
namespace Microsoft.VisualStudio.TestPlatform.PlatformAbstractions.Interfaces | ||
{ | ||
using System; | ||
using System.Collections.Generic; | ||
|
||
/// <summary> | ||
/// The AssemblyResolver interface. | ||
/// </summary> | ||
public interface IAssemblyResolver : IDisposable | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Assembly resolution is probably a runtime aspect (and not a system wide aspect). Suggest to move to Interfaces/Runtime. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. resolved |
||
{ | ||
/// <summary> | ||
/// Sets up the Assembly resovler for current Appdomain | ||
/// </summary> | ||
void SetCurrentDomainAssemblyResolve(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Require documentation. |
||
|
||
/// <summary> | ||
/// Removes the Assembly resolver from current Appdomain | ||
/// </summary> | ||
void RemoveCurrentDomainAssemblyResolve(); | ||
|
||
/// <summary> | ||
/// Add the probing directories look for in case of Aseembly Resolve Event | ||
/// </summary> | ||
/// <param name="directories"></param> | ||
void AddSearchDirectories(IEnumerable<string> directories); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
// Copyright (c) Microsoft Corporation. All rights reserved. | ||
// Licensed under the MIT license. See LICENSE file in the project root for full license information. | ||
|
||
namespace Microsoft.VisualStudio.TestPlatform.ObjectModel | ||
{ | ||
public enum PlatformTraceLevel | ||
{ | ||
/// <summary> | ||
/// Output no tracing and debugging messages.. | ||
/// </summary> | ||
Off = 0, | ||
|
||
/// <summary> | ||
/// Output error-handling messages. | ||
/// </summary> | ||
Error = 1, | ||
|
||
/// <summary> | ||
/// Output warnings and error-handling messages. | ||
/// </summary> | ||
Warning = 2, | ||
|
||
/// <summary> | ||
/// Output informational messages, warnings, and error-handling messages.. | ||
/// </summary> | ||
Info = 3, | ||
|
||
/// <summary> | ||
/// Output all debugging and tracing messages.. | ||
/// </summary> | ||
Verbose = 4 | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need System.Threading for net46 only?