[llvm] [Support] Add CURL HTTP Client.

[noajshu]

Summary

Provides an implementation of HTTPClient that wraps libcurl.

[noajshu]

Refactor to considerably simplify CurlHTTPClient.

[noajshu]

Currently there are no unit tests in this diff. I wonder if we need unit tests for the interfacing with Curl? @labath suggested an approach that uses a CurlInterface type class and mocks it up.

Another option could be to merge this with D112751 and make it so that the base HTTPClient just uses curl implementations by default. Then there would just be one (non-abstract) HTTPClient class.

[labath]

llvm/lib/Support/CurlHTTPClient.cpp

45–46 ↗

(On Diff #383719)

I'm not sure if we need this bookkeeping here. It makes the API redundant, and the same logic could be implemented in BufferedHTTPResponseHandler.

51–54 ↗

(On Diff #383719)

I don't think this function adds any value now...

81–82 ↗

(On Diff #383719)

Are you sure this works? I was expecting to see a CurlHTTPRequest object here.

And the reason I am looking at this is because I was wondering if we could store the llvm::Error (from the previous commit) inside CurlHTTPRequest. That way an Error member wouldn't be a part of any public API, and here its lifetime would be clearly limited to the duration of this function.

[noajshu]

Implement CurlRequest error state, switch Handler.handle* methods to return Errors.
Rebase on top of D112751.

[noajshu]

If we do want to merge the CulrHTTPClient into the base HTTPClient so that the default class just uses curl, what's the preferable way to do this? One option is just to move all code in CurlHTTPClient.cpp into HTTPClient.cpp, wrapping it in a #ifdef LLVM_ENABLE_CURL.

llvm/lib/Support/CurlHTTPClient.cpp

81–82 ↗

(On Diff #383719)

Good catch. That was a mistake. There are no end-to-end tests running actual curl requests so I did not catch this. The only test checks the debuginfod client.

I like the idea of storing the error in the CurlHTTPRequest. I have implemented this change.

[noajshu]

replace curlSetOpt with curl_easy_setopt

[noajshu]

Move added Curl cmake var from D112758 to D112753.

[labath]

In D112753#3104614, @noajshu wrote:

If we do want to merge the CulrHTTPClient into the base HTTPClient so that the default class just uses curl, what's the preferable way to do this? One option is just to move all code in CurlHTTPClient.cpp into HTTPClient.cpp, wrapping it in a #ifdef LLVM_ENABLE_CURL.

I'm not aware of an exact precedent (due to this introducing a class instead of a bunch of free functions, etc.), but if I try to apply what happens with other external dependencies (see e.g. Compression.h) I get a static bool (CURL)HTTPClient::isAvailable() function, and an llvm_unreachable inside all of the member functions (when curl is unavailable).

llvm/include/llvm/Support/CurlHTTPClient.h

20 ↗

(On Diff #384309)

This definitely shouldn't be at the top level, but I'm not sure if you need it at all (I count exactly one usage of that).

35 ↗

(On Diff #384309)

did you want a reference here? Plain const on a by-value argument is fairly useless.

llvm/lib/Support/CurlHTTPClient.cpp

32 ↗

(On Diff #384309)

remove this, and have caller use std::move

97 ↗

(On Diff #384309)

/*no else*/ if (CurlRequest.ErrorState) return CurlRequest.ErrorState; ?

[noajshu]

Avoid polluting namespace with top-level typedef.
Pass HTTPRequest by const reference.

[noajshu]

Use LLVM_ENABLE_CURL to decide which implementation is used for HTTPClient.

[noajshu]

In D112753#3105456, @labath wrote:

In D112753#3104614, @noajshu wrote:

If we do want to merge the CulrHTTPClient into the base HTTPClient so that the default class just uses curl, what's the preferable way to do this? One option is just to move all code in CurlHTTPClient.cpp into HTTPClient.cpp, wrapping it in a #ifdef LLVM_ENABLE_CURL.

I'm not aware of an exact precedent (due to this introducing a class instead of a bunch of free functions, etc.), but if I try to apply what happens with other external dependencies (see e.g. Compression.h) I get a static bool (CURL)HTTPClient::isAvailable() function, and an llvm_unreachable inside all of the member functions (when curl is unavailable).

Thanks! Is this what you had in mind?

[labath]

In D112753#3107913, @noajshu wrote:

In D112753#3105456, @labath wrote:

In D112753#3104614, @noajshu wrote:

If we do want to merge the CulrHTTPClient into the base HTTPClient so that the default class just uses curl, what's the preferable way to do this? One option is just to move all code in CurlHTTPClient.cpp into HTTPClient.cpp, wrapping it in a #ifdef LLVM_ENABLE_CURL.

I'm not aware of an exact precedent (due to this introducing a class instead of a bunch of free functions, etc.), but if I try to apply what happens with other external dependencies (see e.g. Compression.h) I get a static bool (CURL)HTTPClient::isAvailable() function, and an llvm_unreachable inside all of the member functions (when curl is unavailable).

Thanks! Is this what you had in mind?

Almost.

llvm/include/llvm/Support/HTTPClient.h

95

I think this should be static, so it can be invoked before creating a client object.

[noajshu]

Rebase against D112751

[noajshu]

Default build with libcurl to "ON".
Several style / commenting changes.

[noajshu]

llvm/include/llvm/Support/HTTPClient.h

83

Hi @labath, do you think we should follow the style guide recommendation for fallible constructors instead of using curlInit? That is, HTTPClient would have a private constructor and a static Expected<HTTPClient> Create(), which would just run the code inside curlInit.

[labath]

llvm/include/llvm/Support/HTTPClient.h

83

Umm... maybe?

I don't think that doing this would be a bad choice, but for me, the more interesting question is whether we should rely on the implicit curl_global_init call that happens as a part of this. This seems to be discouraged by curl, and this kind of initialization is not a particularly nice thing for a library (which llvm is) to do.

If we can guarantee that curl_global_init was called, then we might not even need a fallible constructor here. The man page is not particularly clear on this, but I would expect that the only reason this can fail in this case is memory (or some other soft resource) exhaustion, and llvm is not robust against those anyway...

[noajshu]

llvm/include/llvm/Support/HTTPClient.h

83

Thank you so much for pointing out this issue.

Three solutions come to mind:

1. The elegant C++ solution:

// HTTPClient.cpp
class CurlGlobalState {
public:
  CurlGlobalState() {
    curl_global_init(CURL_GLOBAL_ALL);
  }
  ~CurlGlobalState() {
    curl_global_cleanup(CURL_GLOBAL_ALL);
  }
};
static const CurlGlobalState CurlState;

The only potential issue here is that a library user loading LLVM Support as a Windows DLL could hit a deadlock (noted here, more details here).

2. The less-elegant solution: exposing the global init and cleanup functions to the user, e.g. wrapped by:

static HTTPClient::globalInit();
static HTTPClient::globalCleanup();

and just asking users to call these at the beginning and end of the program.

3. The LLVM-ey compromise:

Throw the global init and cleanup into InitLLVM which seems to have been made for purposes just like this.

Do you lean towards any particular solution?

[noajshu]

llvm/include/llvm/Support/HTTPClient.h

83

After some more looking, I think the ManagedStatic template may be just what we want. I'm running some experiments to see if it will fit our needs here.

[labath]

llvm/include/llvm/Support/HTTPClient.h

83

ManagedStatic would get around the ban on global constructors, but delaying the initialization until the first use means that the this could race with other calls to curl_global_init. If all uses go through this code, then it would be fine, but there's no way of knowing what will other libraries linked to the main application be doing.

This wouldn't be the first nor the worst case of llvm doing something non-library-friendly (I'm thinking of signal handlers here). However, it is not a practice I support, so I would prefer a solution (like InitLLVM) which lets the main application choose when and how the initialization happens.

[noajshu]

Perform global curl (de)initialization in static member functions of HTTPClient.

[noajshu]

Invoke Curl global init / deinit from InitLLVM's ctor / dtor.

[noajshu]

llvm/include/llvm/Support/HTTPClient.h

83

Thank you for pointing out the issues with using ManagedStatic. I've moved the global init / deinit into static methods and modified InitLLVM's ctor / dtor to call these.

[noajshu]

Add cmakedefine for LLVM_ENABLE_CURL.

[dblaikie]

llvm/lib/Support/InitLLVM.cpp

63

Does "isAvailable" need to be checked? The non-curl implementation doesn't fail on initialize or cleanup, I think? It's just a no-op there. So maybe calling these without the conditionality would be OK?

[noajshu]

Remove unnecessary checks of isAvailable before global init/deinit of HTTP client.
Also only define IsInitialized state for the curl client implementation enabled by -DLLVM_ENABLE_CURL, as it is not needed for the unimplemented client.

[noajshu]

llvm/lib/Support/InitLLVM.cpp

63

Thanks, indeed, good point. I removed this and only declare IsInitialized when the curl client is enabled since it serves no purpose for the "unimplemented" form of the HTTP client.

[dblaikie]

llvm/lib/Support/HTTPClient.cpp

112

Generally this function should accept Error by value (or rvalue reference) and callers should pass with std::move.

179–181

I /think/ the ErrorState should be tested/returned first, otherwise if the Code codepath is hit (or the CurlRes codepath above) will hit an assertion due to the ErrorState being unchecked.

[noajshu]

All return paths from Curl-enabled HTTPClient::perform after initialization of CurlHTTPRequest pass back the CurlRequest.ErrorState so it gets checked.

[dblaikie]

Looks good to me - how about you, @labath ?

[labath]

I'm thinking about llvm-config.h vs config.h (the difference is that the first is shipped alongside the compiler and exposed to users). We put most of our defines in the latter, but I can't really say why this shouldn't be exposed through llvm-config.h, so meh...

[noajshu]

Rename unit tests and add additional test.

[noajshu]

Rebase against D112751.

[noajshu]

Add implementation of HTTPClient::setTimeout

[noajshu]

Change timeouts to be in std::chrono::milliseconds

[dblaikie]

Generally still looks good to me.

[noajshu]

Rebase against main.

[hans]

In Chromium we just noticed that our clang binary grew a dependency on libcurl-gnutls.so.4 after this change.

That seems odd since clang doesn't have any business talking to the network. I assume this is really for Debuginfod?

If I understand correctly, the HTTPClient::initialize() call in InitLLVM::InitLLVM() forces the library into *all* LLVM tools by default. The extra dependency seems odd for some tools ("did clang start phoning home with my source?") and doing the initialization as part of each compile step also seems wasteful. Couldn't the initialization happen lazily on the first call to HTTPClient from tools that need it instead?

[thakis]

+1. I think a better model to follow is how we link libxml2 only into lld instead into all binaries. (Which means we'd move HTTPClient.cpp out of Support into its own library, which seems like it makes sense anyways.) See https://reviews.llvm.org/D35819 ​for the example.

[noajshu]

In D112753#3171414, @hans wrote:

In Chromium we just noticed that our clang binary grew a dependency on libcurl-gnutls.so.4 after this change.

That seems odd since clang doesn't have any business talking to the network. I assume this is really for Debuginfod?

Thanks for bringing this up. Yes, this is for the debuginfod client library (D112758). The default value for LLVM_ENABLE_CURL is ON. So if it finds libcurl on your system, it will use it.
If your clang build were configured with -DLLVM_ENABLE_CURL=OFF, the dependency on libcurl would disappear.
Do you think we should change the default value to OFF?

If I understand correctly, the HTTPClient::initialize() call in InitLLVM::InitLLVM() forces the library into *all* LLVM tools by default. The extra dependency seems odd for some tools ("did clang start phoning home with my source?") and doing the initialization as part of each compile step also seems wasteful. Couldn't the initialization happen lazily on the first call to HTTPClient from tools that need it instead?

Curl's global initialization is not thread-safe and should be called at the beginning of the program. This is why we added it to InitLLVM. There is some relevant discussion on this here: https://reviews.llvm.org/D112753#inline-1082560
If LLVM_ENABLE_CURL is OFF then the initialize is a no-op. However, if built with curl, tools that do not use it will still call the initialization step. As you point out, this is wasteful. So do you think we should remove it from InitLLVM and require user tools to separately init/deinit? Or, should we add a default-false argument InitializeHTTPClient to InitLLVM's ctor?

Thanks again for bringing this up.

[noajshu]

In D112753#3171514, @thakis wrote:

+1. I think a better model to follow is how we link libxml2 only into lld instead into all binaries. (Which means we'd move HTTPClient.cpp out of Support into its own library, which seems like it makes sense anyways.) See https://reviews.llvm.org/D35819 ​for the example.

Thanks, we could move this to a separate library. We would also have to remove the global init/deinit from InitLLVM or else Support would depend on this library.

[thakis]

Yup, that sounds right.

If it takes a while to move this to a separate lib, I think it'd be good to change the default here to OFF until then :)

[noajshu]

In D112753#3171903, @thakis wrote:

Yup, that sounds right.

If it takes a while to move this to a separate lib, I think it'd be good to change the default here to OFF until then :)

Thanks! I just added you as a reviewer of D115131 which moves the HTTPClient.cpp to the Debuginfod library.

[dim]

This breaks building on FreeBSD, where I see:

-- Found LibXml2: /usr/local/lib/libxml2.so (found version "2.9.12")
-- Looking for xmlReadMemory
-- Looking for xmlReadMemory - found
-- Found CURL: /usr/local/lib/libcurl.so (found version "7.79.1")
-- Looking for curl_easy_init
-- Looking for curl_easy_init - found
-- Looking for el_init in edit
-- Looking for el_init in edit - found
...

and then later:

FAILED: lib/Support/CMakeFiles/LLVMSupport.dir/HTTPClient.cpp.o
/usr/bin/clang++ -DGTEST_HAS_RTTI=0 -D_DEBUG -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/home/dim/obj/llvm-llvmorg-14-init-11213-ge0b259f22c00-freebsd13-amd64-ninja-clang-rel-1/lib/Support -I/home/dim/src/llvm/llvm-project/llvm/lib/Support -I/home/dim/obj/llvm-llvmorg-14-init-11213-ge0b259f22c00-freebsd13-amd64-ninja-clang-rel-1/include -I/home/dim/src/llvm/llvm-project/llvm/include -fPIC -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -fdiagnostics-color -ffunction-sections -fdata-sections  -O3 -DNDEBUG -UNDEBUG -std=c++14  -fno-exceptions -fno-rtti -MD -MT lib/Support/CMakeFiles/LLVMSupport.dir/HTTPClient.cpp.o -MF lib/Support/CMakeFiles/LLVMSupport.dir/HTTPClient.cpp.o.d -o lib/Support/CMakeFiles/LLVMSupport.dir/HTTPClient.cpp.o -c /home/dim/src/llvm/llvm-project/llvm/lib/Support/HTTPClient.cpp
/home/dim/src/llvm/llvm-project/llvm/lib/Support/HTTPClient.cpp:23:10: fatal error: 'curl/curl.h' file not found
#include <curl/curl.h>
         ^~~~~~~~~~~~~
1 error generated.

It looks like the -I flags aren't correctly detected, since it doesn't pass -I/usr/local/include, as it should:

$ curl-config --cflags
-I/usr/local/include

$ curl-config --libs
-L/usr/local/lib -lcurl

[noajshu]

Thanks @dim, I have a patch that should fix this D115189
Would that solve the breakage on freebsd?

[dim]

In D112753#3180838, @noajshu wrote:

Thanks @dim, I have a patch that should fix this D115189
Would that solve the breakage on freebsd?

No, as I went back in history to find this commit that introduced the feature. I'm not a cmake expert, but I think the problem lies in the way features are tested in llvm/cmake/config-ix.cmake, e.g.:

if(CURL_FOUND)
  # Check if curl we found is usable; for example, we may have found a 32-bit
  # library on a 64-bit system which would result in a link-time failure.
  cmake_push_check_state()
  list(APPEND CMAKE_REQUIRED_INCLUDES ${CURL_INCLUDE_DIRS})
  list(APPEND CMAKE_REQUIRED_LIBRARIES ${CURL_LIBRARY})
  check_symbol_exists(curl_easy_init curl/curl.h HAVE_CURL)
  cmake_pop_check_state()
  if(LLVM_ENABLE_CURL STREQUAL FORCE_ON AND NOT HAVE_CURL)
    message(FATAL_ERROR "Failed to configure curl")
  endif()
endif()

As far as I understand it, the cmake_pop_check_state() after the checks *restores* the CMAKE_REQUIRED_INCLUDES to its previous value, and therefore removes the appended CURL_INCLUDE_DIRS value.

Strangely, most of the external library checks go this way, e.g. for libxml just above:

if(LibXml2_FOUND)
  # Check if libxml2 we found is usable; for example, we may have found a 32-bit
  # library on a 64-bit system which would result in a link-time failure.
  cmake_push_check_state()
  list(APPEND CMAKE_REQUIRED_INCLUDES ${LIBXML2_INCLUDE_DIRS})
  list(APPEND CMAKE_REQUIRED_LIBRARIES ${LIBXML2_LIBRARIES})
  list(APPEND CMAKE_REQUIRED_DEFINITIONS ${LIBXML2_DEFINITIONS})
  check_symbol_exists(xmlReadMemory libxml/xmlreader.h HAVE_LIBXML2)
  cmake_pop_check_state()
  if(LLVM_ENABLE_LIBXML2 STREQUAL FORCE_ON AND NOT HAVE_LIBXML2)
    message(FATAL_ERROR "Failed to configure libxml2")
  endif()
endif()

I simply don't understand how this can work, if the detected include dirs are not in a system-default location, such as /usr/include...

[noajshu]

In D112753#3180933, @dim wrote:

In D112753#3180838, @noajshu wrote:

Thanks @dim, I have a patch that should fix this D115189
Would that solve the breakage on freebsd?

No, as I went back in history to find this commit that introduced the feature. I'm not a cmake expert, but I think the problem lies in the way features are tested in llvm/cmake/config-ix.cmake, e.g.:

if(CURL_FOUND)
  # Check if curl we found is usable; for example, we may have found a 32-bit
  # library on a 64-bit system which would result in a link-time failure.
  cmake_push_check_state()
  list(APPEND CMAKE_REQUIRED_INCLUDES ${CURL_INCLUDE_DIRS})
  list(APPEND CMAKE_REQUIRED_LIBRARIES ${CURL_LIBRARY})
  check_symbol_exists(curl_easy_init curl/curl.h HAVE_CURL)
  cmake_pop_check_state()
  if(LLVM_ENABLE_CURL STREQUAL FORCE_ON AND NOT HAVE_CURL)
    message(FATAL_ERROR "Failed to configure curl")
  endif()
endif()

As far as I understand it, the cmake_pop_check_state() after the checks *restores* the CMAKE_REQUIRED_INCLUDES to its previous value, and therefore removes the appended CURL_INCLUDE_DIRS value.

Strangely, most of the external library checks go this way, e.g. for libxml just above:

if(LibXml2_FOUND)
  # Check if libxml2 we found is usable; for example, we may have found a 32-bit
  # library on a 64-bit system which would result in a link-time failure.
  cmake_push_check_state()
  list(APPEND CMAKE_REQUIRED_INCLUDES ${LIBXML2_INCLUDE_DIRS})
  list(APPEND CMAKE_REQUIRED_LIBRARIES ${LIBXML2_LIBRARIES})
  list(APPEND CMAKE_REQUIRED_DEFINITIONS ${LIBXML2_DEFINITIONS})
  check_symbol_exists(xmlReadMemory libxml/xmlreader.h HAVE_LIBXML2)
  cmake_pop_check_state()
  if(LLVM_ENABLE_LIBXML2 STREQUAL FORCE_ON AND NOT HAVE_LIBXML2)
    message(FATAL_ERROR "Failed to configure libxml2")
  endif()
endif()

I simply don't understand how this can work, if the detected include dirs are not in a system-default location, such as /usr/include...

Thanks!
My hypothesis is that the imported libs added to Support are not pulling in their includes to the target because they are linked with PRIVATE. If this is the case, maybe we need to add ADDITIONAL_HEADER_DIRS includes for those here: https://github.com/llvm/llvm-project/blob/main/llvm/lib/Support/CMakeLists.txt#L252

[noajshu]

@dim, I just committed D115189 which should solve your problem. Thanks for your help and please let me know if your build still fails.

[dim]

In D112753#3181695, @noajshu wrote:

@dim, I just committed D115189 which should solve your problem. Thanks for your help and please let me know if your build still fails.

Thanks, I checked and indeed just before rG54b35c09b2cd it still fails in the same way, but after rG54b35c09b2cd it works!

[MaskRay]

llvm/lib/Support/HTTPClient.cpp

153

For non-Debug build types, LLVM_ENABLE_ASSERTIONS=off is the default and assert is a no-op.

I fixed this in 3dd2d4c0a239ce78421d51491456cb1a075ad2f3

llvm/CMakeLists.txt

 
 set(LLVM_ENABLE_ZLIB "ON" CACHE STRING "Use zlib for compression/decompression if available. Can be ON, OFF, or FORCE_ON")
 
+set(LLVM_ENABLE_CURL "ON" CACHE STRING "Use libcurl for the HTTP client if available. Can be ON, OFF, or FORCE_ON")
+
 set(LLVM_Z3_INSTALL_DIR "" CACHE STRING "Install directory of the Z3 solver.")
 
 option(LLVM_ENABLE_Z3_SOLVER

llvm/include/llvm/Config/llvm-config.h.cmake

 /* Define if we have z3 and want to build it */
 #cmakedefine LLVM_WITH_Z3 ${LLVM_WITH_Z3}
 
+/* Define if we have curl and want to use it */
+#cmakedefine LLVM_ENABLE_CURL ${LLVM_ENABLE_CURL}
+
 /* Define if LLVM was built with a dependency to the libtensorflow dynamic library */
 #cmakedefine LLVM_HAVE_TF_API
 

llvm/include/llvm/Support/HTTPClient.h

 
 /// A reusable client that can perform HTTPRequests through a network socket.
 class HTTPClient {
+#ifdef LLVM_ENABLE_CURL
+  void *Curl = nullptr;
+  static bool IsInitialized;
+#endif
+
 public:
   HTTPClient();
   ~HTTPClient();

llvm/lib/Support/CMakeLists.txt

   set(system_libs ${system_libs} ${Z3_LIBRARIES})
 endif()
 
+# Link LibCURL if the user wants it
+if (LLVM_ENABLE_CURL)
+  set(system_libs ${system_libs} ${CURL_LIBRARIES})
+endif()
+
 # Override the C runtime allocator on Windows and embed it into LLVM tools & libraries
 if(LLVM_INTEGRATED_CRT_ALLOC)
   if (CMAKE_BUILD_TYPE AND NOT ${LLVM_USE_CRT_${uppercase_CMAKE_BUILD_TYPE}} MATCHES "^(MT|MTd)$")

llvm/lib/Support/HTTPClient.cpp

 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/MemoryBuffer.h"
+#ifdef LLVM_ENABLE_CURL
+#include <curl/curl.h>
+#endif
 
 using namespace llvm;
 

   return perform(Request);
 }
 
+#ifdef LLVM_ENABLE_CURL
+
+bool HTTPClient::isAvailable() { return true; }
+
+bool HTTPClient::IsInitialized = false;
+
+void HTTPClient::initialize() {
+  if (!IsInitialized) {
+    curl_global_init(CURL_GLOBAL_ALL);
+    IsInitialized = true;
+  }
+}
+
+void HTTPClient::cleanup() {
+  if (IsInitialized) {
+    curl_global_cleanup();
+    IsInitialized = false;
+  }
+}
+
+/// CurlHTTPRequest and the curl{Header,Write}Function are implementation
+/// details used to work with Curl. Curl makes callbacks with a single
+/// customizable pointer parameter.
+struct CurlHTTPRequest {
+  CurlHTTPRequest(HTTPResponseHandler &Handler) : Handler(Handler) {}
+  void storeError(Error Err) {
+    ErrorState = joinErrors(std::move(Err), std::move(ErrorState));
+  }
+  HTTPResponseHandler &Handler;
+  llvm::Error ErrorState = Error::success();
+};
+
+static size_t curlHeaderFunction(char *Contents, size_t Size, size_t NMemb,
+                                 CurlHTTPRequest *CurlRequest) {
+  assert(Size == 1 && "The Size passed by libCURL to CURLOPT_HEADERFUNCTION "
+                      "should always be 1.");
+  if (Error Err =
+          CurlRequest->Handler.handleHeaderLine(StringRef(Contents, NMemb))) {
+    CurlRequest->storeError(std::move(Err));
+    return 0;
+  }
+  return NMemb;
+}
+
+static size_t curlWriteFunction(char *Contents, size_t Size, size_t NMemb,
+                                CurlHTTPRequest *CurlRequest) {
+  Size *= NMemb;
+  if (Error Err =
+          CurlRequest->Handler.handleBodyChunk(StringRef(Contents, Size))) {
+    CurlRequest->storeError(std::move(Err));
+    return 0;
+  }
+  return Size;
+}
+
+HTTPClient::HTTPClient() {
+  assert(IsInitialized &&
+         "Must call HTTPClient::initialize() at the beginning of main().");
+  if (Curl)
+    return;
+  assert((Curl = curl_easy_init()) && "Curl could not be initialized.");
+  // Set the callback hooks.
+  curl_easy_setopt(Curl, CURLOPT_WRITEFUNCTION, curlWriteFunction);
+  curl_easy_setopt(Curl, CURLOPT_HEADERFUNCTION, curlHeaderFunction);
+}
+
+HTTPClient::~HTTPClient() { curl_easy_cleanup(Curl); }
+
+Error HTTPClient::perform(const HTTPRequest &Request,
+                          HTTPResponseHandler &Handler) {
+  if (Request.Method != HTTPMethod::GET)
+    return createStringError(errc::invalid_argument,
+                             "Unsupported CURL request method.");
+
+  SmallString<128> Url = Request.Url;
+  curl_easy_setopt(Curl, CURLOPT_URL, Url.c_str());
+  curl_easy_setopt(Curl, CURLOPT_FOLLOWLOCATION, Request.FollowRedirects);
+
+  CurlHTTPRequest CurlRequest(Handler);
+  curl_easy_setopt(Curl, CURLOPT_WRITEDATA, &CurlRequest);
+  curl_easy_setopt(Curl, CURLOPT_HEADERDATA, &CurlRequest);
+  CURLcode CurlRes = curl_easy_perform(Curl);
+  if (CurlRes != CURLE_OK)
+    return joinErrors(std::move(CurlRequest.ErrorState),
+                      createStringError(errc::io_error,
+                                        "curl_easy_perform() failed: %s\n",
+                                        curl_easy_strerror(CurlRes)));
+  if (CurlRequest.ErrorState)
+    return std::move(CurlRequest.ErrorState);
+
+  unsigned Code;
+  curl_easy_getinfo(Curl, CURLINFO_RESPONSE_CODE, &Code);
+  if (Error Err = Handler.handleStatusCode(Code))
+    return joinErrors(std::move(CurlRequest.ErrorState), std::move(Err));
+
+  return std::move(CurlRequest.ErrorState);
+}
+
+#else
+
 HTTPClient::HTTPClient() = default;
 
 HTTPClient::~HTTPClient() = default;
 
 bool HTTPClient::isAvailable() { return false; }
 
+void HTTPClient::initialize() {}
+
 void HTTPClient::cleanup() {}
 
 Error HTTPClient::perform(const HTTPRequest &Request,
                           HTTPResponseHandler &Handler) {
   llvm_unreachable("No HTTP Client implementation available.");
 }
+
+#endif

llvm/lib/Support/InitLLVM.cpp

 
 #include "llvm/Support/InitLLVM.h"
 #include "llvm/Support/Error.h"
+#include "llvm/Support/HTTPClient.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/Process.h"

   Argc = Args.size() - 1;
   Argv = Args.data();
 #endif
+
+  HTTPClient::initialize();
 }
 
-InitLLVM::~InitLLVM() { llvm_shutdown(); }
+InitLLVM::~InitLLVM() {
+  HTTPClient::cleanup();
+  llvm_shutdown();
+}

llvm/unittests/Support/HTTPClient.cpp

 
 using namespace llvm;
 
-TEST(HTTPClientTests, bufferedHTTPResponseHandlerNoContentLengthLifecycleTest) {
-  BufferedHTTPResponseHandler Handler;
-  EXPECT_EQ(Handler.ResponseBuffer.Code, 0u);
-  EXPECT_EQ(Handler.ResponseBuffer.Body, nullptr);
-
-  // A body chunk passed before the content-length header is an error.
-  EXPECT_THAT_ERROR(Handler.handleBodyChunk("body"),
-                    Failed<llvm::StringError>());
-  EXPECT_THAT_ERROR(Handler.handleHeaderLine("a header line"), Succeeded());
-  EXPECT_THAT_ERROR(Handler.handleBodyChunk("body"),
-                    Failed<llvm::StringError>());
-}
-
-TEST(HTTPClientTests, bufferedHTTPResponseHandlerLifecycleTest) {
+TEST(BufferedHTTPResponseHandler, Lifecycle) {
   BufferedHTTPResponseHandler Handler;
   EXPECT_THAT_ERROR(Handler.handleHeaderLine("Content-Length: 36\r\n"),
                     Succeeded());

   EXPECT_EQ(Handler.ResponseBuffer.Code, 400u);
 }
 
-TEST(HTTPClientTests, bufferedHTTPResponseHandlerZeroContentLengthTest) {
+TEST(BufferedHTTPResponseHandler, NoContentLengthLifecycle) {
+  BufferedHTTPResponseHandler Handler;
+  EXPECT_EQ(Handler.ResponseBuffer.Code, 0u);
+  EXPECT_EQ(Handler.ResponseBuffer.Body, nullptr);
+
+  // A body chunk passed before the content-length header is an error.
+  EXPECT_THAT_ERROR(Handler.handleBodyChunk("body"),
+                    Failed<llvm::StringError>());
+  EXPECT_THAT_ERROR(Handler.handleHeaderLine("a header line"), Succeeded());
+  EXPECT_THAT_ERROR(Handler.handleBodyChunk("body"),
+                    Failed<llvm::StringError>());
+}
+
+TEST(BufferedHTTPResponseHandler, ZeroContentLength) {
   BufferedHTTPResponseHandler Handler;
   EXPECT_THAT_ERROR(Handler.handleHeaderLine("Content-Length: 0"), Succeeded());
   EXPECT_NE(Handler.ResponseBuffer.Body, nullptr);

                     Failed<llvm::StringError>());
 }
 
-TEST(HTTPClientTests, bufferedHTTPResponseHandlerMalformedContentLengthTest) {
+TEST(BufferedHTTPResponseHandler, MalformedContentLength) {
   // Check that several invalid content lengths are ignored.
   BufferedHTTPResponseHandler Handler;
   EXPECT_EQ(Handler.ResponseBuffer.Body, nullptr);

   EXPECT_THAT_ERROR(Handler.handleBodyChunk("non-empty body content"),
                     Failed<llvm::StringError>());
 }
+
+#ifdef LLVM_ENABLE_CURL
+
+TEST(HTTPClient, isAvailable) { EXPECT_TRUE(HTTPClient::isAvailable()); }
+
+#endif