PR comments WIP

Author: vicroms

azure-pipelines/end-to-end-tests-dir/asset-caching.ps1

@@ -188,26 +188,10 @@ if (-not ($actual -match $expected)) {
 }
 
 # ... also with multiple authoritative URLs
-if ($IsWindows) {
-    Refresh-TestRoot
-    $expected = @(
-    "^Downloading example3\.html, trying https://nonexistent\.example\.com",
-    "Trying https://raw\.githubusercontent\.com/microsoft/vcpkg-tool/1767aaee7b229c609f7ad5cf2f57b6a6cc309fb8/LICENSE\.txt",
-    "Successfully downloaded example3\.html",
-    "$"
-    ) -join "`n"
-
-    $actual = Run-VcpkgAndCaptureOutput @commonArgs x-download "$TestDownloadsRoot/example3.html" --sha512 65077997890f66f6041bb3284bb7b88e27631411ccbc253201ca4e00c4bcc58c0d77edffda4975498797cc10772c7fd68fbeb13cc4ac493a3471a9d49e5b6f24 --url https://nonexistent.example.com --url https://raw.githubusercontent.com/microsoft/vcpkg-tool/1767aaee7b229c609f7ad5cf2f57b6a6cc309fb8/LICENSE.txt
-    Throw-IfFailed
-    if (-not ($actual -match $expected)) {
-        throw "Failure: azurl (no), x-block-origin (no), asset-cache (n/a), download (succeed)"
-    }
-}
-
-# Force curl with --header
 Refresh-TestRoot
 $expected = @(
 "^Downloading example3\.html, trying https://nonexistent\.example\.com",
+"error: curl operation failed with error code 6 \(Couldn't resolve host name\)\. This is a non-transient error, won't retry downloading from https://nonexistent\.example\.com",
 "Trying https://raw\.githubusercontent\.com/microsoft/vcpkg-tool/1767aaee7b229c609f7ad5cf2f57b6a6cc309fb8/LICENSE\.txt"
 "Successfully downloaded example3\.html",
 "$"

include/vcpkg/base/contractual-constants.h

@@ -207,7 +207,6 @@ namespace vcpkg
     inline constexpr StringLiteral SwitchDebug = "debug";
     inline constexpr StringLiteral SwitchDebugBin = "debug-bin";
     inline constexpr StringLiteral SwitchDebugEnv = "debug-env";
-    inline constexpr StringLiteral SwitchDeleteFileAfterUpload = "delete-file-after-upload";
     inline constexpr StringLiteral SwitchDgml = "dgml";
     inline constexpr StringLiteral SwitchDisableMetrics = "disable-metrics";
     inline constexpr StringLiteral SwitchDot = "dot";

include/vcpkg/base/curl.h

@@ -1,17 +1,69 @@
+#pragma once
 
-#ifdef _MSC_VER
-#pragma warning(push)           // Save current warning state
-#pragma warning(disable : 6101) // Disable specific warning (e.g., warning 4996)
-#endif
+#include <vcpkg/base/fwd/span.h>
 
+#include <vcpkg/base/pragmas.h>
+
+#include <vcpkg/commands.version.h>
+
+VCPKG_MSVC_WARNING(push)
+// note: disable warning triggered by curl headers
+// ws2tcpip.h(968): warning C6101: Returning uninitialized memory '*Mtu':  A successful path through the function does
+// not set the named _Out_ parameter.
+VCPKG_MSVC_WARNING(disable : 6101)
 #include <curl/curl.h>
 #include <curl/multi.h>
-
-#ifdef _MSC_VER
-#pragma warning(pop)
-#endif
+VCPKG_MSVC_WARNING(pop)
 
 namespace vcpkg
 {
-    CURLcode curl_global_init_status() noexcept;
+    CURLcode get_curl_global_init_status() noexcept;
+
+    struct CurlEasyHandle
+    {
+        CurlEasyHandle();
+        CurlEasyHandle(CurlEasyHandle&& other) noexcept;
+        CurlEasyHandle& operator=(CurlEasyHandle&& other) noexcept;
+        ~CurlEasyHandle();
+
+        CURL* get();
+
+    private:
+        CURL* m_ptr = nullptr;
+    };
+
+    struct CurlMultiHandle
+    {
+        CurlMultiHandle();
+        CurlMultiHandle(CurlMultiHandle&& other) noexcept;
+        CurlMultiHandle& operator=(CurlMultiHandle&& other) noexcept;
+        ~CurlMultiHandle();
+
+        // Adds an easy handle to the multi handle but doesn't take ownership of it.
+        // Makes sure that the easy handle is removed from the multi handle on cleanup.
+        void add_easy_handle(CurlEasyHandle& easy_handle);
+
+        CURLM* get();
+
+    private:
+        CURLM* m_ptr = nullptr;
+        std::vector<CURL*> m_easy_handles;
+    };
+
+    struct CurlHeaders
+    {
+        CurlHeaders();
+        CurlHeaders(View<std::string> headers);
+        CurlHeaders(CurlHeaders&& other) noexcept;
+        CurlHeaders& operator=(CurlHeaders&& other) noexcept;
+        ~CurlHeaders();
+
+        curl_slist* get() const;
+
+    private:
+        curl_slist* m_headers = nullptr;
+    };
+
+    constexpr char vcpkg_curl_user_agent[] =
+        "vcpkg/" VCPKG_BASE_VERSION_AS_STRING "-" VCPKG_VERSION_AS_STRING " (curl)";
 }

include/vcpkg/base/message-data.inc.h

@@ -863,7 +863,6 @@ DECLARE_MESSAGE(CmdUpdateRegistryAllOrTargets,
                 (),
                 "",
                 "Update registry requires either a list of artifact registry names or URiIs to update, or --all.")
-DECLARE_MESSAGE(CmdUploadMetricsDeleteFileAfterUpload, (), "", "Delete metrics payload file after upload")
 DECLARE_MESSAGE(CmdUpgradeOptNoDryRun, (), "", "Actually upgrade")
 DECLARE_MESSAGE(CmdUpgradeOptNoKeepGoing, (), "", "Stop installing packages on failure")
 DECLARE_MESSAGE(CmdUseExample1,
@@ -964,13 +963,17 @@ DECLARE_MESSAGE(CreatingNuGetPackage, (), "", "Creating NuGet package...")
 DECLARE_MESSAGE(CreatingZipArchive, (), "", "Creating zip archive...")
 DECLARE_MESSAGE(CreationFailed, (msg::path), "", "Creating {path} failed.")
 DECLARE_MESSAGE(CurlFailedGeneric,
-                (msg::exit_code, msg::error_msg),
+                (msg::exit_code),
                 "curl is the name of a program, see curl.se.",
-                "curl operation failed with error code {exit_code} ({error_msg}).")
-DECLARE_MESSAGE(CurlFailedGenericWithRetry,
-                (msg::exit_code, msg::error_msg, msg::count, msg::value),
-                "curl is the name of a program, see curl.se. {value} is the maximum amount of retries.",
-                "curl operation failed with error code {exit_code} ({error_msg}) retry {count} of {value}.")
+                "curl operation failed with error code {exit_code}")
+DECLARE_MESSAGE(CurlFailedGenericNoRetryAddendum,
+                (msg::url),
+                "curl is the name of a program, see curl.se.",
+                "This is a non-transient error, won't retry downloading from {url}.")
+DECLARE_MESSAGE(CurlFailedTimeoutRetry,
+                (msg::count, msg::value),
+                "{count} is the current retry atttempt, {value} is the maximum amount of retries.",
+                "The operation timed out retry {count} of {value}.")
 DECLARE_MESSAGE(CurlFailedHttpResponse,
                 (msg::exit_code),
                 "curl is the name of a program, see curl.se.",
@@ -979,10 +982,10 @@ DECLARE_MESSAGE(CurlFailedHttpResponseWithRetry,
                 (msg::exit_code, msg::count, msg::value),
                 "curl is the name of a program, see curl.se. {value} is the maximum amount of retries.",
                 "curl operation failed with HTTP response code {exit_code} retry {count} of {value}.")
-DECLARE_MESSAGE(CurlFailedToPutHttp,
-                (msg::exit_code, msg::error_msg, msg::url, msg::value),
+DECLARE_MESSAGE(CurlFailedToPut,
+                (msg::url, msg::value),
                 "curl is the name of a program, see curl.se. {value} is an HTTP status code",
-                "curl failed to put file to {url} with exit code {exit_code} ({error_msg}) and http code {value}.")
+                "curl failed to PUT file to {url} with response code {value}.")
 DECLARE_MESSAGE(CurrentCommitBaseline,
                 (msg::commit_sha),
                 "",

include/vcpkg/metrics.h

@@ -198,5 +198,5 @@ namespace vcpkg
     extern std::atomic<bool> g_should_send_metrics;
 
     void flush_global_metrics(const Filesystem&);
-    bool curl_upload_metrics(StringView payload);
+    bool curl_upload_metrics(const std::string& payload);
 }

locales/messages.json

@@ -502,7 +502,6 @@
   "CmdUpdateRegistrySynopsis": "Re-downloads an artifact registry",
   "CmdUpgradeOptNoDryRun": "Actually upgrade",
   "CmdUpgradeOptNoKeepGoing": "Stop installing packages on failure",
-  "CmdUploadMetricsDeleteFileAfterUpload": "Delete metrics payload file after upload",
   "CmdUseExample1": "vcpkg use <artifact name>",
   "_CmdUseExample1.comment": "This is a command line, only the <artifact name> part should be localized.",
   "CmdUseSynopsis": "Activate a single artifact in this shell",
@@ -561,16 +560,18 @@
   "CreatingZipArchive": "Creating zip archive...",
   "CreationFailed": "Creating {path} failed.",
   "_CreationFailed.comment": "An example of {path} is /foo/bar.",
-  "CurlFailedGeneric": "curl operation failed with error code {exit_code} ({error_msg}).",
-  "_CurlFailedGeneric.comment": "curl is the name of a program, see curl.se. An example of {exit_code} is 127. An example of {error_msg} is File Not Found.",
-  "CurlFailedGenericWithRetry": "curl operation failed with error code {exit_code} ({error_msg}) retry {count} of {value}.",
-  "_CurlFailedGenericWithRetry.comment": "curl is the name of a program, see curl.se. {value} is the maximum amount of retries. An example of {exit_code} is 127. An example of {error_msg} is File Not Found. An example of {count} is 42.",
+  "CurlFailedGeneric": "curl operation failed with error code {exit_code}",
+  "_CurlFailedGeneric.comment": "curl is the name of a program, see curl.se. An example of {exit_code} is 127.",
+  "CurlFailedGenericNoRetryAddendum": "This is a non-transient error, won't retry downloading from {url}.",
+  "_CurlFailedGenericNoRetryAddendum.comment": "curl is the name of a program, see curl.se. An example of {url} is https://github.com/microsoft/vcpkg.",
   "CurlFailedHttpResponse": "curl operation failed with HTTP response code {exit_code}.",
   "_CurlFailedHttpResponse.comment": "curl is the name of a program, see curl.se. An example of {exit_code} is 127.",
   "CurlFailedHttpResponseWithRetry": "curl operation failed with HTTP response code {exit_code} retry {count} of {value}.",
   "_CurlFailedHttpResponseWithRetry.comment": "curl is the name of a program, see curl.se. {value} is the maximum amount of retries. An example of {exit_code} is 127. An example of {count} is 42.",
-  "CurlFailedToPutHttp": "curl failed to put file to {url} with exit code {exit_code} ({error_msg}) and http code {value}.",
-  "_CurlFailedToPutHttp.comment": "curl is the name of a program, see curl.se. {value} is an HTTP status code An example of {exit_code} is 127. An example of {error_msg} is File Not Found. An example of {url} is https://github.com/microsoft/vcpkg.",
+  "CurlFailedTimeoutRetry": "The operation timed out retry {count} of {value}.",
+  "_CurlFailedTimeoutRetry.comment": "{count} is the current retry atttempt, {value} is the maximum amount of retries. An example of {count} is 42.",
+  "CurlFailedToPut": "curl failed to PUT file to {url} with response code {value}.",
+  "_CurlFailedToPut.comment": "curl is the name of a program, see curl.se. {value} is an HTTP status code An example of {url} is https://github.com/microsoft/vcpkg.",
   "CurrentCommitBaseline": "You can use the current commit as a baseline, which is:\n\t\"builtin-baseline\": \"{commit_sha}\"",
   "_CurrentCommitBaseline.comment": "An example of {commit_sha} is 7cfad47ae9f68b183983090afd6337cd60fd4949.",
   "CycleDetectedDuring": "cycle detected during {spec}:",

src/vcpkg.cpp

@@ -136,12 +136,6 @@ namespace
 
     void inner(const Filesystem& fs, const VcpkgCmdArguments& args, const BundleSettings& bundle)
     {
-        // Ensure that we call curl_global_init only once in the program.
-        if (CURLE_OK != curl_global_init_status())
-        {
-            Debug::println("Failed to initialize CURL");
-        }
-
         // track version on each invocation
         get_global_metrics_collector().track_string(StringMetric::VcpkgVersion, vcpkg_executable_version);
 

src/vcpkg/base/curl.cpp

@@ -1,6 +1,7 @@
+#include <vcpkg/base/checks.h>
 #include <vcpkg/base/curl.h>
 
-namespace vcpkg
+namespace
 {
     struct CurlGlobalInit
     {
@@ -17,10 +18,107 @@ namespace vcpkg
     private:
         CURLcode init_status;
     };
+}
 
-    CURLcode curl_global_init_status() noexcept
+namespace vcpkg
+{
+    CURLcode get_curl_global_init_status() noexcept
     {
         static CurlGlobalInit g_curl_global_init;
         return g_curl_global_init.get_init_status();
     }
+
+    CurlEasyHandle::CurlEasyHandle() : m_ptr(nullptr) { vcpkg::get_curl_global_init_status(); }
+    CurlEasyHandle::CurlEasyHandle(CurlEasyHandle&& other) noexcept : m_ptr(std::exchange(other.m_ptr, nullptr)) { }
+    CurlEasyHandle& CurlEasyHandle::operator=(CurlEasyHandle&& other) noexcept
+    {
+        m_ptr = std::exchange(other.m_ptr, nullptr);
+        return *this;
+    }
+    CurlEasyHandle::~CurlEasyHandle()
+    {
+        if (m_ptr)
+        {
+            curl_easy_cleanup(m_ptr);
+        }
+    }
+    CURL* CurlEasyHandle::get()
+    {
+        if (!m_ptr)
+        {
+            m_ptr = curl_easy_init();
+            if (!m_ptr)
+            {
+                Checks::unreachable(VCPKG_LINE_INFO);
+            }
+        }
+        return m_ptr;
+    }
+
+    CurlMultiHandle::CurlMultiHandle() : m_ptr(nullptr) { vcpkg::get_curl_global_init_status(); }
+    CurlMultiHandle::CurlMultiHandle(CurlMultiHandle&& other) noexcept
+        : m_ptr(std::exchange(other.m_ptr, nullptr)), m_easy_handles(std::move(other.m_easy_handles))
+    {
+    }
+    CurlMultiHandle& CurlMultiHandle::operator=(CurlMultiHandle&& other) noexcept
+    {
+        m_ptr = std::exchange(other.m_ptr, nullptr);
+        m_easy_handles = std::move(other.m_easy_handles);
+        return *this;
+    }
+    CurlMultiHandle::~CurlMultiHandle()
+    {
+        for (auto* easy_handle : m_easy_handles)
+        {
+            curl_multi_remove_handle(m_ptr, easy_handle);
+        }
+
+        if (m_ptr)
+        {
+            curl_multi_cleanup(m_ptr);
+        }
+    }
+    void CurlMultiHandle::add_easy_handle(CurlEasyHandle& easy_handle)
+    {
+        auto* handle = easy_handle.get();
+        if (curl_multi_add_handle(this->get(), handle) == CURLM_OK)
+        {
+            m_easy_handles.push_back(handle);
+        }
+    }
+    CURLM* CurlMultiHandle::get()
+    {
+        if (!m_ptr)
+        {
+            m_ptr = curl_multi_init();
+            if (!m_ptr)
+            {
+                Checks::unreachable(VCPKG_LINE_INFO);
+            }
+        }
+        return m_ptr;
+    }
+
+    CurlHeaders::CurlHeaders() : m_headers(nullptr) { }
+    CurlHeaders::CurlHeaders(View<std::string> headers) : m_headers(nullptr)
+    {
+        for (const auto& header : headers)
+        {
+            m_headers = curl_slist_append(m_headers, header.c_str());
+        }
+    }
+    CurlHeaders::CurlHeaders(CurlHeaders&& other) noexcept : m_headers(std::exchange(other.m_headers, nullptr)) { }
+    CurlHeaders& CurlHeaders::operator=(CurlHeaders&& other) noexcept
+    {
+        m_headers = std::exchange(other.m_headers, nullptr);
+        return *this;
+    }
+    CurlHeaders::~CurlHeaders()
+    {
+        if (m_headers)
+        {
+            curl_slist_free_all(m_headers);
+        }
+    }
+    curl_slist* CurlHeaders::get() const { return m_headers; }
 }