Skip to content

Commit

Permalink
- Add visibility macros. Hide boost symbols in arrow_io
Browse files Browse the repository at this point in the history
- Hack around Travis CI inability to use its boost static libraries
- Use parquet_shared name
- More informative verbose test logs
- Fix some gtest-1.7.0 crankiness
- Fix a valgrind shared_ptr possible memory leak stemming from static variable
  referenced at compile-time in libarrow_parquet
- Fix a bunch of compiler warnings in release builds

Change-Id: I4f519751becaf6fbdbc0e2fe79938dec13876b30
  • Loading branch information
wesm committed Jul 10, 2016
1 parent fab4c82 commit 69b03b0
Show file tree
Hide file tree
Showing 50 changed files with 440 additions and 245 deletions.
1 change: 0 additions & 1 deletion ci/travis_install_conda.sh
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,3 @@ conda install --yes conda-build jinja2 anaconda-client

# faster builds, please
conda install -y nomkl

2 changes: 1 addition & 1 deletion ci/travis_script_cpp.sh
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,6 @@ make lint
# make check-clang-tidy
# fi

ctest -L unittest
ctest -VV -L unittest

popd
6 changes: 4 additions & 2 deletions ci/travis_script_python.sh
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ PYTHON_DIR=$TRAVIS_BUILD_DIR/python
# Re-use conda installation from C++
export MINICONDA=$TRAVIS_BUILD_DIR/miniconda
export PATH="$MINICONDA/bin:$PATH"
export LD_LIBRARY_PATH="$MINICONDA/lib:$LD_LIBRARY_PATH"
export PARQUET_HOME=$MINICONDA

# Share environment with C++
Expand All @@ -32,12 +31,15 @@ python_version_tests() {
# Expensive dependencies install from Continuum package repo
conda install -y pip numpy pandas cython

conda install -y parquet-cpp arrow-cpp -c apache/channel/dev

# Other stuff pip install
pip install -r requirements.txt

export ARROW_HOME=$ARROW_CPP_INSTALL

python setup.py build_ext --inplace
python setup.py build_ext \
--inplace

python -m pytest -vv -r sxX pyarrow
}
Expand Down
218 changes: 119 additions & 99 deletions cpp/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,22 @@ endif(CCACHE_FOUND)

# Top level cmake dir
if("${CMAKE_SOURCE_DIR}" STREQUAL "${CMAKE_CURRENT_SOURCE_DIR}")
option(ARROW_BUILD_STATIC
"Build the libarrow static libraries"
ON)

option(ARROW_BUILD_SHARED
"Build the libarrow shared libraries"
ON)

option(ARROW_PARQUET
"Build the Parquet adapter and link to libparquet"
OFF)

option(ARROW_TEST_MEMCHECK
"Run the test suite using valgrind --tool=memcheck"
OFF)
"Run the test suite using valgrind --tool=memcheck"
OFF)

option(ARROW_BUILD_TESTS
"Build the Arrow googletest unit tests"
ON)
Expand All @@ -66,6 +76,10 @@ if("${CMAKE_SOURCE_DIR}" STREQUAL "${CMAKE_CURRENT_SOURCE_DIR}")
"Build the Arrow IO extensions for the Hadoop file system"
OFF)

option(ARROW_BOOST_USE_SHARED
"Rely on boost shared libraries where relevant"
ON)

option(ARROW_SSE3
"Build Arrow with SSE3"
ON)
Expand Down Expand Up @@ -169,21 +183,10 @@ if ("${COMPILER_FAMILY}" STREQUAL "clang")
# http://petereisentraut.blogspot.com/2011/05/ccache-and-clang.html
# http://petereisentraut.blogspot.com/2011/09/ccache-and-clang-part-2.html
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Qunused-arguments")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-unused-local-typedef")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${CMAKE_CLANG_OPTIONS}")
endif()

# Sanity check linking option.
if (NOT ARROW_LINK)
set(ARROW_LINK "d")
elseif(NOT ("auto" MATCHES "^${ARROW_LINK}" OR
"dynamic" MATCHES "^${ARROW_LINK}" OR
"static" MATCHES "^${ARROW_LINK}"))
message(FATAL_ERROR "Unknown value for ARROW_LINK, must be auto|dynamic|static")
else()
# Remove all but the first letter.
string(SUBSTRING "${ARROW_LINK}" 0 1 ARROW_LINK)
endif()

