Resolve dependencies from GAC (#951)

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 <[email protected]>
Co-authored-by: Sanan Yuzbashiyev <[email protected]>

Author: 3 people

src/Adapter/PlatformServices.Desktop/Deployment/AssemblyLoadWorker.cs

@@ -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);
 
@@ -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);
@@ -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)
@@ -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);
@@ -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");
 
@@ -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);

src/Adapter/PlatformServices.Desktop/Utilities/DesktopAssemblyUtility.cs

@@ -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);
 
@@ -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

src/Adapter/PlatformServices.Desktop/Utilities/DesktopDeploymentUtility.cs

@@ -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);
@@ -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)