Skip to content
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

Microsoft.WinGet.Client custom assembly load context #3150

Merged
merged 27 commits into from
Apr 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
31374a1
convert json to hashtable
msftrubengu Apr 1, 2023
86e491a
Keep improving. TODO: Isolate for ACL changes
msftrubengu Apr 7, 2023
efb8b21
On a journey
msftrubengu Apr 8, 2023
37fac28
Move non project files
msftrubengu Apr 8, 2023
bf6094d
More moves
msftrubengu Apr 10, 2023
db6388d
more more
msftrubengu Apr 10, 2023
05348f3
Merge remote-tracking branch 'upstream/master' into improvecli
msftrubengu Apr 10, 2023
2d42dae
Start moving cmdlets
msftrubengu Apr 10, 2023
962753f
source and version
msftrubengu Apr 10, 2023
066139e
Bases
msftrubengu Apr 11, 2023
a1c0a0c
install and update
msftrubengu Apr 11, 2023
d8b30da
uninstall
msftrubengu Apr 11, 2023
e651457
builds
msftrubengu Apr 12, 2023
a36c558
rename to cmdlets
msftrubengu Apr 12, 2023
02f4959
Implements calling internal cmds and load unmanaged
msftrubengu Apr 12, 2023
c1b3b29
Start building module
msftrubengu Apr 13, 2023
1c7eb32
Produce module, rename to cmdlet
msftrubengu Apr 13, 2023
333b624
Fix ps1xml. Fix finding protected properties. Add pbds in debug. TODO…
msftrubengu Apr 13, 2023
1d7eee3
Copy unmanaged dlls to engine
msftrubengu Apr 13, 2023
8b7f46c
Spelling, fix tests
msftrubengu Apr 13, 2023
30d4673
ok...
msftrubengu Apr 14, 2023
fef41a2
E2E tests
msftrubengu Apr 14, 2023
737777f
Move enums
msftrubengu Apr 19, 2023
22bda81
Shared and direct dependencies
msftrubengu Apr 19, 2023
9580335
remove commented code
msftrubengu Apr 19, 2023
9b41a9f
msftrubengu Apr 19, 2023
50fefd8
Fix test
msftrubengu Apr 19, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .github/actions/spelling/expect.txt
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ debian
deigh
deleteifnotneeded
desktopappinstaller
dic
diskfull
dismapi
dllimport
Expand Down Expand Up @@ -171,6 +172,7 @@ ISVs
itr
IWin
JArray
JDictionary
jdk
jfearn
JObject
Expand Down Expand Up @@ -239,6 +241,7 @@ nativehandle
NESTEDINSTALLER
netfx
netlify
NETSDK
Newtonsoft
NOEXPAND
NOLINKINFO
Expand Down
29 changes: 28 additions & 1 deletion src/AppInstallerCLI.sln
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "CertificateResources", "Cer
EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "PowerShell", "PowerShell", "{7C218A3E-9BC8-48FF-B91B-BCACD828C0C9}"
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.WinGet.Client", "PowerShell\Microsoft.WinGet.Client\Microsoft.WinGet.Client.csproj", "{463C0EF3-DF38-4C3D-8E7E-D4901E0CDC6C}"
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.WinGet.Client.Cmdlets", "PowerShell\Microsoft.WinGet.Client.Cmdlets\Microsoft.WinGet.Client.Cmdlets.csproj", "{463C0EF3-DF38-4C3D-8E7E-D4901E0CDC6C}"
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.Management.Deployment.Projection", "Microsoft.Management.Deployment.Projection\Microsoft.Management.Deployment.Projection.csproj", "{0B104762-5CD8-47EE-A904-71C1C3F84DCD}"
EndProject
Expand Down Expand Up @@ -161,6 +161,8 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.Management.Config
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "ConfigurationRemotingServer", "ConfigurationRemotingServer\ConfigurationRemotingServer.csproj", "{6597EB04-D105-49A7-A5A3-D27FE1DF895E}"
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.WinGet.Client.Engine", "PowerShell\Microsoft.WinGet.Client.Engine\Microsoft.WinGet.Client.Engine.csproj", "{1F56BECB-D65D-4BBA-8788-6671B251392A}"
EndProject
Global
GlobalSection(SolutionConfigurationPlatforms) = preSolution
Debug|ARM64 = Debug|ARM64
Expand Down Expand Up @@ -874,6 +876,30 @@ Global
{6597EB04-D105-49A7-A5A3-D27FE1DF895E}.TestRelease|x64.Build.0 = Release|x64
{6597EB04-D105-49A7-A5A3-D27FE1DF895E}.TestRelease|x86.ActiveCfg = Release|x86
{6597EB04-D105-49A7-A5A3-D27FE1DF895E}.TestRelease|x86.Build.0 = Release|x86
{1F56BECB-D65D-4BBA-8788-6671B251392A}.Debug|ARM64.ActiveCfg = Debug|ARM64
{1F56BECB-D65D-4BBA-8788-6671B251392A}.Debug|ARM64.Build.0 = Debug|ARM64
{1F56BECB-D65D-4BBA-8788-6671B251392A}.Debug|x64.ActiveCfg = Debug|x64
{1F56BECB-D65D-4BBA-8788-6671B251392A}.Debug|x64.Build.0 = Debug|x64
{1F56BECB-D65D-4BBA-8788-6671B251392A}.Debug|x86.ActiveCfg = Debug|x86
{1F56BECB-D65D-4BBA-8788-6671B251392A}.Debug|x86.Build.0 = Debug|x86
{1F56BECB-D65D-4BBA-8788-6671B251392A}.Fuzzing|ARM64.ActiveCfg = Debug|ARM64
{1F56BECB-D65D-4BBA-8788-6671B251392A}.Fuzzing|ARM64.Build.0 = Debug|ARM64
{1F56BECB-D65D-4BBA-8788-6671B251392A}.Fuzzing|x64.ActiveCfg = Debug|x64
{1F56BECB-D65D-4BBA-8788-6671B251392A}.Fuzzing|x64.Build.0 = Debug|x64
{1F56BECB-D65D-4BBA-8788-6671B251392A}.Fuzzing|x86.ActiveCfg = Debug|x86
{1F56BECB-D65D-4BBA-8788-6671B251392A}.Fuzzing|x86.Build.0 = Debug|x86
{1F56BECB-D65D-4BBA-8788-6671B251392A}.Release|ARM64.ActiveCfg = Release|ARM64
{1F56BECB-D65D-4BBA-8788-6671B251392A}.Release|ARM64.Build.0 = Release|ARM64
{1F56BECB-D65D-4BBA-8788-6671B251392A}.Release|x64.ActiveCfg = Release|x64
{1F56BECB-D65D-4BBA-8788-6671B251392A}.Release|x64.Build.0 = Release|x64
{1F56BECB-D65D-4BBA-8788-6671B251392A}.Release|x86.ActiveCfg = Release|x86
{1F56BECB-D65D-4BBA-8788-6671B251392A}.Release|x86.Build.0 = Release|x86
{1F56BECB-D65D-4BBA-8788-6671B251392A}.TestRelease|ARM64.ActiveCfg = Release|ARM64
{1F56BECB-D65D-4BBA-8788-6671B251392A}.TestRelease|ARM64.Build.0 = Release|ARM64
{1F56BECB-D65D-4BBA-8788-6671B251392A}.TestRelease|x64.ActiveCfg = Release|x64
{1F56BECB-D65D-4BBA-8788-6671B251392A}.TestRelease|x64.Build.0 = Release|x64
{1F56BECB-D65D-4BBA-8788-6671B251392A}.TestRelease|x86.ActiveCfg = Release|x86
{1F56BECB-D65D-4BBA-8788-6671B251392A}.TestRelease|x86.Build.0 = Release|x86
EndGlobalSection
GlobalSection(SolutionProperties) = preSolution
HideSolutionNode = FALSE
Expand All @@ -899,6 +925,7 @@ Global
{787EC629-C0FB-4BA9-9746-4A82CD06B73E} = {60618CAC-2995-4DF9-9914-45C6FC02C995}
{8E43F982-40D5-4DF1-9044-C08047B5F43B} = {8D53D749-D51C-46F8-A162-9371AAA6C2E7}
{BB14D603-F44E-4415-8770-BF3E13F4C17F} = {60618CAC-2995-4DF9-9914-45C6FC02C995}
{1F56BECB-D65D-4BBA-8788-6671B251392A} = {7C218A3E-9BC8-48FF-B91B-BCACD828C0C9}
EndGlobalSection
GlobalSection(ExtensibilityGlobals) = postSolution
SolutionGuid = {B6FDB70C-A751-422C-ACD1-E35419495857}
Expand Down
78 changes: 15 additions & 63 deletions src/AppInstallerCLIE2ETests/PowerShell/WinGetClientModule.cs
Original file line number Diff line number Diff line change
Expand Up @@ -90,85 +90,37 @@ public void GetWinGetSource()
}

