diff --git a/cpp/CHANGELOG.md b/cpp/CHANGELOG.md index 820b16756b66b98d72542d40f46db9a9be76ea05..6dc2369b47ec0d1f6f79fe0591bd3394823e5301 100644 --- a/cpp/CHANGELOG.md +++ b/cpp/CHANGELOG.md @@ -7,6 +7,7 @@ Please mark all change in change log and use the ticket from JIRA. ## Bug - MS-568 - Fix gpuresource free error - MS-572 - Milvus crash when get SIGINT +- MS-577 - Unittest Query randomly hung ## Improvement - MS-552 - Add and change the easylogging library @@ -25,6 +26,7 @@ Please mark all change in change log and use the ticket from JIRA. - MS-561 - Add contributing guidelines, code of conduct and README docs - MS-567 - Add NOTICE.md - MS-569 - Complete the NOTICE.md +- MS-575 - Add Clang-format & Clang-tidy & Cpplint # Milvus 0.4.0 (2019-09-12) diff --git a/cpp/CMakeLists.txt b/cpp/CMakeLists.txt index bb15ddf075f780be799bb1dbc49e820c7c0d27aa..01007bf08538c24366f5725111dfeac9cd664326 100644 --- a/cpp/CMakeLists.txt +++ b/cpp/CMakeLists.txt @@ -21,6 +21,8 @@ cmake_minimum_required(VERSION 3.14) message(STATUS "Building using CMake version: ${CMAKE_VERSION}") +set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} "${CMAKE_CURRENT_SOURCE_DIR}/cmake") + MACRO (GET_CURRENT_TIME CURRENT_TIME) execute_process(COMMAND "date" +"%Y-%m-%d %H:%M.%S" OUTPUT_VARIABLE ${CURRENT_TIME}) ENDMACRO (GET_CURRENT_TIME) @@ -41,6 +43,10 @@ endif() set(MILVUS_VERSION "${GIT_BRANCH_NAME}") string(REGEX MATCH "[0-9]+\\.[0-9]+\\.[0-9]" MILVUS_VERSION "${MILVUS_VERSION}") +set(CLANG_FORMAT_VERSION "6.0") +find_package(ClangTools) +set(BUILD_SUPPORT_DIR "${CMAKE_SOURCE_DIR}/build-support") + if(CMAKE_BUILD_TYPE STREQUAL "Release") set(BUILD_TYPE "Release") else() @@ -111,8 +117,6 @@ else() include_directories(${MYSQL_INCLUDE_DIR}) endif() -set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} "${CMAKE_CURRENT_SOURCE_DIR}/cmake") - set(MILVUS_SOURCE_DIR ${PROJECT_SOURCE_DIR}) set(MILVUS_BINARY_DIR ${PROJECT_BINARY_DIR}) set(MILVUS_ENGINE_SRC ${PROJECT_SOURCE_DIR}/src) @@ -152,3 +156,9 @@ install(FILES conf/log_config.conf DESTINATION conf) + +add_custom_target(format ${BUILD_SUPPORT_DIR}/run_clang_format.py + ${CLANG_FORMAT_BIN} + ${BUILD_SUPPORT_DIR}/clang_format_exclusions.txt + ${CMAKE_CURRENT_SOURCE_DIR}/src) + diff --git a/cpp/build-support/clang_format_exclusions.txt b/cpp/build-support/clang_format_exclusions.txt new file mode 100644 index 0000000000000000000000000000000000000000..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 diff --git a/cpp/build-support/run_clang_format.py b/cpp/build-support/run_clang_format.py new file mode 100755 index 0000000000000000000000000000000000000000..4235b2d4af6281a64ab358c3265809004e3a56be --- /dev/null +++ b/cpp/build-support/run_clang_format.py @@ -0,0 +1,79 @@ +#!/usr/bin/env python +# 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. + +import fnmatch +import os +import subprocess +import sys + +if len(sys.argv) < 4: + sys.stderr.write("Usage: %s $CLANG_FORMAT_VERSION exclude_globs.txt " + "$source_dir\n" % + sys.argv[0]) + sys.exit(1) + +CLANG_FORMAT = sys.argv[1] +EXCLUDE_GLOBS_FILENAME = sys.argv[2] +SOURCE_DIR = sys.argv[3] + +if len(sys.argv) > 4: + CHECK_FORMAT = int(sys.argv[4]) == 1 +else: + CHECK_FORMAT = False + + +exclude_globs = [line.strip() for line in open(EXCLUDE_GLOBS_FILENAME, "r")] + +files_to_format = [] +matches = [] +for directory, subdirs, files in os.walk(SOURCE_DIR): + for name in files: + name = os.path.join(directory, name) + if not (name.endswith('.h') or name.endswith('.cpp') or name.endswith('.cuh') or name.endswith('.cu')): + continue + + excluded = False + for g in exclude_globs: + if fnmatch.fnmatch(name, g): + excluded = True + break + if not excluded: + files_to_format.append(name) + +if CHECK_FORMAT: + output = subprocess.check_output([CLANG_FORMAT, '-output-replacements-xml'] + + files_to_format, + stderr=subprocess.STDOUT).decode('utf8') + + to_fix = [] + for line in output.split('\n'): + if 'offset' in line: + to_fix.append(line) + + if len(to_fix) > 0: + print("clang-format checks failed, run 'make format' to fix") + sys.exit(-1) +else: + try: + cmd = [CLANG_FORMAT, '-i'] + files_to_format + subprocess.check_output(cmd, stderr=subprocess.STDOUT) + except Exception as e: + print(e) + print(' '.join(cmd)) + raise + diff --git a/cpp/cmake/FindClangTools.cmake b/cpp/cmake/FindClangTools.cmake new file mode 100644 index 0000000000000000000000000000000000000000..075d9381c50d2c04e03f0aa0619a31555112fad5 --- /dev/null +++ b/cpp/cmake/FindClangTools.cmake @@ -0,0 +1,104 @@ +# +# Licensed 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. +# +# Tries to find the clang-tidy and clang-format modules +# +# Usage of this module as follows: +# +# find_package(ClangTools) +# +# Variables used by this module, they can change the default behaviour and need +# to be set before calling find_package: +# +# ClangToolsBin_HOME - +# When set, this path is inspected instead of standard library binary locations +# to find clang-tidy and clang-format +# +# This module defines +# CLANG_TIDY_BIN, The path to the clang tidy binary +# CLANG_TIDY_FOUND, Whether clang tidy was found +# CLANG_FORMAT_BIN, The path to the clang format binary +# CLANG_TIDY_FOUND, Whether clang format was found + +find_program(CLANG_TIDY_BIN + NAMES + clang-tidy-6.0 + clang-tidy-5.0 + clang-tidy-4.0 + clang-tidy-3.9 + clang-tidy-3.8 + clang-tidy-3.7 + clang-tidy-3.6 + clang-tidy + PATHS ${ClangTools_PATH} $ENV{CLANG_TOOLS_PATH} /usr/local/bin /usr/bin + NO_DEFAULT_PATH +) + +if ( "${CLANG_TIDY_BIN}" STREQUAL "CLANG_TIDY_BIN-NOTFOUND" ) + set(CLANG_TIDY_FOUND 0) + message("clang-tidy not found") +else() + set(CLANG_TIDY_FOUND 1) + message("clang-tidy found at ${CLANG_TIDY_BIN}") +endif() + +if (CLANG_FORMAT_VERSION) + find_program(CLANG_FORMAT_BIN + NAMES clang-format-${CLANG_FORMAT_VERSION} + PATHS + ${ClangTools_PATH} + $ENV{CLANG_TOOLS_PATH} + /usr/local/bin /usr/bin + NO_DEFAULT_PATH + ) + + # If not found yet, search alternative locations + if (("${CLANG_FORMAT_BIN}" STREQUAL "CLANG_FORMAT_BIN-NOTFOUND") AND APPLE) + # Homebrew ships older LLVM versions in /usr/local/opt/llvm@version/ + STRING(REGEX REPLACE "^([0-9]+)\\.[0-9]+" "\\1" CLANG_FORMAT_MAJOR_VERSION "${CLANG_FORMAT_VERSION}") + STRING(REGEX REPLACE "^[0-9]+\\.([0-9]+)" "\\1" CLANG_FORMAT_MINOR_VERSION "${CLANG_FORMAT_VERSION}") + if ("${CLANG_FORMAT_MINOR_VERSION}" STREQUAL "0") + find_program(CLANG_FORMAT_BIN + NAMES clang-format + PATHS /usr/local/opt/llvm@${CLANG_FORMAT_MAJOR_VERSION}/bin + NO_DEFAULT_PATH + ) + else() + find_program(CLANG_FORMAT_BIN + NAMES clang-format + PATHS /usr/local/opt/llvm@${CLANG_FORMAT_VERSION}/bin + NO_DEFAULT_PATH + ) + endif() + endif() +else() + find_program(CLANG_FORMAT_BIN + NAMES clang-format-4.0 + clang-format-3.9 + clang-format-3.8 + clang-format-3.7 + clang-format-3.6 + clang-format + PATHS ${ClangTools_PATH} $ENV{CLANG_TOOLS_PATH} /usr/local/bin /usr/bin + NO_DEFAULT_PATH + ) +endif() + +if ( "${CLANG_FORMAT_BIN}" STREQUAL "CLANG_FORMAT_BIN-NOTFOUND" ) + set(CLANG_FORMAT_FOUND 0) + message("clang-format not found") +else() + set(CLANG_FORMAT_FOUND 1) + message("clang-format found at ${CLANG_FORMAT_BIN}") +endif() + diff --git a/cpp/src/scheduler/JobMgr.cpp b/cpp/src/scheduler/JobMgr.cpp index c5c14c1b414cfb2843304d5cbfd3ed088c0e0494..31db4b344ef35bad3e5ecd5e020f1eb91029afac 100644 --- a/cpp/src/scheduler/JobMgr.cpp +++ b/cpp/src/scheduler/JobMgr.cpp @@ -32,8 +32,8 @@ JobMgr::JobMgr(ResourceMgrPtr res_mgr) void JobMgr::Start() { if (not running_) { - worker_thread_ = std::thread(&JobMgr::worker_function, this); running_ = true; + worker_thread_ = std::thread(&JobMgr::worker_function, this); } } diff --git a/cpp/unittest/server/rpc_test.cpp b/cpp/unittest/server/rpc_test.cpp index f89c98eb9dcd98f3d1394796d8b01a85f875506b..69b258aa756ac2985391035aa22dcaee54a40b46 100644 --- a/cpp/unittest/server/rpc_test.cpp +++ b/cpp/unittest/server/rpc_test.cpp @@ -62,6 +62,7 @@ class RpcHandlerTest : public testing::Test { res_mgr->Connect("cpu", "gtx1660", PCIE); res_mgr->Start(); engine::SchedInst::GetInstance()->Start(); + engine::JobMgrInst::GetInstance()->Start(); engine::DBOptions opt; @@ -105,6 +106,7 @@ class RpcHandlerTest : public testing::Test { void TearDown() override { server::DBWrapper::GetInstance().StopService(); + engine::JobMgrInst::GetInstance()->Stop(); engine::ResMgrInst::GetInstance()->Stop(); engine::SchedInst::GetInstance()->Stop(); boost::filesystem::remove_all("/tmp/milvus_test"); @@ -146,7 +148,7 @@ std::string CurrentTmDate(int64_t offset_day = 0) { } -TEST_F(RpcHandlerTest, HasTableTest) { +TEST_F(RpcHandlerTest, HAS_TABLE_TEST) { ::grpc::ServerContext context; ::milvus::grpc::TableName request; ::milvus::grpc::BoolReply reply; @@ -158,7 +160,7 @@ TEST_F(RpcHandlerTest, HasTableTest) { ASSERT_EQ(error_code, ::milvus::grpc::ErrorCode::SUCCESS); } -TEST_F(RpcHandlerTest, IndexTest) { +TEST_F(RpcHandlerTest, INDEX_TEST) { ::grpc::ServerContext context; ::milvus::grpc::IndexParam request; ::milvus::grpc::Status response; @@ -194,7 +196,7 @@ TEST_F(RpcHandlerTest, IndexTest) { handler->DropIndex(&context, &table_name, &status); } -TEST_F(RpcHandlerTest, InsertTest) { +TEST_F(RpcHandlerTest, INSERT_TEST) { ::grpc::ServerContext context; ::milvus::grpc::InsertParam request; ::milvus::grpc::Status response; @@ -213,7 +215,7 @@ TEST_F(RpcHandlerTest, InsertTest) { ASSERT_EQ(vector_ids.vector_id_array_size(), VECTOR_COUNT); } -TEST_F(RpcHandlerTest, SearchTest) { +TEST_F(RpcHandlerTest, SEARCH_TEST) { ::grpc::ServerContext context; ::milvus::grpc::SearchParam request; ::milvus::grpc::TopKQueryResultList response; @@ -281,7 +283,7 @@ TEST_F(RpcHandlerTest, SearchTest) { handler->SearchInFiles(&context, &search_in_files_param, &response); } -TEST_F(RpcHandlerTest, TablesTest) { +TEST_F(RpcHandlerTest, TABLES_TEST) { ::grpc::ServerContext context; ::milvus::grpc::TableSchema tableschema; ::milvus::grpc::Status response; @@ -387,7 +389,7 @@ TEST_F(RpcHandlerTest, TablesTest) { ASSERT_EQ(error_code, ::milvus::grpc::ErrorCode::SUCCESS); } -TEST_F(RpcHandlerTest, CmdTest) { +TEST_F(RpcHandlerTest, CMD_TEST) { ::grpc::ServerContext context; ::milvus::grpc::Command command; command.set_cmd("version"); @@ -401,7 +403,7 @@ TEST_F(RpcHandlerTest, CmdTest) { handler->Cmd(&context, &command, &reply); } -TEST_F(RpcHandlerTest, DeleteByRangeTest) { +TEST_F(RpcHandlerTest, DELETE_BY_RANGE_TEST) { ::grpc::ServerContext context; ::milvus::grpc::DeleteByRangeParam request; ::milvus::grpc::Status status; @@ -459,7 +461,7 @@ protected: } -TEST_F(RpcSchedulerTest, BaseTaskTest){ +TEST_F(RpcSchedulerTest, BASE_TASK_TEST){ auto status = task_ptr->Execute(); ASSERT_TRUE(status.ok());