Shorter default installer log filename (#5705)

## Change
Make `OutputTimePoint` more flexible by allowing arbitrary fields and
behaviors to be requested via flags.

Shorten the default installer log filename by removing the leading
`WinGet-` and dropping the milliseconds. Some installers use the log
file path as a base with no error handling to ensure a valid path is
produced. Anything we can remove means more likely success.

Author: JohnMcPMS

src/AppInstallerCLICore/Workflows/ShellExecuteInstallerHandler.cpp

@@ -173,11 +173,9 @@ namespace AppInstaller::CLI::Workflow
                 const auto& manifest = context.Get<Execution::Data::Manifest>();
 
                 auto path = Runtime::GetPathTo(Runtime::PathName::DefaultLogLocation);
-                path /= Logging::FileLogger::DefaultPrefix();
+                path /= Utility::ConvertToUTF16(manifest.Id + '.' + manifest.Version);
                 path += '-';
-                path += Utility::ConvertToUTF16(manifest.Id + '.' + manifest.Version);
-                path += '-';
-                path += Utility::GetCurrentTimeForFilename();
+                path += Utility::GetCurrentTimeForFilename(true);
                 path += Logging::FileLogger::DefaultExt();
 
                 logPath = path.u8string();

src/AppInstallerCLITests/DateTime.cpp

@@ -82,3 +82,12 @@ TEST_CASE("GetTimePointFromVersion", "[datetime]")
     system_clock::time_point now = system_clock::now();
     REQUIRE(GetTimePointFromVersion(UInt64Version{ StringFromTimePoint(now) }) == time_point_cast<minutes>(now));
 }
+
+TEST_CASE("ShortFileTime", "[datetime]")
+{
+    auto shortTime = GetCurrentTimeForFilename(true);
+    auto longTime = GetCurrentTimeForFilename(false);
+    INFO(shortTime);
+    INFO(longTime);
+    REQUIRE(shortTime.length() < longTime.length());
+}

src/AppInstallerSharedLib/DateTime.cpp

@@ -3,37 +3,93 @@
 #include "pch.h"
 #include "Public/AppInstallerDateTime.h"
 
