Conversation
There was a problem hiding this comment.
Pull Request Overview
Adds detailed logging throughout the MSI preparation and execution steps to help diagnose failures and track progress.
- Introduces a
Loggersingleton that writes timestamped messages to a temp-directory log file. - Wraps key operations (
computeMD5, resource extraction, directory creation, file mapping, and execution) withLogcalls.
Comments suppressed due to low confidence (1)
installer/installer_setup/main.cpp:29
- The code uses
std::filesystem::pathbut does not#include <filesystem>. Add#include <filesystem>near the top to avoid build errors.
#include <ctime>
installer/installer_setup/main.cpp
Outdated
| } | ||
| const auto now = std::chrono::system_clock::now(); | ||
| const auto now_c = std::chrono::system_clock::to_time_t(now); | ||
| ofs << std::ctime(&now_c) << " " << message << std::endl; |
There was a problem hiding this comment.
Using std::ctime embeds a trailing newline and relies on a static buffer, which is not thread-safe. Consider using std::localtime_s (or std::localtime_r) with std::put_time to format the timestamp without an extra newline and in a threadsafe manner.
| ofs << std::ctime(&now_c) << " " << message << std::endl; | |
| std::tm localTime; | |
| localtime_s(&localTime, &now_c); | |
| std::ostringstream timestamp; | |
| timestamp << std::put_time(&localTime, "%Y-%m-%d %H:%M:%S"); | |
| ofs << timestamp.str() << " " << message << std::endl; |
| } | ||
| logPath /= "boinc_installer.log"; |
There was a problem hiding this comment.
If GetTempPath fails, logPath remains empty and the log will open in the working directory. Add handling for a failed GetTempPath (e.g., fallback or an early exit) to avoid writing logs to an unintended location.
| } | |
| logPath /= "boinc_installer.log"; | |
| } else { | |
| // Fallback to a path relative to the executable | |
| logPath = std::filesystem::current_path(); | |
| logPath /= "boinc_installer.log"; | |
| } |
|
|
||
| void Log(const std::string& message) { | ||
| Logger::Instance().Log(message); | ||
| } |
There was a problem hiding this comment.
[nitpick] The free function Log may collide with other symbols or macros. Consider placing it in a namespace (e.g., installer::Log) or renaming it to avoid global scope pollution.
| void Log(const std::string& message) { | |
| Logger::Instance().Log(message); | |
| } | |
| namespace installer { | |
| void Log(const std::string& message) { | |
| Logger::Instance().Log(message); | |
| } | |
| } |
…r execution Signed-off-by: Vitalii Koshura <lestat.de.lionkur@gmail.com>
018967e to
7eba605
Compare
…nstaller [windows][installer] add additional logging during MSI preparation for execution
No description provided.