From 4968df49d5ee25565fa9c37719e976c06787d97a Mon Sep 17 00:00:00 2001 From: Easton Pillay Date: Thu, 9 Dec 2021 13:32:08 -0600 Subject: [PATCH] Added argument to control whether to upgrade packages if they have "unknown" versions (#1765) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Added --include-unknown command to `upgrade`. * Added documentation of new arg, and moved if. * Fixed grammar for comment. * Added messaging/loop suggestions from code review. * Apply suggestions from code review (localization) Co-authored-by: Chacón * Added count of unknown packages. * Added count to upgrade --all too. * Attempted to fix comments/phrasing for localization. * Undid change to AppInstallerCLICore project -- sorry! * Removed unnecessary if statement for ShouldListUpgrade. Co-authored-by: Chacón --- doc/windows/package-manager/winget/upgrade.md | 10 +- .../Commands/UpgradeCommand.cpp | 11 ++- src/AppInstallerCLICore/ExecutionArgs.h | 13 +++ src/AppInstallerCLICore/Resources.h | 4 + .../Workflows/UpdateFlow.cpp | 95 ++++++++++++------- .../Workflows/WorkflowBase.cpp | 12 +++ .../Shared/Strings/en-us/winget.resw | 15 +++ 7 files changed, 116 insertions(+), 44 deletions(-) diff --git a/doc/windows/package-manager/winget/upgrade.md b/doc/windows/package-manager/winget/upgrade.md index afb54ee714..42acf7b7be 100644 --- a/doc/windows/package-manager/winget/upgrade.md +++ b/doc/windows/package-manager/winget/upgrade.md @@ -18,15 +18,6 @@ The **upgrade** command requires that you specify the exact string to upgrade. I ![upgrade command](images/upgrade.png) -## Arguments - -The following arguments are available. - -| Argument | Description | -|-------------|-------------| -| **-q,--query** | The query used to search for an app. | -| **-?, --help** | Get additional help on this command. | - ## Options The options allow you to customize the upgrade experience to meet your needs. @@ -47,6 +38,7 @@ The options allow you to customize the upgrade experience to meet your needs. | **-l, --location** | Location to upgrade to (if supported). | | **--force** | When a hash mismatch is discovered will ignore the error and attempt to install the package. | | **--all** | Updates all available packages to the latest application. | +| **--include-unknown** | Attempt to upgrade a package even if the package's current version is unknown. | ### Example queries The following example upgrades a specific version of an application. diff --git a/src/AppInstallerCLICore/Commands/UpgradeCommand.cpp b/src/AppInstallerCLICore/Commands/UpgradeCommand.cpp index 783cd32252..9a0b182690 100644 --- a/src/AppInstallerCLICore/Commands/UpgradeCommand.cpp +++ b/src/AppInstallerCLICore/Commands/UpgradeCommand.cpp @@ -19,8 +19,14 @@ namespace AppInstaller::CLI { bool ShouldListUpgrade(Context& context) { - return context.Args.Empty() || - (context.Args.GetArgsCount() == 1 && context.Args.Contains(Execution::Args::Type::Source)); + for (Execution::Args::Type type : context.Args.GetTypes()) + { + if (type != Execution::Args::Type::Source && type != Execution::Args::Type::IncludeUnknown) + { + return false; + } + } + return true; } } @@ -46,6 +52,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 } }; } diff --git a/src/AppInstallerCLICore/ExecutionArgs.h b/src/AppInstallerCLICore/ExecutionArgs.h index 5632992b75..f5d231c6af 100644 --- a/src/AppInstallerCLICore/ExecutionArgs.h +++ b/src/AppInstallerCLICore/ExecutionArgs.h @@ -85,6 +85,7 @@ 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 // Used for demonstration purposes ExperimentalArg, @@ -141,6 +142,18 @@ namespace AppInstaller::CLI::Execution return m_parsedArgs.size(); } + std::vector GetTypes() + { + std::vector types; + + for (auto const& i : m_parsedArgs) + { + types.emplace_back(i.first); + } + + return types; + } + private: std::map> m_parsedArgs; }; diff --git a/src/AppInstallerCLICore/Resources.h b/src/AppInstallerCLICore/Resources.h index 09629e5437..5f5af6ce33 100644 --- a/src/AppInstallerCLICore/Resources.h +++ b/src/AppInstallerCLICore/Resources.h @@ -104,6 +104,7 @@ namespace AppInstaller::CLI::Resource WINGET_DEFINE_RESOURCE_STRINGID(ImportPackageAlreadyInstalled); WINGET_DEFINE_RESOURCE_STRINGID(ImportSearchFailed); WINGET_DEFINE_RESOURCE_STRINGID(ImportSourceNotInstalled); + WINGET_DEFINE_RESOURCE_STRINGID(IncludeUnknownArgumentDescription); WINGET_DEFINE_RESOURCE_STRINGID(InstallAndUpgradeCommandsReportDependencies); WINGET_DEFINE_RESOURCE_STRINGID(InstallArchitectureArgumentDescription); WINGET_DEFINE_RESOURCE_STRINGID(InstallationAbandoned); @@ -340,6 +341,9 @@ namespace AppInstaller::CLI::Resource 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(UpgradeUnknownVersionExplanation); WINGET_DEFINE_RESOURCE_STRINGID(Usage); WINGET_DEFINE_RESOURCE_STRINGID(ValidateCommandLongDescription); WINGET_DEFINE_RESOURCE_STRINGID(ValidateCommandReportDependencies); diff --git a/src/AppInstallerCLICore/Workflows/UpdateFlow.cpp b/src/AppInstallerCLICore/Workflows/UpdateFlow.cpp index a64924d1e3..861d97f42c 100644 --- a/src/AppInstallerCLICore/Workflows/UpdateFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/UpdateFlow.cpp @@ -44,30 +44,32 @@ namespace AppInstaller::CLI::Workflow bool updateFound = false; bool installedTypeInapplicable = false; - // The version keys should have already been sorted by version - const auto& versionKeys = package->GetAvailableVersionKeys(); - for (const auto& key : versionKeys) + if (!installedVersion.IsUnknown() || context.Args.Contains(Execution::Args::Type::IncludeUnknown)) { - // Check Update Version - if (IsUpdateVersionApplicable(installedVersion, Utility::Version(key.Version))) + // The version keys should have already been sorted by version + const auto& versionKeys = package->GetAvailableVersionKeys(); + for (const auto& key : versionKeys) { - auto packageVersion = package->GetAvailableVersion(key); - auto manifest = packageVersion->GetManifest(); - - // Check applicable Installer - auto [installer, inapplicabilities] = manifestComparator.GetPreferredInstaller(manifest); - if (!installer.has_value()) + // Check Update Version + if (IsUpdateVersionApplicable(installedVersion, Utility::Version(key.Version))) { - // 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()) + auto packageVersion = package->GetAvailableVersion(key); + auto manifest = packageVersion->GetManifest(); + + // Check applicable Installer + auto [installer, inapplicabilities] = manifestComparator.GetPreferredInstaller(manifest); + if (!installer.has_value()) { - installedTypeInapplicable = true; + // 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; } - continue; - } - Logging::Telemetry().LogSelectedInstaller( static_cast(installer->Arch), installer->Url, @@ -86,16 +88,25 @@ namespace AppInstaller::CLI::Workflow context.Add(std::move(packageVersion)); context.Add(std::move(installer)); - updateFound = true; - break; + updateFound = true; + break; + } + else + { + // Any following versions are not applicable + break; + } } - else + } + else + { + // the package has an unknown version and the user did not request to upgrade it anyway. + if (m_reportUpdateNotFound) { - // Any following versions are not applicable - break; + context.Reporter.Info() << Resource::String::UpgradeUnknownVersionExplanation << std::endl; } + AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_UPDATE_NOT_APPLICABLE); } - if (!updateFound) { if (m_reportUpdateNotFound) @@ -132,6 +143,7 @@ namespace AppInstaller::CLI::Workflow const auto& matches = context.Get().Matches; std::vector> packagesToInstall; bool updateAllFoundUpdate = false; + int unknownPackagesCount = 0; for (const auto& match : matches) { @@ -139,8 +151,20 @@ namespace AppInstaller::CLI::Workflow auto updateContextPtr = context.CreateSubContext(); Execution::Context& updateContext = *updateContextPtr; auto previousThreadGlobals = updateContext.SetForCurrentThread(); + auto installedVersion = match.Package->GetInstalledVersion(); updateContext.Add(match.Package); + + if (context.Args.Contains(Execution::Args::Type::IncludeUnknown)) + { + updateContext.Args.AddArg(Execution::Args::Type::IncludeUnknown); + } + 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++; + continue; + } updateContext << Workflow::GetInstalledPackageVersion << @@ -160,14 +184,19 @@ namespace AppInstaller::CLI::Workflow if (!updateAllFoundUpdate) { context.Reporter.Info() << Resource::String::UpdateNotApplicable << std::endl; - return; } - - context.Add(std::move(packagesToInstall)); - context << - InstallMultiplePackages( - Resource::String::InstallAndUpgradeCommandsReportDependencies, - APPINSTALLER_CLI_ERROR_UPDATE_ALL_HAS_FAILURE, - { APPINSTALLER_CLI_ERROR_UPDATE_NOT_APPLICABLE }); + else + { + context.Add(std::move(packagesToInstall)); + context << + InstallMultiplePackages( + Resource::String::InstallAndUpgradeCommandsReportDependencies, + APPINSTALLER_CLI_ERROR_UPDATE_ALL_HAS_FAILURE, + { APPINSTALLER_CLI_ERROR_UPDATE_NOT_APPLICABLE }); + } + if (unknownPackagesCount > 0 && !context.Args.Contains(Execution::Args::Type::IncludeUnknown)) + { + context.Reporter.Info() << unknownPackagesCount << " " << (unknownPackagesCount == 1 ? Resource::String::UpgradeUnknownCountSingle : Resource::String::UpgradeUnknownCount) << std::endl; + } } -} \ No newline at end of file +} diff --git a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp index e34c970dbc..56c18c569c 100644 --- a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp +++ b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp @@ -708,6 +708,7 @@ namespace AppInstaller::CLI::Workflow }); int availableUpgradesCount = 0; + int unknownPackagesCount = 0; auto &source = context.Get(); bool shouldShowSource = source.IsComposite() && source.GetAvailableSources().size() > 1; @@ -720,6 +721,13 @@ namespace AppInstaller::CLI::Workflow auto latestVersion = match.Package->GetLatestAvailableVersion(); bool updateAvailable = match.Package->IsUpdateAvailable(); + if (m_onlyShowUpgrades && !context.Args.Contains(Execution::Args::Type::IncludeUnknown) && Utility::Version(installedVersion->GetProperty(PackageVersionProperty::Version)).IsUnknown()) + { + // We are only showing upgrades, and the user did not request to include packages with unknown versions. + unknownPackagesCount++; + continue; + } + // The only time we don't want to output a line is when filtering and no update is available. if (updateAvailable || !m_onlyShowUpgrades) { @@ -766,6 +774,10 @@ 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)) + { + context.Reporter.Info() << unknownPackagesCount << " " << (unknownPackagesCount == 1 ? Resource::String::UpgradeUnknownCountSingle : Resource::String::UpgradeUnknownCount) << std::endl; + } } diff --git a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw index 9600bf50eb..bdc7b6b6d7 100644 --- a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw +++ b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw @@ -1250,4 +1250,19 @@ Please specify one of them using the `--source` option to proceed. Select the architecture to install + + Upgrade packages even if their current version cannot be determined + + + This package's version number cannot be determined. To upgrade it anyway, add the argument --include-unknown to your previous command. + {Locked="--include-unknown"} + + + packages 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 has a version number 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. + \ No newline at end of file