hiredis icon indicating copy to clipboard operation
hiredis copied to clipboard

CMake FetchContent support

Open ivan-ushakov opened this issue 1 year ago • 7 comments

Is it possible to make FetchContent support? If I use this declaration:

FetchContent_Declare(
  hiredis
  GIT_REPOSITORY https://github.com/redis/hiredis
  GIT_TAG origin/master
  GIT_SHALLOW TRUE
)

set(DISABLE_TESTS OFF)

FetchContent_MakeAvailable(hiredis)

I get header files inside _deps/hiredis-src, not include/hiredis/ and this leads to build error for Redis++ library.

ivan-ushakov avatar Sep 19 '22 18:09 ivan-ushakov

Hi,

I'm honestly not sure what the right way to get this to work is. It looks like using _deps/<project>-src is just what CMake does by default.

Here's a blog post saying as much.

I suppose you would need to somehow forward the custom include directory to redisplusplus, but I'm not familiar enough with CMake to tell you how 😅

Edit: Looks like you can tell redisplusplus where to look for hiredis with -DCMAKE_PREFIX_PATH=/path/to/hiredis, so perhaps you can set that after fetching and before trying to build redisplusplus.

michael-grunder avatar Sep 19 '22 19:09 michael-grunder

@ivan-ushakov I tried this out and got hiredis and redis++ to build using following gist.

I found two issues during this:

  • I needed to disable building tests in redis++ as well due to its use of find_library(hiredis)
  • redis++ seems to export a different include path than expected.

The tests in redis++ uses find_library(hiredis) instead of find_package(hiredis) and I believe FetchContent only works well with find_package(). By disabling the building of tests the cmake generation works. To fix the exported include path I had to include <redis++.h> instead of <sw/redis++/redis++.h> in main.cc. I believe both are possible issues in redis++.

Would the setup from the gist work for you?

bjosv avatar Sep 22 '22 09:09 bjosv

Sorry! The gist above might not work after all.. I found out that I had hiredis installed in an additional non-default/weird place and CMake actually found that when building.. back to the drawing board..

bjosv avatar Sep 22 '22 12:09 bjosv

Updated the gist to make it work, but requires CMake 3.24. I had to to jump through hoops, so there might be a nicer solution..

bjosv avatar Sep 22 '22 13:09 bjosv

Updated the gist to make it work, but requires CMake 3.24. I had to to jump through hoops, so there might be a nicer solution..

With this two hacks:

SOURCE_DIR _deps/hiredis

# Set include directory to enable redis++ to find it
include_directories(${CMAKE_BINARY_DIR}/_deps)

It works, but maybe in future this could be solved without hacks.

ivan-ushakov avatar Sep 22 '22 15:09 ivan-ushakov