From bef119b03b1124c2890bfeadf41d8a6d65df5019 Mon Sep 17 00:00:00 2001 From: yuyoyuppe Date: Tue, 9 Nov 2021 22:48:07 +0300 Subject: [PATCH] [Setup] Add logging for registry changes + add logger for powerpreview - cleanup logger project + remove SettingsAPI dependency --- .github/actions/spell-check/expect.txt | 3 +- installer/PowerToysSetup.sln | 12 +++ .../CustomAction.cpp | 34 +++++++- .../PowerToysSetupCustomActions.vcxproj | 6 ++ .../PowerToysSetupCustomActions/stdafx.h | 1 + src/common/logger/logger.cpp | 45 +++++++--- src/common/logger/logger.h | 1 + src/common/logger/logger.vcxproj | 3 - src/common/logger/logger_settings.cpp | 2 +- src/common/logger/logger_settings.h | 2 + src/common/utils/registry.h | 86 +++++++++++++++---- .../ShortcutGuide/ShortcutGuide.vcxproj | 3 + .../ShortcutGuideModuleInterface.vcxproj | 3 + .../fancyzones/FancyZones/FancyZones.vcxproj | 3 + .../FancyZonesModuleInterface.vcxproj | 3 + .../UnitTests/UnitTests.vcxproj | 3 + .../common/KeyboardManagerCommon.vcxproj | 3 + .../previewpane/powerpreview/powerpreview.cpp | 14 ++- 18 files changed, 187 insertions(+), 40 deletions(-) diff --git a/.github/actions/spell-check/expect.txt b/.github/actions/spell-check/expect.txt index 5efd5f1d1f..48d6554619 100644 --- a/.github/actions/spell-check/expect.txt +++ b/.github/actions/spell-check/expect.txt @@ -631,6 +631,7 @@ Farbraum FARPROC fdw feimage +FFAA ffcd FFDDDDDD fff @@ -655,7 +656,6 @@ finalizer findfast findstr FIXEDFILEINFO -FFAA FLASHZONES FLASHZONESONQUICKSWITCH flt @@ -2439,6 +2439,7 @@ workaround Workflow workspaces wostream +wostringstream wox wparam wpf diff --git a/installer/PowerToysSetup.sln b/installer/PowerToysSetup.sln index e9e00eaa84..cd095d5369 100644 --- a/installer/PowerToysSetup.sln +++ b/installer/PowerToysSetup.sln @@ -7,6 +7,10 @@ Project("{930C7802-8A8C-48F9-8165-68863BCCD9DD}") = "PowerToysSetup", "PowerToys EndProject Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "PowerToysSetupCustomActions", "PowerToysSetupCustomActions\PowerToysSetupCustomActions.vcxproj", "{32F3882B-F2D6-4586-B5ED-11E39E522BD3}" EndProject +Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "spdlog", "..\src\logging\logging.vcxproj", "{7E1E3F13-2BD6-3F75-A6A7-873A2B55C60F}" +EndProject +Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "logger", "..\src\common\logger\logger.vcxproj", "{D9B8FC84-322A-4F9F-BBB9-20915C47DDFD}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|x64 = Debug|x64 @@ -21,6 +25,14 @@ Global {32F3882B-F2D6-4586-B5ED-11E39E522BD3}.Debug|x64.Build.0 = Debug|x64 {32F3882B-F2D6-4586-B5ED-11E39E522BD3}.Release|x64.ActiveCfg = Release|x64 {32F3882B-F2D6-4586-B5ED-11E39E522BD3}.Release|x64.Build.0 = Release|x64 + {7E1E3F13-2BD6-3F75-A6A7-873A2B55C60F}.Debug|x64.ActiveCfg = Debug|x64 + {7E1E3F13-2BD6-3F75-A6A7-873A2B55C60F}.Debug|x64.Build.0 = Debug|x64 + {7E1E3F13-2BD6-3F75-A6A7-873A2B55C60F}.Release|x64.ActiveCfg = Release|x64 + {7E1E3F13-2BD6-3F75-A6A7-873A2B55C60F}.Release|x64.Build.0 = Release|x64 + {D9B8FC84-322A-4F9F-BBB9-20915C47DDFD}.Debug|x64.ActiveCfg = Debug|x64 + {D9B8FC84-322A-4F9F-BBB9-20915C47DDFD}.Debug|x64.Build.0 = Debug|x64 + {D9B8FC84-322A-4F9F-BBB9-20915C47DDFD}.Release|x64.ActiveCfg = Release|x64 + {D9B8FC84-322A-4F9F-BBB9-20915C47DDFD}.Release|x64.Build.0 = Release|x64 EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE diff --git a/installer/PowerToysSetupCustomActions/CustomAction.cpp b/installer/PowerToysSetupCustomActions/CustomAction.cpp index 972f1d2dfa..e23b7102e0 100644 --- a/installer/PowerToysSetupCustomActions/CustomAction.cpp +++ b/installer/PowerToysSetupCustomActions/CustomAction.cpp @@ -2,6 +2,9 @@ #include "resource.h" #include +#include + +#include "../../src/common/logger/logger.h" #include "../../src/common/utils/MsiUtils.h" #include "../../src/common/utils/modulesRegistry.h" #include "../../src/common/updating/installer.h" @@ -26,6 +29,24 @@ const DWORD USERNAME_LEN = UNLEN + 1; // User Name + '\0' static const wchar_t* POWERTOYS_EXE_COMPONENT = L"{A2C66D91-3485-4D00-B04D-91844E6B345B}"; static const wchar_t* POWERTOYS_UPGRADE_CODE = L"{42B84BF7-5FBF-473B-9C8B-049DC16F7708}"; +struct WcaSink : spdlog::sinks::base_sink +{ + virtual void sink_it_(const spdlog::details::log_msg& msg) override + { + WcaLog(LOGMSG_STANDARD, msg.payload.data()); + } + virtual void flush_() override + { + // we don't need to flush wca log manually + } +}; + +void initSystemLogger() +{ + static std::once_flag initLoggerFlag; + std::call_once(initLoggerFlag, []() { Logger::init(std::vector{ std::make_shared() }); }); +} + HRESULT getInstallFolder(MSIHANDLE hInstall, std::wstring& installationDir) { DWORD len = 0; @@ -34,7 +55,7 @@ HRESULT getInstallFolder(MSIHANDLE hInstall, std::wstring& installationDir) len += 1; installationDir.resize(len); HRESULT hr = MsiGetPropertyW(hInstall, L"CustomActionData", installationDir.data(), &len); - if(installationDir.length()) + if (installationDir.length()) { installationDir.resize(installationDir.length() - 1); } @@ -44,24 +65,30 @@ LExit: } UINT __stdcall ApplyModulesRegistryChangeSetsCA(MSIHANDLE hInstall) { + initSystemLogger(); HRESULT hr = S_OK; UINT er = ERROR_SUCCESS; std::wstring installationFolder; + bool failedToApply = false; hr = WcaInitialize(hInstall, "ApplyModulesRegistryChangeSets"); ExitOnFailure(hr, "Failed to initialize"); hr = getInstallFolder(hInstall, installationFolder); ExitOnFailure(hr, "Failed to get installFolder."); + for (const auto& changeSet : getAllModulesChangeSets(installationFolder, false)) { if (!changeSet.apply()) { WcaLog(LOGMSG_STANDARD, "Couldn't apply registry changeSet"); + failedToApply = true; } } - ExitOnFailure(hr, "Failed to extract msix"); - + if (!failedToApply) + { + WcaLog(LOGMSG_STANDARD, "All registry changeSets applied successfully"); + } LExit: er = SUCCEEDED(hr) ? ERROR_SUCCESS : ERROR_INSTALL_FAILURE; return WcaFinalize(er); @@ -69,6 +96,7 @@ LExit: UINT __stdcall UnApplyModulesRegistryChangeSetsCA(MSIHANDLE hInstall) { + initSystemLogger(); HRESULT hr = S_OK; UINT er = ERROR_SUCCESS; std::wstring installationFolder; diff --git a/installer/PowerToysSetupCustomActions/PowerToysSetupCustomActions.vcxproj b/installer/PowerToysSetupCustomActions/PowerToysSetupCustomActions.vcxproj index 07c14882cc..935e5b7994 100644 --- a/installer/PowerToysSetupCustomActions/PowerToysSetupCustomActions.vcxproj +++ b/installer/PowerToysSetupCustomActions/PowerToysSetupCustomActions.vcxproj @@ -30,6 +30,7 @@ v142 + @@ -122,6 +123,11 @@ + + + {d9b8fc84-322a-4f9f-bbb9-20915c47ddfd} + + diff --git a/installer/PowerToysSetupCustomActions/stdafx.h b/installer/PowerToysSetupCustomActions/stdafx.h index 7d80cbe2a1..2174541f00 100644 --- a/installer/PowerToysSetupCustomActions/stdafx.h +++ b/installer/PowerToysSetupCustomActions/stdafx.h @@ -27,6 +27,7 @@ #include #include #include +#include #include #include diff --git a/src/common/logger/logger.cpp b/src/common/logger/logger.cpp index fa6252db14..444c0e787b 100644 --- a/src/common/logger/logger.cpp +++ b/src/common/logger/logger.cpp @@ -3,7 +3,7 @@ #include "pch.h" #include "framework.h" #include "logger.h" -#include +#include #include #include #include @@ -16,26 +16,32 @@ using spdlog::sinks::daily_file_sink_mt; using spdlog::sinks::msvc_sink_mt; using std::make_shared; -std::map logLevelMapping = { - { L"trace", level_enum::trace }, - { L"debug", level_enum::debug }, - { L"info", level_enum::info }, - { L"warn", level_enum::warn }, - { L"err", level_enum::err }, - { L"critical", level_enum::critical }, - { L"off", level_enum::off }, -}; +namespace +{ + const std::unordered_map logLevelMapping = { + { L"trace", level_enum::trace }, + { L"debug", level_enum::debug }, + { L"info", level_enum::info }, + { L"warn", level_enum::warn }, + { L"err", level_enum::err }, + { L"critical", level_enum::critical }, + { L"off", level_enum::off }, + }; +} level_enum getLogLevel(std::wstring_view logSettingsPath) { auto logLevel = get_log_settings(logSettingsPath).logLevel; - level_enum result = logLevelMapping[LogSettings::defaultLogLevel]; - if (logLevelMapping.find(logLevel) != logLevelMapping.end()) + if (auto it = logLevelMapping.find(logLevel); it != logLevelMapping.end()) { - result = logLevelMapping[logLevel]; + return it->second; } - return result; + if (auto it = logLevelMapping.find(LogSettings::defaultLogLevel); it != logLevelMapping.end()) + { + return it->second; + } + return level_enum::trace; } std::shared_ptr Logger::logger = spdlog::null_logger_mt("null"); @@ -89,3 +95,14 @@ void Logger::init(std::string loggerName, std::wstring logFilePath, std::wstring spdlog::flush_every(std::chrono::seconds(3)); logger->info("{} logger is initialized", loggerName); } + +void Logger::init(std::vector sinks) +{ + auto logger = std::make_shared("", begin(sinks), end(sinks)); + if (!logger) + { + return; + } + + Logger::logger = logger; +} diff --git a/src/common/logger/logger.h b/src/common/logger/logger.h index 04f71043ca..fa8ebb1d8c 100644 --- a/src/common/logger/logger.h +++ b/src/common/logger/logger.h @@ -13,6 +13,7 @@ public: Logger() = delete; static void init(std::string loggerName, std::wstring logFilePath, std::wstring_view logSettingsPath); + static void init(std::vector sinks); // log message should not be localized template diff --git a/src/common/logger/logger.vcxproj b/src/common/logger/logger.vcxproj index 7dde0c7c82..09649a7ce7 100644 --- a/src/common/logger/logger.vcxproj +++ b/src/common/logger/logger.vcxproj @@ -52,9 +52,6 @@ {7e1e3f13-2bd6-3f75-a6a7-873a2b55c60f} - - {6955446d-23f7-4023-9bb3-8657f904af99} - diff --git a/src/common/logger/logger_settings.cpp b/src/common/logger/logger_settings.cpp index 7484f42a5d..30fce84272 100644 --- a/src/common/logger/logger_settings.cpp +++ b/src/common/logger/logger_settings.cpp @@ -9,7 +9,7 @@ using namespace winrt::Windows::Data::Json; LogSettings::LogSettings() { - this->logLevel = LogSettings::defaultLogLevel; + logLevel = defaultLogLevel; } std::optional from_file(std::wstring_view file_name) diff --git a/src/common/logger/logger_settings.h b/src/common/logger/logger_settings.h index cada3f537f..1015ef9f57 100644 --- a/src/common/logger/logger_settings.h +++ b/src/common/logger/logger_settings.h @@ -13,6 +13,8 @@ struct LogSettings inline const static std::wstring actionRunnerLogPath = L"RunnerLogs\\action-runner-log.txt"; inline const static std::string updateLoggerName = "update"; inline const static std::wstring updateLogPath = L"UpdateLogs\\update-log.txt"; + inline const static std::string fileExplorerLoggerName = "FileExplorer"; + inline const static std::wstring fileExplorerLogPath = L"Logs\\file-explorer-log.txt"; inline const static std::string launcherLoggerName = "launcher"; inline const static std::wstring launcherLogPath = L"LogsModuleInterface\\launcher-log.txt"; inline const static std::wstring awakeLogPath = L"Logs\\awake-log.txt"; diff --git a/src/common/utils/registry.h b/src/common/utils/registry.h index d0ebcb1879..43ea144ba5 100644 --- a/src/common/utils/registry.h +++ b/src/common/utils/registry.h @@ -9,6 +9,8 @@ #include #include +#include "../logger/logger.h" +#include "../utils/winapi_error.h" #include "../version/version.h" namespace registry @@ -35,7 +37,28 @@ namespace registry template overloaded(Ts...) -> overloaded; + + inline const wchar_t* getScopeName(HKEY scope) + { + if (scope == HKEY_LOCAL_MACHINE) + { + return L"HKLM"; + } + else if (scope == HKEY_CURRENT_USER) + { + return L"HKCU"; + } + else if (scope == HKEY_CLASSES_ROOT) + { + return L"HKCR"; + } + else + { + return L"HK??"; + } + } } + struct ValueChange { using value_t = std::variant; @@ -51,11 +74,28 @@ namespace registry { } + std::wstring toString() const + { + using namespace detail; + + std::wstring value_str; + std::visit(overloaded{ [&](DWORD value) { + std::wostringstream oss; + oss << value; + value_str = oss.str(); + }, + [&](const std::wstring& value) { value_str = value; } }, + value); + + return fmt::format(L"{}\\{}\\{}:{}", detail::getScopeName(scope), path, name ? *name : L"Default", value_str); + } + bool isApplied() const { HKEY key{}; - if (RegOpenKeyExW(scope, path.c_str(), 0, KEY_READ, &key) != ERROR_SUCCESS) + if (auto res = RegOpenKeyExW(scope, path.c_str(), 0, KEY_READ, &key); res != ERROR_SUCCESS) { + Logger::info(L"isApplied of {}: RegOpenKeyExW failed: {}", toString(), get_last_error_or_default(res)); return false; } detail::on_exit closeKey{ [key] { RegCloseKey(key); } }; @@ -65,13 +105,15 @@ namespace registry DWORD retrievedType{}; wchar_t buffer[VALUE_BUFFER_SIZE]; DWORD valueSize = sizeof(buffer); - if (RegQueryValueExW(key, - name.has_value() ? name->c_str() : nullptr, - 0, - &retrievedType, - reinterpret_cast(&buffer), - &valueSize) != ERROR_SUCCESS) + if (auto res = RegQueryValueExW(key, + name.has_value() ? name->c_str() : nullptr, + 0, + &retrievedType, + reinterpret_cast(&buffer), + &valueSize); + res != ERROR_SUCCESS) { + Logger::info(L"isApplied of {}: RegQueryValueExW failed: {}", toString(), get_last_error_or_default(res)); return false; } @@ -94,9 +136,10 @@ namespace registry { HKEY key{}; - if (RegCreateKeyExW(scope, path.c_str(), 0, nullptr, REG_OPTION_NON_VOLATILE, KEY_WRITE, nullptr, &key, nullptr) != - ERROR_SUCCESS) + if (auto res = RegCreateKeyExW(scope, path.c_str(), 0, nullptr, REG_OPTION_NON_VOLATILE, KEY_WRITE, nullptr, &key, nullptr); res != + ERROR_SUCCESS) { + Logger::error(L"apply of {}: RegCreateKeyExW failed: {}", toString(), get_last_error_or_default(res)); return false; } detail::on_exit closeKey{ [key] { RegCloseKey(key); } }; @@ -106,26 +149,35 @@ namespace registry DWORD valueType; valueToBuffer(value, buffer, valueSize, valueType); - return RegSetValueExW(key, - name.has_value() ? name->c_str() : nullptr, - 0, - valueType, - reinterpret_cast(buffer), - valueSize) == ERROR_SUCCESS; + if (auto res = RegSetValueExW(key, + name.has_value() ? name->c_str() : nullptr, + 0, + valueType, + reinterpret_cast(buffer), + valueSize); + res != ERROR_SUCCESS) + { + Logger::error(L"apply of {}: RegSetValueExW failed: {}", toString(), get_last_error_or_default(res)); + return false; + } + + return true; } bool unApply() const { HKEY key{}; - if (RegOpenKeyExW(scope, path.c_str(), 0, KEY_ALL_ACCESS, &key) != ERROR_SUCCESS) + if (auto res = RegOpenKeyExW(scope, path.c_str(), 0, KEY_ALL_ACCESS, &key); res != ERROR_SUCCESS) { + Logger::error(L"unApply of {}: RegOpenKeyExW failed: {}", toString(), get_last_error_or_default(res)); return false; } detail::on_exit closeKey{ [key] { RegCloseKey(key); } }; // delete the value itself - if (RegDeleteKeyValueW(scope, path.c_str(), name.has_value() ? name->c_str() : nullptr) != ERROR_SUCCESS) + if (auto res = RegDeleteKeyValueW(scope, path.c_str(), name.has_value() ? name->c_str() : nullptr); res != ERROR_SUCCESS) { + Logger::error(L"unApply of {}: RegDeleteKeyValueW failed: {}", toString(), get_last_error_or_default(res)); return false; } diff --git a/src/modules/ShortcutGuide/ShortcutGuide/ShortcutGuide.vcxproj b/src/modules/ShortcutGuide/ShortcutGuide/ShortcutGuide.vcxproj index f9f67fcd76..d85a26b05b 100644 --- a/src/modules/ShortcutGuide/ShortcutGuide/ShortcutGuide.vcxproj +++ b/src/modules/ShortcutGuide/ShortcutGuide/ShortcutGuide.vcxproj @@ -157,6 +157,9 @@ {d9b8fc84-322a-4f9f-bbb9-20915c47ddfd} + + {6955446d-23f7-4023-9bb3-8657f904af99} + {98537082-0fdb-40de-abd8-0dc5a4269bab} diff --git a/src/modules/ShortcutGuide/ShortcutGuideModuleInterface/ShortcutGuideModuleInterface.vcxproj b/src/modules/ShortcutGuide/ShortcutGuideModuleInterface/ShortcutGuideModuleInterface.vcxproj index 22aa591f1f..a9ad4129b9 100644 --- a/src/modules/ShortcutGuide/ShortcutGuideModuleInterface/ShortcutGuideModuleInterface.vcxproj +++ b/src/modules/ShortcutGuide/ShortcutGuideModuleInterface/ShortcutGuideModuleInterface.vcxproj @@ -80,6 +80,9 @@ {d9b8fc84-322a-4f9f-bbb9-20915c47ddfd} + + {6955446d-23f7-4023-9bb3-8657f904af99} + diff --git a/src/modules/fancyzones/FancyZones/FancyZones.vcxproj b/src/modules/fancyzones/FancyZones/FancyZones.vcxproj index 40b872f210..fb8f7d7a6a 100644 --- a/src/modules/fancyzones/FancyZones/FancyZones.vcxproj +++ b/src/modules/fancyzones/FancyZones/FancyZones.vcxproj @@ -166,6 +166,9 @@ {d9b8fc84-322a-4f9f-bbb9-20915c47ddfd} + + {6955446d-23f7-4023-9bb3-8657f904af99} + {f9c68edf-ac74-4b77-9af1-005d9c9f6a99} diff --git a/src/modules/fancyzones/FancyZonesModuleInterface/FancyZonesModuleInterface.vcxproj b/src/modules/fancyzones/FancyZonesModuleInterface/FancyZonesModuleInterface.vcxproj index a8cf7552b7..6b79113e63 100644 --- a/src/modules/fancyzones/FancyZonesModuleInterface/FancyZonesModuleInterface.vcxproj +++ b/src/modules/fancyzones/FancyZonesModuleInterface/FancyZonesModuleInterface.vcxproj @@ -56,6 +56,9 @@ {d9b8fc84-322a-4f9f-bbb9-20915c47ddfd} + + {6955446d-23f7-4023-9bb3-8657f904af99} + {f9c68edf-ac74-4b77-9af1-005d9c9f6a99} diff --git a/src/modules/fancyzones/FancyZonesTests/UnitTests/UnitTests.vcxproj b/src/modules/fancyzones/FancyZonesTests/UnitTests/UnitTests.vcxproj index 4f5e34ca86..2ada2e6a78 100644 --- a/src/modules/fancyzones/FancyZonesTests/UnitTests/UnitTests.vcxproj +++ b/src/modules/fancyzones/FancyZonesTests/UnitTests/UnitTests.vcxproj @@ -62,6 +62,9 @@ {caba8dfb-823b-4bf2-93ac-3f31984150d9} + + {6955446d-23f7-4023-9bb3-8657f904af99} + {f9c68edf-ac74-4b77-9af1-005d9c9f6a99} diff --git a/src/modules/keyboardmanager/common/KeyboardManagerCommon.vcxproj b/src/modules/keyboardmanager/common/KeyboardManagerCommon.vcxproj index 62cec9b6b0..5820b4a7c1 100644 --- a/src/modules/keyboardmanager/common/KeyboardManagerCommon.vcxproj +++ b/src/modules/keyboardmanager/common/KeyboardManagerCommon.vcxproj @@ -71,6 +71,9 @@ {d9b8fc84-322a-4f9f-bbb9-20915c47ddfd} + + {6955446d-23f7-4023-9bb3-8657f904af99} + diff --git a/src/modules/previewpane/powerpreview/powerpreview.cpp b/src/modules/previewpane/powerpreview/powerpreview.cpp index e7c8768ad5..de6b2433d2 100644 --- a/src/modules/previewpane/powerpreview/powerpreview.cpp +++ b/src/modules/previewpane/powerpreview/powerpreview.cpp @@ -11,12 +11,20 @@ #include #include +#include + // Constructor PowerPreviewModule::PowerPreviewModule() : m_moduleName(GET_RESOURCE_STRING(IDS_MODULE_NAME)), app_key(powerpreviewConstants::ModuleKey) { const std::wstring installationDir = get_module_folderpath(); + + std::filesystem::path logFilePath(PTSettingsHelper::get_module_save_folder_location(this->app_key)); + logFilePath.append(LogSettings::fileExplorerLogPath); + Logger::init(LogSettings::fileExplorerLoggerName, logFilePath.wstring(), PTSettingsHelper::get_log_settings_file_location()); + + Logger::info("Initializing PowerPreviewModule"); const bool installPerUser = false; m_fileExplorerModules.push_back({ .settingName = L"svg-previewer-toggle-setting", .settingDescription = GET_RESOURCE_STRING(IDS_PREVPANE_SVG_SETTINGS_DESCRIPTION), @@ -133,7 +141,10 @@ void PowerPreviewModule::disable() { for (auto& fileExplorerModule : m_fileExplorerModules) { - fileExplorerModule.registryChanges.unApply(); + if (!fileExplorerModule.registryChanges.unApply()) + { + Logger::error(L"Couldn't disable file explorer module {} during module disable() call", fileExplorerModule.settingName); + } } } else @@ -207,6 +218,7 @@ void PowerPreviewModule::apply_settings(const PowerToysSettings::PowerToyValues& } else { + Logger::error(L"Couldn't {} file explorer module {} during apply_settings", *toggle ? L"enable " : L"disable", fileExplorerModule.settingName); Trace::PowerPreviewSettingsUpdateFailed(fileExplorerModule.settingName.c_str(), !*toggle, *toggle, true); } }