+using namespace std::chrono;
+
 namespace AppInstaller::Utility
 {
-    // If moved to C++20, this can be replaced with standard library implementations.
-    void OutputTimePoint(std::ostream& stream, const std::chrono::system_clock::time_point& time, bool useRFC3339)
+    namespace
     {
-        using namespace std::chrono;
-
-        tm localTime{};
-        auto tt = system_clock::to_time_t(time);
-        _localtime64_s(&localTime, &tt);
-
-        stream
-            << std::setw(4) << (1900 + localTime.tm_year) << '-'
-            << std::setw(2) << std::setfill('0') << (1 + localTime.tm_mon) << '-'
-            << std::setw(2) << std::setfill('0') << localTime.tm_mday << (useRFC3339 ? 'T' : ' ')
-            << std::setw(2) << std::setfill('0') << localTime.tm_hour << ':'
-            << std::setw(2) << std::setfill('0') << localTime.tm_min << ':'
-            << std::setw(2) << std::setfill('0') << localTime.tm_sec << '.';
-
-        // Get partial seconds
-        auto sinceEpoch = time.time_since_epoch();
-        auto leftoverMillis = duration_cast<milliseconds>(sinceEpoch) - duration_cast<seconds>(sinceEpoch);
+        struct OutputTimePointContext
+        {
+            OutputTimePointContext(std::ostream& stream, const std::chrono::system_clock::time_point& time, TimeFacet facet) :
+                Stream(stream), Time(time), Facet(facet)
+            {
+                auto tt = system_clock::to_time_t(time);
+                _localtime64_s(&LocalTime, &tt);
+            }
+
+            std::ostream& Stream;
+            const std::chrono::system_clock::time_point& Time;
+            tm LocalTime{};
+            TimeFacet Facet;
+        };
+
+        struct OutputTimePointFacetInfo
+        {
+            TimeFacet Facet;
+            char FollowingSeparator;
+            void (*Action)(const OutputTimePointContext&);
+        };
+    }
 
-        stream << std::setw(3) << std::setfill('0') << leftoverMillis.count();
+    void OutputTimePoint(std::ostream& stream, const std::chrono::system_clock::time_point& time, bool useRFC3339)
+    {
+        OutputTimePoint(stream, time, TimeFacet::Default | (useRFC3339 ? TimeFacet::RFC3339 : TimeFacet::None));
+    }
 
-        if (useRFC3339)
+    // If moved to C++20, this can be replaced with standard library implementations.
+    void OutputTimePoint(std::ostream& stream, const std::chrono::system_clock::time_point& time, TimeFacet facet)
+    {
+        OutputTimePointContext context{ stream, time, facet };
+        using Ctx = const OutputTimePointContext&;
+
+        bool useRFC3339 = WI_IsFlagSet(facet, TimeFacet::RFC3339);
+        bool filename = WI_IsFlagSet(facet, TimeFacet::Filename);
+        char day_time_separator = useRFC3339 ? 'T' : (filename ? '-' : ' ');
+        char time_field_separator = filename ? '-' : ':';
+
+        bool needsSeparator = false;
+        char currentSeparator = '-';
+
+        for (const auto& info : {
+            OutputTimePointFacetInfo{ TimeFacet::ShortYear, '-', [](Ctx ctx) { ctx.Stream << (ctx.LocalTime.tm_year - 100); }},
+            OutputTimePointFacetInfo{ TimeFacet::Year, '-', [](Ctx ctx) { ctx.Stream << (1900 + ctx.LocalTime.tm_year); }},
+            OutputTimePointFacetInfo{ TimeFacet::Month, '-', [](Ctx ctx) { ctx.Stream << std::setw(2) << std::setfill('0') << (1 + ctx.LocalTime.tm_mon); }},
+            OutputTimePointFacetInfo{ TimeFacet::Day, day_time_separator, [](Ctx ctx) { ctx.Stream << std::setw(2) << std::setfill('0') << ctx.LocalTime.tm_mday; }},
+            OutputTimePointFacetInfo{ TimeFacet::Hour, time_field_separator, [](Ctx ctx) { ctx.Stream << std::setw(2) << std::setfill('0') << ctx.LocalTime.tm_hour; }},
+            OutputTimePointFacetInfo{ TimeFacet::Minute, time_field_separator, [](Ctx ctx) { ctx.Stream << std::setw(2) << std::setfill('0') << ctx.LocalTime.tm_min; }},
+            OutputTimePointFacetInfo{ TimeFacet::Second, '.', [](Ctx ctx) { ctx.Stream << std::setw(2) << std::setfill('0') << ctx.LocalTime.tm_sec; }},
+            OutputTimePointFacetInfo{ TimeFacet::Millisecond, '-', [](Ctx ctx)
+            {
+                // Get partial seconds
+                auto sinceEpoch = ctx.Time.time_since_epoch();
+                auto leftoverMillis = duration_cast<milliseconds>(sinceEpoch) - duration_cast<seconds>(sinceEpoch);
+
+                ctx.Stream << std::setw(3) << std::setfill('0') << leftoverMillis.count();
+            }},
+            OutputTimePointFacetInfo{ TimeFacet::RFC3339, '\0', [](Ctx ctx)
+            {
+                // RFC 3339 requires adding time zone info.
+                // No need to bother getting the actual time zone as we don't need it.
+                // -00:00 represents an unspecified time zone, not UTC.
+                ctx.Stream << "00:00";
+            }},
+            })
         {
-            // RFC 3339 requires adding time zone info.
-            // No need to bother getting the actual time zone as we don't need it.
-            // -00:00 represents an unspecified time zone, not UTC.
-            stream << "-00:00";
+            if (WI_AreAllFlagsSet(facet, info.Facet))
+            {
+                if (needsSeparator)
+                {
+                    stream << currentSeparator;
+                }
+
+                info.Action(context);
+                needsSeparator = true;
+            }
+
+            // Getting this right for every mix of facets is probably not possible.
+            // Future needs can dictate changes here.
+            currentSeparator = info.FollowingSeparator;
         }
     }
 
@@ -44,16 +100,16 @@ namespace AppInstaller::Utility
         return std::move(stream).str();
     }
 
