From b7d28b6e3d59e68cab74308e5799d0564729b28b Mon Sep 17 00:00:00 2001 From: Jiaming Yuan Date: Wed, 30 Jan 2019 05:47:03 +0000 Subject: [PATCH 01/10] Basic script for using compilation database. * Add `GENERATE_COMPILATION_DATABASE' to CMake. * Rearrange CMakeLists.txt. * Add basic python clang-tidy script. * Remove modernize-use-auto. --- .clang-tidy | 2 +- CMakeLists.txt | 89 ++++++++++++++++++------ doc/contribute.rst | 24 +++++++ tests/ci_build/tidy.py | 149 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 242 insertions(+), 22 deletions(-) create mode 100755 tests/ci_build/tidy.py diff --git a/.clang-tidy b/.clang-tidy index 959b0a438afb..1708dcf527c3 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -1,4 +1,4 @@ -Checks: 'modernize-*,-modernize-make-*,-modernize-raw-string-literal,google-*,-google-default-arguments,-clang-diagnostic-#pragma-messages,readability-identifier-naming' +Checks: 'modernize-*,-modernize-make-*,-modernize-use-auto,-modernize-raw-string-literal,google-*,-google-default-arguments,-clang-diagnostic-#pragma-messages,readability-identifier-naming' CheckOptions: - { key: readability-identifier-naming.ClassCase, value: CamelCase } - { key: readability-identifier-naming.StructCase, value: CamelCase } diff --git a/CMakeLists.txt b/CMakeLists.txt index aec9c06645b3..d2377e70a6de 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -123,8 +123,6 @@ file(GLOB_RECURSE SOURCES # Only add main function for executable target list(REMOVE_ITEM SOURCES ${PROJECT_SOURCE_DIR}/src/cli_main.cc) -file(GLOB_RECURSE TEST_SOURCES "tests/cpp/*.cc") - file(GLOB_RECURSE CUDA_SOURCES src/*.cu src/*.cuh @@ -140,7 +138,7 @@ if(PLUGIN_DENSE_PARSER) endif() # rabit -# TODO: Create rabit cmakelists.txt +# TODO: Use CMakeLists.txt from rabit. set(RABIT_SOURCES rabit/src/allreduce_base.cc rabit/src/allreduce_robust.cc @@ -151,6 +149,7 @@ set(RABIT_EMPTY_SOURCES rabit/src/engine_empty.cc rabit/src/c_api.cc ) + if(MINGW OR R_LIB) # build a dummy rabit library add_library(rabit STATIC ${RABIT_EMPTY_SOURCES}) @@ -158,7 +157,11 @@ else() add_library(rabit STATIC ${RABIT_SOURCES}) endif() -if(USE_CUDA) +if (GENERATE_COMPILATION_DATABASE) + set(CMAKE_EXPORT_COMPILE_COMMANDS ON) +endif (GENERATE_COMPILATION_DATABASE) + +if(USE_CUDA AND (NOT GENERATE_COMPILATION_DATABASE)) find_package(CUDA 8.0 REQUIRED) cmake_minimum_required(VERSION 3.5) @@ -168,7 +171,7 @@ if(USE_CUDA) if(USE_NCCL) find_package(Nccl REQUIRED) - include_directories(${NCCL_INCLUDE_DIR}) + cuda_include_directories(${NCCL_INCLUDE_DIR}) add_definitions(-DXGBOOST_USE_NCCL) endif() @@ -188,6 +191,38 @@ if(USE_CUDA) target_link_libraries(gpuxgboost ${NCCL_LIB_NAME}) endif() list(APPEND LINK_LIBRARIES gpuxgboost) + +elseif (USE_CUDA AND GENERATE_COMPILATION_DATABASE) + cmake_minimum_required(VERSION 3.8) + + find_package(CUDA 8.0 REQUIRED) + enable_language(CUDA) + set(CMAKE_CUDA_COMPILER clang++) + set(CUDA_SEPARABLE_COMPILATION ON) + if (NOT CLANG_CUDA_GENCODE) + set(CLANG_CUDA_GENCODE "--cuda-gpu-arch=sm_70") + endif (NOT CLANG_CUDA_GENCODE) + set(CMAKE_CUDA_FLAGS " -Wno-deprecated ${CLANG_CUDA_GENCODE} -fPIC ${GENCODE} -std=c++11 -x cuda") + message(STATUS "CMAKE_CUDA_FLAGS: ${CMAKE_CUDA_FLAGS}") + + add_library(gpuxgboost STATIC ${CUDA_SOURCES}) + + if(USE_NCCL) + find_package(Nccl REQUIRED) + target_include_directories(gpuxgboost PUBLIC ${NCCL_INCLUDE_DIR}) + target_compile_definitions(gpuxgboost PUBLIC -DXGBOOST_USE_NCCL) + target_link_libraries(gpuxgboost PUBLIC ${NCCL_LIB_NAME}) + endif() + + target_compile_definitions(gpuxgboost PUBLIC -DXGBOOST_USE_CUDA) + # A hack for CMake to make arguments valid for clang++ + string(REPLACE "-x cu" "-x cuda" CMAKE_CUDA_COMPILE_PTX_COMPILATION + ${CMAKE_CUDA_COMPILE_PTX_COMPILATION}) + string(REPLACE "-x cu" "-x cuda" CMAKE_CUDA_COMPILE_WHOLE_COMPILATION + ${CMAKE_CUDA_COMPILE_WHOLE_COMPILATION}) + string(REPLACE "-x cu" "-x cuda" CMAKE_CUDA_COMPILE_SEPARABLE_COMPILATION + ${CMAKE_CUDA_COMPILE_SEPARABLE_COMPILATION}) + target_include_directories(gpuxgboost PUBLIC cub) endif() @@ -203,7 +238,6 @@ endif() add_library(objxgboost OBJECT ${SOURCES}) - # building shared library for R package if(R_LIB) find_package(LibR REQUIRED) @@ -211,13 +245,13 @@ if(R_LIB) list(APPEND LINK_LIBRARIES "${LIBR_CORE_LIBRARY}") MESSAGE(STATUS "LIBR_CORE_LIBRARY " ${LIBR_CORE_LIBRARY}) - include_directories( + # Shared library target for the R package + add_library(xgboost SHARED $) + include_directories(xgboost "${LIBR_INCLUDE_DIRS}" "${PROJECT_SOURCE_DIR}" ) - # Shared library target for the R package - add_library(xgboost SHARED $) target_link_libraries(xgboost ${LINK_LIBRARIES}) # R uses no lib prefix in shared library names of its packages set_target_properties(xgboost PROPERTIES PREFIX "") @@ -229,7 +263,7 @@ if(R_LIB) # use a dummy location for any other remaining installs set(CMAKE_INSTALL_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/dummy_inst") -# main targets: shared library & exe + # main targets: shared library & exe else() # Executable add_executable(runxgboost $ src/cli_main.cc) @@ -252,20 +286,20 @@ else() add_dependencies(xgboost runxgboost) endif() - # JVM if(JVM_BINDINGS) find_package(JNI QUIET REQUIRED) - include_directories(${JNI_INCLUDE_DIRS} jvm-packages/xgboost4j/src/native) - add_library(xgboost4j SHARED - $ - jvm-packages/xgboost4j/src/native/xgboost4j.cpp) - set_output_directory(xgboost4j ${PROJECT_SOURCE_DIR}/lib) + $ + jvm-packages/xgboost4j/src/native/xgboost4j.cpp) + target_include_directories(xgboost4j + PRIVATE ${JNI_INCLUDE_DIRS} + PRIVATE jvm-packages/xgboost4j/src/native) target_link_libraries(xgboost4j - ${LINK_LIBRARIES} - ${JAVA_JVM_LIBRARY}) + ${LINK_LIBRARIES} + ${JAVA_JVM_LIBRARY}) + set_output_directory(xgboost4j ${PROJECT_SOURCE_DIR}/lib) endif() @@ -274,18 +308,31 @@ if(GOOGLE_TEST) enable_testing() find_package(GTest REQUIRED) + file(GLOB_RECURSE TEST_SOURCES "tests/cpp/*.cc") auto_source_group("${TEST_SOURCES}") - include_directories(${GTEST_INCLUDE_DIRS}) - if(USE_CUDA) + if(USE_CUDA AND (NOT GENERATE_COMPILATION_DATABASE)) file(GLOB_RECURSE CUDA_TEST_SOURCES "tests/cpp/*.cu") + cuda_include_directories(${GTEST_INCLUDE_DIRS}) cuda_compile(CUDA_TEST_OBJS ${CUDA_TEST_SOURCES}) + elseif (USE_CUDA AND GENERATE_COMPILATION_DATABASE) + file(GLOB_RECURSE CUDA_TEST_SOURCES "tests/cpp/*.cu") else() set(CUDA_TEST_OBJS "") endif() - add_executable(testxgboost ${TEST_SOURCES} ${CUDA_TEST_OBJS} $) + if (USE_CUDA AND GENERATE_COMPILATION_DATABASE) + add_executable(testxgboost ${TEST_SOURCES} ${CUDA_TEST_SOURCES} + $) + target_include_directories(testxgboost PRIVATE cub) + else () + add_executable(testxgboost ${TEST_SOURCES} ${CUDA_TEST_OBJS} + $) + endif () + set_output_directory(testxgboost ${PROJECT_SOURCE_DIR}) + target_include_directories(testxgboost + PRIVATE ${GTEST_INCLUDE_DIRS}) target_link_libraries(testxgboost ${GTEST_LIBRARIES} ${LINK_LIBRARIES}) add_test(TestXGBoost testxgboost) diff --git a/doc/contribute.rst b/doc/contribute.rst index 6335a6498986..d2232548bffc 100644 --- a/doc/contribute.rst +++ b/doc/contribute.rst @@ -19,6 +19,7 @@ Everyone is more than welcome to contribute. It is a way to make the project bet * `Documents`_ * `Testcases`_ * `Sanitizers`_ +* `clang-tidy`_ * `Examples`_ * `Core Library`_ * `Python Package`_ @@ -169,6 +170,29 @@ environment variable: For details, please consult `official documentation `_ for sanitizers. +********** +clang-tidy +********** +To run clang-tidy on both C++ and CUDA source code, run the following command +from the top level source tree: + + .. code-black:: bash + cd /path/to/xgboost/ + python tests/ci_build/tidy.py + +The script accepts two optional integer arguments, namely --cpp and --cuda. +By default they are both set to 1. If you want to exclude CUDA source from +linting, use: + + .. code-black:: bash + cd /path/to/xgboost/ + python tests/ci_build/tidy.py --cuda=0 + +Similarly, if you want to exclude C++ source from linting: + + .. code-black:: bash + cd /path/to/xgboost/ + python tests/ci_build/tidy.py --cpp=0 ******** Examples diff --git a/tests/ci_build/tidy.py b/tests/ci_build/tidy.py new file mode 100755 index 000000000000..082dda61c9d3 --- /dev/null +++ b/tests/ci_build/tidy.py @@ -0,0 +1,149 @@ +#!/usr/bin/env python +import subprocess +import yaml +import json +from multiprocessing import Pool, cpu_count +import shutil +import os +# import sys +import re +import argparse + + +def call(args): + '''Subprocess run wrapper.''' + completed = subprocess.run(args, stdout=subprocess.PIPE) + out = completed.stdout.decode('utf-8') + matched = re.match('.*xgboost.*warning.*', out) + if matched is None: + return_code = 0 + else: + print(out, '\n') + return_code = 1 + return completed.returncode | return_code + + +class ClangTidy(object): + ''' + clang tidy wrapper. + Args: + cpp_lint: Run linter on C++ source code. + cuda_lint: Run linter on CUDA source code. + ''' + def __init__(self, cpp_lint, cuda_lint): + self.cpp_lint = cpp_lint + self.cuda_lint = cuda_lint + print('Run linter on CUDA: ', self.cuda_lint) + print('Run linter on C++:', self.cpp_lint) + if not self.cpp_lint and not self.cuda_lint: + raise ValueError('Both --cpp and --cuda are set to 0.') + self.root_path = os.path.abspath(os.path.curdir) + print('Project root:', self.root_path) + if not self.root_path.endswith('xgboost'): + raise ValueError('Linter should be invoked in project root.') + self.cdb_path = os.path.join(self.root_path, 'cdb') + + def __enter__(self): + if os.path.exists(self.cdb_path): + shutil.rmtree(self.cdb_path) + self._generate_cdb() + return self + + def __exit__(self, *args): + if os.path.exists(self.cdb_path): + shutil.rmtree(self.cdb_path) + + def _generate_cdb(self): + '''Run CMake to generate compilation database.''' + os.mkdir(self.cdb_path) + os.chdir(self.cdb_path) + cmake_args = ['cmake', '..', '-DGENERATE_COMPILATION_DATABASE=ON', + '-DGOOGLE_TEST=ON'] + if self.cuda_lint: + cmake_args.extend(['-DUSE_CUDA=ON', '-DUSE_NCCL=ON']) + subprocess.run(cmake_args) + os.chdir(self.root_path) + + def _configure_flags(self, path, command): + common_args = ['clang-tidy', + '-config='+str(self.clang_tidy)] + common_args.append(path) + common_args.append('--') + + command = command.split()[1:] # remove clang/c++/g++ + # filter out not used flags + if '-fuse-ld=gold' in command: + command.remove('-fuse-ld=gold') + if '-rdynamic' in command: + command.remove('-rdynamic') + if '-Xcompiler=-fPIC' in command: + command.remove('-Xcompiler=-fPIC') + if '-Xcompiler=-fPIE' in command: + command.remove('-Xcompiler=-fPIE') + if '-c' in command: + index = command.index('-c') + del command[index+1] + command.remove('-c') + if '-o' in command: + index = command.index('-o') + del command[index+1] + command.remove('-o') + + common_args.extend(command) + + # Two passes, one for device code another for host code. + if path.endswith('cu'): + args = [common_args.copy(), common_args.copy()] + args[0].append('--cuda-host-only') + args[1].append('--cuda-device-only') + else: + args = [common_args.copy()] + for a in args: + a.append('-Wno-unused-command-line-argument') + return args + + def _configure(self): + '''Load and configure compile_commands and clang_tidy.''' + + def should_lint(path): + if not self.cpp_lint and path.endswith('.cc'): + return False + return True + + cdb_file = os.path.join(self.cdb_path, 'compile_commands.json') + with open(cdb_file, 'r') as fd: + self.compile_commands = json.load(fd) + tidy_file = os.path.join(self.root_path, '.clang-tidy') + with open(tidy_file) as fd: + self.clang_tidy = yaml.load(fd) + all_files = [] + for entry in self.compile_commands: + path = entry['file'] + if should_lint(path): + print(path) + args = self._configure_flags(path, entry['command']) + all_files.extend(args) + return all_files + + def run(self): + '''Run clang-tidy.''' + all_files = self._configure() + with Pool(cpu_count()) as pool: + results = pool.map(call, all_files) + passed = True + if 1 in results: + print('Please correct clang-tidy warnings.') + passed = False + return passed + + +if __name__ == '__main__': + parser = argparse.ArgumentParser(description='Run clang-tidy.') + parser.add_argument('--cpp', type=int, default=1) + parser.add_argument('--cuda', type=int, default=1) + args = parser.parse_args() + with ClangTidy(args.cpp, args.cuda) as linter: + passed = linter.run() + # Uncomment it once the code base is clang-tidy conformant. + # if not passed: + # sys.exit(1) From 5b0c4dde2cd47167474ea36fa661a97200b44085 Mon Sep 17 00:00:00 2001 From: Philip Hyunsu Cho Date: Wed, 30 Jan 2019 05:47:03 +0000 Subject: [PATCH 02/10] Document that Python 3 is required for Clang-Tidy --- doc/contribute.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/contribute.rst b/doc/contribute.rst index d2232548bffc..e04a9ded9a2c 100644 --- a/doc/contribute.rst +++ b/doc/contribute.rst @@ -178,7 +178,7 @@ from the top level source tree: .. code-black:: bash cd /path/to/xgboost/ - python tests/ci_build/tidy.py + python3 tests/ci_build/tidy.py The script accepts two optional integer arguments, namely --cpp and --cuda. By default they are both set to 1. If you want to exclude CUDA source from @@ -186,13 +186,13 @@ linting, use: .. code-black:: bash cd /path/to/xgboost/ - python tests/ci_build/tidy.py --cuda=0 + python3 tests/ci_build/tidy.py --cuda=0 Similarly, if you want to exclude C++ source from linting: .. code-black:: bash cd /path/to/xgboost/ - python tests/ci_build/tidy.py --cpp=0 + python3 tests/ci_build/tidy.py --cpp=0 ******** Examples From 66dcb13fd79b3a8b75d370561e5720cd0428a316 Mon Sep 17 00:00:00 2001 From: Philip Hyunsu Cho Date: Wed, 30 Jan 2019 05:47:03 +0000 Subject: [PATCH 03/10] Explicitly specify Google Test path in clang-tidy script --- doc/contribute.rst | 6 ++++-- tests/ci_build/tidy.py | 11 ++++++++--- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/doc/contribute.rst b/doc/contribute.rst index e04a9ded9a2c..cde56efe0144 100644 --- a/doc/contribute.rst +++ b/doc/contribute.rst @@ -178,9 +178,11 @@ from the top level source tree: .. code-black:: bash cd /path/to/xgboost/ - python3 tests/ci_build/tidy.py + python3 tests/ci_build/tidy.py --gtest-path=/path/to/google-test -The script accepts two optional integer arguments, namely --cpp and --cuda. +The script requires the full path of Google Test library via the ``--gtest-path`` argument. + +Also, the script accepts two optional integer arguments, namely ``--cpp`` and ``--cuda``. By default they are both set to 1. If you want to exclude CUDA source from linting, use: diff --git a/tests/ci_build/tidy.py b/tests/ci_build/tidy.py index 082dda61c9d3..53eb86c7fba4 100755 --- a/tests/ci_build/tidy.py +++ b/tests/ci_build/tidy.py @@ -27,12 +27,15 @@ class ClangTidy(object): ''' clang tidy wrapper. Args: + gtest_path: Full path of Google Test library. cpp_lint: Run linter on C++ source code. cuda_lint: Run linter on CUDA source code. ''' - def __init__(self, cpp_lint, cuda_lint): + def __init__(self, gtest_path, cpp_lint, cuda_lint): + self.gtest_path = gtest_path self.cpp_lint = cpp_lint self.cuda_lint = cuda_lint + print('Using Google Test from {}'.format(self.gtest_path)) print('Run linter on CUDA: ', self.cuda_lint) print('Run linter on C++:', self.cpp_lint) if not self.cpp_lint and not self.cuda_lint: @@ -58,7 +61,7 @@ def _generate_cdb(self): os.mkdir(self.cdb_path) os.chdir(self.cdb_path) cmake_args = ['cmake', '..', '-DGENERATE_COMPILATION_DATABASE=ON', - '-DGOOGLE_TEST=ON'] + '-DGOOGLE_TEST=ON', '-DGTEST_ROOT={}'.format(self.gtest_path)] if self.cuda_lint: cmake_args.extend(['-DUSE_CUDA=ON', '-DUSE_NCCL=ON']) subprocess.run(cmake_args) @@ -141,8 +144,10 @@ def run(self): parser = argparse.ArgumentParser(description='Run clang-tidy.') parser.add_argument('--cpp', type=int, default=1) parser.add_argument('--cuda', type=int, default=1) + parser.add_argument('--gtest-path', required=True, + help='Full path of Google Test library directory') args = parser.parse_args() - with ClangTidy(args.cpp, args.cuda) as linter: + with ClangTidy(args.gtest_path, args.cpp, args.cuda) as linter: passed = linter.run() # Uncomment it once the code base is clang-tidy conformant. # if not passed: From 4799c96c6b8781562ec6c53feca60bfc997ef513 Mon Sep 17 00:00:00 2001 From: Philip Cho Date: Tue, 29 Jan 2019 23:55:58 -0800 Subject: [PATCH 04/10] Refine logic for correct path detection In Jenkins, the project root is of form /home/ubuntu/workspace/xgboost_PR-XXXX --- tests/ci_build/tidy.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/ci_build/tidy.py b/tests/ci_build/tidy.py index 53eb86c7fba4..4d3c41b99ce7 100755 --- a/tests/ci_build/tidy.py +++ b/tests/ci_build/tidy.py @@ -42,7 +42,7 @@ def __init__(self, gtest_path, cpp_lint, cuda_lint): raise ValueError('Both --cpp and --cuda are set to 0.') self.root_path = os.path.abspath(os.path.curdir) print('Project root:', self.root_path) - if not self.root_path.endswith('xgboost'): + if 'xgboost' not in os.path.basename(self.root_path.rstrip('/')): raise ValueError('Linter should be invoked in project root.') self.cdb_path = os.path.join(self.root_path, 'cdb') From fe9e091b0bb642d1216b2f82dc44033ac1577823 Mon Sep 17 00:00:00 2001 From: Philip Hyunsu Cho Date: Wed, 30 Jan 2019 06:32:15 +0000 Subject: [PATCH 05/10] Add clang-tidy to Jenkins --- Jenkinsfile | 32 ++++++++++++++++++++++++++++++- tests/ci_build/setup_cuda_path.sh | 4 ++++ 2 files changed, 35 insertions(+), 1 deletion(-) create mode 100644 tests/ci_build/setup_cuda_path.sh diff --git a/Jenkinsfile b/Jenkinsfile index 143cba5969cf..700f44eae705 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -53,7 +53,7 @@ pipeline { parallel (buildMatrix.findAll{it['enabled']}.collectEntries{ c -> def buildName = utils.getBuildName(c) utils.buildFactory(buildName, c, false, this.&buildPlatformCmake) - }) + } + [ "clang-tidy" : { buildClangTidyJob() } ]) } } } @@ -106,3 +106,33 @@ def buildPlatformCmake(buildName, conf, nodeReq, dockerTarget) { } } } + +/** + * Run a clang-tidy job on a GPU machine + */ +def buildClangTidyJob() { + def nodeReq = "linux && gpu && unrestricted" + node(nodeReq) { + unstash name: 'srcs' + echo "Running clang-tidy job..." + // Install Google Test and Python yaml + sh """ + pip3 install pyyaml + + rm -rf gtest googletest-release-1.7.0 + wget -nc https://github.com/google/googletest/archive/release-1.7.0.zip + jar -xf release-1.7.0.zip + mv googletest-release-1.7.0 gtest && cd gtest + cmake . && make + mkdir lib && mv libgtest.a lib + cd .. + rm -rf release-1.7.0.zip* + """ + // Run clang-tidy job + sh """#!/bin/bash + source tests/ci_build/setup_cuda_path.sh + python3 tests/ci_build/tidy.py --gtest-path=${WORKSPACE}/gtest + """ + } + } + diff --git a/tests/ci_build/setup_cuda_path.sh b/tests/ci_build/setup_cuda_path.sh new file mode 100644 index 000000000000..bb3abd063bf7 --- /dev/null +++ b/tests/ci_build/setup_cuda_path.sh @@ -0,0 +1,4 @@ +#!/bin/bash + +export PATH=/usr/local/cuda/bin${PATH:+:${PATH}} +export LD_LIBRARY_PATH=/usr/local/cuda/lib64${LD_LIBRARY_PATH:+:${LD_LIBRARY_PATH}} From b10af807f522f1bcb5be5e1c5e052d8348cc21af Mon Sep 17 00:00:00 2001 From: Philip Hyunsu Cho Date: Sun, 3 Feb 2019 09:12:55 +0000 Subject: [PATCH 06/10] Display error logs --- tests/ci_build/tidy.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/ci_build/tidy.py b/tests/ci_build/tidy.py index 4d3c41b99ce7..0e454b444249 100755 --- a/tests/ci_build/tidy.py +++ b/tests/ci_build/tidy.py @@ -12,13 +12,15 @@ def call(args): '''Subprocess run wrapper.''' - completed = subprocess.run(args, stdout=subprocess.PIPE) - out = completed.stdout.decode('utf-8') - matched = re.match('.*xgboost.*warning.*', out) + completed = subprocess.run(args, stdout=subprocess.PIPE, + stderr=subprocess.DEVNULL) + error_msg = completed.stdout.decode('utf-8') + matched = re.match('.*xgboost.*warning.*', error_msg, + re.MULTILINE | re.DOTALL) if matched is None: return_code = 0 else: - print(out, '\n') + print(error_msg, '\n') return_code = 1 return completed.returncode | return_code From 1d043c35da48377a93110dcf93e7a3a6b617a767 Mon Sep 17 00:00:00 2001 From: Philip Hyunsu Cho Date: Sun, 3 Feb 2019 09:23:02 +0000 Subject: [PATCH 07/10] Apply header filter --- tests/ci_build/tidy.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/ci_build/tidy.py b/tests/ci_build/tidy.py index 0e454b444249..4d27703bea4e 100755 --- a/tests/ci_build/tidy.py +++ b/tests/ci_build/tidy.py @@ -71,6 +71,7 @@ def _generate_cdb(self): def _configure_flags(self, path, command): common_args = ['clang-tidy', + "-header-filter='(xgboost\\/src|xgboost\\/include)'", '-config='+str(self.clang_tidy)] common_args.append(path) common_args.append('--') From b643f11c6d017d68ac60c0e0e5be66f519be422e Mon Sep 17 00:00:00 2001 From: fis Date: Sun, 3 Feb 2019 18:09:59 +0800 Subject: [PATCH 08/10] Lower the gpu-arch and disable header-filter for now. --- CMakeLists.txt | 3 ++- tests/ci_build/tidy.py | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index d2377e70a6de..4a7ebdf25ee7 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -193,6 +193,7 @@ if(USE_CUDA AND (NOT GENERATE_COMPILATION_DATABASE)) list(APPEND LINK_LIBRARIES gpuxgboost) elseif (USE_CUDA AND GENERATE_COMPILATION_DATABASE) + # Enable CUDA language to generate a compilation database. cmake_minimum_required(VERSION 3.8) find_package(CUDA 8.0 REQUIRED) @@ -200,7 +201,7 @@ elseif (USE_CUDA AND GENERATE_COMPILATION_DATABASE) set(CMAKE_CUDA_COMPILER clang++) set(CUDA_SEPARABLE_COMPILATION ON) if (NOT CLANG_CUDA_GENCODE) - set(CLANG_CUDA_GENCODE "--cuda-gpu-arch=sm_70") + set(CLANG_CUDA_GENCODE "--cuda-gpu-arch=sm_35") endif (NOT CLANG_CUDA_GENCODE) set(CMAKE_CUDA_FLAGS " -Wno-deprecated ${CLANG_CUDA_GENCODE} -fPIC ${GENCODE} -std=c++11 -x cuda") message(STATUS "CMAKE_CUDA_FLAGS: ${CMAKE_CUDA_FLAGS}") diff --git a/tests/ci_build/tidy.py b/tests/ci_build/tidy.py index 4d27703bea4e..7007f4125daa 100755 --- a/tests/ci_build/tidy.py +++ b/tests/ci_build/tidy.py @@ -5,7 +5,6 @@ from multiprocessing import Pool, cpu_count import shutil import os -# import sys import re import argparse @@ -63,7 +62,8 @@ def _generate_cdb(self): os.mkdir(self.cdb_path) os.chdir(self.cdb_path) cmake_args = ['cmake', '..', '-DGENERATE_COMPILATION_DATABASE=ON', - '-DGOOGLE_TEST=ON', '-DGTEST_ROOT={}'.format(self.gtest_path)] + '-DGOOGLE_TEST=ON', '-DGTEST_ROOT={}'.format( + self.gtest_path)] if self.cuda_lint: cmake_args.extend(['-DUSE_CUDA=ON', '-DUSE_NCCL=ON']) subprocess.run(cmake_args) @@ -71,7 +71,7 @@ def _generate_cdb(self): def _configure_flags(self, path, command): common_args = ['clang-tidy', - "-header-filter='(xgboost\\/src|xgboost\\/include)'", + # "-header-filter='(xgboost\\/src|xgboost\\/include)'", '-config='+str(self.clang_tidy)] common_args.append(path) common_args.append('--') From 8e8287e08e3f9f1eb0972b2734225b30ca3f0c79 Mon Sep 17 00:00:00 2001 From: Philip Hyunsu Cho Date: Mon, 4 Feb 2019 21:11:12 +0000 Subject: [PATCH 09/10] Run clang-tidy in CUDA 9.2 container --- Jenkinsfile | 19 +++--------- tests/ci_build/Dockerfile.clang_tidy | 45 ++++++++++++++++++++++++++++ tests/ci_build/clang_tidy.sh | 12 ++++++++ tests/ci_build/setup_cuda_path.sh | 4 --- tests/ci_build/tidy.py | 2 -- 5 files changed, 61 insertions(+), 21 deletions(-) create mode 100644 tests/ci_build/Dockerfile.clang_tidy create mode 100755 tests/ci_build/clang_tidy.sh delete mode 100644 tests/ci_build/setup_cuda_path.sh diff --git a/Jenkinsfile b/Jenkinsfile index 700f44eae705..49746a8db946 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -115,23 +115,12 @@ def buildClangTidyJob() { node(nodeReq) { unstash name: 'srcs' echo "Running clang-tidy job..." + // Invoke command inside docker // Install Google Test and Python yaml + dockerTarget = "gpu" + dockerArgs = "--build-arg CUDA_VERSION=9.2" sh """ - pip3 install pyyaml - - rm -rf gtest googletest-release-1.7.0 - wget -nc https://github.com/google/googletest/archive/release-1.7.0.zip - jar -xf release-1.7.0.zip - mv googletest-release-1.7.0 gtest && cd gtest - cmake . && make - mkdir lib && mv libgtest.a lib - cd .. - rm -rf release-1.7.0.zip* - """ - // Run clang-tidy job - sh """#!/bin/bash - source tests/ci_build/setup_cuda_path.sh - python3 tests/ci_build/tidy.py --gtest-path=${WORKSPACE}/gtest + ${dockerRun} ${dockerTarget} ${dockerArgs} tests/ci_build/clang_tidy.sh """ } } diff --git a/tests/ci_build/Dockerfile.clang_tidy b/tests/ci_build/Dockerfile.clang_tidy new file mode 100644 index 000000000000..1d09fce96999 --- /dev/null +++ b/tests/ci_build/Dockerfile.clang_tidy @@ -0,0 +1,45 @@ +ARG CUDA_VERSION +FROM nvidia/cuda:$CUDA_VERSION-devel-ubuntu18.04 + +# Environment +ENV DEBIAN_FRONTEND noninteractive + +# Install all basic requirements +RUN \ + apt-get update && \ + apt-get install -y tar unzip wget git build-essential cmake python3 python3-pip llvm-7 clang-tidy-7 clang-7 + +# Set default clang-tidy version +RUN \ + update-alternatives --install /usr/bin/clang-tidy clang-tidy /usr/bin/clang-tidy-7 100 && \ + update-alternatives --install /usr/bin/clang clang /usr/bin/clang-7 100 + +# NCCL2 (License: https://docs.nvidia.com/deeplearning/sdk/nccl-sla/index.html) +RUN \ + export CUDA_SHORT=`echo $CUDA_VERSION | egrep -o '[0-9]+\.[0-9]'` && \ + if [ "${CUDA_SHORT}" != "10.0" ]; then \ + wget https://developer.download.nvidia.com/compute/redist/nccl/v2.2/nccl_2.2.13-1%2Bcuda${CUDA_SHORT}_x86_64.txz && \ + tar xf "nccl_2.2.13-1+cuda${CUDA_SHORT}_x86_64.txz" && \ + cp nccl_2.2.13-1+cuda${CUDA_SHORT}_x86_64/include/nccl.h /usr/include && \ + cp nccl_2.2.13-1+cuda${CUDA_SHORT}_x86_64/lib/* /usr/lib && \ + rm -f nccl_2.2.13-1+cuda${CUDA_SHORT}_x86_64.txz && \ + rm -r nccl_2.2.13-1+cuda${CUDA_SHORT}_x86_64; fi + +# Install Python packages +RUN \ + pip3 install pyyaml + +ENV GOSU_VERSION 1.10 + +# Install lightweight sudo (not bound to TTY) +RUN set -ex; \ + wget -O /usr/local/bin/gosu "https://github.com/tianon/gosu/releases/download/$GOSU_VERSION/gosu-amd64" && \ + chmod +x /usr/local/bin/gosu && \ + gosu nobody true + +# Default entry-point to use if running locally +# It will preserve attributes of created files +COPY entrypoint.sh /scripts/ + +WORKDIR /workspace +ENTRYPOINT ["/scripts/entrypoint.sh"] diff --git a/tests/ci_build/clang_tidy.sh b/tests/ci_build/clang_tidy.sh new file mode 100755 index 000000000000..392d910e4f3a --- /dev/null +++ b/tests/ci_build/clang_tidy.sh @@ -0,0 +1,12 @@ +#!/bin/bash + +rm -rf gtest googletest-release-1.7.0 +wget -nc https://github.com/google/googletest/archive/release-1.7.0.zip +unzip -n release-1.7.0.zip +mv googletest-release-1.7.0 gtest && cd gtest +cmake . && make +mkdir lib && mv libgtest.a lib +cd .. +rm -rf release-1.7.0.zip* + +python3 tests/ci_build/tidy.py --gtest-path=${PWD}/gtest diff --git a/tests/ci_build/setup_cuda_path.sh b/tests/ci_build/setup_cuda_path.sh deleted file mode 100644 index bb3abd063bf7..000000000000 --- a/tests/ci_build/setup_cuda_path.sh +++ /dev/null @@ -1,4 +0,0 @@ -#!/bin/bash - -export PATH=/usr/local/cuda/bin${PATH:+:${PATH}} -export LD_LIBRARY_PATH=/usr/local/cuda/lib64${LD_LIBRARY_PATH:+:${LD_LIBRARY_PATH}} diff --git a/tests/ci_build/tidy.py b/tests/ci_build/tidy.py index 7007f4125daa..76a044228ac8 100755 --- a/tests/ci_build/tidy.py +++ b/tests/ci_build/tidy.py @@ -43,8 +43,6 @@ def __init__(self, gtest_path, cpp_lint, cuda_lint): raise ValueError('Both --cpp and --cuda are set to 0.') self.root_path = os.path.abspath(os.path.curdir) print('Project root:', self.root_path) - if 'xgboost' not in os.path.basename(self.root_path.rstrip('/')): - raise ValueError('Linter should be invoked in project root.') self.cdb_path = os.path.join(self.root_path, 'cdb') def __enter__(self): From 1e9c7578aec2c525bc5d039c5014b9cb28a8800e Mon Sep 17 00:00:00 2001 From: Philip Hyunsu Cho Date: Mon, 4 Feb 2019 17:34:54 -0800 Subject: [PATCH 10/10] Use clang_tidy container --- Jenkinsfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Jenkinsfile b/Jenkinsfile index 49746a8db946..215e9102e845 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -117,7 +117,7 @@ def buildClangTidyJob() { echo "Running clang-tidy job..." // Invoke command inside docker // Install Google Test and Python yaml - dockerTarget = "gpu" + dockerTarget = "clang_tidy" dockerArgs = "--build-arg CUDA_VERSION=9.2" sh """ ${dockerRun} ${dockerTarget} ${dockerArgs} tests/ci_build/clang_tidy.sh