Refactor PowerShell Core to use the default CoreCLR loader instead - #3903
Refactor PowerShell Core to use the default CoreCLR loader instead#3903Dongbo Wang (daxian-dbw) merged 4 commits into
Conversation
Sorry, something went wrong.
Steve Lee (SteveL-MSFT)
left a comment
There was a problem hiding this comment.
LGTM
Sorry, something went wrong.
Aditya Patwardhan (adityapatwardhan)
left a comment
There was a problem hiding this comment.
Approved with 1 comment. Probably we need not worry about it right now, as this code might be merged into SMA.dll in the future.
Sorry, something went wrong.
There was a problem hiding this comment.
What is the localization story for these messages?
Sorry, something went wrong.
There was a problem hiding this comment.
Sorry for the late response. These strings are currently not localized because the loading of a satellite resource DLL when a loading exception happens causes a recursion. It's explained in the comment above the first resource string. Eventually, after moving the remaining code from PowerShellAssemblyLoadContext.dll to SMA.dll, we can reconsider the localization.
Sorry, something went wrong.
|
Minor update to add back Microsoft.PowerShell.CoreCLR.AssemblyExtensions.LoadFrom(string assemblyPath) for the short term. This is because PackageManagement depends on this API and thus removing this API will instantly break PackageManagement. Since the LoadFrom functionality has been removed from our assembly load context, we just call Assembly.LoadFrom(string) in the above API. I also added ObsoleteAttribute to this API to prevent new code from using it. brywang-msft is working on migrating PackageManagement to .NET Core 2.0. Once that work is done, the above API will be permanently removed from PowerShell Core. This is added to the follow-up tasks in the issue. I have verified that PackageManagement works with this update. |
Sorry, something went wrong.
…owerShell#3903) Remove the code that spins up our own assembly load context. Keep the code that registers our `Resolve` method to the default loader's `Resolving` event, so that we can continue to do special assembly resolution as needed (such as the GAC probing logic needed for consuming FullCLR PS modules). Essentially, the assembly `Microsoft.PowerShell.CoreCLR.AssemblyLoadContext.dll` is not needed anymore, the remaining code should be moved to S.M.A.dll. However, that will break DSC and other native hosts that are hosting powershell. So this assembly is kept for now.
Partially fix #3649
Issue Summary
The API AppDomain.GetAssemblies is brought back in .NET Core 2.0 which returns the loaded assemblies from the default loader. Therefore it's possible now for powershell to just depend on the default CoreCLR loader without having our own assembly load context getting in the picture. This would greatly simplify the scenario of hosting powershell in applications.
Fix
Remove the code that spins up our own assembly load context. Keep the code that registers our Resolve method to the default loader's Resolving event, so that we can continue to do special assembly resolution as needed (such as the GAC probing logic needed for consuming FullCLR PS modules).
Essentially, the assembly Microsoft.PowerShell.CoreCLR.AssemblyLoadContext.dll is not needed anymore, the remaining code should be moved to S.M.A.dll. However, that will break DSC and other native hosts that are hosting powershell. So this assembly is kept for now.
CoreFX Fixes needed
https://github.com/dotnet/corefx/issues/18989
https://github.com/dotnet/corefx/issues/18877
https://github.com/dotnet/corefx/issues/18791
This PR is not blocked by those CoreFX issues, but it's incomplete until we have the fixes. Those issues have already been addressed, and #3887 will get those fixes for us.
Note that this PR is NOT blocked by #3887.
Follow-up work
This PR addresses the first task listed in #3649. Three tasks are remaining.