Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CMake FetchContent support #1119

Open
ivan-ushakov opened this issue Sep 19, 2022 · 7 comments
Open

CMake FetchContent support #1119

ivan-ushakov opened this issue Sep 19, 2022 · 7 comments
Labels

Comments

@ivan-ushakov
Copy link

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.

@michael-grunder
Copy link
Collaborator

michael-grunder commented Sep 19, 2022

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.

@bjosv
Copy link
Contributor

bjosv commented Sep 22, 2022

@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
Copy link
Contributor

bjosv commented Sep 22, 2022

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
Copy link
Contributor

bjosv commented Sep 22, 2022

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..

@ivan-ushakov
Copy link
Author

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.

@cwaldren
Copy link

Thank you for the gist.

A note for people searching: redis-plus-plus's 1.3.8 release breaks this workaround by introducing a feature test using CheckSymbolExists ("Support keepalive with customized interval" changelog entry.)

It seems that the hiredis::hiredis alias isn't available for the macro to use. Haven't found a solution yet.

@vrince
Copy link

vrince commented Apr 30, 2024

Worked for me out of the box (the same way I FetchContent other projects):

# hiredis
include(FetchContent)
FetchContent_Declare(hiredis 
  GIT_REPOSITORY https://github.com/redis/hiredis.git 
  GIT_TAG v1.2.0 
  GIT_SHALLOW TRUE 
  GIT_PROGRESS TRUE)

if(NOT hiredis)
    FetchContent_Populate(hiredis)
    add_subdirectory(${hiredis_SOURCE_DIR} ${hiredis_BINARY_DIR} EXCLUDE_FROM_ALL)
endif()

Then later you can use hiredis with:

target_link_libraries(your-target PRIVATE hiredis)
...

Cmake v 3.22.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants