[PATCH] D112753: [llvm] [Support] Add CURL HTTP Client.

Noah Shutty via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 3 20:13:32 PDT 2021


noajshu added a comment.

In D112753#3105456 <https://reviews.llvm.org/D112753#3105456>, @labath wrote:

> In D112753#3104614 <https://reviews.llvm.org/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?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112753/new/

https://reviews.llvm.org/D112753



More information about the llvm-commits mailing list