ARROW-4599: [C++] Add support for system GFlags

kou · kou · commit 24a98e4cab48 · 2019-02-19T15:06:32.000+09:00
This is needed to build tests with MinGW. Author: Kouhei Sutou <kou@clear-code.com> Author: Wes McKinney <wesm+git@apache.org> Closes #3675 from kou/cpp-system-gflags and squashes the following commits: 38f910f <Wes McKinney> Fix rebase artifact 2caf588 <Kouhei Sutou> Define GFLAGS_IS_A_DLL by target 0f43fc9 <Kouhei Sutou> Don't use vendored GFlags when GFLAGS_HOME is specified ca0f67c <Kouhei Sutou> Remove build directory entirely before new configuration build 9516d2f <Kouhei Sutou> GFLAGS_STATIC isn't defined by CONFIG mode 34cb1a6 <Kouhei Sutou> Add support for system GFlags
diff --git a/ci/appveyor-cpp-build.bat b/ci/appveyor-cpp-build.bat
@@ -44,6 +44,7 @@ if "%JOB%" == "Static_Crt_Build" (
   cmake --build . --config Debug || exit /B
   ctest --output-on-failure -j2 || exit /B
   popd
+  rmdir /S /Q cpp\build-debug
 
   mkdir cpp\build-release
   pushd cpp\build-release
diff --git a/ci/travis_before_script_cpp.sh b/ci/travis_before_script_cpp.sh
@@ -53,7 +53,8 @@ CMAKE_COMMON_FLAGS="\
 -DARROW_NO_DEPRECATED_API=ON \
 -DARROW_EXTRA_ERROR_CONTEXT=ON"
 CMAKE_LINUX_FLAGS=""
-CMAKE_OSX_FLAGS=""
+CMAKE_OSX_FLAGS="\
+-DARROW_GFLAGS_USE_SHARED=OFF"
 
 if [ "$ARROW_TRAVIS_USE_TOOLCHAIN" == "1" ]; then
   CMAKE_COMMON_FLAGS="${CMAKE_COMMON_FLAGS} -DARROW_JEMALLOC=ON"
diff --git a/cpp/CMakeLists.txt b/cpp/CMakeLists.txt
@@ -198,6 +198,8 @@ Note that this requires linking Boost statically" OFF)
   option(ARROW_PROTOBUF_USE_SHARED
          "Rely on Protocol Buffers shared libraries where relevant" OFF)
 
+  option(ARROW_GFLAGS_USE_SHARED "Rely on GFlags shared libraries where relevant" ON)
+
   option(ARROW_WITH_BACKTRACE "Build with backtrace support" ON)
 
   option(ARROW_USE_GLOG "Build libraries with glog support for pluggable logging" ON)
diff --git a/cpp/cmake_modules/BuildUtils.cmake b/cpp/cmake_modules/BuildUtils.cmake
@@ -18,7 +18,7 @@
 function(ADD_THIRDPARTY_LIB LIB_NAME)
   set(options)
   set(one_value_args SHARED_LIB STATIC_LIB)
-  set(multi_value_args DEPS)
+  set(multi_value_args DEPS INCLUDE_DIRECTORIES)
   cmake_parse_arguments(ARG "${options}" "${one_value_args}" "${multi_value_args}" ${ARGN})
   if(ARG_UNPARSED_ARGUMENTS)
     message(SEND_ERROR "Error: unrecognized arguments: ${ARG_UNPARSED_ARGUMENTS}")
@@ -38,6 +38,10 @@ function(ADD_THIRDPARTY_LIB LIB_NAME)
         PROPERTIES INTERFACE_LINK_LIBRARIES "${ARG_DEPS}")
     endif()
     message(STATUS "Added static library dependency ${AUG_LIB_NAME}: ${ARG_STATIC_LIB}")
+    if(ARG_INCLUDE_DIRECTORIES)
+      set_target_properties(${AUG_LIB_NAME}
+        PROPERTIES INTERFACE_INCLUDE_DIRECTORIES "${ARG_INCLUDE_DIRECTORIES}")
+    endif()
 
     SET(AUG_LIB_NAME "${LIB_NAME}_shared")
     add_library(${AUG_LIB_NAME} SHARED IMPORTED)
@@ -55,6 +59,10 @@ function(ADD_THIRDPARTY_LIB LIB_NAME)
         PROPERTIES INTERFACE_LINK_LIBRARIES "${ARG_DEPS}")
     endif()
     message(STATUS "Added shared library dependency ${AUG_LIB_NAME}: ${ARG_SHARED_LIB}")
