Upgrade c-ares minimum version to 1.15+

[anthonyalayo]

C-ares 1.15 introduced a c-ares-config.cmake file:
c-ares/c-ares@4989438

This file makes the detection of c-ares much better than the current find module implementation:
https://github.com/nghttp2/nghttp2/blob/master/CMakeLists.txt#L61
https://github.com/nghttp2/nghttp2/blob/master/cmake/FindLibcares.cmake

Since Unix traditionally builds packages using autotools instead of CMake, the file will only exist when built by the user.

How does the following logic sound?

1. We update c-ares minimum version to be 1.15 (or higher)
2. We try find_package(c-ares CONFIG) first, and if that fails, we fallback to find_package(Libcares)
3. If find_package(c-ares CONFIG) was successful, we set the same variables to match the module mode.

I picked 1.15 instead of a later version since Ubuntu focal has that version, so there's less breakage across builds:
https://launchpad.net/ubuntu/+source/c-ares

What do you think @tatsuhiro-t ?

[tatsuhiro-t]

Thank you for suggestion. We usually do not bump version just because it has better cmake files, but in this case I think it is OK to bump minimum version because 1.15 is old enough.

[anthonyalayo]

Thank you! For context, users who fetch and build c-ares as part of their build will not have it installed ahead of time. I am one such user, and I hit this exact issue.

In order to make nghttp2 work with c-ares as part of a build that builds curl, this is what I had to do for c-ares:

function(fetchCares)
  message(STATUS "Fetching c-ares")

  include(FetchContent)

  FetchContent_Declare(
    cares
    GIT_REPOSITORY https://github.com/c-ares/c-ares.git
    GIT_TAG cares-${cares_version}
    OVERRIDE_FIND_PACKAGE
  )

  # cares can't handle unity builds
  set(CMAKE_UNITY_BUILD OFF)

  # Set options before fetching the content
  set(CARES_STATIC ${BUILD_STATIC_LIBS})
  set(CARES_SHARED ${BUILD_SHARED_LIBS})
  set(CARES_BUILD_TOOLS OFF)
  set(CARES_INSTALL ON)

  FetchContent_MakeAvailable(cares)

  # quiet warnings on newer compilers
  target_compile_options(c-ares PRIVATE -Wno-conversion -Wno-sign-conversion)

  # hack for curl until cmake-built c-ares is supported
  set(CARES_LIBRARY $<TARGET_FILE:c-ares> CACHE INTERNAL "Curl C-ares Lib")

  set(INCLUDES ${cares_SOURCE_DIR}/include)
  list(APPEND INCLUDES ${cares_BINARY_DIR})
  set(CARES_INCLUDE_DIR ${INCLUDES} CACHE INTERNAL "Curl C-ares Include")

  # hack for nghttp2 until cmake-built c-ares is supported
  set(LIBCARES_LIBRARY ${CARES_LIBRARY} CACHE INTERNAL "Nghttp2 C-ares Lib")
  set(LIBCARES_INCLUDE_DIR ${CARES_INCLUDE_DIR} CACHE INTERNAL "Nghttp2 C-ares Include")

  # Replace underscores with periods
  string(REPLACE "_" "." cares_dotted_version ${cares_version})
  set(LIBCARES_VERSION ${cares_dotted_version} CACHE INTERNAL "Nghttp2 C-ares Version")
endfunction()

fetchCares()

And this is what I had to do for nghttp2:

function(fetchNghttp2)
  message(STATUS "Fetching nghttp2")

  include(FetchContent)

  # the || true is needed, as cmake is not smart and re-tries patches
  set(patch_command git apply ${PROJECT_SOURCE_DIR}/patches/nghttp2.patch || true)

  FetchContent_Declare(
    nghttp2
    GIT_REPOSITORY https://github.com/nghttp2/nghttp2.git
    GIT_TAG ${nghttp2_version}
    PATCH_COMMAND ${patch_command}
    OVERRIDE_FIND_PACKAGE
  )

  # nghttp2 can't handle unity builds
  set(CMAKE_UNITY_BUILD OFF)

  # Set options before fetching the content
  set(ENABLE_STATIC_LIB ${BUILD_STATIC_LIBS})
  set(ENABLE_SHARED_LIB ${BUILD_SHARED_LIBS})
  set(ENABLE_LIB_ONLY ON)
  set(ENABLE_FAILMALLOC OFF)
  set(ENABLE_DOC OFF)

  FetchContent_MakeAvailable(nghttp2)

  set(LIBRARY nghttp2_static)
  if(BUILD_SHARED_LIBS)
    set(LIBRARY nghttp2)
  endif()

  # hack for curl until cmake config support is added to nghttp
  set(NGHTTP2_LIBRARIES $<TARGET_FILE:${LIBRARY}> CACHE INTERNAL "Curl Nghttp2 Lib")

  set(INCLUDES ${nghttp2_SOURCE_DIR}/lib/includes)
  list(APPEND INCLUDES ${nghttp2_BINARY_DIR}/lib/includes)
  set(NGHTTP2_INCLUDE_DIRS ${INCLUDES} CACHE INTERNAL "Curl Nghttp Include")
endfunction()

fetchNghttp2()

Where the patch file is the following:

diff --git a/cmake/FindLibcares.cmake b/cmake/FindLibcares.cmake
index 1fe56ce7..19b188d4 100644
--- a/cmake/FindLibcares.cmake
+++ b/cmake/FindLibcares.cmake
@@ -16,15 +16,6 @@ find_library(LIBCARES_LIBRARY
   HINTS ${PC_LIBCARES_LIBRARY_DIRS}
 )
 
-if(LIBCARES_INCLUDE_DIR)
-  set(_version_regex "^#define[ \t]+ARES_VERSION_STR[ \t]+\"([^\"]+)\".*")
-  file(STRINGS "${LIBCARES_INCLUDE_DIR}/ares_version.h"
-    LIBCARES_VERSION REGEX "${_version_regex}")
-  string(REGEX REPLACE "${_version_regex}" "\\1"
-    LIBCARES_VERSION "${LIBCARES_VERSION}")
-  unset(_version_regex)
-endif()
-
 include(FindPackageHandleStandardArgs)
 # handle the QUIETLY and REQUIRED arguments and set LIBCARES_FOUND to TRUE
 # if all listed variables are TRUE and the requested version matches.

I had to remove the version check, since the version header doesn't exist until after the build.

If we update nghttp2 to do the following:

1. We update c-ares minimum version to be 1.15 (or higher)
2. We try find_package(c-ares CONFIG) first, and if that fails, we fallback to find_package(Libcares)
3. If find_package(c-ares CONFIG) was successful, we set the same variables to match the module mode.

Then when building a project with CMake >= 3.24, find_package() calls will simply redirect to the existing content fetched.

[github-actions]

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days.

[anthonyalayo]

Comment -- I'll go after this

[tatsuhiro-t]

#2137 requires c-ares >= 1.16.0 for ares_getaddrinfo.
Previously we used ares_gethostbyname but it has been deprecated, and the replacement is ares_getaddrinfo which has been added since 1.16.0.

[anthonyalayo]

Great news -- I'll put in the CMake changes afterwards

[github-actions]

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days.