WIP: Add cmake add_subdirectory support

[langep]

Hi,

I am currently working on a project an want to use redis-plus-plus. I am using cmake and want to setup redis-plus-plus as a submodule in my repository and then include it with add_subdirectory in my CMakeLists.txt.

The additions I made:

• Prefix both static and shared library target names with 'redis-plus-plus' so my project can use them inside of target_link_libraries with a useful name.
• Added both redis-plus-plus and hiredis include directories to include_directories.
• .devcontainer/*: I am working on Windows, these files are used by Visual Studio Code's Remote Containers extension to develop inside a Docker container that sets up a functional development environment.

If you are interested in these changes, I can also update the documentation.

You need to clone the repository with the --recursive flag to get the hiredis dependency.
If you have the repository already clone you probably need to run git submodule update --init --recursive

[sewenew]

Hi @langep

Sorry for the late reply. These days I'm busy working on a new open source project.

I'm not quite sure if these changes have some side effect, e.g. conflict with pre-installed hiredis. I'll do some research and review your changes latter.

Regards

[sewenew]

Hi @langep

I tried the git submodule commands. It's great, and makes it easier to manage the dependency. However, when I run make install, it will also overwrite the pre-installed hiredis.

In some of my environments, I cannot overwrite the pre-installed hiredis, since some other applications depend on some specific hiredis version.

So I cannot merge your change into the master branch by now. Sorry for that. I might try that again in the future, when I can overwrite the pre-installed hiredis.

Prefix both static and shared library target names with 'redis-plus-plus'

This is a good idea. I'll modified CMakeLists.txt to add the prefix.

Also, I'll pending the devcontainer stuff, until I learn more about Visual Studio Code's Remote Containers extension.

Sorry again for can't merge the change, and thanks for your pull request :)

Regards

[langep]

@sewenew Thank you for reviewing. I tried to make building from a submodule optional. I also made building the tests optional. The defaults reflect your original code i.e. if you don't change the options it will search for an installed hiredis and build the tests. I also removed the VS Code files from the PR.

When you have time, can you try if it still works for your use cases?

[langep]

If you want to build redis-plus-plus from another project (using the hiredis submodule) you can include the following inside the other project's CMakeLists.txt file.

set(REDIS_PLUS_PLUS_BUILD_HIREDIS_FROM_SUBMODULE ON CACHE BOOL "Build hiredis dependency from submodule instead of system")
set(REDIS_PLUS_PLUS_BUILD_TESTS OFF CACHE BOOL "Build redis-plus-plus tests")
add_subdirectory("${PROJECT_SOURCE_DIR}/third_party/redis-plus-plus" "third_party/redis-plus-plus")

[sewenew]

This seems great! I'll test on multiple environment ASAP. Also can you remove the changes on the .gitignore file? This seems specific to your building environment.

Thank you very much for this pull request!

[sewenew]

Hi @langep

I tried your CMakeLists.txt. However, it failed to build the test on Centos 7 GCC 4.8.5, with REDIS_PLUS_PLUS_BUILD_HIREDIS_FROM_SUBMODULE OFF. The following is the error message:

/usr/bin/ld: cannot find -lhiredis

It fails to locate libhiredis. I also tried to add the following code in test/CMakeLists.txt, but it still has the same problem:

if (REDIS_PLUS_PLUS_BUILD_HIREDIS_FROM_SUBMODULE)
    target_link_libraries(${PROJECT_NAME} libhiredis.a)
else()
    find_library(HIREDIS_STATIC_LIB libhiredis.a)
    target_link_libraries(${PROJECT_NAME} ${HIREDIS_STATIC_LIB})
endif()

[wingunder]

Hi,

I patched a current master branch (3ba84a8) with @langep's patches and tried the following on a Debian 'Bullseye' box.

Without running git submodule update --init --recursive, ie. the redis-plus-plus/hiredis/ dir is completely empty, and having the hiredis dependency turned off, like this:

option(REDIS_PLUS_PLUS_BUILD_HIREDIS_FROM_SUBMODULE "Build hiredis dependency from submodule instead of system" OFF)

The compilation worked perfectly and it linked correctly:

$ ldd ./test/test_redis++ |grep hiredis
        libhiredis.so.0.14 => /lib/x86_64-linux-gnu/libhiredis.so.0.14 (0x00007fb7556fd000)

After running git submodule update --init --recursive, ie. the redis-plus-plus/hiredis/ dir cloned and populated, and setting the hiredis dependency to on, like this:

option(REDIS_PLUS_PLUS_BUILD_HIREDIS_FROM_SUBMODULE "Build hiredis dependency from submodule instead of system" ON)

The compilation worked perfectly and it linked correctly:

$ ldd ./test/test_redis++ |grep hiredis
   libhiredis.so.0.14 => /???/redis-plus-plus/compile/hiredis/libhiredis.so.0.14 (0x00007ff943694000)

I have to stress that I had to do the following, every time I changed the hiredis dependency flag:

$ rm -rf compile
$ mkdir compile
$ cd compile
$ cmake -DCMAKE_BUILD_TYPE=Release ..

Version info:

$ cmake --version
cmake version 3.13.4

CMake suite maintained and supported by Kitware (kitware.com/cmake).

$ /usr/bin/c++ --version
c++ (Debian 9.2.1-8) 9.2.1 20190909
Copyright (C) 2019 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

I'm not a cmake guru and I only have GNU Linux boxes, so this is about as far as I can help with this one.

@sewenew, Could you maybe try to check your cmake version and try the trick of using a fresh compile directory every time you change the hiredis dependency flag?

Regards

[sewenew]

Hi @wingunder

I just tried @langep 's patch again with 3 different compilers, and the result is interesting.

• Apple clang version 11.0.0 (clang-1100.0.33.12) + cmake version 3.10.1
• gcc version 4.8.5 20150623 (Red Hat 4.8.5-39) (GCC) + cmake version 2.8.12.2
• gcc version 7.4.0 (Ubuntu 7.4.0-1ubuntu1~18.04.1) + cmake version 3.10.2

The first 2 compilers failed to compile the test code with REDIS_PLUS_PLUS_BUILD_HIREDIS_FROM_SUBMODULE OFF, i.e. without running git submodule update --init --recursive, and the compilers complain that they cannot find -lhiredis.

However, GCC 7.4.0 could compile the test code perfectly, and passed all tests.

It seems that some old compilers cannot find the hiredis dependency correctly. Maybe I need to modify the CMakeLists.txt. I'll keep trying in the future.

Thanks for your work!

Regards

[wingunder]

Hi @sewenew,
Here's another one for the statistics:

• g++ (Ubuntu 5.4.0-6ubuntu1~16.04.12) 5.4.0 20160609, cmake version 3.5.1

Compiled OK with and without the submodule.
Regards

[sewenew]

Hi @wingunder

Thanks for the info. I'll let you know, when I have progress on this.

Regards