+    if(ARG_INCLUDE_DIRECTORIES)
+      set_target_properties(${AUG_LIB_NAME}
+        PROPERTIES INTERFACE_INCLUDE_DIRECTORIES "${ARG_INCLUDE_DIRECTORIES}")
+    endif()
   elseif(ARG_STATIC_LIB)
     SET(AUG_LIB_NAME "${LIB_NAME}_static")
     add_library(${AUG_LIB_NAME} STATIC IMPORTED)
@@ -65,6 +73,10 @@ function(ADD_THIRDPARTY_LIB LIB_NAME)
         PROPERTIES INTERFACE_LINK_LIBRARIES "${ARG_DEPS}")
     endif()
     message(STATUS "Added static library dependency ${AUG_LIB_NAME}: ${ARG_STATIC_LIB}")
+    if(ARG_INCLUDE_DIRECTORIES)
+      set_target_properties(${AUG_LIB_NAME}
+        PROPERTIES INTERFACE_INCLUDE_DIRECTORIES "${ARG_INCLUDE_DIRECTORIES}")
+    endif()
   elseif(ARG_SHARED_LIB)
     SET(AUG_LIB_NAME "${LIB_NAME}_shared")
     add_library(${AUG_LIB_NAME} SHARED IMPORTED)
@@ -82,6 +94,10 @@ function(ADD_THIRDPARTY_LIB LIB_NAME)
       set_target_properties(${AUG_LIB_NAME}
         PROPERTIES INTERFACE_LINK_LIBRARIES "${ARG_DEPS}")
     endif()
+    if(ARG_INCLUDE_DIRECTORIES)
+      set_target_properties(${AUG_LIB_NAME}
+        PROPERTIES INTERFACE_INCLUDE_DIRECTORIES "${ARG_INCLUDE_DIRECTORIES}")
+    endif()
   else()
     message(FATAL_ERROR "No static or shared library provided for ${LIB_NAME}")
   endif()
diff --git a/cpp/cmake_modules/FindGFlags.cmake b/cpp/cmake_modules/FindGFlags.cmake
@@ -22,58 +22,121 @@
 #  GFLAGS_STATIC_LIB, path to libgflags static library
 #  GFLAGS_FOUND, whether gflags has been found
 
-if( NOT "${GFLAGS_HOME}" STREQUAL "")
-    file( TO_CMAKE_PATH "${GFLAGS_HOME}" _native_path )
-    list( APPEND _gflags_roots ${_native_path} )
-elseif ( GFlags_HOME )
-    list( APPEND _gflags_roots ${GFlags_HOME} )
+find_package(gflags CONFIG)
+set(GFLAGS_FOUND ${gflags_FOUND})
+if(GFLAGS_FOUND)
+  message(STATUS "GFlags include dir: ${GFLAGS_INCLUDE_DIR}")
+  string(TOUPPER "${CMAKE_BUILD_TYPE}" _CONFIG)
+  get_target_property(_GFLAGS_CONFIGURATIONS gflags IMPORTED_CONFIGURATIONS)
+  foreach(_GFLAGS_CONFIG IN LISTS _GFLAGS_CONFIGURATIONS)
+    if("${_GFLAGS_CONFIG}" STREQUAL "${_CONFIG}")
+      set(_GFLAGS_TARGET_CONFIG "${_GFLAGS_CONFIG}")
+    endif()
+  endforeach()
+  if(NOT _GFLAGS_TARGET_CONFIG)
+    list(GET _GFLAGS_CONFIGURATIONS 0 _GFLAGS_TARGET_CONFIG)
+  endif()
+  if(GFLAGS_SHARED)
+    get_target_property(GFLAGS_SHARED_LIB gflags_shared
+                        IMPORTED_LOCATION_${_GFLAGS_TARGET_CONFIG})
+    message(STATUS "GFlags shared library: ${GFLAGS_SHARED_LIB}")
+  endif()
+  if(TARGET gflags_static)
+    set(GFLAGS_STATIC TRUE)
+    get_target_property(GFLAGS_STATIC_LIB gflags_static
+                        IMPORTED_LOCATION_${_GFLAGS_TARGET_CONFIG})
+    message(STATUS "GFlags static library: ${GFLAGS_STATIC_LIB}")
+  endif()
+  unset(_CONFIG)
+  unset(_GFLAGS_CONFIGURATIONS)
+  unset(_GFLAGS_CONFIG)
+  unset(_GFLAGS_TARGET_CONFIG)
+  return()
 endif()
 
