commit 4b613b3057df01b8225ff50a73af971fa27d02b5 parent 1dfa17c89cc16f4844bf76cc8cda093271371684 Author: yao-msft <50888816+yao-msft@users.noreply.github.com> Date: Fri, 7 Oct 2022 16:50:22 -0700 Improve installer selection during upgrade (#2570) Diffstat:
17 files changed, 470 insertions(+), 229 deletions(-)
diff --git a/src/AppInstallerCLICore/Argument.cpp b/src/AppInstallerCLICore/Argument.cpp @@ -5,100 +5,104 @@ #include "Command.h" #include "Resources.h" #include <winget/UserSettings.h> +#include <winget/LocIndependent.h> namespace AppInstaller::CLI { using namespace AppInstaller::CLI::Execution; using namespace Settings; + using namespace AppInstaller::Utility::literals; Argument Argument::ForType(Execution::Args::Type type) { switch (type) { case Args::Type::Query: - return Argument{ "query", 'q', Args::Type::Query, Resource::String::QueryArgumentDescription, ArgumentType::Positional}; + return Argument{ "query"_liv, 'q', Args::Type::Query, Resource::String::QueryArgumentDescription, ArgumentType::Positional}; case Args::Type::Manifest: - return Argument{ "manifest", 'm', Args::Type::Manifest, Resource::String::ManifestArgumentDescription, ArgumentType::Standard, Argument::Visibility::Help, Settings::TogglePolicy::Policy::LocalManifestFiles, Settings::AdminSetting::LocalManifestFiles }; + return Argument{ "manifest"_liv, 'm', Args::Type::Manifest, Resource::String::ManifestArgumentDescription, ArgumentType::Standard, Argument::Visibility::Help, Settings::TogglePolicy::Policy::LocalManifestFiles, Settings::AdminSetting::LocalManifestFiles }; case Args::Type::Id: - return Argument{ "id", NoAlias, Args::Type::Id,Resource::String::IdArgumentDescription, ArgumentType::Standard, Argument::Visibility::Help }; + return Argument{ "id"_liv, NoAlias, Args::Type::Id,Resource::String::IdArgumentDescription, ArgumentType::Standard, Argument::Visibility::Help }; case Args::Type::Name: - return Argument{ "name", NoAlias, Args::Type::Name, Resource::String::NameArgumentDescription, ArgumentType::Standard, Argument::Visibility::Help }; + return Argument{ "name"_liv, NoAlias, Args::Type::Name, Resource::String::NameArgumentDescription, ArgumentType::Standard, Argument::Visibility::Help }; case Args::Type::Moniker: - return Argument{ "moniker", NoAlias, Args::Type::Moniker, Resource::String::MonikerArgumentDescription, ArgumentType::Standard, Argument::Visibility::Help }; + return Argument{ "moniker"_liv, NoAlias, Args::Type::Moniker, Resource::String::MonikerArgumentDescription, ArgumentType::Standard, Argument::Visibility::Help }; case Args::Type::Tag: - return Argument{ "tag", NoAlias, Args::Type::Tag, Resource::String::TagArgumentDescription, ArgumentType::Standard, Argument::Visibility::Help }; + return Argument{ "tag"_liv, NoAlias, Args::Type::Tag, Resource::String::TagArgumentDescription, ArgumentType::Standard, Argument::Visibility::Help }; case Args::Type::Command: - return Argument{ "command", NoAlias, "cmd", Args::Type::Command, Resource::String::CommandArgumentDescription, ArgumentType::Standard, Argument::Visibility::Help }; + return Argument{ "command"_liv, NoAlias, "cmd"_liv, Args::Type::Command, Resource::String::CommandArgumentDescription, ArgumentType::Standard, Argument::Visibility::Help }; case Args::Type::Source: - return Argument{ "source", 's', Args::Type::Source, Resource::String::SourceArgumentDescription, ArgumentType::Standard }; + return Argument{ "source"_liv, 's', Args::Type::Source, Resource::String::SourceArgumentDescription, ArgumentType::Standard }; case Args::Type::DependencySource: - return Argument{ "dependency-source", NoAlias, Args::Type::DependencySource, Resource::String::DependencySourceArgumentDescription, ArgumentType::Standard }; + return Argument{ "dependency-source"_liv, NoAlias, Args::Type::DependencySource, Resource::String::DependencySourceArgumentDescription, ArgumentType::Standard }; case Args::Type::Count: - return Argument{ "count", 'n', Args::Type::Count, Resource::String::CountArgumentDescription, ArgumentType::Standard }; + return Argument{ "count"_liv, 'n', Args::Type::Count, Resource::String::CountArgumentDescription, ArgumentType::Standard }; case Args::Type::Exact: - return Argument{ "exact", 'e', Args::Type::Exact, Resource::String::ExactArgumentDescription, ArgumentType::Flag }; + return Argument{ "exact"_liv, 'e', Args::Type::Exact, Resource::String::ExactArgumentDescription, ArgumentType::Flag }; case Args::Type::Version: - return Argument{ "version", 'v', Args::Type::Version, Resource::String::VersionArgumentDescription, ArgumentType::Standard }; + return Argument{ "version"_liv, 'v', Args::Type::Version, Resource::String::VersionArgumentDescription, ArgumentType::Standard }; case Args::Type::Channel: - return Argument{ "channel", 'c', Args::Type::Channel, Resource::String::ChannelArgumentDescription, ArgumentType::Standard, Argument::Visibility::Hidden }; + return Argument{ "channel"_liv, 'c', Args::Type::Channel, Resource::String::ChannelArgumentDescription, ArgumentType::Standard, Argument::Visibility::Hidden }; case Args::Type::Interactive: - return Argument{ "interactive", 'i', Args::Type::Interactive, Resource::String::InteractiveArgumentDescription, ArgumentType::Flag }; + return Argument{ "interactive"_liv, 'i', Args::Type::Interactive, Resource::String::InteractiveArgumentDescription, ArgumentType::Flag }; case Args::Type::Silent: - return Argument{ "silent", 'h', Args::Type::Silent, Resource::String::SilentArgumentDescription, ArgumentType::Flag }; + return Argument{ "silent"_liv, 'h', Args::Type::Silent, Resource::String::SilentArgumentDescription, ArgumentType::Flag }; case Args::Type::Locale: - return Argument{ "locale", NoAlias, Args::Type::Locale, Resource::String::LocaleArgumentDescription, ArgumentType::Standard }; + return Argument{ "locale"_liv, NoAlias, Args::Type::Locale, Resource::String::LocaleArgumentDescription, ArgumentType::Standard }; + case Args::Type::InstallArchitecture: + return Argument{ "architecture"_liv, 'a', Args::Type::InstallArchitecture, Resource::String::InstallArchitectureArgumentDescription, ArgumentType::Standard, Argument::Visibility::Help }; case Args::Type::Log: - return Argument{ "log", 'o', Args::Type::Log, Resource::String::LogArgumentDescription, ArgumentType::Standard }; + return Argument{ "log"_liv, 'o', Args::Type::Log, Resource::String::LogArgumentDescription, ArgumentType::Standard }; case Args::Type::Override: - return Argument{ "override", NoAlias, Args::Type::Override, Resource::String::OverrideArgumentDescription, ArgumentType::Standard, Argument::Visibility::Help }; + return Argument{ "override"_liv, NoAlias, Args::Type::Override, Resource::String::OverrideArgumentDescription, ArgumentType::Standard, Argument::Visibility::Help }; case Args::Type::InstallLocation: - return Argument{ "location", 'l', Args::Type::InstallLocation, Resource::String::LocationArgumentDescription, ArgumentType::Standard }; + return Argument{ "location"_liv, 'l', Args::Type::InstallLocation, Resource::String::LocationArgumentDescription, ArgumentType::Standard }; case Args::Type::HashOverride: - return Argument{ "force", NoAlias, Args::Type::HashOverride, Resource::String::InstallForceArgumentDescription, ArgumentType::Flag, Settings::TogglePolicy::Policy::HashOverride }; + return Argument{ "force"_liv, NoAlias, Args::Type::HashOverride, Resource::String::InstallForceArgumentDescription, ArgumentType::Flag, Settings::TogglePolicy::Policy::HashOverride }; case Args::Type::AcceptPackageAgreements: - return Argument{ "accept-package-agreements", NoAlias, Args::Type::AcceptPackageAgreements, Resource::String::AcceptPackageAgreementsArgumentDescription, ArgumentType::Flag }; + return Argument{ "accept-package-agreements"_liv, NoAlias, Args::Type::AcceptPackageAgreements, Resource::String::AcceptPackageAgreementsArgumentDescription, ArgumentType::Flag }; case Args::Type::HashFile: - return Argument{ "file", 'f', Args::Type::HashFile, Resource::String::FileArgumentDescription, ArgumentType::Positional, true }; + return Argument{ "file"_liv, 'f', Args::Type::HashFile, Resource::String::FileArgumentDescription, ArgumentType::Positional, true }; case Args::Type::Msix: - return Argument{ "msix", 'm', Args::Type::Msix, Resource::String::MsixArgumentDescription, ArgumentType::Flag }; + return Argument{ "msix"_liv, 'm', Args::Type::Msix, Resource::String::MsixArgumentDescription, ArgumentType::Flag }; case Args::Type::ListVersions: - return Argument{ "versions", NoAlias, Args::Type::ListVersions, Resource::String::VersionsArgumentDescription, ArgumentType::Flag }; + return Argument{ "versions"_liv, NoAlias, Args::Type::ListVersions, Resource::String::VersionsArgumentDescription, ArgumentType::Flag }; case Args::Type::Help: - return Argument{ "help", APPINSTALLER_CLI_HELP_ARGUMENT_TEXT_CHAR, Args::Type::Help, Resource::String::HelpArgumentDescription, ArgumentType::Flag }; + return Argument{ "help"_liv, APPINSTALLER_CLI_HELP_ARGUMENT_TEXT_CHAR, Args::Type::Help, Resource::String::HelpArgumentDescription, ArgumentType::Flag }; case Args::Type::SourceName: - return Argument{ "name", 'n', Args::Type::SourceName,Resource::String::SourceNameArgumentDescription, ArgumentType::Positional, false }; + return Argument{ "name"_liv, 'n', Args::Type::SourceName,Resource::String::SourceNameArgumentDescription, ArgumentType::Positional, false }; case Args::Type::SourceArg: - return Argument{ "arg", 'a', Args::Type::SourceArg, Resource::String::SourceArgArgumentDescription, ArgumentType::Positional, true }; + return Argument{ "arg"_liv, 'a', Args::Type::SourceArg, Resource::String::SourceArgArgumentDescription, ArgumentType::Positional, true }; case Args::Type::SourceType: - return Argument{ "type", 't', Args::Type::SourceType, Resource::String::SourceTypeArgumentDescription, ArgumentType::Positional }; + return Argument{ "type"_liv, 't', Args::Type::SourceType, Resource::String::SourceTypeArgumentDescription, ArgumentType::Positional }; case Args::Type::ValidateManifest: - return Argument{ "manifest", NoAlias, Args::Type::ValidateManifest, Resource::String::ValidateManifestArgumentDescription, ArgumentType::Positional, true }; + return Argument{ "manifest"_liv, NoAlias, Args::Type::ValidateManifest, Resource::String::ValidateManifestArgumentDescription, ArgumentType::Positional, true }; case Args::Type::NoVT: - return Argument{ "no-vt", NoAlias, Args::Type::NoVT, Resource::String::NoVTArgumentDescription, ArgumentType::Flag, Argument::Visibility::Hidden }; + return Argument{ "no-vt"_liv, NoAlias, Args::Type::NoVT, Resource::String::NoVTArgumentDescription, ArgumentType::Flag, Argument::Visibility::Hidden }; case Args::Type::RainbowStyle: - return Argument{ "rainbow", NoAlias, Args::Type::RainbowStyle, Resource::String::RainbowArgumentDescription, ArgumentType::Flag, Argument::Visibility::Hidden }; + return Argument{ "rainbow"_liv, NoAlias, Args::Type::RainbowStyle, Resource::String::RainbowArgumentDescription, ArgumentType::Flag, Argument::Visibility::Hidden }; case Args::Type::RetroStyle: - return Argument{ "retro", NoAlias, Args::Type::RetroStyle, Resource::String::RetroArgumentDescription, ArgumentType::Flag, Argument::Visibility::Hidden }; + return Argument{ "retro"_liv, NoAlias, Args::Type::RetroStyle, Resource::String::RetroArgumentDescription, ArgumentType::Flag, Argument::Visibility::Hidden }; case Args::Type::VerboseLogs: - return Argument{ "verbose-logs", NoAlias, "verbose", Args::Type::VerboseLogs, Resource::String::VerboseLogsArgumentDescription, ArgumentType::Flag}; + return Argument{ "verbose-logs"_liv, NoAlias, "verbose"_liv, Args::Type::VerboseLogs, Resource::String::VerboseLogsArgumentDescription, ArgumentType::Flag}; case Args::Type::CustomHeader: - return Argument{ "header", NoAlias, Args::Type::CustomHeader, Resource::String::HeaderArgumentDescription, ArgumentType::Standard, Argument::Visibility::Help }; + return Argument{ "header"_liv, NoAlias, Args::Type::CustomHeader, Resource::String::HeaderArgumentDescription, ArgumentType::Standard, Argument::Visibility::Help }; case Args::Type::AcceptSourceAgreements: - return Argument{ "accept-source-agreements", NoAlias, Args::Type::AcceptSourceAgreements, Resource::String::AcceptSourceAgreementsArgumentDescription, ArgumentType::Flag }; + return Argument{ "accept-source-agreements"_liv, NoAlias, Args::Type::AcceptSourceAgreements, Resource::String::AcceptSourceAgreementsArgumentDescription, ArgumentType::Flag }; case Args::Type::ExperimentalArg: - return Argument{ "arg", NoAlias, Args::Type::ExperimentalArg, Resource::String::ExperimentalArgumentDescription, ArgumentType::Flag, ExperimentalFeature::Feature::ExperimentalArg }; + return Argument{ "arg"_liv, NoAlias, Args::Type::ExperimentalArg, Resource::String::ExperimentalArgumentDescription, ArgumentType::Flag, ExperimentalFeature::Feature::ExperimentalArg }; case Args::Type::Rename: - return Argument{ "rename", 'r', Args::Type::Rename, Resource::String::RenameArgumentDescription, ArgumentType::Standard, false }; + return Argument{ "rename"_liv, 'r', Args::Type::Rename, Resource::String::RenameArgumentDescription, ArgumentType::Standard, false }; case Args::Type::Purge: - return Argument{ "purge", NoAlias, Args::Type::Purge, Resource::String::PurgeArgumentDescription, ArgumentType::Flag, false }; + return Argument{ "purge"_liv, NoAlias, Args::Type::Purge, Resource::String::PurgeArgumentDescription, ArgumentType::Flag, false }; case Args::Type::Preserve: - return Argument{ "preserve", NoAlias, Args::Type::Preserve, Resource::String::PreserveArgumentDescription, ArgumentType::Flag, false }; + return Argument{ "preserve"_liv, NoAlias, Args::Type::Preserve, Resource::String::PreserveArgumentDescription, ArgumentType::Flag, false }; case Args::Type::Wait: - return Argument{ "wait", NoAlias, Args::Type::Wait, Resource::String::WaitArgumentDescription, ArgumentType::Flag, false }; + return Argument{ "wait"_liv, NoAlias, Args::Type::Wait, Resource::String::WaitArgumentDescription, ArgumentType::Flag, false }; case Args::Type::ProductCode: - return Argument{ "product-code", NoAlias, Args::Type::ProductCode, Resource::String::ProductCodeArgumentDescription, ArgumentType::Standard, false }; + return Argument{ "product-code"_liv, NoAlias, Args::Type::ProductCode, Resource::String::ProductCodeArgumentDescription, ArgumentType::Standard, false }; case Args::Type::OpenLogs: - return Argument{ "open-logs", NoAlias, "logs", Args::Type::OpenLogs, Resource::String::OpenLogsArgumentDescription, ArgumentType::Flag, ExperimentalFeature::Feature::OpenLogsArgument}; + return Argument{ "open-logs"_liv, NoAlias, "logs"_liv, Args::Type::OpenLogs, Resource::String::OpenLogsArgumentDescription, ArgumentType::Flag, ExperimentalFeature::Feature::OpenLogsArgument}; default: THROW_HR(E_UNEXPECTED); } diff --git a/src/AppInstallerCLICore/Command.cpp b/src/AppInstallerCLICore/Command.cpp @@ -708,6 +708,28 @@ namespace AppInstaller::CLI } } + if (execArgs.Contains(Execution::Args::Type::InstallArchitecture)) + { + Utility::Architecture selectedArch = Utility::ConvertToArchitectureEnum(std::string(execArgs.GetArg(Execution::Args::Type::InstallArchitecture))); + if ((selectedArch == Utility::Architecture::Unknown) || (Utility::IsApplicableArchitecture(selectedArch) == Utility::InapplicableArchitecture)) + { + std::vector<Utility::LocIndString> applicableArchitectures; + for (Utility::Architecture i : Utility::GetApplicableArchitectures()) + { + applicableArchitectures.emplace_back(Utility::ToString(i)); + } + throw CommandException(Resource::String::InvalidArgumentValueError, Argument::ForType(Execution::Args::Type::InstallArchitecture).Name(), std::forward<std::vector<Utility::LocIndString>>((applicableArchitectures))); + } + } + + if (execArgs.Contains(Execution::Args::Type::Locale)) + { + if (!Locale::IsWellFormedBcp47Tag(execArgs.GetArg(Execution::Args::Type::Locale))) + { + throw CommandException(Resource::String::InvalidArgumentValueErrorWithoutValidValues, Argument::ForType(Execution::Args::Type::Locale).Name(), {}); + } + } + ValidateArgumentsInternal(execArgs); } diff --git a/src/AppInstallerCLICore/Commands/InstallCommand.cpp b/src/AppInstallerCLICore/Commands/InstallCommand.cpp @@ -17,7 +17,6 @@ namespace AppInstaller::CLI namespace { constexpr Utility::LocIndView s_ArgumentName_Scope = "scope"_liv; - constexpr Utility::LocIndView s_ArgumentName_Architecture = "architecture"_liv; } std::vector<Argument> InstallCommand::GetArguments() const @@ -32,7 +31,7 @@ namespace AppInstaller::CLI Argument::ForType(Args::Type::Channel), Argument::ForType(Args::Type::Source), Argument{ s_ArgumentName_Scope, Argument::NoAlias, Args::Type::InstallScope, Resource::String::InstallScopeDescription, ArgumentType::Standard, Argument::Visibility::Help }, - Argument{ s_ArgumentName_Architecture, 'a', Args::Type::InstallArchitecture, Resource::String::InstallArchitectureArgumentDescription, ArgumentType::Standard, Argument::Visibility::Help}, + Argument::ForType(Args::Type::InstallArchitecture), Argument::ForType(Args::Type::Exact), Argument::ForType(Args::Type::Interactive), Argument::ForType(Args::Type::Silent), @@ -74,6 +73,7 @@ namespace AppInstaller::CLI context << Workflow::CompleteWithSingleSemanticsForValue(valueType); break; + case Args::Type::InstallArchitecture: case Args::Type::Locale: // May well move to CompleteWithSingleSemanticsForValue, // but for now output nothing. @@ -116,27 +116,6 @@ namespace AppInstaller::CLI throw CommandException(Resource::String::InvalidArgumentValueError, s_ArgumentName_Scope, { "user"_lis, "machine"_lis }); } } - if (execArgs.Contains(Args::Type::InstallArchitecture)) - { - Utility::Architecture selectedArch = Utility::ConvertToArchitectureEnum(std::string(execArgs.GetArg(Args::Type::InstallArchitecture))); - if ((selectedArch == Utility::Architecture::Unknown) || (Utility::IsApplicableArchitecture(selectedArch) == Utility::InapplicableArchitecture)) - { - std::vector<Utility::LocIndString> applicableArchitectures; - for (Utility::Architecture i : Utility::GetApplicableArchitectures()) - { - applicableArchitectures.emplace_back(Utility::ToString(i)); - } - throw CommandException(Resource::String::InvalidArgumentValueError, s_ArgumentName_Architecture, std::forward<std::vector<Utility::LocIndString>>((applicableArchitectures))); - } - } - - if (execArgs.Contains(Args::Type::Locale)) - { - if (!Locale::IsWellFormedBcp47Tag(execArgs.GetArg(Args::Type::Locale))) - { - throw CommandException(Resource::String::InvalidArgumentValueErrorWithoutValidValues, Argument::ForType(Args::Type::Locale).Name(), {}); - } - } } void InstallCommand::ExecuteInternal(Context& context) const diff --git a/src/AppInstallerCLICore/Commands/ShowCommand.cpp b/src/AppInstallerCLICore/Commands/ShowCommand.cpp @@ -24,6 +24,8 @@ namespace AppInstaller::CLI Argument::ForType(Execution::Args::Type::Channel), Argument::ForType(Execution::Args::Type::Source), Argument::ForType(Execution::Args::Type::Exact), + Argument::ForType(Execution::Args::Type::InstallArchitecture), + Argument::ForType(Execution::Args::Type::Locale), Argument::ForType(Execution::Args::Type::ListVersions), Argument::ForType(Execution::Args::Type::CustomHeader), Argument::ForType(Execution::Args::Type::AcceptSourceAgreements), diff --git a/src/AppInstallerCLICore/Commands/UpgradeCommand.cpp b/src/AppInstallerCLICore/Commands/UpgradeCommand.cpp @@ -8,10 +8,12 @@ #include "Workflows/WorkflowBase.h" #include "Workflows/DependenciesFlow.h" #include "Resources.h" +#include <winget/LocIndependent.h> using namespace AppInstaller::CLI::Execution; using namespace AppInstaller::Manifest; using namespace AppInstaller::CLI::Workflow; +using namespace AppInstaller::Utility::literals; namespace AppInstaller::CLI { @@ -98,12 +100,14 @@ namespace AppInstaller::CLI Argument::ForType(Args::Type::Log), // -o Argument::ForType(Args::Type::Override), Argument::ForType(Args::Type::InstallLocation), // -l + Argument::ForType(Args::Type::InstallArchitecture), // -a + Argument::ForType(Args::Type::Locale), Argument::ForType(Args::Type::HashOverride), Argument::ForType(Args::Type::AcceptPackageAgreements), Argument::ForType(Args::Type::AcceptSourceAgreements), Argument::ForType(Execution::Args::Type::CustomHeader), - Argument{ "all", 'a', Args::Type::All, Resource::String::UpdateAllArgumentDescription, ArgumentType::Flag}, - Argument{ "include-unknown", 'u', Args::Type::IncludeUnknown, Resource::String::IncludeUnknownArgumentDescription, ArgumentType::Flag}, + Argument{ "all"_liv, 'r', "recurse"_liv, Args::Type::All, Resource::String::UpdateAllArgumentDescription, ArgumentType::Flag }, + Argument{ "include-unknown"_liv, 'u', "unknown"_liv, Args::Type::IncludeUnknown, Resource::String::IncludeUnknownArgumentDescription, ArgumentType::Flag }, }; } @@ -149,6 +153,13 @@ namespace AppInstaller::CLI context << CompleteWithSingleSemanticsForValueUsingExistingSource(valueType); break; + case Args::Type::InstallArchitecture: + case Args::Type::Locale: + // May well move to CompleteWithSingleSemanticsForValue, + // but for now output nothing. + context << + Workflow::CompleteWithEmptySet; + break; } } diff --git a/src/AppInstallerCLICore/Workflows/InstallFlow.cpp b/src/AppInstallerCLICore/Workflows/InstallFlow.cpp @@ -674,9 +674,42 @@ namespace AppInstaller::CLI::Workflow auto trackingCatalog = context.Get<Data::PackageVersion>()->GetSource().GetTrackingCatalog(); - trackingCatalog.RecordInstall( + auto version = trackingCatalog.RecordInstall( manifest, context.Get<Data::Installer>().value(), WI_IsFlagSet(context.GetFlags(), ContextFlag::InstallerExecutionUseUpdate)); + + // Record user intent values. Command args takes precedence. Then previous user intent values. + Repository::IPackageVersion::Metadata installedMetadata; + if (context.Contains(Data::InstalledPackageVersion) && context.Get<Execution::Data::InstalledPackageVersion>()) + { + installedMetadata = context.Get<Data::InstalledPackageVersion>()->GetMetadata(); + } + + if (context.Args.Contains(Execution::Args::Type::InstallArchitecture)) + { + version.SetMetadata(Repository::PackageVersionMetadata::UserIntentArchitecture, context.Args.GetArg(Execution::Args::Type::InstallArchitecture)); + } + else + { + auto itr = installedMetadata.find(Repository::PackageVersionMetadata::UserIntentArchitecture); + if (itr != installedMetadata.end()) + { + version.SetMetadata(Repository::PackageVersionMetadata::UserIntentArchitecture, itr->second); + } + } + + if (context.Args.Contains(Execution::Args::Type::Locale)) + { + version.SetMetadata(Repository::PackageVersionMetadata::UserIntentLocale, context.Args.GetArg(Execution::Args::Type::Locale)); + } + else + { + auto itr = installedMetadata.find(Repository::PackageVersionMetadata::UserIntentLocale); + if (itr != installedMetadata.end()) + { + version.SetMetadata(Repository::PackageVersionMetadata::UserIntentLocale, itr->second); + } + } } } diff --git a/src/AppInstallerCLICore/Workflows/ManifestComparator.cpp b/src/AppInstallerCLICore/Workflows/ManifestComparator.cpp @@ -77,52 +77,100 @@ namespace AppInstaller::CLI::Workflow AICLI_LOG(CLI, Verbose, << "Architecture Comparator created with allowed architectures: " << Utility::ConvertContainerToString(m_allowedArchitectures, Utility::ToString)); } - // TODO: At some point we can do better about matching the currently installed architecture - static std::unique_ptr<MachineArchitectureComparator> Create(const Execution::Context& context, const Repository::IPackageVersion::Metadata&) + static std::unique_ptr<MachineArchitectureComparator> Create(const Execution::Context& context, const Repository::IPackageVersion::Metadata& metadata) { + std::vector<Utility::Architecture> allowedArchitectures; + if (context.Contains(Execution::Data::AllowedArchitectures)) { - const std::vector<Utility::Architecture>& allowedArchitectures = context.Get<Execution::Data::AllowedArchitectures>(); - if (!allowedArchitectures.empty()) + // Com caller can directly set allowed architectures + allowedArchitectures = context.Get<Execution::Data::AllowedArchitectures>(); + } + else if (context.Args.Contains(Execution::Args::Type::InstallArchitecture)) + { + // Arguments provided in command line + allowedArchitectures.emplace_back(Utility::ConvertToArchitectureEnum(context.Args.GetArg(Execution::Args::Type::InstallArchitecture))); + } + else + { + auto userIntentItr = metadata.find(Repository::PackageVersionMetadata::UserIntentArchitecture); + auto installedItr = metadata.find(Repository::PackageVersionMetadata::InstalledArchitecture); + if (userIntentItr != metadata.end()) { - // If the incoming data contains elements, we will use them to construct a final allowed list. - // The algorithm is to take elements until we find Unknown, which indicates that any architecture is - // acceptable at this point. The system supported set of architectures will then be placed at the end. - std::vector<Utility::Architecture> result; - bool addRemainingApplicableArchitectures = false; + // For upgrade, user intent from previous install is considered requirement + allowedArchitectures.emplace_back(Utility::ConvertToArchitectureEnum(userIntentItr->second)); + } + else + { + if (installedItr != metadata.end()) + { + // For upgrade, previous installed architecture should be considered first preference and is always allowed. + // Then check settings requirements and preferences. + allowedArchitectures.emplace_back(Utility::ConvertToArchitectureEnum(installedItr->second)); + } - for (Utility::Architecture architecture : allowedArchitectures) + std::vector<Utility::Architecture> requiredArchitectures = Settings::User().Get<Settings::Setting::InstallArchitectureRequirement>(); + std::vector<Utility::Architecture> optionalArchitectures = Settings::User().Get<Settings::Setting::InstallArchitecturePreference>(); + + if (!requiredArchitectures.empty()) + { + // Required architecture list from settings if applicable + allowedArchitectures.insert(allowedArchitectures.end(), requiredArchitectures.begin(), requiredArchitectures.end()); + } + else { - if (architecture == Utility::Architecture::Unknown) + // Preferred architecture list from settings if applicable, add Unknown to indicate allowing remaining applicable + if (!optionalArchitectures.empty()) { - addRemainingApplicableArchitectures = true; - break; + allowedArchitectures.insert(allowedArchitectures.end(), optionalArchitectures.begin(), optionalArchitectures.end()); } - // If the architecture is applicable and not already in our result set... - if (Utility::IsApplicableArchitecture(architecture) != Utility::InapplicableArchitecture && - Utility::IsApplicableArchitecture(architecture, result) == Utility::InapplicableArchitecture) - { - result.push_back(architecture); - } + allowedArchitectures.emplace_back(Utility::Architecture::Unknown); + } + } + } + + if (!allowedArchitectures.empty()) + { + // If the incoming data contains elements, we will use them to construct a final allowed list. + // The algorithm is to take elements until we find Unknown, which indicates that any architecture is + // acceptable at this point. The system supported set of architectures will then be placed at the end. + std::vector<Utility::Architecture> result; + bool addRemainingApplicableArchitectures = false; + + for (Utility::Architecture architecture : allowedArchitectures) + { + if (architecture == Utility::Architecture::Unknown) + { + addRemainingApplicableArchitectures = true; + break; + } + + // If the architecture is applicable and not already in our result set... + if (Utility::IsApplicableArchitecture(architecture) != Utility::InapplicableArchitecture && + Utility::IsApplicableArchitecture(architecture, result) == Utility::InapplicableArchitecture) + { + result.push_back(architecture); } + } - if (addRemainingApplicableArchitectures) + if (addRemainingApplicableArchitectures) + { + for (Utility::Architecture architecture : Utility::GetApplicableArchitectures()) { - for (Utility::Architecture architecture : Utility::GetApplicableArchitectures()) + if (Utility::IsApplicableArchitecture(architecture, result) == Utility::InapplicableArchitecture) { - if (Utility::IsApplicableArchitecture(architecture, result) == Utility::InapplicableArchitecture) - { - result.push_back(architecture); - } + result.push_back(architecture); } } - - return std::make_unique<MachineArchitectureComparator>(std::move(result)); } - } - return std::make_unique<MachineArchitectureComparator>(); + return std::make_unique<MachineArchitectureComparator>(std::move(result)); + } + else + { + return std::make_unique<MachineArchitectureComparator>(); + } } InapplicabilityFlags IsApplicable(const Manifest::ManifestInstaller& installer) override @@ -403,91 +451,68 @@ namespace AppInstaller::CLI::Workflow bool m_allowUnknownInAdditionToRequired; }; - struct InstalledLocaleComparator : public details::ComparisonField - { - InstalledLocaleComparator(std::string installedLocale) : - details::ComparisonField("Installed Locale"), m_installedLocale(std::move(installedLocale)) {} - - static std::unique_ptr<InstalledLocaleComparator> Create(const Repository::IPackageVersion::Metadata& installationMetadata) - { - // Check for an existing install and require a compatible locale. - auto installerLocaleItr = installationMetadata.find(Repository::PackageVersionMetadata::InstalledLocale); - if (installerLocaleItr != installationMetadata.end()) - { - return std::make_unique<InstalledLocaleComparator>(installerLocaleItr->second); - } - - return {}; - } - - InapplicabilityFlags IsApplicable(const Manifest::ManifestInstaller& installer) override - { - // We have to assume an unknown installer locale will match our installed locale, or the entire catalog would stop working for upgrade. - if (installer.Locale.empty() || - Locale::GetDistanceOfLanguage(m_installedLocale, installer.Locale) >= Locale::MinimumDistanceScoreAsCompatibleMatch) - { - return InapplicabilityFlags::None; - } - - return InapplicabilityFlags::InstalledLocale; - } - - std::string ExplainInapplicable(const Manifest::ManifestInstaller& installer) override - { - std::string result = "Installer locale is not compatible with currently installed locale: "; - result += installer.Locale; - result += " not compatible with "; - result += m_installedLocale; - return result; - } - - bool IsFirstBetter(const Manifest::ManifestInstaller& first, const Manifest::ManifestInstaller& second) override - { - double firstScore = first.Locale.empty() ? Locale::UnknownLanguageDistanceScore : Locale::GetDistanceOfLanguage(m_installedLocale, first.Locale); - double secondScore = second.Locale.empty() ? Locale::UnknownLanguageDistanceScore : Locale::GetDistanceOfLanguage(m_installedLocale, second.Locale); - - return firstScore > secondScore; - } - - private: - std::string m_installedLocale; - }; - struct LocaleComparator : public details::ComparisonField { - LocaleComparator(std::vector<std::string> preference, std::vector<std::string> requirement) : - details::ComparisonField("Locale"), m_preference(std::move(preference)), m_requirement(std::move(requirement)) + LocaleComparator(std::vector<std::string> preference, std::vector<std::string> requirement, bool isInstalledLocale) : + details::ComparisonField("Locale"), m_preference(std::move(preference)), m_requirement(std::move(requirement)), m_isInstalledLocale(isInstalledLocale) { m_requirementAsString = Utility::ConvertContainerToString(m_requirement); m_preferenceAsString = Utility::ConvertContainerToString(m_preference); - AICLI_LOG(CLI, Verbose, << "Locale Comparator created with Required Locales: " << m_requirementAsString << " , Preferred Locales: " << m_preferenceAsString); + AICLI_LOG(CLI, Verbose, + << "Locale Comparator created with Required Locales: " << m_requirementAsString + << " , Preferred Locales: " << m_preferenceAsString + << " , IsInstalledLocale: " << m_isInstalledLocale); } - static std::unique_ptr<LocaleComparator> Create(const Execution::Args& args) + static std::unique_ptr<LocaleComparator> Create(const Execution::Args& args, const Repository::IPackageVersion::Metadata& metadata) { std::vector<std::string> preference; std::vector<std::string> requirement; + // This is for installed locale case, where the locale is a preference but requires at least compatible match. + bool isInstalledLocale = false; - // Preference will come from winget settings or Preferred Languages settings. winget settings takes precedence. - preference = Settings::User().Get<Settings::Setting::InstallLocalePreference>(); - if (preference.empty()) + auto userIntentItr = metadata.find(Repository::PackageVersionMetadata::UserIntentLocale); + auto installedItr = metadata.find(Repository::PackageVersionMetadata::InstalledLocale); + // Requirement may come from args, previous user intent or settings; args overrides previous user intent then settings. + if (args.Contains(Execution::Args::Type::Locale)) { - preference = Locale::GetUserPreferredLanguages(); + requirement.emplace_back(args.GetArg(Execution::Args::Type::Locale)); + } + else if (userIntentItr != metadata.end()) + { + requirement.emplace_back(userIntentItr->second); + isInstalledLocale = true; + } + else + { + if (installedItr == metadata.end()) + { + // If it's an upgrade of previous package, no need to set requirements from settings + // as previous installed locale will be used later. + requirement = Settings::User().Get<Settings::Setting::InstallLocaleRequirement>(); + } } - // Requirement may come from args or settings; args overrides settings. - if (args.Contains(Execution::Args::Type::Locale)) + // Preference will come from previous installed locale, winget settings or Preferred Languages settings. + // Previous installed locale goes first, then winget settings, then Preferred Languages settings. + // Previous installed locale also requires at least compatible locale match. + if (installedItr != metadata.end()) { - requirement.emplace_back(args.GetArg(Execution::Args::Type::Locale)); + preference.emplace_back(installedItr->second); + isInstalledLocale = true; } else { - requirement = Settings::User().Get<Settings::Setting::InstallLocaleRequirement>(); + preference = Settings::User().Get<Settings::Setting::InstallLocalePreference>(); + if (preference.empty()) + { + preference = Locale::GetUserPreferredLanguages(); + } } if (!preference.empty() || !requirement.empty()) { - return std::make_unique<LocaleComparator>(preference, requirement); + return std::make_unique<LocaleComparator>(preference, requirement, isInstalledLocale); } else { @@ -497,20 +522,40 @@ namespace AppInstaller::CLI::Workflow InapplicabilityFlags IsApplicable(const Manifest::ManifestInstaller& installer) override { - if (m_requirement.empty()) - { - return InapplicabilityFlags::None; - } + InapplicabilityFlags inapplicableFlag = m_isInstalledLocale ? InapplicabilityFlags::InstalledLocale : InapplicabilityFlags::Locale; - for (auto const& requiredLocale : m_requirement) + if (!m_requirement.empty()) { - if (Locale::GetDistanceOfLanguage(requiredLocale, installer.Locale) >= Locale::MinimumDistanceScoreAsPerfectMatch) + // Check if requirement is satisfied + for (auto const& requiredLocale : m_requirement) { - return InapplicabilityFlags::None; + if (Locale::GetDistanceOfLanguage(requiredLocale, installer.Locale) >= Locale::MinimumDistanceScoreAsPerfectMatch) + { + return InapplicabilityFlags::None; + } } + + return inapplicableFlag; } + else if (m_isInstalledLocale && !m_preference.empty()) + { + // For installed locale preference, check at least compatible match for preference + for (auto const& preferredLocale : m_preference) + { + // We have to assume an unknown installer locale will match our installed locale, or the entire catalog would stop working for upgrade. + if (installer.Locale.empty() || + Locale::GetDistanceOfLanguage(preferredLocale, installer.Locale) >= Locale::MinimumDistanceScoreAsCompatibleMatch) + { + return InapplicabilityFlags::None; + } + } - return InapplicabilityFlags::Locale; + return inapplicableFlag; + } + else + { + return InapplicabilityFlags::None; + } } std::string ExplainInapplicable(const Manifest::ManifestInstaller& installer) override @@ -519,6 +564,8 @@ namespace AppInstaller::CLI::Workflow result += installer.Locale; result += "Required locales: "; result += m_requirementAsString; + result += " Or does not satisfy compatible match for Preferred Locales: "; + result += m_preferenceAsString; return result; } @@ -550,6 +597,7 @@ namespace AppInstaller::CLI::Workflow std::vector<std::string> m_requirement; std::string m_requirementAsString; std::string m_preferenceAsString; + bool m_isInstalledLocale = false; }; struct MarketFilter : public details::FilterField @@ -618,17 +666,7 @@ namespace AppInstaller::CLI::Workflow // Filter order is not important, but comparison order determines priority. // TODO: There are improvements to be made here around ordering, especially in the context of implicit vs explicit vs command line preferences. AddComparator(InstalledTypeComparator::Create(installationMetadata)); - - auto installedLocaleComparator = InstalledLocaleComparator::Create(installationMetadata); - if (installedLocaleComparator) - { - AddComparator(std::move(installedLocaleComparator)); - } - else - { - AddComparator(LocaleComparator::Create(context.Args)); - } - + AddComparator(LocaleComparator::Create(context.Args, installationMetadata)); AddComparator(ScopeComparator::Create(context)); AddComparator(MachineArchitectureComparator::Create(context, installationMetadata)); } diff --git a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp @@ -990,26 +990,6 @@ namespace AppInstaller::CLI::Workflow { installationMetadata = context.Get<Execution::Data::InstalledPackageVersion>()->GetMetadata(); } - if (context.Args.Contains(Execution::Args::Type::InstallArchitecture)) - { - // arguments override settings. - context.Add<Execution::Data::AllowedArchitectures>({ Utility::ConvertToArchitectureEnum(std::string(context.Args.GetArg(Execution::Args::Type::InstallArchitecture))) }); - } - else - { - std::vector<Utility::Architecture> requiredArchitectures = Settings::User().Get<Settings::Setting::InstallArchitectureRequirement>(); - std::vector<Utility::Architecture> optionalArchitectures = Settings::User().Get<Settings::Setting::InstallArchitecturePreference>(); - - if (!requiredArchitectures.empty()) - { - context.Add<Execution::Data::AllowedArchitectures>({ requiredArchitectures.begin(), requiredArchitectures.end() }); - } - else if (!optionalArchitectures.empty()) - { - optionalArchitectures.emplace_back(Utility::Architecture::Unknown); - context.Add<Execution::Data::AllowedArchitectures>({ optionalArchitectures.begin(), optionalArchitectures.end() }); - } - } ManifestComparator manifestComparator(context, installationMetadata); auto [installer, inapplicabilities] = manifestComparator.GetPreferredInstaller(context.Get<Execution::Data::Manifest>()); diff --git a/src/AppInstallerCLITests/CompositeSource.cpp b/src/AppInstallerCLITests/CompositeSource.cpp @@ -855,6 +855,67 @@ TEST_CASE("CompositeSource_TrackingPackageFound", "[CompositeSource]") REQUIRE(result.Matches[0].Package->GetLatestAvailableVersion()); } +TEST_CASE("CompositeSource_TrackingPackageFound_MetadataPopulatedFromTracking", "[CompositeSource]") +{ + std::string availableID = "Available.ID"; + std::string pfn = "sortof_apfn"; + + auto installedPackage = MakeInstalled().WithPFN(pfn); + auto availablePackage = MakeAvailable().WithPFN(pfn).WithId(availableID).WithDefaultName(s_Everything_Query); + + CompositeWithTrackingTestSetup setup; + setup.Installed->Everything.Matches.emplace_back(installedPackage, Criteria()); + setup.Installed->SearchFunction = [&](const SearchRequest& request) + { + RequireIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); + + SearchResult result; + result.Matches.emplace_back(installedPackage, Criteria()); + return result; + }; + + setup.Available->Everything.Matches.emplace_back(availablePackage, Criteria()); + setup.Available->SearchFunction = [&](const SearchRequest& request) + { + if (request.Filters.empty()) + { + RequireIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); + } + else + { + REQUIRE(request.Filters.size() == 1); + RequireIncludes(request.Filters, PackageMatchField::Id, MatchType::CaseInsensitive, availableID); + } + + SearchResult result; + result.Matches.emplace_back(availablePackage, Criteria()); + return result; + }; + + auto manifestId = setup.Tracking->GetIndex().AddManifest(availablePackage); + + // Add test PackageVersionMetadata to be populated to InstalledVersion metadata + setup.Tracking->GetIndex().SetMetadataByManifestId(manifestId, Repository::PackageVersionMetadata::TrackingWriteTime, "100"); + setup.Tracking->GetIndex().SetMetadataByManifestId(manifestId, Repository::PackageVersionMetadata::UserIntentArchitecture, "X86"); + setup.Tracking->GetIndex().SetMetadataByManifestId(manifestId, Repository::PackageVersionMetadata::UserIntentLocale, "en-US"); + setup.Tracking->GetIndex().SetMetadataByManifestId(manifestId, Repository::PackageVersionMetadata::InstalledArchitecture, "X86"); + setup.Tracking->GetIndex().SetMetadataByManifestId(manifestId, Repository::PackageVersionMetadata::InstalledLocale, "en-US"); + setup.Tracking->GetIndex().SetMetadataByManifestId(manifestId, Repository::PackageVersionMetadata::PinnedState, "PinnedByManifest"); + + SearchResult result = setup.Search(); + + REQUIRE(result.Matches.size() == 1); + REQUIRE(result.Matches[0].Package); + REQUIRE(result.Matches[0].Package->GetInstalledVersion()); + + auto metadata = result.Matches[0].Package->GetInstalledVersion()->GetMetadata(); + REQUIRE(metadata[Repository::PackageVersionMetadata::UserIntentArchitecture] == "X86"); + REQUIRE(metadata[Repository::PackageVersionMetadata::UserIntentLocale] == "en-US"); + REQUIRE(metadata[Repository::PackageVersionMetadata::InstalledArchitecture] == "X86"); + REQUIRE(metadata[Repository::PackageVersionMetadata::InstalledLocale] == "en-US"); + REQUIRE(metadata[Repository::PackageVersionMetadata::PinnedState] == "PinnedByManifest"); +} + TEST_CASE("CompositeSource_TrackingFound_AvailableNot", "[CompositeSource]") { std::string availableID = "Available.ID"; diff --git a/src/AppInstallerCLITests/ManifestComparator.cpp b/src/AppInstallerCLITests/ManifestComparator.cpp @@ -334,7 +334,7 @@ TEST_CASE("ManifestComparator_ScopeCompare", "[manifest_comparator]") } } -TEST_CASE("ManifestComparator_InstalledLocaleComparator_Uknown", "[manifest_comparator]") +TEST_CASE("ManifestComparator_LocaleComparator_Installed_WithUknown", "[manifest_comparator]") { Manifest manifest; ManifestInstaller unknown = AddInstaller(manifest, Architecture::Neutral, InstallerTypeEnum::Msi, ScopeEnum::User, "", ""); @@ -376,7 +376,7 @@ TEST_CASE("ManifestComparator_InstalledLocaleComparator_Uknown", "[manifest_comp } } -TEST_CASE("ManifestComparator_InstalledLocaleComparator", "[manifest_comparator]") +TEST_CASE("ManifestComparator_LocaleComparator_Installed", "[manifest_comparator]") { Manifest manifest; ManifestInstaller frFR = AddInstaller(manifest, Architecture::Neutral, InstallerTypeEnum::Msi, ScopeEnum::User, "", "fr-FR"); @@ -416,6 +416,30 @@ TEST_CASE("ManifestComparator_InstalledLocaleComparator", "[manifest_comparator] REQUIRE(!result); RequireInapplicabilities(inapplicabilities, { InapplicabilityFlags::InstalledLocale, InapplicabilityFlags::InstalledLocale }); } + SECTION("en-US installed but fr-fr as user intent") + { + IPackageVersion::Metadata metadata; + metadata[PackageVersionMetadata::InstalledLocale] = "en-US"; + metadata[PackageVersionMetadata::UserIntentLocale] = "fr-FR"; + + ManifestComparator mc(ManifestComparatorTestContext{}, metadata); + auto [result, inapplicabilities] = mc.GetPreferredInstaller(manifest); + + RequireInstaller(result, frFR); + RequireInapplicabilities(inapplicabilities, { InapplicabilityFlags::InstalledLocale }); // en-US inapplicable + } + SECTION("en-US installed but zh-CN as user intent") + { + IPackageVersion::Metadata metadata; + metadata[PackageVersionMetadata::InstalledLocale] = "en-US"; + metadata[PackageVersionMetadata::UserIntentLocale] = "zh-CN"; + + ManifestComparator mc(ManifestComparatorTestContext{}, metadata); + auto [result, inapplicabilities] = mc.GetPreferredInstaller(manifest); + + REQUIRE(!result); + RequireInapplicabilities(inapplicabilities, { InapplicabilityFlags::InstalledLocale, InapplicabilityFlags::InstalledLocale }); + } } TEST_CASE("ManifestComparator_LocaleComparator", "[manifest_comparator]") @@ -530,6 +554,52 @@ TEST_CASE("ManifestComparator_AllowedArchitecture", "[manifest_comparator]") } } +TEST_CASE("ManifestComparator_Architectures_WithUserIntent", "[manifest_comparator]") +{ + Manifest manifest; + ManifestInstaller x86 = AddInstaller(manifest, Architecture::X86, InstallerTypeEnum::Msi, ScopeEnum::User, "", ""); + ManifestInstaller x64 = AddInstaller(manifest, Architecture::X64, InstallerTypeEnum::Msi, ScopeEnum::User, "", ""); + + SECTION("x86 installed") + { + IPackageVersion::Metadata metadata; + metadata[PackageVersionMetadata::InstalledArchitecture] = "x86"; + + ManifestComparator mc(ManifestComparatorTestContext{}, metadata); + auto [result, inapplicabilities] = mc.GetPreferredInstaller(manifest); + + RequireInstaller(result, x86); + REQUIRE(inapplicabilities.size() == 0); + } + SECTION("x86 installed but x64 as user intent") + { + IPackageVersion::Metadata metadata; + metadata[PackageVersionMetadata::InstalledArchitecture] = "x86"; + metadata[PackageVersionMetadata::UserIntentArchitecture] = "x64"; + + ManifestComparator mc(ManifestComparatorTestContext{}, metadata); + auto [result, inapplicabilities] = mc.GetPreferredInstaller(manifest); + + RequireInstaller(result, x64); + RequireInapplicabilities(inapplicabilities, { InapplicabilityFlags::MachineArchitecture }); + } + SECTION("x86 installed but x64 as user intent") + { + Manifest x86OnlyManifest; + AddInstaller(x86OnlyManifest, Architecture::X86, InstallerTypeEnum::Msi, ScopeEnum::User, "", ""); + + IPackageVersion::Metadata metadata; + metadata[PackageVersionMetadata::InstalledArchitecture] = "x86"; + metadata[PackageVersionMetadata::UserIntentArchitecture] = "x64"; + + ManifestComparator mc(ManifestComparatorTestContext{}, metadata); + auto [result, inapplicabilities] = mc.GetPreferredInstaller(x86OnlyManifest); + + REQUIRE(!result); + RequireInapplicabilities(inapplicabilities, { InapplicabilityFlags::MachineArchitecture }); + } +} + TEST_CASE("ManifestComparator_UnsupportedOSArchitecture", "[manifest_comparator]") { auto systemArchitecture = GetSystemArchitecture(); diff --git a/src/AppInstallerCommonCore/Architecture.cpp b/src/AppInstallerCommonCore/Architecture.cpp @@ -159,7 +159,7 @@ namespace AppInstaller::Utility } } - Architecture ConvertToArchitectureEnum(const std::string& archStr) + Architecture ConvertToArchitectureEnum(std::string_view archStr) { std::string arch = ToLower(archStr); if (arch == "x86") @@ -183,7 +183,7 @@ namespace AppInstaller::Utility return Architecture::Neutral; } - AICLI_LOG(YAML, Info, << "ConvertToArchitectureEnum: Unknown architecture: " << archStr); + AICLI_LOG(Core, Info, << "ConvertToArchitectureEnum: Unknown architecture: " << archStr); return Architecture::Unknown; } diff --git a/src/AppInstallerCommonCore/Public/AppInstallerArchitecture.h b/src/AppInstallerCommonCore/Public/AppInstallerArchitecture.h @@ -20,7 +20,7 @@ namespace AppInstaller::Utility }; // Converts a string to corresponding enum - Architecture ConvertToArchitectureEnum(const std::string& archStr); + Architecture ConvertToArchitectureEnum(std::string_view archStr); // Converts an Architecture to a string_view LocIndView ToString(Architecture architecture); diff --git a/src/AppInstallerRepositoryCore/CompositeSource.cpp b/src/AppInstallerRepositoryCore/CompositeSource.cpp @@ -85,11 +85,13 @@ namespace AppInstaller::Repository } } - // For a given package from a tracking catalog, get the latest write time. + // For a given package from a tracking catalog, get the latest write time and its corresponding package version. // Look at all versions rather than just the latest to account for the potential of downgrading. - std::chrono::system_clock::time_point GetLatestTrackingPackageWriteTime(const std::shared_ptr<IPackage>& trackingPackage) + std::pair<std::chrono::system_clock::time_point, std::shared_ptr<IPackageVersion>> GetLatestTrackingWriteTimeAndPackageVersion( + const std::shared_ptr<IPackage>& trackingPackage) { - std::chrono::system_clock::time_point result{}; + std::chrono::system_clock::time_point resultTime{}; + std::shared_ptr<IPackageVersion> resultVersion; for (const auto& key : trackingPackage->GetAvailableVersionKeys()) { @@ -109,15 +111,16 @@ namespace AppInstaller::Repository std::chrono::system_clock::time_point versionTime = Utility::ConvertUnixEpochToSystemClock(unixEpoch); - if (versionTime > result) + if (versionTime > resultTime) { - result = versionTime; + resultTime = versionTime; + resultVersion = version; } } } } - return result; + return { resultTime, std::move(resultVersion) }; } // TODO: Note: Currently this function assumes the all versions in the available package is from one source. @@ -251,8 +254,8 @@ namespace AppInstaller::Repository // A composite package installed version that allows us to override the source or the version. struct CompositeInstalledVersion : public IPackageVersion { - CompositeInstalledVersion(std::shared_ptr<IPackageVersion> baseInstalledVersion, Source trackingSource, std::string overrideVersion = {}) : - m_baseInstalledVersion(std::move(baseInstalledVersion)), m_trackingSource(std::move(trackingSource)), m_overrideVersion(std::move(overrideVersion)) + CompositeInstalledVersion(std::shared_ptr<IPackageVersion> baseInstalledVersion, Source trackingSource, std::shared_ptr<IPackageVersion> trackingPackageVersion, std::string overrideVersion = {}) : + m_baseInstalledVersion(std::move(baseInstalledVersion)), m_trackingSource(std::move(trackingSource)), m_trackingPackageVersion(std::move(trackingPackageVersion)), m_overrideVersion(std::move(overrideVersion)) {} Utility::LocIndString GetProperty(PackageVersionProperty property) const override @@ -289,13 +292,32 @@ namespace AppInstaller::Repository Metadata GetMetadata() const override { - return m_baseInstalledVersion->GetMetadata(); + auto result = m_baseInstalledVersion->GetMetadata(); + + // Populate metadata from tracking package version if not present in base installed version. + if (m_trackingPackageVersion) + { + auto trackingMetadata = m_trackingPackageVersion->GetMetadata(); + for (auto metadataItem : { PackageVersionMetadata::InstalledArchitecture, PackageVersionMetadata::InstalledLocale, + PackageVersionMetadata::UserIntentArchitecture, PackageVersionMetadata::UserIntentLocale, PackageVersionMetadata::PinnedState }) + { + auto itr = trackingMetadata.find(metadataItem); + auto existingItr = result.find(metadataItem); + if (itr != trackingMetadata.end() && existingItr == result.end()) + { + result[metadataItem] = itr->second; + } + } + } + + return result; } private: std::shared_ptr<IPackageVersion> m_baseInstalledVersion; Source m_trackingSource; std::string m_overrideVersion; + std::shared_ptr<IPackageVersion> m_trackingPackageVersion; }; // A composite package for the CompositeSource. @@ -320,9 +342,9 @@ namespace AppInstaller::Repository Utility::LocIndString GetProperty(PackageProperty property) const override { std::shared_ptr<IPackageVersion> truth = GetLatestAvailableVersion(); - if (!truth && m_trackingPackage) + if (!truth) { - truth = m_trackingPackage->GetLatestAvailableVersion(); + truth = m_trackingPackageVersion; } if (!truth) { @@ -347,7 +369,7 @@ namespace AppInstaller::Repository auto installedVersion = m_installedPackage->GetInstalledVersion(); if (installedVersion) { - return std::make_shared<CompositeInstalledVersion>(std::move(installedVersion), m_trackingSource, m_overrideInstalledVersion); + return std::make_shared<CompositeInstalledVersion>(std::move(installedVersion), m_trackingSource, m_trackingPackageVersion, m_overrideInstalledVersion); } } @@ -438,10 +460,11 @@ namespace AppInstaller::Repository TrySetOverrideInstalledVersion(); } - void SetTracking(Source trackingSource, std::shared_ptr<IPackage> trackingPackage) + void SetTracking(Source trackingSource, std::shared_ptr<IPackage> trackingPackage, std::shared_ptr<IPackageVersion> trackingPackageVersion) { m_trackingSource = std::move(trackingSource); m_trackingPackage = std::move(trackingPackage); + m_trackingPackageVersion = std::move(trackingPackageVersion); } private: @@ -466,6 +489,7 @@ namespace AppInstaller::Repository std::shared_ptr<IPackage> m_availablePackage; Source m_trackingSource; std::shared_ptr<IPackage> m_trackingPackage; + std::shared_ptr<IPackageVersion> m_trackingPackageVersion; std::string m_overrideInstalledVersion; }; @@ -924,6 +948,7 @@ namespace AppInstaller::Repository Source trackedSource; std::shared_ptr<IPackage> trackingPackage; + std::shared_ptr<IPackageVersion> trackingPackageVersion; std::chrono::system_clock::time_point trackingPackageTime; std::shared_ptr<IPackage> availablePackage; @@ -947,12 +972,13 @@ namespace AppInstaller::Repository // Determine the candidate package with the latest install time if (candidatePackage) { - std::chrono::system_clock::time_point candidateTime = GetLatestTrackingPackageWriteTime(candidatePackage); + auto [candidateTime, candidateVersion] = GetLatestTrackingWriteTimeAndPackageVersion(candidatePackage); if (!trackingPackage || candidateTime > trackingPackageTime) { trackedSource = source; trackingPackage = std::move(candidatePackage); + trackingPackageVersion = std::move(candidateVersion); trackingPackageTime = candidateTime; } } @@ -997,7 +1023,7 @@ namespace AppInstaller::Repository } compositePackage->SetAvailablePackage(std::move(availablePackage)); - compositePackage->SetTracking(std::move(trackedSource), std::move(trackingPackage)); + compositePackage->SetTracking(std::move(trackedSource), std::move(trackingPackage), std::move(trackingPackageVersion)); } // Move the installed result into the composite result @@ -1054,7 +1080,9 @@ namespace AppInstaller::Repository std::move(installedPackage), GetTrackedPackageFromAvailableSource(result, source, match.Package->GetProperty(PackageProperty::Id))); - compositePackage->SetTracking(source, std::move(match.Package)); + auto [writeTime, trackingPackageVersion] = GetLatestTrackingWriteTimeAndPackageVersion(match.Package); + + compositePackage->SetTracking(source, std::move(match.Package), std::move(trackingPackageVersion)); result.Matches.emplace_back(std::move(compositePackage), match.MatchCriteria); } diff --git a/src/AppInstallerRepositoryCore/PackageTrackingCatalog.cpp b/src/AppInstallerRepositoryCore/PackageTrackingCatalog.cpp @@ -192,21 +192,19 @@ namespace AppInstaller::Repository SQLiteIndex::IdType Id; }; - PackageTrackingCatalog::Version::Version() = default; PackageTrackingCatalog::Version::Version(const Version&) = default; PackageTrackingCatalog::Version& PackageTrackingCatalog::Version::operator=(const Version&) = default; PackageTrackingCatalog::Version::Version(Version&&) noexcept = default; PackageTrackingCatalog::Version& PackageTrackingCatalog::Version::operator=(Version&&) noexcept = default; PackageTrackingCatalog::Version::~Version() = default; - PackageTrackingCatalog::Version::Version(std::shared_ptr<implementation>&& value) : - m_implementation(std::move(value)) {} + PackageTrackingCatalog::Version::Version(PackageTrackingCatalog& catalog, std::shared_ptr<implementation>&& value) : + m_catalog(catalog), m_implementation(std::move(value)) {} void PackageTrackingCatalog::Version::SetMetadata(PackageVersionMetadata metadata, const Utility::NormalizedString& value) { - UNREFERENCED_PARAMETER(metadata); - UNREFERENCED_PARAMETER(value); - THROW_HR(E_NOTIMPL); + auto& index = m_catalog.m_implementation->Source->GetIndex(); + index.SetMetadataByManifestId(m_implementation->Id, metadata, value); } PackageTrackingCatalog::Version PackageTrackingCatalog::RecordInstall( @@ -243,9 +241,16 @@ namespace AppInstaller::Repository index.SetMetadataByManifestId(manifestId, PackageVersionMetadata::PinnedState, ToString(PackagePinnedState::PinnedByManifest)); } + // Record installed architecture and locale if applicable + index.SetMetadataByManifestId(manifestId, PackageVersionMetadata::InstalledArchitecture, ToString(installer.Arch)); + if (!installer.Locale.empty()) + { + index.SetMetadataByManifestId(manifestId, PackageVersionMetadata::InstalledLocale, installer.Locale); + } + std::shared_ptr<Version::implementation> result = std::make_shared<Version::implementation>(); result->Id = manifestId; - return { std::move(result) }; + return { *this, std::move(result) }; } void PackageTrackingCatalog::RecordUninstall(const Utility::LocIndString& packageIdentifier) diff --git a/src/AppInstallerRepositoryCore/Public/winget/PackageTrackingCatalog.h b/src/AppInstallerRepositoryCore/Public/winget/PackageTrackingCatalog.h @@ -42,7 +42,6 @@ namespace AppInstaller::Repository { friend PackageTrackingCatalog; - Version(); Version(const Version&); Version& operator=(const Version&); Version(Version&&) noexcept; @@ -54,8 +53,9 @@ namespace AppInstaller::Repository private: struct implementation; - Version(std::shared_ptr<implementation>&& value); + Version(PackageTrackingCatalog& catalog, std::shared_ptr<implementation>&& value); std::shared_ptr<implementation> m_implementation; + PackageTrackingCatalog& m_catalog; }; // Records an installation of the given package. diff --git a/src/AppInstallerRepositoryCore/Public/winget/RepositorySearch.h b/src/AppInstallerRepositoryCore/Public/winget/RepositorySearch.h @@ -183,6 +183,10 @@ namespace AppInstaller::Repository InstalledArchitecture, // The PackagePinnedState of the installed package PinnedState, + // The Architecture of user intent + UserIntentArchitecture, + // The locale of user intent + UserIntentLocale, }; // Convert a PackageVersionMetadata to a string. diff --git a/src/AppInstallerRepositoryCore/RepositorySearch.cpp b/src/AppInstallerRepositoryCore/RepositorySearch.cpp @@ -84,6 +84,10 @@ namespace AppInstaller::Repository case PackageVersionMetadata::Publisher: return "Publisher"sv; case PackageVersionMetadata::InstalledLocale: return "InstalledLocale"sv; case PackageVersionMetadata::TrackingWriteTime: return "TrackingWriteTime"sv; + case PackageVersionMetadata::InstalledArchitecture: return "InstalledArchitecture"sv; + case PackageVersionMetadata::PinnedState: return "PinnedState"sv; + case PackageVersionMetadata::UserIntentArchitecture: return "UserIntentArchitecture"sv; + case PackageVersionMetadata::UserIntentLocale: return "UserIntentLocale"sv; default: return "Unknown"sv; } }