/// <summary>
/// Tests WinGetPackage cmdlets.
/// Find-WinGetPackage.
/// Install-WinGetPackage.
/// Get-WinGetPackage.
/// Update-WinGetPackage.
/// Uninstall-WinGetPackage.
/// </summary>
[Test]
public void FindWinGetPackage()
public void WinGetPackageCmdlets()
{
if (!Environment.Is64BitProcess)
{
return;
}

// TODO: It would be nice to add an installer to be use ONLY by the PowerShell cmdlets for E2E to avoid conflicts and bad
// cleanups in other tests.
var result = TestCommon.RunPowerShellCoreCommandWithResult(Constants.FindCmdlet, $"-Id {Constants.ExeInstallerPackageId}");
Assert.AreEqual(Constants.ErrorCode.S_OK, result.ExitCode, $"ExitCode: {result.ExitCode} Failed with the following output: {result.StdOut}; {result.StdErr}");
Assert.IsTrue(result.StdOut.Contains("TestExeInstaller"));
}

/// <summary>
/// Tests Get-WinGetPackage.
/// </summary>
[Test]
public void GetWinGetPackage()
{
if (!Environment.Is64BitProcess)
{
return;
}
var installResult = TestCommon.RunPowerShellCoreCommandWithResult(Constants.InstallCmdlet, $"-Id {Constants.ExeInstallerPackageId} -Version 1.0.0.0");
Assert.AreEqual(Constants.ErrorCode.S_OK, installResult.ExitCode, $"Failed with the following output: {installResult.StdOut}");

var installResult = TestCommon.RunPowerShellCoreCommandWithResult(Constants.InstallCmdlet, $"-Id {Constants.MsiInstallerPackageId}");
var getResult = TestCommon.RunPowerShellCoreCommandWithResult(Constants.GetCmdlet, $"-Id {Constants.MsiInstallerPackageId}");
var uninstallResult = TestCommon.RunPowerShellCoreCommandWithResult(Constants.UninstallCmdlet, $"-Id {Constants.MsiInstallerPackageId}");
var updateResult = TestCommon.RunPowerShellCoreCommandWithResult(Constants.UpdateCmdlet, $"-Id {Constants.ExeInstallerPackageId}");
Assert.AreEqual(Constants.ErrorCode.S_OK, updateResult.ExitCode, $"Failed with the following output: {updateResult.StdOut}");

Assert.AreEqual(Constants.ErrorCode.S_OK, installResult.ExitCode, $"ExitCode: {installResult.ExitCode}; Failed with the following output: {installResult.StdOut}; {installResult.StdErr}");
var getResult = TestCommon.RunPowerShellCoreCommandWithResult(Constants.GetCmdlet, $"-Id {Constants.ExeInstallerPackageId}");
Assert.AreEqual(Constants.ErrorCode.S_OK, getResult.ExitCode, $"Failed with the following output: {getResult.StdOut}");
Assert.AreEqual(Constants.ErrorCode.S_OK, uninstallResult.ExitCode, $"Failed with the following output: {uninstallResult.StdOut}");

Assert.IsTrue(!string.IsNullOrEmpty(installResult.StdOut));
Assert.IsTrue(getResult.StdOut.Contains("TestMsiInstaller"));
Assert.IsTrue(!string.IsNullOrEmpty(uninstallResult.StdOut));
}

