From f811d12c1b629a1e9ce90d715e857af3d7caa81a Mon Sep 17 00:00:00 2001 From: Kouhei Sutou Date: Thu, 24 Jan 2019 11:00:18 +0900 Subject: [PATCH 1/2] [C++] Add support for system Google Test Google Test is installed in /usr/lib/x86_64-linux-gnu on Debian. Google Test is installed as shared library on MSYS2. This change includes some fixes to build tests with MinGW. They are just for building some tests. They aren't completed changes yet. We need more works to run all tests with MinGW build. --- cpp/CMakeLists.txt | 8 +-- cpp/cmake_modules/BuildUtils.cmake | 2 +- cpp/cmake_modules/FindGTest.cmake | 71 +++++++++++++++------ cpp/cmake_modules/ThirdpartyToolchain.cmake | 26 ++++++-- cpp/src/arrow/CMakeLists.txt | 4 +- cpp/src/arrow/flight/CMakeLists.txt | 6 +- cpp/src/arrow/gpu/CMakeLists.txt | 2 +- cpp/src/arrow/io/test-common.h | 17 ++--- cpp/src/arrow/python/CMakeLists.txt | 2 +- cpp/src/arrow/python/util/CMakeLists.txt | 4 +- cpp/src/arrow/util/CMakeLists.txt | 2 +- cpp/src/parquet/CMakeLists.txt | 4 +- 12 files changed, 92 insertions(+), 56 deletions(-) diff --git a/cpp/CMakeLists.txt b/cpp/CMakeLists.txt index fa696707869..b8a0154716b 100644 --- a/cpp/CMakeLists.txt +++ b/cpp/CMakeLists.txt @@ -796,8 +796,8 @@ set(ARROW_TEST_STATIC_LINK_LIBS arrow_testing_static arrow_static ${ARROW_LINK_LIBS} - gtest_main_static - gtest_static) + ${GTEST_MAIN_LIBRARY} + ${GTEST_LIBRARY}) set(ARROW_TEST_SHARED_LINK_LIBS arrow_testing_shared @@ -807,8 +807,8 @@ set(ARROW_TEST_SHARED_LINK_LIBS ${BOOST_SYSTEM_LIBRARY} ${BOOST_FILESYSTEM_LIBRARY} ${BOOST_REGEX_LIBRARY} - gtest_main_static - gtest_static) + ${GTEST_MAIN_LIBRARY} + ${GTEST_LIBRARY}) if(NOT MSVC) set(ARROW_TEST_SHARED_LINK_LIBS diff --git a/cpp/cmake_modules/BuildUtils.cmake b/cpp/cmake_modules/BuildUtils.cmake index fffd15819f8..f2ddad5c45c 100644 --- a/cpp/cmake_modules/BuildUtils.cmake +++ b/cpp/cmake_modules/BuildUtils.cmake @@ -478,7 +478,7 @@ function(ADD_TEST_CASE REL_TEST_NAME) bash -c "cd '${CMAKE_SOURCE_DIR}'; \ valgrind --suppressions=valgrind.supp --tool=memcheck --gen-suppressions=all \ --leak-check=full --leak-check-heuristics=stdstring --error-exitcode=1 ${TEST_PATH}") - elseif(MSVC) + elseif(WIN32) add_test(${TEST_NAME} ${TEST_PATH}) else() add_test(${TEST_NAME} diff --git a/cpp/cmake_modules/FindGTest.cmake b/cpp/cmake_modules/FindGTest.cmake index 8a31ae6e063..c7496c6a3b9 100644 --- a/cpp/cmake_modules/FindGTest.cmake +++ b/cpp/cmake_modules/FindGTest.cmake @@ -28,7 +28,9 @@ # GTEST_INCLUDE_DIR, directory containing headers # GTEST_LIBS, directory containing gtest libraries # GTEST_STATIC_LIB, path to libgtest.a +# GTEST_STATIC_MAIN_LIB, path to libgtest_main.a # GTEST_SHARED_LIB, path to libgtest's shared library +# GTEST_SHARED_MAIN_LIB, path to libgtest_main's shared library # GTEST_FOUND, whether gtest has been found if( NOT "${GTEST_HOME}" STREQUAL "") @@ -38,34 +40,60 @@ elseif ( GTest_HOME ) list( APPEND _gtest_roots ${GTest_HOME} ) endif() +set(GTEST_STATIC_LIB_NAME + ${CMAKE_STATIC_LIBRARY_PREFIX}gtest${CMAKE_STATIC_LIBRARY_SUFFIX}) +set(GTEST_MAIN_STATIC_LIB_NAME + ${CMAKE_STATIC_LIBRARY_PREFIX}gtest_main${CMAKE_STATIC_LIBRARY_SUFFIX}) +set(GTEST_SHARED_LIB_NAME + ${CMAKE_SHARED_LIBRARY_PREFIX}gtest${CMAKE_SHARED_LIBRARY_SUFFIX}) +set(GTEST_MAIN_SHARED_LIB_NAME + ${CMAKE_SHARED_LIBRARY_PREFIX}gtest_main${CMAKE_SHARED_LIBRARY_SUFFIX}) + # Try the parameterized roots, if they exist -if ( _gtest_roots ) - find_path( GTEST_INCLUDE_DIR NAMES gtest/gtest.h - PATHS ${_gtest_roots} NO_DEFAULT_PATH - PATH_SUFFIXES "include" ) - find_library( GTEST_LIBRARIES NAMES gtest gtest_main - PATHS ${_gtest_roots} NO_DEFAULT_PATH - PATH_SUFFIXES "lib" ) -else () - find_path( GTEST_INCLUDE_DIR NAMES gtest/gtest.h ) - find_library( GTEST_LIBRARIES NAMES gtest ) -endif () +if(_gtest_roots) + find_path(GTEST_INCLUDE_DIR NAMES gtest/gtest.h + PATHS ${_gtest_roots} NO_DEFAULT_PATH + PATH_SUFFIXES "include") + set(lib_dirs + "lib/${CMAKE_LIBRARY_ARCHITECTURE}" + "lib64" + "lib") + find_library(GTEST_STATIC_LIB NAMES ${GTEST_STATIC_LIB_NAME} + PATHS ${_gtest_roots} NO_DEFAULT_PATH + PATH_SUFFIXES ${lib_dirs}) + find_library(GTEST_MAIN_STATIC_LIB NAMES ${GTEST_MAIN_STATIC_LIB_NAME} + PATHS ${_gtest_roots} NO_DEFAULT_PATH + PATH_SUFFIXES ${lib_dirs}) + find_library(GTEST_SHARED_LIB NAMES ${GTEST_SHARED_LIB_NAME} + PATHS ${_gtest_roots} NO_DEFAULT_PATH + PATH_SUFFIXES ${lib_dirs}) + find_library(GTEST_MAIN_SHARED_LIB NAMES ${GTEST_MAIN_SHARED_LIB_NAME} + PATHS ${_gtest_roots} NO_DEFAULT_PATH + PATH_SUFFIXES ${lib_dirs}) +else() + find_path(GTEST_INCLUDE_DIR NAMES gtest/gtest.h) + find_library(GTEST_STATIC_LIB NAMES ${GTEST_STATIC_LIB_NAME}) + find_library(GTEST_MAIN_STATIC_LIB NAMES ${GTEST_MAIN_STATIC_LIB_NAME}) + find_library(GTEST_SHARED_LIB NAMES ${GTEST_SHARED_LIB_NAME}) + find_library(GTEST_MAIN_SHARED_LIB NAMES ${GTEST_MAIN_SHARED_LIB_NAME}) +endif() -if (GTEST_INCLUDE_DIR AND GTEST_LIBRARIES) +if(GTEST_INCLUDE_DIR AND + (GTEST_STATIC_LIB AND GTEST_MAIN_STATIC_LIB) OR + (GTEST_SHARED_LIB AND GTEST_MAIN_SHARED_LIB)) set(GTEST_FOUND TRUE) - get_filename_component( GTEST_LIBS ${GTEST_LIBRARIES} PATH ) - set(GTEST_LIB_NAME gtest) - set(GTEST_STATIC_LIB ${GTEST_LIBS}/${CMAKE_STATIC_LIBRARY_PREFIX}${GTEST_LIB_NAME}${CMAKE_STATIC_LIBRARY_SUFFIX}) - set(GTEST_MAIN_STATIC_LIB ${GTEST_LIBS}/${CMAKE_STATIC_LIBRARY_PREFIX}${GTEST_LIB_NAME}_main${CMAKE_STATIC_LIBRARY_SUFFIX}) - set(GTEST_SHARED_LIB ${GTEST_LIBS}/${CMAKE_SHARED_LIBRARY_PREFIX}${GTEST_LIB_NAME}${CMAKE_SHARED_LIBRARY_SUFFIX}) -else () +else() set(GTEST_FOUND FALSE) -endif () +endif() if (GTEST_FOUND) if (NOT GTest_FIND_QUIETLY) - message(STATUS "Found the GTest library: ${GTEST_LIBRARIES}") + message(STATUS "Found the GTest library:") + message(STATUS "GTEST_STATIC_LIB: ${GTEST_STATIC_LIB}") + message(STATUS "GTEST_MAIN_STATIC_LIB: ${GTEST_MAIN_STATIC_LIB}") + message(STATUS "GTEST_SHARED_LIB: ${GTEST_SHARED_LIB}") + message(STATUS "GTEST_MAIN_SHARED_LIB: ${GTEST_MAIN_SHARED_LIB}") endif () else () if (NOT GTest_FIND_QUIETLY) @@ -86,7 +114,8 @@ endif () mark_as_advanced( GTEST_INCLUDE_DIR GTEST_LIBS - GTEST_LIBRARIES GTEST_STATIC_LIB + GTEST_MAIN_STATIC_LIB GTEST_SHARED_LIB + GTEST_MAIN_SHARED_LIB ) diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake index 1668c00cd44..5a75a9b00d7 100644 --- a/cpp/cmake_modules/ThirdpartyToolchain.cmake +++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake @@ -634,16 +634,28 @@ if(ARROW_BUILD_TESTS OR ARROW_BUILD_BENCHMARKS) endif() message(STATUS "GTest include dir: ${GTEST_INCLUDE_DIR}") - message(STATUS "GTest static library: ${GTEST_STATIC_LIB}") include_directories(SYSTEM ${GTEST_INCLUDE_DIR}) - ADD_THIRDPARTY_LIB(gtest - STATIC_LIB ${GTEST_STATIC_LIB}) - ADD_THIRDPARTY_LIB(gtest_main - STATIC_LIB ${GTEST_MAIN_STATIC_LIB}) + if(GTEST_STATIC_LIB) + message(STATUS "GTest static library: ${GTEST_STATIC_LIB}") + ADD_THIRDPARTY_LIB(gtest + STATIC_LIB ${GTEST_STATIC_LIB}) + ADD_THIRDPARTY_LIB(gtest_main + STATIC_LIB ${GTEST_MAIN_STATIC_LIB}) + set(GTEST_LIBRARY gtest_static) + set(GTEST_MAIN_LIBRARY gtest_main_static) + else() + message(STATUS "GTest shared library: ${GTEST_SHARED_LIB}") + ADD_THIRDPARTY_LIB(gtest + SHARED_LIB ${GTEST_SHARED_LIB}) + ADD_THIRDPARTY_LIB(gtest_main + SHARED_LIB ${GTEST_MAIN_SHARED_LIB}) + set(GTEST_LIBRARY gtest_shared) + set(GTEST_MAIN_LIBRARY gtest_main_shared) + endif() if(GTEST_VENDORED) - add_dependencies(gtest_static googletest_ep) - add_dependencies(gtest_main_static googletest_ep) + add_dependencies(${GTEST_LIBRARY} googletest_ep) + add_dependencies(${GTEST_MAIN_LIBRARY} googletest_ep) endif() endif() diff --git a/cpp/src/arrow/CMakeLists.txt b/cpp/src/arrow/CMakeLists.txt index 244d0b9342f..a33fce372d7 100644 --- a/cpp/src/arrow/CMakeLists.txt +++ b/cpp/src/arrow/CMakeLists.txt @@ -241,8 +241,8 @@ if (ARROW_BUILD_TESTS OR ARROW_BUILD_BENCHMARKS) ADD_ARROW_LIB(arrow_testing SOURCES test-util.cc OUTPUTS ARROW_TESTING_LIBRARIES - DEPENDENCIES gtest_static - SHARED_LINK_LIBS arrow_shared gtest_static + DEPENDENCIES ${GTEST_LIBRARY} + SHARED_LINK_LIBS arrow_shared ${GTEST_LIBRARY} STATIC_LINK_LIBS arrow_static) if (ARROW_BUILD_STATIC AND WIN32) diff --git a/cpp/src/arrow/flight/CMakeLists.txt b/cpp/src/arrow/flight/CMakeLists.txt index 41d49a12008..b0006ac7f5c 100644 --- a/cpp/src/arrow/flight/CMakeLists.txt +++ b/cpp/src/arrow/flight/CMakeLists.txt @@ -88,7 +88,7 @@ if (ARROW_BUILD_TESTS OR ARROW_BUILD_BENCHMARKS) target_link_libraries(flight-test-server ${ARROW_FLIGHT_TEST_STATIC_LINK_LIBS} gflags_static - gtest_static) + ${GTEST_LIBRARY}) # This is needed for the unit tests if (ARROW_BUILD_TESTS) @@ -117,7 +117,7 @@ if (ARROW_BUILD_BENCHMARKS) arrow_flight_static ${ARROW_FLIGHT_STATIC_LINK_LIBS} gflags_static - gtest_static) + ${GTEST_LIBRARY}) add_executable(flight-benchmark flight-benchmark.cc @@ -126,7 +126,7 @@ if (ARROW_BUILD_BENCHMARKS) arrow_flight_static ${ARROW_FLIGHT_STATIC_LINK_LIBS} gflags_static - gtest_static) + ${GTEST_LIBRARY}) add_dependencies(flight-benchmark flight-perf-server) endif(ARROW_BUILD_BENCHMARKS) diff --git a/cpp/src/arrow/gpu/CMakeLists.txt b/cpp/src/arrow/gpu/CMakeLists.txt index 2fcdf23e42a..204cb5e313f 100644 --- a/cpp/src/arrow/gpu/CMakeLists.txt +++ b/cpp/src/arrow/gpu/CMakeLists.txt @@ -88,7 +88,7 @@ if (ARROW_BUILD_BENCHMARKS) cuda_add_executable(arrow-cuda-benchmark cuda-benchmark.cc) target_link_libraries(arrow-cuda-benchmark arrow_cuda_shared - gtest_static + ${GTEST_LIBRARY} ${ARROW_BENCHMARK_LINK_LIBS}) add_dependencies(arrow_cuda-benchmarks arrow-cuda-benchmark) endif() diff --git a/cpp/src/arrow/io/test-common.h b/cpp/src/arrow/io/test-common.h index a091b01d32c..93a0952935d 100644 --- a/cpp/src/arrow/io/test-common.h +++ b/cpp/src/arrow/io/test-common.h @@ -25,16 +25,11 @@ #include #include -#ifndef _MSC_VER -#include -#endif - -#if defined(__MINGW32__) // MinGW -// nothing -#elif defined(_MSC_VER) // Visual Studio +#ifdef _WIN32 #include -#else // POSIX / Linux -// nothing +#include +#else +#include #endif #include "arrow/buffer.h" @@ -64,7 +59,7 @@ static inline bool FileExists(const std::string& path) { return std::ifstream(path.c_str()).good(); } -#if defined(_MSC_VER) +#if defined(_WIN32) static inline void InvalidParamHandler(const wchar_t* expr, const wchar_t* func, const wchar_t* source_file, unsigned int source_line, uintptr_t reserved) { @@ -74,7 +69,7 @@ static inline void InvalidParamHandler(const wchar_t* expr, const wchar_t* func, #endif static inline bool FileIsClosed(int fd) { -#if defined(_MSC_VER) +#if defined(_WIN32) // Disables default behavior on wrong params which causes the application to crash // https://msdn.microsoft.com/en-us/library/ksazx244.aspx _set_invalid_parameter_handler(InvalidParamHandler); diff --git a/cpp/src/arrow/python/CMakeLists.txt b/cpp/src/arrow/python/CMakeLists.txt index 0f037ad4b05..7f1a0b5086e 100644 --- a/cpp/src/arrow/python/CMakeLists.txt +++ b/cpp/src/arrow/python/CMakeLists.txt @@ -108,7 +108,7 @@ if (ARROW_BUILD_TESTS) util/test_main.cc) target_link_libraries(arrow_python_test_main - gtest_static) + ${GTEST_LIBRARY}) target_include_directories(arrow_python_test_main SYSTEM PUBLIC ${ARROW_PYTHON_INCLUDES}) diff --git a/cpp/src/arrow/python/util/CMakeLists.txt b/cpp/src/arrow/python/util/CMakeLists.txt index 8edde12558f..30c75ef4509 100644 --- a/cpp/src/arrow/python/util/CMakeLists.txt +++ b/cpp/src/arrow/python/util/CMakeLists.txt @@ -25,13 +25,13 @@ if (PYARROW_BUILD_TESTS) if (APPLE) target_link_libraries(arrow/python_test_main - gtest_static + ${GTEST_LIBRARY} dl) set_target_properties(arrow/python_test_main PROPERTIES LINK_FLAGS "-undefined dynamic_lookup") else() target_link_libraries(arrow/python_test_main - gtest_static + ${GTEST_LIBRARY} pthread dl ) diff --git a/cpp/src/arrow/util/CMakeLists.txt b/cpp/src/arrow/util/CMakeLists.txt index 54ff5674fdf..fefc8d6da80 100644 --- a/cpp/src/arrow/util/CMakeLists.txt +++ b/cpp/src/arrow/util/CMakeLists.txt @@ -45,7 +45,7 @@ if (ARROW_BUILD_BENCHMARKS) endif() # TODO(wesm): Some benchmarks include gtest.h - add_dependencies(arrow_benchmark_main gtest_static) + add_dependencies(arrow_benchmark_main ${GTEST_LIBRARY}) endif() ADD_ARROW_TEST(bit-util-test) diff --git a/cpp/src/parquet/CMakeLists.txt b/cpp/src/parquet/CMakeLists.txt index f6796726fce..96eb90e4a20 100644 --- a/cpp/src/parquet/CMakeLists.txt +++ b/cpp/src/parquet/CMakeLists.txt @@ -81,8 +81,8 @@ if(MSVC) endif() set(PARQUET_MIN_TEST_LIBS - gtest_main_static - gtest_static) + ${GTEST_MAIN_LIBRARY} + ${GTEST_LIBRARY}) if (APPLE) set(PARQUET_MIN_TEST_LIBS From e89e289711a6b7d4942c35d9ce003bf927633f3b Mon Sep 17 00:00:00 2001 From: Kouhei Sutou Date: Thu, 24 Jan 2019 14:17:50 +0900 Subject: [PATCH 2/2] Fix lint --- cpp/src/arrow/io/test-common.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/src/arrow/io/test-common.h b/cpp/src/arrow/io/test-common.h index 93a0952935d..d33e1011756 100644 --- a/cpp/src/arrow/io/test-common.h +++ b/cpp/src/arrow/io/test-common.h @@ -26,8 +26,8 @@ #include #ifdef _WIN32 -#include #include +#include #else #include #endif