From ffab684f4d893e4d4f0bed0d9faa8f9929aa8408 Mon Sep 17 00:00:00 2001 From: JohnMcPMS Date: Wed, 19 Jun 2024 11:34:46 -0700 Subject: [PATCH] Make SxS stable (#4563) ## Change Makes the side by side work stable and fixes a few tests that failed after. These tests simply needed to be updated; no product code was changed to resolve their issues. --- .github/actions/spelling/patterns.txt | 1 + doc/Settings.md | 11 -- .../JSON/settings/settings.schema.0.2.json | 5 - src/AppInstallerCLICore/Argument.cpp | 2 +- .../Workflows/RepairFlow.cpp | 1 + .../Workflows/WorkflowBase.cpp | 39 ++--- src/AppInstallerCLIE2ETests/Constants.cs | 1 + src/AppInstallerCLIE2ETests/ListCommand.cs | 3 +- src/AppInstallerCLITests/CompositeSource.cpp | 65 ++++---- src/AppInstallerCLITests/TestCommon.cpp | 3 - .../ExperimentalFeature.cpp | 4 - .../Public/winget/ExperimentalFeature.h | 9 +- .../Public/winget/UserSettings.h | 2 - src/AppInstallerCommonCore/UserSettings.cpp | 1 - .../CompositeSource.cpp | 151 +++++++----------- .../Microsoft/ARPHelper.cpp | 21 +-- .../PredefinedInstalledSourceFactory.cpp | 42 ++--- 17 files changed, 130 insertions(+), 231 deletions(-) diff --git a/.github/actions/spelling/patterns.txt b/.github/actions/spelling/patterns.txt index 3155d2004a..9b06b0ee86 100644 --- a/.github/actions/spelling/patterns.txt +++ b/.github/actions/spelling/patterns.txt @@ -1,6 +1,7 @@ # See https://github.com/check-spelling/check-spelling/wiki/Configuration-Examples:-patterns WinGetDevCLI_8wekyb3d8bbwe +8wekyb3d8bbwe _tisf_sqliteReturnValue microsoft\.com/[^])">]* S[Hh][Aa]256: [0-9A-Fa-f]{64} diff --git a/doc/Settings.md b/doc/Settings.md index 0e6417710a..3b2f014878 100644 --- a/doc/Settings.md +++ b/doc/Settings.md @@ -312,17 +312,6 @@ You can enable the feature as shown below. }, ``` -### 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 - }, -``` - ### configureSelfElevate This feature enables configure commands to request elevation as needed. diff --git a/schemas/JSON/settings/settings.schema.0.2.json b/schemas/JSON/settings/settings.schema.0.2.json index b588606b36..4271121140 100644 --- a/schemas/JSON/settings/settings.schema.0.2.json +++ b/schemas/JSON/settings/settings.schema.0.2.json @@ -271,11 +271,6 @@ "type": "boolean", "default": false }, - "sideBySide": { - "description": "Enable support for improved side-by-side handling", - "type": "boolean", - "default": false - }, "configureSelfElevate": { "description": "Enable configure commands request elevation as needed", "type": "boolean", diff --git a/src/AppInstallerCLICore/Argument.cpp b/src/AppInstallerCLICore/Argument.cpp index 98b5740894..bb05b1c517 100644 --- a/src/AppInstallerCLICore/Argument.cpp +++ b/src/AppInstallerCLICore/Argument.cpp @@ -415,7 +415,7 @@ namespace AppInstaller::CLI 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 }; + return Argument{ type, Resource::String::UninstallAllVersionsArgumentDescription, ArgumentType::Flag, Argument::Visibility::Help }; case Args::Type::TargetVersion: return Argument{ type, Resource::String::TargetVersionArgumentDescription, ArgumentType::Standard }; case Args::Type::Proxy: diff --git a/src/AppInstallerCLICore/Workflows/RepairFlow.cpp b/src/AppInstallerCLICore/Workflows/RepairFlow.cpp index 9f113de15b..4bfc1d4a23 100644 --- a/src/AppInstallerCLICore/Workflows/RepairFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/RepairFlow.cpp @@ -398,6 +398,7 @@ namespace AppInstaller::CLI::Workflow context << SetPackageFamilyNamesInContext; } + break; case InstallerTypeEnum::MSStore: break; case InstallerTypeEnum::Portable: diff --git a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp index dbf3e754bf..b946af471d 100644 --- a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp +++ b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp @@ -1383,41 +1383,34 @@ namespace AppInstaller::CLI::Workflow void GetInstalledPackageVersion(Execution::Context& context) { - if (ExperimentalFeature::IsEnabled(ExperimentalFeature::Feature::SideBySide)) - { - std::shared_ptr installed = context.Get()->GetInstalled(); + std::shared_ptr installed = context.Get()->GetInstalled(); - if (installed) + 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)) { - // 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); + 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 + if (!installedVersion) { - context.Add(installed->GetLatestVersion()); + 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(nullptr); + context.Add(installed->GetLatestVersion()); } } else { - context.Add(GetInstalledVersion(context.Get())); + context.Add(nullptr); } } diff --git a/src/AppInstallerCLIE2ETests/Constants.cs b/src/AppInstallerCLIE2ETests/Constants.cs index 7d4ceb98c9..378ce3b3af 100644 --- a/src/AppInstallerCLIE2ETests/Constants.cs +++ b/src/AppInstallerCLIE2ETests/Constants.cs @@ -50,6 +50,7 @@ public class Constants public const string AICLIPackageFamilyName = "WinGetDevCLI_8wekyb3d8bbwe"; public const string AICLIPackageName = "WinGetDevCLI"; + public const string AICLIPackagePublisherHash = "8wekyb3d8bbwe"; public const string AICLIAppId = "WinGetDev"; public const string TestPackage = "TëstPackage.msix"; diff --git a/src/AppInstallerCLIE2ETests/ListCommand.cs b/src/AppInstallerCLIE2ETests/ListCommand.cs index 498a8c45de..b0fe1ba68a 100644 --- a/src/AppInstallerCLIE2ETests/ListCommand.cs +++ b/src/AppInstallerCLIE2ETests/ListCommand.cs @@ -23,7 +23,8 @@ public void ListSelf() { var result = TestCommon.RunAICLICommand("list", Constants.AICLIPackageFamilyName); Assert.AreEqual(Constants.ErrorCode.S_OK, result.ExitCode); - Assert.True(result.StdOut.Contains(Constants.AICLIPackageFamilyName)); + Assert.True(result.StdOut.Contains(Constants.AICLIPackageName)); + Assert.True(result.StdOut.Contains(Constants.AICLIPackagePublisherHash)); } /// diff --git a/src/AppInstallerCLITests/CompositeSource.cpp b/src/AppInstallerCLITests/CompositeSource.cpp index d1c464a8fc..28c98ba198 100644 --- a/src/AppInstallerCLITests/CompositeSource.cpp +++ b/src/AppInstallerCLITests/CompositeSource.cpp @@ -423,17 +423,18 @@ TEST_CASE("CompositeSource_OnlyAvailableFoundByRootSearch", "[CompositeSource]") RequireSearchRequestIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); SearchResult result; - result.Matches.emplace_back(setup.MakeInstalled().WithPFN(pfn), Criteria()); + result.Matches.emplace_back(setup.MakeInstalled().WithPFN(pfn), Criteria(PackageMatchField::PackageFamilyName)); return result; }; - setup.Available->Everything.Matches.emplace_back(setup.MakeAvailable().WithPFN(pfn), Criteria()); + std::shared_ptr availablePackage = setup.MakeAvailable().WithPFN(pfn).ToPackage(); + setup.Available->Everything.Matches.emplace_back(availablePackage, Criteria()); setup.Available->SearchFunction = [&](const SearchRequest& request) { RequireSearchRequestIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); SearchResult result; - result.Matches.emplace_back(setup.MakeAvailable().WithPFN(pfn), Criteria()); + result.Matches.emplace_back(availablePackage, Criteria(PackageMatchField::PackageFamilyName)); return result; }; @@ -717,7 +718,7 @@ TEST_CASE("CompositeSource_MultipleAvailableSources_ReverseMatchBoth", "[Composi RequireSearchRequestIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); SearchResult result; - result.Matches.emplace_back(installedPackage, Criteria()); + result.Matches.emplace_back(installedPackage, Criteria(PackageMatchField::PackageFamilyName)); return result; }; @@ -728,8 +729,9 @@ TEST_CASE("CompositeSource_MultipleAvailableSources_ReverseMatchBoth", "[Composi REQUIRE(result.Matches.size() == 1); REQUIRE(GetInstalledVersion(result.Matches[0].Package)); - REQUIRE(result.Matches[0].Package->GetAvailable().size() == 1); + REQUIRE(result.Matches[0].Package->GetAvailable().size() == 2); REQUIRE(result.Matches[0].Package->GetAvailable()[0]->GetVersionKeys().size() == 1); + REQUIRE(result.Matches[0].Package->GetAvailable()[1]->GetVersionKeys().size() == 1); } TEST_CASE("CompositeSource_IsSame", "[CompositeSource]") @@ -1031,7 +1033,7 @@ TEST_CASE("CompositeSource_TrackingFound_AvailablePath", "[CompositeSource]") RequireSearchRequestIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); SearchResult result; - result.Matches.emplace_back(installedPackage, Criteria()); + result.Matches.emplace_back(installedPackage, Criteria(PackageMatchField::PackageFamilyName)); return result; }; @@ -1120,17 +1122,22 @@ struct ExpectedResultsForPinning std::vector AvailableVersions; }; -void RequireExpectedResultsWithPin(std::shared_ptr package, const ExpectedResultsForPinning& expectedResult) +void RequireExpectedResultsWithPin(std::shared_ptr package, const ExpectedResultsForPinning& expectedResult, std::shared_ptr packageVersion = {}) { PinningData pinningData{ PinningData::Disposition::ReadOnly }; auto availableVersions = GetAvailableVersionsForInstalledVersion(package); + if (!packageVersion) + { + packageVersion = GetInstalledVersion(package); + } + for (const auto& entry : expectedResult.ResultsForPinBehavior) { auto pinBehavior = entry.first; const auto& result = entry.second; - auto evaluator = pinningData.CreatePinStateEvaluator(pinBehavior, GetInstalledVersion(package)); + auto evaluator = pinningData.CreatePinStateEvaluator(pinBehavior, packageVersion); auto latestAvailable = evaluator.GetLatestAvailableVersionForPins(availableVersions); REQUIRE(evaluator.IsUpdate(latestAvailable) == result.IsUpdateAvailable); @@ -1150,13 +1157,13 @@ void RequireExpectedResultsWithPin(std::shared_ptr package, c REQUIRE(availableVersionKeys.size() == expectedResult.AvailableVersions.size()); for (size_t i = 0; i < availableVersionKeys.size(); ++i) { - auto evaluator = pinningData.CreatePinStateEvaluator(PinBehavior::ConsiderPins, GetInstalledVersion(package)); + auto evaluator = pinningData.CreatePinStateEvaluator(PinBehavior::ConsiderPins, packageVersion); - auto packageVersion = availableVersions->GetVersion(expectedResult.AvailableVersions[i]); - REQUIRE(packageVersion); + auto availableVersion = availableVersions->GetVersion(expectedResult.AvailableVersions[i]); + REQUIRE(availableVersion); REQUIRE(availableVersionKeys[i].SourceId == expectedResult.AvailableVersions[i].SourceId); REQUIRE(availableVersionKeys[i].Version == expectedResult.AvailableVersions[i].Version); - REQUIRE(evaluator.EvaluatePinType(packageVersion) == expectedResult.AvailableVersions[i].PinnedState); + REQUIRE(evaluator.EvaluatePinType(availableVersion) == expectedResult.AvailableVersions[i].PinnedState); } } @@ -1418,12 +1425,12 @@ TEST_CASE("CompositeSource_Pinning_MultipleInstalled", "[CompositeSource][PinFlo SearchResult result; if (isSearchById || SearchRequestIncludes(request.Inclusions, PackageMatchField::ProductCode, MatchType::Exact, productCode1)) { - result.Matches.emplace_back(installedPackage1, Criteria()); + result.Matches.emplace_back(installedPackage1, Criteria(request.Inclusions[0].Field)); } if (isSearchById || SearchRequestIncludes(request.Inclusions, PackageMatchField::ProductCode, MatchType::Exact, productCode2)) { - result.Matches.emplace_back(installedPackage2, Criteria()); + result.Matches.emplace_back(installedPackage2, Criteria(request.Inclusions[0].Field)); } return result; @@ -1497,18 +1504,22 @@ TEST_CASE("CompositeSource_Pinning_MultipleInstalled", "[CompositeSource][PinFlo searchRequest.Inclusions.emplace_back(PackageMatchField::Id, MatchType::Exact, packageId); SearchResult result = setup.Composite.Search(searchRequest); - REQUIRE(result.Matches.size() == 2); + REQUIRE(result.Matches.size() == 1); + auto installedPackage = result.Matches[0].Package->GetInstalled(); + REQUIRE(installedPackage); + auto installedVersions = installedPackage->GetVersionKeys(); + REQUIRE(installedVersions.size() == 2); // Here we assume that the order we return the packages in the installed source // search is preserved. We'll need to change it if that stops being the case. - auto package1 = result.Matches[0].Package; - REQUIRE(package1); + auto packageVersion1 = installedPackage->GetVersion(installedVersions[1]); + REQUIRE(packageVersion1); - auto package2 = result.Matches[1].Package; - REQUIRE(package2); + auto packageVersion2 = installedPackage->GetVersion(installedVersions[0]); + REQUIRE(packageVersion2); - RequireExpectedResultsWithPin(package1, expectedResult1); - RequireExpectedResultsWithPin(package2, expectedResult2); + RequireExpectedResultsWithPin(result.Matches[0].Package, expectedResult1, packageVersion1); + RequireExpectedResultsWithPin(result.Matches[0].Package, expectedResult2, packageVersion2); } TEST_CASE("CompositeSource_CorrelateToInstalledContainsManifestData", "[CompositeSource]") @@ -1597,8 +1608,6 @@ TEST_CASE("CompositeSource_Respects_FeatureFlag_ManifestMayContainAdditionalSyst TEST_CASE("CompositeSource_SxS_TwoVersions_NoAvailable", "[CompositeSource][SideBySide]") { - auto enableFeature = TestUserSettings::EnableExperimentalFeature(Settings::ExperimentalFeature::Feature::SideBySide); - std::string productCode1 = "PC1"; std::string productCode2 = "PC2"; @@ -1615,8 +1624,6 @@ TEST_CASE("CompositeSource_SxS_TwoVersions_NoAvailable", "[CompositeSource][Side TEST_CASE("CompositeSource_SxS_TwoVersions_DifferentAvailable", "[CompositeSource][SideBySide]") { - auto enableFeature = TestUserSettings::EnableExperimentalFeature(Settings::ExperimentalFeature::Feature::SideBySide); - std::string productCode1 = "PC1"; std::string productCode2 = "PC2"; @@ -1660,8 +1667,6 @@ TEST_CASE("CompositeSource_SxS_TwoVersions_DifferentAvailable", "[CompositeSourc 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"; @@ -1698,8 +1703,6 @@ TEST_CASE("CompositeSource_SxS_TwoVersions_SameAvailable", "[CompositeSource][Si 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"; @@ -1740,8 +1743,6 @@ TEST_CASE("CompositeSource_SxS_ThreeVersions_SameAvailable", "[CompositeSource][ 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"; @@ -1780,8 +1781,6 @@ TEST_CASE("CompositeSource_SxS_TwoVersions_SameAvailable_Tracking", "[CompositeS 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"; diff --git a/src/AppInstallerCLITests/TestCommon.cpp b/src/AppInstallerCLITests/TestCommon.cpp index 6735056f5e..1c0e3eff0d 100644 --- a/src/AppInstallerCLITests/TestCommon.cpp +++ b/src/AppInstallerCLITests/TestCommon.cpp @@ -264,9 +264,6 @@ namespace TestCommon // 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; case Settings::ExperimentalFeature::Feature::StoreDownload: result->Set(true); break; diff --git a/src/AppInstallerCommonCore/ExperimentalFeature.cpp b/src/AppInstallerCommonCore/ExperimentalFeature.cpp index f009f88597..e1c2a55276 100644 --- a/src/AppInstallerCommonCore/ExperimentalFeature.cpp +++ b/src/AppInstallerCommonCore/ExperimentalFeature.cpp @@ -44,8 +44,6 @@ namespace AppInstaller::Settings return userSettings.Get(); case ExperimentalFeature::Feature::Configuration03: return userSettings.Get(); - case ExperimentalFeature::Feature::SideBySide: - return userSettings.Get(); case ExperimentalFeature::Feature::ConfigureSelfElevation: return userSettings.Get(); case ExperimentalFeature::Feature::StoreDownload: @@ -87,8 +85,6 @@ namespace AppInstaller::Settings return ExperimentalFeature{ "Resume", "resume", "https://aka.ms/winget-settings", Feature::Resume }; case Feature::Configuration03: return ExperimentalFeature{ "Configuration Schema 0.3", "configuration03", "https://aka.ms/winget-settings", Feature::Configuration03 }; - case Feature::SideBySide: - return ExperimentalFeature{ "Side-by-side improvements", "sideBySide", "https://aka.ms/winget-settings", Feature::SideBySide }; case Feature::ConfigureSelfElevation: return ExperimentalFeature{ "Configure Self Elevation", "configureSelfElevate", "https://aka.ms/winget-settings", Feature::ConfigureSelfElevation }; case Feature::StoreDownload: diff --git a/src/AppInstallerCommonCore/Public/winget/ExperimentalFeature.h b/src/AppInstallerCommonCore/Public/winget/ExperimentalFeature.h index b88e1d9c35..f014cf0c3f 100644 --- a/src/AppInstallerCommonCore/Public/winget/ExperimentalFeature.h +++ b/src/AppInstallerCommonCore/Public/winget/ExperimentalFeature.h @@ -25,11 +25,10 @@ namespace AppInstaller::Settings DirectMSI = 0x1, Resume = 0x2, Configuration03 = 0x4, - SideBySide = 0x8, - ConfigureSelfElevation = 0x10, - StoreDownload = 0x20, - ConfigureExport = 0x40, - IndexV2 = 0x80, + ConfigureSelfElevation = 0x8, + StoreDownload = 0x10, + ConfigureExport = 0x20, + IndexV2 = 0x40, 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 9c84196340..6f8b61783f 100644 --- a/src/AppInstallerCommonCore/Public/winget/UserSettings.h +++ b/src/AppInstallerCommonCore/Public/winget/UserSettings.h @@ -73,7 +73,6 @@ namespace AppInstaller::Settings EFDirectMSI, EFResume, EFConfiguration03, - EFSideBySide, EFConfigureSelfElevation, EFStoreDownload, EFIndexV2, @@ -158,7 +157,6 @@ namespace AppInstaller::Settings SETTINGMAPPING_SPECIALIZATION(Setting::EFDirectMSI, bool, bool, false, ".experimentalFeatures.directMSI"sv); SETTINGMAPPING_SPECIALIZATION(Setting::EFResume, bool, bool, false, ".experimentalFeatures.resume"sv); SETTINGMAPPING_SPECIALIZATION(Setting::EFConfiguration03, bool, bool, false, ".experimentalFeatures.configuration03"sv); - SETTINGMAPPING_SPECIALIZATION(Setting::EFSideBySide, bool, bool, false, ".experimentalFeatures.sideBySide"sv); SETTINGMAPPING_SPECIALIZATION(Setting::EFConfigureSelfElevation, bool, bool, false, ".experimentalFeatures.configureSelfElevate"sv); SETTINGMAPPING_SPECIALIZATION(Setting::EFStoreDownload, bool, bool, false, ".experimentalFeatures.storeDownload"sv); SETTINGMAPPING_SPECIALIZATION(Setting::EFIndexV2, bool, bool, true, ".experimentalFeatures.indexV2"sv); diff --git a/src/AppInstallerCommonCore/UserSettings.cpp b/src/AppInstallerCommonCore/UserSettings.cpp index b3289f28e3..c299a37140 100644 --- a/src/AppInstallerCommonCore/UserSettings.cpp +++ b/src/AppInstallerCommonCore/UserSettings.cpp @@ -261,7 +261,6 @@ namespace AppInstaller::Settings WINGET_VALIDATE_PASS_THROUGH(EFDirectMSI) WINGET_VALIDATE_PASS_THROUGH(EFResume) WINGET_VALIDATE_PASS_THROUGH(EFConfiguration03) - WINGET_VALIDATE_PASS_THROUGH(EFSideBySide) WINGET_VALIDATE_PASS_THROUGH(EFConfigureSelfElevation) WINGET_VALIDATE_PASS_THROUGH(EFStoreDownload) WINGET_VALIDATE_PASS_THROUGH(EFIndexV2) diff --git a/src/AppInstallerRepositoryCore/CompositeSource.cpp b/src/AppInstallerRepositoryCore/CompositeSource.cpp index e09a8c52f0..a0495d628a 100644 --- a/src/AppInstallerRepositoryCore/CompositeSource.cpp +++ b/src/AppInstallerRepositoryCore/CompositeSource.cpp @@ -702,9 +702,6 @@ namespace AppInstaller::Repository { if (availablePackage) { - // Disable primary if feature not enabled - setPrimary = setPrimary && ExperimentalFeature::IsEnabled(ExperimentalFeature::Feature::SideBySide); - m_availablePackages.emplace_back(OnlyAvailable(availablePackage)); if (setPrimary) @@ -1162,11 +1159,6 @@ namespace AppInstaller::Repository // the installed source. This would allow for folding even when the package is not in any available source. void FoldResults() { - if (!ExperimentalFeature::IsEnabled(ExperimentalFeature::Feature::SideBySide)) - { - return; - } - // The key to uniquely identify the package in the map struct InstalledResultFoldKey { @@ -1626,112 +1618,85 @@ namespace AppInstaller::Repository // Correlate against installed (allow exceptions out as we own the installed source) SearchResult installedCrossRef = m_installedSource.Search(systemReferenceSearch); - if (ExperimentalFeature::IsEnabled(ExperimentalFeature::Feature::SideBySide)) + for (const auto& installedMatch : installedCrossRef.Matches) { - for (const auto& installedMatch : installedCrossRef.Matches) + if (!IsStrongMatchField(installedMatch.MatchCriteria.Field)) { - 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) { - // 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; - } + correlationConfirmation = result.SearchAndHandleFailures(source, result.GetSystemReferenceStrings(installedMatch.Package->GetInstalled().get()).CreateInclusionsSearchRequest(SearchPurpose::CorrelationToAvailable)); } - // 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) + if (correlationConfirmation.Matches.empty()) { - // 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; - } + // We probably made the correlation due to tracking data, keep it. } - else + else if (correlationConfirmation.Matches.size() > 1) { - result.Matches.emplace_back(std::make_shared(installedMatch.Package), match.MatchCriteria); - resultPackage = result.Matches.back().Package; + // 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; } - - bool setPrimary = false; - if (trackingPackage) + else if (!OnlyAvailable(correlationConfirmation.Matches[0].Package)->IsSame(OnlyAvailable(match.Package).get())) { - auto trackingPackageTime = GetLatestTrackingWriteTime(trackingPackage); + // 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 (trackingPackageTime > resultPackage->GetTrackingPackageWriteTime()) + 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) { - resultPackage->SetTracking(source, std::move(trackingPackage), trackingPackageTime); - setPrimary = true; + // 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; } } - resultPackage->AddAvailablePackage(std::move(match.Package), setPrimary); - - foundInstalledMatch = true; + if (foundSameSource) + { + continue; + } + } + else + { + result.Matches.emplace_back(std::make_shared(installedMatch.Package), match.MatchCriteria); + resultPackage = result.Matches.back().Package; } - } - 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"); - }); - if (installedPackage && !result.ContainsInstalledPackage(installedPackage->GetInstalled().get())) + bool setPrimary = false; + if (trackingPackage) { - // 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; - auto compositePackage = std::make_shared(installedPackage, std::move(match.Package)); - if (trackingPackage) + auto trackingPackageTime = GetLatestTrackingWriteTime(trackingPackage); + + if (trackingPackageTime > resultPackage->GetTrackingPackageWriteTime()) { - auto trackingPackageTime = GetLatestTrackingWriteTime(trackingPackage); - compositePackage->SetTracking(source, std::move(trackingPackage), trackingPackageTime); + resultPackage->SetTracking(source, std::move(trackingPackage), trackingPackageTime); + setPrimary = true; } - result.Matches.emplace_back(std::move(compositePackage), match.MatchCriteria); } + + resultPackage->AddAvailablePackage(std::move(match.Package), setPrimary); + + foundInstalledMatch = true; } } diff --git a/src/AppInstallerRepositoryCore/Microsoft/ARPHelper.cpp b/src/AppInstallerRepositoryCore/Microsoft/ARPHelper.cpp index 99003d67ba..063e02284a 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/ARPHelper.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/ARPHelper.cpp @@ -398,24 +398,13 @@ namespace AppInstaller::Repository::Microsoft Manifest::Manifest manifest; manifest.DefaultLocalization.Add({ "ARP" }); - // Use the key name as the Id, as it is supposed to be unique. - // TODO: We probably want something better here, like constructing the value as - // `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). - if (Settings::ExperimentalFeature::IsEnabled(Settings::ExperimentalFeature::Feature::SideBySide)) - { - char separator = '\\'; + // Construct a unique name for this entry + const char separator = '\\'; - std::ostringstream stream; - stream << "ARP" << separator << scope << separator << architecture << separator << productCode; + std::ostringstream stream; + stream << "ARP" << separator << scope << separator << architecture << separator << productCode; - manifest.Id = stream.str(); - } - else - { - manifest.Id = productCode; - } + manifest.Id = stream.str(); manifest.Installers.emplace_back(); // TODO: This likely needs some cleanup applied, as it looks like INNO tends to append an "_is#" diff --git a/src/AppInstallerRepositoryCore/Microsoft/PredefinedInstalledSourceFactory.cpp b/src/AppInstallerRepositoryCore/Microsoft/PredefinedInstalledSourceFactory.cpp index 7a6c57763d..8e7d0cad44 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/PredefinedInstalledSourceFactory.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/PredefinedInstalledSourceFactory.cpp @@ -104,14 +104,7 @@ namespace AppInstaller::Repository::Microsoft Utility::NormalizedString fullName = Utility::ConvertToUTF8(packageId.FullName()); Utility::NormalizedString familyName = Utility::ConvertToUTF8(packageId.FamilyName()); - if (Settings::ExperimentalFeature::IsEnabled(Settings::ExperimentalFeature::Feature::SideBySide)) - { - manifest.Id = "MSIX\\" + fullName; - } - else - { - manifest.Id = familyName; - } + manifest.Id = "MSIX\\" + fullName; // Get version std::ostringstream strstr; @@ -166,34 +159,17 @@ namespace AppInstaller::Repository::Microsoft manifest.Installers[0].PackageFamilyName = familyName; - try - { - // Use the full name as a unique key for the path - auto manifestId = index.AddManifest(manifest); + // Use the full name as a unique key for the path + auto manifestId = index.AddManifest(manifest); - index.SetMetadataByManifestId(manifestId, PackageVersionMetadata::InstalledType, - Manifest::InstallerTypeToString(Manifest::InstallerTypeEnum::Msix)); + 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) + auto architecture = Utility::ConvertToArchitectureEnum(packageId.Architecture()); + if (architecture) { - if (HRESULT_FROM_WIN32(ERROR_ALREADY_EXISTS) == resultException.GetErrorCode() && package.IsFramework()) - { - // 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; + index.SetMetadataByManifestId(manifestId, PackageVersionMetadata::InstalledArchitecture, + ToString(architecture.value())); } } }