/// <summary>
/// Tests Install-WinGetPackage.
/// </summary>
[Test]
public void InstallWinGetPackage()
{
if (!Environment.Is64BitProcess)
{
return;
}

var installResult = TestCommon.RunPowerShellCoreCommandWithResult(Constants.InstallCmdlet, $"-Id {Constants.ExeInstallerPackageId}");
var uninstallResult = TestCommon.RunPowerShellCoreCommandWithResult(Constants.UninstallCmdlet, $"-Id {Constants.ExeInstallerPackageId}");

Assert.AreEqual(Constants.ErrorCode.S_OK, installResult.ExitCode, $"ExitCode: {installResult.ExitCode}; Failed with the following output: {installResult.StdOut}; {installResult.StdErr}");
Assert.AreEqual(Constants.ErrorCode.S_OK, uninstallResult.ExitCode, $"Failed with the following output: {uninstallResult.StdOut}");

Assert.IsTrue(!string.IsNullOrEmpty(installResult.StdOut));
Assert.IsTrue(!string.IsNullOrEmpty(uninstallResult.StdOut));
}

/// <summary>
/// Tests Update-WinGetPackage.
/// </summary>
[Test]
public void UpdateWinGetPackage()
{
if (!Environment.Is64BitProcess)
{
return;
}

var installResult = TestCommon.RunPowerShellCoreCommandWithResult(Constants.InstallCmdlet, $"-Id {Constants.ExeInstallerPackageId} -Version 1.0.0.0");
var updateResult = TestCommon.RunPowerShellCoreCommandWithResult(Constants.UpdateCmdlet, $"-Id {Constants.ExeInstallerPackageId}");
var getResult = TestCommon.RunPowerShellCoreCommandWithResult(Constants.GetCmdlet, $"-Id {Constants.ExeInstallerPackageId}");
var uninstallResult = TestCommon.RunPowerShellCoreCommandWithResult(Constants.UninstallCmdlet, $"-Id {Constants.ExeInstallerPackageId}");

Assert.AreEqual(Constants.ErrorCode.S_OK, installResult.ExitCode, $"Failed with the following output: {installResult.StdOut}");
Assert.AreEqual(Constants.ErrorCode.S_OK, updateResult.ExitCode, $"Failed with the following output: {updateResult.StdOut}");
Assert.AreEqual(Constants.ErrorCode.S_OK, getResult.ExitCode, $"Failed with the following output: {getResult.StdOut}");
Assert.AreEqual(Constants.ErrorCode.S_OK, uninstallResult.ExitCode, $"Failed with the following output: {uninstallResult.StdOut}");

Assert.IsTrue(!string.IsNullOrEmpty(installResult.StdOut));
Expand Down Expand Up @@ -260,7 +212,7 @@ public void GetWinGetUserSettings_BadJsonFile()
// trying to load the runspace. This is most probably because the same dll is already loaded.
// Check the type the long way.
dynamic exception = cmdletException.InnerException;
Assert.AreEqual(exception.GetType().ToString(), "Microsoft.WinGet.Client.Exceptions.UserSettingsReadException");
Assert.AreEqual(exception.GetType().ToString(), "Microsoft.WinGet.Client.Engine.Exceptions.UserSettingsReadException");
}

