You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by we...@apache.org on 2019/01/27 16:55:19 UTC

[arrow] branch master updated: ARROW-4352: [C++] Add support for system Google Test

This is an automated email from the ASF dual-hosted git repository.

wesm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/arrow.git


The following commit(s) were added to refs/heads/master by this push:
     new 91461df  ARROW-4352: [C++] Add support for system Google Test
91461df is described below

commit 91461dfbf1c1ad0191a5042304d96de5f98c3b1a
Author: Kouhei Sutou <ko...@clear-code.com>
AuthorDate: Sun Jan 27 10:55:15 2019 -0600

    ARROW-4352: [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.
    
    Author: Kouhei Sutou <ko...@clear-code.com>
    
    Closes #3469 from kou/cpp-system-gtest and squashes the following commits:
    
    e89e2897 <Kouhei Sutou> Fix lint
    f811d12c <Kouhei Sutou>  Add support for system Google Test
---
 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 736ef84..5309c35 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 1275bc0..1591d86 100644
--- a/cpp/cmake_modules/BuildUtils.cmake
+++ b/cpp/cmake_modules/BuildUtils.cmake
@@ -484,7 +484,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 8a31ae6..c7496c6 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 1668c00..5a75a9b 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 1be46b5..22ce6e9 100644
--- a/cpp/src/arrow/CMakeLists.txt
+++ b/cpp/src/arrow/CMakeLists.txt
@@ -242,8 +242,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 41d49a1..b0006ac 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 2fcdf23..204cb5e 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 a091b01..d33e101 100644
--- a/cpp/src/arrow/io/test-common.h
+++ b/cpp/src/arrow/io/test-common.h
@@ -25,16 +25,11 @@
 #include <string>
 #include <vector>
 
-#ifndef _MSC_VER
-#include <fcntl.h>
-#endif
-
-#if defined(__MINGW32__)  // MinGW
-// nothing
-#elif defined(_MSC_VER)  // Visual Studio
+#ifdef _WIN32
+#include <crtdbg.h>
 #include <io.h>
-#else  // POSIX / Linux
-// nothing
+#else
+#include <fcntl.h>
 #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 0f037ad..7f1a0b5 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 8edde12..30c75ef 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 54ff567..fefc8d6 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 9316854..18cc7e0 100644
--- a/cpp/src/parquet/CMakeLists.txt
+++ b/cpp/src/parquet/CMakeLists.txt
@@ -98,8 +98,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