diff --git a/.github/actions/spelling/expect.txt b/.github/actions/spelling/expect.txt index 9f28a6b5b3..de71978068 100644 --- a/.github/actions/spelling/expect.txt +++ b/.github/actions/spelling/expect.txt @@ -504,6 +504,7 @@ sfsclient SHCONTF shellapi SHGDN +shortguid SHOWNORMAL sid Sideload diff --git a/doc/ReleaseNotes.md b/doc/ReleaseNotes.md index 8ec1e210c6..40270c62b0 100644 --- a/doc/ReleaseNotes.md +++ b/doc/ReleaseNotes.md @@ -1,3 +1,2 @@ ## New in v1.28 - - +* Added a user setting for controlling the default naming strategy for installer logs diff --git a/doc/Settings.md b/doc/Settings.md index 9593346539..b5d0e2b247 100644 --- a/doc/Settings.md +++ b/doc/Settings.md @@ -286,6 +286,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" + }, +``` + ## Network The `network` settings influence how winget uses the network to retrieve packages and metadata. diff --git a/schemas/JSON/settings/settings.schema.0.2.json b/schemas/JSON/settings/settings.schema.0.2.json index bb5f3fce28..85d947a01f 100644 --- a/schemas/JSON/settings/settings.schema.0.2.json +++ b/schemas/JSON/settings/settings.schema.0.2.json @@ -46,42 +46,52 @@ "Logging": { "description": "Logging settings", "type": "object", - "properties": { - "level": { - "description": "Preferred logging level", - "type": "string", - "enum": [ - "verbose", - "info", - "warning", - "error", - "critical" - ] - }, - "channels": { - "description": "The logging channels to enable", - "type": "array", - "items": { - "uniqueItems": true, - "type": "string", - "enum": [ - "fail", - "cli", - "sql", - "repo", - "yaml", - "core", - "test", - "config", - "default", - "all" - ], - "maxLength": 20 - }, - "minItems": 0, - "maxItems": 20 + "properties": { + "level": { + "description": "Preferred logging level", + "type": "string", + "enum": [ + "verbose", + "info", + "warning", + "error", + "critical" + ] + }, + "channels": { + "description": "The logging channels to enable", + "type": "array", + "items": { + "uniqueItems": true, + "type": "string", + "enum": [ + "fail", + "cli", + "sql", + "repo", + "yaml", + "core", + "test", + "config", + "default", + "all" + ], + "maxLength": 20 + }, + "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" + ] + } } - } }, "InstallPrefReq": { "description": "Shared schema for preferences and requirements", diff --git a/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp b/src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp index 726a547f85..7682989c21 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/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 b06bc57170..3af3c3f0d0 100644 --- a/src/AppInstallerCommonCore/Public/winget/UserSettings.h +++ b/src/AppInstallerCommonCore/Public/winget/UserSettings.h @@ -101,6 +101,7 @@ namespace AppInstaller::Settings // Logging LoggingLevelPreference, LoggingChannelPreference, + LoggingFileNameStrategy, // Uninstall behavior UninstallPurgePortablePackage, // Download behavior @@ -196,6 +197,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); // Interactivity SETTINGMAPPING_SPECIALIZATION(Setting::InteractivityDisable, bool, bool, false, ".interactivity.disable"sv); diff --git a/src/AppInstallerCommonCore/UserSettings.cpp b/src/AppInstallerCommonCore/UserSettings.cpp index 5d7b000949..9bd48a8f73 100644 --- a/src/AppInstallerCommonCore/UserSettings.cpp +++ b/src/AppInstallerCommonCore/UserSettings.cpp @@ -473,6 +473,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 {}; + } } #ifndef AICLI_DISABLE_TEST_HOOKS diff --git a/src/AppInstallerSharedLib/Public/AppInstallerLogging.h b/src/AppInstallerSharedLib/Public/AppInstallerLogging.h index 81bd5a5e6e..511bb09f6e 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, + }; + // The interface that a log target must implement. struct ILogger {