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

[Pavel Labath via Phabricator via llvm-commits]

labath added a comment.

In D112753#3107913 <https://reviews.llvm.org/D112753#3107913>, @noajshu wrote:

> 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?

Almost.



================
Comment at: llvm/include/llvm/Support/HTTPClient.h:95
+  /// implementation
+  bool isAvailable();
+
----------------
I think this should be static, so it can be invoked before creating a client object.


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

https://reviews.llvm.org/D112753