winget-cli

Unnamed repository; edit this file 'description' to name the repository.
Log | Files | Refs | README | LICENSE

commit aeb5284c1193784e39d1828b5a761847be6ce998
parent 3cea563a94951955b99fea0a72558c84a91e5900
Author: JohnMcPMS <johnmcp@microsoft.com>
Date:   Thu,  6 Jul 2023 10:55:34 -0700

Do not attempt post install ARP correlation if PackageFamilyName is provided and present for the user (#3391)

The simple change is to not attempt the confidence interval-based correlation after installing an installer that has a `PackageFamilyName` value present, when that family name is found to be registered for the user.
Diffstat:
Mazure-pipelines.yml | 2+-
Msrc/AppInstallerCLICore/Workflows/InstallFlow.cpp | 9+++++++++
Msrc/AppInstallerCLIE2ETests/AppInstallerCLIE2ETests.csproj | 9---------
Msrc/AppInstallerCLIE2ETests/InstallCommand.cs | 46++++++++++++++++++++++++++++++++++++++++++++++
Msrc/AppInstallerCLIE2ETests/TestCommon.cs | 43+++++++++++++++++++++++++++++++++++++++++--
Asrc/AppInstallerCLIE2ETests/TestData/Manifests/TestExeInstaller_InstallMSIX.yaml | 20++++++++++++++++++++
Msrc/AppInstallerCommonCore/Deployment.cpp | 10++++++++++
Msrc/AppInstallerCommonCore/Public/AppInstallerDeployment.h | 3+++
Msrc/WinGetUtilInterop.UnitTests/WinGetUtilInterop.UnitTests.csproj | 2+-
9 files changed, 131 insertions(+), 13 deletions(-)

diff --git a/azure-pipelines.yml b/azure-pipelines.yml @@ -275,7 +275,7 @@ jobs: - task: CopyFiles@2 displayName: 'Copy Files: WinGetUtilInterop.UnitTests' inputs: - SourceFolder: '$(Build.SourcesDirectory)\src\WinGetUtilInterop.UnitTests\bin\$(BuildConfiguration)\netcoreapp3.1' + SourceFolder: '$(Build.SourcesDirectory)\src\WinGetUtilInterop.UnitTests\bin\$(BuildConfiguration)\net6.0' TargetFolder: '$(Build.ArtifactStagingDirectory)\WinGetUtilInterop.UnitTests\' CleanTargetFolder: true OverWrite: true diff --git a/src/AppInstallerCLICore/Workflows/InstallFlow.cpp b/src/AppInstallerCLICore/Workflows/InstallFlow.cpp @@ -691,6 +691,15 @@ namespace AppInstaller::CLI::Workflow return; } + // If the installer claims to have a PackageFamilyName, and that family name is currently registered for the user, + // let that be the correlated item and skip any attempt at further ARP correlation. + const auto& installer = context.Get<Execution::Data::Installer>(); + + if (installer && !installer->PackageFamilyName.empty() && Deployment::IsRegistered(installer->PackageFamilyName)) + { + return; + } + const auto& manifest = context.Get<Execution::Data::Manifest>(); auto& arpCorrelationData = context.Get<Execution::Data::ARPCorrelationData>(); diff --git a/src/AppInstallerCLIE2ETests/AppInstallerCLIE2ETests.csproj b/src/AppInstallerCLIE2ETests/AppInstallerCLIE2ETests.csproj @@ -49,15 +49,6 @@ </ItemGroup> <ItemGroup> - <None Remove="TestData\Configuration\ConfigServerUnexpectedExit.yml" /> - <None Remove="TestData\Configuration\Configure_TestRepo.yml" /> - <None Remove="TestData\Configuration\DependentResources_Failure.yml" /> - <None Remove="TestData\Configuration\IndependentResources_OneFailure.yml" /> - <None Remove="TestData\configuration\Init-TestRepository.ps1" /> - <None Remove="TestData\Configuration\ShowDetails_TestRepo.yml" /> - </ItemGroup> - - <ItemGroup> <Content Include="..\..\doc\admx\DesktopAppInstaller.admx" Link="TestData\DesktopAppInstaller.admx"> <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory> </Content> diff --git a/src/AppInstallerCLIE2ETests/InstallCommand.cs b/src/AppInstallerCLIE2ETests/InstallCommand.cs @@ -671,5 +671,51 @@ namespace AppInstallerCLIE2ETests TestCommon.VerifyPortablePackage(Path.Combine(installDir, packageDirName), commandAlias, fileName, productCode, true); Assert.True(TestCommon.VerifyTestExeInstalledAndCleanup(testDir)); } + + /// <summary> + /// This test flow is intended to test an EXE that actually installs an MSIX internally, and whose name+publisher + /// information resembles an existing installation. Given this, the goal is to get correlation to stick to the + /// MSIX rather than the ARP entry that we would match with in the absence of the package family name being present. + /// </summary> + [Test] + public void InstallExeThatInstallsMSIX() + { + string targetPackageIdentifier = "AppInstallerTest.TestExeInstallerInstallsMSIX"; + string fakeProductCode = "e35f5799-cce3-41fd-886c-c36fcb7104fe"; + + // Insert fake ARP entry as if a non-MSIX version of the package is already installed. + // The name here must not match the normalized name of the package, but be close enough to meet + // the confidence requirements for correlation after an install operation (so we drop one word here). + TestCommon.CreateARPEntry(fakeProductCode, new + { + DisplayName = "EXE Installer that Installs MSIX", + Publisher = "AppInstallerTest", + DisplayVersion = "1.0.0", + }); + + // We should not find it before installing because the normalized name doesn't match + var result = TestCommon.RunAICLICommand("list", targetPackageIdentifier); + Assert.AreEqual(Constants.ErrorCode.ERROR_NO_APPLICATIONS_FOUND, result.ExitCode); + + // Add the MSIX to simulate an installer doing it + TestCommon.InstallMsix(TestCommon.MsixInstallerPath); + + // Install our exe that "installs" the MSIX + result = TestCommon.RunAICLICommand("install", $"{targetPackageIdentifier} --force"); + Assert.AreEqual(Constants.ErrorCode.S_OK, result.ExitCode); + + // We should find the package now, and it should be correlated to the MSIX (although we don't actually know that from this probe) + result = TestCommon.RunAICLICommand("list", targetPackageIdentifier); + Assert.AreEqual(Constants.ErrorCode.S_OK, result.ExitCode); + + // Remove the MSIX outside of winget's knowledge to keep the tracking data. + TestCommon.RemoveMsix(Constants.MsixInstallerName); + + // We should not find the package now that the MSIX is gone, confirming that it was correlated + result = TestCommon.RunAICLICommand("list", targetPackageIdentifier); + Assert.AreEqual(Constants.ErrorCode.ERROR_NO_APPLICATIONS_FOUND, result.ExitCode); + + TestCommon.RemoveARPEntry(fakeProductCode); + } } } \ No newline at end of file diff --git a/src/AppInstallerCLIE2ETests/TestCommon.cs b/src/AppInstallerCLIE2ETests/TestCommon.cs @@ -9,6 +9,7 @@ namespace AppInstallerCLIE2ETests using System; using System.Diagnostics; using System.IO; + using System.Reflection; using System.Threading; using Microsoft.Win32; using NUnit.Framework; @@ -695,8 +696,7 @@ namespace AppInstallerCLIE2ETests /// <param name="value">Value.</param> public static void ModifyPortableARPEntryValue(string productCode, string name, string value) { - const string uninstallSubKey = @"Software\Microsoft\Windows\CurrentVersion\Uninstall"; - using (RegistryKey uninstallRegistryKey = Registry.CurrentUser.OpenSubKey(uninstallSubKey, true)) + using (RegistryKey uninstallRegistryKey = Registry.CurrentUser.OpenSubKey(Constants.UninstallSubKey, true)) { RegistryKey entry = uninstallRegistryKey.OpenSubKey(productCode, true); entry.SetValue(name, value); @@ -793,6 +793,45 @@ namespace AppInstallerCLIE2ETests } /// <summary> + /// Creates an ARP entry from the given values. + /// </summary> + /// <param name="productCode">Product code of the entry.</param> + /// <param name="properties">The properties to set in the entry.</param> + /// <param name="scope">Scope of the entry.</param> + public static void CreateARPEntry( + string productCode, + object properties, + Scope scope = Scope.User) + { + RegistryKey baseKey = (scope == Scope.User) ? Registry.CurrentUser : Registry.LocalMachine; + using (RegistryKey uninstallRegistryKey = baseKey.OpenSubKey(Constants.UninstallSubKey, true)) + { + RegistryKey entry = uninstallRegistryKey.CreateSubKey(productCode, true); + + foreach (PropertyInfo property in properties.GetType().GetProperties()) + { + entry.SetValue(property.Name, property.GetValue(properties)); + } + } + } + + /// <summary> + /// Removes an ARP entry. + /// </summary> + /// <param name="productCode">Product code of the entry.</param> + /// <param name="scope">Scope of the entry.</param> + public static void RemoveARPEntry( + string productCode, + Scope scope = Scope.User) + { + RegistryKey baseKey = (scope == Scope.User) ? Registry.CurrentUser : Registry.LocalMachine; + using (RegistryKey uninstallRegistryKey = baseKey.OpenSubKey(Constants.UninstallSubKey, true)) + { + uninstallRegistryKey.DeleteSubKey(productCode); + } + } + + /// <summary> /// Run command result. /// </summary> public struct RunCommandResult diff --git a/src/AppInstallerCLIE2ETests/TestData/Manifests/TestExeInstaller_InstallMSIX.yaml b/src/AppInstallerCLIE2ETests/TestData/Manifests/TestExeInstaller_InstallMSIX.yaml @@ -0,0 +1,20 @@ +Id: AppInstallerTest.TestExeInstallerInstallsMSIX +Name: EXE Installer that Installs MSIX - extra +Version: 1.0.0.0 +Publisher: AppInstallerTest +License: Test +Installers: + - Arch: x86 + Url: https://localhost:5001/TestKit/AppInstallerTestExeInstaller/AppInstallerTestExeInstaller.exe + Sha256: <EXEHASH> + InstallerType: exe + PackageFamilyName: 6c6338fe-41b7-46ca-8ba6-b5ad5312bb0e_8wekyb3d8bbwe + Switches: + Custom: /execustom + SilentWithProgress: /exeswp + Silent: /exesilent + Interactive: /exeinteractive + Language: /exeenus + Log: /LogFile <LOGPATH> + InstallLocation: /InstallDir <INSTALLPATH> +ManifestVersion: 0.1.0 diff --git a/src/AppInstallerCommonCore/Deployment.cpp b/src/AppInstallerCommonCore/Deployment.cpp @@ -312,4 +312,14 @@ namespace AppInstaller::Deployment RemovePackage(packageFullName, RemovalOptions::RemoveForAllUsers, progress); } } + + bool IsRegistered(std::string_view packageFamilyName) + { + std::wstring wideFamilyName = Utility::ConvertToUTF16(packageFamilyName); + + PackageManager packageManager; + auto packages = packageManager.FindPackagesForUser({}, wideFamilyName); + + return packages.begin() != packages.end(); + } } diff --git a/src/AppInstallerCommonCore/Public/AppInstallerDeployment.h b/src/AppInstallerCommonCore/Public/AppInstallerDeployment.h @@ -44,4 +44,7 @@ namespace AppInstaller::Deployment std::string_view packageFamilyName, std::string_view packageFullName, IProgressCallback& callback); + + // Calls winrt::Windows::Management::Deployment::PackageManager::FindPackagesForUser + bool IsRegistered(std::string_view packageFamilyName); } diff --git a/src/WinGetUtilInterop.UnitTests/WinGetUtilInterop.UnitTests.csproj b/src/WinGetUtilInterop.UnitTests/WinGetUtilInterop.UnitTests.csproj @@ -1,7 +1,7 @@ <Project Sdk="Microsoft.NET.Sdk"> <PropertyGroup> - <TargetFramework>netcoreapp3.1</TargetFramework> + <TargetFramework>net6.0</TargetFramework> </PropertyGroup> <ItemGroup>