[CI] Simplify CMake build with modern CMake techniques (#5871)

* [CI] Simplify CMake build * Make sure that plugins can be built * [CI] Install lz4 on Mac
2020-07-08 04:23:24 -07:00 · 2020-07-08 04:23:24 -07:00 · 0d411b0397
commit 0d411b0397
parent 22a31b1faa
10 changed files with 42 additions and 56 deletions
--- a/.travis.yml
+++ b/.travis.yml
@ -43,6 +43,7 @@ addons:
      - graphviz
      - openssl
      - libgit2
+      - lz4
      - wget
      - r
    update: true
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@ -112,6 +112,9 @@ if (USE_OPENMP)
  find_package(OpenMP REQUIRED)
 endif (USE_OPENMP)

+# core xgboost
+add_subdirectory(${xgboost_SOURCE_DIR}/src)
+
 # dmlc-core
 msvc_use_static_runtime()
 add_subdirectory(${xgboost_SOURCE_DIR}/dmlc-core)
@ -119,7 +122,6 @@ set_target_properties(dmlc PROPERTIES
  CXX_STANDARD 14
  CXX_STANDARD_REQUIRED ON
  POSITION_INDEPENDENT_CODE ON)
-list(APPEND LINKED_LIBRARIES_PRIVATE dmlc)
 if (MSVC)
  target_compile_options(dmlc PRIVATE
                         -D_CRT_SECURE_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE)
@ -128,6 +130,7 @@ if (MSVC)
                           -D_CRT_SECURE_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE)
  endif (TARGET dmlc_unit_tests)
 endif (MSVC)
+target_link_libraries(objxgboost PUBLIC dmlc)

 # rabit
 set(RABIT_BUILD_DMLC OFF)
@ -136,13 +139,13 @@ set(RABIT_WITH_R_LIB ${R_LIB})
 add_subdirectory(rabit)

 if (RABIT_MOCK)
-  list(APPEND LINKED_LIBRARIES_PRIVATE rabit_mock_static)
+  target_link_libraries(objxgboost PUBLIC rabit_mock_static)
  if (MSVC)
    target_compile_options(rabit_mock_static PRIVATE
                           -D_CRT_SECURE_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE)
  endif (MSVC)
 else()
-  list(APPEND LINKED_LIBRARIES_PRIVATE rabit)
+  target_link_libraries(objxgboost PUBLIC rabit)
  if (MSVC)
    target_compile_options(rabit PRIVATE
                           -D_CRT_SECURE_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE)
@ -164,19 +167,16 @@ if (R_LIB)
  add_subdirectory(${xgboost_SOURCE_DIR}/R-package)
 endif (R_LIB)

-# core xgboost
-list(APPEND LINKED_LIBRARIES_PRIVATE Threads::Threads ${CMAKE_THREAD_LIBS_INIT})
+# Plugin
 add_subdirectory(${xgboost_SOURCE_DIR}/plugin)
-add_subdirectory(${xgboost_SOURCE_DIR}/src)
-target_link_libraries(objxgboost PUBLIC dmlc)
-set(XGBOOST_OBJ_SOURCES "${XGBOOST_OBJ_SOURCES};$<TARGET_OBJECTS:objxgboost>")

 #-- library
 if (BUILD_STATIC_LIB)
-  add_library(xgboost STATIC ${XGBOOST_OBJ_SOURCES})
+  add_library(xgboost STATIC)
 else (BUILD_STATIC_LIB)
-  add_library(xgboost SHARED ${XGBOOST_OBJ_SOURCES})
+  add_library(xgboost SHARED)
 endif (BUILD_STATIC_LIB)
+target_link_libraries(xgboost PRIVATE objxgboost)

 if (USE_NVTX)
  enable_nvtx(xgboost)
@ -192,7 +192,6 @@ target_include_directories(xgboost
  INTERFACE
  $<INSTALL_INTERFACE:${CMAKE_INSTALL_PREFIX}/include>
  $<BUILD_INTERFACE:${CMAKE_CURRENT_LIST_DIR}/include>)
-target_link_libraries(xgboost PRIVATE ${LINKED_LIBRARIES_PRIVATE})

 # This creates its own shared library `xgboost4j'.
 if (JVM_BINDINGS)
@ -201,7 +200,8 @@ endif (JVM_BINDINGS)
 #-- End shared library

 #-- CLI for xgboost
