Add setting for installer log file names

Author: Trenly

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.

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",

src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp

@@ -171,11 +171,41 @@ namespace AppInstaller::CLI::Workflow
             else
             {
                 const auto& manifest = context.Get<Execution::Data::Manifest>();
-
+                const Logging::LogNameStrategy logNameStrategy = Settings::User().Get<Settings::Setting::LoggingFileNameStrategy>();
                 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 <DefaultLogLocation>\<ManifestId>.<ManifestVersion>-<Timestamp>.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 <DefaultLogLocation>\<Timestamp>.log
+                        path /= Utility::GetCurrentTimeForFilename(true);
+                        break;
+                    case Logging::LogNameStrategy::Guid:
+                        // Use a GUID for log file name
+                        // Results in <DefaultLogLocation>\<GUID>.log
+                        path /= Utility::CreateNewGuidNameWString();
+                        break;
+                    case Logging::LogNameStrategy::ShortGuid:
+                        // Use the first 8 characters of a GUID for log file name
+                        // Results in <DefaultLogLocation>\<First 8 characters of GUID>.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();

src/AppInstallerCLITests/UserSettings.cpp

@@ -318,6 +318,72 @@ TEST_CASE("SettingLoggingLevelPreference", "[settings]")
         REQUIRE(userSettingTest.Get<Setting::LoggingLevelPreference>() == Level::Info);
         REQUIRE(userSettingTest.GetWarnings().size() == 1);
     }
+}
+
+TEST_CASE("SettingLoggingFileNameStrategy", "[settings]") {
+    auto again = DeleteUserSettingsFiles();
+
+    SECTION("Default value")
+    {
+        UserSettingsTest userSettingTest;
+
+        REQUIRE(userSettingTest.Get<Setting::LoggingFileNameStrategy>() == 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<Setting::LoggingFileNameStrategy>() == 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<Setting::LoggingFileNameStrategy>() == 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<Setting::LoggingFileNameStrategy>() == 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<Setting::LoggingFileNameStrategy>() == 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<Setting::LoggingFileNameStrategy>() == 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<Setting::LoggingFileNameStrategy>() == LogNameStrategy::Manifest);
+        REQUIRE(userSettingTest.GetWarnings().size() == 1);
+    }
 }
 
 TEST_CASE("SettingAutoUpdateIntervalInMinutes", "[settings]")

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<std::string>, 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);
 

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

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
     {