-if (MSVC AND NOT DEFINED GFLAGS_MSVC_STATIC_LIB_SUFFIX)
+pkg_check_modules(GFLAGS gflags)
+if(GFLAGS_FOUND)
+  set(GFLAGS_INCLUDE_DIR "${GFLAGS_INCLUDEDIR}")
+  message(STATUS "GFlags include dir: ${GFLAGS_INCLUDE_DIR}")
+  find_library(GFLAGS_SHARED_LIB
+               NAMES "${GFLAGS_LIBRARIES}"
+               PATHS "${GFLAGS_LIBDIR}"
+               NO_DEFAULT_PATH)
+  if(GFLAGS_SHARED_LIB)
+    message(STATUS "GFlags shared library: ${GFLAGS_SHARED_LIB}")
+    add_thirdparty_lib(gflags
+                       SHARED_LIB
+                       "${GFLAGS_SHARED_LIB}"
+                       INCLUDE_DIRECTORIES
+                       "${GFLAGS_INCLUDE_DIR}")
+    target_compile_definitions(gflags_shared INTERFACE "GFLAGS_IS_A_DLL=1")
+    set(GFLAGS_STATIC_LIB FLASE)
+    return()
+  else()
+    set(GFLAGS_FOUND FALSE)
+  endif()
+endif()
+
+if(GFLAGS_HOME)
+  set(GFlags_ROOT "${GFLAGS_HOME}")
+endif()
+if(CMAKE_VERSION VERSION_LESS 3.12)
+  list(INSERT CMAKE_PREFIX_PATH 0 "${GFlags_ROOT}")
+endif()
+
+if(MSVC AND NOT DEFINED GFLAGS_MSVC_STATIC_LIB_SUFFIX)
   set(GFLAGS_MSVC_STATIC_LIB_SUFFIX "_static")
 endif()
 
 set(GFLAGS_STATIC_LIB_SUFFIX
-  "${GFLAGS_MSVC_STATIC_LIB_SUFFIX}${CMAKE_STATIC_LIBRARY_SUFFIX}")
+    "${GFLAGS_MSVC_STATIC_LIB_SUFFIX}${CMAKE_STATIC_LIBRARY_SUFFIX}")
 
 set(GFLAGS_STATIC_LIB_NAME
-  ${CMAKE_STATIC_LIBRARY_PREFIX}gflags${GFLAGS_STATIC_LIB_SUFFIX})
+    "${CMAKE_STATIC_LIBRARY_PREFIX}gflags${GFLAGS_STATIC_LIB_SUFFIX}")
 
-message(STATUS "GFLAGS_HOME: ${GFLAGS_HOME}")
+find_path(GFLAGS_INCLUDE_DIR NAMES gflags/gflags.h)
+find_library(GFLAGS_SHARED_LIB NAMES gflags)
+find_library(GFLAGS_STATIC_LIB NAMES ${GFLAGS_STATIC_LIB_NAME})
 
-if ( _gflags_roots )
-  find_path(GFLAGS_INCLUDE_DIR NAMES gflags/gflags.h
-    PATHS ${_gflags_roots}
-    NO_DEFAULT_PATH
-    PATH_SUFFIXES "include" )
-  find_library(GFLAGS_STATIC_LIB NAMES ${GFLAGS_STATIC_LIB_NAME}
-    PATHS ${_gflags_roots}
-    NO_DEFAULT_PATH
-    PATH_SUFFIXES "lib" )
-  find_library(GFLAGS_SHARED_LIB NAMES gflags
-    PATHS ${_gflags_roots}
-    NO_DEFAULT_PATH
-    PATH_SUFFIXES "lib" )
+if(GFLAGS_INCLUDE_DIR)
+  message(STATUS "GFlags include dir: ${GFLAGS_INCLUDE_DIR}")
+  if(GFLAGS_SHARED_LIB)
+    message(STATUS "GFlags shared library: ${GFLAGS_SHARED_LIB}")
+    set(GFLAGS_SHARED TRUE)
+  else()
+    set(GFLAGS_SHARED FALSE)
+  endif()
+  if(GFLAGS_STATIC_LIB)
+    message(STATUS "GFlags static library: ${GFLAGS_STATIC_LIB}")
+    set(GFLAGS_STATIC TRUE)
+  else()
+    set(GFLAGS_STATIC FALSE)
+  endif()
+  if(GFLAGS_SHARED OR GFLAGS_STATIC)
+    set(GFLAGS_FOUND TRUE)
+    if(MSVC)
+      set(GFLAGS_MSVC_DEPS "shlwapi.lib")
+    endif()
+    add_thirdparty_lib(gflags
+                       SHARED_LIB
+                       "${GFLAGS_SHARED_LIB}"
+                       STATIC_LIB
+                       "${GFLAGS_STATIC_LIB}"
+                       INCLUDE_DIRECTORIES
+                       "${GFLAGS_INCLUDE_DIR}"
+                       DEPS
+                       "${GFLAGS_MSVC_DEPS}")
+    if(GFLAGS_SHARED)
+      target_compile_definitions(gflags_shared INTERFACE "GFLAGS_IS_A_DLL=1")
+    endif()
+    if(GFLAGS_STATIC)
+      target_compile_definitions(gflags_static INTERFACE "GFLAGS_IS_A_DLL=0")
+    endif()
+  else()
+    set(GFLAGS_FOUND FALSE)
+  endif()
 else()
