[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

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

[noajshu]

llvm/CMakeLists.txt

407

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

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

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/CMakeLists.txt

if (NOT Z3_FOUND)
     message(FATAL_ERROR "LLVM_ENABLE_Z3_SOLVER cannot be enabled when Z3 is not available.")
   endif()
 
   set(LLVM_WITH_Z3 1)
 endif()
 
 set(LLVM_ENABLE_Z3_SOLVER_DEFAULT "${Z3_FOUND}")
 
+if (LLVM_ENABLE_DEBUGINFOD_CLIENT)
+  set(LLVM_WITH_CURL 1)
+  set(CURL_LIBRARY "-lcurl")
+  find_package(CURL 7.74.0 REQUIRED)
+  add_compile_definitions(
+    LLVM_ENABLE_DEBUGINFOD_CLIENT
+    LLVM_WITH_CURL)
+endif()
 
 if( LLVM_TARGETS_TO_BUILD STREQUAL "all" )
   set( LLVM_TARGETS_TO_BUILD ${LLVM_ALL_TARGETS} )
 endif()
 
 set(LLVM_TARGETS_TO_BUILD
    ${LLVM_TARGETS_TO_BUILD}
    ${LLVM_EXPERIMENTAL_TARGETS_TO_BUILD})