Skip to content

[windows][installer] add additional logging during MSI preparation for execution #6402

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 25, 2025

Conversation

AenBleidd
Copy link
Member

No description provided.

@github-project-automation github-project-automation bot moved this to In progress in Client/Manager Jun 25, 2025
@AenBleidd AenBleidd added this to the Client/Manager 8.2.5 milestone Jun 25, 2025
@AenBleidd AenBleidd marked this pull request as ready for review June 25, 2025 00:52
@Copilot Copilot AI review requested due to automatic review settings June 25, 2025 00:52
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Adds detailed logging throughout the MSI preparation and execution steps to help diagnose failures and track progress.

  • Introduces a Logger singleton that writes timestamped messages to a temp-directory log file.
  • Wraps key operations (computeMD5, resource extraction, directory creation, file mapping, and execution) with Log calls.
Comments suppressed due to low confidence (1)

installer/installer_setup/main.cpp:29

  • The code uses std::filesystem::path but does not #include <filesystem>. Add #include <filesystem> near the top to avoid build errors.
#include <ctime>

}
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;
Copy link
Preview

Copilot AI Jun 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
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;

Copilot uses AI. Check for mistakes.

Comment on lines +60 to +63
}
logPath /= "boinc_installer.log";
Copy link
Preview

Copilot AI Jun 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
}
logPath /= "boinc_installer.log";
} else {
// Fallback to a path relative to the executable
logPath = std::filesystem::current_path();
logPath /= "boinc_installer.log";
}

Copilot uses AI. Check for mistakes.

Comment on lines +74 to +79

void Log(const std::string& message) {
Logger::Instance().Log(message);
}
Copy link
Preview

Copilot AI Jun 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Suggested change
void Log(const std::string& message) {
Logger::Instance().Log(message);
}
namespace installer {
void Log(const std::string& message) {
Logger::Instance().Log(message);
}
}

Copilot uses AI. Check for mistakes.

…r execution

Signed-off-by: Vitalii Koshura <lestat.de.lionkur@gmail.com>
@AenBleidd AenBleidd force-pushed the vko_add_more_logging_to_windows_installer branch from 018967e to 7eba605 Compare June 25, 2025 01:07
@AenBleidd AenBleidd merged commit 96dfe3f into master Jun 25, 2025
175 checks passed
@AenBleidd AenBleidd deleted the vko_add_more_logging_to_windows_installer branch June 25, 2025 01:28
@github-project-automation github-project-automation bot moved this from In progress to Merged in Client/Manager Jun 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Merged
Development

Successfully merging this pull request may close these issues.

1 participant