From 7b54daebf20581263896e493f2d5d054e7eb723b Mon Sep 17 00:00:00 2001 From: Dongsheng He Date: Wed, 10 Apr 2024 15:37:29 +0800 Subject: [PATCH] [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 | 11 ++--- CMakeLists.txt | 17 ++++---- .../brpc/CMakeLists.download_brpc.in | 31 ++++++++++++++ cmake/thirdparty/brpc/SetupBrpc.cmake | 41 ++++++++++++++++++ .../gtest/CMakeLists.download_gtest.in | 30 +++++++++++++ cmake/thirdparty/gtest/SetupGtest.cmake | 42 +++++++++++++++++++ src/CMakeLists.txt | 2 +- src/braft/file_system_adaptor.cpp | 28 +++++++------ src/braft/fsm_caller.cpp | 2 +- src/braft/snapshot.cpp | 10 +++-- src/braft/snapshot_executor.cpp | 1 + test/CMakeLists.txt | 8 +--- test/test_node.cpp | 4 +- test/util.h | 3 +- 14 files changed, 187 insertions(+), 43 deletions(-) create mode 100644 cmake/thirdparty/brpc/CMakeLists.download_brpc.in create mode 100644 cmake/thirdparty/brpc/SetupBrpc.cmake create mode 100644 cmake/thirdparty/gtest/CMakeLists.download_gtest.in create mode 100644 cmake/thirdparty/gtest/SetupGtest.cmake diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 757a18e9..5466bdc1 100644 --- a/.github/workflows/build.yml +++ b/.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: | diff --git a/CMakeLists.txt b/CMakeLists.txt index 110daa30..d90c6614 100644 --- a/CMakeLists.txt +++ b/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) diff --git a/cmake/thirdparty/brpc/CMakeLists.download_brpc.in b/cmake/thirdparty/brpc/CMakeLists.download_brpc.in new file mode 100644 index 00000000..d2f0790c --- /dev/null +++ b/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 "" +) + diff --git a/cmake/thirdparty/brpc/SetupBrpc.cmake b/cmake/thirdparty/brpc/SetupBrpc.cmake new file mode 100644 index 00000000..a2968e4e --- /dev/null +++ b/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) diff --git a/cmake/thirdparty/gtest/CMakeLists.download_gtest.in b/cmake/thirdparty/gtest/CMakeLists.download_gtest.in new file mode 100644 index 00000000..abb00581 --- /dev/null +++ b/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 "" +) diff --git a/cmake/thirdparty/gtest/SetupGtest.cmake b/cmake/thirdparty/gtest/SetupGtest.cmake new file mode 100644 index 00000000..3a96b89c --- /dev/null +++ b/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) \ No newline at end of file diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index f587464a..59c8498a 100644 --- a/src/CMakeLists.txt +++ b/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) diff --git a/src/braft/file_system_adaptor.cpp b/src/braft/file_system_adaptor.cpp index 8509544e..ac163d25 100644 --- a/src/braft/file_system_adaptor.cpp +++ b/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(offset) < + static_cast(_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(offset) > + static_cast(_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); diff --git a/src/braft/fsm_caller.cpp b/src/braft/fsm_caller.cpp index 98913eea..a0b68019 100644 --- a/src/braft/fsm_caller.cpp +++ b/src/braft/fsm_caller.cpp @@ -63,7 +63,7 @@ int FSMCaller::run(void* meta, bthread::TaskIterator& 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) { diff --git a/src/braft/snapshot.cpp b/src/braft/snapshot.cpp index 7a97132a..91a8385b 100644 --- a/src/braft/snapshot.cpp +++ b/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"); diff --git a/src/braft/snapshot_executor.cpp b/src/braft/snapshot_executor.cpp index e33ece86..d0227ce8 100644 --- a/src/braft/snapshot_executor.cpp +++ b/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"; diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 2adb666f..4c5e67f3 100644 --- a/test/CMakeLists.txt +++ b/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_link_libraries(${BRAFT_UT_WE} - #"-Xlinker \"-(\"" ${GTEST_MAIN_LIB} ${GTEST_LIB} ${GPERFTOOLS_LIBRARY} ${DYNAMIC_LIB} - # "-Xlinker \"-)\"" ) endforeach() diff --git a/test/test_node.cpp b/test/test_node.cpp index c0739cf2..10cc01ac 100644 --- a/test/test_node.cpp +++ b/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")); diff --git a/test/util.h b/test/util.h index bb38d150..4730fbdb 100644 --- a/test/util.h +++ b/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; }