Skip to content

Commit

Permalink
Resolve dependencies from GAC (#951)
Browse files Browse the repository at this point in the history 
Cherry-picked from #950

> When there are dependencies that the project installs, but they would only resolve via a reference that is located in GAC (e.g. System.ValueTuple via netstandard) then the deployment into Out folder would ignore it.
> 
> If user then installs a version that is newer than what is in GAC it will get into their assembly redirects, but won't get copied into Out folder, resulting into TypeLoad exception for tests with DeploymentItem attribute.
> 
> This change fixes the resolver to look through all the dependencies including GAC dependencies and then copy over only the ones that are found in the bin folder, because ultimately Out should be just a subset (or the same) as the contents in bin folder.
> 
> Looking through GAC assemblies does not seem to add significant overhead the whole resolve is under 300ms so hopefully we don't need an option to configure enabling this.
> 
> (The new log messages won't go into diag log, because it probably is not correctly initialized in the new appdomain that loads the dll, but they will be written to Debug Trace and can be observed by DebugView++ or DebugView.)

Co-authored-by: nohwnd <me@jakubjares.com>
Co-authored-by: Sanan Yuzbashiyev <Sanan07@users.noreply.github.com>
  • Loading branch information
@Haplois @nohwnd @Sanan07
3 people authored Aug 30, 2021
1 parent afabf2c commit b86a8fb
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 17 deletions.
36 changes: 21 additions & 15 deletions src/Adapter/PlatformServices.Desktop/Deployment/AssemblyLoadWorker.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,19 +46,24 @@ public string[] GetFullPathToDependentAssemblies(string assemblyPath, out IList<
Assembly assembly = null;
try
{
EqtTrace.Verbose($"AssemblyLoadWorker.GetFullPathToDependentAssemblies: Reflection loading {assemblyPath}.");

// First time we load in LoadFromContext to avoid issues.
assembly = this.assemblyUtility.ReflectionOnlyLoadFrom(assemblyPath);
}
catch (Exception ex)
{
EqtTrace.Error($"AssemblyLoadWorker.GetFullPathToDependentAssemblies: Reflection loading of {assemblyPath} failed:");
EqtTrace.Error(ex);

warnings.Add(ex.Message);
return new string[0]; // Otherwise just return no dependencies.
}

Debug.Assert(assembly != null, "assembly");

List<string> result = new List<string>();
List<string> visitedAssemblies = new List<string>();
HashSet<string> visitedAssemblies = new HashSet<string>();

visitedAssemblies.Add(assembly.FullName);

Expand Down Expand Up @@ -149,9 +154,11 @@ private string GetTargetFrameworkStringFromAssembly(Assembly assembly)
/// <param name="result"> The result. </param>
/// <param name="visitedAssemblies"> The visited Assemblies. </param>
/// <param name="warnings"> The warnings. </param>
private void ProcessChildren(Assembly assembly, IList<string> result, IList<string> visitedAssemblies, IList<string> warnings)
private void ProcessChildren(Assembly assembly, IList<string> result, ISet<string> visitedAssemblies, IList<string> warnings)
{
Debug.Assert(assembly != null, "assembly");

EqtTrace.Verbose($"AssemblyLoadWorker.GetFullPathToDependentAssemblies: Processing assembly {assembly.FullName}.");
foreach (AssemblyName reference in assembly.GetReferencedAssemblies())
{
this.GetDependentAssembliesInternal(reference.FullName, result, visitedAssemblies, warnings);
Expand All @@ -161,6 +168,7 @@ private void ProcessChildren(Assembly assembly, IList<string> result, IList<stri
var modules = new Module[0];
try
{
EqtTrace.Verbose($"AssemblyLoadWorker.GetFullPathToDependentAssemblies: Getting modules of {assembly.FullName}.");
modules = assembly.GetModules();
}
catch (FileNotFoundException e)
Expand Down Expand Up @@ -192,13 +200,12 @@ private void ProcessChildren(Assembly assembly, IList<string> result, IList<stri
continue;
}

if (visitedAssemblies.Contains(m.Name))
if (!visitedAssemblies.Add(m.Name))
{
// The assembly was already in the set, meaning that we already visited it.
continue;
}

visitedAssemblies.Add(m.Name);

if (!File.Exists(m.FullyQualifiedName))
{
string warning = string.Format(CultureInfo.CurrentCulture, Resource.MissingDeploymentDependencyWithoutReason, m.FullyQualifiedName);
Expand All @@ -219,20 +226,21 @@ private void ProcessChildren(Assembly assembly, IList<string> result, IList<stri
/// <param name="visitedAssemblies"> The visited Assemblies. </param>
/// <param name="warnings"> The warnings. </param>
[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Design", "CA1031:DoNotCatchGeneralExceptionTypes", Justification = "Requirement is to handle all kinds of user exceptions and message appropriately.")]
private void GetDependentAssembliesInternal(string assemblyString, IList<string> result, IList<string> visitedAssemblies, IList<string> warnings)
private void GetDependentAssembliesInternal(string assemblyString, IList<string> result, ISet<string> visitedAssemblies, IList<string> warnings)
{
Debug.Assert(!string.IsNullOrEmpty(assemblyString), "assemblyString");

if (visitedAssemblies.Contains(assemblyString))
if (!visitedAssemblies.Add(assemblyString))
{
// The assembly was already in the hashset, so we already visited it.
return;
}

visitedAssemblies.Add(assemblyString);

Assembly assembly = null;
try
{
EqtTrace.Verbose($"AssemblyLoadWorker.GetDependentAssembliesInternal: Reflection loading {assemblyString}.");

string postPolicyAssembly = AppDomain.CurrentDomain.ApplyPolicy(assemblyString);
Debug.Assert(!string.IsNullOrEmpty(postPolicyAssembly), "postPolicyAssembly");

Expand All @@ -241,17 +249,15 @@ private void GetDependentAssembliesInternal(string assemblyString, IList<string>
}
catch (Exception ex)
{
EqtTrace.Error($"AssemblyLoadWorker.GetDependentAssembliesInternal: Reflection loading {assemblyString} failed:.");
EqtTrace.Error(ex);

string warning = string.Format(CultureInfo.CurrentCulture, Resource.MissingDeploymentDependency, assemblyString, ex.Message);
warnings.Add(warning);
return;
}

// As soon as we find GAC or internal assembly we do not look further.
if (assembly.GlobalAssemblyCache)
{
return;
}

EqtTrace.Verbose($"AssemblyLoadWorker.GetDependentAssembliesInternal: Assembly {assemblyString} was added as dependency.");
result.Add(assembly.Location);

this.ProcessChildren(assembly, result, visitedAssemblies, warnings);
Expand Down
16 changes: 14 additions & 2 deletions src/Adapter/PlatformServices.Desktop/Utilities/DesktopAssemblyUtility.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,8 @@ internal virtual string[] GetFullPathToDependentAssemblies(string assemblyPath,
EqtTrace.InfoIf(EqtTrace.IsInfoEnabled, "AssemblyDependencyFinder.GetDependentAssemblies: start.");

AppDomainSetup setupInfo = new AppDomainSetup();
setupInfo.ApplicationBase = Path.GetDirectoryName(Path.GetFullPath(assemblyPath));
var dllDirectory = Path.GetDirectoryName(Path.GetFullPath(assemblyPath));
setupInfo.ApplicationBase = dllDirectory;

Debug.Assert(string.IsNullOrEmpty(configFile) || File.Exists(configFile), "Config file is specified but does not exist: {0}", configFile);

Expand Down Expand Up @@ -227,7 +228,18 @@ internal virtual string[] GetFullPathToDependentAssemblies(string assemblyPath,

EqtTrace.InfoIf(EqtTrace.IsInfoEnabled, "AssemblyDependencyFinder.GetDependentAssemblies: loaded the worker.");

return worker.GetFullPathToDependentAssemblies(assemblyPath, out warnings);
var allDependencies = worker.GetFullPathToDependentAssemblies(assemblyPath, out warnings);
var dependenciesFromDllDirectory = new List<string>();
var dllDirectoryUppercase = dllDirectory.ToUpperInvariant();
foreach (var dependency in allDependencies)
{
if (dependency.ToUpperInvariant().Contains(dllDirectoryUppercase))
{
dependenciesFromDllDirectory.Add(dependency);
}
}

return dependenciesFromDllDirectory.ToArray();
}
}
finally
Expand Down
3 changes: 3 additions & 0 deletions src/Adapter/PlatformServices.Desktop/Utilities/DesktopDeploymentUtility.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,8 @@ private void AddDependencies(string testSource, string configFile, IList<Deploym
Debug.Assert(deploymentItems != null, "deploymentItems should not be null.");
Debug.Assert(Path.IsPathRooted(testSource), "path should be rooted.");

var sw = Stopwatch.StartNew();

// Note: if this is not an assembly we simply return empty array, also:
// we do recursive search and report missing.
string[] references = this.AssemblyUtility.GetFullPathToDependentAssemblies(testSource, configFile, out var warningList);
Expand All @@ -230,6 +232,7 @@ private void AddDependencies(string testSource, string configFile, IList<Deploym
if (EqtTrace.IsInfoEnabled)
{
EqtTrace.Info("DeploymentManager: Source:{0} has following references", testSource);
EqtTrace.Info("DeploymentManager: Resolving dependencies took {0} ms", sw.ElapsedMilliseconds);
}

foreach (string reference in references)
Expand Down

0 comments on commit b86a8fb

Please sign in to comment.