winget-cli

commit 07c6b7aeb783c23c69f87fd314417792e5aa7bb0
parent 4bb67d7692da3aa5c3c7e42c39169f87110e7923
Author: yao-msft <50888816+yao-msft@users.noreply.github.com>
Date:   Mon,  6 Apr 2020 13:52:19 -0700

Bug fixes (#78)

Manifest validation: Check Url empty before calling IsValidUrl to avoid duplicate error reporting

Create new log file for each run
Diffstat:
M
src/AppInstallerCommonCore/DateTime.cpp
 | 
12
++++++++++++
M
src/AppInstallerCommonCore/FileLogger.cpp
 | 
5
+++--
M
src/AppInstallerCommonCore/Public/AppInstallerDateTime.h
 | 
3
+++
M
src/AppInstallerCommonCore/Runtime.cpp
 | 
14
++++++++++++--
M
src/AppInstallerRepositoryCore/Manifest/ManifestInstaller.cpp
 | 
3
++-

5 files changed, 32 insertions(+), 5 deletions(-)
diff --git a/src/AppInstallerCommonCore/DateTime.cpp b/src/AppInstallerCommonCore/DateTime.cpp
@@ -27,6 +27,18 @@ namespace AppInstaller::Utility
         stream << std::setw(3) << std::setfill('0') << leftoverMillis.count();
     }
 
+    std::string GetCurrentTimeForFilename()
+    {
+        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(), ' ', '-');
+
+        return result;
+    }
+
     int64_t GetCurrentUnixEpoch()
     {
         static_assert(std::is_same_v<int64_t, decltype(time(nullptr))>, "time returns a 64-bit integer");
diff --git a/src/AppInstallerCommonCore/FileLogger.cpp b/src/AppInstallerCommonCore/FileLogger.cpp
@@ -6,7 +6,8 @@
 #include "Public/AppInstallerRuntime.h"
 #include "Public/AppInstallerDateTime.h"
 
-#define AICLI_FILELOGGER_DEFAULT_FILE "AICLI.log"
+#define AICLI_FILELOGGER_DEFAULT_FILE_PREFIX "AICLI-"
+#define AICLI_FILELOGGER_DEFAULT_FILE_EXT ".log"
 
 namespace AppInstaller::Logging
 {
@@ -17,7 +18,7 @@ namespace AppInstaller::Logging
         {
             m_name = "file";
             m_filePath = Runtime::GetPathToTemp();
-            m_filePath /= AICLI_FILELOGGER_DEFAULT_FILE;
+            m_filePath /= AICLI_FILELOGGER_DEFAULT_FILE_PREFIX + Utility::GetCurrentTimeForFilename() + AICLI_FILELOGGER_DEFAULT_FILE_EXT;
         }
         else
         {
diff --git a/src/AppInstallerCommonCore/Public/AppInstallerDateTime.h b/src/AppInstallerCommonCore/Public/AppInstallerDateTime.h
@@ -12,6 +12,9 @@ namespace AppInstaller::Utility
     // Time is also assumed to be after the epoch.
     void OutputTimepoint(std::ostream& stream, const std::chrono::system_clock::time_point& time);
 
+    // Gets the current time as a string. Can be used as a file name.
+    std::string GetCurrentTimeForFilename();
+
     // Gets the current time as a unix epoch value.
     int64_t GetCurrentUnixEpoch();
 
diff --git a/src/AppInstallerCommonCore/Runtime.cpp b/src/AppInstallerCommonCore/Runtime.cpp
@@ -5,6 +5,8 @@
 #include "Public/AppInstallerRuntime.h"
 #include "Public/AppInstallerStrings.h"
 
+#define AICLI_DEFAULT_TEMP_DIRECTORY "AICLI"
+
 namespace AppInstaller::Runtime
 {
     namespace
@@ -169,16 +171,24 @@ namespace AppInstaller::Runtime
 
     std::filesystem::path GetPathToTemp()
     {
+        std::filesystem::path result;
+
         if (IsRunningInPackagedContext())
         {
-            return { winrt::Windows::Storage::ApplicationData::Current().TemporaryFolder().Path().c_str() };
+            result.assign(winrt::Windows::Storage::ApplicationData::Current().TemporaryFolder().Path().c_str());
         }
         else
         {
             wchar_t tempPath[MAX_PATH + 1];
             DWORD tempChars = GetTempPathW(ARRAYSIZE(tempPath), tempPath);
-            return { std::wstring_view{ tempPath, static_cast<size_t>(tempChars) } };
+            result.assign(std::wstring_view{ tempPath, static_cast<size_t>(tempChars) });
         }
+
+        result /= AICLI_DEFAULT_TEMP_DIRECTORY;
+
+        std::filesystem::create_directories(result);
+
+        return result;
     }
 
     std::filesystem::path GetPathToLocalState()
diff --git a/src/AppInstallerRepositoryCore/Manifest/ManifestInstaller.cpp b/src/AppInstallerRepositoryCore/Manifest/ManifestInstaller.cpp
@@ -62,7 +62,8 @@ namespace AppInstaller::Manifest
                 resultErrors.emplace_back(ManifestError::ExeInstallerMissingSilentSwitches);
             }
 
-            if (IsValidURL(NULL, Utility::ConvertToUTF16(Url).c_str(), 0) == S_FALSE)
+            // Check empty string before calling IsValidUrl to avoid duplicate error reporting.
+            if (!Url.empty() && IsValidURL(NULL, Utility::ConvertToUTF16(Url).c_str(), 0) == S_FALSE)
             {
                 resultErrors.emplace_back(ManifestError::InvalidFieldValue, "Url", Url);
             }