# ASAN / TSAN / UBSAN
include(san-config)

Expand All @@ -203,61 +206,11 @@ if ("${ARROW_GENERATE_COVERAGE}")
# For coverage to work properly, we need to use static linkage. Otherwise,
# __gcov_flush() doesn't properly flush coverage from every module.
# See http://stackoverflow.com/questions/28164543/using-gcov-flush-within-a-library-doesnt-force-the-other-modules-to-yield-gc
if("${ARROW_LINK}" STREQUAL "a")
message("Using static linking for coverage build")
set(ARROW_LINK "s")
elseif("${ARROW_LINK}" STREQUAL "d")
message(SEND_ERROR "Cannot use coverage with dynamic linking")
endif()
endif()

# If we still don't know what kind of linking to perform, choose based on
# build type (developers like fast builds).
if ("${ARROW_LINK}" STREQUAL "a")
if ("${CMAKE_BUILD_TYPE}" STREQUAL "DEBUG" OR
"${CMAKE_BUILD_TYPE}" STREQUAL "FASTDEBUG")
message("Using dynamic linking for ${CMAKE_BUILD_TYPE} builds")
set(ARROW_LINK "d")
else()
message("Using static linking for ${CMAKE_BUILD_TYPE} builds")
set(ARROW_LINK "s")
if(NOT ARROW_BUILD_STATIC)
message(SEND_ERROR "Coverage requires the static lib to be built")
endif()
endif()

# Are we using the gold linker? It doesn't work with dynamic linking as
# weak symbols aren't properly overridden, causing tcmalloc to be omitted.
# Let's flag this as an error in RELEASE builds (we shouldn't release a
# product like this).
#
# See https://sourceware.org/bugzilla/show_bug.cgi?id=16979 for details.
#
# The gold linker is only for ELF binaries, which OSX doesn't use. We can
# just skip.
if (NOT APPLE)
execute_process(COMMAND ${CMAKE_CXX_COMPILER} -Wl,--version OUTPUT_VARIABLE LINKER_OUTPUT)
endif ()
if (LINKER_OUTPUT MATCHES "gold")
if ("${ARROW_LINK}" STREQUAL "d" AND
"${CMAKE_BUILD_TYPE}" STREQUAL "RELEASE")
message(SEND_ERROR "Cannot use gold with dynamic linking in a RELEASE build "
"as it would cause tcmalloc symbols to get dropped")
else()
message("Using gold linker")
endif()
set(ARROW_USING_GOLD 1)
else()
message("Using ld linker")
endif()

# Having set ARROW_LINK due to build type and/or sanitizer, it's now safe to
# act on its value.
if ("${ARROW_LINK}" STREQUAL "d")
set(BUILD_SHARED_LIBS ON)

# Position independent code is only necessary when producing shared objects.
add_definitions(-fPIC)
endif()

# set compile output directory
string (TOLOWER ${CMAKE_BUILD_TYPE} BUILD_SUBDIR_NAME)

Expand Down Expand Up @@ -290,6 +243,15 @@ set(LIBRARY_OUTPUT_DIRECTORY "${BUILD_OUTPUT_ROOT_DIRECTORY}")
set(EXECUTABLE_OUTPUT_PATH "${BUILD_OUTPUT_ROOT_DIRECTORY}")
include_directories(src)

############################################################
# Visibility
############################################################
# For generate_export_header() and add_compiler_export_flags().
include(GenerateExportHeader)

# Sets -fvisibility=hidden for gcc
add_compiler_export_flags()

############################################################
# Benchmarking
############################################################
Expand Down Expand Up @@ -360,7 +322,7 @@ endfunction()
#
# Arguments after the test name will be passed to set_tests_properties().
function(ADD_ARROW_TEST REL_TEST_NAME)
if(NO_TESTS)
if(NO_TESTS OR NOT ARROW_BUILD_STATIC)
return()
endif()
get_filename_component(TEST_NAME ${REL_TEST_NAME} NAME_WE)
Expand All @@ -377,13 +339,13 @@ function(ADD_ARROW_TEST REL_TEST_NAME)
endif()

