Skip to content

Update libuv find & link configuration to support both static and sha…#668

Merged
sewenew merged 3 commits intosewenew:masterfrom
feihongmeilian:master
Jan 7, 2026
Merged

Update libuv find & link configuration to support both static and sha…#668
sewenew merged 3 commits intosewenew:masterfrom
feihongmeilian:master

Conversation

@feihongmeilian
Copy link
Contributor

The libuv library lookup can be explicitly specified by setting the libuv_DIR CMake variable to the directory containing the libuv's CMake config file (e.g. <libuv_install_prefix>/lib/cmake/libuv), which allows flexible custom selection of the libuv version/path for static & shared library linkage compatibility.

CMakeLists.txt Outdated
# libuv dependency
find_path(REDIS_PLUS_PLUS_ASYNC_LIB_HEADER NAMES uv.h)
find_library(REDIS_PLUS_PLUS_ASYNC_LIB uv)
find_package(libuv REQUIRED CONFIG)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not work, if uv is not installed with cmake. Could you please change the code to make it compatible with both cases, e.g. find_package and find_path. You can take hiredis dependency for an example:

find_package(lib QUIET)
if (NOT libuv_FOUND)
  find_path(REDIS_PLUS_PLUS_ASYNC_LIB_HEADER NAMES uv.h)
  find_library(REDIS_PLUS_PLUS_ASYNC_LIB uv)
endif()

CMakeLists.txt Outdated
if(REDIS_PLUS_PLUS_BUILD_ASYNC)
target_include_directories(${STATIC_LIB} PUBLIC $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/${REDIS_PLUS_PLUS_ASYNC_FUTURE_HEADER}>)
target_include_directories(${STATIC_LIB} PUBLIC $<BUILD_INTERFACE:${REDIS_PLUS_PLUS_ASYNC_LIB_HEADER}>)
target_link_libraries(${STATIC_LIB} PUBLIC $<TARGET_NAME:libuv::uv>)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if libuv_FOUND
  // new code
else()
  // old code
endif()

CMakeLists.txt Outdated
# libuv dependency
find_path(REDIS_PLUS_PLUS_ASYNC_LIB_HEADER NAMES uv.h)
find_library(REDIS_PLUS_PLUS_ASYNC_LIB uv)
find_package(libuv NO_MODULE CONFIG)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like NO_MODULE and CONFIG are synonymous, and you only need to specify one of them?

@sewenew
Copy link
Owner

sewenew commented Jan 6, 2026

@feihongmeilian The code looks good to me, except a concern on the options of find_package. I'm not an expert on cmake. Could you please double check if we need both CONFIG and NO_MODULE options?

Regrads

@feihongmeilian
Copy link
Contributor Author

Confirmed that the NO_MODULE and CONFIG parameters are indeed redundant. I have removed them and verified that the compilation passes locally.

@sewenew sewenew merged commit e46c778 into sewenew:master Jan 7, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants