From 1b9954e3a430324d04d8104b9df5c8c3261782c5 Mon Sep 17 00:00:00 2001 From: John McPherson Date: Thu, 25 Jan 2024 08:42:18 -0800 Subject: [PATCH 01/24] Change enumeration to prevent any duplicates from being excluded --- doc/Settings.md | 22 ++++++------ .../JSON/settings/settings.schema.0.2.json | 15 +++++--- src/AppInstallerCLI.sln | 10 ++++-- src/AppInstallerCommonCore/Architecture.cpp | 19 +++++++++++ .../ExperimentalFeature.cpp | 4 +++ .../Public/AppInstallerArchitecture.h | 6 +++- .../Public/winget/ExperimentalFeature.h | 1 + .../Public/winget/UserSettings.h | 2 ++ src/AppInstallerCommonCore/UserSettings.cpp | 1 + .../Microsoft/ARPHelper.cpp | 16 +++++++-- .../PredefinedInstalledSourceFactory.cpp | 34 +++++++++++++++---- .../Converters.cpp | 16 +-------- 12 files changed, 103 insertions(+), 43 deletions(-) diff --git a/doc/Settings.md b/doc/Settings.md index 4efd036b8d..bb45d3449d 100644 --- a/doc/Settings.md +++ b/doc/Settings.md @@ -280,17 +280,6 @@ You can enable the feature as shown below. }, ``` -### configuration - -This feature enables the configuration commands. These commands allow configuring the system into a desired state. -You can enable the feature as shown below. - -```json - "experimentalFeatures": { - "configuration": true - }, -``` - ### windowsFeature This feature enables the ability to enable Windows Feature dependencies during installation. @@ -333,4 +322,15 @@ You can enable the feature as shown below. "experimentalFeatures": { "configuration03": true }, +``` + +### sideBySide + +This feature enables experimental improvements for supporting multiple instances of a package being installed on a system. +You can enable the feature as shown below. + +```json + "experimentalFeatures": { + "sideBySide": true + }, ``` \ No newline at end of file diff --git a/schemas/JSON/settings/settings.schema.0.2.json b/schemas/JSON/settings/settings.schema.0.2.json index 157d761006..a4abfcee2d 100644 --- a/schemas/JSON/settings/settings.schema.0.2.json +++ b/schemas/JSON/settings/settings.schema.0.2.json @@ -266,11 +266,6 @@ "type": "boolean", "default": false }, - "configuration": { - "description": "Enable support for configuration", - "type": "boolean", - "default": false - }, "windowsFeature": { "description": "Enable support for enabling Windows Feature(s)", "type": "boolean", @@ -280,6 +275,16 @@ "description": "Enable support for initiating a reboot", "type": "boolean", "default": false + }, + "configuration03": { + "description": "Enable support for configuration schema 0.3", + "type": "boolean", + "default": false + }, + "sideBySide": { + "description": "Enable support for improved side-by-side handling", + "type": "boolean", + "default": false } } } diff --git a/src/AppInstallerCLI.sln b/src/AppInstallerCLI.sln index 02c29202fc..da7d2d3d3e 100644 --- a/src/AppInstallerCLI.sln +++ b/src/AppInstallerCLI.sln @@ -180,6 +180,11 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "WinGetSourceCreator", "WinG EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.WinGet.SharedLib", "PowerShell\Microsoft.WinGet.SharedLib\Microsoft.WinGet.SharedLib.csproj", "{272B2B0E-40D4-4F0F-B187-519A6EF89B10}" EndProject +Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "schema", "schema", "{46E419FD-C77F-4EFC-A0FC-2BAD93E60D12}" + ProjectSection(SolutionItems) = preProject + ..\schemas\JSON\settings\settings.schema.0.2.json = ..\schemas\JSON\settings\settings.schema.0.2.json + EndProjectSection +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -1405,8 +1410,8 @@ Global {1622DA16-914F-4F57-A259-D5169003CC8C} = {6D7776A8-42FE-46DD-B0F8-712F35EA0C79} {3BAF989F-7F65-465B-ACE8-BAFE42D1017E} = {EA8CD934-0702-4911-A2C5-A40600E616DE} {358BC478-0624-4AD1-A933-0422B5292AF8} = {60618CAC-2995-4DF9-9914-45C6FC02C995} - {7D05F64D-CE5A-42AA-A2C1-E91458F061CF} = {8D53D749-D51C-46F8-A162-9371AAA6C2E7} - {952B513F-8A00-4D74-9271-925AFB3C6252} = {8D53D749-D51C-46F8-A162-9371AAA6C2E7} + {7D05F64D-CE5A-42AA-A2C1-E91458F061CF} = {46E419FD-C77F-4EFC-A0FC-2BAD93E60D12} + {952B513F-8A00-4D74-9271-925AFB3C6252} = {46E419FD-C77F-4EFC-A0FC-2BAD93E60D12} {2ACDE176-F13F-42FA-8159-C34FA3D37837} = {8D53D749-D51C-46F8-A162-9371AAA6C2E7} {866C3F06-636F-4BE8-BC24-5F86ECC606A1} = {60618CAC-2995-4DF9-9914-45C6FC02C995} {1A47951F-5C7A-4D6D-BB5F-D77484437940} = {8D53D749-D51C-46F8-A162-9371AAA6C2E7} @@ -1421,6 +1426,7 @@ Global {167F634B-A3AD-494E-8E67-B888103E35FF} = {7C218A3E-9BC8-48FF-B91B-BCACD828C0C9} {C54F80ED-B736-49B0-9BD3-662F57024D01} = {7C218A3E-9BC8-48FF-B91B-BCACD828C0C9} {272B2B0E-40D4-4F0F-B187-519A6EF89B10} = {7C218A3E-9BC8-48FF-B91B-BCACD828C0C9} + {46E419FD-C77F-4EFC-A0FC-2BAD93E60D12} = {8D53D749-D51C-46F8-A162-9371AAA6C2E7} EndGlobalSection GlobalSection(ExtensibilityGlobals) = postSolution SolutionGuid = {B6FDB70C-A751-422C-ACD1-E35419495857} diff --git a/src/AppInstallerCommonCore/Architecture.cpp b/src/AppInstallerCommonCore/Architecture.cpp index ffab223d45..6f8ec557f3 100644 --- a/src/AppInstallerCommonCore/Architecture.cpp +++ b/src/AppInstallerCommonCore/Architecture.cpp @@ -187,6 +187,25 @@ namespace AppInstaller::Utility return Architecture::Unknown; } + std::optional<::AppInstaller::Utility::Architecture> ConvertToArchitectureEnum(winrt::Windows::System::ProcessorArchitecture architecture) + { + switch (architecture) + { + case winrt::Windows::System::ProcessorArchitecture::X86: + return ::AppInstaller::Utility::Architecture::X86; + case winrt::Windows::System::ProcessorArchitecture::Arm: + return ::AppInstaller::Utility::Architecture::Arm; + case winrt::Windows::System::ProcessorArchitecture::X64: + return ::AppInstaller::Utility::Architecture::X64; + case winrt::Windows::System::ProcessorArchitecture::Neutral: + return ::AppInstaller::Utility::Architecture::Neutral; + case winrt::Windows::System::ProcessorArchitecture::Arm64: + return ::AppInstaller::Utility::Architecture::Arm64; + } + + return {}; + } + LocIndView ToString(Architecture architecture) { switch (architecture) diff --git a/src/AppInstallerCommonCore/ExperimentalFeature.cpp b/src/AppInstallerCommonCore/ExperimentalFeature.cpp index 10dacf1fd0..76ecfa7f8a 100644 --- a/src/AppInstallerCommonCore/ExperimentalFeature.cpp +++ b/src/AppInstallerCommonCore/ExperimentalFeature.cpp @@ -48,6 +48,8 @@ namespace AppInstaller::Settings return userSettings.Get(); case ExperimentalFeature::Feature::Reboot: return userSettings.Get(); + case ExperimentalFeature::Feature::SideBySide: + return userSettings.Get(); default: THROW_HR(E_UNEXPECTED); } @@ -85,6 +87,8 @@ namespace AppInstaller::Settings return ExperimentalFeature{ "Configuration Schema 0.3", "configuration03", "https://aka.ms/winget-settings", Feature::Configuration03 }; case Feature::Reboot: return ExperimentalFeature{ "Reboot", "reboot", "https://aka.ms/winget-settings", Feature::Reboot }; + case Feature::SideBySide: + return ExperimentalFeature{ "Side-by-side improvements", "sideBySide", "https://aka.ms/winget-settings", Feature::SideBySide }; default: THROW_HR(E_UNEXPECTED); } diff --git a/src/AppInstallerCommonCore/Public/AppInstallerArchitecture.h b/src/AppInstallerCommonCore/Public/AppInstallerArchitecture.h index e75e0c7918..c2d9127035 100644 --- a/src/AppInstallerCommonCore/Public/AppInstallerArchitecture.h +++ b/src/AppInstallerCommonCore/Public/AppInstallerArchitecture.h @@ -1,9 +1,10 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. #pragma once - +#include #include #include +#include namespace AppInstaller::Utility { @@ -22,6 +23,9 @@ namespace AppInstaller::Utility // Converts a string to corresponding enum Architecture ConvertToArchitectureEnum(std::string_view archStr); + // Converts an ProcessorArchitecture to an Architecture + std::optional ConvertToArchitectureEnum(winrt::Windows::System::ProcessorArchitecture architecture); + // Converts an Architecture to a string_view LocIndView ToString(Architecture architecture); diff --git a/src/AppInstallerCommonCore/Public/winget/ExperimentalFeature.h b/src/AppInstallerCommonCore/Public/winget/ExperimentalFeature.h index 79c8e583d9..b79bb43d6a 100644 --- a/src/AppInstallerCommonCore/Public/winget/ExperimentalFeature.h +++ b/src/AppInstallerCommonCore/Public/winget/ExperimentalFeature.h @@ -27,6 +27,7 @@ namespace AppInstaller::Settings Resume = 0x4, Configuration03 = 0x8, Reboot = 0x10, + SideBySide = 0x20, Max, // This MUST always be after all experimental features // Features listed after Max will not be shown with the features command diff --git a/src/AppInstallerCommonCore/Public/winget/UserSettings.h b/src/AppInstallerCommonCore/Public/winget/UserSettings.h index 1983ed0679..4492a33a39 100644 --- a/src/AppInstallerCommonCore/Public/winget/UserSettings.h +++ b/src/AppInstallerCommonCore/Public/winget/UserSettings.h @@ -74,6 +74,7 @@ namespace AppInstaller::Settings EFResume, EFConfiguration03, EFReboot, + EFSideBySide, // Telemetry TelemetryDisable, // Install behavior @@ -155,6 +156,7 @@ namespace AppInstaller::Settings SETTINGMAPPING_SPECIALIZATION(Setting::EFResume, bool, bool, false, ".experimentalFeatures.resume"sv); SETTINGMAPPING_SPECIALIZATION(Setting::EFConfiguration03, bool, bool, false, ".experimentalFeatures.configuration03"sv); SETTINGMAPPING_SPECIALIZATION(Setting::EFReboot, bool, bool, false, ".experimentalFeatures.reboot"sv); + SETTINGMAPPING_SPECIALIZATION(Setting::EFSideBySide, bool, bool, false, ".experimentalFeatures.sideBySide"sv); // Telemetry SETTINGMAPPING_SPECIALIZATION(Setting::TelemetryDisable, bool, bool, false, ".telemetry.disable"sv); // Install behavior diff --git a/src/AppInstallerCommonCore/UserSettings.cpp b/src/AppInstallerCommonCore/UserSettings.cpp index c4f9c87d98..b6303113dc 100644 --- a/src/AppInstallerCommonCore/UserSettings.cpp +++ b/src/AppInstallerCommonCore/UserSettings.cpp @@ -263,6 +263,7 @@ namespace AppInstaller::Settings WINGET_VALIDATE_PASS_THROUGH(EFResume) WINGET_VALIDATE_PASS_THROUGH(EFConfiguration03) WINGET_VALIDATE_PASS_THROUGH(EFReboot) + WINGET_VALIDATE_PASS_THROUGH(EFSideBySide) WINGET_VALIDATE_PASS_THROUGH(AnonymizePathForDisplay) WINGET_VALIDATE_PASS_THROUGH(TelemetryDisable) WINGET_VALIDATE_PASS_THROUGH(InteractivityDisable) diff --git a/src/AppInstallerRepositoryCore/Microsoft/ARPHelper.cpp b/src/AppInstallerRepositoryCore/Microsoft/ARPHelper.cpp index 5bb9ef32ed..6e93bfe834 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/ARPHelper.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/ARPHelper.cpp @@ -403,7 +403,19 @@ namespace AppInstaller::Repository::Microsoft // `Publisher.DisplayName`. We would need to ensure that there are no matches // against the rest of the data however (might happen if same package is // installed for multiple architectures/languages). - manifest.Id = productCode; + if (Settings::ExperimentalFeature::IsEnabled(Settings::ExperimentalFeature::Feature::SideBySide)) + { + char separator = '\\'; + + std::ostringstream stream; + stream << "ARP" << separator << scope << separator << architecture << separator << productCode; + + manifest.Id = stream.str(); + } + else + { + manifest.Id = productCode; + } manifest.Installers.emplace_back(); // TODO: This likely needs some cleanup applied, as it looks like INNO tends to append an "_is#" @@ -491,7 +503,7 @@ namespace AppInstaller::Repository::Microsoft try { // Use the ProductCode as a unique key for the path - manifestIdOpt = index.AddManifest(manifest, Utility::ConvertToUTF16(manifest.Installers[0].ProductCode)); + manifestIdOpt = index.AddManifest(manifest); } catch (...) { diff --git a/src/AppInstallerRepositoryCore/Microsoft/PredefinedInstalledSourceFactory.cpp b/src/AppInstallerRepositoryCore/Microsoft/PredefinedInstalledSourceFactory.cpp index dd2408e5ef..8558e8650e 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/PredefinedInstalledSourceFactory.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/PredefinedInstalledSourceFactory.cpp @@ -9,6 +9,7 @@ #include #include +#include using namespace std::string_literals; using namespace std::string_view_literals; @@ -100,9 +101,17 @@ namespace AppInstaller::Repository::Microsoft } auto packageId = package.Id(); + Utility::NormalizedString fullName = Utility::ConvertToUTF8(packageId.FullName()); Utility::NormalizedString familyName = Utility::ConvertToUTF8(packageId.FamilyName()); - manifest.Id = familyName; + if (Settings::ExperimentalFeature::IsEnabled(Settings::ExperimentalFeature::Feature::SideBySide)) + { + manifest.Id = "MSIX\\" + fullName; + } + else + { + manifest.Id = familyName; + } // Get version std::ostringstream strstr; @@ -142,11 +151,11 @@ namespace AppInstaller::Repository::Microsoft catch (const winrt::hresult_error& hre) { AICLI_LOG(Repo, Warning, << "winrt::hresult_error[0x" << Logging::SetHRFormat << hre.code() << ": " << - Utility::ConvertToUTF8(hre.message()) << "] exception thrown when getting DisplayName for " << familyName); + Utility::ConvertToUTF8(hre.message()) << "] exception thrown when getting DisplayName for " << fullName); } catch (...) { - AICLI_LOG(Repo, Warning, << "Unknown exception thrown when getting DisplayName for " << familyName); + AICLI_LOG(Repo, Warning, << "Unknown exception thrown when getting DisplayName for " << fullName); } } @@ -160,17 +169,28 @@ namespace AppInstaller::Repository::Microsoft try { // Use the full name as a unique key for the path - auto manifestId = index.AddManifest(manifest, std::filesystem::path{ packageId.FullName().c_str() }); + auto manifestId = index.AddManifest(manifest); index.SetMetadataByManifestId(manifestId, PackageVersionMetadata::InstalledType, Manifest::InstallerTypeToString(Manifest::InstallerTypeEnum::Msix)); + + auto architecture = Utility::ConvertToArchitectureEnum(packageId.Architecture()); + if (architecture) + { + index.SetMetadataByManifestId(manifestId, PackageVersionMetadata::InstalledArchitecture, + ToString(architecture.value())); + } } catch (const wil::ResultException& resultException) { if (HRESULT_FROM_WIN32(ERROR_ALREADY_EXISTS) == resultException.GetErrorCode() && package.IsFramework()) { - // There may be multiple packages with same package family name for framework packages. - continue; + // This should not be needed with the SxS changes + if (!Settings::ExperimentalFeature::IsEnabled(Settings::ExperimentalFeature::Feature::SideBySide)) + { + // There may be multiple packages with same package family name for framework packages. + continue; + } } throw; @@ -183,7 +203,7 @@ namespace AppInstaller::Repository::Microsoft AICLI_LOG(Repo, Verbose, << "Creating PredefinedInstalledSource with filter [" << PredefinedInstalledSourceFactory::FilterToString(filter) << ']'); // Create an in memory index - SQLiteIndex index = SQLiteIndex::CreateNew(SQLITE_MEMORY_DB_CONNECTION_TARGET, SQLite::Version::Latest()); + SQLiteIndex index = SQLiteIndex::CreateNew(SQLITE_MEMORY_DB_CONNECTION_TARGET, SQLite::Version::Latest(), SQLiteIndex::CreateOptions::SupportPathless); // Put installed packages into the index if (filter == PredefinedInstalledSourceFactory::Filter::None || filter == PredefinedInstalledSourceFactory::Filter::ARP || diff --git a/src/Microsoft.Management.Deployment/Converters.cpp b/src/Microsoft.Management.Deployment/Converters.cpp index e96fe4b735..a737228ee3 100644 --- a/src/Microsoft.Management.Deployment/Converters.cpp +++ b/src/Microsoft.Management.Deployment/Converters.cpp @@ -207,21 +207,7 @@ namespace winrt::Microsoft::Management::Deployment::implementation std::optional<::AppInstaller::Utility::Architecture> GetUtilityArchitecture(winrt::Windows::System::ProcessorArchitecture architecture) { - switch (architecture) - { - case winrt::Windows::System::ProcessorArchitecture::X86: - return ::AppInstaller::Utility::Architecture::X86; - case winrt::Windows::System::ProcessorArchitecture::Arm: - return ::AppInstaller::Utility::Architecture::Arm; - case winrt::Windows::System::ProcessorArchitecture::X64: - return ::AppInstaller::Utility::Architecture::X64; - case winrt::Windows::System::ProcessorArchitecture::Neutral: - return ::AppInstaller::Utility::Architecture::Neutral; - case winrt::Windows::System::ProcessorArchitecture::Arm64: - return ::AppInstaller::Utility::Architecture::Arm64; - } - - return {}; + return ::AppInstaller::Utility::ConvertToArchitectureEnum(architecture); } std::optional GetWindowsSystemProcessorArchitecture(::AppInstaller::Utility::Architecture architecture) From 0fc500848c749fcd2716d73e1064684ba4c6000f Mon Sep 17 00:00:00 2001 From: JohnMcPMS Date: Thu, 25 Jan 2024 16:31:25 -0800 Subject: [PATCH 02/24] Create primary available package in composite --- .../Public/winget/Pin.h | 12 +- .../CompositeSource.cpp | 103 ++++++++++++------ 2 files changed, 81 insertions(+), 34 deletions(-) diff --git a/src/AppInstallerCommonCore/Public/winget/Pin.h b/src/AppInstallerCommonCore/Public/winget/Pin.h index 51376ce4fb..2d86e98de4 100644 --- a/src/AppInstallerCommonCore/Public/winget/Pin.h +++ b/src/AppInstallerCommonCore/Public/winget/Pin.h @@ -40,7 +40,7 @@ namespace AppInstaller::Pinning // but they break when the package is updated outside of winget. struct PinKey { - PinKey() {} + PinKey() = default; PinKey(const Manifest::Manifest::string_t& packageId, std::string_view sourceId) : PackageId(packageId), SourceId(sourceId) {} @@ -71,12 +71,18 @@ namespace AppInstaller::Pinning // Used for logging std::string ToString() const; - const std::string PackageId; - const std::string SourceId; + std::string PackageId; + std::string SourceId; }; struct Pin { + Pin(const Pin&) = default; + Pin& operator=(const Pin& other) = default; + + Pin(Pin&&) = default; + Pin& operator=(Pin&&) = default; + static Pin CreateBlockingPin(PinKey&& pinKey); static Pin CreatePinningPin(PinKey&& pinKey); static Pin CreateGatingPin(PinKey&& pinKey, Utility::GatedVersion&& gatedVersion); diff --git a/src/AppInstallerRepositoryCore/CompositeSource.cpp b/src/AppInstallerRepositoryCore/CompositeSource.cpp index da014e1a51..747d3c1fea 100644 --- a/src/AppInstallerRepositoryCore/CompositeSource.cpp +++ b/src/AppInstallerRepositoryCore/CompositeSource.cpp @@ -423,9 +423,16 @@ namespace AppInstaller::Repository { static constexpr IPackageType PackageType = IPackageType::PinnablePackage; - PinnablePackage() {} + PinnablePackage() = default; + + PinnablePackage(const PinnablePackage&) = default; + PinnablePackage& operator=(const PinnablePackage&) = default; + + PinnablePackage(PinnablePackage&&) = default; + PinnablePackage& operator=(PinnablePackage&&) = default; + PinnablePackage(std::shared_ptr package, std::optional pin = {}) - : m_package(package), m_pin(pin) + : m_package(std::move(package)), m_pin(std::move(pin)) { // Get the source ID for available packages auto availableVersion = m_package->GetLatestAvailableVersion(PinBehavior::IgnorePins); @@ -569,9 +576,9 @@ namespace AppInstaller::Repository Utility::LocIndString GetProperty(PackageProperty property) const override { std::shared_ptr truth; - if (m_defaultAvailablePackage) + if (m_primaryAvailablePackage) { - truth = m_defaultAvailablePackage->GetLatestAvailableVersion(PinBehavior::IgnorePins); + truth = m_primaryAvailablePackage->GetLatestAvailableVersion(PinBehavior::IgnorePins); } if (!truth) { @@ -616,21 +623,33 @@ namespace AppInstaller::Repository std::vector result; auto installedPin = GetInstalledPin(); - for (const auto& availablePackage : m_availablePackages) - { - auto versionKeys = availablePackage.GetAvailableVersionKeys(pinBehavior); - - // The version keys we have already have pin information from the available package. - // Here we also add information from the installed package. - if (installedPin) + auto processSinglePackage = [&](const PinnablePackage& package) { - for (auto& versionKey : versionKeys) + auto versionKeys = package.GetAvailableVersionKeys(pinBehavior); + + // The version keys we have already have pin information from the available package. + // Here we also add information from the installed package. + if (installedPin) { - versionKey.PinnedState = GetPinnedStateForVersion(versionKey, installedPin, pinBehavior); + for (auto& versionKey : versionKeys) + { + versionKey.PinnedState = GetPinnedStateForVersion(versionKey, installedPin, pinBehavior); + } } - } - std::copy(versionKeys.begin(), versionKeys.end(), std::back_inserter(result)); + std::copy(versionKeys.begin(), versionKeys.end(), std::back_inserter(result)); + }; + + if (ExperimentalFeature::IsEnabled(ExperimentalFeature::Feature::SideBySide) && m_primaryAvailablePackage) + { + processSinglePackage(m_primaryAvailablePackage.value()); + } + else + { + for (const auto& availablePackage : m_availablePackages) + { + processSinglePackage(availablePackage); + } } // Remove all elements whose channel does not match the installed package. @@ -664,18 +683,33 @@ namespace AppInstaller::Repository std::pair, Pinning::PinType> GetAvailableVersionAndPin(const PackageVersionKey& versionKey) const override { - for (const auto& availablePackage : m_availablePackages) + if (ExperimentalFeature::IsEnabled(ExperimentalFeature::Feature::SideBySide) && m_primaryAvailablePackage) { - if (!Utility::IsEmptyOrWhitespace(versionKey.SourceId) && versionKey.SourceId != availablePackage.GetSourceId()) + if (Utility::IsEmptyOrWhitespace(versionKey.SourceId) || versionKey.SourceId == m_primaryAvailablePackage->GetSourceId()) { - continue; + auto result = m_primaryAvailablePackage->GetAvailableVersionAndPin(versionKey); + if (result.first) + { + result.second = GetPinnedStateForVersion(versionKey, GetInstalledPin(), PinBehavior::ConsiderPins); + return result; + } } - - auto result = availablePackage.GetAvailableVersionAndPin(versionKey); - if (result.first) + } + else + { + for (const auto& availablePackage : m_availablePackages) { - result.second = GetPinnedStateForVersion(versionKey, GetInstalledPin(), PinBehavior::ConsiderPins); - return result; + if (!Utility::IsEmptyOrWhitespace(versionKey.SourceId) && versionKey.SourceId != availablePackage.GetSourceId()) + { + continue; + } + + auto result = availablePackage.GetAvailableVersionAndPin(versionKey); + if (result.first) + { + result.second = GetPinnedStateForVersion(versionKey, GetInstalledPin(), PinBehavior::ConsiderPins); + return result; + } } } @@ -767,18 +801,25 @@ namespace AppInstaller::Repository return m_trackingPackage; } - void AddAvailablePackage(std::shared_ptr availablePackage) + void AddAvailablePackage(std::shared_ptr availablePackage, bool setPrimary = false) { + // Disable primary if feature not enabled + setPrimary = setPrimary && ExperimentalFeature::IsEnabled(ExperimentalFeature::Feature::SideBySide); + if (availablePackage) { - if (!m_defaultAvailablePackage) + m_availablePackages.emplace_back(std::move(availablePackage)); + + if (setPrimary) { - // Set override only with the first available version found - m_defaultAvailablePackage = availablePackage; - TrySetOverrideInstalledVersion(m_defaultAvailablePackage); + m_primaryAvailablePackage = m_availablePackages.back(); } - m_availablePackages.emplace_back(std::move(availablePackage)); + // Set override for primary or with the first available version found + if (setPrimary || m_availablePackages.size() == 1) + { + TrySetOverrideInstalledVersion(m_availablePackages.back().GetPackage()); + } } } @@ -825,7 +866,7 @@ namespace AppInstaller::Repository private: // Try to set a version that will override the version string from the installed package - void TrySetOverrideInstalledVersion(std::shared_ptr availablePackage) + void TrySetOverrideInstalledVersion(const std::shared_ptr& availablePackage) { if (m_installedPackage && availablePackage) { @@ -852,7 +893,7 @@ namespace AppInstaller::Repository std::shared_ptr m_trackingPackage; std::shared_ptr m_trackingPackageVersion; std::string m_overrideInstalledVersion; - std::shared_ptr m_defaultAvailablePackage; + std::optional m_primaryAvailablePackage; std::vector m_availablePackages; }; From 5af51073dff871ce80f92817854cbfc4f07ce58f Mon Sep 17 00:00:00 2001 From: John McPherson Date: Mon, 29 Jan 2024 13:56:52 -0800 Subject: [PATCH 03/24] wip --- .../CompositeSource.cpp | 143 ++++++++++++++---- src/AppInstallerRepositoryCore/pch.h | 1 + 2 files changed, 114 insertions(+), 30 deletions(-) diff --git a/src/AppInstallerRepositoryCore/CompositeSource.cpp b/src/AppInstallerRepositoryCore/CompositeSource.cpp index 747d3c1fea..d46b1d774a 100644 --- a/src/AppInstallerRepositoryCore/CompositeSource.cpp +++ b/src/AppInstallerRepositoryCore/CompositeSource.cpp @@ -557,7 +557,7 @@ namespace AppInstaller::Repository { static constexpr IPackageType PackageType = IPackageType::CompositePackage; - CompositePackage(std::shared_ptr installedPackage, std::shared_ptr availablePackage = {}) + CompositePackage(std::shared_ptr installedPackage, std::shared_ptr availablePackage = {}, bool setPrimary = false) { // Grab the installed version's channel to allow for filtering in calls to get available info. if (installedPackage) @@ -570,7 +570,7 @@ namespace AppInstaller::Repository } } - AddAvailablePackage(std::move(availablePackage)); + AddAvailablePackage(std::move(availablePackage), setPrimary); } Utility::LocIndString GetProperty(PackageProperty property) const override @@ -823,16 +823,36 @@ namespace AppInstaller::Repository } } - void SetTracking(Source trackingSource, std::shared_ptr trackingPackage, std::shared_ptr trackingPackageVersion) + void SetTracking( + Source trackingSource, + std::shared_ptr trackingPackage, + std::shared_ptr trackingPackageVersion, + std::chrono::system_clock::time_point trackingWriteTime) { m_trackingSource = std::move(trackingSource); m_trackingPackage = std::move(trackingPackage); m_trackingPackageVersion = std::move(trackingPackageVersion); + m_trackingWriteTime = trackingWriteTime; } // Gets the information about the pins that exist for this package void GetExistingPins(PinningIndex& pinningIndex) { + if (ExperimentalFeature::IsEnabled(ExperimentalFeature::Feature::SideBySide) && m_primaryAvailablePackage) + { + // Safeguard in case a package with no available sneaks in as we intentionally do in tests + if (!m_primaryAvailablePackage->GetPackage()->GetAvailableVersionKeys().empty()) + { + Pinning::PinKey pinKey = GetPinKeyForAvailable(m_primaryAvailablePackage->GetPackage().get()); + + auto pin = pinningIndex.GetPin(pinKey); + if (pin.has_value()) + { + m_primaryAvailablePackage->SetPin(std::move(pin.value())); + } + } + } + for (auto& availablePackage : m_availablePackages) { // Safeguard in case a package with no available sneaks in as we intentionally do in tests @@ -864,6 +884,16 @@ namespace AppInstaller::Repository } } + std::optional& GetPrimaryAvailablePackage() + { + return m_primaryAvailablePackage; + } + + std::vector& GetAvailablePackages() + { + return m_availablePackages; + } + private: // Try to set a version that will override the version string from the installed package void TrySetOverrideInstalledVersion(const std::shared_ptr& availablePackage) @@ -892,6 +922,7 @@ namespace AppInstaller::Repository Source m_trackingSource; std::shared_ptr m_trackingPackage; std::shared_ptr m_trackingPackageVersion; + std::chrono::system_clock::time_point m_trackingWriteTime; std::string m_overrideInstalledVersion; std::optional m_primaryAvailablePackage; std::vector m_availablePackages; @@ -1100,9 +1131,11 @@ namespace AppInstaller::Repository return false; } - // Destructively converts the result to the standard variant. - operator SearchResult() && + // *Destructively* converts the result to the standard variant. + SearchResult ConvertToSearchResult() { + AddPinInfoToCompositeSearchResult(); + SearchResult result; result.Matches.reserve(Matches.size()); @@ -1160,6 +1193,54 @@ namespace AppInstaller::Repository return result; } + // Group results by their available package correlations. + void FoldInstalledResults() + { + if (!ExperimentalFeature::IsEnabled(ExperimentalFeature::Feature::SideBySide)) + { + return; + } + + struct InstalledResultFoldKey + { + InstalledResultFoldKey() + + size_t operator()() + { + + } + }; + + std::unordered_map foldData; + + // Attempt to fold all primary package matches first. + // Packages without primaries will still be indexed into the hash table. + size_t primaryFoldCount = 0; + for (size_t i = 0; i < Matches.size(); ++i) + { + CompositeResultMatch& currentMatch = Matches[i]; + + // Check current match for fold target + if (currentMatch.Package->GetPrimaryAvailablePackage()) + { + // TODO: Check for a matching primary in our hash table + // IF found, fold into previous value + // IF NOT found, add to hash table + } + else + { + // TODO: index available packages into hash table + } + } + + // After primary matches are folded, attempt to fold results without primary matches. + // The latest primary match will be preferred as a tiebreak. + + + // Get rid of the excess entries + Matches.resize(Matches.size() - primaryFoldCount); + } + std::vector Matches; bool Truncated = false; std::vector Failures; @@ -1249,6 +1330,23 @@ namespace AppInstaller::Repository } } } + + // Adds all the pin information to the results from a search to a CompositeSource. + void AddPinInfoToCompositeSearchResult() + { + if (!Matches.empty()) + { + // Look up any pins for the packages found + auto pinningIndex = PinningIndex::OpenOrCreateDefault(); + if (pinningIndex) + { + for (auto& match : Matches) + { + match.Package->GetExistingPins(*pinningIndex); + } + } + } + } }; std::shared_ptr GetTrackedPackageFromAvailableSource(CompositeResult& result, const Source& source, const Utility::LocIndString& identifier) @@ -1273,23 +1371,6 @@ namespace AppInstaller::Repository return {}; } - - // Adds all the pin information to the results from a search to a CompositeSource. - void AddPinInfoToCompositeSearchResult(CompositeResult& result) - { - if (!result.Matches.empty()) - { - // Look up any pins for the packages found - auto pinningIndex = PinningIndex::OpenOrCreateDefault(); - if (pinningIndex) - { - for (auto& match : result.Matches) - { - match.Package->GetExistingPins(*pinningIndex); - } - } - } - } } CompositeSource::CompositeSource(std::string identifier) @@ -1459,9 +1540,9 @@ namespace AppInstaller::Repository auto availablePackage = GetTrackedPackageFromAvailableSource(result, trackedSource, trackingPackage->GetProperty(PackageProperty::Id)); if (availablePackage) { - compositePackage->AddAvailablePackage(std::move(availablePackage)); + compositePackage->AddAvailablePackage(std::move(availablePackage), true); } - compositePackage->SetTracking(std::move(trackedSource), std::move(trackingPackage), std::move(trackingPackageVersion)); + compositePackage->SetTracking(std::move(trackedSource), std::move(trackingPackage), std::move(trackingPackageVersion), trackingPackageTime); } // Search sources and add to result @@ -1499,11 +1580,13 @@ namespace AppInstaller::Repository result.Matches.emplace_back(std::move(compositePackage), std::move(match.MatchCriteria)); } + // Group multiple instances of installed items into a single result item + result.FoldInstalledResults(); + // Optimization for the "everything installed" case, no need to allow for reverse correlations if (request.IsForEverything() && m_searchBehavior == CompositeSearchBehavior::Installed) { - AddPinInfoToCompositeSearchResult(result); - return std::move(result); + return result.ConvertToSearchResult(); } } @@ -1548,11 +1631,12 @@ namespace AppInstaller::Repository { auto compositePackage = std::make_shared( std::move(installedPackage), - GetTrackedPackageFromAvailableSource(result, source, match.Package->GetProperty(PackageProperty::Id))); + GetTrackedPackageFromAvailableSource(result, source, match.Package->GetProperty(PackageProperty::Id)), + true); auto [writeTime, trackingPackageVersion] = GetLatestTrackingWriteTimeAndPackageVersion(match.Package); - compositePackage->SetTracking(source, std::move(match.Package), std::move(trackingPackageVersion)); + compositePackage->SetTracking(source, std::move(match.Package), std::move(trackingPackageVersion), writeTime); result.Matches.emplace_back(std::move(compositePackage), match.MatchCriteria); } @@ -1620,8 +1704,7 @@ namespace AppInstaller::Repository result.Matches.erase(result.Matches.begin() + request.MaximumResults, result.Matches.end()); } - AddPinInfoToCompositeSearchResult(result); - return std::move(result); + return result.ConvertToSearchResult(); } // An available search goes through each source, searching individually and then sorting the full result set. diff --git a/src/AppInstallerRepositoryCore/pch.h b/src/AppInstallerRepositoryCore/pch.h index 512990f9bd..2199bd6606 100644 --- a/src/AppInstallerRepositoryCore/pch.h +++ b/src/AppInstallerRepositoryCore/pch.h @@ -66,6 +66,7 @@ #include #include #include +#include #include #include From 04a638bddedbe2ed862aa4455925c86552a373e4 Mon Sep 17 00:00:00 2001 From: John McPherson Date: Wed, 31 Jan 2024 21:20:08 -0800 Subject: [PATCH 04/24] WIP; folding after installed --- .../CompositeSource.cpp | 139 ++++++++++++++++-- 1 file changed, 128 insertions(+), 11 deletions(-) diff --git a/src/AppInstallerRepositoryCore/CompositeSource.cpp b/src/AppInstallerRepositoryCore/CompositeSource.cpp index d46b1d774a..0b3c141be8 100644 --- a/src/AppInstallerRepositoryCore/CompositeSource.cpp +++ b/src/AppInstallerRepositoryCore/CompositeSource.cpp @@ -582,15 +582,15 @@ namespace AppInstaller::Repository } if (!truth) { - truth = m_trackingPackageVersion; + truth = GetLatestAvailableVersion(PinBehavior::IgnorePins); } if (!truth) { - truth = GetInstalledVersion(); + truth = m_trackingPackageVersion; } if (!truth) { - truth = GetLatestAvailableVersion(PinBehavior::IgnorePins); + truth = GetInstalledVersion(); } switch (property) @@ -1201,44 +1201,161 @@ namespace AppInstaller::Repository return; } + // The key to uniquely identify the package in the map struct InstalledResultFoldKey { - InstalledResultFoldKey() + InstalledResultFoldKey() = default; + + InstalledResultFoldKey(const std::shared_ptr& package) + { + std::shared_ptr latestAvailable = package->GetLatestAvailableVersion(PinBehavior::IgnorePins); + if (latestAvailable) + { + SourceIdentifier = latestAvailable->GetSource().GetIdentifier(); + PackageIdentifier = latestAvailable->GetProperty(PackageVersionProperty::Id); + } + } + + // Hash operation + size_t operator()(const InstalledResultFoldKey& value) const noexcept + { + std::hash hashString; + return hashString(value.SourceIdentifier) ^ (hashString(value.PackageIdentifier) << 1); + } - size_t operator()() + bool operator==(const InstalledResultFoldKey& other) const noexcept { + // Treat both empty as invalid and never equal + if (SourceIdentifier.empty() && PackageIdentifier.empty()) + { + return false; + } + return SourceIdentifier == other.SourceIdentifier && PackageIdentifier == other.PackageIdentifier; } + + std::string SourceIdentifier; + std::string PackageIdentifier; + }; + + // The data for a package in the map + struct InstalledResultFoldData + { + InstalledResultFoldData() = default; + explicit InstalledResultFoldData(size_t primaryPackageIndex) : PrimaryPackageIndex(primaryPackageIndex) {} + + std::optional PrimaryPackageIndex; + std::vector NonPrimaryPackageIndices; }; std::unordered_map foldData; // Attempt to fold all primary package matches first. // Packages without primaries will still be indexed into the hash table. - size_t primaryFoldCount = 0; + size_t foldCount = 0; for (size_t i = 0; i < Matches.size(); ++i) { CompositeResultMatch& currentMatch = Matches[i]; + size_t matchMoveLocation = i - foldCount; // Check current match for fold target if (currentMatch.Package->GetPrimaryAvailablePackage()) { - // TODO: Check for a matching primary in our hash table - // IF found, fold into previous value - // IF NOT found, add to hash table + InstalledResultFoldKey key{ currentMatch.Package->GetPrimaryAvailablePackage()->GetPackage() }; + + auto itr = foldData.find(key); + if (itr != foldData.end()) + { + if (itr->second.PrimaryPackageIndex) + { + // TODO: add installed version into target package + ++foldCount; + continue; + } + else + { + itr->second.PrimaryPackageIndex = matchMoveLocation; + } + } + else + { + foldData[key] = InstalledResultFoldData{ matchMoveLocation }; + } } else { - // TODO: index available packages into hash table + for (const auto& availablePackage : currentMatch.Package->GetAvailablePackages()) + { + InstalledResultFoldKey key{ availablePackage.GetPackage() }; + + auto itr = foldData.find(key); + if (itr == foldData.end()) + { + itr = foldData.insert({ key, {} }).first; + } + + itr->second.NonPrimaryPackageIndices.emplace_back(matchMoveLocation); + } + } + + if (matchMoveLocation != i) + { + Matches[matchMoveLocation] = std::move(Matches[i]); } } + // Get rid of the excess entries + Matches.erase(Matches.end() - foldCount, Matches.end()); + // After primary matches are folded, attempt to fold results without primary matches. // The latest primary match will be preferred as a tiebreak. + foldCount = 0; + for (size_t i = 0; i < Matches.size(); ++i) + { + CompositeResultMatch& currentMatch = Matches[i]; + size_t matchMoveLocation = i - foldCount; + + if (!currentMatch.Package->GetPrimaryAvailablePackage()) + { + InstalledResultFoldData* latestPrimaryAvailable = nullptr; + std::vector availableFoldData; + + for (const auto& availablePackage : currentMatch.Package->GetAvailablePackages()) + { + auto& packageFoldData = foldData.at(availablePackage.GetPackage()); + + if (packageFoldData.PrimaryPackageIndex) + { + if (!latestPrimaryAvailable || + Matches[latestPrimaryAvailable->PrimaryPackageIndex.value()].Package->GetTrackingPackageWriteTime() < Matches[packageFoldData.PrimaryPackageIndex.value()].Package->GetTrackingPackageWriteTime()) + { + latestPrimaryAvailable = &packageFoldData; + } + } + else + { + availableFoldData.emplace_back(&packageFoldData); + } + } + + if (latestPrimaryAvailable) + { + // TODO: Fold into latest primary + ++foldCount; + continue; + } + + // TODO: Given a set of + } + if (matchMoveLocation != i) + { + Matches[matchMoveLocation] = std::move(Matches[i]); + } + } // Get rid of the excess entries - Matches.resize(Matches.size() - primaryFoldCount); + Matches.erase(Matches.end() - foldCount, Matches.end()); } std::vector Matches; From 349aa61ac130b14309e25912a122afaad1abee1c Mon Sep 17 00:00:00 2001 From: John McPherson Date: Thu, 1 Feb 2024 13:46:40 -0800 Subject: [PATCH 05/24] Initial folding pass complete, needs to actually fold --- .../CompositeSource.cpp | 100 +++++++++++++----- 1 file changed, 71 insertions(+), 29 deletions(-) diff --git a/src/AppInstallerRepositoryCore/CompositeSource.cpp b/src/AppInstallerRepositoryCore/CompositeSource.cpp index 0b3c141be8..d332635757 100644 --- a/src/AppInstallerRepositoryCore/CompositeSource.cpp +++ b/src/AppInstallerRepositoryCore/CompositeSource.cpp @@ -835,6 +835,11 @@ namespace AppInstaller::Repository m_trackingWriteTime = trackingWriteTime; } + std::chrono::system_clock::time_point GetTrackingPackageWriteTime() const + { + return m_trackingWriteTime; + } + // Gets the information about the pins that exist for this package void GetExistingPins(PinningIndex& pinningIndex) { @@ -922,7 +927,7 @@ namespace AppInstaller::Repository Source m_trackingSource; std::shared_ptr m_trackingPackage; std::shared_ptr m_trackingPackageVersion; - std::chrono::system_clock::time_point m_trackingWriteTime; + std::chrono::system_clock::time_point m_trackingWriteTime = std::chrono::system_clock::time_point::min(); std::string m_overrideInstalledVersion; std::optional m_primaryAvailablePackage; std::vector m_availablePackages; @@ -1193,7 +1198,25 @@ namespace AppInstaller::Repository return result; } - // Group results by their available package correlations. + // Group results in an attempt to have a single result that covers all installed versions. + // This is expected to be called immediately after the installed search portion, + // when each result will contain a single installed version and some number of available packages. + // + // The folds that happen are: + // 1. When results have the same primary available package (the primary available package is set due to tracking data) + // 2. When a result has no primary available package, but another result does have a primary that matches one of the availables + // a. Choose the latest primary if there are multiple + // 3. When multiple results have no primary available package and share the same available package set + // a. There are many potential additional rules that could be made here, but we will start with the simplest version. + // + // Potential improvements: + // 1. Attempting correlation of non-primary available packages to allow folding in more complex cases + // a. For example, if installed A has {source1:package1, source2:package2} and installed B has {source1:package1}, can we + // make sure that source1:package1 and source2:package2 are in fact "the same" to confidently say that installed A and B + // are side by side versions. + // 2. Attempt correlation by installed data only + // a. We can potentially detect multiple instances of the same installed item with the same correlation logic turned back on + // the installed source. This would allow for folding even when the package is not in any available source. void FoldInstalledResults() { if (!ExperimentalFeature::IsEnabled(ExperimentalFeature::Feature::SideBySide)) @@ -1252,11 +1275,9 @@ namespace AppInstaller::Repository // Attempt to fold all primary package matches first. // Packages without primaries will still be indexed into the hash table. - size_t foldCount = 0; for (size_t i = 0; i < Matches.size(); ++i) { CompositeResultMatch& currentMatch = Matches[i]; - size_t matchMoveLocation = i - foldCount; // Check current match for fold target if (currentMatch.Package->GetPrimaryAvailablePackage()) @@ -1269,17 +1290,16 @@ namespace AppInstaller::Repository if (itr->second.PrimaryPackageIndex) { // TODO: add installed version into target package - ++foldCount; - continue; + currentMatch.Package.reset(); } else { - itr->second.PrimaryPackageIndex = matchMoveLocation; + itr->second.PrimaryPackageIndex = i; } } else { - foldData[key] = InstalledResultFoldData{ matchMoveLocation }; + foldData[key] = InstalledResultFoldData{ i }; } } else @@ -1294,26 +1314,22 @@ namespace AppInstaller::Repository itr = foldData.insert({ key, {} }).first; } - itr->second.NonPrimaryPackageIndices.emplace_back(matchMoveLocation); + itr->second.NonPrimaryPackageIndices.emplace_back(i); } } - - if (matchMoveLocation != i) - { - Matches[matchMoveLocation] = std::move(Matches[i]); - } } - // Get rid of the excess entries - Matches.erase(Matches.end() - foldCount, Matches.end()); - // After primary matches are folded, attempt to fold results without primary matches. - // The latest primary match will be preferred as a tiebreak. - foldCount = 0; + // The latest primary match will be preferred. for (size_t i = 0; i < Matches.size(); ++i) { CompositeResultMatch& currentMatch = Matches[i]; - size_t matchMoveLocation = i - foldCount; + + // Skip any matches that we have already folded + if (!currentMatch.Package) + { + continue; + } if (!currentMatch.Package->GetPrimaryAvailablePackage()) { @@ -1340,22 +1356,48 @@ namespace AppInstaller::Repository if (latestPrimaryAvailable) { - // TODO: Fold into latest primary - ++foldCount; + // TODO: Fold with the latest primary, preserving the one with the lower index + // TODO: reset the Package from the match that is not kept continue; } - // TODO: Given a set of - } + // First, find the intersection of all results that contain all of the packages from this result. + std::vector candidateMatches; + for (size_t j = 0; j < availableFoldData.size(); ++j) + { + InstalledResultFoldData* packageFoldData = availableFoldData[j]; - if (matchMoveLocation != i) - { - Matches[matchMoveLocation] = std::move(Matches[i]); + if (j == 0) + { + candidateMatches = packageFoldData->NonPrimaryPackageIndices; + } + else + { + std::vector temp; + std::set_intersection( + candidateMatches.begin(), candidateMatches.end(), + packageFoldData->NonPrimaryPackageIndices.begin(), packageFoldData->NonPrimaryPackageIndices.end(), + std::back_inserter(temp)); + candidateMatches = std::move(temp); + } + } + + // Now exclude both our own result and any that have a different (larger) number of available packages + candidateMatches.erase(std::remove_if(candidateMatches.begin(), candidateMatches.end(), + [&](size_t index) { return index == i || Matches[index].Package->GetAvailablePackages().size() != currentMatch.Package->GetAvailablePackages().size(); }), + candidateMatches.end()); + + // All of these remaining values should be folded in to our result + for (size_t foldTarget : candidateMatches) + { + // TODO: Fold into our result + Matches[foldTarget].Package.reset(); + } } } - // Get rid of the excess entries - Matches.erase(Matches.end() - foldCount, Matches.end()); + // Get rid of the folded results; we reset the Package to indicate that it is no longer valid + Matches.erase(std::remove_if(Matches.begin(), Matches.end(), [&](const CompositeResultMatch& match) { return !match.Package; }), Matches.end()); } std::vector Matches; From a5d262c710c7412912dea826764634d143c56a7a Mon Sep 17 00:00:00 2001 From: John McPherson Date: Fri, 2 Feb 2024 13:43:48 -0800 Subject: [PATCH 06/24] wip; adding support for multiple installed package entries --- src/AppInstallerCLICore/Workflows/PinFlow.cpp | 2 +- .../CompositeSource.cpp | 51 ++++++++++++------- 2 files changed, 33 insertions(+), 20 deletions(-) diff --git a/src/AppInstallerCLICore/Workflows/PinFlow.cpp b/src/AppInstallerCLICore/Workflows/PinFlow.cpp index c5b5440db6..ae6b005ed0 100644 --- a/src/AppInstallerCLICore/Workflows/PinFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/PinFlow.cpp @@ -43,7 +43,7 @@ namespace AppInstaller::CLI::Workflow auto installedVersion = package->GetInstalledVersion(); if (installedVersion) { - pinKeys.emplace(Pinning::PinKey::GetPinKeyForInstalled(installedVersion->GetProperty(PackageVersionProperty::Id))); + pinKeys.emplace(Pinning::PinKey::GetPinKeyForInstalled(installedVersion)); } } else diff --git a/src/AppInstallerRepositoryCore/CompositeSource.cpp b/src/AppInstallerRepositoryCore/CompositeSource.cpp index d332635757..1e746e0d0e 100644 --- a/src/AppInstallerRepositoryCore/CompositeSource.cpp +++ b/src/AppInstallerRepositoryCore/CompositeSource.cpp @@ -562,8 +562,8 @@ namespace AppInstaller::Repository // Grab the installed version's channel to allow for filtering in calls to get available info. if (installedPackage) { - m_installedPackage.emplace(installedPackage); - auto installedVersion = m_installedPackage->GetInstalledVersion(); + m_installedPackages.emplace_back(std::move(installedPackage)); + auto installedVersion = m_installedPackages.front().GetPackage()->GetInstalledVersion(); if (installedVersion) { m_installedChannel = installedVersion->GetProperty(PackageVersionProperty::Channel); @@ -735,15 +735,18 @@ namespace AppInstaller::Repository const CompositePackage* otherComposite = PackageCast(other); if (!otherComposite || - static_cast(m_installedPackage) != static_cast(otherComposite->m_installedPackage) || + m_installedPackages.size() != otherComposite->m_installedPackages.size() || m_availablePackages.size() != otherComposite->m_availablePackages.size()) { return false; } - - if (m_installedPackage && !m_installedPackage->GetPackage()->IsSame(otherComposite->m_installedPackage->GetPackage().get())) + + for (size_t i = 0; i < m_installedPackages.size(); ++i) { - return false; + if (!m_installedPackages[i].GetPackage()->IsSame(otherComposite->m_installedPackages[i].GetPackage().get())) + { + return false; + } } for (size_t i = 0; i < m_availablePackages.size(); ++i) @@ -875,16 +878,14 @@ namespace AppInstaller::Repository } } - if (m_installedPackage) + for (auto& installedPackage : m_installedPackages) { - Pinning::PinKey pinKey = Pinning::PinKey::GetPinKeyForInstalled( - m_installedPackage->GetProperty(PackageProperty::Id).get() - ); + Pinning::PinKey pinKey = Pinning::PinKey::GetPinKeyForInstalled(installedPackage); auto pin = pinningIndex.GetPin(pinKey); if (pin.has_value()) { - m_installedPackage->SetPin(std::move(pin.value())); + installedPackage.SetPin(std::move(pin.value())); } } } @@ -899,6 +900,11 @@ namespace AppInstaller::Repository return m_availablePackages; } + void FoldInstalledIn(const std::shared_ptr& other) + { + std::move(other->m_installedPackages.begin(), other->m_installedPackages.end(), std::back_inserter(m_installedPackages)); + } + private: // Try to set a version that will override the version string from the installed package void TrySetOverrideInstalledVersion(const std::shared_ptr& availablePackage) @@ -922,13 +928,13 @@ namespace AppInstaller::Repository return m_installedPackage ? m_installedPackage->GetPin() : std::nullopt; } - std::optional m_installedPackage; - Utility::LocIndString m_installedChannel; + std::vector m_installedPackages; + Utility::LocIndString m_installedChannel_FIXUP; Source m_trackingSource; std::shared_ptr m_trackingPackage; std::shared_ptr m_trackingPackageVersion; std::chrono::system_clock::time_point m_trackingWriteTime = std::chrono::system_clock::time_point::min(); - std::string m_overrideInstalledVersion; + std::string m_overrideInstalledVersion_FIXUP; std::optional m_primaryAvailablePackage; std::vector m_availablePackages; }; @@ -1204,7 +1210,7 @@ namespace AppInstaller::Repository // // The folds that happen are: // 1. When results have the same primary available package (the primary available package is set due to tracking data) - // 2. When a result has no primary available package, but another result does have a primary that matches one of the availables + // 2. When a result has no primary available package, but another result does have a primary that matches one of the available // a. Choose the latest primary if there are multiple // 3. When multiple results have no primary available package and share the same available package set // a. There are many potential additional rules that could be made here, but we will start with the simplest version. @@ -1289,7 +1295,7 @@ namespace AppInstaller::Repository { if (itr->second.PrimaryPackageIndex) { - // TODO: add installed version into target package + Matches[itr->second.PrimaryPackageIndex.value()].Package->FoldInstalledIn(currentMatch.Package); currentMatch.Package.reset(); } else @@ -1356,8 +1362,15 @@ namespace AppInstaller::Repository if (latestPrimaryAvailable) { - // TODO: Fold with the latest primary, preserving the one with the lower index - // TODO: reset the Package from the match that is not kept + Matches[latestPrimaryAvailable->PrimaryPackageIndex.value()].Package->FoldInstalledIn(currentMatch.Package); + currentMatch.Package.reset(); + + // If the result with the primary is later, move it forward + if (latestPrimaryAvailable->PrimaryPackageIndex.value() > i) + { + currentMatch.Package = std::move(Matches[latestPrimaryAvailable->PrimaryPackageIndex.value()].Package); + Matches[latestPrimaryAvailable->PrimaryPackageIndex.value()].Package.reset(); + } continue; } @@ -1390,7 +1403,7 @@ namespace AppInstaller::Repository // All of these remaining values should be folded in to our result for (size_t foldTarget : candidateMatches) { - // TODO: Fold into our result + currentMatch.Package->FoldInstalledIn(Matches[foldTarget].Package); Matches[foldTarget].Package.reset(); } } From 29d60fc15fb2266af9616137455d3f1b4b778c99 Mon Sep 17 00:00:00 2001 From: John McPherson Date: Wed, 7 Feb 2024 11:39:00 -0800 Subject: [PATCH 07/24] pause for integration of pinning changes --- .../CompositeSource.cpp | 80 +++++++++++-------- 1 file changed, 46 insertions(+), 34 deletions(-) diff --git a/src/AppInstallerRepositoryCore/CompositeSource.cpp b/src/AppInstallerRepositoryCore/CompositeSource.cpp index 1e746e0d0e..3fe3086cd0 100644 --- a/src/AppInstallerRepositoryCore/CompositeSource.cpp +++ b/src/AppInstallerRepositoryCore/CompositeSource.cpp @@ -552,6 +552,25 @@ namespace AppInstaller::Repository std::optional m_pin; }; + // A pinnable package that is installed. + struct InstalledPinnablePackage + { + InstalledPinnablePackage(std::shared_ptr package) + : Pinnable(std::move(package)) + { + // Get the version for installed packages + auto installedVersion = Pinnable.GetInstalledVersion(); + if (installedVersion) + { + Version.Assign(installedVersion->GetProperty(PackageVersionProperty::Version)); + } + } + + PinnablePackage Pinnable; + Utility::Version Version; + std::string OverrideVersion; + }; + // A composite package for the CompositeSource. struct CompositePackage : public IPackage { @@ -563,11 +582,6 @@ namespace AppInstaller::Repository if (installedPackage) { m_installedPackages.emplace_back(std::move(installedPackage)); - auto installedVersion = m_installedPackages.front().GetPackage()->GetInstalledVersion(); - if (installedVersion) - { - m_installedChannel = installedVersion->GetProperty(PackageVersionProperty::Channel); - } } AddAvailablePackage(std::move(availablePackage), setPrimary); @@ -606,12 +620,14 @@ namespace AppInstaller::Repository std::shared_ptr GetInstalledVersion() const override { - if (m_installedPackage) + if (!m_installedPackages.empty()) { - auto installedVersion = m_installedPackage->GetInstalledVersion(); + const InstalledPinnablePackage& firstInstalled = m_installedPackages.front(); + + auto installedVersion = firstInstalled.Pinnable.GetInstalledVersion(); if (installedVersion) { - return std::make_shared(std::move(installedVersion), m_trackingSource, m_trackingPackageVersion, m_overrideInstalledVersion); + return std::make_shared(std::move(installedVersion), m_trackingSource, m_trackingPackageVersion, firstInstalled.OverrideVersion); } } @@ -652,11 +668,7 @@ namespace AppInstaller::Repository } } - // Remove all elements whose channel does not match the installed package. - std::string_view channel = m_installedChannel; - result.erase( - std::remove_if(result.begin(), result.end(), [&](const PackageVersionKey& pvk) { return !Utility::ICUCaseInsensitiveEquals(pvk.Channel, channel); }), - result.end()); + // TODO: Remove all elements whose channel does not match the installed packages. // Put latest versions at the front; for versions available from multiple sources maintain the order they were added in std::stable_sort(result.begin(), result.end()); @@ -743,7 +755,7 @@ namespace AppInstaller::Repository for (size_t i = 0; i < m_installedPackages.size(); ++i) { - if (!m_installedPackages[i].GetPackage()->IsSame(otherComposite->m_installedPackages[i].GetPackage().get())) + if (!m_installedPackages[i].Pinnable.GetPackage()->IsSame(otherComposite->m_installedPackages[i].Pinnable.GetPackage().get())) { return false; } @@ -787,15 +799,14 @@ namespace AppInstaller::Repository return false; } - std::shared_ptr GetInstalledPackage() const + bool ContainsInstalledPackage(const IPackage* installedPackage) const { - if (m_installedPackage) + for (const InstalledPinnablePackage& installed : m_installedPackages) { - return m_installedPackage->GetPackage(); - } - else - { - return {}; + if (installed.Pinnable.GetPackage()->IsSame(installedPackage)) + { + return true; + } } } @@ -885,7 +896,7 @@ namespace AppInstaller::Repository auto pin = pinningIndex.GetPin(pinKey); if (pin.has_value()) { - installedPackage.SetPin(std::move(pin.value())); + installedPackage.Pinnable.SetPin(std::move(pin.value())); } } } @@ -903,21 +914,25 @@ namespace AppInstaller::Repository void FoldInstalledIn(const std::shared_ptr& other) { std::move(other->m_installedPackages.begin(), other->m_installedPackages.end(), std::back_inserter(m_installedPackages)); + std::sort(m_installedPackages.begin(), m_installedPackages.end(), [](const InstalledPinnablePackage& a, const InstalledPinnablePackage& b) { return a.Version < b.Version; }); } private: // Try to set a version that will override the version string from the installed package void TrySetOverrideInstalledVersion(const std::shared_ptr& availablePackage) { - if (m_installedPackage && availablePackage) + if (availablePackage) { - auto installedVersion = m_installedPackage->GetInstalledVersion(); - if (installedVersion) + for (InstalledPinnablePackage& package : m_installedPackages) { - auto installedType = Manifest::ConvertToInstallerTypeEnum(installedVersion->GetMetadata()[PackageVersionMetadata::InstalledType]); - if (Manifest::DoesInstallerTypeSupportArpVersionRange(installedType)) + auto installedVersion = package.Pinnable.GetInstalledVersion(); + if (installedVersion) { - m_overrideInstalledVersion = GetMappedInstalledVersion(installedVersion->GetProperty(PackageVersionProperty::Version), availablePackage); + auto installedType = Manifest::ConvertToInstallerTypeEnum(installedVersion->GetMetadata()[PackageVersionMetadata::InstalledType]); + if (Manifest::DoesInstallerTypeSupportArpVersionRange(installedType)) + { + package.OverrideVersion = GetMappedInstalledVersion(installedVersion->GetProperty(PackageVersionProperty::Version), availablePackage); + } } } } @@ -928,13 +943,11 @@ namespace AppInstaller::Repository return m_installedPackage ? m_installedPackage->GetPin() : std::nullopt; } - std::vector m_installedPackages; - Utility::LocIndString m_installedChannel_FIXUP; + std::vector m_installedPackages; Source m_trackingSource; std::shared_ptr m_trackingPackage; std::shared_ptr m_trackingPackageVersion; std::chrono::system_clock::time_point m_trackingWriteTime = std::chrono::system_clock::time_point::min(); - std::string m_overrideInstalledVersion_FIXUP; std::optional m_primaryAvailablePackage; std::vector m_availablePackages; }; @@ -1128,12 +1141,11 @@ namespace AppInstaller::Repository } // Determines if the results contain the given installed package. - bool ContainsInstalledPackage(const IPackage* installedPackage) + bool ContainsInstalledPackage(const IPackage* installedPackage) const { for (auto& match : Matches) { - const std::shared_ptr& matchPackage = match.Package->GetInstalledPackage(); - if (matchPackage && matchPackage->IsSame(installedPackage)) + if (match.Package->ContainsInstalledPackage(installedPackage)) { return true; } From e93c0381fcdf3d5c8be8bf856b6afd8d2471daa3 Mon Sep 17 00:00:00 2001 From: John McPherson Date: Thu, 15 Feb 2024 17:39:09 -0800 Subject: [PATCH 08/24] stash --- src/AppInstallerRepositoryCore/CompositeSource.cpp | 6 +++--- src/AppInstallerRepositoryCore/Microsoft/PinningIndex.h | 3 +-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/AppInstallerRepositoryCore/CompositeSource.cpp b/src/AppInstallerRepositoryCore/CompositeSource.cpp index 64acb1a3aa..4c79b4c126 100644 --- a/src/AppInstallerRepositoryCore/CompositeSource.cpp +++ b/src/AppInstallerRepositoryCore/CompositeSource.cpp @@ -394,15 +394,15 @@ namespace AppInstaller::Repository { std::vector result; - auto processSinglePackage = [&](const PinnablePackage& package) + auto processSinglePackage = [&](const std::shared_ptr& package) { - auto versionKeys = package.GetAvailableVersionKeys(); + auto versionKeys = package->GetAvailableVersionKeys(); std::copy(versionKeys.begin(), versionKeys.end(), std::back_inserter(result)); }; if (ExperimentalFeature::IsEnabled(ExperimentalFeature::Feature::SideBySide) && m_primaryAvailablePackage) { - processSinglePackage(m_primaryAvailablePackage.value()); + processSinglePackage(m_primaryAvailablePackage); } else { diff --git a/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.h b/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.h index 45cf3103ba..672f8dba8f 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.h +++ b/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.h @@ -29,8 +29,7 @@ namespace AppInstaller::Repository::Microsoft return { filePath, disposition, std::move(indexFile) }; } - // Opens or creates a PinningIndex database on the default path. - // openDisposition is only used when opening an existing database. + // Opens the PinningIndex database on the default path if it exists. // Returns nullptr in case of error. static std::shared_ptr OpenIfExists(OpenDisposition openDisposition = OpenDisposition::Read); From c3531fbe78875318e300dba792969ee10bf7d661 Mon Sep 17 00:00:00 2001 From: John McPherson Date: Tue, 27 Feb 2024 11:49:24 -0800 Subject: [PATCH 09/24] Manual merge of CompositeSource --- .../CompositeSource.cpp | 306 +++--------------- 1 file changed, 52 insertions(+), 254 deletions(-) diff --git a/src/AppInstallerRepositoryCore/CompositeSource.cpp b/src/AppInstallerRepositoryCore/CompositeSource.cpp index da855c4fc4..6d09a5f1d5 100644 --- a/src/AppInstallerRepositoryCore/CompositeSource.cpp +++ b/src/AppInstallerRepositoryCore/CompositeSource.cpp @@ -346,48 +346,6 @@ namespace AppInstaller::Repository { static constexpr IPackageType PackageType = IPackageType::CompositeInstalledPackage; -<<<<<<< HEAD - CompositePackage(std::shared_ptr installedPackage, std::shared_ptr availablePackage = {}, bool setPrimary = false) - { - // Grab the installed version's channel to allow for filtering in calls to get available info. - if (installedPackage) - { - m_installedPackages.emplace_back(std::move(installedPackage)); - } - - AddAvailablePackage(std::move(availablePackage), setPrimary); - } - - Utility::LocIndString GetProperty(PackageProperty property) const override - { - std::shared_ptr truth; - if (m_primaryAvailablePackage) - { - truth = m_primaryAvailablePackage->GetLatestAvailableVersion(); - } - if (!truth) - { - truth = GetLatestAvailableVersion(); - } - if (!truth) - { - truth = m_trackingPackageVersion; - } - if (!truth) - { - truth = GetInstalledVersion(); - } - - switch (property) - { - case PackageProperty::Id: - return truth->GetProperty(PackageVersionProperty::Id); - case PackageProperty::Name: - return truth->GetProperty(PackageVersionProperty::Name); - default: - THROW_HR(E_UNEXPECTED); - } -======= CompositeInstalledPackage(std::shared_ptr package, Source trackingSource, std::shared_ptr trackingPackageVersion, std::string overrideVersion = {}) : m_package(std::move(package)), m_trackingSource(std::move(trackingSource)), m_trackingPackageVersion(std::move(trackingPackageVersion)), m_overrideVersion(std::move(overrideVersion)) {} @@ -395,61 +353,16 @@ namespace AppInstaller::Repository Utility::LocIndString GetProperty(PackageProperty property) const override { return m_package->GetProperty(property); ->>>>>>> master } std::vector GetVersionKeys() const override { -<<<<<<< HEAD - if (!m_installedPackages.empty()) - { - const InstalledPinnablePackage& firstInstalled = m_installedPackages.front(); - - auto installedVersion = firstInstalled.Pinnable.GetInstalledVersion(); - if (installedVersion) - { - return std::make_shared(std::move(installedVersion), m_trackingSource, m_trackingPackageVersion, firstInstalled.OverrideVersion); - } - } - - return {}; - } - - std::vector GetAvailableVersionKeys() const override - { - std::vector result; - - auto processSinglePackage = [&](const std::shared_ptr& package) - { - auto versionKeys = package->GetAvailableVersionKeys(); - std::copy(versionKeys.begin(), versionKeys.end(), std::back_inserter(result)); - }; - - if (ExperimentalFeature::IsEnabled(ExperimentalFeature::Feature::SideBySide) && m_primaryAvailablePackage) - { - processSinglePackage(m_primaryAvailablePackage); - } - else - { - for (const auto& availablePackage : m_availablePackages) - { - processSinglePackage(availablePackage); - } - } - - // TODO: Remove all elements whose channel does not match the installed packages. - - // Put latest versions at the front; for versions available from multiple sources maintain the order they were added in - std::stable_sort(result.begin(), result.end()); - -======= auto result = m_package->GetVersionKeys(); THROW_HR_IF(E_UNEXPECTED, result.size() != 1); if (!m_overrideVersion.empty()) { result.front().Version = m_overrideVersion; } ->>>>>>> master return result; } @@ -460,31 +373,6 @@ namespace AppInstaller::Repository std::shared_ptr GetLatestVersion() const override { -<<<<<<< HEAD - if (ExperimentalFeature::IsEnabled(ExperimentalFeature::Feature::SideBySide) && m_primaryAvailablePackage) - { - auto latestAvailable = m_primaryAvailablePackage->GetLatestAvailableVersion(); - if (Utility::IsEmptyOrWhitespace(versionKey.SourceId) || versionKey.SourceId == latestAvailable->GetSource().GetIdentifier()) - { - return availablePackage->GetAvailableVersion(versionKey); - } - } - else - { - for (const auto& availablePackage : m_availablePackages) - { - if (!Utility::IsEmptyOrWhitespace(versionKey.SourceId)) - { - auto latestAvailable = availablePackage->GetLatestAvailableVersion(); - if (latestAvailable && versionKey.SourceId != latestAvailable->GetSource().GetIdentifier()) - { - continue; - } - } - - return availablePackage->GetAvailableVersion(versionKey); - } -======= return std::make_shared(m_package->GetLatestVersion(), m_trackingSource, m_trackingPackageVersion, m_overrideVersion); } @@ -494,7 +382,6 @@ namespace AppInstaller::Repository if (m_trackingSource) { return m_trackingSource; ->>>>>>> master } return m_package->GetSource(); @@ -504,49 +391,12 @@ namespace AppInstaller::Repository { const CompositeInstalledPackage* otherPackage = PackageCast(other); -<<<<<<< HEAD - if (!otherComposite || - m_installedPackages.size() != otherComposite->m_installedPackages.size() || - m_availablePackages.size() != otherComposite->m_availablePackages.size()) -======= if (otherPackage) ->>>>>>> master { return m_package->IsSame(otherPackage->m_package.get()); } -<<<<<<< HEAD - - for (size_t i = 0; i < m_installedPackages.size(); ++i) - { - if (!m_installedPackages[i].Pinnable.GetPackage()->IsSame(otherComposite->m_installedPackages[i].Pinnable.GetPackage().get())) - { - return false; - } - } - - for (const auto& availablePackage : m_availablePackages) - { - bool foundMatch = false; - for (const auto& otherAvailablePackage : otherComposite->m_availablePackages) - { - if (availablePackage->IsSame(otherAvailablePackage.get())) - { - foundMatch = true; - break; - } - } - - if (!foundMatch) - { - return false; - } - } - - return true; -======= return false; ->>>>>>> master } const void* CastTo(IPackageType type) const override @@ -559,6 +409,20 @@ namespace AppInstaller::Repository return nullptr; } + bool ContainsInstalledPackage(const IPackage* installedPackage) const + { + // SXS_TODO: m_packages of course + for (const auto& package : m_packages) + { + if (package->IsSame(installedPackage)) + { + return true; + } + } + + return false; + } + private: std::shared_ptr m_package; Source m_trackingSource; @@ -570,43 +434,44 @@ namespace AppInstaller::Repository struct CompositePackage : public ICompositePackage { // The availablePackage may only contain one available package within it, as it is expected to be the output of a search on a single source. - CompositePackage(const std::shared_ptr& installedPackage, const std::shared_ptr& availablePackage = {}) + CompositePackage(const std::shared_ptr& installedPackage, const std::shared_ptr& availablePackage = {}, bool setPrimary = false) { if (installedPackage) { - m_installedPackage = installedPackage->GetInstalled(); + m_installedPackage = std::make_shared(installedPackage); } - AddAvailablePackage(availablePackage); + AddAvailablePackage(availablePackage, setPrimary); } Utility::LocIndString GetProperty(PackageProperty property) const override { - std::shared_ptr truth; - if (m_defaultAvailablePackage) + IPackage* truth = nullptr; + if (m_primaryAvailablePackage) { - truth = m_defaultAvailablePackage; + truth = m_primaryAvailablePackage.get(); + } + if (!truth && !m_availablePackages.empty()) + { + truth = m_availablePackages[0].get(); } if (!truth) { - truth = m_trackingPackage; + truth = m_trackingPackage.get(); } if (!truth) { - truth = m_installedPackage; + truth = m_installedPackage.get(); } + THROW_HR_IF(E_UNEXPECTED, !truth); + return truth->GetProperty(property); } std::shared_ptr GetInstalled() override { - if (m_installedPackage) - { - return std::make_shared(m_installedPackage, m_trackingSource, m_trackingPackageVersion, m_overrideInstalledVersion); - } - - return {}; + return m_installedPackage; } std::vector> GetAvailable() override @@ -630,41 +495,24 @@ namespace AppInstaller::Repository return false; } - bool ContainsInstalledPackage(const IPackage* installedPackage) const + std::shared_ptr GetInstalledPackage() const { -<<<<<<< HEAD - for (const auto& installed : m_installedPackages) - { - if (installed->IsSame(installedPackage)) - { - return true; - } - } - - return false; -======= return m_installedPackage; ->>>>>>> master } - const std::shared_ptr& GetTrackingPackage() const + bool ContainsInstalledPackage(const IPackage* installedPackage) const { - return m_trackingPackage; + return m_installedPackage ? m_installedPackage->ContainsInstalledPackage(installedPackage) : false; } -<<<<<<< HEAD - void AddAvailablePackage(std::shared_ptr availablePackage, bool setPrimary = false) -======= - void AddAvailablePackage(const std::shared_ptr& availablePackage) ->>>>>>> master + void AddAvailablePackage(const std::shared_ptr& availablePackage, bool setPrimary = false) { // Disable primary if feature not enabled setPrimary = setPrimary && ExperimentalFeature::IsEnabled(ExperimentalFeature::Feature::SideBySide); if (availablePackage) { -<<<<<<< HEAD - m_availablePackages.emplace_back(std::move(availablePackage)); + m_availablePackages.emplace_back(OnlyAvailable(availablePackage)); if (setPrimary) { @@ -674,20 +522,8 @@ namespace AppInstaller::Repository // Set override for primary or with the first available version found if (setPrimary || m_availablePackages.size() == 1) { - TrySetOverrideInstalledVersion(m_availablePackages.back().GetPackage()); - } -======= - std::shared_ptr singlePackage = OnlyAvailable(availablePackage); - - if (!m_defaultAvailablePackage) - { - // Set override only with the first available version found - m_defaultAvailablePackage = singlePackage; - TrySetOverrideInstalledVersion(m_defaultAvailablePackage); + TrySetOverrideInstalledVersion(m_availablePackages.back()); } - - m_availablePackages.emplace_back(std::move(singlePackage)); ->>>>>>> master } } @@ -703,6 +539,16 @@ namespace AppInstaller::Repository m_trackingWriteTime = trackingWriteTime; } + const Source& GetTrackingSource() const + { + return m_trackingSource; + } + + const std::shared_ptr& GetTrackingPackage() const + { + return m_trackingPackage; + } + std::chrono::system_clock::time_point GetTrackingPackageWriteTime() const { return m_trackingWriteTime; @@ -713,34 +559,21 @@ namespace AppInstaller::Repository return m_primaryAvailablePackage; } - std::vector>& GetAvailablePackages() - { - return m_availablePackages; - } - + // SXS_TODO: Move to CompositeInstalled void FoldInstalledIn(const std::shared_ptr& other) { std::move(other->m_installedPackages.begin(), other->m_installedPackages.end(), std::back_inserter(m_installedPackages)); std::sort(m_installedPackages.begin(), m_installedPackages.end(), [](const InstalledPinnablePackage& a, const InstalledPinnablePackage& b) { return a.Version < b.Version; }); } - const Source& GetTrackingSource() const - { - return m_trackingSource; - } - private: + // SXS_TODO: Move to CompositeInstalled // Try to set a version that will override the version string from the installed package void TrySetOverrideInstalledVersion(const std::shared_ptr& availablePackage) { if (availablePackage) { -<<<<<<< HEAD for (InstalledPinnablePackage& package : m_installedPackages) -======= - auto installedVersion = m_installedPackage->GetLatestVersion(); - if (installedVersion) ->>>>>>> master { auto installedVersion = package.Pinnable.GetInstalledVersion(); if (installedVersion) @@ -755,11 +588,8 @@ namespace AppInstaller::Repository } } -<<<<<<< HEAD - std::vector> m_installedPackages; -======= - std::shared_ptr m_installedPackage; ->>>>>>> master + // SXS_TODO: Evaluate additional changes to installed package + std::shared_ptr m_installedPackage; Source m_trackingSource; std::shared_ptr m_trackingPackage; std::shared_ptr m_trackingPackageVersion; @@ -960,7 +790,7 @@ namespace AppInstaller::Repository } // Determines if the results contain the given installed package. - bool ContainsInstalledPackage(const IPackage* installedPackage) const + bool ContainsInstalledPackage(const IPackage* installedPackage) const { for (auto& match : Matches) { @@ -976,8 +806,6 @@ namespace AppInstaller::Repository // *Destructively* converts the result to the standard variant. SearchResult ConvertToSearchResult() { - AddPinInfoToCompositeSearchResult(); - SearchResult result; result.Matches.reserve(Matches.size()); @@ -1333,23 +1161,6 @@ namespace AppInstaller::Repository } } } - - // Adds all the pin information to the results from a search to a CompositeSource. - void AddPinInfoToCompositeSearchResult() - { - if (!Matches.empty()) - { - // Look up any pins for the packages found - auto pinningIndex = PinningIndex::OpenOrCreateDefault(); - if (pinningIndex) - { - for (auto& match : Matches) - { - match.Package->GetExistingPins(*pinningIndex); - } - } - } - } }; std::shared_ptr GetTrackedPackageFromAvailableSource(CompositeResult& result, const Source& source, const Utility::LocIndString& identifier) @@ -1549,12 +1360,8 @@ namespace AppInstaller::Repository auto availablePackage = GetTrackedPackageFromAvailableSource(result, trackedSource, trackingPackage->GetProperty(PackageProperty::Id)); if (availablePackage) { -<<<<<<< HEAD compositePackage->AddAvailablePackage(std::move(availablePackage), true); -======= - compositePackage->AddAvailablePackage(std::move(availablePackage)); foundAvailablePackageFromTracking = true; ->>>>>>> master } compositePackage->SetTracking(std::move(trackedSource), std::move(trackingPackage), std::move(trackingPackageVersion), trackingPackageTime); } @@ -1650,22 +1457,13 @@ namespace AppInstaller::Repository if (installedPackage && !result.ContainsInstalledPackage(installedPackage->GetInstalled().get())) { auto compositePackage = std::make_shared( -<<<<<<< HEAD - std::move(installedPackage), + installedPackage, GetTrackedPackageFromAvailableSource(result, source, match.Package->GetProperty(PackageProperty::Id)), true); auto [writeTime, trackingPackageVersion] = GetLatestTrackingWriteTimeAndPackageVersion(match.Package); - compositePackage->SetTracking(source, std::move(match.Package), std::move(trackingPackageVersion), writeTime); -======= - installedPackage, - GetTrackedPackageFromAvailableSource(result, source, match.Package->GetProperty(PackageProperty::Id))); - - auto [writeTime, trackingPackageVersion] = GetLatestTrackingWriteTimeAndPackageVersion(match.Package); - - compositePackage->SetTracking(source, OnlyAvailable(match.Package), std::move(trackingPackageVersion)); ->>>>>>> master + compositePackage->SetTracking(source, OnlyAvailable(match.Package), std::move(trackingPackageVersion), writeTime); result.Matches.emplace_back(std::move(compositePackage), match.MatchCriteria); } From 935bdc3c291958f94a689759be3d32ffe50e091b Mon Sep 17 00:00:00 2001 From: John McPherson Date: Tue, 27 Feb 2024 13:53:00 -0800 Subject: [PATCH 10/24] CS refactor adaptation in progress --- .../CompositeSource.cpp | 86 ++++++++----------- 1 file changed, 38 insertions(+), 48 deletions(-) diff --git a/src/AppInstallerRepositoryCore/CompositeSource.cpp b/src/AppInstallerRepositoryCore/CompositeSource.cpp index 6d09a5f1d5..6547793701 100644 --- a/src/AppInstallerRepositoryCore/CompositeSource.cpp +++ b/src/AppInstallerRepositoryCore/CompositeSource.cpp @@ -346,34 +346,31 @@ namespace AppInstaller::Repository { static constexpr IPackageType PackageType = IPackageType::CompositeInstalledPackage; - CompositeInstalledPackage(std::shared_ptr package, Source trackingSource, std::shared_ptr trackingPackageVersion, std::string overrideVersion = {}) : - m_package(std::move(package)), m_trackingSource(std::move(trackingSource)), m_trackingPackageVersion(std::move(trackingPackageVersion)), m_overrideVersion(std::move(overrideVersion)) + CompositeInstalledPackage(std::shared_ptr package, Source trackingSource, std::shared_ptr trackingPackage) : + m_packages({ std::move(package) }), m_trackingSource(std::move(trackingSource)), m_trackingPackage(std::move(trackingPackage)) {} Utility::LocIndString GetProperty(PackageProperty property) const override { - return m_package->GetProperty(property); + // SXS_TODO: convert m_packageWithHighestVersion to be version key data[0].package index + return m_packages[m_packageWithHighestVersion]->GetProperty(property); } std::vector GetVersionKeys() const override { - auto result = m_package->GetVersionKeys(); - THROW_HR_IF(E_UNEXPECTED, result.size() != 1); - if (!m_overrideVersion.empty()) - { - result.front().Version = m_overrideVersion; - } - return result; + // SXS_TODO: Create version key data, including version overrides, as new packages are added + // SXS_TODO: Override source with installed package ID } std::shared_ptr GetVersion(const PackageVersionKey& versionKey) const { + // SXS_TODO: Possibly with version selection functions, actual mapping return (GetVersionKeys().front().IsMatch(versionKey)) ? GetLatestVersion() : std::shared_ptr{}; } std::shared_ptr GetLatestVersion() const override { - return std::make_shared(m_package->GetLatestVersion(), m_trackingSource, m_trackingPackageVersion, m_overrideVersion); + return GetVersion({}); } Source GetSource() const override @@ -409,9 +406,33 @@ namespace AppInstaller::Repository return nullptr; } + void SetTracking( + Source trackingSource, + std::shared_ptr trackingPackage, + std::chrono::system_clock::time_point trackingWriteTime) + { + m_trackingSource = std::move(trackingSource); + m_trackingPackage = std::move(trackingPackage); + m_trackingWriteTime = trackingWriteTime; + } + + Source GetTrackingSource() const + { + return m_trackingSource; + } + + const std::shared_ptr& GetTrackingPackage() const + { + return m_trackingPackage; + } + + std::chrono::system_clock::time_point GetTrackingPackageWriteTime() const + { + return m_trackingWriteTime; + } + bool ContainsInstalledPackage(const IPackage* installedPackage) const { - // SXS_TODO: m_packages of course for (const auto& package : m_packages) { if (package->IsSame(installedPackage)) @@ -424,10 +445,11 @@ namespace AppInstaller::Repository } private: - std::shared_ptr m_package; + std::vector> m_packages; + size_t m_packageWithHighestVersion = 0; Source m_trackingSource; - std::shared_ptr m_trackingPackageVersion; - std::string m_overrideVersion; + std::shared_ptr m_trackingPackage; + std::chrono::system_clock::time_point m_trackingWriteTime = std::chrono::system_clock::time_point::min(); }; // An ICompositePackage for the CompositeSource. @@ -495,7 +517,7 @@ namespace AppInstaller::Repository return false; } - std::shared_ptr GetInstalledPackage() const + const std::shared_ptr& GetInstalledPackage() const { return m_installedPackage; } @@ -527,33 +549,6 @@ namespace AppInstaller::Repository } } - void SetTracking( - Source trackingSource, - std::shared_ptr trackingPackage, - std::shared_ptr trackingPackageVersion, - std::chrono::system_clock::time_point trackingWriteTime) - { - m_trackingSource = std::move(trackingSource); - m_trackingPackage = std::move(trackingPackage); - m_trackingPackageVersion = std::move(trackingPackageVersion); - m_trackingWriteTime = trackingWriteTime; - } - - const Source& GetTrackingSource() const - { - return m_trackingSource; - } - - const std::shared_ptr& GetTrackingPackage() const - { - return m_trackingPackage; - } - - std::chrono::system_clock::time_point GetTrackingPackageWriteTime() const - { - return m_trackingWriteTime; - } - std::shared_ptr& GetPrimaryAvailablePackage() { return m_primaryAvailablePackage; @@ -588,12 +583,7 @@ namespace AppInstaller::Repository } } - // SXS_TODO: Evaluate additional changes to installed package std::shared_ptr m_installedPackage; - Source m_trackingSource; - std::shared_ptr m_trackingPackage; - std::shared_ptr m_trackingPackageVersion; - std::chrono::system_clock::time_point m_trackingWriteTime = std::chrono::system_clock::time_point::min(); std::shared_ptr m_primaryAvailablePackage; std::vector> m_availablePackages; }; From d8e468c50e77435ba0160a4052760e1b346aa0d9 Mon Sep 17 00:00:00 2001 From: John McPherson Date: Wed, 28 Feb 2024 11:48:41 -0800 Subject: [PATCH 11/24] installed folding working again --- src/AppInstallerCLICore/Workflows/PinFlow.cpp | 22 +- .../CompositeSource.cpp | 231 ++++++++++++++---- .../Public/winget/RepositorySearch.h | 2 +- 3 files changed, 205 insertions(+), 50 deletions(-) diff --git a/src/AppInstallerCLICore/Workflows/PinFlow.cpp b/src/AppInstallerCLICore/Workflows/PinFlow.cpp index cc0f28ec2a..ace205fc44 100644 --- a/src/AppInstallerCLICore/Workflows/PinFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/PinFlow.cpp @@ -31,6 +31,26 @@ namespace AppInstaller::CLI::Workflow } } + void GetPinKeysForInstalled(const std::shared_ptr& installedVersion, std::set& pinKeys) + { + auto installedType = Manifest::ConvertToInstallerTypeEnum(installedVersion->GetMetadata()[PackageVersionMetadata::InstalledType]); + std::vector propertyStrings; + + if (Manifest::DoesInstallerTypeUsePackageFamilyName(installedType)) + { + propertyStrings = installedVersion->GetMultiProperty(PackageVersionMultiProperty::PackageFamilyName); + } + else if (Manifest::DoesInstallerTypeUseProductCode(installedType)) + { + propertyStrings = installedVersion->GetMultiProperty(PackageVersionMultiProperty::ProductCode); + } + + for (const auto& value : propertyStrings) + { + pinKeys.emplace(Pinning::PinKey::GetPinKeyForInstalled(value)); + } + } + std::set GetPinKeysForPackage(Execution::Context& context) { auto package = context.Get(); @@ -42,7 +62,7 @@ namespace AppInstaller::CLI::Workflow auto installedVersion = GetInstalledVersion(package); if (installedVersion) { - pinKeys.emplace(Pinning::PinKey::GetPinKeyForInstalled(installedVersion)); + GetPinKeysForInstalled(installedVersion, pinKeys); } } else diff --git a/src/AppInstallerRepositoryCore/CompositeSource.cpp b/src/AppInstallerRepositoryCore/CompositeSource.cpp index 6547793701..362ed77afa 100644 --- a/src/AppInstallerRepositoryCore/CompositeSource.cpp +++ b/src/AppInstallerRepositoryCore/CompositeSource.cpp @@ -346,26 +346,54 @@ namespace AppInstaller::Repository { static constexpr IPackageType PackageType = IPackageType::CompositeInstalledPackage; - CompositeInstalledPackage(std::shared_ptr package, Source trackingSource, std::shared_ptr trackingPackage) : - m_packages({ std::move(package) }), m_trackingSource(std::move(trackingSource)), m_trackingPackage(std::move(trackingPackage)) - {} + CompositeInstalledPackage(std::shared_ptr package) + { + AddPackageAndVersionKeyData(std::move(package)); + } Utility::LocIndString GetProperty(PackageProperty property) const override { - // SXS_TODO: convert m_packageWithHighestVersion to be version key data[0].package index - return m_packages[m_packageWithHighestVersion]->GetProperty(property); + // Use the highest version for package properties + return m_packages[m_versionKeyData[0].PackageIndex]->GetProperty(property); } std::vector GetVersionKeys() const override { - // SXS_TODO: Create version key data, including version overrides, as new packages are added - // SXS_TODO: Override source with installed package ID + return { m_versionKeyData.begin(), m_versionKeyData.end() }; } - std::shared_ptr GetVersion(const PackageVersionKey& versionKey) const + std::shared_ptr GetVersion(const PackageVersionKey& versionKey) const override { - // SXS_TODO: Possibly with version selection functions, actual mapping - return (GetVersionKeys().front().IsMatch(versionKey)) ? GetLatestVersion() : std::shared_ptr{}; + std::shared_ptr installedVersion; + std::string overrideVersion; + + for (const VersionKeyData& key : m_versionKeyData) + { + if (key.IsMatch(versionKey)) + { + installedVersion = key.InstalledVersion; + overrideVersion = key.Version; + } + } + + // Get the appropriate tracking version or latest if it is not found. + // The tracking package uses the mapped version. + std::shared_ptr trackingPackageVersion; + if (m_trackingPackage) + { + // Remove our use of the package id as source + PackageVersionKey versionKey_NoSource = versionKey; + versionKey_NoSource.SourceId.clear(); + + trackingPackageVersion = m_trackingPackage->GetVersion(versionKey_NoSource); + + if (!trackingPackageVersion) + { + trackingPackageVersion = m_trackingPackage->GetLatestVersion(); + } + } + + return installedVersion ? std::make_shared(std::move(installedVersion), m_trackingSource, std::move(trackingPackageVersion), std::move(overrideVersion)) : nullptr; } std::shared_ptr GetLatestVersion() const override @@ -376,12 +404,8 @@ namespace AppInstaller::Repository Source GetSource() const override { // If there is a tracking source, use it instead to indicate that it came from there. - if (m_trackingSource) - { - return m_trackingSource; - } - - return m_package->GetSource(); + // Otherwise, all of the installed packages should be from the same source. + return m_trackingSource ? m_trackingSource : m_packages[0]->GetSource(); } bool IsSame(const IPackage* other) const override @@ -390,7 +414,31 @@ namespace AppInstaller::Repository if (otherPackage) { - return m_package->IsSame(otherPackage->m_package.get()); + if (m_packages.size() != otherPackage->m_packages.size()) + { + return false; + } + + for (const auto& subPackage : m_packages) + { + bool foundSame = false; + + for (const auto& otherSubPackage : otherPackage->m_packages) + { + if (subPackage->IsSame(otherSubPackage.get())) + { + foundSame = true; + break; + } + } + + if (!foundSame) + { + return false; + } + } + + return true; } return false; @@ -444,9 +492,75 @@ namespace AppInstaller::Repository return false; } + void FoldInstalledIn(const std::shared_ptr& other) + { + for (const auto& package : other->m_packages) + { + AddPackageAndVersionKeyData(package); + } + } + + // Set a version that will override the version string from the installed package + void SetOverrideInstalledVersion(const std::shared_ptr& availablePackage) + { + if (availablePackage) + { + for (auto& key : m_versionKeyData) + { + auto installedType = Manifest::ConvertToInstallerTypeEnum(key.InstalledVersion->GetMetadata()[PackageVersionMetadata::InstalledType]); + if (Manifest::DoesInstallerTypeSupportArpVersionRange(installedType)) + { + key.Version = GetMappedInstalledVersion(key.InstalledVersion->GetProperty(PackageVersionProperty::Version), availablePackage); + } + } + } + } + private: + // Contains information about all of the version keys. + // We use the `SourceId` field to store the installed package identifier so that we can disambiguate keys is they have the same version. + struct VersionKeyData : public PackageVersionKey + { + size_t PackageIndex; + std::shared_ptr InstalledVersion; + Utility::VersionAndChannel VersionAndChannel; + + bool operator<(const VersionKeyData& other) const + { + return VersionAndChannel < other.VersionAndChannel; + } + }; + + // Adds the package and version key data to the composite. + // The version keys are then sorted so that the first (index 0) in the vector has the highest version. + // Note that it may tied for highest version if, for instance, the same version is installed for different architectures. + void AddPackageAndVersionKeyData(std::shared_ptr package) + { + size_t packageIndex = m_packages.size(); + std::string packageIdentifier = package->GetProperty(PackageProperty::Id); + + for (const auto& versionKey : package->GetVersionKeys()) + { + VersionKeyData keyData{ versionKey }; + + keyData.PackageIndex = packageIndex; + keyData.InstalledVersion = package->GetVersion(versionKey); + + // We use the `SourceId` field to store the installed package identifier so that we can disambiguate keys is they have the same version. + keyData.SourceId = packageIdentifier; + + keyData.VersionAndChannel = Utility::VersionAndChannel{ keyData.Version, keyData.Channel }; + + m_versionKeyData.emplace_back(std::move(keyData)); + } + + m_packages.emplace_back(std::move(package)); + + std::sort(m_versionKeyData.begin(), m_versionKeyData.end()); + } + std::vector> m_packages; - size_t m_packageWithHighestVersion = 0; + std::vector m_versionKeyData; Source m_trackingSource; std::shared_ptr m_trackingPackage; std::chrono::system_clock::time_point m_trackingWriteTime = std::chrono::system_clock::time_point::min(); @@ -460,7 +574,7 @@ namespace AppInstaller::Repository { if (installedPackage) { - m_installedPackage = std::make_shared(installedPackage); + m_installedPackage = std::make_shared(installedPackage->GetInstalled()); } AddAvailablePackage(availablePackage, setPrimary); @@ -478,10 +592,6 @@ namespace AppInstaller::Repository truth = m_availablePackages[0].get(); } if (!truth) - { - truth = m_trackingPackage.get(); - } - if (!truth) { truth = m_installedPackage.get(); } @@ -501,6 +611,11 @@ namespace AppInstaller::Repository return m_availablePackages; } + const std::vector>& GetAvailablePackages() + { + return m_availablePackages; + } + bool IsSameAsAnyAvailable(const IPackage* other) const { if (other) @@ -554,32 +669,54 @@ namespace AppInstaller::Repository return m_primaryAvailablePackage; } - // SXS_TODO: Move to CompositeInstalled + Source GetTrackingSource() const + { + return m_installedPackage ? m_installedPackage->GetTrackingSource() : Source{}; + } + + std::shared_ptr GetTrackingPackage() const + { + return m_installedPackage ? m_installedPackage->GetTrackingPackage() : std::shared_ptr{}; + } + + std::chrono::system_clock::time_point GetTrackingPackageWriteTime() const + { + return m_installedPackage ? m_installedPackage->GetTrackingPackageWriteTime() : std::chrono::system_clock::time_point::min(); + } + + void SetTracking( + Source trackingSource, + std::shared_ptr trackingPackage, + std::chrono::system_clock::time_point trackingWriteTime) + { + if (m_installedPackage) + { + m_installedPackage->SetTracking(std::move(trackingSource), std::move(trackingPackage), trackingWriteTime); + } + } + void FoldInstalledIn(const std::shared_ptr& other) { - std::move(other->m_installedPackages.begin(), other->m_installedPackages.end(), std::back_inserter(m_installedPackages)); - std::sort(m_installedPackages.begin(), m_installedPackages.end(), [](const InstalledPinnablePackage& a, const InstalledPinnablePackage& b) { return a.Version < b.Version; }); + if (other->m_installedPackage) + { + if (m_installedPackage) + { + m_installedPackage->FoldInstalledIn(other->m_installedPackage); + } + else + { + m_installedPackage = other->m_installedPackage; + } + } } private: - // SXS_TODO: Move to CompositeInstalled // Try to set a version that will override the version string from the installed package void TrySetOverrideInstalledVersion(const std::shared_ptr& availablePackage) { - if (availablePackage) + if (m_installedPackage && availablePackage) { - for (InstalledPinnablePackage& package : m_installedPackages) - { - auto installedVersion = package.Pinnable.GetInstalledVersion(); - if (installedVersion) - { - auto installedType = Manifest::ConvertToInstallerTypeEnum(installedVersion->GetMetadata()[PackageVersionMetadata::InstalledType]); - if (Manifest::DoesInstallerTypeSupportArpVersionRange(installedType)) - { - package.OverrideVersion = GetMappedInstalledVersion(installedVersion->GetProperty(PackageVersionProperty::Version), availablePackage); - } - } - } + m_installedPackage->SetOverrideInstalledVersion(availablePackage); } } @@ -886,7 +1023,7 @@ namespace AppInstaller::Repository InstalledResultFoldKey(const std::shared_ptr& package) { - std::shared_ptr latestAvailable = package->GetLatestAvailableVersion(PinBehavior::IgnorePins); + std::shared_ptr latestAvailable = package->GetLatestVersion(); if (latestAvailable) { SourceIdentifier = latestAvailable->GetSource().GetIdentifier(); @@ -937,7 +1074,7 @@ namespace AppInstaller::Repository // Check current match for fold target if (currentMatch.Package->GetPrimaryAvailablePackage()) { - InstalledResultFoldKey key{ currentMatch.Package->GetPrimaryAvailablePackage()->GetPackage() }; + InstalledResultFoldKey key{ currentMatch.Package->GetPrimaryAvailablePackage() }; auto itr = foldData.find(key); if (itr != foldData.end()) @@ -961,7 +1098,7 @@ namespace AppInstaller::Repository { for (const auto& availablePackage : currentMatch.Package->GetAvailablePackages()) { - InstalledResultFoldKey key{ availablePackage.GetPackage() }; + InstalledResultFoldKey key{ availablePackage }; auto itr = foldData.find(key); if (itr == foldData.end()) @@ -993,7 +1130,7 @@ namespace AppInstaller::Repository for (const auto& availablePackage : currentMatch.Package->GetAvailablePackages()) { - auto& packageFoldData = foldData.at(availablePackage.GetPackage()); + auto& packageFoldData = foldData.at(availablePackage); if (packageFoldData.PrimaryPackageIndex) { @@ -1308,7 +1445,6 @@ namespace AppInstaller::Repository Source trackedSource; std::shared_ptr trackingPackage; - std::shared_ptr trackingPackageVersion; std::chrono::system_clock::time_point trackingPackageTime; bool foundAvailablePackageFromTracking = false; @@ -1338,7 +1474,6 @@ namespace AppInstaller::Repository { trackedSource = source; trackingPackage = OnlyAvailable(candidatePackage); - trackingPackageVersion = std::move(candidateVersion); trackingPackageTime = candidateTime; } } @@ -1353,7 +1488,7 @@ namespace AppInstaller::Repository compositePackage->AddAvailablePackage(std::move(availablePackage), true); foundAvailablePackageFromTracking = true; } - compositePackage->SetTracking(std::move(trackedSource), std::move(trackingPackage), std::move(trackingPackageVersion), trackingPackageTime); + compositePackage->SetTracking(std::move(trackedSource), std::move(trackingPackage), trackingPackageTime); } // Search sources and add to result @@ -1453,7 +1588,7 @@ namespace AppInstaller::Repository auto [writeTime, trackingPackageVersion] = GetLatestTrackingWriteTimeAndPackageVersion(match.Package); - compositePackage->SetTracking(source, OnlyAvailable(match.Package), std::move(trackingPackageVersion), writeTime); + compositePackage->SetTracking(source, OnlyAvailable(match.Package), writeTime); result.Matches.emplace_back(std::move(compositePackage), match.MatchCriteria); } diff --git a/src/AppInstallerRepositoryCore/Public/winget/RepositorySearch.h b/src/AppInstallerRepositoryCore/Public/winget/RepositorySearch.h index 1ad88150ad..36bf5774ee 100644 --- a/src/AppInstallerRepositoryCore/Public/winget/RepositorySearch.h +++ b/src/AppInstallerRepositoryCore/Public/winget/RepositorySearch.h @@ -242,7 +242,7 @@ namespace AppInstaller::Repository { PackageVersionKey() = default; - PackageVersionKey(Utility::NormalizedString sourceId, Utility::NormalizedString version, Utility::NormalizedString channel) : + PackageVersionKey(std::string sourceId, Utility::NormalizedString version, Utility::NormalizedString channel) : SourceId(std::move(sourceId)), Version(std::move(version)), Channel(std::move(channel)) {} // The source id that this version came from. From 142556b672223a0c11f892a9e3ef270264e3689b Mon Sep 17 00:00:00 2001 From: John McPherson Date: Wed, 28 Feb 2024 15:54:41 -0800 Subject: [PATCH 12/24] list shows all installed versions --- .../Workflows/WorkflowBase.cpp | 10 +- .../CompositeSource.cpp | 13 +- tools/DevInSandbox/InSandboxScript.ps1 | 31 +++ .../Launch-DevPackageInSandbox.ps1 | 216 ++++++++++++++++++ 4 files changed, 266 insertions(+), 4 deletions(-) create mode 100644 tools/DevInSandbox/InSandboxScript.ps1 create mode 100644 tools/DevInSandbox/Launch-DevPackageInSandbox.ps1 diff --git a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp index b0045126af..5565ec1d96 100644 --- a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp +++ b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp @@ -800,10 +800,16 @@ namespace AppInstaller::CLI::Workflow for (const auto& match : searchResult.Matches) { - auto installedVersion = GetInstalledVersion(match.Package); + auto installedPackage = match.Package->GetInstalled(); + if (!installedPackage) + { + continue; + } - if (installedVersion) + for (const auto& installedVersionKey : installedPackage->GetVersionKeys()) { + auto installedVersion = installedPackage->GetVersion(installedVersionKey); + auto evaluator = pinningData.CreatePinStateEvaluator(pinBehavior, installedVersion); auto availableVersions = GetAvailableVersionsForInstalledVersion(match.Package, installedVersion); diff --git a/src/AppInstallerRepositoryCore/CompositeSource.cpp b/src/AppInstallerRepositoryCore/CompositeSource.cpp index 362ed77afa..b7f0c6185f 100644 --- a/src/AppInstallerRepositoryCore/CompositeSource.cpp +++ b/src/AppInstallerRepositoryCore/CompositeSource.cpp @@ -505,10 +505,11 @@ namespace AppInstaller::Repository { if (availablePackage) { + m_availablePackageVersionOverride = availablePackage; + for (auto& key : m_versionKeyData) { - auto installedType = Manifest::ConvertToInstallerTypeEnum(key.InstalledVersion->GetMetadata()[PackageVersionMetadata::InstalledType]); - if (Manifest::DoesInstallerTypeSupportArpVersionRange(installedType)) + if (Manifest::DoesInstallerTypeSupportArpVersionRange(key.InstalledType)) { key.Version = GetMappedInstalledVersion(key.InstalledVersion->GetProperty(PackageVersionProperty::Version), availablePackage); } @@ -523,6 +524,7 @@ namespace AppInstaller::Repository { size_t PackageIndex; std::shared_ptr InstalledVersion; + Manifest::InstallerTypeEnum InstalledType; Utility::VersionAndChannel VersionAndChannel; bool operator<(const VersionKeyData& other) const @@ -549,6 +551,12 @@ namespace AppInstaller::Repository // We use the `SourceId` field to store the installed package identifier so that we can disambiguate keys is they have the same version. keyData.SourceId = packageIdentifier; + keyData.InstalledType = Manifest::ConvertToInstallerTypeEnum(keyData.InstalledVersion->GetMetadata()[PackageVersionMetadata::InstalledType]); + if (m_availablePackageVersionOverride && Manifest::DoesInstallerTypeSupportArpVersionRange(keyData.InstalledType)) + { + keyData.Version = GetMappedInstalledVersion(keyData.InstalledVersion->GetProperty(PackageVersionProperty::Version), m_availablePackageVersionOverride); + } + keyData.VersionAndChannel = Utility::VersionAndChannel{ keyData.Version, keyData.Channel }; m_versionKeyData.emplace_back(std::move(keyData)); @@ -564,6 +572,7 @@ namespace AppInstaller::Repository Source m_trackingSource; std::shared_ptr m_trackingPackage; std::chrono::system_clock::time_point m_trackingWriteTime = std::chrono::system_clock::time_point::min(); + std::shared_ptr m_availablePackageVersionOverride; }; // An ICompositePackage for the CompositeSource. diff --git a/tools/DevInSandbox/InSandboxScript.ps1 b/tools/DevInSandbox/InSandboxScript.ps1 new file mode 100644 index 0000000000..8158343f60 --- /dev/null +++ b/tools/DevInSandbox/InSandboxScript.ps1 @@ -0,0 +1,31 @@ +Param( + [String[]] $DesktopAppInstallerDependencyPath +) + +$ProgressPreference = 'SilentlyContinue' + +$desktopPath = "C:\Users\WDAGUtilityAccount\Desktop" + +Write-Host @" +--> Installing WinGet + +"@ + +foreach($dependency in $DesktopAppInstallerDependencyPath) +{ + Write-Host @" + ----> Installing $dependency +"@ + Add-AppxPackage -Path $dependency +} + +Write-Host @" + ----> Enabling dev mode +"@ +reg add "HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Windows\CurrentVersion\AppModelUnlock" /t REG_DWORD /f /v "AllowDevelopmentWithoutDevLicense" /d "1" + +$devPackageManifestPath = Join-Path $desktopPath "DevPackage\AppxManifest.xml" +Write-Host @" + ----> Installing $devPackageManifestPath +"@ +Add-AppxPackage -Path $devPackageManifestPath -Register diff --git a/tools/DevInSandbox/Launch-DevPackageInSandbox.ps1 b/tools/DevInSandbox/Launch-DevPackageInSandbox.ps1 new file mode 100644 index 0000000000..f21a494791 --- /dev/null +++ b/tools/DevInSandbox/Launch-DevPackageInSandbox.ps1 @@ -0,0 +1,216 @@ +Param( + [Parameter(HelpMessage = "The directory where local dev build is located; only the release build works.")] + [String] $DevPackagePath +) + +$ErrorActionPreference = "Stop" + +# Validate that the local dev manifest exists + +if (-not $DevPackagePath) +{ + $DevPackagePath = Join-Path $PSScriptRoot "..\..\src\AppInstallerCLIPackage\bin\x64\Release\AppX" +} + +$DevPackagePath = [System.IO.Path]::GetFullPath($DevPackagePath) + +if ($DevPackagePath.ToLower().Contains("debug")) +{ + Write-Error -Category InvalidArgument -Message @" +The Debug dev package does not work for unknown reasons. +Use the Release build or figure out how to make debug work and fix the scripts. +"@ +} + +if (-not (Test-Path (Join-Path $DevPackagePath "AppxManifest.xml"))) +{ + Write-Error -Category InvalidArgument -Message @" +AppxManifest.xml does not exist in the path $DevPackagePath +Either build the local dev package, or provide the location using -DevPackagePath +"@ +} + +# Check if Windows Sandbox is enabled + +if (-Not (Get-Command 'WindowsSandbox' -ErrorAction SilentlyContinue)) +{ + Write-Error -Category NotInstalled -Message @' +Windows Sandbox does not seem to be available. Check the following URL for prerequisites and further details: +https://docs.microsoft.com/windows/security/threat-protection/windows-sandbox/windows-sandbox-overview + +You can run the following command in an elevated PowerShell for enabling Windows Sandbox: +$ Enable-WindowsOptionalFeature -Online -FeatureName 'Containers-DisposableClientVM' +'@ +} + +# Close Windows Sandbox + +function Close-WindowsSandbox { + $sandbox = Get-Process 'WindowsSandboxClient' -ErrorAction SilentlyContinue + if ($sandbox) + { + Write-Host '--> Closing Windows Sandbox' + + $sandboxServer = Get-Process 'WindowsSandbox' -ErrorAction SilentlyContinue + + $sandbox | Stop-Process + $sandbox | Wait-Process -Timeout 120 + + # Also wait for the server to close + if ($sandboxServer) + { + try + { + $sandboxServer | Wait-Process -Timeout 120 + } + catch [System.TimeoutException] + { + # Force stop the server if it does not automatically stop + $sandboxServer | Stop-Process + $sandboxServer | Wait-Process -Timeout 120 + } + + } + + Write-Host + } + Remove-Variable sandbox +} + +Close-WindowsSandbox + +# Initialize Temp Folder + +$tempFolderName = 'DevInSandboxStaging' +$tempFolder = Join-Path -Path ([System.IO.Path]::GetTempPath()) -ChildPath $tempFolderName + +New-Item $tempFolder -ItemType Directory -ErrorAction SilentlyContinue | Out-Null + +# Set dependencies + +$desktopInSandbox = 'C:\Users\WDAGUtilityAccount\Desktop' + +[Net.ServicePointManager]::SecurityProtocol = [Net.SecurityProtocolType]::Tls12 +$WebClient = New-Object System.Net.WebClient + +# Hide the progress bar of Invoke-WebRequest +$oldProgressPreference = $ProgressPreference +$ProgressPreference = 'SilentlyContinue' + +$ProgressPreference = $oldProgressPreference + +$vcLibsUwp = @{ + fileName = 'Microsoft.VCLibs.x64.14.00.Desktop.appx' + url = 'https://aka.ms/Microsoft.VCLibs.x64.14.00.Desktop.appx' + hash = '9BFDE6CFCC530EF073AB4BC9C4817575F63BE1251DD75AAA58CB89299697A569' + folderInLocal = Join-Path ${env:ProgramFiles(x86)} "Microsoft SDKs\Windows Kits\10\ExtensionSDKs\Microsoft.VCLibs.Desktop\14.0\Appx\Retail\x64" +} + +$dependencies = @($vcLibsUwp) + +# Clean temp directory + +Get-ChildItem $tempFolder -Recurse -Exclude $dependencies.fileName | Remove-Item -Force -Recurse + +# Download dependencies + +Write-Host '--> Checking dependencies' + +foreach ($dependency in $dependencies) +{ + $dependency.pathInSandbox = Join-Path -Path $desktopInSandbox -ChildPath (Join-Path -Path $tempFolderName -ChildPath $dependency.fileName) + + # First see if the file exists locally to copy instead of downloading + if ($dependency.folderInLocal -ne $null) + { + $dependencyFilePath = Join-Path -Path $dependency.folderInLocal -ChildPath $dependency.fileName + if (Test-Path -Path $dependencyFilePath -PathType Leaf) + { + $dependencyFilePath + $tempFolder + Copy-Item -Path $dependencyFilePath -Destination $tempFolder -Force + continue + } + } + + # File does not exist locally, we need to download + + $dependency.file = Join-Path -Path $tempFolder -ChildPath $dependency.fileName + + # Only download if the file does not exist, or its hash does not match. + if (-Not ((Test-Path -Path $dependency.file -PathType Leaf) -And $dependency.hash -eq $(Get-FileHash $dependency.file).Hash)) + { + Write-Host @" + - Downloading: + $($dependency.url) +"@ + + try + { + $WebClient.DownloadFile($dependency.url, $dependency.file) + } + catch + { + #Pass the exception as an inner exception + throw [System.Net.WebException]::new("Error downloading $($dependency.url).",$_.Exception) + } + if (-not ($dependency.hash -eq $(Get-FileHash $dependency.file).Hash)) + { + throw [System.Activities.VersionMismatchException]::new('Dependency hash does not match the downloaded file') + } + } +} + +# Copy main script + +$mainPs1FileName = 'InSandboxScript.ps1' +Copy-Item (Join-Path $PSScriptRoot $mainPs1FileName) (Join-Path $tempFolder $mainPs1FileName) + +$dependenciesPathsInSandbox = "@('$($vcLibsUwp.pathInSandbox)')" +$bootstrapPs1Content = ".\$mainPs1FileName -DesktopAppInstallerDependencyPath @($dependenciesPathsInSandbox)" + +$bootstrapPs1FileName = 'Bootstrap.ps1' +$bootstrapPs1Content | Out-File (Join-Path $tempFolder $bootstrapPs1FileName) -Force + +# Create Windows Sandbox configuration file + +$bootstrapPs1InSandbox = Join-Path -Path $desktopInSandbox -ChildPath (Join-Path -Path $tempFolderName -ChildPath $bootstrapPs1FileName) +$tempFolderInSandbox = Join-Path -Path $desktopInSandbox -ChildPath $tempFolderName + +$devPackageInSandbox = Join-Path -Path $desktopInSandbox -ChildPath "DevPackage" +$devPackageXMLFragment = "" + +$devPackageXMLFragment = @" + + $DevPackagePath + $devPackageInSandbox + +"@ + +$sandboxTestWsbContent = @" + + + + $tempFolder + true + + $devPackageXMLFragment + + + PowerShell Start-Process PowerShell -WindowStyle Maximized -WorkingDirectory '$tempFolderInSandbox' -ArgumentList '-ExecutionPolicy Bypass -NoExit -NoLogo -File $bootstrapPs1InSandbox' + + +"@ + +$sandboxTestWsbFileName = 'SandboxTest.wsb' +$sandboxTestWsbFile = Join-Path -Path $tempFolder -ChildPath $sandboxTestWsbFileName +$sandboxTestWsbContent | Out-File $sandboxTestWsbFile -Force + +Write-Host @" +--> Starting Windows Sandbox + $sandboxTestWsbFile +"@ + +Write-Host + +WindowsSandbox $SandboxTestWsbFile From 5ec99fae8e0cd9ee59786d4c9aee99f2843b1b54 Mon Sep 17 00:00:00 2001 From: John McPherson Date: Wed, 28 Feb 2024 17:29:57 -0800 Subject: [PATCH 13/24] spelling and a few comment thoughts --- .github/actions/spelling/excludes.txt | 1 + src/AppInstallerRepositoryCore/CompositeSource.cpp | 3 +++ 2 files changed, 4 insertions(+) diff --git a/.github/actions/spelling/excludes.txt b/.github/actions/spelling/excludes.txt index fbabf579ad..4bda655f7d 100644 --- a/.github/actions/spelling/excludes.txt +++ b/.github/actions/spelling/excludes.txt @@ -91,5 +91,6 @@ # Because it doesn't handle argument -Words well ^src/PowerShell/tests/ ^tools/CorrelationTestbed/.*\.ps1$ +^tools/DevInSandbox/.*\.ps1$ ^tools/COMTrace/ComTrace.wprp$ ignore$ diff --git a/src/AppInstallerRepositoryCore/CompositeSource.cpp b/src/AppInstallerRepositoryCore/CompositeSource.cpp index b7f0c6185f..d027fc55ed 100644 --- a/src/AppInstallerRepositoryCore/CompositeSource.cpp +++ b/src/AppInstallerRepositoryCore/CompositeSource.cpp @@ -1579,6 +1579,7 @@ namespace AppInstaller::Repository // Correlate against installed (allow exceptions out as we own the installed source) SearchResult installedCrossRef = m_installedSource.Search(systemReferenceSearch); + // SXS_TODO: If feature enabled, take all matches and filter out weak match results by doing an installed item correlation back to the source auto installedPackage = GetMatchingPackage(installedCrossRef.Matches, [&]() { AICLI_LOG(Repo, Info, @@ -1588,6 +1589,7 @@ namespace AppInstaller::Repository AICLI_LOG(Repo, Warning, << " Appropriate installed package could not be determined"); }); + // SXS_TODO: IFF none of the installed matches are present, create a new entry (maybe also allow weak matches to be thrown out this way too?) if (installedPackage && !result.ContainsInstalledPackage(installedPackage->GetInstalled().get())) { auto compositePackage = std::make_shared( @@ -1632,6 +1634,7 @@ namespace AppInstaller::Repository // Correlate against installed (allow exceptions out as we own the installed source) SearchResult installedCrossRef = m_installedSource.Search(systemReferenceSearch); + // SXS_TODO: Figure out how to handle first, then do tracking second auto installedPackage = GetMatchingPackage(installedCrossRef.Matches, [&]() { AICLI_LOG(Repo, Info, From 610e580d2aaffd2c513c073f2e951af236145ba6 Mon Sep 17 00:00:00 2001 From: John McPherson Date: Tue, 5 Mar 2024 14:03:04 -0800 Subject: [PATCH 14/24] Refactor installed search/tracking --- .../CompositeSource.cpp | 129 ++++++++---------- 1 file changed, 57 insertions(+), 72 deletions(-) diff --git a/src/AppInstallerRepositoryCore/CompositeSource.cpp b/src/AppInstallerRepositoryCore/CompositeSource.cpp index d027fc55ed..b2cf456e62 100644 --- a/src/AppInstallerRepositoryCore/CompositeSource.cpp +++ b/src/AppInstallerRepositoryCore/CompositeSource.cpp @@ -100,14 +100,13 @@ namespace AppInstaller::Repository } } - // For a given package from a tracking catalog, get the latest write time and its corresponding package version. + // For a given package from a tracking catalog, get the latest write time. // Look at all versions rather than just the latest to account for the potential of downgrading. - std::pair> GetLatestTrackingWriteTimeAndPackageVersion( + std::chrono::system_clock::time_point GetLatestTrackingWriteTime( const std::shared_ptr& trackingCompositePackage) { std::shared_ptr trackingPackage = OnlyAvailable(trackingCompositePackage); - std::chrono::system_clock::time_point resultTime{}; - std::shared_ptr resultVersion; + std::chrono::system_clock::time_point result{}; for (const auto& key : trackingPackage->GetVersionKeys()) { @@ -127,16 +126,15 @@ namespace AppInstaller::Repository std::chrono::system_clock::time_point versionTime = Utility::ConvertUnixEpochToSystemClock(unixEpoch); - if (versionTime > resultTime) + if (versionTime > result) { - resultTime = versionTime; - resultVersion = version; + result = versionTime; } } } } - return { resultTime, std::move(resultVersion) }; + return result; } // An installed package's version reported in ARP does not necessarily match the versions used for the manifest. @@ -653,11 +651,11 @@ namespace AppInstaller::Repository void AddAvailablePackage(const std::shared_ptr& availablePackage, bool setPrimary = false) { - // Disable primary if feature not enabled - setPrimary = setPrimary && ExperimentalFeature::IsEnabled(ExperimentalFeature::Feature::SideBySide); - if (availablePackage) { + // Disable primary if feature not enabled + setPrimary = setPrimary && ExperimentalFeature::IsEnabled(ExperimentalFeature::Feature::SideBySide); + m_availablePackages.emplace_back(OnlyAvailable(availablePackage)); if (setPrimary) @@ -942,6 +940,8 @@ namespace AppInstaller::Repository // *Destructively* converts the result to the standard variant. SearchResult ConvertToSearchResult() { + FoldResults(); + SearchResult result; result.Matches.reserve(Matches.size()); @@ -1018,7 +1018,7 @@ namespace AppInstaller::Repository // 2. Attempt correlation by installed data only // a. We can potentially detect multiple instances of the same installed item with the same correlation logic turned back on // the installed source. This would allow for folding even when the package is not in any available source. - void FoldInstalledResults() + void FoldResults() { if (!ExperimentalFeature::IsEnabled(ExperimentalFeature::Feature::SideBySide)) { @@ -1450,79 +1450,46 @@ namespace AppInstaller::Repository if (!installedPackageData.SystemReferenceStrings.empty()) { SearchRequest systemReferenceSearch = installedPackageData.CreateInclusionsSearchRequest(SearchPurpose::CorrelationToAvailable); - AICLI_LOG(Repo, Info, << "Finding available package from installed package using system reference search: " << systemReferenceSearch.ToString()); - - Source trackedSource; - std::shared_ptr trackingPackage; - std::chrono::system_clock::time_point trackingPackageTime; - bool foundAvailablePackageFromTracking = false; + AICLI_LOG(Repo, Verbose, << "Finding available package from installed package using system reference search: " << systemReferenceSearch.ToString()); - // Check the tracking catalog first to see if there is a correlation there. - // TODO: When the issue with support for multiple available packages is fixed, this should move into - // the below available sources loop as we will check all sources at that point. + // Search sources and add to result for (const auto& source : m_availableSources) { + AICLI_LOG(Repo, Verbose, << " ... searching source: " << source.GetDetails().Name << " [" << source.GetIdentifier() << ']'); + + // Find the tracking result with the latest timestamp. auto trackingCatalog = source.GetTrackingCatalog(); SearchResult trackingResult = trackingCatalog.Search(systemReferenceSearch); - std::shared_ptr candidatePackage = GetMatchingPackage(trackingResult.Matches, - [&]() { - AICLI_LOG(Repo, Info, - << "Found multiple matches for installed package [" << installedVersion->GetProperty(PackageVersionProperty::Id) << - "] in tracking catalog for source [" << source.GetIdentifier() << "] when searching for [" << systemReferenceSearch.ToString() << "]"); - }, [&] { - AICLI_LOG(Repo, Warning, << " Appropriate tracking package could not be determined"); - }); + std::shared_ptr trackingPackage; + std::chrono::system_clock::time_point trackingPackageTime; + bool trackingSet = false; - // Determine the candidate package with the latest install time - if (candidatePackage) + for (const auto& trackingMatch : trackingResult.Matches) { - auto [candidateTime, candidateVersion] = GetLatestTrackingWriteTimeAndPackageVersion(candidatePackage); + auto candidateTime = GetLatestTrackingWriteTime(trackingMatch.Package); if (!trackingPackage || candidateTime > trackingPackageTime) { - trackedSource = source; - trackingPackage = OnlyAvailable(candidatePackage); + trackingPackage = OnlyAvailable(trackingMatch.Package); trackingPackageTime = candidateTime; } } - } - // Directly search for the available package from tracking information. - if (trackingPackage) - { - auto availablePackage = GetTrackedPackageFromAvailableSource(result, trackedSource, trackingPackage->GetProperty(PackageProperty::Id)); - if (availablePackage) + if (trackingPackage && trackingPackageTime > compositePackage->GetTrackingPackageWriteTime()) { - compositePackage->AddAvailablePackage(std::move(availablePackage), true); - foundAvailablePackageFromTracking = true; + AICLI_LOG(Repo, Verbose, << " ... setting latest tracking package to: " << trackingPackage->GetProperty(PackageProperty::Id)); + compositePackage->SetTracking(source, trackingPackage, trackingPackageTime); + trackingSet = true; } - compositePackage->SetTracking(std::move(trackedSource), std::move(trackingPackage), trackingPackageTime); - } - // Search sources and add to result - for (const auto& source : m_availableSources) - { - // Do not attempt to correlate local packages against this source - if (!source.GetDetails().SupportInstalledSearchCorrelation) + // Attempt to correlate local packages against this source if supported. + SearchResult availableResult; + if (source.GetDetails().SupportInstalledSearchCorrelation) { - continue; + availableResult = result.SearchAndHandleFailures(source, systemReferenceSearch); } - // Skip the source that the tracking correlation result came from if we found one - if (foundAvailablePackageFromTracking && compositePackage->GetTrackingSource() == source) - { - continue; - } - - SearchResult availableResult = result.SearchAndHandleFailures(source, systemReferenceSearch); - - if (availableResult.Matches.empty()) - { - continue; - } - - // We will keep matching packages found from all sources, but generally we will use only the first one. auto availablePackage = GetMatchingPackage(availableResult.Matches, [&]() { AICLI_LOG(Repo, Info, @@ -1532,8 +1499,31 @@ namespace AppInstaller::Repository AICLI_LOG(Repo, Warning, << " Appropriate available package could not be determined"); }); - // For non pinning cases. We found some matching packages here, don't keep going. - compositePackage->AddAvailablePackage(availablePackage); + if (trackingPackage) + { + auto trackingIdentifier = trackingPackage->GetProperty(PackageProperty::Id); + + // We always want to take the available search result if it exists as the package may have been updated. + if (availablePackage) + { + auto availableIdentifier = availablePackage->GetProperty(PackageProperty::Id); + if (!Utility::ICUCaseInsensitiveEquals(availableIdentifier, trackingIdentifier)) + { + AICLI_LOG(Repo, Verbose, << " ... overriding tracking package (" << trackingIdentifier << ") with available package (" << availableIdentifier << ")"); + } + } + else + { + AICLI_LOG(Repo, Verbose, << " ... using tracking package: " << trackingIdentifier); + availablePackage = GetTrackedPackageFromAvailableSource(result, source, trackingIdentifier); + } + } + + if (availablePackage) + { + AICLI_LOG(Repo, Verbose, << " ... adding available package: " << availablePackage->GetProperty(PackageProperty::Id)); + compositePackage->AddAvailablePackage(availablePackage, trackingSet); + } } } @@ -1541,9 +1531,6 @@ namespace AppInstaller::Repository result.Matches.emplace_back(std::move(compositePackage), std::move(match.MatchCriteria)); } - // Group multiple instances of installed items into a single result item - result.FoldInstalledResults(); - // Optimization for the "everything installed" case, no need to allow for reverse correlations if (request.IsForEverything() && m_searchBehavior == CompositeSearchBehavior::Installed) { @@ -1597,9 +1584,7 @@ namespace AppInstaller::Repository GetTrackedPackageFromAvailableSource(result, source, match.Package->GetProperty(PackageProperty::Id)), true); - auto [writeTime, trackingPackageVersion] = GetLatestTrackingWriteTimeAndPackageVersion(match.Package); - - compositePackage->SetTracking(source, OnlyAvailable(match.Package), writeTime); + compositePackage->SetTracking(source, OnlyAvailable(match.Package), GetLatestTrackingWriteTime(match.Package)); result.Matches.emplace_back(std::move(compositePackage), match.MatchCriteria); } From 1b0d8f6b09852fc9382dc6e77f447a33ab1a5981 Mon Sep 17 00:00:00 2001 From: John McPherson Date: Wed, 6 Mar 2024 11:09:56 -0800 Subject: [PATCH 15/24] Available correlation complete --- .../CompositeSource.cpp | 422 ++++++++++-------- 1 file changed, 231 insertions(+), 191 deletions(-) diff --git a/src/AppInstallerRepositoryCore/CompositeSource.cpp b/src/AppInstallerRepositoryCore/CompositeSource.cpp index b2cf456e62..e468f77beb 100644 --- a/src/AppInstallerRepositoryCore/CompositeSource.cpp +++ b/src/AppInstallerRepositoryCore/CompositeSource.cpp @@ -103,9 +103,8 @@ namespace AppInstaller::Repository // For a given package from a tracking catalog, get the latest write time. // Look at all versions rather than just the latest to account for the potential of downgrading. std::chrono::system_clock::time_point GetLatestTrackingWriteTime( - const std::shared_ptr& trackingCompositePackage) + const std::shared_ptr& trackingPackage) { - std::shared_ptr trackingPackage = OnlyAvailable(trackingCompositePackage); std::chrono::system_clock::time_point result{}; for (const auto& key : trackingPackage->GetVersionKeys()) @@ -844,13 +843,122 @@ namespace AppInstaller::Repository result.Purpose = searchPurpose; return result; } + + std::shared_ptr AddSystemReferenceStringsFromTrackingPackage(const PackageTrackingCatalog& trackingCatalog, const Utility::LocIndString& identifier, std::string_view sourceIdentifier) + { + SearchRequest trackingRequest; + trackingRequest.Filters.emplace_back(PackageMatchField::Id, MatchType::CaseInsensitive, identifier.get()); + + SearchResult trackingResult = trackingCatalog.Search(trackingRequest); + + if (trackingResult.Matches.size() == 1) + { + std::shared_ptr result = OnlyAvailable(trackingResult.Matches[0].Package); + AddSystemReferenceStrings(result.get()); + return result; + } + else + { + AICLI_LOG(Repo, Warning, << "Found multiple results for Id [" << identifier << "] in tracking catalog for: " << sourceIdentifier); + return {}; + } + } + + void AddSystemReferenceStrings(IPackage* package) + { + for (auto const& versionKey : package->GetVersionKeys()) + { + auto version = package->GetVersion(versionKey); + AddSystemReferenceStrings(version.get()); + } + } + + void AddSystemReferenceStrings(IPackageVersion* version) + { + GetSystemReferenceStrings( + version, + PackageVersionMultiProperty::PackageFamilyName, + PackageMatchField::PackageFamilyName); + + GetSystemReferenceStrings( + version, + PackageVersionMultiProperty::ProductCode, + PackageMatchField::ProductCode); + + GetSystemReferenceStrings( + version, + PackageVersionMultiProperty::UpgradeCode, + PackageMatchField::UpgradeCode); + + GetNameAndPublisher( + version); + } + + void AddSystemReferenceStringsFromManifest(const Manifest::Manifest& manifest) + { + for (const auto& pfn : manifest.GetPackageFamilyNames()) + { + AddIfNotPresent(SystemReferenceString{ PackageMatchField::PackageFamilyName, Utility::LocIndString{ pfn } }); + } + for (const auto& productCode : manifest.GetProductCodes()) + { + AddIfNotPresent(SystemReferenceString{ PackageMatchField::ProductCode, Utility::LocIndString{ productCode } }); + } + for (const auto& upgradeCode : manifest.GetUpgradeCodes()) + { + AddIfNotPresent(SystemReferenceString{ PackageMatchField::UpgradeCode, Utility::LocIndString{ upgradeCode } }); + } + for (const auto& name : manifest.GetPackageNames()) + { + for (const auto& publisher : manifest.GetPublishers()) + { + AddIfNotPresent(SystemReferenceString{ + PackageMatchField::NormalizedNameAndPublisher, + Utility::LocIndString{ name }, + Utility::LocIndString{ publisher } }); + } + } + } + + private: + void GetSystemReferenceStrings( + IPackageVersion* installedVersion, + PackageVersionMultiProperty prop, + PackageMatchField field) + { + for (auto&& string : installedVersion->GetMultiProperty(prop)) + { + AddIfNotPresent(SystemReferenceString{ field, std::move(string) }); + } + } + + void GetNameAndPublisher( + IPackageVersion* installedVersion) + { + // Unfortunately the names and publishers are unique and not tied to each other strictly, so we need + // to go broad on the matches. Future work can hopefully make name and publisher operate more as a unit, + // but for now we have to search for the cartesian of these... + auto names = installedVersion->GetMultiProperty(PackageVersionMultiProperty::Name); + auto publishers = installedVersion->GetMultiProperty(PackageVersionMultiProperty::Publisher); + + for (size_t i = 0; i < names.size(); ++i) + { + for (size_t j = 0; j < publishers.size(); ++j) + { + AddIfNotPresent(SystemReferenceString{ + PackageMatchField::NormalizedNameAndPublisher, + names[i], + publishers[j] }); + } + } + } }; - // For a given package version, prepares the results for it. - PackageData GetSystemReferenceStrings(IPackageVersion* version) + // For a given package, prepares the results for it. + PackageData GetSystemReferenceStrings(IPackage* package) { PackageData result; - AddSystemReferenceStrings(version, result); + result.AddSystemReferenceStrings(package); return result; } @@ -881,60 +989,44 @@ namespace AppInstaller::Repository for (auto const& versionKey : availablePackage->GetVersionKeys()) { auto packageVersion = availablePackage->GetVersion(versionKey); - AddSystemReferenceStrings(packageVersion.get(), result); + result.AddSystemReferenceStrings(packageVersion.get()); if (downloadManifests && manifestsDownloaded < c_downloadManifestsLimit) { auto manifest = packageVersion->GetManifest(); - AddSystemReferenceStringsFromManifest(manifest, result); + result.AddSystemReferenceStringsFromManifest(manifest); manifestsDownloaded++; } } return result; } - // Check for a package already in the result that should have been correlated already. - // If we find one, see if we should upgrade it's match criteria. - // If we don't, return package data for further use. - std::optional CheckForExistingResultFromTrackingPackageMatch(const ResultMatch& trackingMatch) + // Determines if the results contain the given installed package. + bool ContainsInstalledPackage(const IPackage* installedPackage) const { - std::shared_ptr trackingMatchPackage = OnlyAvailable(trackingMatch.Package); - for (auto& match : Matches) { - const std::shared_ptr& trackingPackage = match.Package->GetTrackingPackage(); - if (trackingPackage && trackingPackage->IsSame(trackingMatchPackage.get())) + if (match.Package->ContainsInstalledPackage(installedPackage)) { - if (ResultMatchComparator{}(trackingMatch, match)) - { - match.MatchCriteria = trackingMatch.MatchCriteria; - } - - return {}; + return true; } } - PackageData result; - for (auto const& versionKey : trackingMatchPackage->GetVersionKeys()) - { - auto packageVersion = trackingMatchPackage->GetVersion(versionKey); - AddSystemReferenceStrings(packageVersion.get(), result); - } - return result; + return false; } // Determines if the results contain the given installed package. - bool ContainsInstalledPackage(const IPackage* installedPackage) const + std::shared_ptr FindInstalledPackage(const IPackage* installedPackage) const { for (auto& match : Matches) { if (match.Package->ContainsInstalledPackage(installedPackage)) { - return true; + return match.Package; } } - return false; + return {}; } // *Destructively* converts the result to the standard variant. @@ -1211,92 +1303,6 @@ namespace AppInstaller::Repository std::vector Matches; bool Truncated = false; std::vector Failures; - - private: - void AddSystemReferenceStrings(IPackageVersion* version, PackageData& data) - { - GetSystemReferenceStrings( - version, - PackageVersionMultiProperty::PackageFamilyName, - PackageMatchField::PackageFamilyName, - data); - - GetSystemReferenceStrings( - version, - PackageVersionMultiProperty::ProductCode, - PackageMatchField::ProductCode, - data); - - GetSystemReferenceStrings( - version, - PackageVersionMultiProperty::UpgradeCode, - PackageMatchField::UpgradeCode, - data); - - GetNameAndPublisher( - version, - data); - } - - void AddSystemReferenceStringsFromManifest(const Manifest::Manifest& manifest, PackageData& data) - { - for (const auto& pfn : manifest.GetPackageFamilyNames()) - { - data.AddIfNotPresent(SystemReferenceString{ PackageMatchField::PackageFamilyName, Utility::LocIndString{ pfn } }); - } - for (const auto& productCode : manifest.GetProductCodes()) - { - data.AddIfNotPresent(SystemReferenceString{ PackageMatchField::ProductCode, Utility::LocIndString{ productCode } }); - } - for (const auto& upgradeCode : manifest.GetUpgradeCodes()) - { - data.AddIfNotPresent(SystemReferenceString{ PackageMatchField::UpgradeCode, Utility::LocIndString{ upgradeCode } }); - } - for (const auto& name : manifest.GetPackageNames()) - { - for (const auto& publisher : manifest.GetPublishers()) - { - data.AddIfNotPresent(SystemReferenceString{ - PackageMatchField::NormalizedNameAndPublisher, - Utility::LocIndString{ name }, - Utility::LocIndString{ publisher } }); - } - } - } - - void GetSystemReferenceStrings( - IPackageVersion* installedVersion, - PackageVersionMultiProperty prop, - PackageMatchField field, - PackageData& data) - { - for (auto&& string : installedVersion->GetMultiProperty(prop)) - { - data.AddIfNotPresent(SystemReferenceString{ field, std::move(string) }); - } - } - - void GetNameAndPublisher( - IPackageVersion* installedVersion, - PackageData& data) - { - // Unfortunately the names and publishers are unique and not tied to each other strictly, so we need - // to go broad on the matches. Future work can hopefully make name and publisher operate more as a unit, - // but for now we have to search for the cartesian of these... - auto names = installedVersion->GetMultiProperty(PackageVersionMultiProperty::Name); - auto publishers = installedVersion->GetMultiProperty(PackageVersionMultiProperty::Publisher); - - for (size_t i = 0; i < names.size(); ++i) - { - for (size_t j = 0; j < publishers.size(); ++j) - { - data.AddIfNotPresent(SystemReferenceString{ - PackageMatchField::NormalizedNameAndPublisher, - names[i], - publishers[j] }); - } - } - } }; std::shared_ptr GetTrackedPackageFromAvailableSource(CompositeResult& result, const Source& source, const Utility::LocIndString& identifier) @@ -1426,17 +1432,11 @@ namespace AppInstaller::Repository } std::shared_ptr compositePackage = std::make_shared(match.Package); - std::shared_ptr installedVersion; - auto installedPackage = compositePackage->GetInstalled(); - if (installedPackage) - { - installedVersion = installedPackage->GetLatestVersion(); - } - if (!installedVersion) + if (!installedPackage) { - // One would think that the installed version coming directly from our own installed source + // One would think that the installed package coming directly from our own installed source // would never be null, but it is sometimes. Rather than making users suffer through crashes // that break their entire experience, lets log a few things and then ignore this match. AICLI_LOG(Repo, Warning, << "CompositeSource: The installed version of the package '" << @@ -1444,7 +1444,7 @@ namespace AppInstaller::Repository continue; } - auto installedPackageData = result.GetSystemReferenceStrings(installedVersion.get()); + auto installedPackageData = result.GetSystemReferenceStrings(installedPackage.get()); // Create a search request to run against all available sources if (!installedPackageData.SystemReferenceStrings.empty()) @@ -1467,7 +1467,7 @@ namespace AppInstaller::Repository for (const auto& trackingMatch : trackingResult.Matches) { - auto candidateTime = GetLatestTrackingWriteTime(trackingMatch.Package); + auto candidateTime = GetLatestTrackingWriteTime(OnlyAvailable(trackingMatch.Package)); if (!trackingPackage || candidateTime > trackingPackageTime) { @@ -1493,7 +1493,7 @@ namespace AppInstaller::Repository auto availablePackage = GetMatchingPackage(availableResult.Matches, [&]() { AICLI_LOG(Repo, Info, - << "Found multiple matches for installed package [" << installedVersion->GetProperty(PackageVersionProperty::Id) << + << "Found multiple matches for installed package [" << installedPackage->GetProperty(PackageProperty::Id) << "] in source [" << source.GetIdentifier() << "] when searching for [" << systemReferenceSearch.ToString() << "]"); }, [&] { AICLI_LOG(Repo, Warning, << " Appropriate available package could not be determined"); @@ -1541,62 +1541,14 @@ namespace AppInstaller::Repository // Search available sources for (const auto& source : m_availableSources) { - // Search the tracking catalog as it can potentially get better correlations auto trackingCatalog = source.GetTrackingCatalog(); - SearchResult trackingResult = trackingCatalog.Search(request); - - for (auto&& match : trackingResult.Matches) - { - // Check for a package already in the result that should have been correlated already. - auto packageData = result.CheckForExistingResultFromTrackingPackageMatch(match); - - // If found existing package in the result, continue - if (!packageData) - { - continue; - } - - // If no package was found that was already in the results, do a correlation lookup with the installed - // source to create a new composite package entry if we find any packages there. - if (packageData && !packageData->SystemReferenceStrings.empty()) - { - SearchRequest systemReferenceSearch = packageData->CreateInclusionsSearchRequest(SearchPurpose::CorrelationToInstalled); - - AICLI_LOG(Repo, Info, << "Finding installed package from tracking package using system reference search: " << systemReferenceSearch.ToString()); - // Correlate against installed (allow exceptions out as we own the installed source) - SearchResult installedCrossRef = m_installedSource.Search(systemReferenceSearch); - - // SXS_TODO: If feature enabled, take all matches and filter out weak match results by doing an installed item correlation back to the source - auto installedPackage = GetMatchingPackage(installedCrossRef.Matches, - [&]() { - AICLI_LOG(Repo, Info, - << "Found multiple matches for tracking package [" << match.Package->GetProperty(PackageProperty::Id) << - "] in source [" << source.GetIdentifier() << "] when searching for [" << systemReferenceSearch.ToString() << "]"); - }, [&] { - AICLI_LOG(Repo, Warning, << " Appropriate installed package could not be determined"); - }); - - // SXS_TODO: IFF none of the installed matches are present, create a new entry (maybe also allow weak matches to be thrown out this way too?) - if (installedPackage && !result.ContainsInstalledPackage(installedPackage->GetInstalled().get())) - { - auto compositePackage = std::make_shared( - installedPackage, - GetTrackedPackageFromAvailableSource(result, source, match.Package->GetProperty(PackageProperty::Id)), - true); - - compositePackage->SetTracking(source, OnlyAvailable(match.Package), GetLatestTrackingWriteTime(match.Package)); - - result.Matches.emplace_back(std::move(compositePackage), match.MatchCriteria); - } - } - } SearchResult availableResult = result.SearchAndHandleFailures(source, request); bool downloadManifests = source.QueryFeatureFlag(SourceFeatureFlag::ManifestMayContainAdditionalSystemReferenceStrings); for (auto&& match : availableResult.Matches) { - // Check for a package already in the result that should have been correlated already. + // Check for the package already in the result. // In cases that PackageData will be created, also download manifests for system reference strings // when search result is small (currently limiting to 1). auto packageData = result.CheckForExistingResultFromAvailablePackageMatch(match, downloadManifests && availableResult.Matches.size() == 1); @@ -1607,33 +1559,121 @@ namespace AppInstaller::Repository continue; } + // Use data from the tracking catalog as it can potentially get better correlations + auto trackingPackage = packageData->AddSystemReferenceStringsFromTrackingPackage(trackingCatalog, match.Package->GetProperty(PackageProperty::Id), source.GetDetails().Name); + // If no package was found that was already in the results, do a correlation lookup with the installed // source to create a new composite package entry if we find any packages there. bool foundInstalledMatch = false; - if (packageData && !packageData->SystemReferenceStrings.empty()) + if (!packageData->SystemReferenceStrings.empty()) { // Create a search request to run against the installed source SearchRequest systemReferenceSearch = packageData->CreateInclusionsSearchRequest(SearchPurpose::CorrelationToInstalled); - AICLI_LOG(Repo, Info, << "Finding installed package from available package using system reference search: " << systemReferenceSearch.ToString()); + AICLI_LOG(Repo, Verbose, << "Finding installed package from available package using system reference search: " << systemReferenceSearch.ToString()); // Correlate against installed (allow exceptions out as we own the installed source) SearchResult installedCrossRef = m_installedSource.Search(systemReferenceSearch); - // SXS_TODO: Figure out how to handle first, then do tracking second - auto installedPackage = GetMatchingPackage(installedCrossRef.Matches, - [&]() { - AICLI_LOG(Repo, Info, + if (ExperimentalFeature::IsEnabled(ExperimentalFeature::Feature::SideBySide)) + { + for (const auto& installedMatch : installedCrossRef.Matches) + { + if (!IsStrongMatchField(installedMatch.MatchCriteria.Field)) + { + // For weak correlations, do an installed -> available check to ensure that there are no other strong correlations. + SearchResult correlationConfirmation; + if (source.GetDetails().SupportInstalledSearchCorrelation) + { + correlationConfirmation = result.SearchAndHandleFailures(source, result.GetSystemReferenceStrings(installedMatch.Package->GetInstalled().get()).CreateInclusionsSearchRequest(SearchPurpose::CorrelationToAvailable)); + } + + if (correlationConfirmation.Matches.empty()) + { + // We probably made the correlation due to tracking data, keep it. + } + else if (correlationConfirmation.Matches.size() > 1) + { + // There is contention for the correlation. + AICLI_LOG(Repo, Verbose, << " ... installed package [" << installedMatch.Package->GetProperty(PackageProperty::Id) << + "] had multiple correlations and is being ignored as a match for [" << match.Package->GetProperty(PackageProperty::Id) << "]"); + continue; + } + else if (!OnlyAvailable(correlationConfirmation.Matches[0].Package)->IsSame(OnlyAvailable(match.Package).get())) + { + // The only correlation is not to the current package. + AICLI_LOG(Repo, Verbose, << " ... installed package [" << installedMatch.Package->GetProperty(PackageProperty::Id) << + "] was found through available package [" << match.Package->GetProperty(PackageProperty::Id) << "], but only correlated to [" << + correlationConfirmation.Matches[0].Package->GetProperty(PackageProperty::Id) << "] and is being ignored"); + continue; + } + } + + // Now that we know we need to add this available package, determine how exactly + std::shared_ptr resultPackage = result.FindInstalledPackage(installedMatch.Package->GetInstalled().get()); + + if (resultPackage) + { + // Check for a package from the same source already present on the result package. + bool foundSameSource = false; + + for (const auto& availablePackage : resultPackage->GetAvailablePackages()) + { + if (availablePackage->GetSource() == source) + { + // TODO: May need to add more data so that we can choose the proper correlation, but it may also be very difficult to get through + // the gauntlet of other checks and arrive in this situation. + AICLI_LOG(Repo, Verbose, << " ... found [" << availablePackage->GetProperty(PackageProperty::Id) << + "] already correlated to [" << installedMatch.Package->GetProperty(PackageProperty::Id) << "] from the same source [" << + source.GetDetails().Name << "] as [" << match.Package->GetProperty(PackageProperty::Id) << "]; ignoring the second correlation"); + foundSameSource = true; + } + } + + if (foundSameSource) + { + continue; + } + } + else + { + result.Matches.emplace_back(std::make_shared(installedMatch.Package), match.MatchCriteria); + resultPackage = result.Matches.back().Package; + } + + bool setPrimary = false; + if (trackingPackage) + { + auto trackingPackageTime = GetLatestTrackingWriteTime(trackingPackage); + + if (trackingPackageTime > resultPackage->GetTrackingPackageWriteTime()) + { + resultPackage->SetTracking(source, std::move(trackingPackage), trackingPackageTime); + setPrimary = true; + } + } + + resultPackage->AddAvailablePackage(std::move(match.Package), setPrimary); + + foundInstalledMatch = true; + } + } + else + { + auto installedPackage = GetMatchingPackage(installedCrossRef.Matches, + [&]() { + AICLI_LOG(Repo, Info, << "Found multiple matches for available package [" << match.Package->GetProperty(PackageProperty::Id) << "] in source [" << source.GetIdentifier() << "] when searching for [" << systemReferenceSearch.ToString() << "]"); - }, [&] { - AICLI_LOG(Repo, Warning, << " Appropriate installed package could not be determined"); - }); + }, [&] { + AICLI_LOG(Repo, Warning, << " Appropriate installed package could not be determined"); + }); - if (installedPackage && !result.ContainsInstalledPackage(installedPackage->GetInstalled().get())) - { - // TODO: Needs a whole separate change to fix the fact that we don't support multiple available packages and what the different search behaviors mean - foundInstalledMatch = true; - result.Matches.emplace_back(std::make_shared(installedPackage, std::move(match.Package)), match.MatchCriteria); + if (installedPackage && !result.ContainsInstalledPackage(installedPackage->GetInstalled().get())) + { + // TODO: Needs a whole separate change to fix the fact that we don't support multiple available packages and what the different search behaviors mean + foundInstalledMatch = true; + result.Matches.emplace_back(std::make_shared(installedPackage, std::move(match.Package)), match.MatchCriteria); + } } } From 6d2371c6c17531f1c8e74f404fb40992770656aa Mon Sep 17 00:00:00 2001 From: John McPherson Date: Wed, 6 Mar 2024 12:18:19 -0800 Subject: [PATCH 16/24] Update command UX and fix bug --- .../Workflows/InstallFlow.cpp | 5 ++-- .../Workflows/UpdateFlow.cpp | 1 + .../Workflows/WorkflowBase.cpp | 10 +++++-- .../CompositeSource.cpp | 30 +++++++++++-------- .../Schema/1_0/SearchResultsTable_1_0.cpp | 8 ++--- 5 files changed, 34 insertions(+), 20 deletions(-) diff --git a/src/AppInstallerCLICore/Workflows/InstallFlow.cpp b/src/AppInstallerCLICore/Workflows/InstallFlow.cpp index 88eb099583..bc8ed19cc7 100644 --- a/src/AppInstallerCLICore/Workflows/InstallFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/InstallFlow.cpp @@ -374,8 +374,9 @@ namespace AppInstaller::CLI::Workflow void ExecuteInstallerForType::operator()(Execution::Context& context) const { bool isUpdate = WI_IsFlagSet(context.GetFlags(), Execution::ContextFlag::InstallerExecutionUseUpdate); - UpdateBehaviorEnum updateBehavior = context.Get().value().UpdateBehavior; - bool doUninstallPrevious = isUpdate && (updateBehavior == UpdateBehaviorEnum::UninstallPrevious || context.Args.Contains(Execution::Args::Type::UninstallPrevious)); + //UpdateBehaviorEnum updateBehavior = context.Get().value().UpdateBehavior; + //bool doUninstallPrevious = isUpdate && (updateBehavior == UpdateBehaviorEnum::UninstallPrevious || context.Args.Contains(Execution::Args::Type::UninstallPrevious)); + bool doUninstallPrevious = false; Synchronization::CrossProcessInstallLock lock; if (!ExemptFromSingleInstallLocking(m_installerType)) diff --git a/src/AppInstallerCLICore/Workflows/UpdateFlow.cpp b/src/AppInstallerCLICore/Workflows/UpdateFlow.cpp index 4c72c468d5..99d0a82e04 100644 --- a/src/AppInstallerCLICore/Workflows/UpdateFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/UpdateFlow.cpp @@ -87,6 +87,7 @@ namespace AppInstaller::CLI::Workflow // The only way to enter this portion of the statement with isUpgrade is if the version is available if (isUpgrade) { + AICLI_LOG(CLI, Verbose, << "Updating from [" << installedVersion.ToString() << "] to [" << key.Version << "]"); upgradeVersionAvailable = true; } diff --git a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp index 5565ec1d96..d5c7cb0718 100644 --- a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp +++ b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp @@ -806,15 +806,21 @@ namespace AppInstaller::CLI::Workflow continue; } + // We only want to evaluate update availability for the latest version. + bool isFirstInstalledVersion = true; + for (const auto& installedVersionKey : installedPackage->GetVersionKeys()) { + bool isFirstInstalledVersionLocal = isFirstInstalledVersion; + isFirstInstalledVersion = false; + auto installedVersion = installedPackage->GetVersion(installedVersionKey); auto evaluator = pinningData.CreatePinStateEvaluator(pinBehavior, installedVersion); auto availableVersions = GetAvailableVersionsForInstalledVersion(match.Package, installedVersion); auto latestVersion = evaluator.GetLatestAvailableVersionForPins(availableVersions); - bool updateAvailable = evaluator.IsUpdate(latestVersion); + bool updateAvailable = isFirstInstalledVersionLocal && evaluator.IsUpdate(latestVersion); bool updateIsPinned = false; if (m_onlyShowUpgrades && !context.Args.Contains(Execution::Args::Type::IncludeUnknown) && Utility::Version(installedVersion->GetProperty(PackageVersionProperty::Version)).IsUnknown() && updateAvailable) @@ -824,7 +830,7 @@ namespace AppInstaller::CLI::Workflow continue; } - if (m_onlyShowUpgrades && !updateAvailable) + if (m_onlyShowUpgrades && !updateAvailable && isFirstInstalledVersionLocal) { // Reuse the evaluator to check if there is an update outside of the pinning auto unpinnedLatestVersion = availableVersions->GetLatestVersion(); diff --git a/src/AppInstallerRepositoryCore/CompositeSource.cpp b/src/AppInstallerRepositoryCore/CompositeSource.cpp index e468f77beb..7cddaf7278 100644 --- a/src/AppInstallerRepositoryCore/CompositeSource.cpp +++ b/src/AppInstallerRepositoryCore/CompositeSource.cpp @@ -370,27 +370,33 @@ namespace AppInstaller::Repository { installedVersion = key.InstalledVersion; overrideVersion = key.Version; + break; } } - // Get the appropriate tracking version or latest if it is not found. - // The tracking package uses the mapped version. - std::shared_ptr trackingPackageVersion; - if (m_trackingPackage) + if (installedVersion) { - // Remove our use of the package id as source - PackageVersionKey versionKey_NoSource = versionKey; - versionKey_NoSource.SourceId.clear(); + // Get the appropriate tracking version or latest if it is not found. + // The tracking package uses the mapped version. + std::shared_ptr trackingPackageVersion; + if (m_trackingPackage) + { + // Remove our use of the package id as source + PackageVersionKey versionKey_NoSource = versionKey; + versionKey_NoSource.SourceId.clear(); - trackingPackageVersion = m_trackingPackage->GetVersion(versionKey_NoSource); + trackingPackageVersion = m_trackingPackage->GetVersion(versionKey_NoSource); - if (!trackingPackageVersion) - { - trackingPackageVersion = m_trackingPackage->GetLatestVersion(); + if (!trackingPackageVersion) + { + trackingPackageVersion = m_trackingPackage->GetLatestVersion(); + } } + + return std::make_shared(std::move(installedVersion), m_trackingSource, std::move(trackingPackageVersion), std::move(overrideVersion)); } - return installedVersion ? std::make_shared(std::move(installedVersion), m_trackingSource, std::move(trackingPackageVersion), std::move(overrideVersion)) : nullptr; + return nullptr; } std::shared_ptr GetLatestVersion() const override diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/SearchResultsTable_1_0.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/SearchResultsTable_1_0.cpp index f5508e4a3d..dd34ecbc57 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/SearchResultsTable_1_0.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/SearchResultsTable_1_0.cpp @@ -105,7 +105,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 SQLite::Statement statement = builder.Prepare(m_connection); BindStatementForMatchType(statement, filter, bindIndex); statement.Execute(); - AICLI_LOG(Repo, Verbose, << "Search found " << m_connection.GetChanges() << " rows"); + AICLI_LOG(SQL, Verbose, << "Search found " << m_connection.GetChanges() << " rows"); } void SearchResultsTable::RemoveDuplicateManifestRows() @@ -128,7 +128,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 EndParenthetical(); builder.Execute(m_connection); - AICLI_LOG(Repo, Verbose, << "Removed " << m_connection.GetChanges() << " duplicate rows"); + AICLI_LOG(SQL, Verbose, << "Removed " << m_connection.GetChanges() << " duplicate rows"); } void SearchResultsTable::PrepareToFilter() @@ -170,7 +170,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 SQLite::Statement statement = builder.Prepare(m_connection); BindStatementForMatchType(statement, filter, bindIndex); statement.Execute(); - AICLI_LOG(Repo, Verbose, << "Filter kept " << m_connection.GetChanges() << " rows"); + AICLI_LOG(SQL, Verbose, << "Filter kept " << m_connection.GetChanges() << " rows"); } void SearchResultsTable::CompleteFilter() @@ -180,7 +180,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 builder.DeleteFrom(GetQualifiedName()).Where(s_SearchResultsTable_Filter).Equals(false); builder.Execute(m_connection); - AICLI_LOG(Repo, Verbose, << "Filter deleted " << m_connection.GetChanges() << " rows"); + AICLI_LOG(SQL, Verbose, << "Filter deleted " << m_connection.GetChanges() << " rows"); } ISQLiteIndex::SearchResult SearchResultsTable::GetSearchResults(size_t limit) From 675f3beaa6862b8c405124827944aa98cb3a19ae Mon Sep 17 00:00:00 2001 From: John McPherson Date: Wed, 6 Mar 2024 18:26:55 -0800 Subject: [PATCH 17/24] list/upgrade/repair/uninstall commands updated --- src/AppInstallerCLICore/Argument.cpp | 10 ++- src/AppInstallerCLICore/Argument.h | 1 + .../Commands/RepairCommand.cpp | 8 +- .../Commands/UninstallCommand.cpp | 9 +-- src/AppInstallerCLICore/ExecutionArgs.h | 2 + src/AppInstallerCLICore/Resources.h | 3 + .../Workflows/CompletionFlow.cpp | 26 +++++++ .../Workflows/RepairFlow.cpp | 2 +- .../Workflows/UninstallFlow.cpp | 75 ++++++++++++++++++- .../Workflows/UninstallFlow.h | 7 ++ .../Workflows/WorkflowBase.cpp | 40 +++++++++- .../Workflows/WorkflowBase.h | 6 ++ .../Shared/Strings/en-us/winget.resw | 10 +++ 13 files changed, 188 insertions(+), 11 deletions(-) diff --git a/src/AppInstallerCLICore/Argument.cpp b/src/AppInstallerCLICore/Argument.cpp index 7497468840..e3140e21a0 100644 --- a/src/AppInstallerCLICore/Argument.cpp +++ b/src/AppInstallerCLICore/Argument.cpp @@ -30,7 +30,7 @@ namespace AppInstaller::CLI case Execution::Args::Type::Query: return { type, "query"_liv, 'q', ArgTypeCategory::PackageQuery | ArgTypeCategory::SinglePackageQuery }; case Execution::Args::Type::MultiQuery: - return { type, "query"_liv, 'q', ArgTypeCategory::PackageQuery }; + return { type, "query"_liv, 'q', ArgTypeCategory::PackageQuery | ArgTypeCategory::MultiplePackages }; case Execution::Args::Type::Manifest: return { type, "manifest"_liv, 'm', ArgTypeCategory::Manifest }; @@ -101,6 +101,10 @@ namespace AppInstaller::CLI return { type, "preserve"_liv, ArgTypeCategory::None, ArgTypeExclusiveSet::PurgePreserve }; case Execution::Args::Type::ProductCode: return { type, "product-code"_liv, ArgTypeCategory::SinglePackageQuery }; + case Execution::Args::Type::AllVersions: + return { type, "all-versions"_liv, "all"_liv, ArgTypeCategory::CopyFlagToSubContext, ArgTypeExclusiveSet::AllAndTargetVersion }; + case Execution::Args::Type::TargetVersion: + return { type, "version"_liv, 'v', ArgTypeCategory::SinglePackageQuery, ArgTypeExclusiveSet::AllAndTargetVersion }; //Source Command case Execution::Args::Type::SourceName: @@ -379,6 +383,10 @@ namespace AppInstaller::CLI return Argument{ type, Resource::String::AllowRebootArgumentDescription, ArgumentType::Flag }; case Args::Type::IgnoreResumeLimit: return Argument{ type, Resource::String::IgnoreResumeLimitArgumentDescription, ArgumentType::Flag, ExperimentalFeature::Feature::Resume }; + case Args::Type::AllVersions: + return Argument{ type, Resource::String::UninstallAllVersionsArgumentDescription, ArgumentType::Flag, Argument::Visibility::Help, ExperimentalFeature::Feature::SideBySide }; + case Args::Type::TargetVersion: + return Argument{ type, Resource::String::TargetVersionArgumentDescription, ArgumentType::Standard }; default: THROW_HR(E_UNEXPECTED); } diff --git a/src/AppInstallerCLICore/Argument.h b/src/AppInstallerCLICore/Argument.h index d33741eaf1..96990fbece 100644 --- a/src/AppInstallerCLICore/Argument.h +++ b/src/AppInstallerCLICore/Argument.h @@ -85,6 +85,7 @@ namespace AppInstaller::CLI PurgePreserve = 0x4, PinType = 0x8, StubType = 0x10, + AllAndTargetVersion = 0x20, // This must always be at the end Max diff --git a/src/AppInstallerCLICore/Commands/RepairCommand.cpp b/src/AppInstallerCLICore/Commands/RepairCommand.cpp index 58668a783b..210dabb584 100644 --- a/src/AppInstallerCLICore/Commands/RepairCommand.cpp +++ b/src/AppInstallerCLICore/Commands/RepairCommand.cpp @@ -20,7 +20,7 @@ namespace AppInstaller::CLI Argument::ForType(Args::Type::Name), // -n Argument::ForType(Args::Type::Channel), Argument::ForType(Args::Type::Moniker), // -mn - Argument::ForType(Args::Type::Version), // -v + Argument::ForType(Args::Type::TargetVersion), // -v Argument::ForType(Args::Type::ProductCode), Argument::ForType(Args::Type::InstallArchitecture), // -arch Argument{ Execution::Args::Type::InstallScope, Resource::String::InstalledScopeArgumentDescription, ArgumentType::Standard, Argument::Visibility::Help }, @@ -60,12 +60,16 @@ namespace AppInstaller::CLI return; } + context << + Workflow::OpenSource() << + Workflow::OpenCompositeSource(Repository::PredefinedSource::Installed); + switch (valueType) { case Execution::Args::Type::Id: case Execution::Args::Type::Name: case Execution::Args::Type::Moniker: - case Execution::Args::Type::Version: + case Execution::Args::Type::TargetVersion: case Execution::Args::Type::Channel: case Execution::Args::Type::Source: context << diff --git a/src/AppInstallerCLICore/Commands/UninstallCommand.cpp b/src/AppInstallerCLICore/Commands/UninstallCommand.cpp index a0b0ae91a8..bf531daa61 100644 --- a/src/AppInstallerCLICore/Commands/UninstallCommand.cpp +++ b/src/AppInstallerCLICore/Commands/UninstallCommand.cpp @@ -23,11 +23,12 @@ namespace AppInstaller::CLI Argument::ForType(Args::Type::Name), Argument::ForType(Args::Type::Moniker), Argument::ForType(Args::Type::ProductCode), - Argument::ForType(Args::Type::Version), + Argument::ForType(Args::Type::TargetVersion), + Argument::ForType(Args::Type::AllVersions), Argument::ForType(Args::Type::Channel), Argument::ForType(Args::Type::Source), Argument::ForType(Args::Type::Exact), - Argument{ Execution::Args::Type::InstallScope, Resource::String::InstalledScopeArgumentDescription, ArgumentType::Standard, Argument::Visibility::Help }, + Argument{ Args::Type::InstallScope, Resource::String::InstalledScopeArgumentDescription, ArgumentType::Standard, Argument::Visibility::Help }, Argument::ForType(Args::Type::Interactive), Argument::ForType(Args::Type::Silent), Argument::ForType(Args::Type::Force), @@ -75,7 +76,7 @@ namespace AppInstaller::CLI case Execution::Args::Type::Id: case Execution::Args::Type::Name: case Execution::Args::Type::Moniker: - case Execution::Args::Type::Version: + case Execution::Args::Type::TargetVersion: case Execution::Args::Type::Channel: case Execution::Args::Type::Source: case Execution::Args::Type::ProductCode: @@ -111,7 +112,6 @@ namespace AppInstaller::CLI // --manifest case where new manifest is provided context << Workflow::GetManifestFromArg << - Workflow::ReportManifestIdentity << Workflow::SearchSourceUsingManifest << Workflow::EnsureOneMatchFromSearchResult(OperationType::Uninstall) << Workflow::UninstallSinglePackage; @@ -125,7 +125,6 @@ namespace AppInstaller::CLI Workflow::SearchSourceForSingle << Workflow::HandleSearchResultFailures << Workflow::EnsureOneMatchFromSearchResult(OperationType::Uninstall) << - Workflow::ReportPackageIdentity << Workflow::UninstallSinglePackage; } else diff --git a/src/AppInstallerCLICore/ExecutionArgs.h b/src/AppInstallerCLICore/ExecutionArgs.h index ad1b70496a..8010d563cd 100644 --- a/src/AppInstallerCLICore/ExecutionArgs.h +++ b/src/AppInstallerCLICore/ExecutionArgs.h @@ -54,6 +54,8 @@ namespace AppInstaller::CLI::Execution Purge, // Removes all files and directories related to a package during an uninstall. Only applies to the portable installerType. Preserve, // Retains any files and directories created by the portable exe. ProductCode, // Uninstalls using the product code as the identifier. + AllVersions, // Uninstall all versions of the package + TargetVersion, // The specific version to target //Source Command SourceName, diff --git a/src/AppInstallerCLICore/Resources.h b/src/AppInstallerCLICore/Resources.h index 65bb45e870..69e5961eb9 100644 --- a/src/AppInstallerCLICore/Resources.h +++ b/src/AppInstallerCLICore/Resources.h @@ -562,6 +562,7 @@ namespace AppInstaller::CLI::Resource WINGET_DEFINE_RESOURCE_STRINGID(StateHeader); WINGET_DEFINE_RESOURCE_STRINGID(SystemArchitecture); WINGET_DEFINE_RESOURCE_STRINGID(TagArgumentDescription); + WINGET_DEFINE_RESOURCE_STRINGID(TargetVersionArgumentDescription); WINGET_DEFINE_RESOURCE_STRINGID(ThankYou); WINGET_DEFINE_RESOURCE_STRINGID(ThirdPartSoftwareNotices); WINGET_DEFINE_RESOURCE_STRINGID(ToolDescription); @@ -573,9 +574,11 @@ namespace AppInstaller::CLI::Resource WINGET_DEFINE_RESOURCE_STRINGID(Unavailable); WINGET_DEFINE_RESOURCE_STRINGID(UnexpectedErrorExecutingCommand); WINGET_DEFINE_RESOURCE_STRINGID(UninstallAbandoned); + WINGET_DEFINE_RESOURCE_STRINGID(UninstallAllVersionsArgumentDescription); WINGET_DEFINE_RESOURCE_STRINGID(UninstallCommandLongDescription); WINGET_DEFINE_RESOURCE_STRINGID(UninstallCommandReportDependencies); WINGET_DEFINE_RESOURCE_STRINGID(UninstallCommandShortDescription); + WINGET_DEFINE_RESOURCE_STRINGID(UninstallFailedDueToMultipleVersions); WINGET_DEFINE_RESOURCE_STRINGID(UninstallFailedWithCode); WINGET_DEFINE_RESOURCE_STRINGID(UninstallFlowStartingPackageUninstall); WINGET_DEFINE_RESOURCE_STRINGID(UninstallFlowUninstallSuccess); diff --git a/src/AppInstallerCLICore/Workflows/CompletionFlow.cpp b/src/AppInstallerCLICore/Workflows/CompletionFlow.cpp index 57789b8b1d..2090335e46 100644 --- a/src/AppInstallerCLICore/Workflows/CompletionFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/CompletionFlow.cpp @@ -82,6 +82,25 @@ namespace AppInstaller::CLI::Workflow } } + void CompleteWithSearchResultInstalledVersions(Execution::Context& context) + { + const std::string& word = context.Get().Word(); + auto stream = context.Reporter.Completion(); + + auto installedPackage = context.Get()->GetInstalled(); + + if (installedPackage) + { + for (const auto& vc : installedPackage->GetVersionKeys()) + { + if (word.empty() || Utility::ICUCaseInsensitiveStartsWith(vc.Version, word)) + { + OutputCompletionString(stream, vc.Version); + } + } + } + } + void CompleteWithSearchResultChannels(Execution::Context& context) { const std::string& word = context.Get().Word(); @@ -174,6 +193,13 @@ namespace AppInstaller::CLI::Workflow Workflow::EnsureOneMatchFromSearchResult(OperationType::Completion) << Workflow::CompleteWithSearchResultVersions; break; + case Execution::Args::Type::TargetVersion: + // Here we require that the standard search finds a single entry, and we list the installed versions. + context << + Workflow::SearchSourceForSingle << + Workflow::EnsureOneMatchFromSearchResult(OperationType::Completion) << + Workflow::CompleteWithSearchResultInstalledVersions; + break; case Execution::Args::Type::Channel: // Here we require that the standard search finds a single entry, and we list those channels. context << diff --git a/src/AppInstallerCLICore/Workflows/RepairFlow.cpp b/src/AppInstallerCLICore/Workflows/RepairFlow.cpp index f4158c4622..44e05419d5 100644 --- a/src/AppInstallerCLICore/Workflows/RepairFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/RepairFlow.cpp @@ -441,7 +441,7 @@ namespace AppInstaller::CLI::Workflow AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_NO_APPLICABLE_INSTALLER); } - std::string_view requestedVersion = context.Args.Contains(Execution::Args::Type::Version) ? context.Args.GetArg(Execution::Args::Type::Version) : installedVersion.ToString(); + std::string_view requestedVersion = context.Args.Contains(Execution::Args::Type::TargetVersion) ? context.Args.GetArg(Execution::Args::Type::TargetVersion) : installedVersion.ToString(); // If it's Store source with only one version unknown, use the unknown version for available version mapping. const auto& package = context.Get(); auto packageVersions = GetAvailableVersionsForInstalledVersion(package, installedPackage); diff --git a/src/AppInstallerCLICore/Workflows/UninstallFlow.cpp b/src/AppInstallerCLICore/Workflows/UninstallFlow.cpp index 225e58cd64..d187580144 100644 --- a/src/AppInstallerCLICore/Workflows/UninstallFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/UninstallFlow.cpp @@ -69,9 +69,82 @@ namespace AppInstaller::CLI::Workflow } void UninstallSinglePackage(Execution::Context& context) + { + std::shared_ptr package = context.Get(); + std::shared_ptr installed = package->GetInstalled(); + std::vector installedVersionKeys; + if (installed) + { + installedVersionKeys = installed->GetVersionKeys(); + } + + // Handle + if (installedVersionKeys.size() > 1 && context.Args.Contains(Execution::Args::Type::AllVersions)) + { + bool allSucceeded = true; + size_t versionsCount = installedVersionKeys.size(); + size_t versionsProgress = 0; + + for (const auto& key : installedVersionKeys) + { + context.Reporter.Info() << '(' << ++versionsProgress << '/' << versionsCount << ") "_liv; + + // We want to do best effort to uninstall all versions regardless of previous failures + auto subContextPtr = context.CreateSubContext(); + Execution::Context& uninstallContext = *subContextPtr; + auto previousThreadGlobals = uninstallContext.SetForCurrentThread(); + + uninstallContext.Add(package); + uninstallContext.Add(installed->GetVersion(key)); + + // Prevent individual exceptions from breaking out of the loop + try + { + uninstallContext << + Workflow::UninstallSinglePackageVersion; + } + catch (...) + { + uninstallContext.SetTerminationHR(Workflow::HandleException(uninstallContext, std::current_exception())); + } + + uninstallContext.Reporter.Info() << std::endl; + + if (uninstallContext.IsTerminated()) + { + if (context.IsTerminated() && context.GetTerminationHR() == E_ABORT) + { + // This means that the subcontext being terminated is due to an overall abort + context.Reporter.Info() << Resource::String::Cancelled << std::endl; + return; + } + + allSucceeded = false; + } + } + + if (!allSucceeded) + { + AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_MULTIPLE_UNINSTALL_FAILED); + } + } + else if (installedVersionKeys.size() > 1 && !context.Args.Contains(Execution::Args::Type::TargetVersion)) + { + context.Reporter.Error() << Resource::String::UninstallFailedDueToMultipleVersions << std::endl; + AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_MULTIPLE_APPLICATIONS_FOUND); + } + else + { + context << + Workflow::GetInstalledPackageVersion << + Workflow::UninstallSinglePackageVersion; + } + } + + void UninstallSinglePackageVersion(Execution::Context& context) { context << - Workflow::GetInstalledPackageVersion << + Workflow::ReportInstalledPackageVersionIdentity << Workflow::EnsureSupportForUninstall << Workflow::GetUninstallInfo << Workflow::GetDependenciesInfoForUninstall << diff --git a/src/AppInstallerCLICore/Workflows/UninstallFlow.h b/src/AppInstallerCLICore/Workflows/UninstallFlow.h index 5beb75fcde..4952bf0062 100644 --- a/src/AppInstallerCLICore/Workflows/UninstallFlow.h +++ b/src/AppInstallerCLICore/Workflows/UninstallFlow.h @@ -12,6 +12,13 @@ namespace AppInstaller::CLI::Workflow // Outputs: None void UninstallSinglePackage(Execution::Context& context); + // Uninstalls a single package version. This also does the reporting, user interaction, and recording + // for single-package uninstallation. + // RequiredArgs: None + // Inputs: InstalledPackageVersion + // Outputs: None + void UninstallSinglePackageVersion(Execution::Context& context); + // Uninstalls multiple packages. // RequiredArgs: None // Inputs: PackageSubContexts diff --git a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp index d5c7cb0718..752455b7d8 100644 --- a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp +++ b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp @@ -1226,6 +1226,13 @@ namespace AppInstaller::CLI::Workflow ReportIdentity(context, {}, Resource::String::ReportIdentityFound, package->GetProperty(PackageProperty::Name), package->GetProperty(PackageProperty::Id)); } + void ReportInstalledPackageVersionIdentity(Execution::Context& context) + { + auto package = context.Get(); + auto version = context.Get(); + ReportIdentity(context, {}, Resource::String::ReportIdentityFound, version->GetProperty(PackageVersionProperty::Name), package ? package->GetProperty(PackageProperty::Id) : version->GetProperty(PackageVersionProperty::Id)); + } + void ReportManifestIdentity(Execution::Context& context) { const auto& manifest = context.Get(); @@ -1361,7 +1368,38 @@ namespace AppInstaller::CLI::Workflow void GetInstalledPackageVersion(Execution::Context& context) { - context.Add(GetInstalledVersion(context.Get())); + std::shared_ptr installed = context.Get()->GetInstalled(); + + if (ExperimentalFeature::IsEnabled(ExperimentalFeature::Feature::SideBySide)) + { + if (installed) + { + // TODO: This may need to be expanded dramatically to enable targeting across a variety of dimensions (architecture, etc.) + // Alternatively, if we make it easier to see the fully unique package identifiers, we may avoid that need. + if (context.Args.Contains(Execution::Args::Type::TargetVersion)) + { + Repository::PackageVersionKey versionKey{ "", context.Args.GetArg(Execution::Args::Type::TargetVersion) , "" }; + std::shared_ptr installedVersion = installed->GetVersion(versionKey); + + if (!installedVersion) + { + context.Reporter.Error() << Resource::String::GetManifestResultVersionNotFound(Utility::LocIndView{ versionKey.Version }) << std::endl; + // This error maintains consistency with passing an available version to commands + AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_NO_MANIFEST_FOUND); + } + + context.Add(std::move(installedVersion)); + } + else + { + context.Add(installed->GetLatestVersion()); + } + } + } + else + { + context.Add(GetInstalledVersion(context.Get())); + } } void ReportExecutionStage::operator()(Execution::Context& context) const diff --git a/src/AppInstallerCLICore/Workflows/WorkflowBase.h b/src/AppInstallerCLICore/Workflows/WorkflowBase.h index 6595256426..c8cdce1f2c 100644 --- a/src/AppInstallerCLICore/Workflows/WorkflowBase.h +++ b/src/AppInstallerCLICore/Workflows/WorkflowBase.h @@ -353,6 +353,12 @@ namespace AppInstaller::CLI::Workflow // Outputs: None void ReportPackageIdentity(Execution::Context& context); + // Reports the installed package version identity. + // Required Args: None + // Inputs: InstalledPackageVersion + // Outputs: None + void ReportInstalledPackageVersionIdentity(Execution::Context& context); + // Reports the manifest's identity. // Required Args: None // Inputs: Manifest diff --git a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw index eba36a6e48..eb7f2d25dc 100644 --- a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw +++ b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw @@ -2817,4 +2817,14 @@ Please specify one of them using the --source option to proceed. The SQLite connection was terminated to prevent corruption. + + Uninstall all versions + + + The version to act upon + + + Multiple versions of this package are installed. Either refine the search, pass the `--version` argument to select one, or pass the `--all-versions` flag to uninstall all of them. + {Locked="--version,--all-versions"} + \ No newline at end of file From 407d92363d3154297a94d5652361eaa0215b48b6 Mon Sep 17 00:00:00 2001 From: John McPherson Date: Thu, 7 Mar 2024 13:25:04 -0800 Subject: [PATCH 18/24] minor fixes --- src/AppInstallerCLICore/Workflows/InstallFlow.cpp | 5 ++--- src/AppInstallerCLICore/Workflows/UninstallFlow.cpp | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/AppInstallerCLICore/Workflows/InstallFlow.cpp b/src/AppInstallerCLICore/Workflows/InstallFlow.cpp index bc8ed19cc7..88eb099583 100644 --- a/src/AppInstallerCLICore/Workflows/InstallFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/InstallFlow.cpp @@ -374,9 +374,8 @@ namespace AppInstaller::CLI::Workflow void ExecuteInstallerForType::operator()(Execution::Context& context) const { bool isUpdate = WI_IsFlagSet(context.GetFlags(), Execution::ContextFlag::InstallerExecutionUseUpdate); - //UpdateBehaviorEnum updateBehavior = context.Get().value().UpdateBehavior; - //bool doUninstallPrevious = isUpdate && (updateBehavior == UpdateBehaviorEnum::UninstallPrevious || context.Args.Contains(Execution::Args::Type::UninstallPrevious)); - bool doUninstallPrevious = false; + UpdateBehaviorEnum updateBehavior = context.Get().value().UpdateBehavior; + bool doUninstallPrevious = isUpdate && (updateBehavior == UpdateBehaviorEnum::UninstallPrevious || context.Args.Contains(Execution::Args::Type::UninstallPrevious)); Synchronization::CrossProcessInstallLock lock; if (!ExemptFromSingleInstallLocking(m_installerType)) diff --git a/src/AppInstallerCLICore/Workflows/UninstallFlow.cpp b/src/AppInstallerCLICore/Workflows/UninstallFlow.cpp index d187580144..169b3b77be 100644 --- a/src/AppInstallerCLICore/Workflows/UninstallFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/UninstallFlow.cpp @@ -78,7 +78,7 @@ namespace AppInstaller::CLI::Workflow installedVersionKeys = installed->GetVersionKeys(); } - // Handle + // Handle multiple installed versions when we have been told to uninstall all of them. if (installedVersionKeys.size() > 1 && context.Args.Contains(Execution::Args::Type::AllVersions)) { bool allSucceeded = true; From 5d45564770fe6d645d6acefb73d2e0a46e57595a Mon Sep 17 00:00:00 2001 From: John McPherson Date: Mon, 11 Mar 2024 17:25:48 -0700 Subject: [PATCH 19/24] Ignore empty packages --- src/AppInstallerRepositoryCore/CompositeSource.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/AppInstallerRepositoryCore/CompositeSource.cpp b/src/AppInstallerRepositoryCore/CompositeSource.cpp index 7cddaf7278..279328108f 100644 --- a/src/AppInstallerRepositoryCore/CompositeSource.cpp +++ b/src/AppInstallerRepositoryCore/CompositeSource.cpp @@ -541,6 +541,13 @@ namespace AppInstaller::Repository // Note that it may tied for highest version if, for instance, the same version is installed for different architectures. void AddPackageAndVersionKeyData(std::shared_ptr package) { + // We don't want this to happen, but it could. Rather than a crash, we will log it and move on. + if (!package) + { + AICLI_LOG(Repo, Verbose, << "AddPackageAndVersionKeyData called with an empty package"); + return; + } + size_t packageIndex = m_packages.size(); std::string packageIdentifier = package->GetProperty(PackageProperty::Id); From 51d7835225c04df68a0277d796f25632ff334a0a Mon Sep 17 00:00:00 2001 From: John McPherson Date: Tue, 12 Mar 2024 17:34:46 -0700 Subject: [PATCH 20/24] Existing unit tests pass --- src/AppInstallerCLITests/CompositeSource.cpp | 234 +++++++++--------- .../PredefinedInstalledSource.cpp | 11 +- .../CompositeSource.cpp | 27 +- 3 files changed, 157 insertions(+), 115 deletions(-) diff --git a/src/AppInstallerCLITests/CompositeSource.cpp b/src/AppInstallerCLITests/CompositeSource.cpp index dbbddb7cff..1617b79239 100644 --- a/src/AppInstallerCLITests/CompositeSource.cpp +++ b/src/AppInstallerCLITests/CompositeSource.cpp @@ -32,9 +32,10 @@ struct ComponentTestSource : public TestSource { ComponentTestSource() = default; - ComponentTestSource(std::string_view identifier) + ComponentTestSource(std::string_view identifier, SourceOrigin origin = SourceOrigin::Default) { Details.Identifier = identifier; + Details.Origin = origin; } SearchResult Search(const SearchRequest& request) const override @@ -52,47 +53,6 @@ struct ComponentTestSource : public TestSource SearchResult Everything; }; -// A helper to create the sources used by the majority of tests in this file. -struct CompositeTestSetup -{ - CompositeTestSetup(CompositeSearchBehavior behavior = CompositeSearchBehavior::Installed) : Composite("*Tests") - { - Installed = std::make_shared("InstalledTestSource1"); - Available = std::make_shared("AvailableTestSource1"); - Composite.SetInstalledSource(Source{ Installed }, behavior); - Composite.AddAvailableSource(Source{ Available }); - } - - SearchResult Search() - { - SearchRequest request; - request.Query = RequestMatch(MatchType::Exact, s_Everything_Query); - return Composite.Search(request); - } - - std::shared_ptr Installed; - std::shared_ptr Available; - CompositeSource Composite; -}; - -// A helper to create the sources used by the majority of tests in this file. -struct CompositeWithTrackingTestSetup : public CompositeTestSetup -{ - CompositeWithTrackingTestSetup() : TrackingFactory([&](const SourceDetails&) { return Tracking; }) - { - Tracking = std::make_shared(SourceDetails{}, SQLiteIndex::CreateNew(SQLITE_MEMORY_DB_CONNECTION_TARGET)); - TestHook_SetSourceFactoryOverride(std::string{ PackageTrackingCatalogSourceFactory::Type() }, TrackingFactory); - } - - ~CompositeWithTrackingTestSetup() - { - TestHook_ClearSourceFactoryOverrides(); - } - - TestSourceFactory TrackingFactory; - std::shared_ptr Tracking; -}; - // A helper to make matches. struct Criteria : public PackageMatchFilter { @@ -190,15 +150,66 @@ struct TestPackageHelper bool m_hideSystemReferenceStrings = false; }; -TestPackageHelper MakeInstalled() +// A helper to create the sources used by the majority of tests in this file. +struct CompositeTestSetup { - return { /* isInstalled */ true}; -} + CompositeTestSetup(CompositeSearchBehavior behavior = CompositeSearchBehavior::Installed) : Composite("*Tests") + { + Installed = std::make_shared("InstalledTestSource1", SourceOrigin::Predefined); + Available = std::make_shared("AvailableTestSource1"); + Composite.SetInstalledSource(Source{ Installed }, behavior); + Composite.AddAvailableSource(Source{ Available }); + } + + SearchResult Search() + { + SearchRequest request; + request.Query = RequestMatch(MatchType::Exact, s_Everything_Query); + return Composite.Search(request); + } + + TestPackageHelper MakeInstalled(std::shared_ptr source) + { + return { /* isInstalled */ true, std::move(source)}; + } + + TestPackageHelper MakeInstalled() + { + return MakeInstalled(Installed); + } + + TestPackageHelper MakeAvailable(std::shared_ptr source) + { + return { /* isInstalled */ false, std::move(source) }; + } + + TestPackageHelper MakeAvailable() + { + return MakeAvailable(Available); + } -TestPackageHelper MakeAvailable(std::shared_ptr source) + std::shared_ptr Installed; + std::shared_ptr Available; + CompositeSource Composite; +}; + +// A helper to create the sources used by the majority of tests in this file. +struct CompositeWithTrackingTestSetup : public CompositeTestSetup { - return { /* isInstalled */ false, source}; -} + CompositeWithTrackingTestSetup() : TrackingFactory([&](const SourceDetails&) { return Tracking; }) + { + Tracking = std::make_shared(SourceDetails{}, SQLiteIndex::CreateNew(SQLITE_MEMORY_DB_CONNECTION_TARGET)); + TestHook_SetSourceFactoryOverride(std::string{ PackageTrackingCatalogSourceFactory::Type() }, TrackingFactory); + } + + ~CompositeWithTrackingTestSetup() + { + TestHook_ClearSourceFactoryOverrides(); + } + + TestSourceFactory TrackingFactory; + std::shared_ptr Tracking; +}; bool SearchRequestIncludes(const std::vector& filters, PackageMatchField field, MatchType type, std::optional value = {}) { @@ -227,7 +238,7 @@ TEST_CASE("CompositeSource_PackageFamilyName_NotAvailable", "[CompositeSource]") std::string pfn = "sortof_apfn"; CompositeTestSetup setup; - setup.Installed->Everything.Matches.emplace_back(MakeInstalled().WithPFN(pfn), Criteria()); + setup.Installed->Everything.Matches.emplace_back(setup.MakeInstalled().WithPFN(pfn), Criteria()); SearchResult result = setup.Search(); @@ -241,13 +252,13 @@ TEST_CASE("CompositeSource_PackageFamilyName_Available", "[CompositeSource]") std::string pfn = "sortof_apfn"; CompositeTestSetup setup; - setup.Installed->Everything.Matches.emplace_back(MakeInstalled().WithPFN(pfn), Criteria()); + setup.Installed->Everything.Matches.emplace_back(setup.MakeInstalled().WithPFN(pfn), Criteria()); setup.Available->SearchFunction = [&](const SearchRequest& request) { RequireSearchRequestIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); SearchResult result; - result.Matches.emplace_back(MakeAvailable(setup.Available).WithPFN(pfn), Criteria()); + result.Matches.emplace_back(setup.MakeAvailable().WithPFN(pfn), Criteria()); return result; }; @@ -264,7 +275,7 @@ TEST_CASE("CompositeSource_ProductCode_NotAvailable", "[CompositeSource]") std::string pc = "thiscouldbeapc"; CompositeTestSetup setup; - setup.Installed->Everything.Matches.emplace_back(MakeInstalled().WithPC(pc), Criteria()); + setup.Installed->Everything.Matches.emplace_back(setup.MakeInstalled().WithPC(pc), Criteria()); SearchResult result = setup.Search(); @@ -278,13 +289,13 @@ TEST_CASE("CompositeSource_ProductCode_Available", "[CompositeSource]") std::string pc = "thiscouldbeapc"; CompositeTestSetup setup; - setup.Installed->Everything.Matches.emplace_back(MakeInstalled().WithPC(pc), Criteria()); + setup.Installed->Everything.Matches.emplace_back(setup.MakeInstalled().WithPC(pc), Criteria()); setup.Available->SearchFunction = [&](const SearchRequest& request) { RequireSearchRequestIncludes(request.Inclusions, PackageMatchField::ProductCode, MatchType::Exact, pc); SearchResult result; - result.Matches.emplace_back(MakeAvailable(setup.Available).WithPC(pc), Criteria()); + result.Matches.emplace_back(setup.MakeAvailable().WithPC(pc), Criteria()); return result; }; @@ -299,13 +310,13 @@ TEST_CASE("CompositeSource_ProductCode_Available", "[CompositeSource]") TEST_CASE("CompositeSource_NameAndPublisher_Match", "[CompositeSource]") { CompositeTestSetup setup; - setup.Installed->Everything.Matches.emplace_back(MakeInstalled(), Criteria()); + setup.Installed->Everything.Matches.emplace_back(setup.MakeInstalled(), Criteria()); setup.Available->SearchFunction = [&](const SearchRequest& request) { RequireSearchRequestIncludes(request.Inclusions, PackageMatchField::NormalizedNameAndPublisher, MatchType::Exact); SearchResult result; - result.Matches.emplace_back(MakeAvailable(setup.Available), Criteria()); + result.Matches.emplace_back(setup.MakeAvailable(), Criteria()); return result; }; @@ -322,12 +333,12 @@ TEST_CASE("CompositeSource_MultiMatch_FindsStrongMatch", "[CompositeSource]") std::string name = "MatchingName"; CompositeTestSetup setup; - setup.Installed->Everything.Matches.emplace_back(MakeInstalled().WithPFN("sortof_apfn"), Criteria()); + setup.Installed->Everything.Matches.emplace_back(setup.MakeInstalled().WithPFN("sortof_apfn"), Criteria()); setup.Available->SearchFunction = [&](const SearchRequest&) { SearchResult result; - result.Matches.emplace_back(MakeAvailable(setup.Available).WithId("A different ID"), Criteria(PackageMatchField::NormalizedNameAndPublisher)); - result.Matches.emplace_back(MakeAvailable(setup.Available).WithDefaultName(name), Criteria(PackageMatchField::PackageFamilyName)); + result.Matches.emplace_back(setup.MakeAvailable().WithId("A different ID"), Criteria(PackageMatchField::NormalizedNameAndPublisher)); + result.Matches.emplace_back(setup.MakeAvailable().WithDefaultName(name), Criteria(PackageMatchField::PackageFamilyName)); return result; }; @@ -345,12 +356,12 @@ TEST_CASE("CompositeSource_MultiMatch_FindsStrongMatch", "[CompositeSource]") TEST_CASE("CompositeSource_MultiMatch_DoesNotFindStrongMatch", "[CompositeSource]") { CompositeTestSetup setup; - setup.Installed->Everything.Matches.emplace_back(MakeInstalled().WithPFN("sortof_apfn"), Criteria()); + setup.Installed->Everything.Matches.emplace_back(setup.MakeInstalled().WithPFN("sortof_apfn"), Criteria()); setup.Available->SearchFunction = [&](const SearchRequest&) { SearchResult result; - result.Matches.emplace_back(MakeAvailable(setup.Available).WithId("A different ID"), Criteria(PackageMatchField::NormalizedNameAndPublisher)); - result.Matches.emplace_back(MakeAvailable(setup.Available).WithId("Another diff ID"), Criteria(PackageMatchField::NormalizedNameAndPublisher)); + result.Matches.emplace_back(setup.MakeAvailable().WithId("A different ID"), Criteria(PackageMatchField::NormalizedNameAndPublisher)); + result.Matches.emplace_back(setup.MakeAvailable().WithId("Another diff ID"), Criteria(PackageMatchField::NormalizedNameAndPublisher)); return result; }; @@ -366,8 +377,8 @@ TEST_CASE("CompositeSource_FoundByBothRootSearches", "[CompositeSource]") std::string pfn = "sortof_apfn"; CompositeTestSetup setup; - auto installedPackage = MakeInstalled().WithPFN(pfn); - auto availablePackage = MakeAvailable(setup.Available).WithPFN(pfn); + auto installedPackage = setup.MakeInstalled().WithPFN(pfn); + auto availablePackage = setup.MakeAvailable().WithPFN(pfn); setup.Installed->Everything.Matches.emplace_back(installedPackage, Criteria()); setup.Installed->SearchFunction = [&](const SearchRequest& request) @@ -407,17 +418,17 @@ TEST_CASE("CompositeSource_OnlyAvailableFoundByRootSearch", "[CompositeSource]") RequireSearchRequestIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); SearchResult result; - result.Matches.emplace_back(MakeInstalled().WithPFN(pfn), Criteria()); + result.Matches.emplace_back(setup.MakeInstalled().WithPFN(pfn), Criteria()); return result; }; - setup.Available->Everything.Matches.emplace_back(MakeAvailable(setup.Available).WithPFN(pfn), Criteria()); + setup.Available->Everything.Matches.emplace_back(setup.MakeAvailable().WithPFN(pfn), Criteria()); setup.Available->SearchFunction = [&](const SearchRequest& request) { RequireSearchRequestIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); SearchResult result; - result.Matches.emplace_back(MakeAvailable(setup.Available).WithPFN(pfn), Criteria()); + result.Matches.emplace_back(setup.MakeAvailable().WithPFN(pfn), Criteria()); return result; }; @@ -434,13 +445,13 @@ TEST_CASE("CompositeSource_FoundByAvailableRootSearch_NotInstalled", "[Composite std::string pfn = "sortof_apfn"; CompositeTestSetup setup; - setup.Available->Everything.Matches.emplace_back(MakeAvailable(setup.Available).WithPFN(pfn), Criteria()); + setup.Available->Everything.Matches.emplace_back(setup.MakeAvailable().WithPFN(pfn), Criteria()); setup.Available->SearchFunction = [&](const SearchRequest& request) { RequireSearchRequestIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); SearchResult result; - result.Matches.emplace_back(MakeAvailable(setup.Available).WithPFN(pfn), Criteria()); + result.Matches.emplace_back(setup.MakeAvailable().WithPFN(pfn), Criteria()); return result; }; @@ -456,8 +467,8 @@ TEST_CASE("CompositeSource_UpdateWithBetterMatchCriteria", "[CompositeSource]") MatchType type = MatchType::Exact; CompositeTestSetup setup; - auto installedPackage = MakeInstalled().WithPFN(pfn); - auto availablePackage = MakeAvailable(setup.Available).WithPFN(pfn); + auto installedPackage = setup.MakeInstalled().WithPFN(pfn); + auto availablePackage = setup.MakeAvailable().WithPFN(pfn); setup.Installed->Everything.Matches.emplace_back(installedPackage, Criteria()); @@ -504,7 +515,7 @@ TEST_CASE("CompositePackage_PropertyFromInstalled", "[CompositeSource]") std::string id = "Special test ID"; CompositeTestSetup setup; - setup.Installed->Everything.Matches.emplace_back(MakeInstalled().WithId(id), Criteria()); + setup.Installed->Everything.Matches.emplace_back(setup.MakeInstalled().WithId(id), Criteria()); SearchResult result = setup.Search(); @@ -518,11 +529,11 @@ TEST_CASE("CompositePackage_PropertyFromAvailable", "[CompositeSource]") std::string pfn = "sortof_apfn"; CompositeTestSetup setup; - setup.Installed->Everything.Matches.emplace_back(MakeInstalled().WithPFN(pfn), Criteria()); + setup.Installed->Everything.Matches.emplace_back(setup.MakeInstalled().WithPFN(pfn), Criteria()); setup.Available->SearchFunction = [&](const SearchRequest&) { SearchResult result; - result.Matches.emplace_back(MakeAvailable(setup.Available).WithId(id), Criteria()); + result.Matches.emplace_back(setup.MakeAvailable().WithId(id), Criteria()); return result; }; @@ -538,7 +549,7 @@ TEST_CASE("CompositePackage_AvailableVersions_ChannelFilteredOut", "[CompositeSo std::string channel = "Channel"; CompositeTestSetup setup; - setup.Installed->Everything.Matches.emplace_back(MakeInstalled().WithPFN(pfn), Criteria()); + setup.Installed->Everything.Matches.emplace_back(setup.MakeInstalled().WithPFN(pfn), Criteria()); setup.Available->SearchFunction = [&](const SearchRequest&) { Manifest::Manifest noChannel = MakeDefaultManifest(); @@ -580,7 +591,7 @@ TEST_CASE("CompositePackage_AvailableVersions_NoChannelFilteredOut", "[Composite std::string channel = "Channel"; CompositeTestSetup setup; - setup.Installed->Everything.Matches.emplace_back(MakeInstalled().WithPFN(pfn).WithChannel(channel), Criteria()); + setup.Installed->Everything.Matches.emplace_back(setup.MakeInstalled().WithPFN(pfn).WithChannel(channel), Criteria()); setup.Available->SearchFunction = [&](const SearchRequest&) { Manifest::Manifest noChannel = MakeDefaultManifest(); @@ -628,14 +639,14 @@ TEST_CASE("CompositeSource_MultipleAvailableSources_MatchAll", "[CompositeSource std::shared_ptr secondAvailable = std::make_shared(); setup.Composite.AddAvailableSource(Source{ secondAvailable }); - setup.Installed->Everything.Matches.emplace_back(MakeInstalled().WithPFN(pfn), Criteria()); + setup.Installed->Everything.Matches.emplace_back(setup.MakeInstalled().WithPFN(pfn), Criteria()); setup.Available->SearchFunction = [&](const SearchRequest& request) { RequireSearchRequestIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); SearchResult result; - result.Matches.emplace_back(MakeAvailable(setup.Available).WithDefaultName(firstName), Criteria()); + result.Matches.emplace_back(setup.MakeAvailable().WithDefaultName(firstName), Criteria()); return result; }; @@ -644,7 +655,7 @@ TEST_CASE("CompositeSource_MultipleAvailableSources_MatchAll", "[CompositeSource RequireSearchRequestIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); SearchResult result; - result.Matches.emplace_back(MakeAvailable(secondAvailable).WithDefaultName(secondName), Criteria()); + result.Matches.emplace_back(setup.MakeAvailable(secondAvailable).WithDefaultName(secondName), Criteria()); return result; }; @@ -667,14 +678,14 @@ TEST_CASE("CompositeSource_MultipleAvailableSources_MatchSecond", "[CompositeSou std::shared_ptr secondAvailable = std::make_shared(); setup.Composite.AddAvailableSource(Source{ secondAvailable }); - setup.Installed->Everything.Matches.emplace_back(MakeInstalled().WithPFN(pfn), Criteria()); + setup.Installed->Everything.Matches.emplace_back(setup.MakeInstalled().WithPFN(pfn), Criteria()); secondAvailable->SearchFunction = [&](const SearchRequest& request) { RequireSearchRequestIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); SearchResult result; - result.Matches.emplace_back(MakeAvailable(setup.Available).WithDefaultName(secondName), Criteria()); + result.Matches.emplace_back(setup.MakeAvailable().WithDefaultName(secondName), Criteria()); return result; }; @@ -690,9 +701,9 @@ TEST_CASE("CompositeSource_MultipleAvailableSources_ReverseMatchBoth", "[Composi { std::string pfn = "sortof_apfn"; - auto installedPackage = MakeInstalled().WithPFN(pfn); - CompositeTestSetup setup; + auto installedPackage = setup.MakeInstalled().WithPFN(pfn); + std::shared_ptr secondAvailable = std::make_shared(); setup.Composite.AddAvailableSource(Source{ secondAvailable }); @@ -705,8 +716,8 @@ TEST_CASE("CompositeSource_MultipleAvailableSources_ReverseMatchBoth", "[Composi return result; }; - setup.Available->Everything.Matches.emplace_back(MakeAvailable(setup.Available).WithPFN(pfn), Criteria()); - secondAvailable->Everything.Matches.emplace_back(MakeAvailable(setup.Available).WithPFN(pfn), Criteria()); + setup.Available->Everything.Matches.emplace_back(setup.MakeAvailable().WithPFN(pfn), Criteria()); + secondAvailable->Everything.Matches.emplace_back(setup.MakeAvailable().WithPFN(pfn), Criteria()); SearchResult result = setup.Search(); @@ -719,7 +730,7 @@ TEST_CASE("CompositeSource_MultipleAvailableSources_ReverseMatchBoth", "[Composi TEST_CASE("CompositeSource_IsSame", "[CompositeSource]") { CompositeTestSetup setup; - setup.Installed->Everything.Matches.emplace_back(MakeInstalled().WithPFN("sortof_apfn"), Criteria()); + setup.Installed->Everything.Matches.emplace_back(setup.MakeInstalled().WithPFN("sortof_apfn"), Criteria()); SearchResult result1 = setup.Search(); REQUIRE(result1.Matches.size() == 1); @@ -740,7 +751,7 @@ TEST_CASE("CompositeSource_AvailableSearchFailure", "[CompositeSource]") AvailableSucceeds->SearchFunction = [&](const SearchRequest&) { SearchResult result; - result.Matches.emplace_back(MakeAvailable({}).WithPFN(pfn), Criteria()); + result.Matches.emplace_back(TestPackageHelper{ /* isInstalled */ false }.WithPFN(pfn), Criteria()); return result; }; @@ -784,8 +795,8 @@ TEST_CASE("CompositeSource_InstalledToAvailableCorrelationSearchFailure", "[Comp std::string pfn = "sortof_apfn"; CompositeTestSetup setup; - setup.Installed->Everything.Matches.emplace_back(MakeInstalled().WithPFN(pfn), Criteria()); - setup.Available->Everything.Matches.emplace_back(MakeAvailable(setup.Available).WithPFN(pfn), Criteria()); + setup.Installed->Everything.Matches.emplace_back(setup.MakeInstalled().WithPFN(pfn), Criteria()); + setup.Available->Everything.Matches.emplace_back(setup.MakeAvailable().WithPFN(pfn), Criteria()); std::shared_ptr AvailableFails = std::make_shared(); AvailableFails->SearchFunction = [&](const SearchRequest&) -> SearchResult { THROW_HR(expectedHR); }; @@ -823,7 +834,7 @@ TEST_CASE("CompositeSource_InstalledAvailableSearchFailure", "[CompositeSource]" setup.Available->SearchFunction = [&](const SearchRequest&) { SearchResult result; - result.Matches.emplace_back(MakeAvailable(setup.Available).WithPFN(pfn), Criteria()); + result.Matches.emplace_back(setup.MakeAvailable().WithPFN(pfn), Criteria()); return result; }; @@ -864,8 +875,8 @@ TEST_CASE("CompositeSource_TrackingPackageFound", "[CompositeSource]") std::string pfn = "sortof_apfn"; CompositeWithTrackingTestSetup setup; - auto installedPackage = MakeInstalled().WithPFN(pfn); - auto availablePackage = MakeAvailable(setup.Available).WithPFN(pfn).WithId(availableID).WithDefaultName(s_Everything_Query); + auto installedPackage = setup.MakeInstalled().WithPFN(pfn); + auto availablePackage = setup.MakeAvailable().WithPFN(pfn).WithId(availableID).WithDefaultName(s_Everything_Query); setup.Installed->Everything.Matches.emplace_back(installedPackage, Criteria()); setup.Installed->SearchFunction = [&](const SearchRequest& request) @@ -914,8 +925,8 @@ TEST_CASE("CompositeSource_TrackingPackageFound_MetadataPopulatedFromTracking", std::string pfn = "sortof_apfn"; CompositeWithTrackingTestSetup setup; - auto installedPackage = MakeInstalled().WithPFN(pfn); - auto availablePackage = MakeAvailable(setup.Available).WithPFN(pfn).WithId(availableID).WithDefaultName(s_Everything_Query); + auto installedPackage = setup.MakeInstalled().WithPFN(pfn); + auto availablePackage = setup.MakeAvailable().WithPFN(pfn).WithId(availableID).WithDefaultName(s_Everything_Query); setup.Installed->Everything.Matches.emplace_back(installedPackage, Criteria()); setup.Installed->SearchFunction = [&](const SearchRequest& request) @@ -975,8 +986,8 @@ TEST_CASE("CompositeSource_TrackingFound_AvailableNot", "[CompositeSource]") std::string pfn = "sortof_apfn"; CompositeWithTrackingTestSetup setup; - auto installedPackage = MakeInstalled().WithPFN(pfn); - auto availablePackage = MakeAvailable(setup.Available).WithPFN(pfn).WithId(availableID).WithDefaultName(s_Everything_Query); + auto installedPackage = setup.MakeInstalled().WithPFN(pfn); + auto availablePackage = setup.MakeAvailable().WithPFN(pfn).WithId(availableID).WithDefaultName(s_Everything_Query); setup.Installed->Everything.Matches.emplace_back(installedPackage, Criteria()); setup.Installed->SearchFunction = [&](const SearchRequest& request) @@ -1007,8 +1018,8 @@ TEST_CASE("CompositeSource_TrackingFound_AvailablePath", "[CompositeSource]") std::string availableID = "Available.ID"; std::string pfn = "sortof_apfn"; - auto installedPackage = MakeInstalled().WithPFN(pfn); - auto availablePackage = MakeAvailable(setup.Available).WithPFN(pfn).WithId(availableID).WithDefaultName(s_Everything_Query); + auto installedPackage = setup.MakeInstalled().WithPFN(pfn); + auto availablePackage = setup.MakeAvailable().WithPFN(pfn).WithId(availableID).WithDefaultName(s_Everything_Query); setup.Installed->SearchFunction = [&](const SearchRequest& request) { @@ -1049,8 +1060,8 @@ TEST_CASE("CompositeSource_TrackingFound_NotInstalled", "[CompositeSource]") std::string pfn = "sortof_apfn"; CompositeWithTrackingTestSetup setup; - auto installedPackage = MakeInstalled().WithPFN(pfn); - auto availablePackage = MakeAvailable(setup.Available).WithPFN(pfn).WithId(availableID).WithDefaultName(s_Everything_Query); + auto installedPackage = setup.MakeInstalled().WithPFN(pfn); + auto availablePackage = setup.MakeAvailable().WithPFN(pfn).WithId(availableID).WithDefaultName(s_Everything_Query); setup.Available->Everything.Matches.emplace_back(availablePackage, Criteria()); @@ -1064,7 +1075,7 @@ TEST_CASE("CompositeSource_TrackingFound_NotInstalled", "[CompositeSource]") TEST_CASE("CompositeSource_NullInstalledVersion", "[CompositeSource]") { CompositeTestSetup setup; - setup.Installed->Everything.Matches.emplace_back(MakeAvailable(setup.Available), Criteria()); + setup.Installed->Everything.Matches.emplace_back(setup.MakeAvailable(), Criteria()); // We are mostly testing to see if a null installed version causes an AV or not SearchResult result = setup.Search(); @@ -1074,7 +1085,7 @@ TEST_CASE("CompositeSource_NullInstalledVersion", "[CompositeSource]") TEST_CASE("CompositeSource_NullAvailableVersion", "[CompositeSource]") { CompositeTestSetup setup{ CompositeSearchBehavior::AvailablePackages }; - setup.Available->Everything.Matches.emplace_back(MakeInstalled(), Criteria()); + setup.Available->Everything.Matches.emplace_back(setup.MakeInstalled(), Criteria()); // We are mostly testing to see if a null available version causes an AV or not REQUIRE_THROWS_HR(setup.Search(), E_UNEXPECTED); @@ -1156,7 +1167,7 @@ TEST_CASE("CompositeSource_Pinning_AvailableVersionPinned", "[CompositeSource][P TestUserSettings userSettings; CompositeTestSetup setup; - auto installedPackage = TestCompositePackage::Make(MakeDefaultManifest("1.0.1"sv), TestCompositePackage::MetadataMap{}); + auto installedPackage = setup.MakeInstalled().WithVersion("1.0.1"sv); setup.Installed->Everything.Matches.emplace_back(installedPackage, Criteria()); setup.Available->SearchFunction = [&](const SearchRequest&) @@ -1267,7 +1278,7 @@ TEST_CASE("CompositeSource_Pinning_OneSourcePinned", "[CompositeSource][PinFlow] TestUserSettings userSettings; CompositeTestSetup setup; - auto installedPackage = TestCompositePackage::Make(MakeDefaultManifest("1.0"sv), TestCompositePackage::MetadataMap{}); + auto installedPackage = setup.MakeInstalled().WithVersion("1.0"sv); setup.Installed->Everything.Matches.emplace_back(installedPackage, Criteria()); setup.Available->SearchFunction = [&](const SearchRequest&) @@ -1323,7 +1334,7 @@ TEST_CASE("CompositeSource_Pinning_OneSourceGated", "[CompositeSource][PinFlow]" TestUserSettings userSettings; CompositeTestSetup setup; - auto installedPackage = TestCompositePackage::Make(MakeDefaultManifest("1.0"sv), TestCompositePackage::MetadataMap{}); + auto installedPackage = setup.MakeInstalled().WithVersion("1.0.1"sv); setup.Installed->Everything.Matches.emplace_back(installedPackage, Criteria()); setup.Available->SearchFunction = [&](const SearchRequest&) @@ -1389,11 +1400,12 @@ TEST_CASE("CompositeSource_Pinning_MultipleInstalled", "[CompositeSource][PinFlo std::string productCode1 = "product-code1"; std::string productCode2 = "product-code2"; + CompositeTestSetup setup; + // Installed packages differ in product code and version - auto installedPackage1 = MakeInstalled().WithId(productCode1).WithPC(productCode1).WithVersion("1.1"sv); - auto installedPackage2 = MakeInstalled().WithId(productCode2).WithPC(productCode2).WithVersion("1.2"sv); + auto installedPackage1 = setup.MakeInstalled().WithId(productCode1).WithPC(productCode1).WithVersion("1.1"sv); + auto installedPackage2 = setup.MakeInstalled().WithId(productCode2).WithPC(productCode2).WithVersion("1.2"sv); - CompositeTestSetup setup; setup.Installed->SearchFunction = [&](const SearchRequest& request) { bool isSearchById = SearchRequestIncludes(request.Inclusions, PackageMatchField::Id, MatchType::Exact, packageId); @@ -1416,7 +1428,7 @@ TEST_CASE("CompositeSource_Pinning_MultipleInstalled", "[CompositeSource][PinFlo setup.Available->SearchFunction = [&](const SearchRequest&) { SearchResult result; - result.Matches.emplace_back(MakeAvailable(setup.Available).WithId(packageId).WithVersion("2.0"sv), Criteria()); + result.Matches.emplace_back(setup.MakeAvailable().WithId(packageId).WithVersion("2.0"sv), Criteria()); return result; }; @@ -1520,7 +1532,7 @@ TEST_CASE("CompositeSource_CorrelateToInstalledContainsManifestData", "[Composit setup.Available->SearchFunction = [&](const SearchRequest&) { SearchResult result; - result.Matches.emplace_back(MakeAvailable(setup.Available).WithPC("hello"), Criteria()); + result.Matches.emplace_back(setup.MakeAvailable().WithPC("hello"), Criteria()); return result; }; @@ -1551,7 +1563,7 @@ TEST_CASE("CompositeSource_Respects_FeatureFlag_ManifestMayContainAdditionalSyst setup.Available->SearchFunction = [&](const SearchRequest&) { SearchResult result; - result.Matches.emplace_back(MakeAvailable(setup.Available).WithId(id).WithPC(productCode1).HideSRS(), Criteria()); + result.Matches.emplace_back(setup.MakeAvailable().WithId(id).WithPC(productCode1).HideSRS(), Criteria()); return result; }; diff --git a/src/AppInstallerCLITests/PredefinedInstalledSource.cpp b/src/AppInstallerCLITests/PredefinedInstalledSource.cpp index 23efc240bf..1a5c1ec942 100644 --- a/src/AppInstallerCLITests/PredefinedInstalledSource.cpp +++ b/src/AppInstallerCLITests/PredefinedInstalledSource.cpp @@ -147,6 +147,11 @@ std::shared_ptr CreatePredefinedInstalledSource(Factory::Filter filter return factory->Create(details)->Open(progress); } +SQLiteIndex CreateMemoryIndex() +{ + return SQLiteIndex::CreateNew(SQLITE_MEMORY_DB_CONNECTION_TARGET, SQLite::Version::Latest(), SQLiteIndex::CreateOptions::SupportPathless); +} + TEST_CASE("ARPHelper_GetARPForArchitecture", "[arphelper][list]") { auto systemArch = GetSystemArchitecture(); @@ -296,7 +301,7 @@ TEST_CASE("ARPHelper_PopulateIndexFromKey_Single", "[arphelper][list]") AddARPEntryToKey(root.get(), helper, entry); - auto index = SQLiteIndex::CreateNew(SQLITE_MEMORY_DB_CONNECTION_TARGET); + auto index = CreateMemoryIndex(); helper.PopulateIndexFromKey(index, key, s_TestScope, "TestArchitecture"); auto result = index.Search({}); @@ -332,7 +337,7 @@ TEST_CASE("ARPHelper_PopulateIndexFromKey_SingleValid", "[arphelper][list]") { "Nothing" }, }); - auto index = SQLiteIndex::CreateNew(SQLITE_MEMORY_DB_CONNECTION_TARGET); + auto index = CreateMemoryIndex(); helper.PopulateIndexFromKey(index, key, s_TestScope, "TestArchitecture"); auto result = index.Search({}); @@ -368,7 +373,7 @@ TEST_CASE("ARPHelper_PopulateIndexFromKey_Two", "[arphelper][list]") AddARPEntryToKey(root.get(), helper, entry1); AddARPEntryToKey(root.get(), helper, entry2); - auto index = SQLiteIndex::CreateNew(SQLITE_MEMORY_DB_CONNECTION_TARGET); + auto index = CreateMemoryIndex(); helper.PopulateIndexFromKey(index, key, s_TestScope, "TestArchitecture"); REQUIRE(index.Search({}).Matches.size() == 2); diff --git a/src/AppInstallerRepositoryCore/CompositeSource.cpp b/src/AppInstallerRepositoryCore/CompositeSource.cpp index 279328108f..7fae371c9c 100644 --- a/src/AppInstallerRepositoryCore/CompositeSource.cpp +++ b/src/AppInstallerRepositoryCore/CompositeSource.cpp @@ -350,6 +350,8 @@ namespace AppInstaller::Repository Utility::LocIndString GetProperty(PackageProperty property) const override { + THROW_HR_IF(E_UNEXPECTED, m_packages.empty() || m_versionKeyData.empty()); + // Use the highest version for package properties return m_packages[m_versionKeyData[0].PackageIndex]->GetProperty(property); } @@ -520,6 +522,11 @@ namespace AppInstaller::Repository } } + bool IsEmpty() const + { + return m_versionKeyData.empty(); + } + private: // Contains information about all of the version keys. // We use the `SourceId` field to store the installed package identifier so that we can disambiguate keys is they have the same version. @@ -558,6 +565,12 @@ namespace AppInstaller::Repository keyData.PackageIndex = packageIndex; keyData.InstalledVersion = package->GetVersion(versionKey); + if (!keyData.InstalledVersion) + { + AICLI_LOG(Repo, Verbose, << "AddPackageAndVersionKeyData: Package [" << packageIdentifier << "] did not return a version for [" << versionKey.Version << "]"); + continue; + } + // We use the `SourceId` field to store the installed package identifier so that we can disambiguate keys is they have the same version. keyData.SourceId = packageIdentifier; @@ -594,6 +607,12 @@ namespace AppInstaller::Repository if (installedPackage) { m_installedPackage = std::make_shared(installedPackage->GetInstalled()); + + // If the installed package result existed, but didn't actually create any installed versions, drop it. + if (m_installedPackage->IsEmpty()) + { + m_installedPackage.reset(); + } } AddAvailablePackage(availablePackage, setPrimary); @@ -1685,7 +1704,13 @@ namespace AppInstaller::Repository { // TODO: Needs a whole separate change to fix the fact that we don't support multiple available packages and what the different search behaviors mean foundInstalledMatch = true; - result.Matches.emplace_back(std::make_shared(installedPackage, std::move(match.Package)), match.MatchCriteria); + auto compositePackage = std::make_shared(installedPackage, std::move(match.Package)); + if (trackingPackage) + { + auto trackingPackageTime = GetLatestTrackingWriteTime(trackingPackage); + compositePackage->SetTracking(source, std::move(trackingPackage), trackingPackageTime); + } + result.Matches.emplace_back(std::move(compositePackage), match.MatchCriteria); } } } From 8795687c0862f4394667e465f07fc76ea14a2c8f Mon Sep 17 00:00:00 2001 From: John McPherson Date: Fri, 15 Mar 2024 10:31:16 -0700 Subject: [PATCH 21/24] Add unit tests --- src/AppInstallerCLITests/CompositeSource.cpp | 228 ++++++++++++++++++- src/AppInstallerCLITests/TestCommon.cpp | 17 ++ src/AppInstallerCLITests/TestCommon.h | 4 + src/AppInstallerCLITests/TestSource.cpp | 28 +-- src/AppInstallerCLITests/TestSource.h | 1 + 5 files changed, 264 insertions(+), 14 deletions(-) diff --git a/src/AppInstallerCLITests/CompositeSource.cpp b/src/AppInstallerCLITests/CompositeSource.cpp index 1617b79239..d1c464a8fc 100644 --- a/src/AppInstallerCLITests/CompositeSource.cpp +++ b/src/AppInstallerCLITests/CompositeSource.cpp @@ -120,7 +120,7 @@ struct TestPackageHelper return *this; } - operator std::shared_ptr() + std::shared_ptr ToPackage() { if (!m_package) { @@ -137,6 +137,11 @@ struct TestPackageHelper return m_package; } + operator std::shared_ptr() + { + return ToPackage(); + } + operator const Manifest::Manifest& () const { return m_manifest; @@ -1589,3 +1594,224 @@ TEST_CASE("CompositeSource_Respects_FeatureFlag_ManifestMayContainAdditionalSyst REQUIRE(productCodeSearched); } } + +TEST_CASE("CompositeSource_SxS_TwoVersions_NoAvailable", "[CompositeSource][SideBySide]") +{ + auto enableFeature = TestUserSettings::EnableExperimentalFeature(Settings::ExperimentalFeature::Feature::SideBySide); + + std::string productCode1 = "PC1"; + std::string productCode2 = "PC2"; + + CompositeTestSetup setup; + auto availablePackage = setup.MakeAvailable(); + + setup.Installed->Everything.Matches.emplace_back(setup.MakeInstalled().WithVersion("1.0").WithPC(productCode1), Criteria()); + setup.Installed->Everything.Matches.emplace_back(setup.MakeInstalled().WithVersion("2.0").WithPC(productCode2), Criteria()); + + SearchResult result = setup.Search(); + + REQUIRE(result.Matches.size() == 2); +} + +TEST_CASE("CompositeSource_SxS_TwoVersions_DifferentAvailable", "[CompositeSource][SideBySide]") +{ + auto enableFeature = TestUserSettings::EnableExperimentalFeature(Settings::ExperimentalFeature::Feature::SideBySide); + + std::string productCode1 = "PC1"; + std::string productCode2 = "PC2"; + + CompositeTestSetup setup; + auto availablePackage1 = setup.MakeAvailable().ToPackage(); + auto availablePackage2 = setup.MakeAvailable().ToPackage(); + + setup.Installed->Everything.Matches.emplace_back(setup.MakeInstalled().WithVersion("1.0").WithPC(productCode1), Criteria()); + setup.Installed->Everything.Matches.emplace_back(setup.MakeInstalled().WithVersion("2.0").WithPC(productCode2), Criteria()); + + setup.Available->SearchFunction = [&](const SearchRequest& request) + { + SearchResult result; + + std::string productCode; + for (const auto& item : request.Inclusions) + { + if (item.Field == PackageMatchField::ProductCode) + { + productCode = item.Value; + break; + } + } + + if (productCode == productCode1) + { + result.Matches.emplace_back(availablePackage1, Criteria()); + } + else if (productCode == productCode1) + { + result.Matches.emplace_back(availablePackage2, Criteria()); + } + + return result; + }; + + SearchResult result = setup.Search(); + + REQUIRE(result.Matches.size() == 2); +} + +TEST_CASE("CompositeSource_SxS_TwoVersions_SameAvailable", "[CompositeSource][SideBySide]") +{ + auto enableFeature = TestUserSettings::EnableExperimentalFeature(Settings::ExperimentalFeature::Feature::SideBySide); + + std::string version1 = "1.0"; + std::string version2 = "2.0"; + std::string productCode1 = "PC1"; + std::string productCode2 = "PC2"; + + CompositeTestSetup setup; + auto availablePackage = setup.MakeAvailable().ToPackage(); + + setup.Installed->Everything.Matches.emplace_back(setup.MakeInstalled().WithVersion(version1).WithPC(productCode1), Criteria()); + setup.Installed->Everything.Matches.emplace_back(setup.MakeInstalled().WithVersion(version2).WithPC(productCode2), Criteria()); + + setup.Available->SearchFunction = [&](const SearchRequest&) + { + SearchResult result; + result.Matches.emplace_back(availablePackage, Criteria()); + return result; + }; + + SearchResult result = setup.Search(); + + REQUIRE(result.Matches.size() == 1); + auto package = result.Matches[0].Package; + REQUIRE(package); + auto installedPackage = package->GetInstalled(); + REQUIRE(installedPackage); + auto installedVersions = installedPackage->GetVersionKeys(); + REQUIRE(installedVersions.size() == 2); + REQUIRE(std::any_of(installedVersions.begin(), installedVersions.end(), [&](const PackageVersionKey& key) { return key.Version == version1; })); + REQUIRE(std::any_of(installedVersions.begin(), installedVersions.end(), [&](const PackageVersionKey& key) { return key.Version == version2; })); + auto availablePackages = package->GetAvailable(); + REQUIRE(availablePackages.size() == 1); + REQUIRE(availablePackages[0]->IsSame(availablePackage->Available[0].get())); +} + +TEST_CASE("CompositeSource_SxS_ThreeVersions_SameAvailable", "[CompositeSource][SideBySide]") +{ + auto enableFeature = TestUserSettings::EnableExperimentalFeature(Settings::ExperimentalFeature::Feature::SideBySide); + + std::string version1 = "1.0"; + std::string version2 = "2.0"; + std::string version3 = "3.0"; + std::string productCode1 = "PC1"; + std::string productCode2 = "PC2"; + std::string productCode3 = "PC3"; + + CompositeTestSetup setup; + auto availablePackage = setup.MakeAvailable().ToPackage(); + + setup.Installed->Everything.Matches.emplace_back(setup.MakeInstalled().WithVersion(version1).WithPC(productCode1), Criteria()); + setup.Installed->Everything.Matches.emplace_back(setup.MakeInstalled().WithVersion(version2).WithPC(productCode2), Criteria()); + setup.Installed->Everything.Matches.emplace_back(setup.MakeInstalled().WithVersion(version3).WithPC(productCode3), Criteria()); + + setup.Available->SearchFunction = [&](const SearchRequest&) + { + SearchResult result; + result.Matches.emplace_back(availablePackage, Criteria()); + return result; + }; + + SearchResult result = setup.Search(); + + REQUIRE(result.Matches.size() == 1); + auto package = result.Matches[0].Package; + REQUIRE(package); + auto installedPackage = package->GetInstalled(); + REQUIRE(installedPackage); + auto installedVersions = installedPackage->GetVersionKeys(); + REQUIRE(installedVersions.size() == 3); + REQUIRE(std::any_of(installedVersions.begin(), installedVersions.end(), [&](const PackageVersionKey& key) { return key.Version == version1; })); + REQUIRE(std::any_of(installedVersions.begin(), installedVersions.end(), [&](const PackageVersionKey& key) { return key.Version == version2; })); + REQUIRE(std::any_of(installedVersions.begin(), installedVersions.end(), [&](const PackageVersionKey& key) { return key.Version == version3; })); + auto availablePackages = package->GetAvailable(); + REQUIRE(availablePackages.size() == 1); + REQUIRE(availablePackages[0]->IsSame(availablePackage->Available[0].get())); +} + +TEST_CASE("CompositeSource_SxS_TwoVersions_SameAvailable_Tracking", "[CompositeSource][SideBySide]") +{ + auto enableFeature = TestUserSettings::EnableExperimentalFeature(Settings::ExperimentalFeature::Feature::SideBySide); + + std::string version1 = "1.0"; + std::string version2 = "2.0"; + std::string productCode1 = "PC1"; + std::string productCode2 = "PC2"; + + CompositeWithTrackingTestSetup setup; + auto installedPackage1 = setup.MakeInstalled().WithVersion(version1).WithPC(productCode1); + auto availablePackage = setup.MakeAvailable().ToPackage(); + + setup.Installed->Everything.Matches.emplace_back(installedPackage1, Criteria()); + setup.Installed->Everything.Matches.emplace_back(setup.MakeInstalled().WithVersion(version2).WithPC(productCode2), Criteria()); + setup.Tracking->GetIndex().AddManifest(installedPackage1); + + setup.Available->SearchFunction = [&](const SearchRequest&) + { + SearchResult result; + result.Matches.emplace_back(availablePackage, Criteria()); + return result; + }; + + SearchResult result = setup.Search(); + + REQUIRE(result.Matches.size() == 1); + auto package = result.Matches[0].Package; + REQUIRE(package); + auto installedPackage = package->GetInstalled(); + REQUIRE(installedPackage); + auto installedVersions = installedPackage->GetVersionKeys(); + REQUIRE(installedVersions.size() == 2); + REQUIRE(std::any_of(installedVersions.begin(), installedVersions.end(), [&](const PackageVersionKey& key) { return key.Version == version1; })); + REQUIRE(std::any_of(installedVersions.begin(), installedVersions.end(), [&](const PackageVersionKey& key) { return key.Version == version2; })); + auto availablePackages = package->GetAvailable(); + REQUIRE(availablePackages.size() == 1); + REQUIRE(availablePackages[0]->IsSame(availablePackage->Available[0].get())); +} + +TEST_CASE("CompositeSource_SxS_Available_TwoVersions_SameAvailable", "[CompositeSource][SideBySide]") +{ + auto enableFeature = TestUserSettings::EnableExperimentalFeature(Settings::ExperimentalFeature::Feature::SideBySide); + + std::string version1 = "1.0"; + std::string version2 = "2.0"; + std::string productCode1 = "PC1"; + std::string productCode2 = "PC2"; + + CompositeTestSetup setup; + auto availablePackage = setup.MakeAvailable().ToPackage(); + + setup.Installed->SearchFunction = [&](const SearchRequest&) + { + SearchResult result; + result.Matches.emplace_back(setup.MakeInstalled().WithVersion(version1).WithPC(productCode1), Criteria()); + result.Matches.emplace_back(setup.MakeInstalled().WithVersion(version2).WithPC(productCode2), Criteria()); + return result; + }; + + setup.Available->Everything.Matches.emplace_back(availablePackage, Criteria()); + + SearchResult result = setup.Search(); + + REQUIRE(result.Matches.size() == 1); + auto package = result.Matches[0].Package; + REQUIRE(package); + auto installedPackage = package->GetInstalled(); + REQUIRE(installedPackage); + auto installedVersions = installedPackage->GetVersionKeys(); + REQUIRE(installedVersions.size() == 2); + REQUIRE(std::any_of(installedVersions.begin(), installedVersions.end(), [&](const PackageVersionKey& key) { return key.Version == version1; })); + REQUIRE(std::any_of(installedVersions.begin(), installedVersions.end(), [&](const PackageVersionKey& key) { return key.Version == version2; })); + auto availablePackages = package->GetAvailable(); + REQUIRE(availablePackages.size() == 1); + REQUIRE(availablePackages[0]->IsSame(availablePackage->Available[0].get())); +} diff --git a/src/AppInstallerCLITests/TestCommon.cpp b/src/AppInstallerCLITests/TestCommon.cpp index 1364f49e4b..64bf580cc5 100644 --- a/src/AppInstallerCLITests/TestCommon.cpp +++ b/src/AppInstallerCLITests/TestCommon.cpp @@ -249,6 +249,23 @@ namespace TestCommon AppInstaller::Settings::SetUserSettingsOverride(nullptr); } + std::unique_ptr TestUserSettings::EnableExperimentalFeature(Settings::ExperimentalFeature::Feature feature, bool keepFileSettings) + { + std::unique_ptr result = std::make_unique(keepFileSettings); + + // Due to the template usage, this needs to be updated for any features that want to use it. + switch (feature) + { + case Settings::ExperimentalFeature::Feature::SideBySide: + result->Set(true); + break; + default: + THROW_HR(E_NOTIMPL); + } + + return result; + } + bool InstallCertFromSignedPackage(const std::filesystem::path& package) { auto [certContext, certStore] = AppInstaller::Msix::GetCertContextFromMsix(package); diff --git a/src/AppInstallerCLITests/TestCommon.h b/src/AppInstallerCLITests/TestCommon.h index 0d3e699767..a6f844cf10 100644 --- a/src/AppInstallerCLITests/TestCommon.h +++ b/src/AppInstallerCLITests/TestCommon.h @@ -5,10 +5,12 @@ #include #include #include +#include #include #include #include +#include #include #define REQUIRE_THROWS_HR(_expr_, _hr_) REQUIRE_THROWS_MATCHES(_expr_, wil::ResultException, ::TestCommon::ResultExceptionHRMatcher(_hr_)) @@ -141,6 +143,8 @@ namespace TestCommon { m_settings[S].emplace(std::move(value)); } + + static std::unique_ptr EnableExperimentalFeature(AppInstaller::Settings::ExperimentalFeature::Feature feature, bool keepFileSettings = false); }; // Below cert installation/uninstallation methods require admin privilege, diff --git a/src/AppInstallerCLITests/TestSource.cpp b/src/AppInstallerCLITests/TestSource.cpp index 06050b6402..8999f1a24f 100644 --- a/src/AppInstallerCLITests/TestSource.cpp +++ b/src/AppInstallerCLITests/TestSource.cpp @@ -9,6 +9,15 @@ using namespace AppInstaller::Repository; namespace TestCommon { + namespace + { + size_t GetNextTestPackageId() + { + static std::atomic_size_t packageId(0); + return ++packageId; + } + } + TestPackageVersion::TestPackageVersion(const Manifest& manifest, MetadataMap installationMetadata, std::weak_ptr source) : VersionManifest(manifest), Metadata(std::move(installationMetadata)), Source(source) {} @@ -120,6 +129,7 @@ namespace TestCommon TestPackage::TestPackage(const std::vector& available, std::weak_ptr source, bool hideSystemReferenceStringsOnVersion) : Source(source) { + DefaultIsSameIdentity = GetNextTestPackageId(); for (const auto& manifest : available) { Versions.emplace_back(TestPackageVersion::Make(manifest, source, hideSystemReferenceStringsOnVersion)); @@ -129,6 +139,7 @@ namespace TestCommon TestPackage::TestPackage(const Manifest& installed, MetadataMap installationMetadata, std::weak_ptr source) : Source(source) { + DefaultIsSameIdentity = GetNextTestPackageId(); Versions.emplace_back(TestPackageVersion::Make(installed, std::move(installationMetadata), source)); } @@ -203,23 +214,14 @@ namespace TestCommon return IsSameOverride(this, other); } - const TestPackage* otherAvailable = PackageCast(other); + const TestPackage* otherPackage = PackageCast(other); - if (!otherAvailable || - Versions.size() != otherAvailable->Versions.size()) + if (otherPackage && DefaultIsSameIdentity == otherPackage->DefaultIsSameIdentity) { - return false; + return true; } - for (size_t i = 0; i < Versions.size(); ++i) - { - if (Versions[i].get() != otherAvailable->Versions[i].get()) - { - return false; - } - } - - return true; + return false; } const void* TestPackage::CastTo(IPackageType type) const diff --git a/src/AppInstallerCLITests/TestSource.h b/src/AppInstallerCLITests/TestSource.h index f86789f0ab..27033c45bf 100644 --- a/src/AppInstallerCLITests/TestSource.h +++ b/src/AppInstallerCLITests/TestSource.h @@ -74,6 +74,7 @@ namespace TestCommon std::vector> Versions; std::weak_ptr Source; + size_t DefaultIsSameIdentity = 0; std::function IsSameOverride; }; From 25c0151b106f0d343a5db3fe6e016b1bf14908e1 Mon Sep 17 00:00:00 2001 From: John McPherson Date: Tue, 19 Mar 2024 10:03:22 -0700 Subject: [PATCH 22/24] Fix tests --- .../Helpers/TestCommon.cs | 2 +- src/AppInstallerCLIE2ETests/ListCommand.cs | 16 +++++++++------- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/AppInstallerCLIE2ETests/Helpers/TestCommon.cs b/src/AppInstallerCLIE2ETests/Helpers/TestCommon.cs index 4c068b9579..1071986a90 100644 --- a/src/AppInstallerCLIE2ETests/Helpers/TestCommon.cs +++ b/src/AppInstallerCLIE2ETests/Helpers/TestCommon.cs @@ -1,4 +1,4 @@ -// ----------------------------------------------------------------------------- +// ----------------------------------------------------------------------------- // // Copyright (c) Microsoft Corporation. Licensed under the MIT License. // diff --git a/src/AppInstallerCLIE2ETests/ListCommand.cs b/src/AppInstallerCLIE2ETests/ListCommand.cs index 52bc0c528e..498a8c45de 100644 --- a/src/AppInstallerCLIE2ETests/ListCommand.cs +++ b/src/AppInstallerCLIE2ETests/ListCommand.cs @@ -1,4 +1,4 @@ -// ----------------------------------------------------------------------------- +// ----------------------------------------------------------------------------- // // Copyright (c) Microsoft Corporation. Licensed under the MIT License. // @@ -105,20 +105,21 @@ public void ListWithUpgradeCode() public void ListWithScopeExeInstalledAsMachine() { System.Guid guid = System.Guid.NewGuid(); + string identifier = "AppInstallerTest.TestExeInstaller"; string productCode = guid.ToString(); var installDir = TestCommon.GetRandomTestDir(); - var result = TestCommon.RunAICLICommand("install", $"AppInstallerTest.TestExeInstaller --override \"/InstallDir {installDir} /ProductID {productCode} /UseHKLM\""); + var result = TestCommon.RunAICLICommand("install", $"{identifier} --override \"/InstallDir {installDir} /ProductID {productCode} /UseHKLM\""); Assert.AreEqual(Constants.ErrorCode.S_OK, result.ExitCode); // List with user scope will not find the package result = TestCommon.RunAICLICommand("list", $"{productCode} --scope user"); Assert.AreEqual(Constants.ErrorCode.ERROR_NO_APPLICATIONS_FOUND, result.ExitCode); - Assert.False(result.StdOut.Contains(productCode)); + Assert.False(result.StdOut.Contains(identifier)); // List with machine scope will find the package result = TestCommon.RunAICLICommand("list", $"{productCode} --scope machine"); Assert.AreEqual(Constants.ErrorCode.S_OK, result.ExitCode); - Assert.True(result.StdOut.Contains(productCode)); + Assert.True(result.StdOut.Contains(identifier)); TestCommon.RunCommand(Path.Combine(installDir, Constants.TestExeUninstallerFileName)); } @@ -130,20 +131,21 @@ public void ListWithScopeExeInstalledAsMachine() public void ListWithScopeExeInstalledAsUser() { System.Guid guid = System.Guid.NewGuid(); + string identifier = "AppInstallerTest.TestExeInstaller"; string productCode = guid.ToString(); var installDir = TestCommon.GetRandomTestDir(); - var result = TestCommon.RunAICLICommand("install", $"AppInstallerTest.TestExeInstaller --override \"/InstallDir {installDir} /ProductID {productCode}\""); + var result = TestCommon.RunAICLICommand("install", $"{identifier} --override \"/InstallDir {installDir} /ProductID {productCode}\""); Assert.AreEqual(Constants.ErrorCode.S_OK, result.ExitCode); // List with user scope will find the package result = TestCommon.RunAICLICommand("list", $"{productCode} --scope user"); Assert.AreEqual(Constants.ErrorCode.S_OK, result.ExitCode); - Assert.True(result.StdOut.Contains(productCode)); + Assert.True(result.StdOut.Contains(identifier)); // List with machine scope will not find the package result = TestCommon.RunAICLICommand("list", $"{productCode} --scope machine"); Assert.AreEqual(Constants.ErrorCode.ERROR_NO_APPLICATIONS_FOUND, result.ExitCode); - Assert.False(result.StdOut.Contains(productCode)); + Assert.False(result.StdOut.Contains(identifier)); TestCommon.RunCommand(Path.Combine(installDir, Constants.TestExeUninstallerFileName)); } From eae34b420af291b44f4c53a2f410d0c7cf8258a4 Mon Sep 17 00:00:00 2001 From: JohnMcPMS Date: Fri, 22 Mar 2024 10:33:54 -0700 Subject: [PATCH 23/24] PR feedback --- src/AppInstallerCLI.sln | 3 --- src/AppInstallerRepositoryCore/CompositeSource.cpp | 12 ++++++++---- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/AppInstallerCLI.sln b/src/AppInstallerCLI.sln index a3d189f72d..c5d8378edb 100644 --- a/src/AppInstallerCLI.sln +++ b/src/AppInstallerCLI.sln @@ -181,9 +181,6 @@ EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.WinGet.SharedLib", "PowerShell\Microsoft.WinGet.SharedLib\Microsoft.WinGet.SharedLib.csproj", "{272B2B0E-40D4-4F0F-B187-519A6EF89B10}" EndProject Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "schema", "schema", "{46E419FD-C77F-4EFC-A0FC-2BAD93E60D12}" - ProjectSection(SolutionItems) = preProject - ..\schemas\JSON\settings\settings.schema.0.2.json = ..\schemas\JSON\settings\settings.schema.0.2.json - EndProjectSection EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution diff --git a/src/AppInstallerRepositoryCore/CompositeSource.cpp b/src/AppInstallerRepositoryCore/CompositeSource.cpp index 7fae371c9c..11ae60ecff 100644 --- a/src/AppInstallerRepositoryCore/CompositeSource.cpp +++ b/src/AppInstallerRepositoryCore/CompositeSource.cpp @@ -338,7 +338,6 @@ namespace AppInstaller::Repository }; // An IPackage for the installed package of a CompositePackage. - // Supports only a single version of a single package at this time. struct CompositeInstalledPackage : public IPackage { static constexpr IPackageType PackageType = IPackageType::CompositeInstalledPackage; @@ -557,6 +556,7 @@ namespace AppInstaller::Repository size_t packageIndex = m_packages.size(); std::string packageIdentifier = package->GetProperty(PackageProperty::Id); + bool versionAdded = false; for (const auto& versionKey : package->GetVersionKeys()) { @@ -571,7 +571,7 @@ namespace AppInstaller::Repository continue; } - // We use the `SourceId` field to store the installed package identifier so that we can disambiguate keys is they have the same version. + // We use the `SourceId` field to store the installed package identifier so that we can disambiguate keys if they have the same version. keyData.SourceId = packageIdentifier; keyData.InstalledType = Manifest::ConvertToInstallerTypeEnum(keyData.InstalledVersion->GetMetadata()[PackageVersionMetadata::InstalledType]); @@ -583,11 +583,15 @@ namespace AppInstaller::Repository keyData.VersionAndChannel = Utility::VersionAndChannel{ keyData.Version, keyData.Channel }; m_versionKeyData.emplace_back(std::move(keyData)); + versionAdded = true; } - m_packages.emplace_back(std::move(package)); + if (versionAdded) + { + m_packages.emplace_back(std::move(package)); - std::sort(m_versionKeyData.begin(), m_versionKeyData.end()); + std::sort(m_versionKeyData.begin(), m_versionKeyData.end()); + } } std::vector> m_packages; From fc4598b098ad601da2611a9ffe7edb57a9b3065e Mon Sep 17 00:00:00 2001 From: JohnMcPMS Date: Fri, 22 Mar 2024 10:39:01 -0700 Subject: [PATCH 24/24] Revert solution changes --- src/AppInstallerCLI.sln | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/AppInstallerCLI.sln b/src/AppInstallerCLI.sln index c5d8378edb..b0517698c8 100644 --- a/src/AppInstallerCLI.sln +++ b/src/AppInstallerCLI.sln @@ -180,7 +180,13 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "WinGetSourceCreator", "WinG EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.WinGet.SharedLib", "PowerShell\Microsoft.WinGet.SharedLib\Microsoft.WinGet.SharedLib.csproj", "{272B2B0E-40D4-4F0F-B187-519A6EF89B10}" EndProject -Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "schema", "schema", "{46E419FD-C77F-4EFC-A0FC-2BAD93E60D12}" +Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Tests", "Tests", "{5A52D9FC-0059-4A4A-8196-427A7AA0D1C5}" + ProjectSection(SolutionItems) = preProject + PowerShell\tests\Microsoft.WinGet.Client.Tests.ps1 = PowerShell\tests\Microsoft.WinGet.Client.Tests.ps1 + PowerShell\tests\Microsoft.WinGet.Configuration.Tests.ps1 = PowerShell\tests\Microsoft.WinGet.Configuration.Tests.ps1 + PowerShell\tests\Microsoft.WinGet.DSC.Tests.ps1 = PowerShell\tests\Microsoft.WinGet.DSC.Tests.ps1 + PowerShell\tests\RunTests.ps1 = PowerShell\tests\RunTests.ps1 + EndProjectSection EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution @@ -1386,8 +1392,8 @@ Global {1622DA16-914F-4F57-A259-D5169003CC8C} = {6D7776A8-42FE-46DD-B0F8-712F35EA0C79} {3BAF989F-7F65-465B-ACE8-BAFE42D1017E} = {EA8CD934-0702-4911-A2C5-A40600E616DE} {358BC478-0624-4AD1-A933-0422B5292AF8} = {60618CAC-2995-4DF9-9914-45C6FC02C995} - {7D05F64D-CE5A-42AA-A2C1-E91458F061CF} = {46E419FD-C77F-4EFC-A0FC-2BAD93E60D12} - {952B513F-8A00-4D74-9271-925AFB3C6252} = {46E419FD-C77F-4EFC-A0FC-2BAD93E60D12} + {7D05F64D-CE5A-42AA-A2C1-E91458F061CF} = {8D53D749-D51C-46F8-A162-9371AAA6C2E7} + {952B513F-8A00-4D74-9271-925AFB3C6252} = {8D53D749-D51C-46F8-A162-9371AAA6C2E7} {2ACDE176-F13F-42FA-8159-C34FA3D37837} = {8D53D749-D51C-46F8-A162-9371AAA6C2E7} {866C3F06-636F-4BE8-BC24-5F86ECC606A1} = {60618CAC-2995-4DF9-9914-45C6FC02C995} {1A47951F-5C7A-4D6D-BB5F-D77484437940} = {8D53D749-D51C-46F8-A162-9371AAA6C2E7} @@ -1402,7 +1408,7 @@ Global {167F634B-A3AD-494E-8E67-B888103E35FF} = {7C218A3E-9BC8-48FF-B91B-BCACD828C0C9} {C54F80ED-B736-49B0-9BD3-662F57024D01} = {7C218A3E-9BC8-48FF-B91B-BCACD828C0C9} {272B2B0E-40D4-4F0F-B187-519A6EF89B10} = {7C218A3E-9BC8-48FF-B91B-BCACD828C0C9} - {46E419FD-C77F-4EFC-A0FC-2BAD93E60D12} = {8D53D749-D51C-46F8-A162-9371AAA6C2E7} + {5A52D9FC-0059-4A4A-8196-427A7AA0D1C5} = {7C218A3E-9BC8-48FF-B91B-BCACD828C0C9} EndGlobalSection GlobalSection(ExtensibilityGlobals) = postSolution SolutionGuid = {B6FDB70C-A751-422C-ACD1-E35419495857}