[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

90

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 ↗

(On Diff #386643)

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 ↗

(On Diff #386643)

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

107

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

174–176

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

148

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

 if (CMAKE_SYSTEM_NAME MATCHES "OS390")
   option(LLVM_ENABLE_THREADS "Use threads if available." OFF)
 else()
   option(LLVM_ENABLE_THREADS "Use threads if available." ON)
 endif()
 
 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
   "Enable Support for the Z3 constraint solver in LLVM."
   ${LLVM_ENABLE_Z3_SOLVER_DEFAULT}
 )
 
 if (LLVM_ENABLE_Z3_SOLVER)

llvm/include/llvm/Support/HTTPClient.h

public:
   /// handled by handleHeaderLine before any calls to handleBodyChunk.
   Error handleHeaderLine(StringRef HeaderLine) override;
   Error handleBodyChunk(StringRef BodyChunk) override;
   Error handleStatusCode(unsigned Code) override;
 };
 
 /// A reusable client that can perform HTTPRequests through a network socket.
 class HTTPClient {
+#ifdef LLVM_ENABLE_CURL
+  void *Curl = nullptr;
+  Error curlInit();
+  ~HTTPClient();
+#endif
+
 public:
   /// Returns true only if LLVM has been compiled with a working HTTPClient.
   static bool isAvailable();
 
   /// Performs the Request, passing response data to the Handler. Returns all
   /// errors which occur during the request. Aborts if an error is returned by a
   /// Handler method.
   Error perform(const HTTPRequest &Request, HTTPResponseHandler &Handler);
 
   /// Performs the Request with the default BufferedHTTPResponseHandler, and
   /// returns its HTTPResponseBuffer or an Error.
   Expected<HTTPResponseBuffer> perform(const HTTPRequest &Request);

llvm/lib/Support/CMakeLists.txt

if (MSVC)
   set (delayload_flags delayimp -delayload:shell32.dll -delayload:ole32.dll)
 endif()
 
 # Link Z3 if the user wants to build it.
 if(LLVM_WITH_Z3)
   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)$")
     message(FATAL_ERROR "LLVM_INTEGRATED_CRT_ALLOC only works with /MT or /MTd. Use LLVM_USE_CRT_${uppercase_CMAKE_BUILD_TYPE} to set the appropriate option.")
   endif()
 
   string(REGEX REPLACE "(/|\\\\)$" "" LLVM_INTEGRATED_CRT_ALLOC "${LLVM_INTEGRATED_CRT_ALLOC}")
 

llvm/lib/Support/HTTPClient.cpp

 //===----------------------------------------------------------------------===//
 
 #include "llvm/Support/HTTPClient.h"
 #include "llvm/ADT/APInt.h"
 #include "llvm/ADT/StringRef.h"
 #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;
 
 HTTPRequest::HTTPRequest(const Twine &Url) { this->Url = Url.str(); }
 
 HTTPResponseHandler::~HTTPResponseHandler() = default;
 
 Expected<HTTPResponseBuffer> HTTPClient::perform(const HTTPRequest &Request) {
Error BufferedHTTPResponseHandler::handleBodyChunk(StringRef BodyChunk) {
   return Error::success();
 }
 
 Error BufferedHTTPResponseHandler::handleStatusCode(unsigned Code) {
   ResponseBuffer.Code = Code;
   return Error::success();
 }
 
+#ifdef LLVM_ENABLE_CURL
+
+bool HTTPClient::isAvailable() { return true; }
+
+/// CurlHTTPRequest and the curl{HeaderLine,BodyChunk}Hook functions 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(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(Err);
+    return 0;
+  }
+  return Size;
+}
+
+Error HTTPClient::curlInit() {
+  if (Curl)
+    return Error::success();
+  if (!(Curl = curl_easy_init()))
+    return createStringError(errc::io_error, "Error initializing curl.");
+
+  // Set the callback hooks on first initialization.
+  curl_easy_setopt(Curl, CURLOPT_WRITEFUNCTION, curlWriteFunction);
+  curl_easy_setopt(Curl, CURLOPT_HEADERFUNCTION, curlHeaderFunction);
+  return Error::success();
+}
+
+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.");
+  if (Error Err = curlInit())
+    return Err;
+
+  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 Err;
+  return std::move(CurlRequest.ErrorState);
+}
+
+#else
+
 bool HTTPClient::isAvailable() { return false; }
 Error HTTPClient::perform(const HTTPRequest &Request,
                           HTTPResponseHandler &Handler) {
   llvm_unreachable("No HTTP Client implementation available.");
 }
+
+#endif