if (ARROW_TEST_MEMCHECK)
SET_PROPERTY(TARGET ${TEST_NAME}
APPEND_STRING PROPERTY
COMPILE_FLAGS " -DARROW_VALGRIND")
add_test(${TEST_NAME}
valgrind --tool=memcheck --leak-check=full --error-exitcode=1 ${TEST_PATH})
SET_PROPERTY(TARGET ${TEST_NAME}
APPEND_STRING PROPERTY
COMPILE_FLAGS " -DARROW_VALGRIND")
add_test(${TEST_NAME}
valgrind --tool=memcheck --leak-check=full --error-exitcode=1 ${TEST_PATH})
else()
add_test(${TEST_NAME}
add_test(${TEST_NAME}
${BUILD_SUPPORT_DIR}/run-test.sh ${CMAKE_BINARY_DIR} test ${TEST_PATH})
endif()
set_tests_properties(${TEST_NAME} PROPERTIES LABELS "unittest")
Expand Down Expand Up @@ -427,19 +389,34 @@ function(ADD_THIRDPARTY_LIB LIB_NAME)
message(SEND_ERROR "Error: unrecognized arguments: ${ARG_UNPARSED_ARGUMENTS}")
endif()

if(("${ARROW_LINK}" STREQUAL "s" AND ARG_STATIC_LIB) OR (NOT ARG_SHARED_LIB))
if(ARG_STATIC_LIB AND ARG_SHARED_LIB)
if(NOT ARG_STATIC_LIB)
message(FATAL_ERROR "No static or shared library provided for ${LIB_NAME}")
endif()

SET(AUG_LIB_NAME "${LIB_NAME}_static")
add_library(${AUG_LIB_NAME} STATIC IMPORTED)
set_target_properties(${AUG_LIB_NAME}
PROPERTIES IMPORTED_LOCATION "${ARG_STATIC_LIB}")
message("Added static library dependency ${LIB_NAME}: ${ARG_STATIC_LIB}")

SET(AUG_LIB_NAME "${LIB_NAME}_shared")
add_library(${AUG_LIB_NAME} SHARED IMPORTED)
set_target_properties(${AUG_LIB_NAME}
PROPERTIES IMPORTED_LOCATION "${ARG_SHARED_LIB}")
message("Added shared library dependency ${LIB_NAME}: ${ARG_SHARED_LIB}")
elseif(ARG_STATIC_LIB)
add_library(${LIB_NAME} STATIC IMPORTED)
set_target_properties(${LIB_NAME}
PROPERTIES IMPORTED_LOCATION "${ARG_STATIC_LIB}")
message("Added static library dependency ${LIB_NAME}: ${ARG_STATIC_LIB}")
else()
elseif(ARG_SHARED_LIB)
add_library(${LIB_NAME} SHARED IMPORTED)
set_target_properties(${LIB_NAME}
PROPERTIES IMPORTED_LOCATION "${ARG_SHARED_LIB}")
message("Added shared library dependency ${LIB_NAME}: ${ARG_SHARED_LIB}")
else()
message(FATAL_ERROR "No static or shared library provided for ${LIB_NAME}")
endif()

if(ARG_DEPS)
Expand Down Expand Up @@ -538,9 +515,17 @@ endif()
############################################################
# Linker setup
############################################################
set(ARROW_MIN_TEST_LIBS arrow arrow_test_main ${ARROW_BASE_LIBS})
set(ARROW_MIN_TEST_LIBS
arrow_static
arrow_test_main
${ARROW_BASE_LIBS})

set(ARROW_TEST_LINK_LIBS ${ARROW_MIN_TEST_LIBS})
set(ARROW_BENCHMARK_LINK_LIBS arrow arrow_benchmark_main ${ARROW_BASE_LIBS})

set(ARROW_BENCHMARK_LINK_LIBS
arrow_static
arrow_benchmark_main
${ARROW_BASE_LIBS})

############################################################
# "make ctags" target
Expand Down Expand Up @@ -576,14 +561,14 @@ endif (UNIX)
if (UNIX)

file(GLOB_RECURSE LINT_FILES
"${CMAKE_CURRENT_SOURCE_DIR}/src/*.h"
"${CMAKE_CURRENT_SOURCE_DIR}/src/*.cc"
)
"${CMAKE_CURRENT_SOURCE_DIR}/src/*.h"
"${CMAKE_CURRENT_SOURCE_DIR}/src/*.cc"
)

