[thirdparty] support import brpc and gtest lib from cmake (#5)

1. support import brpc lib by cmake
2. bump brpc version to 1.8.0
3. bump gtest version to 1.14.0
4. fix compile warnings

.github/workflows/build.yml

@@ -59,17 +59,12 @@ jobs:
         chmod a+x bazel-0.25.2-installer-linux-x86_64.sh
         ./bazel-0.25.2-installer-linux-x86_64.sh --user
         export PATH="$PATH:$HOME/bin"
-    - name: Install brpc
-      working-directory: ${{ github.workspace }}
-      if: ${{ matrix.build_tool == 'cmake' }}
-      run: |
-        mkdir -p thirdparty
-        git clone https://github.com/brpc/brpc.git thirdparty/brpc && cd thirdparty/brpc && mkdir -p bld && cd bld && cmake .. && make -j4 && sudo make install
     - name: Build with cmake
       if: ${{ matrix.build_tool == 'cmake' }}
       run: |
-        cmake -S "${{ github.workspace }}" -B "${{ env.CMAKE_BUILD_DIR }}" -DBUILD_UNIT_TESTS=ON
-        cmake --build "${{ env.CMAKE_BUILD_DIR }}"
+        cmake -S "${{ github.workspace }}" -B "${{ env.CMAKE_BUILD_DIR }}" -DWITH_TESTS=ON
+        cmake --build bld  --parallel 16 -- brpc-static
+        cmake --build "${{ env.CMAKE_BUILD_DIR }}" --parallel 16
     - name: Build with bazel
       if: ${{ matrix.build_tool == 'bazel' }}
       run: |

CMakeLists.txt

@@ -9,6 +9,10 @@ INCLUDE(CPack)
 #option(EXAMPLE_LINK_SO "Whether examples are linked dynamically" OFF)
 option(BRPC_WITH_GLOG "With glog" OFF)
 option(WITH_DEBUG_SYMBOLS "With debug symbols" ON)
+# https://cmake.org/cmake/help/latest/policy/CMP0058.html
+# suppress this  CMP0058 warning
+cmake_policy(SET CMP0058 OLD)
+option(WITH_TESTS "Whether to build unit tests" OFF)
 
 set(WITH_GLOG_VAL "0")
 if(BRPC_WITH_GLOG)
@@ -23,6 +27,11 @@ set(CMAKE_MODULE_PATH ${PROJECT_SOURCE_DIR}/cmake)
 
 include(FindThreads)
 include(FindProtobuf)
+include(thirdparty/brpc/SetupBrpc)
+
+if ((NOT BRPC_INCLUDE_PATH) OR (NOT BRPC_LIB))
+    message(FATAL_ERROR "Fail to find brpc")
+endif()
 
 if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
     # require at least gcc 4.8
@@ -66,12 +75,6 @@ if(LEVELDB_WITH_SNAPPY)
     find_library(SNAPPY_LIB NAMES snappy)
 endif()
 
-find_path(BRPC_INCLUDE_PATH NAMES brpc/server.h)
-find_library(BRPC_LIB NAMES libbrpc.a brpc)
-if ((NOT BRPC_INCLUDE_PATH) OR (NOT BRPC_LIB))
-    message(FATAL_ERROR "Fail to find brpc")
-endif()
-
 if (NOT PROTOBUF_PROTOC_EXECUTABLE)
     get_filename_component(PROTO_LIB_DIR ${PROTOBUF_LIBRARY} DIRECTORY)
     set (PROTOBUF_PROTOC_EXECUTABLE "${PROTO_LIB_DIR}/../bin/protoc")
@@ -226,7 +229,7 @@ endmacro(use_cxx11)
 use_cxx11()
 
 add_subdirectory(src)
-if(BUILD_UNIT_TESTS)
+if(WITH_TESTS)
     add_subdirectory(test)
 endif()
 add_subdirectory(tools)

cmake/thirdparty/brpc/CMakeLists.download_brpc.in

@@ -0,0 +1,31 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+cmake_minimum_required(VERSION 2.8.2)
+
+project(brpc-download NONE)
+
+include(ExternalProject)
+ExternalProject_Add(brpc
+  GIT_REPOSITORY    https://github.com/apache/brpc.git
+  GIT_TAG           1.8.0
+  SOURCE_DIR        "${CMAKE_BINARY_DIR}/_deps/brpc/src"
+  BINARY_DIR        "${CMAKE_BINARY_DIR}/_deps/brpc/build"
+  CONFIGURE_COMMAND ""
+  BUILD_COMMAND     ""
+  INSTALL_COMMAND   ""
+  TEST_COMMAND      ""
+)
+

cmake/thirdparty/brpc/SetupBrpc.cmake

@@ -0,0 +1,41 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# Setup brpc
+configure_file("${PROJECT_SOURCE_DIR}/cmake/thirdparty/brpc/CMakeLists.download_brpc.in" ${PROJECT_BINARY_DIR}/_deps/brpc/CMakeLists.txt)
+
+
+execute_process(COMMAND ${CMAKE_COMMAND} -G "${CMAKE_GENERATOR}" .
+                RESULT_VARIABLE result
+                WORKING_DIRECTORY ${PROJECT_BINARY_DIR}/_deps/brpc)
+
+if(result)
+  message(FATAL_ERROR "CMake step for brpc failed: ${result}")
+endif()
+
+execute_process(COMMAND ${CMAKE_COMMAND} --build .
+                RESULT_VARIABLE result
+                WORKING_DIRECTORY ${PROJECT_BINARY_DIR}/_deps/brpc)
+
+if(result)
+  message(FATAL_ERROR "Build step for brpc failed: ${result}")
+endif()
+
+add_subdirectory(${PROJECT_BINARY_DIR}/_deps/brpc/src
+                 ${PROJECT_BINARY_DIR}/_deps/brpc/build
+                 EXCLUDE_FROM_ALL)
+
+set(BRPC_LIB brpc-static)
+set(BRPC_INCLUDE_PATH  ${PROJECT_BINARY_DIR}/_deps/brpc/build/output/include)

cmake/thirdparty/gtest/CMakeLists.download_gtest.in

@@ -0,0 +1,30 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+cmake_minimum_required(VERSION 2.8.10)
+
+project(googletest NONE)
+
+include(ExternalProject)
+ExternalProject_Add(googletest
+  GIT_REPOSITORY    https://github.com/google/googletest.git
+  GIT_TAG           v1.14.0
+  SOURCE_DIR        "${PROJECT_BINARY_DIR}/_deps/googletest/src"
+  BINARY_DIR        "${PROJECT_BINARY_DIR}/_deps/googletest/build"
+  CONFIGURE_COMMAND ""
+  BUILD_COMMAND     ""
+  INSTALL_COMMAND   ""
+  TEST_COMMAND      ""
+)

cmake/thirdparty/gtest/SetupGtest.cmake

@@ -0,0 +1,42 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# Setup googletest
+configure_file("${PROJECT_SOURCE_DIR}/cmake/thirdparty/gtest/CMakeLists.download_gtest.in" ${PROJECT_BINARY_DIR}/_deps/googletest/CMakeLists.txt)
+
+execute_process(COMMAND ${CMAKE_COMMAND} -G "${CMAKE_GENERATOR}" .
+                RESULT_VARIABLE result
+                WORKING_DIRECTORY ${PROJECT_BINARY_DIR}/_deps/googletest
+)
+if(result)
+  message(FATAL_ERROR "CMake step for googletest failed: ${result}")
+endif()
+
+execute_process(COMMAND ${CMAKE_COMMAND} --build .
+                RESULT_VARIABLE result
+                WORKING_DIRECTORY ${PROJECT_BINARY_DIR}/_deps/googletest
+)
+if(result)
+  message(FATAL_ERROR "Build step for googletest failed: ${result}")
+endif()
+
+set(gtest_force_shared_crt ON CACHE BOOL "" FORCE)
+
+add_subdirectory(${PROJECT_BINARY_DIR}/_deps/googletest/src
+                 ${PROJECT_BINARY_DIR}/_deps/googletest/build
+                 EXCLUDE_FROM_ALL)
+
+set(GTEST_MAIN_LIB gtest_main)
+set(GTEST_LIB gtest)

src/CMakeLists.txt

@@ -1,4 +1,4 @@
-if(BUILD_UNIT_TESTS)
+if(WITH_TESTS)
     set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DUNIT_TEST")
     set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -DUNIT_TEST")
 elseif(NOT DEBUG)

src/braft/file_system_adaptor.cpp

@@ -122,19 +122,21 @@ ssize_t BufferedSequentialWriteFileAdaptor::write(const butil::IOBuf& data, off_
     BRAFT_VLOG << "begin write offset " << offset << ", data_size " << data.size()
               << ", buffer_offset " << _buffer_offset 
               << ", buffer_size " << _buffer_size;
-    if (offset < _buffer_offset + _buffer_size) {
-        LOG(WARNING) << "Fail to write into buffered file adaptor with invalid range"
-                     << ", offset: " << offset
-                     << ", data_size: " << data.size()
-                     << ", buffer_offset: " << _buffer_offset
-                     << ", buffer_size: " << _buffer_size;
-        errno = EINVAL;
-        return -1;
-    } else if (offset > _buffer_offset + _buffer_size) {
-        // passby hole
-        CHECK(_buffer_size == 0);
-        BRAFT_VLOG << "seek to new offset " << offset << " as there is hole";
-        seek(offset);
+    if (static_cast<size_t>(offset) <
+        static_cast<size_t>(_buffer_offset) + _buffer_size) {
+      LOG(WARNING)
+          << "Fail to write into buffered file adaptor with invalid range"
+          << ", offset: " << offset << ", data_size: " << data.size()
+          << ", buffer_offset: " << _buffer_offset
+          << ", buffer_size: " << _buffer_size;
+      errno = EINVAL;
+      return -1;
+    } else if (static_cast<size_t>(offset) >
+               static_cast<size_t>(_buffer_offset) + _buffer_size) {
+      // passby hole
+      CHECK(_buffer_size == 0);
+      BRAFT_VLOG << "seek to new offset " << offset << " as there is hole";
+      seek(offset);
     }
     const size_t saved_size = data.size();
     _buffer.append(data);

src/braft/fsm_caller.cpp

@@ -63,7 +63,7 @@ int FSMCaller::run(void* meta, bthread::TaskIterator<ApplyTask>& iter) {
         return 0;
     }
     int64_t max_committed_index = -1;
-    int64_t counter = 0;
+    size_t counter = 0;
     size_t  batch_size = FLAGS_raft_fsm_caller_commit_batch;
     for (; iter; ++iter) {
         if (iter->type == COMMITTED && counter < batch_size) {

src/braft/snapshot.cpp

@@ -785,7 +785,7 @@ void LocalSnapshotCopier::copy() {
         LOG(WARNING) << "Fail to copy, error_code " << error_code()
                      << " error_msg " << error_cstr() 
                      << " writer path " << _writer->get_path();
-        _writer->set_error(error_code(), error_cstr());
+        _writer->set_error(error_code(), "%s", error_cstr());
     }
     if (_writer) {
         // set_error for copier only when failed to close writer and copier was 
@@ -818,7 +818,8 @@ void LocalSnapshotCopier::load_meta_table() {
     lck.unlock();
     if (!session->status().ok()) {
         LOG(WARNING) << "Fail to copy meta file : " << session->status();
-        set_error(session->status().error_code(), session->status().error_cstr());
+        set_error(session->status().error_code(), "%s",
+                  session->status().error_cstr());
         return;
     }
     if (_remote_snapshot._meta_table.load_from_iobuf_as_remote(meta_buf) != 0) {
@@ -997,8 +998,9 @@ void LocalSnapshotCopier::copy_file(const std::string& filename) {
     _cur_session = NULL;
     lck.unlock();
     if (!session->status().ok()) {
-        set_error(session->status().error_code(), session->status().error_cstr());
-        return;
+      set_error(session->status().error_code(), "%s",
+                session->status().error_cstr());
+      return;
     }
     if (_writer->add_file(filename, &meta) != 0) {
         set_error(EIO, "Fail to add file to writer");

src/braft/snapshot_executor.cpp

@@ -154,6 +154,7 @@ void SnapshotExecutor::do_snapshot(Closure* done) {
         LOG_IF(INFO, _node != NULL) << "node " << _node->node_id()
             << " the gap between fsm applied index " << saved_fsm_applied_index
             << " and last_snapshot_index " << saved_last_snapshot_index
+            << ", last_snapshot_term " << saved_last_snapshot_term 
             << " is less than " << FLAGS_raft_do_snapshot_min_index_gap
             << ", will clear bufferred logs and return success";
 

test/CMakeLists.txt

@@ -1,12 +1,10 @@
 find_package(Gperftools)
+include(thirdparty/gtest/SetupGtest)
+
 include_directories(${GPERFTOOLS_INCLUDE_DIR})
 include_directories(${CMAKE_CURRENT_BINARY_DIR})
 include_directories(${CMAKE_SOURCE_DIR}/test)
 
-find_path(GTEST_HEADER NAMES gtest/gtest.h)
-find_library(GTEST_LIB NAMES gtest)
-find_library(GTEST_MAIN_LIB NAMES gtest_main)
-
 set(CMAKE_CPP_FLAGS "-DGFLAGS_NS=${GFLAGS_NS}")
 set(CMAKE_CPP_FLAGS "${CMAKE_CPP_FLAGS} -D__const__=__unused__ -D_GNU_SOURCE -DUSE_SYMBOLIZE -DNO_TCMALLOC -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -D__STDC_CONSTANT_MACROS -DUNIT_TEST -g -Dprivate=public -Dprotected=public -D__STRICT_ANSI__ -include sstream_workaround.h")
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${CMAKE_CPP_FLAGS} -O2 -pipe -Wall -W -fPIC -fstrict-aliasing -Wno-invalid-offsetof -Wno-unused-parameter -fno-omit-frame-pointer -Wno-unused-result")
@@ -27,11 +25,9 @@ foreach(BRAFT_UT ${TEST_BRAFT_SRCS})
     add_executable(${BRAFT_UT_WE} ${BRAFT_UT} 
                    $<TARGET_OBJECTS:OBJ_LIB>)
     target_link_libraries(${BRAFT_UT_WE}
-        #"-Xlinker \"-(\""
                           ${GTEST_MAIN_LIB}
                           ${GTEST_LIB}
                           ${GPERFTOOLS_LIBRARY}
                           ${DYNAMIC_LIB}
-                          #                  "-Xlinker \"-)\""
                           )
 endforeach()

test/test_node.cpp

@@ -3684,11 +3684,11 @@ TEST_P(NodeTest, readonly) {
     cluster.stop_all();
 }
 
-INSTANTIATE_TEST_CASE_P(NodeTestWithoutPipelineReplication,
+INSTANTIATE_TEST_SUITE_P(NodeTestWithoutPipelineReplication,
                         NodeTest,
                         ::testing::Values("NoReplcation"));
 
-INSTANTIATE_TEST_CASE_P(NodeTestWithPipelineReplication,
+INSTANTIATE_TEST_SUITE_P(NodeTestWithPipelineReplication,
                         NodeTest,
                         ::testing::Values("NoCache", "HasCache"));
 

test/util.h

@@ -83,7 +83,8 @@ class MockFSM : public braft::StateMachine {
             _on_leader_start_closure = NULL;
         }
     }
-    void on_leader_stop(const braft::LeaderChangeContext&) {
+
+    void on_leader_stop(const butil::Status&) {
         _leader_term = -1;
     }