/// <summary>
Expand Down Expand Up @@ -1124,7 +1076,7 @@ public void SetWinGetUserSettings_BadJsonFile_Merge()
// trying to load the runspace. This is most probably because the same dll is already loaded.
// Check the type the long way.
dynamic exception = cmdletException.InnerException;
Assert.AreEqual(exception.GetType().ToString(), "Microsoft.WinGet.Client.Exceptions.UserSettingsReadException");
Assert.AreEqual(exception.GetType().ToString(), "Microsoft.WinGet.Client.Engine.Exceptions.UserSettingsReadException");
}

private bool IsRunning(string processName)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// -----------------------------------------------------------------------------
// <copyright file="ModuleInit.cs" company="Microsoft Corporation">
// Copyright (c) Microsoft Corporation. Licensed under the MIT License.
// </copyright>
// -----------------------------------------------------------------------------
#if !POWERSHELL_WINDOWS
namespace Microsoft.WinGet.Client.Acl
{
using System.Management.Automation;
using System.Runtime.Loader;

/// <summary>
/// Initialization class for this module.
/// </summary>
public class ModuleInit : IModuleAssemblyInitializer, IModuleAssemblyCleanup
{
/// <inheritdoc/>
public void OnImport()
{
AssemblyLoadContext.Default.Resolving += WinGetAssemblyLoadContext.ResolvingHandler;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why aren't we able to create our own context and only use it when the assembly load is coming from this PS module?

My main worry is that if we change the default context, then we end up in a similar situation as without it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that my overall comment has more understanding than this one I made before reading the docs.

}

/// <inheritdoc/>
public void OnRemove(PSModuleInfo module)
{
AssemblyLoadContext.Default.Resolving -= WinGetAssemblyLoadContext.ResolvingHandler;
}
}
}
#endif
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
// -----------------------------------------------------------------------------
// <copyright file="WinGetAssemblyLoadContext.cs" company="Microsoft Corporation">
// Copyright (c) Microsoft Corporation. Licensed under the MIT License.
// </copyright>
// -----------------------------------------------------------------------------
#if !POWERSHELL_WINDOWS
namespace Microsoft.WinGet.Client.Acl
{
using System;
using System.IO;
using System.Reflection;
using System.Runtime.Loader;

/// <summary>
/// Custom assembly load context for this module.
/// This helps us load our dependencies without carrying about apps importing this module.
/// All dependencies except the Engine dll needs to be under a Dependencies directory.
/// </summary>
internal class WinGetAssemblyLoadContext : AssemblyLoadContext
{
private static readonly string SharedDependencyPath = Path.Combine(
Path.GetDirectoryName(typeof(WinGetAssemblyLoadContext).Assembly.Location),
"SharedDependencies");

private static readonly string DirectDependencyPath = Path.Combine(
Path.GetDirectoryName(typeof(WinGetAssemblyLoadContext).Assembly.Location),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like you inverted the name suggestions that I used (completely, so functionally it should work). To me, the "shared" dependencies are the ones that might conflict (and so need to load from the ALC Load) and the "direct" ones are the ones that are directly dependencies of the main DLL (and so need to use Resolving). Feel free to use whatever names you want, other than reversing my suggestion which is confusing to me. 🫠

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🫠

"DirectDependencies");

private static readonly WinGetAssemblyLoadContext WinGetAcl = new ();

private WinGetAssemblyLoadContext()
: base("WinGetAssemblyLoadContext", isCollectible: false)
{
}

/// <summary>
/// Handler to resolve assemblies.
/// </summary>
/// <param name="context">Assembly load context.</param>
/// <param name="assemblyName">Assembly name.</param>
/// <returns>The assembly, null if not in our assembly location.</returns>
internal static Assembly ResolvingHandler(AssemblyLoadContext context, AssemblyName assemblyName)
{
string path = $"{Path.Combine(DirectDependencyPath, assemblyName.Name)}.dll";
if (File.Exists(path))
{
return WinGetAcl.LoadFromAssemblyName(assemblyName);
}

return null;
}

/// <inheritdoc/>
protected override Assembly Load(AssemblyName assemblyName)
{
string path = $"{Path.Combine(SharedDependencyPath, assemblyName.Name)}.dll";
if (File.Exists(path))
{
return this.LoadFromAssemblyPath(path);
}

path = $"{Path.Combine(DirectDependencyPath, assemblyName.Name)}.dll";
if (File.Exists(path))
{
return this.LoadFromAssemblyPath(path);
}

return null;
}

/// <inheritdoc/>
protected override IntPtr LoadUnmanagedDll(string unmanagedDllName)
{
string path = Path.Combine(SharedDependencyPath, unmanagedDllName);
if (File.Exists(path))
{
return this.LoadUnmanagedDllFromPath(path);
}

return IntPtr.Zero;
}
}
}
#endif
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// -----------------------------------------------------------------------------
// <copyright file="AssertWinGetPackageManagerCommand.cs" company="Microsoft Corporation">
// <copyright file="AssertWinGetPackageManagerCmdlet.cs" company="Microsoft Corporation">
// Copyright (c) Microsoft Corporation. Licensed under the MIT License.
// </copyright>
// -----------------------------------------------------------------------------
Expand All @@ -9,7 +9,7 @@ namespace Microsoft.WinGet.Client.Commands
using System.Management.Automation;
using Microsoft.WinGet.Client.Commands.Common;
using Microsoft.WinGet.Client.Common;
using Microsoft.WinGet.Client.Helpers;
using Microsoft.WinGet.Client.Engine.Commands;

/// <summary>
/// Assert-WinGetPackageManager. Verifies winget is installed properly.
Expand All @@ -18,26 +18,23 @@ namespace Microsoft.WinGet.Client.Commands
VerbsLifecycle.Assert,
Constants.WinGetNouns.WinGetPackageManager,
DefaultParameterSetName = Constants.IntegrityVersionSet)]
public class AssertWinGetPackageManagerCommand : BaseIntegrityCommand
public class AssertWinGetPackageManagerCmdlet : WinGetPackageManagerCmdlet
{
/// <summary>
/// Validates winget is installed correctly. If not, throws an exception
/// with the reason why, if any.
/// </summary>
protected override void ProcessRecord()
{
string expectedVersion = string.Empty;
var command = new WinGetPackageManagerCommand(this);
if (this.ParameterSetName == Constants.IntegrityLatestSet)
{
var gitHubRelease = new GitHubRelease();
expectedVersion = gitHubRelease.GetLatestVersionTagName(this.IncludePreRelease.ToBool());
command.AssertUsingLatest(this.IncludePreRelease.ToBool());
}
else
{
expectedVersion = this.Version;
command.Assert(this.Version);
}

WinGetIntegrity.AssertWinGet(this, expectedVersion);
}
}
}
Loading