-  find_path(GFLAGS_INCLUDE_DIR gflags/gflags.h
-    # make sure we don't accidentally pick up a different version
-    NO_CMAKE_SYSTEM_PATH
-    NO_SYSTEM_ENVIRONMENT_PATH)
-  find_library(GFLAGS_STATIC_LIB ${GFLAGS_STATIC_LIB_NAME}
-    NO_CMAKE_SYSTEM_PATH
-    NO_SYSTEM_ENVIRONMENT_PATH)
-  find_library(GFLAGS_SHARED_LIB gflags
-    NO_CMAKE_SYSTEM_PATH
-    NO_SYSTEM_ENVIRONMENT_PATH)
-endif()
-
-if (GFLAGS_INCLUDE_DIR AND GFLAGS_STATIC_LIB)
-  set(GFLAGS_FOUND TRUE)
-else ()
   set(GFLAGS_FOUND FALSE)
-endif ()
+endif()
 
-mark_as_advanced(
-  GFLAGS_INCLUDE_DIR
-  GFLAGS_STATIC_LIB
-)
+mark_as_advanced(GFLAGS_INCLUDE_DIR GFLAGS_SHARED_LIB GFLAGS_STATIC_LIB)
diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake
@@ -605,7 +605,19 @@ endif()
 
 if(ARROW_NEED_GFLAGS)
   # gflags (formerly Googleflags) command line parsing
-  if("${GFLAGS_HOME}" STREQUAL "")
+  find_package(GFlags)
+  if(GFLAGS_FOUND)
+    set(GFLAGS_VENDORED FALSE)
+    get_filename_component(GFLAGS_HOME "${GFLAGS_INCLUDE_DIR}" DIRECTORY)
+    if(ARROW_GFLAGS_USE_SHARED AND GFLAGS_SHARED)
+      set(GFLAGS_LIBRARY gflags_shared)
+    else()
+      set(GFLAGS_LIBRARY gflags_static)
+    endif()
+  elseif(GFLAGS_HOME)
+    message(FATAL_ERROR "No static or shared library provided for gflags: ${GFLAGS_HOME}")
+  else()
+    set(GFLAGS_VENDORED TRUE)
     set(GFLAGS_CMAKE_CXX_FLAGS ${EP_CXX_FLAGS})
 
     set(GFLAGS_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/gflags_ep-prefix/src/gflags_ep")
@@ -634,24 +646,20 @@ if(ARROW_NEED_GFLAGS)
       CMAKE_ARGS ${GFLAGS_CMAKE_ARGS})
 
     add_dependencies(toolchain gflags_ep)
-  else()
-    set(GFLAGS_VENDORED 0)
-    find_package(GFlags REQUIRED)
-  endif()
 
-  message(STATUS "GFlags include dir: ${GFLAGS_INCLUDE_DIR}")
-  message(STATUS "GFlags static library: ${GFLAGS_STATIC_LIB}")
-  include_directories(SYSTEM ${GFLAGS_INCLUDE_DIR})
-  ADD_THIRDPARTY_LIB(gflags
-    STATIC_LIB ${GFLAGS_STATIC_LIB})
-  if(MSVC)
-    set_target_properties(gflags_static
-      PROPERTIES
-      INTERFACE_LINK_LIBRARIES "shlwapi.lib")
-  endif()
-
-  if(GFLAGS_VENDORED)
-    add_dependencies(gflags_static gflags_ep)
+    message(STATUS "GFlags include dir: ${GFLAGS_INCLUDE_DIR}")
+    message(STATUS "GFlags static library: ${GFLAGS_STATIC_LIB}")
+      include_directories(SYSTEM ${GFLAGS_INCLUDE_DIR})
+    ADD_THIRDPARTY_LIB(gflags
+      STATIC_LIB ${GFLAGS_STATIC_LIB})
+    set(GFLAGS_LIBRARY gflags_static)
+    target_compile_definitions(${GFLAGS_LIBRARY} INTERFACE "GFLAGS_IS_A_DLL=0")
+    if(MSVC)
+      set_target_properties(${GFLAGS_LIBRARY}
+	PROPERTIES
+	INTERFACE_LINK_LIBRARIES "shlwapi.lib")
+    endif()
+    add_dependencies(${GFLAGS_LIBRARY} gflags_ep)
   endif()
 endif()
 
