tgockel / zookeeper-cpp

A ZooKeeper client for C++.
http://tgockel.github.io/zookeeper-cpp/
Apache License 2.0
148 stars 40 forks source link

CMake issue when using this project as dependency #115

Open GuillaumeDua opened 3 years ago

GuillaumeDua commented 3 years ago

In the top-level CMakeLists, line 24 :

list(APPEND CMAKE_MODULE_PATH "${CMAKE_SOURCE_DIR}/cmake/modules/")

Should be instead :

list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake/modules/")

With this quickfix, the current project can be now use with FetchContent or git submodules as a dependency ;)

GuillaumeDua commented 3 years ago

Also, about includes directories, some are missing (still when using this project as dependency using FetchContent/ExternalProject_add/git-submodules/etc.)

Quick fix in the building section : add target_include_directories

################################################################################
# Building                                                                     #
################################################################################

build_module(NAME zkpp-tests
             PATH src/zk/tests
             LINK_LIBRARIES
               ${gtest_LIBRARIES}
            )
target_include_directories(zkpp-tests PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/src) # <--- here

build_module(NAME zkpp
             PATH src/zk
             NO_RECURSE
             LINK_LIBRARIES
               ${zookeeper_LIBRARIES}
            )
target_include_directories(zkpp PUBLIC ${CMAKE_CURRENT_SOURCE_DIR}/src) # <--- here

build_module(NAME zkpp-server
             PATH src/zk/server
             LINK_LIBRARIES
               zkpp
            )
target_include_directories(zkpp-server PUBLIC ${CMAKE_CURRENT_SOURCE_DIR}/src) # <--- here

Also, it might be cleaner to split .cpp and .hpp files. So .cpp files won't be part of the include interface

tgockel commented 3 years ago

I'm going to write some better testing around this.