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

[Noah Shutty via Phabricator via llvm-commits]

noajshu added a comment.

In D112753#3180933 <https://reviews.llvm.org/D112753#3180933>, @dim wrote:

> In D112753#3180838 <https://reviews.llvm.org/D112753#3180838>, @noajshu wrote:
>
>> Thanks @dim, I have a patch that should fix this D115189 <https://reviews.llvm.org/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 <https://github.com/llvm/llvm-project/blob/main/llvm/lib/Support/CMakeLists.txt#L252>


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112753