-    std::string GetCurrentTimeForFilename()
+    std::string TimePointToString(const std::chrono::system_clock::time_point& time, TimeFacet facet)
     {
-        std::stringstream stream;
-        OutputTimePoint(stream, std::chrono::system_clock::now());
-
-        auto result = stream.str();
-        std::replace(result.begin(), result.end(), ':', '-');
-        std::replace(result.begin(), result.end(), ' ', '-');
+        std::ostringstream stream;
+        OutputTimePoint(stream, time, facet);
+        return std::move(stream).str();
+    }
 
-        return result;
+    std::string GetCurrentTimeForFilename(bool shortTime)
+    {
+        return TimePointToString(std::chrono::system_clock::now(), (shortTime ? TimeFacet::ShortYearSecondPrecision : TimeFacet::Default) | TimeFacet::Filename);
     }
 
     std::string GetCurrentDateForARP()

src/AppInstallerSharedLib/Public/AppInstallerDateTime.h

@@ -7,16 +7,43 @@
 
 namespace AppInstaller::Utility
 {
+    // The individual aspects of a time point.
+    enum class TimeFacet
+    {
+        None        = 0x000,
+        Millisecond = 0x001,
+        Second      = 0x002,
+        Minute      = 0x004,
+        Hour        = 0x008,
+        Day         = 0x010,
+        Month       = 0x020,
+        Year        = 0x040,
+        // `Year - 2000` [2 digits for 75 more years]
+        ShortYear   = 0x080,
+        // Includes unspecified time zone
+        RFC3339     = 0x100,
+        // Limits special character use
+        Filename    = 0x200,
+
+        Default = Year | Month | Day | Hour | Minute | Second | Millisecond,
+        ShortYearSecondPrecision = ShortYear | Month | Day | Hour | Minute | Second,
+    };
+
+    DEFINE_ENUM_FLAG_OPERATORS(TimeFacet);
+
     // Writes the given time to the given stream.
     // Assumes that system_clock uses Linux epoch (as required by C++20 standard).
     // Time is also assumed to be after the epoch.
     void OutputTimePoint(std::ostream& stream, const std::chrono::system_clock::time_point& time, bool useRFC3339 = false);
+    void OutputTimePoint(std::ostream& stream, const std::chrono::system_clock::time_point& time, TimeFacet facet);
 
     // Converts the time point to a string using OutputTimePoint.
     std::string TimePointToString(const std::chrono::system_clock::time_point& time, bool useRFC3339 = false);
+    std::string TimePointToString(const std::chrono::system_clock::time_point& time, TimeFacet facet);
 
     // Gets the current time as a string. Can be used as a file name.
-    std::string GetCurrentTimeForFilename();
+    // Tries to make things a little bit shorter when shortTime == true.
+    std::string GetCurrentTimeForFilename(bool shortTime = false);
 
     // Gets the current date as a string to be used in the ARP registry.
     std::string GetCurrentDateForARP();

src/WindowsPackageManager/ConfigurationStaticFunctions.cpp

@@ -90,7 +90,7 @@ namespace ConfigurationShim
             auto& diagnosticsLogger = m_threadGlobals.GetDiagnosticLogger();
             diagnosticsLogger.SetEnabledChannels(AppInstaller::Logging::Channel::All);
             diagnosticsLogger.SetLevel(AppInstaller::Logging::Level::Verbose);
-            diagnosticsLogger.AddLogger(std::make_unique<AppInstaller::Logging::FileLogger>("ConfigStatics"sv));
+            diagnosticsLogger.AddLogger(std::make_unique<AppInstaller::Logging::FileLogger>("WinGetCFG"sv));
 
             if (IsConfigurationAvailable())
             {