diff --git a/src/AppInstallerCLICore/Commands/UpgradeCommand.cpp b/src/AppInstallerCLICore/Commands/UpgradeCommand.cpp index 0cb1915c6f..530b091dc5 100644 --- a/src/AppInstallerCLICore/Commands/UpgradeCommand.cpp +++ b/src/AppInstallerCLICore/Commands/UpgradeCommand.cpp @@ -43,7 +43,8 @@ namespace AppInstaller::CLI // either for upgrading or for listing available upgrades. bool HasArgumentsForMultiplePackages(Execution::Args& execArgs) { - return execArgs.Contains(Args::Type::All); + return execArgs.Contains(Args::Type::All) || + execArgs.Contains(Args::Type::IncludeUnknown); } // Determines whether there are any arguments only used as options during an upgrade, @@ -102,7 +103,7 @@ namespace AppInstaller::CLI Argument::ForType(Args::Type::AcceptSourceAgreements), Argument::ForType(Execution::Args::Type::CustomHeader), Argument{ "all", Argument::NoAlias, Args::Type::All, Resource::String::UpdateAllArgumentDescription, ArgumentType::Flag }, - Argument{ "include-unknown", Argument::NoAlias, Args::Type::IncludeUnknown, Resource::String::IncludeUnknownArgumentDescription, ArgumentType::Flag } + Argument{ "include-unknown", Argument::NoAlias, Args::Type::IncludeUnknown, Resource::String::IncludeUnknownArgumentDescription, ArgumentType::Flag }, }; } @@ -158,25 +159,29 @@ namespace AppInstaller::CLI void UpgradeCommand::ValidateArgumentsInternal(Execution::Args& execArgs) const { - if (execArgs.Contains(Execution::Args::Type::Manifest) && + if (execArgs.Contains(Execution::Args::Type::Manifest) && (HasSearchQueryArguments(execArgs) || - HasArgumentsForMultiplePackages(execArgs) || - HasArgumentsForSource(execArgs))) + HasArgumentsForMultiplePackages(execArgs) || + HasArgumentsForSource(execArgs))) { throw CommandException(Resource::String::BothManifestAndSearchQueryProvided); } - - else if (!ShouldListUpgrade(execArgs) - && !HasSearchQueryArguments(execArgs) - && (execArgs.Contains(Args::Type::Log) || - execArgs.Contains(Args::Type::Override) || - execArgs.Contains(Args::Type::InstallLocation) || - execArgs.Contains(Args::Type::HashOverride) || - execArgs.Contains(Args::Type::AcceptPackageAgreements))) + if (!ShouldListUpgrade(execArgs) + && !HasSearchQueryArguments(execArgs) + && (execArgs.Contains(Args::Type::Log) || + execArgs.Contains(Args::Type::Override) || + execArgs.Contains(Args::Type::InstallLocation) || + execArgs.Contains(Args::Type::HashOverride) || + execArgs.Contains(Args::Type::AcceptPackageAgreements))) { throw CommandException(Resource::String::InvalidArgumentWithoutQueryError); } + + if (HasArgumentsForSinglePackage(execArgs) && HasArgumentsForMultiplePackages(execArgs)) + { + throw CommandException(Resource::String::IncompatibleArgumentsProvided); + } } void UpgradeCommand::ExecuteInternal(Execution::Context& context) const diff --git a/src/AppInstallerCLICore/ExecutionArgs.h b/src/AppInstallerCLICore/ExecutionArgs.h index c160cb1688..80568e1b89 100644 --- a/src/AppInstallerCLICore/ExecutionArgs.h +++ b/src/AppInstallerCLICore/ExecutionArgs.h @@ -79,8 +79,11 @@ namespace AppInstaller::CLI::Execution AdminSettingEnable, AdminSettingDisable, + // Upgrade Command + All, // Update all installed packages to latest + IncludeUnknown, // Allow upgrades of packages with unknown versions + // Other - All, // Used in Update command to update all installed packages to latest ListVersions, // Used in Show command to list all available versions of an app NoVT, // Disable VirtualTerminal outputs RetroStyle, // Makes progress display as retro @@ -91,7 +94,6 @@ namespace AppInstaller::CLI::Execution DependencySource, // Index source to be queried against for finding dependencies CustomHeader, // Optional Rest source header AcceptSourceAgreements, // Accept all source agreements - IncludeUnknown, // Used in Upgrade command to allow upgrades of packages with unknown versions Wait, // Prompts the user to press any key before exiting // Used for demonstration purposes diff --git a/src/AppInstallerCLICore/Resources.h b/src/AppInstallerCLICore/Resources.h index 20b807a135..d369b4cb99 100644 --- a/src/AppInstallerCLICore/Resources.h +++ b/src/AppInstallerCLICore/Resources.h @@ -108,6 +108,7 @@ namespace AppInstaller::CLI::Resource WINGET_DEFINE_RESOURCE_STRINGID(ImportSearchFailed); WINGET_DEFINE_RESOURCE_STRINGID(ImportSourceNotInstalled); WINGET_DEFINE_RESOURCE_STRINGID(IncludeUnknownArgumentDescription); + WINGET_DEFINE_RESOURCE_STRINGID(IncompatibleArgumentsProvided); WINGET_DEFINE_RESOURCE_STRINGID(InstallAndUpgradeCommandsReportDependencies); WINGET_DEFINE_RESOURCE_STRINGID(InstallArchitectureArgumentDescription); WINGET_DEFINE_RESOURCE_STRINGID(InstallationAbandoned); @@ -372,12 +373,13 @@ namespace AppInstaller::CLI::Resource WINGET_DEFINE_RESOURCE_STRINGID(UnsupportedArgument); WINGET_DEFINE_RESOURCE_STRINGID(UpdateAllArgumentDescription); WINGET_DEFINE_RESOURCE_STRINGID(UpdateNotApplicable); + WINGET_DEFINE_RESOURCE_STRINGID(UpgradeAvailableForPinned); WINGET_DEFINE_RESOURCE_STRINGID(UpgradeCommandLongDescription); WINGET_DEFINE_RESOURCE_STRINGID(UpgradeCommandShortDescription); WINGET_DEFINE_RESOURCE_STRINGID(UpgradeDifferentInstallTechnology); WINGET_DEFINE_RESOURCE_STRINGID(UpgradeDifferentInstallTechnologyInNewerVersions); - WINGET_DEFINE_RESOURCE_STRINGID(UpgradeUnknownCount); - WINGET_DEFINE_RESOURCE_STRINGID(UpgradeUnknownCountSingle); + WINGET_DEFINE_RESOURCE_STRINGID(UpgradeRequireExplicitCount); + WINGET_DEFINE_RESOURCE_STRINGID(UpgradeUnknownVersionCount); WINGET_DEFINE_RESOURCE_STRINGID(UpgradeUnknownVersionExplanation); WINGET_DEFINE_RESOURCE_STRINGID(Usage); WINGET_DEFINE_RESOURCE_STRINGID(ValidateCommandLongDescription); diff --git a/src/AppInstallerCLICore/Workflows/UpdateFlow.cpp b/src/AppInstallerCLICore/Workflows/UpdateFlow.cpp index 4491f9190b..2d75611b11 100644 --- a/src/AppInstallerCLICore/Workflows/UpdateFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/UpdateFlow.cpp @@ -44,32 +44,41 @@ namespace AppInstaller::CLI::Workflow bool updateFound = false; bool installedTypeInapplicable = false; - if (!installedVersion.IsUnknown() || context.Args.Contains(Execution::Args::Type::IncludeUnknown)) + if (installedVersion.IsUnknown() && !context.Args.Contains(Execution::Args::Type::IncludeUnknown)) { - // The version keys should have already been sorted by version - const auto& versionKeys = package->GetAvailableVersionKeys(); - for (const auto& key : versionKeys) + // the package has an unknown version and the user did not request to upgrade it anyway. + if (m_reportUpdateNotFound) { - // Check Update Version - if (IsUpdateVersionApplicable(installedVersion, Utility::Version(key.Version))) - { - auto packageVersion = package->GetAvailableVersion(key); - auto manifest = packageVersion->GetManifest(); + context.Reporter.Info() << Resource::String::UpgradeUnknownVersionExplanation << std::endl; + } + + AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_UPDATE_NOT_APPLICABLE); + } + + // The version keys should have already been sorted by version + const auto& versionKeys = package->GetAvailableVersionKeys(); + for (const auto& key : versionKeys) + { + // Check Update Version + if (IsUpdateVersionApplicable(installedVersion, Utility::Version(key.Version))) + { + auto packageVersion = package->GetAvailableVersion(key); + auto manifest = packageVersion->GetManifest(); - // Check applicable Installer - auto [installer, inapplicabilities] = manifestComparator.GetPreferredInstaller(manifest); - if (!installer.has_value()) + // Check applicable Installer + auto [installer, inapplicabilities] = manifestComparator.GetPreferredInstaller(manifest); + if (!installer.has_value()) + { + // If there is at least one installer whose only reason is InstalledType. + auto onlyInstalledType = std::find(inapplicabilities.begin(), inapplicabilities.end(), InapplicabilityFlags::InstalledType); + if (onlyInstalledType != inapplicabilities.end()) { - // If there is at least one installer whose only reason is InstalledType. - auto onlyInstalledType = std::find(inapplicabilities.begin(), inapplicabilities.end(), InapplicabilityFlags::InstalledType); - if (onlyInstalledType != inapplicabilities.end()) - { - installedTypeInapplicable = true; - } - - continue; + installedTypeInapplicable = true; } + continue; + } + Logging::Telemetry().LogSelectedInstaller( static_cast(installer->Arch), installer->Url, @@ -88,25 +97,16 @@ namespace AppInstaller::CLI::Workflow context.Add(std::move(packageVersion)); context.Add(std::move(installer)); - updateFound = true; - break; - } - else - { - // Any following versions are not applicable - break; - } + updateFound = true; + break; } - } - else - { - // the package has an unknown version and the user did not request to upgrade it anyway. - if (m_reportUpdateNotFound) + else { - context.Reporter.Info() << Resource::String::UpgradeUnknownVersionExplanation << std::endl; + // Any following versions are not applicable + break; } - AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_UPDATE_NOT_APPLICABLE); } + if (!updateFound) { if (m_reportUpdateNotFound) @@ -143,7 +143,8 @@ namespace AppInstaller::CLI::Workflow const auto& matches = context.Get().Matches; std::vector> packagesToInstall; bool updateAllFoundUpdate = false; - int unknownPackagesCount = 0; + int packagesWithUnknownVersionSkipped = 0; + int packagesThatRequireExplicitSkipped = 0; for (const auto& match : matches) { @@ -154,7 +155,8 @@ namespace AppInstaller::CLI::Workflow auto installedVersion = match.Package->GetInstalledVersion(); updateContext.Add(match.Package); - + + // Filter out packages with unknown installed versions if (context.Args.Contains(Execution::Args::Type::IncludeUnknown)) { updateContext.Args.AddArg(Execution::Args::Type::IncludeUnknown); @@ -162,7 +164,8 @@ namespace AppInstaller::CLI::Workflow else if (Utility::Version(installedVersion->GetProperty(PackageVersionProperty::Version)).IsUnknown()) { // we don't know what the package's version is and the user didn't ask to upgrade it anyway. - unknownPackagesCount++; + AICLI_LOG(CLI, Info, << "Skipping " << match.Package->GetProperty(PackageProperty::Id) << " as it has unknown installed version"); + ++packagesWithUnknownVersionSkipped; continue; } @@ -176,6 +179,24 @@ namespace AppInstaller::CLI::Workflow continue; } + // Filter out packages that require explicit upgrades. + // We require explicit upgrades only if the installed version is pinned, + // either because it was manually pinned or because the manifest indicated + // RequireExplicitUpgrade. + // Note that this does not consider whether the update to be installed has + // RequireExplicitUpgrade. While this has the downside of not working with + // packages installed from another source, it ensures consistency with the + // list of available updates (there we don't have the selected installer) + // and at most we will update each package like this once. + auto installedMetadata = updateContext.Get()->GetMetadata(); + auto pinnedState = ConvertToPackagePinnedStateEnum(installedMetadata[PackageVersionMetadata::PinnedState]); + if (pinnedState != PackagePinnedState::NotPinned) + { + AICLI_LOG(CLI, Info, << "Skipping " << match.Package->GetProperty(PackageProperty::Id) << " as it requires explicit upgrade"); + ++packagesThatRequireExplicitSkipped; + continue; + } + updateAllFoundUpdate = true; AddToPackagesToInstallIfNotPresent(packagesToInstall, std::move(updateContextPtr)); @@ -191,5 +212,17 @@ namespace AppInstaller::CLI::Workflow APPINSTALLER_CLI_ERROR_UPDATE_ALL_HAS_FAILURE, { APPINSTALLER_CLI_ERROR_UPDATE_NOT_APPLICABLE }); } + + if (packagesWithUnknownVersionSkipped > 0) + { + AICLI_LOG(CLI, Info, << packagesWithUnknownVersionSkipped << " package(s) skipped due to unknown installed version"); + context.Reporter.Info() << packagesWithUnknownVersionSkipped << " " << Resource::String::UpgradeUnknownVersionCount << std::endl; + } + + if (packagesThatRequireExplicitSkipped > 0) + { + AICLI_LOG(CLI, Info, << packagesThatRequireExplicitSkipped << " package(s) skipped due to requiring explicit upgrade"); + context.Reporter.Info() << packagesThatRequireExplicitSkipped << " " << Resource::String::UpgradeRequireExplicitCount << std::endl; + } } } diff --git a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp index b13e612a33..65dd2928fd 100644 --- a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp +++ b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp @@ -218,6 +218,44 @@ namespace AppInstaller::CLI::Workflow return accepted; } + + // Data shown on a line of a table displaying installed packages + struct InstalledPackagesTableLine + { + InstalledPackagesTableLine(Utility::LocIndString name, Utility::LocIndString id, Utility::LocIndString installedVersion, Utility::LocIndString availableVersion, Utility::LocIndString source) + : Name(name), Id(id), InstalledVersion(installedVersion), AvailableVersion(availableVersion), Source(source) {} + + Utility::LocIndString Name; + Utility::LocIndString Id; + Utility::LocIndString InstalledVersion; + Utility::LocIndString AvailableVersion; + Utility::LocIndString Source; + }; + + void OutputInstalledPackagesTable(Execution::Context& context, const std::vector& lines) + { + Execution::TableOutput<5> table(context.Reporter, + { + Resource::String::SearchName, + Resource::String::SearchId, + Resource::String::SearchVersion, + Resource::String::AvailableHeader, + Resource::String::SearchSource + }); + + for (const auto& line : lines) + { + table.OutputLine({ + line.Name, + line.Id, + line.InstalledVersion, + line.AvailableVersion, + line.Source + }); + } + + table.Complete(); + } } bool WorkflowTask::operator==(const WorkflowTask& other) const @@ -701,17 +739,11 @@ namespace AppInstaller::CLI::Workflow { auto& searchResult = context.Get(); - Execution::TableOutput<5> table(context.Reporter, - { - Resource::String::SearchName, - Resource::String::SearchId, - Resource::String::SearchVersion, - Resource::String::AvailableHeader, - Resource::String::SearchSource - }); + std::vector lines; + std::vector linesForExplicitUpgrade; int availableUpgradesCount = 0; - int unknownPackagesCount = 0; + int packagesWithUnknownVersionSkipped = 0; auto &source = context.Get(); bool shouldShowSource = source.IsComposite() && source.GetAvailableSources().size() > 1; @@ -727,7 +759,7 @@ namespace AppInstaller::CLI::Workflow if (m_onlyShowUpgrades && !context.Args.Contains(Execution::Args::Type::IncludeUnknown) && Utility::Version(installedVersion->GetProperty(PackageVersionProperty::Version)).IsUnknown() && updateAvailable) { // We are only showing upgrades, and the user did not request to include packages with unknown versions. - unknownPackagesCount++; + ++packagesWithUnknownVersionSkipped; continue; } @@ -751,22 +783,31 @@ namespace AppInstaller::CLI::Workflow // Output using the local PackageName instead of the name in the manifest, to prevent confusion for packages that add multiple // Add/Remove Programs entries. // TODO: De-duplicate this list, and only show (by default) one entry per matched package. - table.OutputLine({ + InstalledPackagesTableLine line( installedVersion->GetProperty(PackageVersionProperty::Name), match.Package->GetProperty(PackageProperty::Id), installedVersion->GetProperty(PackageVersionProperty::Version), availableVersion, - shouldShowSource ? sourceName : ""s - }); - - + shouldShowSource ? sourceName : Utility::LocIndString() + ); + + auto pinnedState = ConvertToPackagePinnedStateEnum(installedVersion->GetMetadata()[PackageVersionMetadata::PinnedState]); + bool requiresExplicitUpgrade = m_onlyShowUpgrades && pinnedState != PackagePinnedState::NotPinned; + if (requiresExplicitUpgrade) + { + linesForExplicitUpgrade.push_back(std::move(line)); + } + else + { + lines.push_back(std::move(line)); + } } } } - table.Complete(); + OutputInstalledPackagesTable(context, lines); - if (table.IsEmpty()) + if (lines.empty()) { context.Reporter.Info() << Resource::String::NoInstalledPackageFound << std::endl; } @@ -782,11 +823,21 @@ namespace AppInstaller::CLI::Workflow context.Reporter.Info() << availableUpgradesCount << ' ' << Resource::String::AvailableUpgrades << std::endl; } } - if (m_onlyShowUpgrades && unknownPackagesCount > 0 && !context.Args.Contains(Execution::Args::Type::IncludeUnknown)) + + if (!linesForExplicitUpgrade.empty()) { - context.Reporter.Info() << unknownPackagesCount << " " << (unknownPackagesCount == 1 ? Resource::String::UpgradeUnknownCountSingle : Resource::String::UpgradeUnknownCount) << std::endl; + context.Reporter.Info() << std::endl << Resource::String::UpgradeAvailableForPinned << std::endl; + OutputInstalledPackagesTable(context, linesForExplicitUpgrade); } + if (m_onlyShowUpgrades) + { + if (packagesWithUnknownVersionSkipped > 0) + { + AICLI_LOG(CLI, Info, << packagesWithUnknownVersionSkipped << " package(s) skipped due to unknown installed version"); + context.Reporter.Info() << packagesWithUnknownVersionSkipped << " " << Resource::String::UpgradeUnknownVersionCount << std::endl; + } + } } void EnsureMatchesFromSearchResult::operator()(Execution::Context& context) const diff --git a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw index 583db2ef73..4fed42652f 100644 --- a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw +++ b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw @@ -1272,6 +1272,14 @@ Please specify one of them using the `--source` option to proceed. package has a version number that cannot be determined. Using "--include-unknown" may show more results. {Locked="--include-unknown"} This string is preceded by a (integer) number of packages that do not have notated versions. + + package(s) have version numbers that cannot be determined. Use "--include-unknown" to see all results. + {Locked="--include-unknown"} This string is preceded by a (integer) number of packages that do not have notated versions. + + + package(s) is pinned and needs to be explicitly upgraded. + This string is preceded by a (integer) number of packages that require explicit upgrades. + The arguments provided can only be used with a query. @@ -1379,4 +1387,11 @@ Please specify one of them using the `--source` option to proceed. Only one non-portable nested installer can be specified for an archive installer + + Incompatible command line arguments provided + + + The following packages have an upgrade available, but require explicit targeting for upgrade: + "require explicit targeting for upgrade" means that the package will not be upgraded with all others unless an extra flag is added, or the package is mentioned explicitly + \ No newline at end of file diff --git a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj index 5bd30e611c..6c92d38159 100644 --- a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj +++ b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj @@ -295,7 +295,7 @@ true - + true @@ -622,7 +622,7 @@ true - + true diff --git a/src/AppInstallerCLITests/WorkFlow.cpp b/src/AppInstallerCLITests/WorkFlow.cpp index 8107d80d5c..6e8411c71b 100644 --- a/src/AppInstallerCLITests/WorkFlow.cpp +++ b/src/AppInstallerCLITests/WorkFlow.cpp @@ -105,9 +105,16 @@ namespace } }; + enum TestSourceSearchOptions + { + None = 0, + UpgradeUsesAgreements, + UpgradeRequiresExplicit, + }; + struct WorkflowTestCompositeSource : public TestSource { - WorkflowTestCompositeSource(bool upgradeUsesLicenses) : m_upgradeUsesLicenses(upgradeUsesLicenses) {} + WorkflowTestCompositeSource(TestSourceSearchOptions searchOptions = TestSourceSearchOptions::None) : m_searchOptions(searchOptions) {} SearchResult Search(const SearchRequest& request) const override { @@ -133,7 +140,17 @@ namespace { auto manifest = YamlParser::CreateFromPath(TestDataFile("InstallFlowTest_Exe.yaml")); auto manifest2 = YamlParser::CreateFromPath(TestDataFile("UpdateFlowTest_Exe.yaml")); - auto manifest3 = YamlParser::CreateFromPath(TestDataFile(m_upgradeUsesLicenses ? "UpdateFlowTest_Exe_2_LicenseAgreement.yaml" : "UpdateFlowTest_Exe_2.yaml")); + Manifest manifest3; + switch (m_searchOptions) + { + case TestSourceSearchOptions::UpgradeUsesAgreements: + manifest3 = YamlParser::CreateFromPath(TestDataFile("UpdateFlowTest_Exe_2_LicenseAgreement.yaml")); + break; + default: + manifest3 = YamlParser::CreateFromPath(TestDataFile("UpdateFlowTest_Exe_2.yaml")); + break; + } + auto testPackage = TestPackage::Make( manifest, @@ -171,12 +188,33 @@ namespace if (input.empty() || input == "AppInstallerCliTest.TestMsixInstaller") { auto manifest = YamlParser::CreateFromPath(TestDataFile("InstallFlowTest_Msix_StreamingFlow.yaml")); - auto manifest2 = YamlParser::CreateFromPath(TestDataFile(m_upgradeUsesLicenses ? "UpdateFlowTest_Msix_LicenseAgreement.yaml" : "UpdateFlowTest_Msix.yaml")); + Manifest manifest2; + switch (m_searchOptions) + { + case TestSourceSearchOptions::UpgradeUsesAgreements: + manifest2 = YamlParser::CreateFromPath(TestDataFile("UpdateFlowTest_Msix_LicenseAgreement.yaml")); + break; + case TestSourceSearchOptions::None: + default: + manifest2 = YamlParser::CreateFromPath(TestDataFile("UpdateFlowTest_Msix.yaml")); + break; + } + + TestPackage::MetadataMap packageMetadata + { + { PackageVersionMetadata::InstalledType, "Msix" }, + }; + + if (m_searchOptions == TestSourceSearchOptions::UpgradeRequiresExplicit) + { + packageMetadata[PackageVersionMetadata::PinnedState] = "PinnedByManifest"; + } + result.Matches.emplace_back( ResultMatch( TestPackage::Make( manifest, - TestPackage::MetadataMap{ { PackageVersionMetadata::InstalledType, "Msix" } }, + packageMetadata, std::vector{ manifest2, manifest }, shared_from_this() ), @@ -338,7 +376,7 @@ namespace { auto manifest = YamlParser::CreateFromPath(TestDataFile("InstallFlowTest_Exe.yaml")); auto manifest2 = YamlParser::CreateFromPath(TestDataFile("UpdateFlowTest_Exe.yaml")); - auto manifest3 = YamlParser::CreateFromPath(TestDataFile(m_upgradeUsesLicenses ? "UpdateFlowTest_Exe_2_LicenseAgreement.yaml" : "UpdateFlowTest_Exe_2.yaml")); + auto manifest3 = YamlParser::CreateFromPath(TestDataFile("UpdateFlowTest_Exe_2.yaml")); result.Matches.emplace_back( ResultMatch( TestPackage::Make( @@ -373,7 +411,7 @@ namespace } private: - bool m_upgradeUsesLicenses; + TestSourceSearchOptions m_searchOptions; }; struct TestContext; @@ -479,7 +517,7 @@ void OverrideForOpenSource(TestContext& context) } }); } -void OverrideForCompositeInstalledSource(TestContext& context, bool upgradeUsesLicenses = false) +void OverrideForCompositeInstalledSource(TestContext& context, TestSourceSearchOptions searchOptions = TestSourceSearchOptions::None) { context.Override({ "OpenSource", [](TestContext&) { @@ -487,7 +525,7 @@ void OverrideForCompositeInstalledSource(TestContext& context, bool upgradeUsesL context.Override({ "OpenCompositeSource", [=](TestContext& context) { - context.Add(Source{ std::make_shared(upgradeUsesLicenses) }); + context.Add(Source{ std::make_shared(searchOptions) }); } }); } @@ -495,13 +533,13 @@ void OverrideForImportSource(TestContext& context, bool useTestCompositeSource = { context.Override({ "OpenPredefinedSource", [=](TestContext& context) { - auto installedSource = useTestCompositeSource? std::make_shared(false) : std::make_shared(); + auto installedSource = useTestCompositeSource ? std::make_shared() : std::make_shared(); context.Add(Source{ installedSource }); } }); context.Override({ Workflow::OpenSourcesForImport, [](TestContext& context) { - context.Add(std::vector{ Source{ std::make_shared(false) } }); + context.Add(std::vector{ Source{ std::make_shared() } }); } }); } @@ -2176,7 +2214,7 @@ TEST_CASE("UpdateFlow_All_LicenseAgreement", "[UpdateFlow][workflow]") std::ostringstream updateOutput; TestContext context{ updateOutput, std::cin }; auto previousThreadGlobals = context.SetForCurrentThread(); - OverrideForCompositeInstalledSource(context, /* upgradeUsesLicenses */ true); + OverrideForCompositeInstalledSource(context, TestSourceSearchOptions::UpgradeUsesAgreements); OverrideForShellExecute(context); OverrideForMSIX(context); OverrideForMSStore(context, true); @@ -2213,7 +2251,7 @@ TEST_CASE("UpdateFlow_All_LicenseAgreement_NotAccepted", "[UpdateFlow][workflow] std::ostringstream updateOutput; TestContext context{ updateOutput, updateInput }; auto previousThreadGlobals = context.SetForCurrentThread(); - OverrideForCompositeInstalledSource(context, /* upgradeUsesLicenses */ true); + OverrideForCompositeInstalledSource(context, TestSourceSearchOptions::UpgradeUsesAgreements); context.Args.AddArg(Execution::Args::Type::All); UpgradeCommand update({}); @@ -2250,6 +2288,75 @@ TEST_CASE("UninstallFlow_UninstallPortable", "[UninstallFlow][workflow]") REQUIRE(std::filesystem::exists(uninstallResultPath.GetPath())); } +TEST_CASE("UpdateFlow_RequireExplicit", "[UpdateFlow][workflow]") +{ + TestCommon::TempFile updateExeResultPath("TestExeInstalled.txt"); + TestCommon::TempFile updateMsixResultPath("TestMsixInstalled.txt"); + + std::ostringstream updateOutput; + TestContext context{ updateOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); + + // Msix package has an update that requires explicit upgrade. + // Exe, Portable, MSStore, Zip are also listed with an available upgrade. + OverrideForCompositeInstalledSource(context, TestSourceSearchOptions::UpgradeRequiresExplicit); + + SECTION("List available upgrades") + { + UpgradeCommand update({}); + update.Execute(context); + INFO(updateOutput.str()); + + // The package that requires explicit upgrade is listed below the header for pinned packages + REQUIRE(updateOutput.str().find("AppInstallerCliTest.TestExeInstaller") != std::string::npos); + + auto pinnedPackagesHeaderPosition = updateOutput.str().find(Resource::LocString(Resource::String::UpgradeAvailableForPinned)); + auto pinnedPackageLinePosition = updateOutput.str().find("AppInstallerCliTest.TestMsixInstaller"); + REQUIRE(pinnedPackagesHeaderPosition != std::string::npos); + REQUIRE(pinnedPackageLinePosition != std::string::npos); + REQUIRE(pinnedPackagesHeaderPosition < pinnedPackageLinePosition); + REQUIRE(updateOutput.str().find(Resource::LocString(Resource::String::UpgradeRequireExplicitCount)) == std::string::npos); + } + + SECTION("Upgrade all except pinned") + { + context.Args.AddArg(Args::Type::All); + OverrideForMSStore(context, true); + OverrideForPortableInstall(context); + OverrideForShellExecute(context); + OverrideForExtractInstallerFromArchive(context); + OverrideForVerifyAndSetNestedInstaller(context); + + UpgradeCommand update({}); + update.Execute(context); + INFO(updateOutput.str()); + + auto s = updateOutput.str(); + + // Verify message is printed for skipped package + REQUIRE(updateOutput.str().find(Resource::LocString(Resource::String::UpgradeRequireExplicitCount)) != std::string::npos); + + // Verify package is not installed, but all others are + REQUIRE(std::filesystem::exists(updateExeResultPath.GetPath())); + REQUIRE(!std::filesystem::exists(updateMsixResultPath.GetPath())); + } + + SECTION("Upgrade explicitly") + { + context.Args.AddArg(Execution::Args::Type::Query, "AppInstallerCliTest.TestMsixInstaller"sv); + OverrideForMSIX(context); + + UpgradeCommand update({}); + update.Execute(context); + INFO(updateOutput.str()); + + REQUIRE(std::filesystem::exists(updateMsixResultPath.GetPath())); + } + + // Command should always succeed + REQUIRE(context.GetTerminationHR() == S_OK); +} + TEST_CASE("UninstallFlow_UninstallExe", "[UninstallFlow][workflow]") { TestCommon::TempFile uninstallResultPath("TestExeUninstalled.txt"); diff --git a/src/AppInstallerRepositoryCore/PackageTrackingCatalog.cpp b/src/AppInstallerRepositoryCore/PackageTrackingCatalog.cpp index a05e3a4527..74084fba8f 100644 --- a/src/AppInstallerRepositoryCore/PackageTrackingCatalog.cpp +++ b/src/AppInstallerRepositoryCore/PackageTrackingCatalog.cpp @@ -215,7 +215,6 @@ namespace AppInstaller::Repository bool isUpgrade) { // TODO: Store additional information from these if needed - UNREFERENCED_PARAMETER(installer); UNREFERENCED_PARAMETER(isUpgrade); auto& index = m_implementation->Source->GetIndex(); @@ -239,6 +238,11 @@ namespace AppInstaller::Repository strstr << Utility::GetCurrentUnixEpoch(); index.SetMetadataByManifestId(manifestId, PackageVersionMetadata::TrackingWriteTime, strstr.str()); + if (installer.RequireExplicitUpgrade) + { + index.SetMetadataByManifestId(manifestId, PackageVersionMetadata::PinnedState, ToString(PackagePinnedState::PinnedByManifest)); + } + std::shared_ptr result = std::make_shared(); result->Id = manifestId; return { std::move(result) }; diff --git a/src/AppInstallerRepositoryCore/Public/winget/RepositorySearch.h b/src/AppInstallerRepositoryCore/Public/winget/RepositorySearch.h index 6e9fe6a8fe..9dd72d70e5 100644 --- a/src/AppInstallerRepositoryCore/Public/winget/RepositorySearch.h +++ b/src/AppInstallerRepositoryCore/Public/winget/RepositorySearch.h @@ -178,11 +178,24 @@ namespace AppInstaller::Repository TrackingWriteTime, // The Architecture of an installed package InstalledArchitecture, + // The PackagePinnedState of the installed package + PinnedState, }; // Convert a PackageVersionMetadata to a string. std::string_view ToString(PackageVersionMetadata pvm); + // Possible pinned states for a package. + // Pinned packages need to be explicitly updated (i.e., are not included in `upgrade --all`) + enum class PackagePinnedState + { + NotPinned, + PinnedByManifest, + }; + + std::string_view ToString(PackagePinnedState state); + PackagePinnedState ConvertToPackagePinnedStateEnum(std::string_view in); + // A single package version. struct IPackageVersion { diff --git a/src/AppInstallerRepositoryCore/RepositorySearch.cpp b/src/AppInstallerRepositoryCore/RepositorySearch.cpp index f27c687c55..0bd637785e 100644 --- a/src/AppInstallerRepositoryCore/RepositorySearch.cpp +++ b/src/AppInstallerRepositoryCore/RepositorySearch.cpp @@ -88,6 +88,30 @@ namespace AppInstaller::Repository } } + std::string_view ToString(PackagePinnedState state) + { + switch (state) + { + case PackagePinnedState::PinnedByManifest: return "PinnedByManifest"sv; + case PackagePinnedState::NotPinned: + default: + return "Unknown"; + } + } + + PackagePinnedState ConvertToPackagePinnedStateEnum(std::string_view in) + { + if (Utility::CaseInsensitiveEquals(in, "PinnedByManifest"sv)) + { + return PackagePinnedState::PinnedByManifest; + } + else + { + return PackagePinnedState::NotPinned; + } + } + + const char* UnsupportedRequestException::what() const noexcept { if (m_whatMessage.empty())