-add_executable(runxgboost ${xgboost_SOURCE_DIR}/src/cli_main.cc ${XGBOOST_OBJ_SOURCES})
+add_executable(runxgboost ${xgboost_SOURCE_DIR}/src/cli_main.cc)
+target_link_libraries(runxgboost PRIVATE objxgboost)
 if (USE_NVTX)
  enable_nvtx(runxgboost)
 endif (USE_NVTX)
@ -211,7 +211,6 @@ target_include_directories(runxgboost
  ${xgboost_SOURCE_DIR}/include
  ${xgboost_SOURCE_DIR}/dmlc-core/include
  ${xgboost_SOURCE_DIR}/rabit/include)
-target_link_libraries(runxgboost PRIVATE ${LINKED_LIBRARIES_PRIVATE})
 set_target_properties(
  runxgboost PROPERTIES
  OUTPUT_NAME xgboost
--- a/2
+++ b/2
@ -189,7 +189,7 @@ def BuildCPU() {
      # This step is not necessary, but here we include it, to ensure that DMLC_CORE_USE_CMAKE flag is correctly propagated
      # We want to make sure that we use the configured header build/dmlc/build_config.h instead of include/dmlc/build_config_default.h.
      # See discussion at https://github.com/dmlc/xgboost/issues/5510
-    ${dockerRun} ${container_type} ${docker_binary} tests/ci_build/build_via_cmake.sh
+    ${dockerRun} ${container_type} ${docker_binary} tests/ci_build/build_via_cmake.sh -DPLUGIN_LZ4=ON -DPLUGIN_DENSE_PARSER=ON
    ${dockerRun} ${container_type} ${docker_binary} build/testxgboost
    """
    // Sanitizer test
--- a/R-package/CMakeLists.txt
+++ b/R-package/CMakeLists.txt
@ -6,8 +6,8 @@ file(GLOB_RECURSE R_SOURCES
  ${CMAKE_CURRENT_LIST_DIR}/src/*.c)
 # Use object library to expose symbols
 add_library(xgboost-r OBJECT ${R_SOURCES})
-
-set(R_DEFINITIONS
+target_compile_definitions(xgboost-r
+  PUBLIC
  -DXGBOOST_STRICT_R_MODE=1
  -DXGBOOST_CUSTOMIZE_GLOBAL_PRNG=1
  -DDMLC_LOG_BEFORE_THROW=0
@ -15,27 +15,23 @@ set(R_DEFINITIONS
  -DDMLC_LOG_CUSTOMIZE=1
  -DRABIT_CUSTOMIZE_MSG_
  -DRABIT_STRICT_CXX98_)
-target_compile_definitions(xgboost-r
-  PRIVATE ${R_DEFINITIONS})
 target_include_directories(xgboost-r
  PRIVATE
  ${LIBR_INCLUDE_DIRS}
  ${PROJECT_SOURCE_DIR}/include
  ${PROJECT_SOURCE_DIR}/dmlc-core/include
  ${PROJECT_SOURCE_DIR}/rabit/include)
+target_link_libraries(xgboost-r PRIVATE ${LIBR_CORE_LIBRARY})
 set_target_properties(
  xgboost-r PROPERTIES
  CXX_STANDARD 14
  CXX_STANDARD_REQUIRED ON
  POSITION_INDEPENDENT_CODE ON)

-set(XGBOOST_DEFINITIONS "${XGBOOST_DEFINITIONS};${R_DEFINITIONS}" PARENT_SCOPE)
-set(XGBOOST_OBJ_SOURCES $<TARGET_OBJECTS:xgboost-r> PARENT_SCOPE)
-set(LINKED_LIBRARIES_PRIVATE ${LINKED_LIBRARIES_PRIVATE} ${LIBR_CORE_LIBRARY} PARENT_SCOPE)
-
-if (USE_OPENMP)
-  target_link_libraries(xgboost-r PRIVATE OpenMP::OpenMP_CXX)
-endif ()
+# Get compilation and link flags of xgboost-r and propagate to objxgboost
+target_link_libraries(objxgboost PUBLIC xgboost-r)
+# Add all objects of xgboost-r to objxgboost
+target_sources(objxgboost INTERFACE $<TARGET_OBJECTS:xgboost-r>)

 set(LIBR_HOME "${LIBR_HOME}" PARENT_SCOPE)
 set(LIBR_EXECUTABLE "${LIBR_EXECUTABLE}" PARENT_SCOPE)
--- a/jvm-packages/CMakeLists.txt
+++ b/jvm-packages/CMakeLists.txt
@ -1,8 +1,8 @@
 find_package(JNI REQUIRED)

 add_library(xgboost4j SHARED
-  ${PROJECT_SOURCE_DIR}/jvm-packages/xgboost4j/src/native/xgboost4j.cpp
-  ${XGBOOST_OBJ_SOURCES})
+  ${PROJECT_SOURCE_DIR}/jvm-packages/xgboost4j/src/native/xgboost4j.cpp)
+target_link_libraries(xgboost4j PRIVATE objxgboost)
 target_include_directories(xgboost4j
  PRIVATE
  ${JNI_INCLUDE_DIRS}
@ -16,7 +16,4 @@ set_target_properties(
  xgboost4j PROPERTIES
  CXX_STANDARD 14
  CXX_STANDARD_REQUIRED ON)
-target_link_libraries(xgboost4j
-  PRIVATE
-  ${LINKED_LIBRARIES_PRIVATE}
-  ${JAVA_JVM_LIBRARY})
+target_link_libraries(xgboost4j PRIVATE ${JAVA_JVM_LIBRARY})
--- a/plugin/CMakeLists.txt
+++ b/plugin/CMakeLists.txt
@ -1,9 +1,8 @@
 if (PLUGIN_LZ4)
-  set(PLUGIN_SOURCES ${PLUGIN_SOURCES}
-    ${xgboost_SOURCE_DIR}/plugin/lz4/sparse_page_lz4_format.cc PARENT_SCOPE)
+  target_sources(objxgboost PRIVATE ${xgboost_SOURCE_DIR}/plugin/lz4/sparse_page_lz4_format.cc)
+  target_link_libraries(objxgboost PUBLIC lz4)
 endif (PLUGIN_LZ4)

 if (PLUGIN_DENSE_PARSER)
-  set(PLUGINS_SOURCES ${PLUGINS_SOURCES}
-    ${xgboost_SOURCE_DIR}/plugin/dense_parser/dense_libsvm.cc PARENT_SCOPE)
+  target_sources(objxgboost PRIVATE ${xgboost_SOURCE_DIR}/plugin/dense_parser/dense_libsvm.cc)
 endif (PLUGIN_DENSE_PARSER)
--- a/plugin/lz4/sparse_page_lz4_format.cc
+++ b/plugin/lz4/sparse_page_lz4_format.cc
@ -156,7 +156,7 @@ inline void CompressArray<DType>::Write(dmlc::Stream* fo) {
 }

 template<typename StorageIndex>
-class SparsePageLZ4Format : public SparsePageFormat {
+class SparsePageLZ4Format : public SparsePageFormat<SparsePage> {
 public:
  explicit SparsePageLZ4Format(bool use_lz4_hc)
      : use_lz4_hc_(use_lz4_hc) {
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@ -2,13 +2,13 @@ file(GLOB_RECURSE CPU_SOURCES *.cc *.h)
 list(REMOVE_ITEM CPU_SOURCES ${xgboost_SOURCE_DIR}/src/cli_main.cc)

 #-- Object library
-# Object library is necessary for jvm-package, which creates its own shared
-# library.
+# Object library is necessary for jvm-package, which creates its own shared library.
+add_library(objxgboost OBJECT)
+target_sources(objxgboost PRIVATE ${CPU_SOURCES})
 if (USE_CUDA)
  file(GLOB_RECURSE CUDA_SOURCES *.cu *.cuh)
-  add_library(objxgboost OBJECT ${CPU_SOURCES} ${CUDA_SOURCES} ${PLUGINS_SOURCES})
-  target_compile_definitions(objxgboost
-    PRIVATE -DXGBOOST_USE_CUDA=1)
+  target_sources(objxgboost PRIVATE ${CUDA_SOURCES})
+  target_compile_definitions(objxgboost PRIVATE -DXGBOOST_USE_CUDA=1)
  target_include_directories(objxgboost PRIVATE ${xgboost_SOURCE_DIR}/cub/)
  target_compile_options(objxgboost PRIVATE
    $<$<COMPILE_LANGUAGE:CUDA>:--expt-extended-lambda>
@ -20,7 +20,7 @@ if (USE_CUDA)
    find_package(Nccl REQUIRED)
    target_include_directories(objxgboost PRIVATE ${NCCL_INCLUDE_DIR})
    target_compile_definitions(objxgboost PRIVATE -DXGBOOST_USE_NCCL=1)
-    list(APPEND SRC_LIBS ${NCCL_LIBRARY})
+    target_link_libraries(objxgboost PUBLIC ${NCCL_LIBRARY})
  endif (USE_NCCL)

  if (USE_NVTX)
@ -47,8 +47,6 @@ if (USE_CUDA)
    CUDA_STANDARD 14
    CUDA_STANDARD_REQUIRED ON
    CUDA_SEPARABLE_COMPILATION OFF)
-else (USE_CUDA)
-  add_library(objxgboost OBJECT ${CPU_SOURCES} ${PLUGINS_SOURCES})
 endif (USE_CUDA)

 target_include_directories(objxgboost
@ -79,8 +77,7 @@ set_target_properties(objxgboost PROPERTIES
 target_compile_definitions(objxgboost
  PRIVATE
  -DDMLC_LOG_CUSTOMIZE=1  # enable custom logging
-  $<$<NOT:$<CXX_COMPILER_ID:MSVC>>:_MWAITXINTRIN_H_INCLUDED>
-  ${XGBOOST_DEFINITIONS})
+  $<$<NOT:$<CXX_COMPILER_ID:MSVC>>:_MWAITXINTRIN_H_INCLUDED>)
 if (USE_DEBUG_OUTPUT)
  target_compile_definitions(objxgboost PRIVATE -DXGBOOST_USE_DEBUG_OUTPUT=1)
 endif (USE_DEBUG_OUTPUT)
@ -97,14 +94,12 @@ if (XGBOOST_BUILTIN_PREFETCH_PRESENT)
 endif (XGBOOST_BUILTIN_PREFETCH_PRESENT)

 find_package(Threads REQUIRED)
-list(APPEND SRC_LIBS Threads::Threads ${CMAKE_THREAD_LIBS_INIT})
+target_link_libraries(objxgboost PUBLIC Threads::Threads ${CMAKE_THREAD_LIBS_INIT})

 if (USE_OPENMP OR USE_CUDA)  # CUDA requires OpenMP
  find_package(OpenMP REQUIRED)
-  list(APPEND SRC_LIBS OpenMP::OpenMP_CXX)
-  target_link_libraries(objxgboost PRIVATE OpenMP::OpenMP_CXX)
+  target_link_libraries(objxgboost PUBLIC OpenMP::OpenMP_CXX)
 endif (USE_OPENMP OR USE_CUDA)
-set(LINKED_LIBRARIES_PRIVATE "${LINKED_LIBRARIES_PRIVATE};${SRC_LIBS}" PARENT_SCOPE)

 # For MSVC: Call msvc_use_static_runtime() once again to completely
 # replace /MD with /MT. See https://github.com/dmlc/xgboost/issues/4462
--- a/tests/ci_build/Dockerfile.cpu
+++ b/tests/ci_build/Dockerfile.cpu
@ -7,7 +7,7 @@ SHELL ["/bin/bash", "-c"]   # Use Bash as shell
 # Install all basic requirements
 RUN \
    apt-get update && \
-    apt-get install -y tar unzip wget git build-essential doxygen graphviz llvm libasan2 libidn11 && \
+    apt-get install -y tar unzip wget git build-essential doxygen graphviz llvm libasan2 libidn11 liblz4-dev && \
    # CMake
    wget -nv -nc https://cmake.org/files/v3.13/cmake-3.13.0-Linux-x86_64.sh --no-check-certificate && \
    bash cmake-3.13.0-Linux-x86_64.sh --skip-license --prefix=/usr && \
--- a/tests/cpp/CMakeLists.txt
+++ b/tests/cpp/CMakeLists.txt
@ -12,8 +12,9 @@ if (USE_CUDA)
  file(GLOB_RECURSE CUDA_TEST_SOURCES "*.cu")
  list(APPEND TEST_SOURCES ${CUDA_TEST_SOURCES})
 endif (USE_CUDA)
-add_executable(testxgboost ${TEST_SOURCES} ${XGBOOST_OBJ_SOURCES}
+add_executable(testxgboost ${TEST_SOURCES}
  ${xgboost_SOURCE_DIR}/plugin/example/custom_obj.cc)
+target_link_libraries(testxgboost PRIVATE objxgboost)

 if (USE_CUDA)
  # OpenMP is mandatory for CUDA
@ -73,10 +74,8 @@ set_target_properties(
  CXX_STANDARD_REQUIRED ON)
 target_link_libraries(testxgboost
  PRIVATE
-  ${GTEST_LIBRARIES}
-  ${LINKED_LIBRARIES_PRIVATE})
+  ${GTEST_LIBRARIES})

-target_compile_definitions(testxgboost PRIVATE ${XGBOOST_DEFINITIONS})
 set_output_directory(testxgboost ${xgboost_BINARY_DIR})

 # This grouping organises source files nicely in visual studio