build: improve c-ares version extraction method

[tchaikov]

this change enhances the version extraction process to handle both literal and macro-based version definitions in c-ares.

in 5d53fb6, we extract the c-ares version by looking for ARES_VERSION_STR and matching the version numbers in it. as, in c-ares 1.32, the line looks like:

  #define ARES_VERSION_STR "1.33.0"

but this method fails for c-ares 1.33+ due to non-literal ARES_VERSION_STR definitionin. as in c-ares 1.33+, it looks like

  #define ARES_VERSION_STR             \
    ARES_STRINGIFY(ARES_VERSION_MAJOR) \
    "." ARES_STRINGIFY(ARES_VERSION_MINOR) "." ARES_STRINGIFY(ARES_VERSION_PATCH)

this fails the parser. and the version number is always empty.

in this change, we match the MAJOR, MINOR and PATCH version components separately. this approach increases resilience. after this change, Seastar is able to detect the versions of both c-ares 1.32 and 1.33.

Refs 5d53fb6
Signed-off-by: Kefu Chai [email protected]

[bradh352]

Out of curiosity, what happens if you just remove your Findc-ares.cmake completely?

c-ares distributes both package config and cmake files when it is installed. And the default cmake FindPackage(c-ares) should automatically set c_ares_VERSION (as well as c_ares_VERSION_MAJOR, c_ares_VERSION_MINOR, and c_ares_VERSION_PATCH).

[tchaikov]

Out of curiosity, what happens if you just remove your Findc-ares.cmake completely?

@bradh352 hi Brad, it depends. on my fedora 41 box, if i remove this file, cmake is able to generate the building system just right, and the bug addressed by this PR goes away. because the maintainer packages the CMake config files in the prebuilt package:

$ rpm -ql c-ares-devel | grep cmake
/usr/lib64/cmake/c-ares
/usr/lib64/cmake/c-ares/c-ares-config-version.cmake
/usr/lib64/cmake/c-ares/c-ares-config.cmake
/usr/lib64/cmake/c-ares/c-ares-targets-noconfig.cmake
/usr/lib64/cmake/c-ares/c-ares-targets.cmake

but unfortunately, the latest debian stable release (bookworm) still does not package them, see https://debian.pkgs.org/12/debian-main-amd64/libc-ares-dev_1.18.1-3_amd64.deb.html .

c-ares distributes both package config and cmake files when it is installed. And the default cmake FindPackage(c-ares) should automatically set c_ares_VERSION (as well as c_ares_VERSION_MAJOR, c_ares_VERSION_MINOR, and c_ares_VERSION_PATCH).

thank you for the excellent CMake integration. but before the popular distros include these files, i think we should at least fall back to searching c-ares in the module mode. even if we can use these two modes when searching for c-ares, i still want to avoid the workaround like

seastar/cmake/SeastarDependencies.cmake

Lines 154 to 168 in 54e25a9

	# workaround for https://gitlab.kitware.com/cmake/cmake/-/issues/25079
	# since protobuf v22.0, it started using abseil, see
	# https://github.com/protocolbuffers/protobuf/releases/tag/v22.0 .
	# but due to https://gitlab.kitware.com/cmake/cmake/-/issues/25079,
	# CMake's FindProtobuf does add this linkage yet. fortunately,
	# ProtobufConfig.cmake provided by protobuf defines this linkage. so we try
	# the CMake package configuration file first, and fall back to CMake's
	# FindProtobuf module.
	find_package (Protobuf QUIET CONFIG)
	if (Protobuf_FOUND AND Protobuf_VERSION VERSION_GREATER_EQUAL 2.5.0)
	# do it again, so the message is printed when the package is found
	find_package(Protobuf CONFIG REQUIRED)
	else ()
	find_package(Protobuf 2.5.0 REQUIRED)
	endif ()

. i think the preferred way to find a package is just to use find_package() without worrying about which mode it uses.

so, IMHO, a simpler approach before debian stable's libc-ares-dev includes the CMake config files is still to keep Findc-ares.cmake around, and to use the module mode.

thank you for pointing this out. i created #2485 to track this cleanup.

[bradh352]

@gjasny since you maintain the debian packaging of c-ares can you look into distributing the cmake files for c-ares on debian? I guess they're missing.

[gjasny]

The version detection should also happen at runtime, not compile time.

At least in Debian your scylladb package won’t get recompiled for a new c-ares package and also older c-ares Debian packages might satisfy the deb package dependency.

Edit: if it’s only the missing header that blocks support, compile time looks right.

[tchaikov]

The version detection should also happen at runtime, not compile time.

hi Gregor , thank you for your comment. my concern is to enable users to build seastar from source on popular distributions, instead of enabling them to use precompiled seastar package like libseastar-dev. so probably we don't need to worry about the runtime behaviour. if, by runtime, you meant "building a seastar application" after libseastar-dev or seastar is installed.

but in the case above, since Findc-ares.cmake is installed along with seastar's cmake config files. i think we should be able to detect both new and old c-ares installations.

At least in Debian your scylladb package won’t get recompiled for a new c-ares package and also older c-ares Debian packages might satisfy the deb package dependency.

ahh, you are talking about "libseastar-dev" then. this package does not exist yet. so...

Edit: if it’s only the missing header that blocks support, compile time looks right.

[tchaikov]

@scylladb/seastar-maint hello maintainers, could you help review this change?