@@ -1757,6 +1765,6 @@ if (ARROW_USE_GLOG)
   else()
     ADD_THIRDPARTY_LIB(glog
       STATIC_LIB ${GLOG_STATIC_LIB}
-      DEPS gflags_static)
+      DEPS ${GFLAGS_LIBRARY})
   endif()
 endif()
diff --git a/cpp/src/arrow/flight/CMakeLists.txt b/cpp/src/arrow/flight/CMakeLists.txt
@@ -112,16 +112,16 @@ add_arrow_test(flight-test
 # Build test server for unit tests or benchmarks
 if(ARROW_BUILD_TESTS OR ARROW_BUILD_BENCHMARKS)
   add_executable(flight-test-server test-server.cc)
-  target_link_libraries(flight-test-server ${ARROW_FLIGHT_TEST_LINK_LIBS} gflags_static
-                        ${GTEST_LIBRARY})
+  target_link_libraries(flight-test-server ${ARROW_FLIGHT_TEST_LINK_LIBS}
+                        ${GFLAGS_LIBRARY} ${GTEST_LIBRARY})
 
   add_executable(flight-test-integration-server test-integration-server.cc)
   target_link_libraries(flight-test-integration-server ${ARROW_FLIGHT_TEST_LINK_LIBS}
-                        gflags_static gtest_static)
+                        ${GFLAGS_LIBRARY} ${GTEST_LIBRARY})
 
   add_executable(flight-test-integration-client test-integration-client.cc)
   target_link_libraries(flight-test-integration-client ${ARROW_FLIGHT_TEST_LINK_LIBS}
-                        gflags_static gtest_static)
+                        ${GFLAGS_LIBRARY} ${GTEST_LIBRARY})
 
   # This is needed for the unit tests
   if(ARROW_BUILD_TESTS)
@@ -144,15 +144,15 @@ if(ARROW_BUILD_BENCHMARKS)
                         arrow_flight_shared
                         arrow_flight_testing_shared
                         ${ARROW_FLIGHT_TEST_LINK_LIBS}
-                        gflags_static
+                        ${GFLAGS_LIBRARY}
                         ${GTEST_LIBRARY})
 
   add_executable(flight-benchmark flight-benchmark.cc perf.pb.cc)
   target_link_libraries(flight-benchmark
                         arrow_flight_static
                         arrow_testing_static
                         ${ARROW_FLIGHT_TEST_LINK_LIBS}
-                        gflags_static
+                        ${GFLAGS_LIBRARY}
                         ${GTEST_LIBRARY})
 
   add_dependencies(flight-benchmark flight-perf-server)
diff --git a/cpp/src/arrow/ipc/CMakeLists.txt b/cpp/src/arrow/ipc/CMakeLists.txt
@@ -28,7 +28,7 @@ add_arrow_test(json-simple-test PREFIX "arrow-ipc")
 add_arrow_test(json-test PREFIX "arrow-ipc")
 
 if(NOT ARROW_BOOST_HEADER_ONLY)
-  add_arrow_test(json-integration-test EXTRA_LINK_LIBS gflags_static)
+  add_arrow_test(json-integration-test EXTRA_LINK_LIBS ${GFLAGS_LIBRARY})
 
   # Test is being built
   if(TARGET arrow-json-integration-test)
diff --git a/cpp/src/arrow/ipc/json-integration-test.cc b/cpp/src/arrow/ipc/json-integration-test.cc
@@ -15,11 +15,6 @@
 // specific language governing permissions and limitations
 // under the License.
 
-// ARROW-3837: Depending on how gflags was built, this might be set to
-// 1 or 0 by default, making it so that we cannot statically link
-// gflags on Windows if it is set to 1
-#define GFLAGS_IS_A_DLL 0
-
 #include <cstdint>
 #include <cstdio>
 #include <cstring>
diff --git a/run-cmake-format.py b/run-cmake-format.py
@@ -25,6 +25,7 @@
 
 patterns = [
     'cpp/CMakeLists.txt',
+    'cpp/cmake_modules/FindGFlags.cmake',
     'cpp/src/**/CMakeLists.txt',
     'cpp/tools/**/CMakeLists.txt',
     'java/gandiva/CMakeLists.txt',
