diff --git a/.github/actions/spelling/expect.txt b/.github/actions/spelling/expect.txt index 5be26a12f9..53175f0afa 100644 --- a/.github/actions/spelling/expect.txt +++ b/.github/actions/spelling/expect.txt @@ -529,6 +529,7 @@ sfsclient SHCONTF shellapi SHGDN +shortguid SHOWNORMAL sid Sideload diff --git a/doc/ReleaseNotes.md b/doc/ReleaseNotes.md index 142236ace8..af1169c07e 100644 --- a/doc/ReleaseNotes.md +++ b/doc/ReleaseNotes.md @@ -52,6 +52,17 @@ The PowerShell module now automatically uses `GH_TOKEN` or `GITHUB_TOKEN` enviro - `winget list` (and similar table commands) no longer truncates output when stdout is redirected to a file or variable — column widths are now computed from the full result set. - Spinner and progress bar output are suppressed when no console is attached, keeping redirected output clean. +### Log file naming strategy + +Added a user setting (`logging.fileNameStrategy`) for controlling the default naming strategy for installer log files. Supported values are `manifest` (default), `timestamp`, `guid`, and `shortguid`. Only applies to logs generated by installers if the installer itself supports the logging switch / parameter. + +| Setting | Description | +| --- | --- | +| manifest | Uses the name of the manifest and a timestamp. Has the same behavior as WinGet 1.28 | +| timestamp | The log name is just a timestamp | +| guid | The log name is a GUID | +| shortguid | The log name is the first 8 characters of a GUID | + ## Bug Fixes * `winget export` now works when the destination path is a hidden file diff --git a/doc/Settings.md b/doc/Settings.md index 4915f81afa..e8ea89b7af 100644 --- a/doc/Settings.md +++ b/doc/Settings.md @@ -311,6 +311,16 @@ In addition, there are special values that cover multiple channels. `default` i }, ``` +### fileNameStrategy + +Sets the default strategy for naming log files for installers that support it. `manifest` is the default and uses the manifest name. `timestamp` uses the date and time. `guid` uses a generated GUID. `shortguid` uses the first 8 characters of a generated GUID. Invalid values will revert to `manifest`. + +```json + "logging": { + "fileNameStrategy": "manifest" | "timestamp" | "guid" | "shortguid" + }, +``` + ### file The `file` settings control the log files generated by winget during operation. These settings apply to the automatic cleanup that happens whenever a Windows Package Manager process is run. diff --git a/schemas/JSON/settings/settings.schema.0.2.json b/schemas/JSON/settings/settings.schema.0.2.json index 25b04e3d1f..534630edd6 100644 --- a/schemas/JSON/settings/settings.schema.0.2.json +++ b/schemas/JSON/settings/settings.schema.0.2.json @@ -81,6 +81,16 @@ "minItems": 0, "maxItems": 20 }, + "fileNameStrategy": { + "description": "Controls the default naming of log files for installers that support it", + "type": "string", + "enum": [ + "manifest", + "timestamp", + "guid", + "shortguid" + ] + }, "file": { "description": "The file settings control the log files generated by winget during operation.", "type": "object", diff --git a/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp b/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp index 37405faf51..66871e39fa 100644 --- a/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp +++ b/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp @@ -171,11 +171,41 @@ namespace AppInstaller::CLI::Workflow else { const auto& manifest = context.Get(); - + const Logging::LogNameStrategy logNameStrategy = Settings::User().Get(); auto path = Runtime::GetPathTo(Runtime::PathName::DefaultLogLocation); - path /= Utility::ConvertToUTF16(manifest.Id + '.' + manifest.Version); - path += '-'; - path += Utility::GetCurrentTimeForFilename(true); + + switch (logNameStrategy) + { + case Logging::LogNameStrategy::Manifest: + // Use manifest ID and version for log file name + // Results in \.-.log + path /= Utility::ConvertToUTF16(manifest.Id + '.' + manifest.Version); + path += '-'; + path += Utility::GetCurrentTimeForFilename(true); + break; + case Logging::LogNameStrategy::Timestamp: + // Use only timestamp for log file name + // Results in \.log + path /= Utility::GetCurrentTimeForFilename(true); + break; + case Logging::LogNameStrategy::Guid: + // Use a GUID for log file name + // Results in \.log + path /= Utility::CreateNewGuidNameWString(); + break; + case Logging::LogNameStrategy::ShortGuid: + // Use the first 8 characters of a GUID for log file name + // Results in \.log + path /= Utility::CreateNewGuidNameWString().substr(0, 8); + break; + default: + // This should never happen due to validation when reading settings, but handle it just in case. + AICLI_LOG(CLI, Error, << "Unknown log naming strategy."); + THROW_HR(E_UNEXPECTED); + break; + } + + // Add the extension to the log file regardless of naming strategy path += Logging::FileLogger::DefaultExt(); logPath = path.u8string(); diff --git a/src/AppInstallerCLITests/InstallFlow.cpp b/src/AppInstallerCLITests/InstallFlow.cpp index c87be7b62c..a192d0a4f4 100644 --- a/src/AppInstallerCLITests/InstallFlow.cpp +++ b/src/AppInstallerCLITests/InstallFlow.cpp @@ -936,6 +936,100 @@ TEST_CASE("ShellExecuteHandlerInstallerArgs", "[InstallFlow][workflow]") } } +TEST_CASE("ShellExecuteHandlerInstallerArgs_LogNamingStrategy", "[InstallFlow][workflow]") +{ + SECTION("Manifest") + { + TestUserSettings testSettings; + testSettings.Set(LogNameStrategy::Manifest); + + std::ostringstream installOutput; + TestContext context{ installOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); + auto manifest = YamlParser::CreateFromPath(TestDataFile("InstallerArgTest_Inno_WithSwitches.yaml")); + context.Add(manifest); + context.Add(manifest.Installers.at(0)); + context << GetInstallerArgs; + + std::string installerArgs = context.Get(); + REQUIRE(installerArgs.find(manifest.Id) != std::string::npos); + REQUIRE(installerArgs.find(manifest.Version) != std::string::npos); + + REQUIRE(context.Contains(Data::LogPath)); + auto logPath = context.Get(); + REQUIRE(logPath.filename().u8string().find(manifest.Id) != std::string::npos); + } + + SECTION("Timestamp") + { + TestUserSettings testSettings; + testSettings.Set(LogNameStrategy::Timestamp); + + std::ostringstream installOutput; + TestContext context{ installOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); + auto manifest = YamlParser::CreateFromPath(TestDataFile("InstallerArgTest_Inno_WithSwitches.yaml")); + context.Add(manifest); + context.Add(manifest.Installers.at(0)); + context << GetInstallerArgs; + + std::string installerArgs = context.Get(); + REQUIRE(installerArgs.find(manifest.Id) == std::string::npos); + REQUIRE(installerArgs.find(manifest.Version) == std::string::npos); + + REQUIRE(context.Contains(Data::LogPath)); + auto logPath = context.Get(); + REQUIRE(logPath.extension().u8string() == std::string{ FileLogger::DefaultExt() }); + REQUIRE(logPath.stem().u8string().find(manifest.Id) == std::string::npos); + } + + SECTION("Guid") + { + TestUserSettings testSettings; + testSettings.Set(LogNameStrategy::Guid); + + std::ostringstream installOutput; + TestContext context{ installOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); + auto manifest = YamlParser::CreateFromPath(TestDataFile("InstallerArgTest_Inno_WithSwitches.yaml")); + context.Add(manifest); + context.Add(manifest.Installers.at(0)); + context << GetInstallerArgs; + + std::string installerArgs = context.Get(); + REQUIRE(installerArgs.find(manifest.Id) == std::string::npos); + + REQUIRE(context.Contains(Data::LogPath)); + auto logPath = context.Get(); + REQUIRE(logPath.extension().u8string() == std::string{ FileLogger::DefaultExt() }); + // A GUID string is 36 characters: xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx + REQUIRE(logPath.stem().u8string().size() == 36); + } + + SECTION("ShortGuid") + { + TestUserSettings testSettings; + testSettings.Set(LogNameStrategy::ShortGuid); + + std::ostringstream installOutput; + TestContext context{ installOutput, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); + auto manifest = YamlParser::CreateFromPath(TestDataFile("InstallerArgTest_Inno_WithSwitches.yaml")); + context.Add(manifest); + context.Add(manifest.Installers.at(0)); + context << GetInstallerArgs; + + std::string installerArgs = context.Get(); + REQUIRE(installerArgs.find(manifest.Id) == std::string::npos); + + REQUIRE(context.Contains(Data::LogPath)); + auto logPath = context.Get(); + REQUIRE(logPath.extension().u8string() == std::string{ FileLogger::DefaultExt() }); + // A short GUID is the first 8 characters of a full GUID + REQUIRE(logPath.stem().u8string().size() == 8); + } +} + TEST_CASE("InstallFlow_SearchAndInstall", "[InstallFlow][workflow]") { TestCommon::TempFile installResultPath("TestExeInstalled.txt"); diff --git a/src/AppInstallerCLITests/UserSettings.cpp b/src/AppInstallerCLITests/UserSettings.cpp index 3d0820f8d5..72ad588591 100644 --- a/src/AppInstallerCLITests/UserSettings.cpp +++ b/src/AppInstallerCLITests/UserSettings.cpp @@ -318,6 +318,72 @@ TEST_CASE("SettingLoggingLevelPreference", "[settings]") REQUIRE(userSettingTest.Get() == Level::Info); REQUIRE(userSettingTest.GetWarnings().size() == 1); } +} + +TEST_CASE("SettingLoggingFileNameStrategy", "[settings]") { + auto again = DeleteUserSettingsFiles(); + + SECTION("Default value") + { + UserSettingsTest userSettingTest; + + REQUIRE(userSettingTest.Get() == LogNameStrategy::Manifest); + REQUIRE(userSettingTest.GetWarnings().size() == 0); + } + SECTION("Manifest") + { + std::string_view json = R"({ "logging": { "fileNameStrategy": "manifest" } })"; + SetSetting(Stream::PrimaryUserSettings, json); + UserSettingsTest userSettingTest; + + REQUIRE(userSettingTest.Get() == LogNameStrategy::Manifest); + REQUIRE(userSettingTest.GetWarnings().size() == 0); + } + SECTION("Timestamp") + { + std::string_view json = R"({ "logging": { "fileNameStrategy": "timestamp" } })"; + SetSetting(Stream::PrimaryUserSettings, json); + UserSettingsTest userSettingTest; + + REQUIRE(userSettingTest.Get() == LogNameStrategy::Timestamp); + REQUIRE(userSettingTest.GetWarnings().size() == 0); + } + SECTION("Guid") + { + std::string_view json = R"({ "logging": { "fileNameStrategy": "guid" } })"; + SetSetting(Stream::PrimaryUserSettings, json); + UserSettingsTest userSettingTest; + + REQUIRE(userSettingTest.Get() == LogNameStrategy::Guid); + REQUIRE(userSettingTest.GetWarnings().size() == 0); + } + SECTION("Short Guid") + { + std::string_view json = R"({ "logging": { "fileNameStrategy": "shortguid" } })"; + SetSetting(Stream::PrimaryUserSettings, json); + UserSettingsTest userSettingTest; + + REQUIRE(userSettingTest.Get() == LogNameStrategy::ShortGuid); + REQUIRE(userSettingTest.GetWarnings().size() == 0); + } + SECTION("Bad value") + { + std::string_view json = R"({ "logging": { "fileNameStrategy": "fake" } })"; + SetSetting(Stream::PrimaryUserSettings, json); + UserSettingsTest userSettingTest; + + REQUIRE(userSettingTest.Get() == LogNameStrategy::Manifest); + REQUIRE(userSettingTest.GetWarnings().size() == 1); + } + SECTION("Bad value type") + { + std::string_view json = R"({ "logging": { "fileNameStrategy": 5 } })"; + SetSetting(Stream::PrimaryUserSettings, json); + UserSettingsTest userSettingTest; + + REQUIRE(userSettingTest.Get() == LogNameStrategy::Manifest); + REQUIRE(userSettingTest.GetWarnings().size() == 1); + } } TEST_CASE("SettingAutoUpdateIntervalInMinutes", "[settings]") diff --git a/src/AppInstallerCommonCore/Public/winget/UserSettings.h b/src/AppInstallerCommonCore/Public/winget/UserSettings.h index 95fbe73549..2986baafb1 100644 --- a/src/AppInstallerCommonCore/Public/winget/UserSettings.h +++ b/src/AppInstallerCommonCore/Public/winget/UserSettings.h @@ -102,6 +102,7 @@ namespace AppInstaller::Settings // Logging LoggingLevelPreference, LoggingChannelPreference, + LoggingFileNameStrategy, LoggingFileAgeLimitInDays, LoggingFileTotalSizeLimitInMB, LoggingFileIndividualSizeLimitInMB, @@ -202,6 +203,7 @@ namespace AppInstaller::Settings // Logging SETTINGMAPPING_SPECIALIZATION(Setting::LoggingLevelPreference, std::string, Logging::Level, Logging::Level::Info, ".logging.level"sv); SETTINGMAPPING_SPECIALIZATION(Setting::LoggingChannelPreference, std::vector, Logging::Channel, Logging::Channel::Defaults, ".logging.channels"sv); + SETTINGMAPPING_SPECIALIZATION(Setting::LoggingFileNameStrategy, std::string, Logging::LogNameStrategy, Logging::LogNameStrategy::Manifest, ".logging.fileNameStrategy"sv); SETTINGMAPPING_SPECIALIZATION(Setting::LoggingFileAgeLimitInDays, uint32_t, std::chrono::hours, (7 * 24h), ".logging.file.ageLimitInDays"sv); SETTINGMAPPING_SPECIALIZATION(Setting::LoggingFileTotalSizeLimitInMB, uint32_t, uint32_t, 128, ".logging.file.totalSizeLimitInMB"sv); SETTINGMAPPING_SPECIALIZATION(Setting::LoggingFileIndividualSizeLimitInMB, uint32_t, uint32_t, 16, ".logging.file.individualSizeLimitInMB"sv); diff --git a/src/AppInstallerCommonCore/UserSettings.cpp b/src/AppInstallerCommonCore/UserSettings.cpp index db276351ff..f350d8562b 100644 --- a/src/AppInstallerCommonCore/UserSettings.cpp +++ b/src/AppInstallerCommonCore/UserSettings.cpp @@ -478,6 +478,33 @@ namespace AppInstaller::Settings return result; } + WINGET_VALIDATE_SIGNATURE(LoggingFileNameStrategy) + { + // logging name strategy possible values + static constexpr std::string_view s_strategy_manifest = "manifest"; + static constexpr std::string_view s_strategy_timestamp = "timestamp"; + static constexpr std::string_view s_strategy_guid = "guid"; + static constexpr std::string_view s_strategy_shortguid = "shortguid"; + + if (Utility::CaseInsensitiveEquals(value, s_strategy_manifest)) + { + return LogNameStrategy::Manifest; + } + else if (Utility::CaseInsensitiveEquals(value, s_strategy_timestamp)) + { + return LogNameStrategy::Timestamp; + } + else if (Utility::CaseInsensitiveEquals(value, s_strategy_guid)) + { + return LogNameStrategy::Guid; + } + else if (Utility::CaseInsensitiveEquals(value, s_strategy_shortguid)) + { + return LogNameStrategy::ShortGuid; + } + return {}; + } + WINGET_VALIDATE_SIGNATURE(LoggingFileAgeLimitInDays) { return value * 24h; diff --git a/src/AppInstallerSharedLib/Public/AppInstallerLogging.h b/src/AppInstallerSharedLib/Public/AppInstallerLogging.h index 90e8511990..44d9b84958 100644 --- a/src/AppInstallerSharedLib/Public/AppInstallerLogging.h +++ b/src/AppInstallerSharedLib/Public/AppInstallerLogging.h @@ -83,6 +83,18 @@ namespace AppInstaller::Logging Crit, }; + enum class LogNameStrategy + { + // The log name is the name of the manifest with a timestamp + Manifest, + // The log name is just a timestamp + Timestamp, + // The log name is a GUID + Guid, + // The log name is the first 8 characters of a GUID + ShortGuid, + }; + // Indicates a location of significance in the logging stream. enum class Tag {