Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hide C++ symbols in libxgboost.so when building Python wheel #5590

Merged
merged 5 commits into from
Apr 24, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 16 additions & 4 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ include(cmake/Utils.cmake)
list(APPEND CMAKE_MODULE_PATH "${xgboost_SOURCE_DIR}/cmake/modules")
cmake_policy(SET CMP0022 NEW)
cmake_policy(SET CMP0079 NEW)
cmake_policy(SET CMP0063 NEW)

if ((${CMAKE_VERSION} VERSION_GREATER 3.13) OR (${CMAKE_VERSION} VERSION_EQUAL 3.13))
cmake_policy(SET CMP0077 NEW)
Expand Down Expand Up @@ -36,6 +37,7 @@ option(USE_DMLC_GTEST "Use google tests bundled with dmlc-core submodule" OFF)
option(USE_NVTX "Build with cuda profiling annotations. Developers only." OFF)
set(NVTX_HEADER_DIR "" CACHE PATH "Path to the stand-alone nvtx header")
option(RABIT_MOCK "Build rabit with mock" OFF)
option(HIDE_CXX_SYMBOLS "Build shared library and hide all C++ symbols" OFF)
## CUDA
option(USE_CUDA "Build with GPU acceleration" OFF)
option(USE_NCCL "Build with NCCL to enable distributed GPU support." OFF)
Expand Down Expand Up @@ -131,6 +133,9 @@ foreach(lib rabit rabit_base rabit_empty rabit_mock rabit_mock_static)
# from dmlc is correctly applied to rabit.
if (TARGET ${lib})
target_link_libraries(${lib} dmlc ${CMAKE_THREAD_LIBS_INIT})
if (HIDE_CXX_SYMBOLS) # Hide all C++ symbols from Rabit
set_target_properties(${lib} PROPERTIES CXX_VISIBILITY_PRESET hidden)
endif (HIDE_CXX_SYMBOLS)
endif (TARGET ${lib})
endforeach()

Expand All @@ -148,10 +153,17 @@ set(XGBOOST_OBJ_SOURCES "${XGBOOST_OBJ_SOURCES};$<TARGET_OBJECTS:objxgboost>")

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

#-- Hide all C++ symbols
if (HIDE_CXX_SYMBOLS)
set_target_properties(objxgboost PROPERTIES CXX_VISIBILITY_PRESET hidden)
set_target_properties(xgboost PROPERTIES CXX_VISIBILITY_PRESET hidden)
endif (HIDE_CXX_SYMBOLS)

target_include_directories(xgboost
INTERFACE
$<INSTALL_INTERFACE:${CMAKE_INSTALL_PREFIX}/include>
Expand Down
2 changes: 1 addition & 1 deletion Jenkinsfile
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ def BuildCUDA(args) {
def docker_binary = "docker"
def docker_args = "--build-arg CUDA_VERSION=${args.cuda_version}"
sh """
${dockerRun} ${container_type} ${docker_binary} ${docker_args} tests/ci_build/build_via_cmake.sh -DUSE_CUDA=ON -DUSE_NCCL=ON -DOPEN_MP:BOOL=ON
${dockerRun} ${container_type} ${docker_binary} ${docker_args} tests/ci_build/build_via_cmake.sh -DUSE_CUDA=ON -DUSE_NCCL=ON -DOPEN_MP:BOOL=ON -DHIDE_CXX_SYMBOLS=ON
${dockerRun} ${container_type} ${docker_binary} ${docker_args} bash -c "cd python-package && rm -rf dist/* && python setup.py bdist_wheel --universal"
${dockerRun} ${container_type} ${docker_binary} ${docker_args} python3 tests/ci_build/rename_whl.py python-package/dist/*.whl ${commit_id} manylinux1_x86_64
"""
Expand Down
2 changes: 1 addition & 1 deletion include/xgboost/c_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
#if defined(_MSC_VER) || defined(_WIN32)
#define XGB_DLL XGB_EXTERN_C __declspec(dllexport)
#else
#define XGB_DLL XGB_EXTERN_C
#define XGB_DLL XGB_EXTERN_C __attribute__ ((visibility ("default")))
#endif // defined(_MSC_VER) || defined(_WIN32)

// manually define unsigned long
Expand Down
5 changes: 5 additions & 0 deletions python-package/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,14 @@
CURRENT_DIR = os.path.abspath(os.path.dirname(__file__))
sys.path.insert(0, CURRENT_DIR)

# Options only effect `python setup.py install`, building `bdist_wheel`
# requires using CMake directly.
USER_OPTIONS = {
'use-openmp': (None, 'Build with OpenMP support.', 1),
'use-cuda': (None, 'Build with GPU acceleration.', 0),
'use-nccl': (None, 'Build with NCCL to enable distributed GPU support.', 0),
'build-with-shared-nccl': (None, 'Build with shared NCCL library.', 0),
'hide-cxx-symbols': (None, 'Hide all C++ symbols during build.', 1),
'use-hdfs': (None, 'Build with HDFS support', 0),
'use-azure': (None, 'Build with AZURE support.', 0),
'use-s3': (None, 'Build with S3 support', 0),
Expand Down Expand Up @@ -103,6 +106,7 @@ def build(self, src_dir, build_dir, generator, build_tool=None, use_omp=1):
if k == 'USE_OPENMP' and use_omp == 0:
continue

self.logger.info('Run CMake command: %s', str(cmake_cmd))
subprocess.check_call(cmake_cmd, cwd=build_dir)

if system() != 'Windows':
Expand Down Expand Up @@ -236,6 +240,7 @@ def initialize_options(self):
self.use_cuda = 0
self.use_nccl = 0
self.build_with_shared_nccl = 0
self.hide_cxx_symbols = 1

self.use_hdfs = 0
self.use_azure = 0
Expand Down
2 changes: 1 addition & 1 deletion rabit
Submodule rabit updated 1 files
+1 −1 include/rabit/c_api.h
6 changes: 6 additions & 0 deletions src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,12 @@ if (USE_CUDA)
)
endif (MSVC)

if (HIDE_CXX_SYMBOLS)
target_compile_options(objxgboost PRIVATE
$<$<COMPILE_LANGUAGE:CUDA>:-Xcompiler=-fvisibility=hidden>
)
endif (HIDE_CXX_SYMBOLS)

set_target_properties(objxgboost PROPERTIES
CUDA_SEPARABLE_COMPILATION OFF)
else (USE_CUDA)
Expand Down
7 changes: 7 additions & 0 deletions tests/cpp/c_api/test_c_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -141,4 +141,11 @@ TEST(CAPI, JsonModelIO) {
ASSERT_EQ(model_str_0.front(), '{');
ASSERT_EQ(model_str_0, model_str_1);
}

TEST(CAPI, CatchDMLCError) {
DMatrixHandle out;
ASSERT_EQ(XGDMatrixCreateFromFile("foo", 0, &out), -1);
EXPECT_THROW({ dmlc::Stream::Create("foo", "r"); }, dmlc::Error);
}

} // namespace xgboost