[Support][llvm] Add optional libCURL dependency to llvm build configuration

[noajshu]

Summary

This finds the curl libraries if LLVM_ENABLE_CURL is set. This is needed to implement the debuginfod client library in LLVM.

[lebedev.ri]

llvm/CMakeLists.txt

407 ↗

(On Diff #377569)

This can't be right, shouldn't you be using CURL_LIBRARIES set by find_package()?

[noajshu]

llvm/CMakeLists.txt

407 ↗

(On Diff #377569)

Thanks, I agree! L407 is needed to avoid the following error which arises if it is removed:

CMake Error at /usr/share/cmake-3.18/Modules/FindPackageHandleStandardArgs.cmake:165 (message):
  Could NOT find CURL (missing: CURL_LIBRARY) (found suitable version
  "7.74.0", minimum required is "7.74.0")
Call Stack (most recent call first):
  /usr/share/cmake-3.18/Modules/FindPackageHandleStandardArgs.cmake:458 (_FPHSA_FAILURE_MESSAGE)
  /usr/share/cmake-3.18/Modules/FindCURL.cmake:169 (find_package_handle_standard_args)
  CMakeLists.txt:407 (find_package)


-- Configuring incomplete, errors occurred!

I tried a few workarounds but setting the CURL_LIBRARY was the simplest fix I found. Do you have any idea what might cause this?

[lebedev.ri]

llvm/CMakeLists.txt

407 ↗

(On Diff #377569)

I don't know why you are seeing that error, but this should be roughly

if (LLVM_ENABLE_DEBUGINFOD_CLIENT)
  set(LLVM_WITH_CURL 1)
  find_package(CURL 7.74.0 REQUIRED)
  add_compile_definitions(
    LLVM_ENABLE_DEBUGINFOD_CLIENT
    LLVM_WITH_CURL)
  # use CURL_LIBRARIES where needed / link to CURL::libcurl imported target.
endif()

Anything that hardcodes -lcurl is wrong.

[phosek]

This logic should go into config-ix.cmake. I'd recommend following the same model we use for zlib here: https://github.com/llvm/llvm-project/blob/main/llvm/cmake/config-ix.cmake#L119. So this should look something like this:

if(LLVM_ENABLE_CURL)
  if(LLVM_ENABLE_CURL STREQUAL FORCE_ON)
    find_package(CURL REQUIRED)
  else()
    find_package(CURL)
  endif()
  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.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()
  set(LLVM_ENABLE_CURL "${HAVE_CURL}")
endif()

To use it, you'd include CURL::libcurl as a dependency in you target, but only if LLVM_ENABLE_CURL is set. See https://github.com/llvm/llvm-project/blob/fd9a5b911d5e85c5184e9ed6907f53f23b41cd99/llvm/lib/Support/CMakeLists.txt#L25 for a zlib example.

llvm/CMakeLists.txt

405 ↗

(On Diff #377569)

Rather than having a LLVM_ENABLE_DEBUGINFOD_CLIENT variable, I'd prefer LLVM_ENABLE_CURL to control whether to search for curl or not. Whether debuginfod client should be built or not could be conditionalized on that.

[noajshu]

rebase against main

[noajshu]

Move CURL dependency logic to config-ix.cmake.
Also update to the same model used for zlib as suggested by phosek.

[noajshu]

Remove extraneous change to llvm/CMakeLists.txt

[phosek]

LGTM

[noajshu]

changed header file from curl.h to curl/curl.h to pass check_symbol_exists test

[noajshu]

rebase against main

[noajshu]

Thanks @phosek. I don’t have commit access, could you land this patch for me? Please use “Noah Shutty <[e-mail]>” to commit the change.

[noajshu]

rebase against D111238

llvm/cmake/config-ix.cmake

if(LibXml2_FOUND)
     cmake_pop_check_state()
     if(LLVM_ENABLE_LIBXML2 STREQUAL FORCE_ON AND NOT HAVE_LIBXML2)
       message(FATAL_ERROR "Failed to configure libxml2")
     endif()
   endif()
   set(LLVM_ENABLE_LIBXML2 "${HAVE_LIBXML2}")
 endif()
 
+if(LLVM_ENABLE_CURL)
+  if(LLVM_ENABLE_CURL STREQUAL FORCE_ON)
+    find_package(CURL REQUIRED)
+  else()
+    find_package(CURL)
+  endif()
+  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.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()
+  set(LLVM_ENABLE_CURL "${HAVE_CURL}")
+endif()
+
 # Don't look for these libraries if we're using MSan, since uninstrumented third
 # party code may call MSan interceptors like strlen, leading to false positives.
 if(NOT LLVM_USE_SANITIZER MATCHES "Memory.*")
   # Don't look for these libraries on Windows.
   if (NOT PURE_WINDOWS)
     # Skip libedit if using ASan as it contains memory leaks.
     if (LLVM_ENABLE_LIBEDIT AND HAVE_HISTEDIT_H AND NOT LLVM_USE_SANITIZER MATCHES ".*Address.*")
       check_library_exists(edit el_init "" HAVE_LIBEDIT)