FOREACH(item ${LINT_FILES})
IF(NOT (item MATCHES "_generated.h"))
IF(NOT (item MATCHES "_generated.h"))
LIST(APPEND FILTERED_LINT_FILES ${item})
ENDIF()
ENDIF()
ENDFOREACH(item ${LINT_FILES})

# Full lint
Expand Down Expand Up @@ -628,7 +613,10 @@ endif()
# Subdirectories
############################################################

set(LIBARROW_LINK_LIBS
set(ARROW_LINK_LIBS
)

set(ARROW_PRIVATE_LINK_LIBS
)

set(ARROW_SRCS
Expand Down Expand Up @@ -660,35 +648,67 @@ set(ARROW_SRCS
src/arrow/util/status.cc
)

set(LIBARROW_LINKAGE "SHARED")

add_library(arrow
${LIBARROW_LINKAGE}
add_library(arrow_objlib OBJECT
${ARROW_SRCS}
)

# Necessary to make static linking into other shared libraries work properly
set_property(TARGET arrow_objlib PROPERTY POSITION_INDEPENDENT_CODE 1)

if(NOT APPLE)
# Localize thirdparty symbols using a linker version script. This hides them
# from the client application. The OS X linker does not support the
# version-script option.
set(SHARED_LINK_FLAGS "-Wl,--version-script=${CMAKE_CURRENT_SOURCE_DIR}/src/arrow/symbols.map")
endif()

if (ARROW_BUILD_SHARED)
add_library(arrow_shared SHARED $<TARGET_OBJECTS:arrow_objlib>)
if(APPLE)
set_target_properties(arrow_shared PROPERTIES LINK_FLAGS "-undefined dynamic_lookup")
endif()
set_target_properties(arrow_shared
PROPERTIES
LIBRARY_OUTPUT_DIRECTORY "${BUILD_OUTPUT_ROOT_DIRECTORY}"
LINK_FLAGS "${SHARED_LINK_FLAGS}"
OUTPUT_NAME "arrow")
target_link_libraries(arrow_shared
LINK_PUBLIC ${ARROW_LINK_LIBS}
LINK_PRIVATE ${ARROW_PRIVATE_LINK_LIBS})

install(TARGETS arrow_shared
LIBRARY DESTINATION lib
ARCHIVE DESTINATION lib)
endif()

if (ARROW_BUILD_STATIC)
add_library(arrow_static STATIC $<TARGET_OBJECTS:arrow_objlib>)
set_target_properties(arrow_static
PROPERTIES
LIBRARY_OUTPUT_DIRECTORY "${BUILD_OUTPUT_ROOT_DIRECTORY}"
OUTPUT_NAME "arrow")

target_link_libraries(arrow_static
LINK_PUBLIC ${ARROW_LINK_LIBS}
LINK_PRIVATE ${ARROW_PRIVATE_LINK_LIBS})

install(TARGETS arrow_static
LIBRARY DESTINATION lib
ARCHIVE DESTINATION lib)
endif()

if (APPLE)
set_target_properties(arrow
set_target_properties(arrow_shared
PROPERTIES
BUILD_WITH_INSTALL_RPATH ON
INSTALL_NAME_DIR "@rpath")
endif()

set_target_properties(arrow
PROPERTIES
LIBRARY_OUTPUT_DIRECTORY "${BUILD_OUTPUT_ROOT_DIRECTORY}"
)
target_link_libraries(arrow ${LIBARROW_LINK_LIBS})

add_subdirectory(src/arrow)
add_subdirectory(src/arrow/io)
add_subdirectory(src/arrow/util)
add_subdirectory(src/arrow/types)

install(TARGETS arrow
LIBRARY DESTINATION lib
ARCHIVE DESTINATION lib)

#----------------------------------------------------------------------
# Parquet adapter library

Expand All @@ -715,7 +735,7 @@ if(ARROW_IPC)
include_directories(SYSTEM ${FLATBUFFERS_INCLUDE_DIR})
add_library(flatbuffers STATIC IMPORTED)
set_target_properties(flatbuffers PROPERTIES
IMPORTED_LOCATION ${FLATBUFFERS_STATIC_LIB})
IMPORTED_LOCATION ${FLATBUFFERS_STATIC_LIB})

add_subdirectory(src/arrow/ipc)
endif()
Loading

0 comments on commit 69b03b0

Please sign in to comment.