From d2805fa890665265728dcd6e157e298f28c2bb85 Mon Sep 17 00:00:00 2001 From: Easton Pillay Date: Thu, 18 Nov 2021 14:54:08 -0500 Subject: [PATCH 1/5] Added settings to select preferred/required architecture. --- doc/Settings.md | 11 + .../JSON/settings/settings.schema.0.2.json | 12 + .../Workflows/ManifestComparator.cpp | 17 +- .../Workflows/WorkflowBase.cpp | 6 + .../Public/winget/UserSettings.h | 7 + src/AppInstallerCommonCore/UserSettings.cpp | 834 +++++++++--------- 6 files changed, 472 insertions(+), 415 deletions(-) diff --git a/doc/Settings.md b/doc/Settings.md index e17085bd0e..60c9b32562 100644 --- a/doc/Settings.md +++ b/doc/Settings.md @@ -78,6 +78,17 @@ The `locale` behavior affects the choice of installer based on installer locale. } }, ``` +### Architecture + +The `architecture` behavior affects what architecture will be selected when installing a package. The matching parameter is `--architecture`. Note that only architectures compatible with your system can be selected. + +```json + "installBehavior": { + "preferences": { + "architecture": "x64" + } + }, +``` ## Telemetry diff --git a/schemas/JSON/settings/settings.schema.0.2.json b/schemas/JSON/settings/settings.schema.0.2.json index 3b00fa66f4..cf12f10aff 100644 --- a/schemas/JSON/settings/settings.schema.0.2.json +++ b/schemas/JSON/settings/settings.schema.0.2.json @@ -54,6 +54,18 @@ }, "minItems": 1, "maxItems": 10 + }, + "architecture": { + "description": "The architecture for a package install", + "type": "string", + "enum": [ + "neutral", + "x64", + "x86", + "arm64", + "arm" + ], + "default": "neutral" } } }, diff --git a/src/AppInstallerCLICore/Workflows/ManifestComparator.cpp b/src/AppInstallerCLICore/Workflows/ManifestComparator.cpp index 213599b9f8..81396d0a08 100644 --- a/src/AppInstallerCLICore/Workflows/ManifestComparator.cpp +++ b/src/AppInstallerCLICore/Workflows/ManifestComparator.cpp @@ -46,10 +46,11 @@ namespace AppInstaller::CLI::Workflow struct MachineArchitectureComparator : public details::ComparisonField { - MachineArchitectureComparator() : details::ComparisonField("Machine Architecture") {} + MachineArchitectureComparator(Utility::Architecture preference) : + details::ComparisonField("Machine Architecture"), m_preference(preference) {} - MachineArchitectureComparator(std::vector allowedArchitectures) : - details::ComparisonField("Machine Architecture"), m_allowedArchitectures(std::move(allowedArchitectures)) + MachineArchitectureComparator(std::vector allowedArchitectures, Utility::Architecture preference) : + details::ComparisonField("Machine Architecture"), m_allowedArchitectures(std::move(allowedArchitectures)), m_preference(preference) { AICLI_LOG(CLI, Verbose, << "Architecture Comparator created with allowed architectures: " << GetAllowedArchitecturesString()); } @@ -57,6 +58,7 @@ namespace AppInstaller::CLI::Workflow // TODO: At some point we can do better about matching the currently installed architecture static std::unique_ptr Create(const Execution::Context& context, const Repository::IPackageVersion::Metadata&) { + Utility::Architecture preference = Settings::User().Get(); if (context.Contains(Execution::Data::AllowedArchitectures)) { const std::vector& allowedArchitectures = context.Get(); @@ -95,11 +97,11 @@ namespace AppInstaller::CLI::Workflow } } - return std::make_unique(std::move(result)); + return std::make_unique(std::move(result), preference); } } - return std::make_unique(); + return std::make_unique(preference); } InapplicabilityFlags IsApplicable(const Manifest::ManifestInstaller& installer) override @@ -129,6 +131,10 @@ namespace AppInstaller::CLI::Workflow bool IsFirstBetter(const Manifest::ManifestInstaller& first, const Manifest::ManifestInstaller& second) override { + if (m_preference != Utility::Architecture::Neutral) + { + return (first.Arch == m_preference && second.Arch != m_preference); + } auto arch1 = CheckAllowedArchitecture(first.Arch); auto arch2 = CheckAllowedArchitecture(second.Arch); @@ -170,6 +176,7 @@ namespace AppInstaller::CLI::Workflow } std::vector m_allowedArchitectures; + Utility::Architecture m_preference; }; struct InstalledTypeComparator : public details::ComparisonField diff --git a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp index fe6d602d4e..2b62a79026 100644 --- a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp +++ b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp @@ -962,14 +962,20 @@ namespace AppInstaller::CLI::Workflow bool isUpdate = WI_IsFlagSet(context.GetFlags(), Execution::ContextFlag::InstallerExecutionUseUpdate); IPackageVersion::Metadata installationMetadata; + Utility::Architecture requiredArchitecture = Settings::User().Get(); if (isUpdate) { installationMetadata = context.Get()->GetMetadata(); } if (context.Args.Contains(Execution::Args::Type::InstallArchitecture)) { + // arguments override settings. context.Add({ Utility::ConvertToArchitectureEnum(std::string(context.Args.GetArg(Execution::Args::Type::InstallArchitecture))) }); } + else if (requiredArchitecture != Utility::Architecture::Neutral) + { + context.Add({ requiredArchitecture }); + } ManifestComparator manifestComparator(context, installationMetadata); auto [installer, inapplicabilities] = manifestComparator.GetPreferredInstaller(context.Get()); diff --git a/src/AppInstallerCommonCore/Public/winget/UserSettings.h b/src/AppInstallerCommonCore/Public/winget/UserSettings.h index fc5147c39a..85050fef45 100644 --- a/src/AppInstallerCommonCore/Public/winget/UserSettings.h +++ b/src/AppInstallerCommonCore/Public/winget/UserSettings.h @@ -13,6 +13,8 @@ #include #include +#include "AppInstallerArchitecture.h" + using namespace std::chrono_literals; using namespace std::string_view_literals; @@ -54,6 +56,7 @@ namespace AppInstaller::Settings DeliveryOptimization, }; + // Enum of settings. // Must start at 0 to enable direct access to variant in UserSettings. // Max must be last and unused. @@ -73,6 +76,8 @@ namespace AppInstaller::Settings InstallScopeRequirement, NetworkDownloader, NetworkDOProgressTimeoutInSeconds, + InstallArchitecturePreference, + InstallArchitectureRequirement, InstallLocalePreference, InstallLocaleRequirement, EFDirectMSI, @@ -116,6 +121,8 @@ namespace AppInstaller::Settings SETTINGMAPPING_SPECIALIZATION(Setting::EFExperimentalArg, bool, bool, false, ".experimentalFeatures.experimentalArg"sv); SETTINGMAPPING_SPECIALIZATION(Setting::EFDependencies, bool, bool, false, ".experimentalFeatures.dependencies"sv); SETTINGMAPPING_SPECIALIZATION(Setting::TelemetryDisable, bool, bool, false, ".telemetry.disable"sv); + SETTINGMAPPING_SPECIALIZATION(Setting::InstallArchitecturePreference, std::string, Utility::Architecture, Utility::Architecture::Neutral, ".installBehavior.preferences.architecture"sv); + SETTINGMAPPING_SPECIALIZATION(Setting::InstallArchitectureRequirement, std::string, Utility::Architecture, Utility::Architecture::Neutral, ".installBehavior.requirements.architecture"sv); SETTINGMAPPING_SPECIALIZATION(Setting::InstallScopePreference, std::string, ScopePreference, ScopePreference::User, ".installBehavior.preferences.scope"sv); SETTINGMAPPING_SPECIALIZATION(Setting::InstallScopeRequirement, std::string, ScopePreference, ScopePreference::None, ".installBehavior.requirements.scope"sv); SETTINGMAPPING_SPECIALIZATION(Setting::NetworkDownloader, std::string, InstallerDownloader, InstallerDownloader::Default, ".network.downloader"sv); diff --git a/src/AppInstallerCommonCore/UserSettings.cpp b/src/AppInstallerCommonCore/UserSettings.cpp index 62dd69ea6a..514ec33a67 100644 --- a/src/AppInstallerCommonCore/UserSettings.cpp +++ b/src/AppInstallerCommonCore/UserSettings.cpp @@ -1,334 +1,348 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. -#include "pch.h" -#include "AppInstallerRuntime.h" -#include "AppInstallerLanguageUtilities.h" -#include "AppInstallerLogging.h" -#include "JsonUtil.h" -#include "winget/Settings.h" -#include "winget/UserSettings.h" -#include "winget/Locale.h" - -namespace AppInstaller::Settings -{ - using namespace std::string_view_literals; - using namespace Runtime; - using namespace Utility; - - static constexpr std::string_view s_SettingEmpty = - R"({ - "$schema": "https://aka.ms/winget-settings.schema.json", - - // For documentation on these settings, see: https://aka.ms/winget-settings - // "source": { - // "autoUpdateIntervalInMinutes": 5 - // }, -})"sv; - - namespace - { - template - inline std::string GetValueString(T value) - { - std::string convertedValue; - - if constexpr (std::is_arithmetic_v) - { - convertedValue = std::to_string(value); - } - else - { - convertedValue = value; - } - - return convertedValue; - } - - template<> - inline std::string GetValueString(std::vector value) - { - std::string convertedValue = "["; - - bool first = true; - for (auto const& entry : value) - { - if (first) - { - first = false; - } - else - { - convertedValue += ", "; - } - - convertedValue += entry; - } - - convertedValue += ']'; - - return convertedValue; - } - - std::optional ParseFile(const StreamDefinition& setting, std::vector& warnings) - { - auto stream = Stream{ setting }.Get(); - if (stream) - { - Json::Value root; - Json::CharReaderBuilder builder; - const std::unique_ptr reader(builder.newCharReader()); - - std::string settingsContentStr = Utility::ReadEntireStream(*stream); - std::string error; - - if (reader->parse(settingsContentStr.c_str(), settingsContentStr.c_str() + settingsContentStr.size(), &root, &error)) - { - return root; - } - - AICLI_LOG(Core, Error, << "Error parsing " << setting.Name << ": " << error); - warnings.emplace_back(StringResource::String::SettingsWarningParseError, setting.Name, error, false); - } - - return {}; - } - - template - std::optional::json_t> GetValueFromPolicy() - { - return GroupPolicies().GetValue::Policy>(); - } - - template - void Validate( - Json::Value& root, - std::map& settings, - std::vector& warnings) - { - // jsoncpp doesn't support std::string_view yet. - auto path = std::string(details::SettingMapping::Path); - - // Settings set by Group Policy override anything else. See if there is one. - auto policyValue = GetValueFromPolicy(); - if (policyValue.has_value()) - { - // If the value is valid, use it. - // Otherwise, fall back to default. - // In any case, we do not need to read the setting from the JSON. - auto validatedValue = details::SettingMapping::Validate(policyValue.value()); - if (validatedValue.has_value()) - { - // Add it to the map - settings[S].emplace( - std::forward::value_t>(validatedValue.value())); - AICLI_LOG(Core, Verbose, << "Valid setting from Group Policy. Field: " << path << " Value: " << GetValueString(policyValue.value())); - } - else - { - auto valueAsString = GetValueString(policyValue.value()); - AICLI_LOG(Core, Error, << "Invalid setting from Group Policy. Field: " << path << " Value: " << valueAsString); - warnings.emplace_back(StringResource::String::SettingsWarningInvalidValueFromPolicy, path, valueAsString); - } - - return; - } - - const Json::Path jsonPath(path); - Json::Value result = jsonPath.resolve(root); - if (!result.isNull()) - { - auto jsonValue = GetValue::json_t>(result); - - if (jsonValue.has_value()) - { - auto validatedValue = details::SettingMapping::Validate(jsonValue.value()); - - if (validatedValue.has_value()) - { - // Finally add it to the map - settings[S].emplace( - std::forward::value_t>(validatedValue.value())); - AICLI_LOG(Core, Verbose, << "Valid setting. Field: " << path << " Value: " << GetValueString(jsonValue.value())); - } - else - { - auto valueAsString = GetValueString(jsonValue.value()); - AICLI_LOG(Core, Error, << "Invalid field value. Field: " << path << " Value: " << valueAsString); - warnings.emplace_back(StringResource::String::SettingsWarningInvalidFieldValue, path, valueAsString); - } - } - else - { - AICLI_LOG(Core, Error, << "Invalid field format. Field: " << path << " Using default"); - warnings.emplace_back(StringResource::String::SettingsWarningInvalidFieldFormat, path); - } - } - else - { - AICLI_LOG(Core, Verbose, << "Setting " << path << " not found. Using default"); - } - } - - template - void ValidateAll( - Json::Value& root, - std::map& settings, - std::vector& warnings, - std::index_sequence) - { - // Use folding to call each setting validate function. - (FoldHelper{}, ..., Validate(S)>(root, settings, warnings)); - } - } - - namespace details - { -#define WINGET_VALIDATE_SIGNATURE(_setting_) \ - std::optional::value_t> \ - SettingMapping::Validate(const SettingMapping::json_t& value) - - // Stamps out a validate function that simply returns the input value. -#define WINGET_VALIDATE_PASS_THROUGH(_setting_) \ - WINGET_VALIDATE_SIGNATURE(_setting_) \ - { \ - return value; \ - } - - WINGET_VALIDATE_SIGNATURE(AutoUpdateTimeInMinutes) - { - return std::chrono::minutes(value); - } - - WINGET_VALIDATE_SIGNATURE(ProgressBarVisualStyle) - { - // progressBar property possible values - static constexpr std::string_view s_progressBar_Accent = "accent"; - static constexpr std::string_view s_progressBar_Rainbow = "rainbow"; - static constexpr std::string_view s_progressBar_Retro = "retro"; - - if (Utility::CaseInsensitiveEquals(value, s_progressBar_Accent)) - { - return VisualStyle::Accent; - } - else if (Utility::CaseInsensitiveEquals(value, s_progressBar_Rainbow)) - { - return VisualStyle::Rainbow; - } - else if (Utility::CaseInsensitiveEquals(value, s_progressBar_Retro)) - { - return VisualStyle::Retro; - } - - return {}; - } - - WINGET_VALIDATE_PASS_THROUGH(EFExperimentalCmd) - WINGET_VALIDATE_PASS_THROUGH(EFExperimentalArg) - WINGET_VALIDATE_PASS_THROUGH(EFDependencies) - WINGET_VALIDATE_PASS_THROUGH(TelemetryDisable) - WINGET_VALIDATE_PASS_THROUGH(EFDirectMSI) - WINGET_VALIDATE_PASS_THROUGH(EnableSelfInitiatedMinidump) - - WINGET_VALIDATE_SIGNATURE(InstallScopePreference) - { - static constexpr std::string_view s_scope_user = "user"; - static constexpr std::string_view s_scope_machine = "machine"; - - if (Utility::CaseInsensitiveEquals(value, s_scope_user)) - { - return ScopePreference::User; - } - else if (Utility::CaseInsensitiveEquals(value, s_scope_machine)) - { - return ScopePreference::Machine; - } - - return {}; - } - - WINGET_VALIDATE_SIGNATURE(InstallScopeRequirement) - { - return SettingMapping::Validate(value); - } - - WINGET_VALIDATE_SIGNATURE(InstallLocalePreference) - { - for (auto const& entry : value) - { - if (!Locale::IsWellFormedBcp47Tag(entry)) - { - return {}; - } - } - - return value; - } - - WINGET_VALIDATE_SIGNATURE(InstallLocaleRequirement) - { - return SettingMapping::Validate(value); - } - - WINGET_VALIDATE_SIGNATURE(NetworkDownloader) - { - static constexpr std::string_view s_downloader_default = "default"; - static constexpr std::string_view s_downloader_wininet = "wininet"; - static constexpr std::string_view s_downloader_do = "do"; - - if (Utility::CaseInsensitiveEquals(value, s_downloader_default)) - { - return InstallerDownloader::Default; - } - else if (Utility::CaseInsensitiveEquals(value, s_downloader_wininet)) - { - return InstallerDownloader::WinInet; - } - else if (Utility::CaseInsensitiveEquals(value, s_downloader_do)) - { - return InstallerDownloader::DeliveryOptimization; - } - - return {}; - } - - WINGET_VALIDATE_SIGNATURE(NetworkDOProgressTimeoutInSeconds) - { - return std::chrono::seconds(value); - } - } - -#ifndef AICLI_DISABLE_TEST_HOOKS - static UserSettings* s_UserSettings_Override = nullptr; - - void SetUserSettingsOverride(UserSettings* value) - { - s_UserSettings_Override = value; - } -#endif - - static std::atomic_bool s_userSettingsInitialized{ false }; - static std::atomic_bool s_userSettingsInInitialization{ false }; - - UserSettings const& UserSettings::Instance() - { -#ifndef AICLI_DISABLE_TEST_HOOKS - if (s_UserSettings_Override) - { - return *s_UserSettings_Override; - } -#endif - if (!s_userSettingsInitialized) - { - s_userSettingsInInitialization = true; - } - - static UserSettings userSettings; - s_userSettingsInitialized = true; - s_userSettingsInInitialization = false; - - return userSettings; - } - +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +#include "pch.h" +#include "AppInstallerRuntime.h" +#include "AppInstallerLanguageUtilities.h" +#include "AppInstallerLogging.h" +#include "JsonUtil.h" +#include "winget/Settings.h" +#include "winget/UserSettings.h" + +#include "AppInstallerArchitecture.h" +#include "winget/Locale.h" + +namespace AppInstaller::Settings +{ + using namespace std::string_view_literals; + using namespace Runtime; + using namespace Utility; + + static constexpr std::string_view s_SettingEmpty = + R"({ + "$schema": "https://aka.ms/winget-settings.schema.json", + + // For documentation on these settings, see: https://aka.ms/winget-settings + // "source": { + // "autoUpdateIntervalInMinutes": 5 + // }, +})"sv; + + namespace + { + template + inline std::string GetValueString(T value) + { + std::string convertedValue; + + if constexpr (std::is_arithmetic_v) + { + convertedValue = std::to_string(value); + } + else + { + convertedValue = value; + } + + return convertedValue; + } + + template<> + inline std::string GetValueString(std::vector value) + { + std::string convertedValue = "["; + + bool first = true; + for (auto const& entry : value) + { + if (first) + { + first = false; + } + else + { + convertedValue += ", "; + } + + convertedValue += entry; + } + + convertedValue += ']'; + + return convertedValue; + } + + std::optional ParseFile(const StreamDefinition& setting, std::vector& warnings) + { + auto stream = Stream{ setting }.Get(); + if (stream) + { + Json::Value root; + Json::CharReaderBuilder builder; + const std::unique_ptr reader(builder.newCharReader()); + + std::string settingsContentStr = Utility::ReadEntireStream(*stream); + std::string error; + + if (reader->parse(settingsContentStr.c_str(), settingsContentStr.c_str() + settingsContentStr.size(), &root, &error)) + { + return root; + } + + AICLI_LOG(Core, Error, << "Error parsing " << setting.Name << ": " << error); + warnings.emplace_back(StringResource::String::SettingsWarningParseError, setting.Name, error, false); + } + + return {}; + } + + template + std::optional::json_t> GetValueFromPolicy() + { + return GroupPolicies().GetValue::Policy>(); + } + + template + void Validate( + Json::Value& root, + std::map& settings, + std::vector& warnings) + { + // jsoncpp doesn't support std::string_view yet. + auto path = std::string(details::SettingMapping::Path); + + // Settings set by Group Policy override anything else. See if there is one. + auto policyValue = GetValueFromPolicy(); + if (policyValue.has_value()) + { + // If the value is valid, use it. + // Otherwise, fall back to default. + // In any case, we do not need to read the setting from the JSON. + auto validatedValue = details::SettingMapping::Validate(policyValue.value()); + if (validatedValue.has_value()) + { + // Add it to the map + settings[S].emplace( + std::forward::value_t>(validatedValue.value())); + AICLI_LOG(Core, Verbose, << "Valid setting from Group Policy. Field: " << path << " Value: " << GetValueString(policyValue.value())); + } + else + { + auto valueAsString = GetValueString(policyValue.value()); + AICLI_LOG(Core, Error, << "Invalid setting from Group Policy. Field: " << path << " Value: " << valueAsString); + warnings.emplace_back(StringResource::String::SettingsWarningInvalidValueFromPolicy, path, valueAsString); + } + + return; + } + + const Json::Path jsonPath(path); + Json::Value result = jsonPath.resolve(root); + if (!result.isNull()) + { + auto jsonValue = GetValue::json_t>(result); + + if (jsonValue.has_value()) + { + auto validatedValue = details::SettingMapping::Validate(jsonValue.value()); + + if (validatedValue.has_value()) + { + // Finally add it to the map + settings[S].emplace( + std::forward::value_t>(validatedValue.value())); + AICLI_LOG(Core, Verbose, << "Valid setting. Field: " << path << " Value: " << GetValueString(jsonValue.value())); + } + else + { + auto valueAsString = GetValueString(jsonValue.value()); + AICLI_LOG(Core, Error, << "Invalid field value. Field: " << path << " Value: " << valueAsString); + warnings.emplace_back(StringResource::String::SettingsWarningInvalidFieldValue, path, valueAsString); + } + } + else + { + AICLI_LOG(Core, Error, << "Invalid field format. Field: " << path << " Using default"); + warnings.emplace_back(StringResource::String::SettingsWarningInvalidFieldFormat, path); + } + } + else + { + AICLI_LOG(Core, Verbose, << "Setting " << path << " not found. Using default"); + } + } + + template + void ValidateAll( + Json::Value& root, + std::map& settings, + std::vector& warnings, + std::index_sequence) + { + // Use folding to call each setting validate function. + (FoldHelper{}, ..., Validate(S)>(root, settings, warnings)); + } + } + + namespace details + { +#define WINGET_VALIDATE_SIGNATURE(_setting_) \ + std::optional::value_t> \ + SettingMapping::Validate(const SettingMapping::json_t& value) + + // Stamps out a validate function that simply returns the input value. +#define WINGET_VALIDATE_PASS_THROUGH(_setting_) \ + WINGET_VALIDATE_SIGNATURE(_setting_) \ + { \ + return value; \ + } + + WINGET_VALIDATE_SIGNATURE(AutoUpdateTimeInMinutes) + { + return std::chrono::minutes(value); + } + + WINGET_VALIDATE_SIGNATURE(ProgressBarVisualStyle) + { + // progressBar property possible values + static constexpr std::string_view s_progressBar_Accent = "accent"; + static constexpr std::string_view s_progressBar_Rainbow = "rainbow"; + static constexpr std::string_view s_progressBar_Retro = "retro"; + + if (Utility::CaseInsensitiveEquals(value, s_progressBar_Accent)) + { + return VisualStyle::Accent; + } + else if (Utility::CaseInsensitiveEquals(value, s_progressBar_Rainbow)) + { + return VisualStyle::Rainbow; + } + else if (Utility::CaseInsensitiveEquals(value, s_progressBar_Retro)) + { + return VisualStyle::Retro; + } + + return {}; + } + + WINGET_VALIDATE_PASS_THROUGH(EFExperimentalCmd) + WINGET_VALIDATE_PASS_THROUGH(EFExperimentalArg) + WINGET_VALIDATE_PASS_THROUGH(EFDependencies) + WINGET_VALIDATE_PASS_THROUGH(TelemetryDisable) + WINGET_VALIDATE_PASS_THROUGH(EFDirectMSI) + WINGET_VALIDATE_PASS_THROUGH(EnableSelfInitiatedMinidump) + WINGET_VALIDATE_SIGNATURE(InstallArchitecturePreference) + { + Utility::Architecture arch = Utility::ConvertToArchitectureEnum(value); + if (Utility::IsApplicableArchitecture(arch) != Utility::InapplicableArchitecture) + { + return arch; + } + return {}; + } + WINGET_VALIDATE_SIGNATURE(InstallArchitectureRequirement) + { + return SettingMapping::Validate(value); + } + WINGET_VALIDATE_SIGNATURE(InstallScopePreference) + { + static constexpr std::string_view s_scope_user = "user"; + static constexpr std::string_view s_scope_machine = "machine"; + + if (Utility::CaseInsensitiveEquals(value, s_scope_user)) + { + return ScopePreference::User; + } + else if (Utility::CaseInsensitiveEquals(value, s_scope_machine)) + { + return ScopePreference::Machine; + } + + return {}; + } + + WINGET_VALIDATE_SIGNATURE(InstallScopeRequirement) + { + return SettingMapping::Validate(value); + } + + WINGET_VALIDATE_SIGNATURE(InstallLocalePreference) + { + for (auto const& entry : value) + { + if (!Locale::IsWellFormedBcp47Tag(entry)) + { + return {}; + } + } + + return value; + } + + WINGET_VALIDATE_SIGNATURE(InstallLocaleRequirement) + { + return SettingMapping::Validate(value); + } + + WINGET_VALIDATE_SIGNATURE(NetworkDownloader) + { + static constexpr std::string_view s_downloader_default = "default"; + static constexpr std::string_view s_downloader_wininet = "wininet"; + static constexpr std::string_view s_downloader_do = "do"; + + if (Utility::CaseInsensitiveEquals(value, s_downloader_default)) + { + return InstallerDownloader::Default; + } + else if (Utility::CaseInsensitiveEquals(value, s_downloader_wininet)) + { + return InstallerDownloader::WinInet; + } + else if (Utility::CaseInsensitiveEquals(value, s_downloader_do)) + { + return InstallerDownloader::DeliveryOptimization; + } + + return {}; + } + + WINGET_VALIDATE_SIGNATURE(NetworkDOProgressTimeoutInSeconds) + { + return std::chrono::seconds(value); + } + } + +#ifndef AICLI_DISABLE_TEST_HOOKS + static UserSettings* s_UserSettings_Override = nullptr; + + void SetUserSettingsOverride(UserSettings* value) + { + s_UserSettings_Override = value; + } +#endif + + static std::atomic_bool s_userSettingsInitialized{ false }; + static std::atomic_bool s_userSettingsInInitialization{ false }; + + UserSettings const& UserSettings::Instance() + { +#ifndef AICLI_DISABLE_TEST_HOOKS + if (s_UserSettings_Override) + { + return *s_UserSettings_Override; + } +#endif + if (!s_userSettingsInitialized) + { + s_userSettingsInInitialization = true; + } + + static UserSettings userSettings; + s_userSettingsInitialized = true; + s_userSettingsInInitialization = false; + + return userSettings; + } + const UserSettings* TryGetUser() { if (s_userSettingsInitialized) @@ -348,82 +362,82 @@ namespace AppInstaller::Settings UserSettings const& User() { return UserSettings::Instance(); - } - - UserSettings::UserSettings() : m_type(UserSettingsType::Default) - { - Json::Value settingsRoot = Json::Value::nullSingleton(); - - // Settings can be loaded from settings.json or settings.json.backup files. - // 0 - Use default (empty) settings if disabled by group policy. - // 1 - Use settings.json if exists and passes parsing. - // 2 - Use settings.backup.json if settings.json fails to parse. - // 3 - Use default (empty) if both settings files fail to load. - - if (!GroupPolicies().IsEnabled(TogglePolicy::Policy::Settings)) - { - AICLI_LOG(Core, Info, << "Ignoring settings file due to group policy. Using default values."); - return; - } - - auto settingsJson = ParseFile(Stream::PrimaryUserSettings, m_warnings); - if (settingsJson.has_value()) - { - AICLI_LOG(Core, Info, << "Settings loaded from " << Stream::PrimaryUserSettings.Name); - m_type = UserSettingsType::Standard; - settingsRoot = settingsJson.value(); - } - - // Settings didn't parse or doesn't exist, try with backup. - if (settingsRoot.isNull()) - { - auto settingsBackupJson = ParseFile(Stream::BackupUserSettings, m_warnings); - if (settingsBackupJson.has_value()) - { - AICLI_LOG(Core, Info, << "Settings loaded from " << Stream::BackupUserSettings.Name); - m_warnings.emplace_back(StringResource::String::SettingsWarningLoadedBackupSettings); - m_type = UserSettingsType::Backup; - settingsRoot = settingsBackupJson.value(); - } - } - - if (!settingsRoot.isNull()) - { - ValidateAll(settingsRoot, m_settings, m_warnings, std::make_index_sequence(Setting::Max)>()); - } - else - { - AICLI_LOG(Core, Info, << "Valid settings file not found. Using default values."); - } - } - - void UserSettings::PrepareToShellExecuteFile() const - { - UserSettingsType userSettingType = GetType(); - - if (userSettingType == UserSettingsType::Default) - { - Stream primarySettings{ Stream::PrimaryUserSettings }; - - // Create settings file if it doesn't exist. - if (!std::filesystem::exists(primarySettings.GetPath())) - { - std::ignore = primarySettings.Set(s_SettingEmpty); - AICLI_LOG(Core, Info, << "Created new settings file"); - } - } - else if (userSettingType == UserSettingsType::Standard) - { - // Settings file was loaded correctly, create backup. - auto from = SettingsFilePath(); - auto to = Stream{ Stream::BackupUserSettings }.GetPath(); - std::filesystem::copy_file(from, to, std::filesystem::copy_options::overwrite_existing); - AICLI_LOG(Core, Info, << "Copied settings to backup file"); - } - } - - std::filesystem::path UserSettings::SettingsFilePath() - { - return Stream{ Stream::PrimaryUserSettings }.GetPath(); - } -} + } + + UserSettings::UserSettings() : m_type(UserSettingsType::Default) + { + Json::Value settingsRoot = Json::Value::nullSingleton(); + + // Settings can be loaded from settings.json or settings.json.backup files. + // 0 - Use default (empty) settings if disabled by group policy. + // 1 - Use settings.json if exists and passes parsing. + // 2 - Use settings.backup.json if settings.json fails to parse. + // 3 - Use default (empty) if both settings files fail to load. + + if (!GroupPolicies().IsEnabled(TogglePolicy::Policy::Settings)) + { + AICLI_LOG(Core, Info, << "Ignoring settings file due to group policy. Using default values."); + return; + } + + auto settingsJson = ParseFile(Stream::PrimaryUserSettings, m_warnings); + if (settingsJson.has_value()) + { + AICLI_LOG(Core, Info, << "Settings loaded from " << Stream::PrimaryUserSettings.Name); + m_type = UserSettingsType::Standard; + settingsRoot = settingsJson.value(); + } + + // Settings didn't parse or doesn't exist, try with backup. + if (settingsRoot.isNull()) + { + auto settingsBackupJson = ParseFile(Stream::BackupUserSettings, m_warnings); + if (settingsBackupJson.has_value()) + { + AICLI_LOG(Core, Info, << "Settings loaded from " << Stream::BackupUserSettings.Name); + m_warnings.emplace_back(StringResource::String::SettingsWarningLoadedBackupSettings); + m_type = UserSettingsType::Backup; + settingsRoot = settingsBackupJson.value(); + } + } + + if (!settingsRoot.isNull()) + { + ValidateAll(settingsRoot, m_settings, m_warnings, std::make_index_sequence(Setting::Max)>()); + } + else + { + AICLI_LOG(Core, Info, << "Valid settings file not found. Using default values."); + } + } + + void UserSettings::PrepareToShellExecuteFile() const + { + UserSettingsType userSettingType = GetType(); + + if (userSettingType == UserSettingsType::Default) + { + Stream primarySettings{ Stream::PrimaryUserSettings }; + + // Create settings file if it doesn't exist. + if (!std::filesystem::exists(primarySettings.GetPath())) + { + std::ignore = primarySettings.Set(s_SettingEmpty); + AICLI_LOG(Core, Info, << "Created new settings file"); + } + } + else if (userSettingType == UserSettingsType::Standard) + { + // Settings file was loaded correctly, create backup. + auto from = SettingsFilePath(); + auto to = Stream{ Stream::BackupUserSettings }.GetPath(); + std::filesystem::copy_file(from, to, std::filesystem::copy_options::overwrite_existing); + AICLI_LOG(Core, Info, << "Copied settings to backup file"); + } + } + + std::filesystem::path UserSettings::SettingsFilePath() + { + return Stream{ Stream::PrimaryUserSettings }.GetPath(); + } +} From 01d102916d3fac8b530b2f23e14f21b96b54001c Mon Sep 17 00:00:00 2001 From: Easton Pillay Date: Wed, 24 Nov 2021 20:40:17 -0500 Subject: [PATCH 2/5] Applied suggestions from code review. Sorry for the delay. --- doc/Settings.md | 6 ++--- .../JSON/settings/settings.schema.0.2.json | 26 +++++++++++-------- .../Workflows/ManifestComparator.cpp | 17 ++++-------- .../Workflows/WorkflowBase.cpp | 20 +++++++++++--- .../Public/winget/UserSettings.h | 4 +-- src/AppInstallerCommonCore/UserSettings.cpp | 19 +++++++++----- 6 files changed, 55 insertions(+), 37 deletions(-) diff --git a/doc/Settings.md b/doc/Settings.md index 60c9b32562..d4ddd316bb 100644 --- a/doc/Settings.md +++ b/doc/Settings.md @@ -78,14 +78,14 @@ The `locale` behavior affects the choice of installer based on installer locale. } }, ``` -### Architecture +### Architectures -The `architecture` behavior affects what architecture will be selected when installing a package. The matching parameter is `--architecture`. Note that only architectures compatible with your system can be selected. +The `architectures` behavior affects what architectures will be selected when installing a package. The matching parameter is `--architecture`. Note that only architectures compatible with your system can be selected. ```json "installBehavior": { "preferences": { - "architecture": "x64" + "architectures": "x64" } }, ``` diff --git a/schemas/JSON/settings/settings.schema.0.2.json b/schemas/JSON/settings/settings.schema.0.2.json index cf12f10aff..00105f1624 100644 --- a/schemas/JSON/settings/settings.schema.0.2.json +++ b/schemas/JSON/settings/settings.schema.0.2.json @@ -55,17 +55,21 @@ "minItems": 1, "maxItems": 10 }, - "architecture": { - "description": "The architecture for a package install", - "type": "string", - "enum": [ - "neutral", - "x64", - "x86", - "arm64", - "arm" - ], - "default": "neutral" + "architectures": { + "description": "The architecture(s) to use for a package install", + "type": "array", + "items": { + "type": "string", + "enum": [ + "neutral", + "x64", + "x86", + "arm64", + "arm" + ], + "minItems": 1, + "maxItems": 4 + } } } }, diff --git a/src/AppInstallerCLICore/Workflows/ManifestComparator.cpp b/src/AppInstallerCLICore/Workflows/ManifestComparator.cpp index 81396d0a08..213599b9f8 100644 --- a/src/AppInstallerCLICore/Workflows/ManifestComparator.cpp +++ b/src/AppInstallerCLICore/Workflows/ManifestComparator.cpp @@ -46,11 +46,10 @@ namespace AppInstaller::CLI::Workflow struct MachineArchitectureComparator : public details::ComparisonField { - MachineArchitectureComparator(Utility::Architecture preference) : - details::ComparisonField("Machine Architecture"), m_preference(preference) {} + MachineArchitectureComparator() : details::ComparisonField("Machine Architecture") {} - MachineArchitectureComparator(std::vector allowedArchitectures, Utility::Architecture preference) : - details::ComparisonField("Machine Architecture"), m_allowedArchitectures(std::move(allowedArchitectures)), m_preference(preference) + MachineArchitectureComparator(std::vector allowedArchitectures) : + details::ComparisonField("Machine Architecture"), m_allowedArchitectures(std::move(allowedArchitectures)) { AICLI_LOG(CLI, Verbose, << "Architecture Comparator created with allowed architectures: " << GetAllowedArchitecturesString()); } @@ -58,7 +57,6 @@ namespace AppInstaller::CLI::Workflow // TODO: At some point we can do better about matching the currently installed architecture static std::unique_ptr Create(const Execution::Context& context, const Repository::IPackageVersion::Metadata&) { - Utility::Architecture preference = Settings::User().Get(); if (context.Contains(Execution::Data::AllowedArchitectures)) { const std::vector& allowedArchitectures = context.Get(); @@ -97,11 +95,11 @@ namespace AppInstaller::CLI::Workflow } } - return std::make_unique(std::move(result), preference); + return std::make_unique(std::move(result)); } } - return std::make_unique(preference); + return std::make_unique(); } InapplicabilityFlags IsApplicable(const Manifest::ManifestInstaller& installer) override @@ -131,10 +129,6 @@ namespace AppInstaller::CLI::Workflow bool IsFirstBetter(const Manifest::ManifestInstaller& first, const Manifest::ManifestInstaller& second) override { - if (m_preference != Utility::Architecture::Neutral) - { - return (first.Arch == m_preference && second.Arch != m_preference); - } auto arch1 = CheckAllowedArchitecture(first.Arch); auto arch2 = CheckAllowedArchitecture(second.Arch); @@ -176,7 +170,6 @@ namespace AppInstaller::CLI::Workflow } std::vector m_allowedArchitectures; - Utility::Architecture m_preference; }; struct InstalledTypeComparator : public details::ComparisonField diff --git a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp index 2b62a79026..7c007d7f8e 100644 --- a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp +++ b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp @@ -962,7 +962,8 @@ namespace AppInstaller::CLI::Workflow bool isUpdate = WI_IsFlagSet(context.GetFlags(), Execution::ContextFlag::InstallerExecutionUseUpdate); IPackageVersion::Metadata installationMetadata; - Utility::Architecture requiredArchitecture = Settings::User().Get(); + std::vector requiredArchitectures = Settings::User().Get(); + std::vector optionalArchitectures = Settings::User().Get(); if (isUpdate) { installationMetadata = context.Get()->GetMetadata(); @@ -972,9 +973,22 @@ namespace AppInstaller::CLI::Workflow // arguments override settings. context.Add({ Utility::ConvertToArchitectureEnum(std::string(context.Args.GetArg(Execution::Args::Type::InstallArchitecture))) }); } - else if (requiredArchitecture != Utility::Architecture::Neutral) + else { - context.Add({ requiredArchitecture }); + std::set settingArchitectures; + if (!optionalArchitectures.empty()) + { + std::copy(optionalArchitectures.begin(), optionalArchitectures.end(), std::inserter(settingArchitectures, settingArchitectures.begin())); + } + if (!requiredArchitectures.empty()) + { + std::copy(requiredArchitectures.begin(), requiredArchitectures.end(), std::inserter(settingArchitectures, settingArchitectures.begin())); + } + else + { + settingArchitectures.emplace(Utility::Architecture::Unknown); + } + context.Add({ settingArchitectures.begin(), settingArchitectures.end() }); } ManifestComparator manifestComparator(context, installationMetadata); auto [installer, inapplicabilities] = manifestComparator.GetPreferredInstaller(context.Get()); diff --git a/src/AppInstallerCommonCore/Public/winget/UserSettings.h b/src/AppInstallerCommonCore/Public/winget/UserSettings.h index 85050fef45..7cf95b2ec1 100644 --- a/src/AppInstallerCommonCore/Public/winget/UserSettings.h +++ b/src/AppInstallerCommonCore/Public/winget/UserSettings.h @@ -121,8 +121,8 @@ namespace AppInstaller::Settings SETTINGMAPPING_SPECIALIZATION(Setting::EFExperimentalArg, bool, bool, false, ".experimentalFeatures.experimentalArg"sv); SETTINGMAPPING_SPECIALIZATION(Setting::EFDependencies, bool, bool, false, ".experimentalFeatures.dependencies"sv); SETTINGMAPPING_SPECIALIZATION(Setting::TelemetryDisable, bool, bool, false, ".telemetry.disable"sv); - SETTINGMAPPING_SPECIALIZATION(Setting::InstallArchitecturePreference, std::string, Utility::Architecture, Utility::Architecture::Neutral, ".installBehavior.preferences.architecture"sv); - SETTINGMAPPING_SPECIALIZATION(Setting::InstallArchitectureRequirement, std::string, Utility::Architecture, Utility::Architecture::Neutral, ".installBehavior.requirements.architecture"sv); + SETTINGMAPPING_SPECIALIZATION(Setting::InstallArchitecturePreference, std::vector, std::vector, {}, ".installBehavior.preferences.architectures"sv); + SETTINGMAPPING_SPECIALIZATION(Setting::InstallArchitectureRequirement, std::vector, std::vector, {}, ".installBehavior.requirements.architectures"sv); SETTINGMAPPING_SPECIALIZATION(Setting::InstallScopePreference, std::string, ScopePreference, ScopePreference::User, ".installBehavior.preferences.scope"sv); SETTINGMAPPING_SPECIALIZATION(Setting::InstallScopeRequirement, std::string, ScopePreference, ScopePreference::None, ".installBehavior.requirements.scope"sv); SETTINGMAPPING_SPECIALIZATION(Setting::NetworkDownloader, std::string, InstallerDownloader, InstallerDownloader::Default, ".network.downloader"sv); diff --git a/src/AppInstallerCommonCore/UserSettings.cpp b/src/AppInstallerCommonCore/UserSettings.cpp index 514ec33a67..8ea3b34585 100644 --- a/src/AppInstallerCommonCore/UserSettings.cpp +++ b/src/AppInstallerCommonCore/UserSettings.cpp @@ -230,19 +230,26 @@ namespace AppInstaller::Settings WINGET_VALIDATE_PASS_THROUGH(TelemetryDisable) WINGET_VALIDATE_PASS_THROUGH(EFDirectMSI) WINGET_VALIDATE_PASS_THROUGH(EnableSelfInitiatedMinidump) + WINGET_VALIDATE_SIGNATURE(InstallArchitecturePreference) { - Utility::Architecture arch = Utility::ConvertToArchitectureEnum(value); - if (Utility::IsApplicableArchitecture(arch) != Utility::InapplicableArchitecture) - { - return arch; - } - return {}; + std::vector archs; + for (auto const& i : value) { + Utility::Architecture arch = Utility::ConvertToArchitectureEnum(i); + if (Utility::IsApplicableArchitecture(arch) == Utility::InapplicableArchitecture) + { + return {}; + } + archs.emplace_back(arch); + } + return archs; } + WINGET_VALIDATE_SIGNATURE(InstallArchitectureRequirement) { return SettingMapping::Validate(value); } + WINGET_VALIDATE_SIGNATURE(InstallScopePreference) { static constexpr std::string_view s_scope_user = "user"; From a7286dcb097936e361884fb006311d5d7fb1bcda Mon Sep 17 00:00:00 2001 From: Easton Pillay Date: Tue, 30 Nov 2021 09:32:21 -0500 Subject: [PATCH 3/5] Applied suggestions from code review. --- .../Workflows/WorkflowBase.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp index 7c007d7f8e..d0e9bbdfb7 100644 --- a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp +++ b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp @@ -962,8 +962,7 @@ namespace AppInstaller::CLI::Workflow bool isUpdate = WI_IsFlagSet(context.GetFlags(), Execution::ContextFlag::InstallerExecutionUseUpdate); IPackageVersion::Metadata installationMetadata; - std::vector requiredArchitectures = Settings::User().Get(); - std::vector optionalArchitectures = Settings::User().Get(); + if (isUpdate) { installationMetadata = context.Get()->GetMetadata(); @@ -975,20 +974,21 @@ namespace AppInstaller::CLI::Workflow } else { + std::vector requiredArchitectures = Settings::User().Get(); + std::vector optionalArchitectures = Settings::User().Get(); std::set settingArchitectures; - if (!optionalArchitectures.empty()) - { - std::copy(optionalArchitectures.begin(), optionalArchitectures.end(), std::inserter(settingArchitectures, settingArchitectures.begin())); - } + if (!requiredArchitectures.empty()) { std::copy(requiredArchitectures.begin(), requiredArchitectures.end(), std::inserter(settingArchitectures, settingArchitectures.begin())); + context.Add({ settingArchitectures.begin(), settingArchitectures.end() }); } - else + else if (!optionalArchitectures.empty()) { settingArchitectures.emplace(Utility::Architecture::Unknown); + std::copy(optionalArchitectures.begin(), optionalArchitectures.end(), std::inserter(settingArchitectures, settingArchitectures.begin())); + context.Add({ settingArchitectures.begin(), settingArchitectures.end() }); } - context.Add({ settingArchitectures.begin(), settingArchitectures.end() }); } ManifestComparator manifestComparator(context, installationMetadata); auto [installer, inapplicabilities] = manifestComparator.GetPreferredInstaller(context.Get()); From f535556c949030640580d074d62ef8e0ad004247 Mon Sep 17 00:00:00 2001 From: Easton Pillay Date: Tue, 30 Nov 2021 09:37:59 -0500 Subject: [PATCH 4/5] Made doc match schema change. --- doc/Settings.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/Settings.md b/doc/Settings.md index d4ddd316bb..8e463f7b28 100644 --- a/doc/Settings.md +++ b/doc/Settings.md @@ -85,7 +85,7 @@ The `architectures` behavior affects what architectures will be selected when in ```json "installBehavior": { "preferences": { - "architectures": "x64" + "architectures": ["x64", "arm64"] } }, ``` From 0384fcd3657942f59bc94aae3dd888742f9ef632 Mon Sep 17 00:00:00 2001 From: Easton Pillay Date: Tue, 30 Nov 2021 14:43:55 -0500 Subject: [PATCH 5/5] Made sure that Architecture::Unknown was going to the back of the vector :D --- schemas/JSON/settings/settings.schema.0.2.json | 1 + src/AppInstallerCLICore/Workflows/WorkflowBase.cpp | 11 +++++------ 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/schemas/JSON/settings/settings.schema.0.2.json b/schemas/JSON/settings/settings.schema.0.2.json index 00105f1624..2a9ab5bbd8 100644 --- a/schemas/JSON/settings/settings.schema.0.2.json +++ b/schemas/JSON/settings/settings.schema.0.2.json @@ -59,6 +59,7 @@ "description": "The architecture(s) to use for a package install", "type": "array", "items": { + "uniqueItems": "true", "type": "string", "enum": [ "neutral", diff --git a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp index d0e9bbdfb7..3c10a3cd4f 100644 --- a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp +++ b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp @@ -976,18 +976,17 @@ namespace AppInstaller::CLI::Workflow { std::vector requiredArchitectures = Settings::User().Get(); std::vector optionalArchitectures = Settings::User().Get(); - std::set settingArchitectures; + if (!requiredArchitectures.empty()) { - std::copy(requiredArchitectures.begin(), requiredArchitectures.end(), std::inserter(settingArchitectures, settingArchitectures.begin())); - context.Add({ settingArchitectures.begin(), settingArchitectures.end() }); + context.Add({ requiredArchitectures.begin(), requiredArchitectures.end() }); } else if (!optionalArchitectures.empty()) { - settingArchitectures.emplace(Utility::Architecture::Unknown); - std::copy(optionalArchitectures.begin(), optionalArchitectures.end(), std::inserter(settingArchitectures, settingArchitectures.begin())); - context.Add({ settingArchitectures.begin(), settingArchitectures.end() }); + optionalArchitectures.emplace_back(Utility::Architecture::Unknown); + context.Add({ optionalArchitectures.begin(), optionalArchitectures.end() }); + } } ManifestComparator manifestComparator(context, installationMetadata);