Added logic in AssemblyLoadContext to load assembly from GAC. #3981
Conversation
|
Rebasing did not get rid of the changes in test unrelated to this commit. |
|
@adityapatwardhan if you click the TestCases tab, the test failure is your new test loading PSScheduledJob module. The other error output isn't fatal and fixed by #3968 |
| @@ -103,6 +108,11 @@ private PowerShellAssemblyLoadContext(string basePaths) | |||
| private readonly Dictionary<string, string> _coreClrTypeCatalog; | |||
| private readonly Lazy<HashSet<string>> _availableDotNetAssemblyNames; | |||
|
|
|||
| private string winDir; | |||
There was a problem hiding this comment.
convention is to use leading underscore for private fields
| assemblyFilePath = null; | ||
| char dirSeparator = IO.Path.DirectorySeparatorChar; | ||
|
|
||
| if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows) && PowerShellAssemblyLoadContextTestHooks.AllowGACLoading) |
There was a problem hiding this comment.
Wouldn't it be better to #ifdef !UNIX line 265 (calling this method) above as well as this whole method?
| if(Directory.Exists(tempAssemblyDirPath)) | ||
| { | ||
| //Enumerate all directories, sort by name and select the last. This selects the latest version. | ||
| var choosenVersionDirectory = Directory.GetDirectories(tempAssemblyDirPath).OrderBy(d => d).LastOrDefault(); |
There was a problem hiding this comment.
I think this is a typo: chosenVersionDirectory
|
@SteveL-MSFT Fixed all CR comments. |
| @@ -251,7 +261,13 @@ private Assembly Resolve(AssemblyLoadContext loadContext, AssemblyName assemblyN | |||
| // In this case, return null so that other Resolving event handlers can kick in to resolve the request. | |||
| if (!isAssemblyFileFound || !isAssemblyFileMatching) | |||
| { | |||
| return null; | |||
| #ifdef !UNIX | |||
There was a problem hiding this comment.
You still have to return null if UNIX right?
When Resolve method is not able to find the assembly, we try to look it up from GAC. If found, the assembly is loaded and cached. There is a TestHook to disable GAC loading, but it is enabled by default.
adityapatwardhan
force-pushed
the
FullClrAssemblyLoad
branch
from
46eaeed
to
a3c0fff
Compare
|
Having a look at the test failure. |
|
@SteveL-MSFT All comments / issues fixed. Please have a look. |
|
@daxian-dbw Can you also have a look. Thanks! |
|
Looking now. |
| /// <summary> | ||
| /// Test hooks for PowershellAssemblyLoadContext | ||
| /// </summary> | ||
| public static class PowerShellAssemblyLoadContextTestHooks |
There was a problem hiding this comment.
We have public static class InternalTestHooks for test hooks.
There was a problem hiding this comment.
@iSazonov That is in a different assembly, so could not re-use it.
Fixes #2592
When Resolve method is not able to find the assembly, we try to look it up from GAC.
If found, the assembly is loaded and cached.
There is a TestHook to disable GAC loading, but it is enabled by default.