Please stop using include(FindXXX) in cmake

[cloudhan]

include(FindProtobuf)
include(FindThreads)

In practice they should be

find_package(Protobuf REQUIRED)
find_package(Threads REQUIRED)

these include thing caused many integration problem. find_package have many special handling of search path, which will let users have fine-grind control over where to find the package.

[zyearn]

what problem did you met in what situation? how did you install your dependencies?

[cloudhan]

The annoying thing happens when you want to use precompiled protobuf library. In the current situation, you are using some internal variable in FindProtobuf.cmake. It makes the down stream cmake file extremely ugly. If you switch to find_package, I can simply manipulate CMAKE_PREFIX_PATH or set Protobuf_DIR to find the correct library. Just a suggestion.

[cloudhan]

OK, to be more specific, what I have done is

cmake_minimum_required(VERSION 3.5)
project(server CXX)

#...

# ${CMAKE_CURRENT_SOURCE_DIR}/cmake has a FindProtobuf.cmake copied from brpc
set(CMAKE_MODULE_PATH ${CMAKE_CURRENT_SOURCE_DIR}/cmake ${CMAKE_MODULE_PATH})

# this is the precompiled protobuf library
set(CMAKE_PREFIX_PATH ${CMAKE_CURRENT_SOURCE_DIR}/3rdparty/protobuf CMAKE_PREFIX_PATH)

# this will use the copied FindProtobuf.cmake
find_package(Protobuf REQUIRED)

# this is for brpc to correctly find 
include_directories(${Protobuf_INCLUDE_DIR}) 
link_directories(${Protobuf_LIBRARY_DIR})

protobuf_generate_cpp(PROTO_SRC PROTO_HEADER proto/XXX.proto)

# brpc is added as a git submodule
add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/3rdparty/brpc)

# ...

target_link_libraries(my_target PRIVATE brpc-shared)

[zyearn]

include(FindProtobuf) will include the cmake/Modules/FindProtobuf.cmake file, which uses find_library to find protolib. While find_library does support CMAKE_PREFIX_PATH, it is enough to set CMAKE_PREFIX_PATH to your precompiled path. In fact, one of our project also uses precompiled protobuf and we set CMAKE_PREFIX_PATH. Under it there is an include/ and lib/ directory. So make sure your precompiled path has the same structure, more detail: https://cmake.org/cmake/help/latest/variable/CMAKE_PREFIX_PATH.html

[zyearn]

But find_package provides more options for user, so we may replace include with find_package. The pr is here https://github.com/apache/incubator-brpc/pull/1032

[cloudhan]

Since #1032 is merged, I think this issue can be closed for now.