commit 8a805ccbe9dbb2cd87709bfa4bce11e8a0fd2510 parent 23f9942415b83e6187beac122de6bd1c0bc2a009 Author: Ruben Guerrero <rubengu@microsoft.com> Date: Thu, 15 Dec 2022 12:11:54 -0800 Fix winget after a call to winget settings export (#2767) Diffstat:
15 files changed, 138 insertions(+), 82 deletions(-)
diff --git a/src/AppInstallerCLICore/Command.cpp b/src/AppInstallerCLICore/Command.cpp @@ -30,8 +30,9 @@ namespace AppInstaller::CLI std::string_view parent, Command::Visibility visibility, Settings::ExperimentalFeature::Feature feature, - Settings::TogglePolicy::Policy groupPolicy) : - m_name(name), m_aliases(std::move(aliases)), m_visibility(visibility), m_feature(feature), m_groupPolicy(groupPolicy) + Settings::TogglePolicy::Policy groupPolicy, + CommandOutputFlags outputFlags) : + m_name(name), m_aliases(std::move(aliases)), m_visibility(visibility), m_feature(feature), m_groupPolicy(groupPolicy), m_outputFlags(outputFlags) { if (!parent.empty()) { @@ -933,7 +934,8 @@ namespace AppInstaller::CLI { try { - if (!Settings::User().GetWarnings().empty()) + if (!Settings::User().GetWarnings().empty() && + !WI_IsFlagSet(command->GetOutputFlags(), CommandOutputFlags::IgnoreSettingsWarnings)) { context.Reporter.Warn() << Resource::String::SettingsWarnings << std::endl; } diff --git a/src/AppInstallerCLICore/Command.h b/src/AppInstallerCLICore/Command.h @@ -44,6 +44,15 @@ namespace AppInstaller::CLI std::vector<Utility::LocIndString> m_params; }; + // Flags to control the behavior of the command output. + enum class CommandOutputFlags : int + { + None = 0x0, + IgnoreSettingsWarnings = 0x1, + }; + + DEFINE_ENUM_FLAG_OPERATORS(CommandOutputFlags); + struct Command { // Controls the visibility of the field. @@ -57,17 +66,27 @@ namespace AppInstaller::CLI Command(std::string_view name, std::string_view parent) : Command(name, {}, parent) {} - Command(std::string_view name,std::vector<std::string_view> aliases, std::string_view parent) : + Command(std::string_view name, std::vector<std::string_view> aliases, std::string_view parent) : Command(name, aliases, parent, Settings::ExperimentalFeature::Feature::None) {} - Command(std::string_view name,std::vector<std::string_view> aliases, std::string_view parent, Command::Visibility visibility) : + Command(std::string_view name, std::string_view parent, CommandOutputFlags outputFlags) : + Command(name, {}, parent, Command::Visibility::Show, Settings::ExperimentalFeature::Feature::None, Settings::TogglePolicy::Policy::None, outputFlags) {} + Command(std::string_view name, std::vector<std::string_view> aliases, std::string_view parent, Command::Visibility visibility) : Command(name, aliases, parent, visibility, Settings::ExperimentalFeature::Feature::None) {} - Command(std::string_view name,std::vector<std::string_view> aliases, std::string_view parent, Settings::ExperimentalFeature::Feature feature) : + Command(std::string_view name, std::vector<std::string_view> aliases, std::string_view parent, Settings::ExperimentalFeature::Feature feature) : Command(name, aliases, parent, Command::Visibility::Show, feature) {} - Command(std::string_view name,std::vector<std::string_view> aliases, std::string_view parent, Settings::TogglePolicy::Policy groupPolicy) : - Command(name, aliases, parent, Command::Visibility::Show, Settings::ExperimentalFeature::Feature::None, groupPolicy) {} - Command(std::string_view name,std::vector<std::string_view> aliases, std::string_view parent, Command::Visibility visibility, Settings::ExperimentalFeature::Feature feature) : - Command(name, aliases, parent, visibility, feature, Settings::TogglePolicy::Policy::None) {} - Command(std::string_view name,std::vector<std::string_view> aliases, std::string_view parent, Command::Visibility visibility, Settings::ExperimentalFeature::Feature feature, Settings::TogglePolicy::Policy groupPolicy); + Command(std::string_view name, std::vector<std::string_view> aliases, std::string_view parent, Settings::TogglePolicy::Policy groupPolicy) : + Command(name, aliases, parent, Command::Visibility::Show, Settings::ExperimentalFeature::Feature::None, groupPolicy, CommandOutputFlags::None) {} + Command(std::string_view name, std::vector<std::string_view> aliases, std::string_view parent, Command::Visibility visibility, Settings::ExperimentalFeature::Feature feature) : + Command(name, aliases, parent, visibility, feature, Settings::TogglePolicy::Policy::None, CommandOutputFlags::None) {} + + Command(std::string_view name, + std::vector<std::string_view> aliases, + std::string_view parent, + Command::Visibility visibility, + Settings::ExperimentalFeature::Feature feature, + Settings::TogglePolicy::Policy groupPolicy, + CommandOutputFlags outputFlags); + virtual ~Command() = default; Command(const Command&) = default; @@ -85,6 +104,7 @@ namespace AppInstaller::CLI Command::Visibility GetVisibility() const; Settings::ExperimentalFeature::Feature Feature() const { return m_feature; } Settings::TogglePolicy::Policy GroupPolicy() const { return m_groupPolicy; } + CommandOutputFlags GetOutputFlags() const { return m_outputFlags; } virtual std::vector<std::unique_ptr<Command>> GetCommands() const { return {}; } virtual std::vector<Argument> GetArguments() const { return {}; } @@ -118,6 +138,7 @@ namespace AppInstaller::CLI Command::Visibility m_visibility; Settings::ExperimentalFeature::Feature m_feature; Settings::TogglePolicy::Policy m_groupPolicy; + CommandOutputFlags m_outputFlags; }; template <typename Container> diff --git a/src/AppInstallerCLICore/Commands/RootCommand.cpp b/src/AppInstallerCLICore/Commands/RootCommand.cpp @@ -27,6 +27,8 @@ using namespace AppInstaller::Utility::literals; namespace AppInstaller::CLI { + using namespace Settings; + namespace { void OutputGroupPolicySourceList(Execution::Context& context, const std::vector<Settings::SourceFromPolicy>& sources, Resource::StringId header) @@ -193,7 +195,7 @@ namespace AppInstaller::CLI }; info << std::endl << Resource::String::Logs << ": "_liv << Runtime::GetPathTo(Runtime::PathName::DefaultLogLocationForDisplay).u8string() << std::endl; - info << std::endl << Resource::String::UserSettings << ": "_liv << Runtime::GetPathTo(Runtime::PathName::UserSettingsFileLocationForDisplay).u8string() << std::endl; + info << std::endl << Resource::String::UserSettings << ": "_liv << UserSettings::SettingsFilePath(true).u8string() << std::endl; info << std::endl; diff --git a/src/AppInstallerCLICore/Commands/SettingsCommand.h b/src/AppInstallerCLICore/Commands/SettingsCommand.h @@ -24,7 +24,7 @@ namespace AppInstaller::CLI struct SettingsExportCommand final : public Command { - SettingsExportCommand(std::string_view parent) : Command("export", parent) {} + SettingsExportCommand(std::string_view parent) : Command("export", parent, CommandOutputFlags::IgnoreSettingsWarnings) {} Resource::LocString ShortDescription() const override; Resource::LocString LongDescription() const override; diff --git a/src/AppInstallerCLICore/Commands/SourceCommand.h b/src/AppInstallerCLICore/Commands/SourceCommand.h @@ -106,7 +106,7 @@ namespace AppInstaller::CLI struct SourceExportCommand final : public Command { - SourceExportCommand(std::string_view parent) : Command("export", parent) {} + SourceExportCommand(std::string_view parent) : Command("export", parent, CommandOutputFlags::IgnoreSettingsWarnings) {} std::vector<Argument> GetArguments() const override; diff --git a/src/AppInstallerCLICore/Workflows/SettingsFlow.cpp b/src/AppInstallerCLICore/Workflows/SettingsFlow.cpp @@ -20,7 +20,7 @@ namespace AppInstaller::CLI::Workflow { root["$schema"] = "https://aka.ms/winget-settings-export.schema.json"; root["adminSettings"] = Json::ValueType::objectValue; - root["userSettingsFile"] = Runtime::GetPathTo(Runtime::PathName::UserSettingsFileLocation).u8string(); + root["userSettingsFile"] = UserSettings::SettingsFilePath().u8string(); } void AddAdminSetting(AdminSetting setting) diff --git a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw @@ -1541,4 +1541,7 @@ Please specify one of them using the --source option to proceed.</value> <data name="UserSettings" xml:space="preserve"> <value>User Settings</value> </data> + <data name="SettingsWarningUsingDefault" xml:space="preserve"> + <value>Settings file couldn't load. Using default values.</value> + </data> </root> diff --git a/src/AppInstallerCLITests/UserSettings.cpp b/src/AppInstallerCLITests/UserSettings.cpp @@ -419,12 +419,16 @@ TEST_CASE("SettingsPortablePackageUserRoot", "[settings]") SECTION("Relative path") { DeleteUserSettingsFiles(); - std::string_view json = R"({ "installBehavior": { "portablePackageUserRoot": %LOCALAPPDATA%/Portable/Root } })"; + std::string_view json = R"({ "installBehavior": { "portablePackageUserRoot": "%LOCALAPPDATA%/Portable/Root" } })"; SetSetting(Stream::PrimaryUserSettings, json); UserSettingsTest userSettingTest; REQUIRE(userSettingTest.Get<Setting::PortablePackageUserRoot>().empty()); - REQUIRE(userSettingTest.GetWarnings().size() == 1); + + auto warnings = userSettingTest.GetWarnings(); + REQUIRE(warnings.size() == 1); + REQUIRE(warnings[0].Message == AppInstaller::StringResource::String::SettingsWarningInvalidFieldValue); + REQUIRE(warnings[0].Path == ".installBehavior.portablePackageUserRoot"); } SECTION("Valid path") { @@ -443,12 +447,16 @@ TEST_CASE("SettingsPortablePackageMachineRoot", "[settings]") SECTION("Relative path") { DeleteUserSettingsFiles(); - std::string_view json = R"({ "installBehavior": { "portablePackageMachineRoot": %LOCALAPPDATA%/Portable/Root } })"; + std::string_view json = R"({ "installBehavior": { "portablePackageMachineRoot": "%LOCALAPPDATA%/Portable/Root" } })"; SetSetting(Stream::PrimaryUserSettings, json); UserSettingsTest userSettingTest; REQUIRE(userSettingTest.Get<Setting::PortablePackageMachineRoot>().empty()); - REQUIRE(userSettingTest.GetWarnings().size() == 1); + + auto warnings = userSettingTest.GetWarnings(); + REQUIRE(warnings.size() == 1); + REQUIRE(warnings[0].Message == AppInstaller::StringResource::String::SettingsWarningInvalidFieldValue); + REQUIRE(warnings[0].Path == ".installBehavior.portablePackageMachineRoot"); } SECTION("Valid path") { diff --git a/src/AppInstallerCommonCore/Filesystem.cpp b/src/AppInstallerCommonCore/Filesystem.cpp @@ -201,4 +201,41 @@ namespace AppInstaller::Filesystem return path; } } + + void ReplaceCommonPathPrefix(std::filesystem::path& source, const std::filesystem::path& prefix, std::string_view replacement) + { + auto prefixItr = prefix.begin(); + auto sourceItr = source.begin(); + + while (prefixItr != prefix.end() && sourceItr != source.end()) + { + if (*prefixItr != *sourceItr) + { + break; + } + + ++prefixItr; + ++sourceItr; + } + + // Only replace source if we found all of prefix + if (prefixItr == prefix.end()) + { + std::filesystem::path temp{ replacement }; + + for (; sourceItr != source.end(); ++sourceItr) + { + temp /= *sourceItr; + } + + source = std::move(temp); + } + } + + std::filesystem::path GetKnownFolderPath(const KNOWNFOLDERID& id) + { + wil::unique_cotaskmem_string knownFolder = nullptr; + THROW_IF_FAILED(SHGetKnownFolderPath(id, KF_FLAG_NO_ALIAS | KF_FLAG_DONT_VERIFY | KF_FLAG_NO_PACKAGE_REDIRECTION, NULL, &knownFolder)); + return knownFolder.get(); + } } \ No newline at end of file diff --git a/src/AppInstallerCommonCore/Public/AppInstallerRuntime.h b/src/AppInstallerCommonCore/Public/AppInstallerRuntime.h @@ -62,10 +62,6 @@ namespace AppInstaller::Runtime PortableLinksUserLocation, // The location where symlinks to portable packages are stored under machine scope. PortableLinksMachineLocation, - // The location of the user settings json file. - UserSettingsFileLocation, - // The location of the user settings json file, anonymized using environment variables. - UserSettingsFileLocationForDisplay, }; // The principal that an ACE applies to. diff --git a/src/AppInstallerCommonCore/Public/winget/Filesystem.h b/src/AppInstallerCommonCore/Public/winget/Filesystem.h @@ -2,6 +2,7 @@ // Licensed under the MIT License. #pragma once #include <filesystem> +#include <shtypes.h> namespace AppInstaller::Filesystem { @@ -35,4 +36,10 @@ namespace AppInstaller::Filesystem // Get expanded file system path. std::filesystem::path GetExpandedPath(const std::string& path); + + // If `source` begins with all of `prefix`, replace that with `replacement`. + void ReplaceCommonPathPrefix(std::filesystem::path& source, const std::filesystem::path& prefix, std::string_view replacement); + + // Gets the path of a known folder. + std::filesystem::path GetKnownFolderPath(const KNOWNFOLDERID& id); } \ No newline at end of file diff --git a/src/AppInstallerCommonCore/Public/winget/Resources.h b/src/AppInstallerCommonCore/Public/winget/Resources.h @@ -43,6 +43,7 @@ namespace AppInstaller WINGET_DEFINE_RESOURCE_STRINGID(SettingsWarningInvalidValueFromPolicy); WINGET_DEFINE_RESOURCE_STRINGID(SettingsWarningLoadedBackupSettings); WINGET_DEFINE_RESOURCE_STRINGID(SettingsWarningParseError); + WINGET_DEFINE_RESOURCE_STRINGID(SettingsWarningUsingDefault); }; } diff --git a/src/AppInstallerCommonCore/Public/winget/UserSettings.h b/src/AppInstallerCommonCore/Public/winget/UserSettings.h @@ -206,7 +206,7 @@ namespace AppInstaller::Settings static UserSettings const& Instance(const std::optional<std::string>& content = std::nullopt); - static std::filesystem::path SettingsFilePath(); + static std::filesystem::path SettingsFilePath(bool forDisplay = false); UserSettings(const UserSettings&) = delete; UserSettings& operator=(const UserSettings&) = delete; diff --git a/src/AppInstallerCommonCore/Runtime.cpp b/src/AppInstallerCommonCore/Runtime.cpp @@ -16,6 +16,7 @@ namespace AppInstaller::Runtime { using namespace Utility; using namespace Settings; + using namespace Filesystem; namespace { @@ -96,13 +97,6 @@ namespace AppInstaller::Runtime static std::map<PathName, PathDetails> s_Path_TestHook_Overrides; #endif - std::filesystem::path GetKnownFolderPath(const KNOWNFOLDERID& id) - { - wil::unique_cotaskmem_string knownFolder = nullptr; - THROW_IF_FAILED(SHGetKnownFolderPath(id, KF_FLAG_NO_ALIAS | KF_FLAG_DONT_VERIFY | KF_FLAG_NO_PACKAGE_REDIRECTION, NULL, &knownFolder)); - return knownFolder.get(); - } - // Gets the user's temp path std::filesystem::path GetPathToUserTemp() { @@ -166,37 +160,6 @@ namespace AppInstaller::Runtime return result; } - // If `source` begins with all of `prefix`, replace that with `replacement`. - void ReplaceCommonPathPrefix(std::filesystem::path& source, const std::filesystem::path& prefix, std::string_view replacement) - { - auto prefixItr = prefix.begin(); - auto sourceItr = source.begin(); - - while (prefixItr != prefix.end() && sourceItr != source.end()) - { - if (*prefixItr != *sourceItr) - { - break; - } - - ++prefixItr; - ++sourceItr; - } - - // Only replace source if we found all of prefix - if (prefixItr == prefix.end()) - { - std::filesystem::path temp{ replacement }; - - for (; sourceItr != source.end(); ++sourceItr) - { - temp /= *sourceItr; - } - - source = std::move(temp); - } - } - DWORD AccessPermissionsFrom(ACEPermissions permissions) { DWORD result = 0; @@ -547,13 +510,6 @@ namespace AppInstaller::Runtime case PathName::PortableLinksMachineLocation: result = GetPathDetailsCommon(path); break; - case PathName::UserSettingsFileLocation: - result.Path = UserSettings::SettingsFilePath(); - break; - case PathName::UserSettingsFileLocationForDisplay: - result.Path = UserSettings::SettingsFilePath(); - ReplaceCommonPathPrefix(result.Path, GetKnownFolderPath(FOLDERID_LocalAppData), "%LOCALAPPDATA%"); - break; default: THROW_HR(E_UNEXPECTED); } @@ -624,13 +580,6 @@ namespace AppInstaller::Runtime case PathName::PortableLinksMachineLocation: result = GetPathDetailsCommon(path); break; - case PathName::UserSettingsFileLocation: - result.Path = UserSettings::SettingsFilePath(); - break; - case PathName::UserSettingsFileLocationForDisplay: - result.Path = UserSettings::SettingsFilePath(); - ReplaceCommonPathPrefix(result.Path, GetKnownFolderPath(FOLDERID_LocalAppData), "%LOCALAPPDATA%"); - break; default: THROW_HR(E_UNEXPECTED); } diff --git a/src/AppInstallerCommonCore/UserSettings.cpp b/src/AppInstallerCommonCore/UserSettings.cpp @@ -7,6 +7,7 @@ #include "winget/JsonUtil.h" #include "winget/Settings.h" #include "winget/UserSettings.h" +#include "winget/filesystem.h" #include "AppInstallerArchitecture.h" #include "winget/Locale.h" @@ -18,6 +19,7 @@ namespace AppInstaller::Settings using namespace Utility; using namespace Logging; using namespace JSON; + using namespace Filesystem; static constexpr std::string_view s_SettingEmpty = R"({ @@ -93,11 +95,22 @@ namespace AppInstaller::Settings std::optional<Json::Value> ParseFile(const StreamDefinition& setting, std::vector<UserSettings::Warning>& warnings) { - auto stream = Stream{ setting }.Get(); - if (stream) + try { - std::string settingsContentStr = Utility::ReadEntireStream(*stream); - return ParseSettingsContent(settingsContentStr, setting.Name, warnings); + auto stream = Stream{ setting }.Get(); + if (stream) + { + std::string settingsContentStr = Utility::ReadEntireStream(*stream); + return ParseSettingsContent(settingsContentStr, setting.Name, warnings); + } + } + catch (const std::exception& e) + { + AICLI_LOG(Core, Error, << "Failed to read " << setting.Name << "; Reason: " << e.what()); + } + catch (...) + { + AICLI_LOG(Core, Error, << "Failed to read " << setting.Name << "; Reason unknown."); } return {}; @@ -505,6 +518,16 @@ namespace AppInstaller::Settings m_type = UserSettingsType::Backup; settingsRoot = settingsBackupJson.value(); } + else + { + // Settings and back up didn't parse or exist. If they exist then warn the user. + auto settingsPath = Stream{ Stream::PrimaryUserSettings }.GetPath(); + auto backupPath = Stream{ Stream::BackupUserSettings }.GetPath(); + if (std::filesystem::exists(settingsPath) || std::filesystem::exists(backupPath)) + { + m_warnings.emplace_back(StringResource::String::SettingsWarningUsingDefault); + } + } } } @@ -543,8 +566,15 @@ namespace AppInstaller::Settings } } - std::filesystem::path UserSettings::SettingsFilePath() + std::filesystem::path UserSettings::SettingsFilePath(bool forDisplay) { - return Stream{ Stream::PrimaryUserSettings }.GetPath(); + auto path = Stream{ Stream::PrimaryUserSettings }.GetPath(); + + if (forDisplay) + { + ReplaceCommonPathPrefix(path, GetKnownFolderPath(FOLDERID_LocalAppData), "%LOCALAPPDATA%"); + } + + return path; } }