You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by ko...@apache.org on 2019/06/17 00:25:18 UTC

[arrow] branch master updated: ARROW-5370: [C++] Use system uriparser if available

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

kou 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 e1aebf6  ARROW-5370: [C++] Use system uriparser if available
e1aebf6 is described below

commit e1aebf61137fdc143e3f845326f4186efc45ae01
Author: Sutou Kouhei <ko...@clear-code.com>
AuthorDate: Mon Jun 17 09:25:08 2019 +0900

    ARROW-5370: [C++] Use system uriparser if available
    
    This change will use uriparser >= 0.9.0 (found via pkgconfig) if it is available in the system toolchain, otherwise it will build from source. Using the SYSTEM method it will not fail for now if the version is not new enough given that the version we require is too new for many Linux distributions. We can look into supporting older uriparser some other time
    
    Author: Sutou Kouhei <ko...@clear-code.com>
    Author: Wes McKinney <we...@apache.org>
    
    Closes #4525 from wesm/ARROW-5370 and squashes the following commits:
    
    e5e40cc83 <Sutou Kouhei> Disable Flight for now
    f26faaa79 <Sutou Kouhei> Install gRPC package for Flight
    8398c2301 <Sutou Kouhei> Enable Flight on MinGW to use uriparser
    5a06e0605 <Sutou Kouhei> Use consistent order
    df2f76fc0 <Sutou Kouhei> Need variable expansion for empty string
    b2bfcff91 <Sutou Kouhei> Use JIRA issue number
    0464cf8d6 <Sutou Kouhei> Use AUTO for uriparser even if ARROW_DEPENDENCY_SOURCE is CONDA
    3caabe4da <Sutou Kouhei> Reuse existing Finduriparser.cmake
    b7cec7c77 <Wes McKinney> Incorporate kou's suggestion
    177fd9508 <Wes McKinney> Use system liburiparser-dev if available
---
 ci/appveyor-cpp-build-mingw.bat             |  9 ++-
 ci/appveyor-cpp-setup-mingw.bat             |  2 +
 cpp/cmake_modules/BuildUtils.cmake          |  1 +
 cpp/cmake_modules/Finduriparser.cmake       | 53 ----------------
 cpp/cmake_modules/FinduriparserAlt.cmake    | 96 +++++++++++++++++++++++++++++
 cpp/cmake_modules/ThirdpartyToolchain.cmake | 29 +++++++--
 run-cmake-format.py                         |  1 +
 7 files changed, 127 insertions(+), 64 deletions(-)

diff --git a/ci/appveyor-cpp-build-mingw.bat b/ci/appveyor-cpp-build-mingw.bat
index 013a5d9..0fccf2a 100644
--- a/ci/appveyor-cpp-build-mingw.bat
+++ b/ci/appveyor-cpp-build-mingw.bat
@@ -42,17 +42,16 @@ pushd %CPP_BUILD_DIR%
 
 cmake ^
     -G "MSYS Makefiles" ^
-    -DCMAKE_INSTALL_PREFIX=%INSTALL_DIR% ^
-    -DCMAKE_BUILD_TYPE=%CMAKE_BUILD_TYPE% ^
     -DARROW_BUILD_STATIC=OFF ^
-    -DARROW_VERBOSE_THIRDPARTY_BUILD=OFF ^
+    -DARROW_BUILD_TESTS=ON ^
     -DARROW_PACKAGE_PREFIX=%MINGW_PREFIX% ^
-    -DARROW_USE_GLOG=OFF ^
     -DARROW_PARQUET=ON ^
     -DARROW_PYTHON=ON ^
+    -DARROW_USE_GLOG=OFF ^
+    -DCMAKE_BUILD_TYPE=%CMAKE_BUILD_TYPE% ^
+    -DCMAKE_INSTALL_PREFIX=%INSTALL_DIR% ^
     -DPythonInterp_FIND_VERSION=ON ^
     -DPythonInterp_FIND_VERSION_MAJOR=3 ^
-    -DARROW_BUILD_TESTS=ON ^
     .. || exit /B
 make -j4 || exit /B
 setlocal
diff --git a/ci/appveyor-cpp-setup-mingw.bat b/ci/appveyor-cpp-setup-mingw.bat
index 18f66c5..b58f8ee 100644
--- a/ci/appveyor-cpp-setup-mingw.bat
+++ b/ci/appveyor-cpp-setup-mingw.bat
@@ -43,6 +43,7 @@ pacman --sync --noconfirm ^
   %MINGW_PACKAGE_PREFIX%-flatbuffers ^
   %MINGW_PACKAGE_PREFIX%-gflags ^
   %MINGW_PACKAGE_PREFIX%-gobject-introspection ^
+  %MINGW_PACKAGE_PREFIX%-grpc ^
   %MINGW_PACKAGE_PREFIX%-gtest ^
   %MINGW_PACKAGE_PREFIX%-gtk-doc ^
   %MINGW_PACKAGE_PREFIX%-lz4 ^
@@ -52,6 +53,7 @@ pacman --sync --noconfirm ^
   %MINGW_PACKAGE_PREFIX%-rapidjson ^
   %MINGW_PACKAGE_PREFIX%-snappy ^
   %MINGW_PACKAGE_PREFIX%-thrift ^
+  %MINGW_PACKAGE_PREFIX%-uriparser ^
   %MINGW_PACKAGE_PREFIX%-zlib ^
   %MINGW_PACKAGE_PREFIX%-zstd || exit /B
 
diff --git a/cpp/cmake_modules/BuildUtils.cmake b/cpp/cmake_modules/BuildUtils.cmake
index 293a7ef..a5fd31c 100644
--- a/cpp/cmake_modules/BuildUtils.cmake
+++ b/cpp/cmake_modules/BuildUtils.cmake
@@ -20,6 +20,7 @@
 # search there as well.
 set(LIB_PATH_SUFFIXES
     "${CMAKE_LIBRARY_ARCHITECTURE}"
+    "lib/${CMAKE_LIBRARY_ARCHITECTURE}"
     "lib64"
     "lib32"
     "lib"
diff --git a/cpp/cmake_modules/Finduriparser.cmake b/cpp/cmake_modules/Finduriparser.cmake
deleted file mode 100644
index a24cca4..0000000
--- a/cpp/cmake_modules/Finduriparser.cmake
+++ /dev/null
@@ -1,53 +0,0 @@
-# Licensed to the Apache Software Foundation (ASF) under one
-# or more contributor license agreements.  See the NOTICE file
-# distributed with this work for additional information
-# regarding copyright ownership.  The ASF licenses this file
-# to you under the Apache License, Version 2.0 (the
-# "License"); you may not use this file except in compliance
-# with the License.  You may obtain a copy of the License at
-#
-#   http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing,
-# software distributed under the License is distributed on an
-# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
-# KIND, either express or implied.  See the License for the
-# specific language governing permissions and limitations
-# under the License.
-
-pkg_check_modules(uriparser_PC liburiparser)
-if(uriparser_PC_FOUND)
-  set(uriparser_INCLUDE_DIR "${uriparser_PC_INCLUDEDIR}")
-
-  list(APPEND uriparser_PC_LIBRARY_DIRS "${uriparser_PC_LIBDIR}")
-  find_library(uriparser_LIB uriparser
-               PATHS ${uriparser_PC_LIBRARY_DIRS}
-               NO_DEFAULT_PATH
-               PATH_SUFFIXES "${CMAKE_LIBRARY_ARCHITECTURE}")
-elseif(uriparser_ROOT)
-  message(STATUS "Using uriparser_ROOT: ${uriparser_ROOT}")
-  find_library(uriparser_LIB
-               NAMES uriparser
-               PATHS ${uriparser_ROOT}
-               PATH_SUFFIXES ${LIB_PATH_SUFFIXES}
-               NO_DEFAULT_PATH)
-  find_path(uriparser_INCLUDE_DIR
-            NAMES uriparser/Uri.h
-            PATHS ${uriparser_ROOT}
-            NO_DEFAULT_PATH
-            PATH_SUFFIXES ${INCLUDE_PATH_SUFFIXES})
-else()
-  find_library(uriparser_LIB
-               NAMES uriparser
-               PATH_SUFFIXES ${LIB_PATH_SUFFIXES})
-  find_path(uriparser_INCLUDE_DIR NAMES uriparser/Uri.h PATH_SUFFIXES ${INCLUDE_PATH_SUFFIXES})
-endif()
-
-find_package_handle_standard_args(uriparser REQUIRED_VARS uriparser_LIB uriparser_INCLUDE_DIR)
-
-if(uriparser_FOUND)
-  add_library(uriparser::uriparser UNKNOWN IMPORTED)
-  set_target_properties(uriparser::uriparser
-                        PROPERTIES IMPORTED_LOCATION "${uriparser_LIB}"
-                                   INTERFACE_INCLUDE_DIRECTORIES "${uriparser_INCLUDE_DIR}")
-endif()
diff --git a/cpp/cmake_modules/FinduriparserAlt.cmake b/cpp/cmake_modules/FinduriparserAlt.cmake
new file mode 100644
index 0000000..567527b
--- /dev/null
+++ b/cpp/cmake_modules/FinduriparserAlt.cmake
@@ -0,0 +1,96 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+if(uriparser_ROOT)
+  find_library(uriparser_LIB
+               NAMES uriparser
+               PATHS ${uriparser_ROOT}
+               PATH_SUFFIXES ${LIB_PATH_SUFFIXES}
+               NO_DEFAULT_PATH)
+  find_path(uriparser_INCLUDE_DIR
+            NAMES uriparser/Uri.h
+            PATHS ${uriparser_ROOT}
+            PATH_SUFFIXES ${INCLUDE_PATH_SUFFIXES}
+            NO_DEFAULT_PATH)
+else()
+  set(uriparser_PC_MODULE liburiparser)
+  if(uriparserAlt_FIND_VERSION)
+    set(uriparser_PC_MODULE "${uriparser_PC_MODULE} >= ${uriparserAlt_FIND_VERSION}")
+  endif()
+  pkg_check_modules(uriparser_PC ${uriparser_PC_MODULE})
+  if(uriparser_PC_FOUND)
+    set(uriparser_VERSION "${uriparser_PC_VERSION}")
+    set(uriparser_INCLUDE_DIR "${uriparser_PC_INCLUDEDIR}")
+    list(APPEND uriparser_PC_LIBRARY_DIRS "${uriparser_PC_LIBDIR}")
+    find_library(uriparser_LIB uriparser
+                 PATHS ${uriparser_PC_LIBRARY_DIRS}
+                 PATH_SUFFIXES "${CMAKE_LIBRARY_ARCHITECTURE}"
+                 NO_DEFAULT_PATH)
+  else()
+    find_library(uriparser_LIB NAMES uriparser PATH_SUFFIXES ${LIB_PATH_SUFFIXES})
+    find_path(uriparser_INCLUDE_DIR
+              NAMES uriparser/Uri.h
+              PATH_SUFFIXES ${INCLUDE_PATH_SUFFIXES})
+  endif()
+endif()
+
+if(NOT uriparser_VERSION AND uriparser_INCLUDE_DIR)
+  file(READ "${uriparser_INCLUDE_DIR}/uriparser/UriBase.h" uriparser_URI_BASE_H_CONTENT)
+  string(REGEX MATCH "#define URI_VER_MAJOR +[0-9]+" uriparser_MAJOR_VERSION_DEFINITION
+               "${uriparser_URI_BASE_H_CONTENT}")
+  string(REGEX
+         REPLACE "^.+ +([0-9]+)$" "\\1" uriparser_MAJOR_VERSION
+                 "${uriparser_MAJOR_VERSION_DEFINITION}")
+  string(REGEX MATCH "#define URI_VER_MINOR +[0-9]+" uriparser_MINOR_VERSION_DEFINITION
+               "${uriparser_URI_BASE_H_CONTENT}")
+  string(REGEX
+         REPLACE "^.+ +([0-9]+)$" "\\1" uriparser_MINOR_VERSION
+                 "${uriparser_MINOR_VERSION_DEFINITION}")
+  string(REGEX MATCH "#define URI_VER_RELEASE +[0-9]+"
+               uriparser_RELEASE_VERSION_DEFINITION "${uriparser_URI_BASE_H_CONTENT}")
+  string(REGEX
+         REPLACE "^.+ +([0-9]+)$" "\\1" uriparser_RELEASE_VERSION
+                 "${uriparser_RELEASE_VERSION_DEFINITION}")
+  if("${uriparser_MAJOR_VERSION}" STREQUAL ""
+     OR "${uriparser_MINOR_VERSION}" STREQUAL ""
+     OR "${uriparser_RELEASE_VERSION}" STREQUAL "")
+    set(uriparser_VERSION "0.0.0")
+  else()
+    set(
+      uriparser_VERSION
+
+      "${uriparser_MAJOR_VERSION}.${uriparser_MINOR_VERSION}.${uriparser_RELEASE_VERSION}"
+      )
+  endif()
+endif()
+
+find_package_handle_standard_args(uriparserAlt
+                                  REQUIRED_VARS
+                                  uriparser_LIB
+                                  uriparser_INCLUDE_DIR
+                                  VERSION_VAR
+                                  uriparser_VERSION)
+
+if(uriparserAlt_FOUND)
+  add_library(uriparser::uriparser UNKNOWN IMPORTED)
+  set_target_properties(uriparser::uriparser
+                        PROPERTIES IMPORTED_LOCATION "${uriparser_LIB}"
+                                   INTERFACE_INCLUDE_DIRECTORIES
+                                   "${uriparser_INCLUDE_DIR}"
+                                   # URI_STATIC_BUILD required on Windows
+                                   INTERFACE_COMPILE_DEFINITIONS "URI_NO_UNICODE")
+endif()
diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake
index 3d922e1..eed156d 100644
--- a/cpp/cmake_modules/ThirdpartyToolchain.cmake
+++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake
@@ -51,7 +51,6 @@ endmacro()
 # ----------------------------------------------------------------------
 # Resolve the dependencies
 
-# TODO: add uriparser here when it gets a conda package
 set(ARROW_THIRDPARTY_DEPENDENCIES
     benchmark
     BOOST
@@ -71,6 +70,7 @@ set(ARROW_THIRDPARTY_DEPENDENCIES
     RapidJSON
     Snappy
     Thrift
+    uriparser
     ZLIB
     ZSTD)
 
@@ -93,6 +93,10 @@ if(ARROW_DEPENDENCY_SOURCE STREQUAL "CONDA")
   endif()
   set(ARROW_ACTUAL_DEPENDENCY_SOURCE "SYSTEM")
   message(STATUS "Using CONDA_PREFIX for ARROW_PACKAGE_PREFIX: ${ARROW_PACKAGE_PREFIX}")
+  # ARROW-5564: Remove this when uriparser gets a conda package
+  if("${uriparser_SOURCE}" STREQUAL "")
+    set(uriparser_SOURCE "AUTO")
+  endif()
 else()
   set(ARROW_ACTUAL_DEPENDENCY_SOURCE "${ARROW_DEPENDENCY_SOURCE}")
 endif()
@@ -604,13 +608,26 @@ macro(build_uriparser)
 endmacro()
 
 if(ARROW_WITH_URIPARSER)
-  # Unless the user overrides uriparser_SOURCE, build uriparser ourselves
-  if("${uriparser_SOURCE}" STREQUAL "")
-    set(uriparser_SOURCE "BUNDLED")
+  set(ARROW_URIPARSER_REQUIRED_VERSION "0.9.0")
+  if(uriparser_SOURCE STREQUAL "AUTO")
+    # Debian does not ship cmake configs for uriparser
+    find_package(uriparser ${ARROW_URIPARSER_REQUIRED_VERSION} QUIET)
+    if(NOT uriparser_FOUND)
+      find_package(uriparserAlt ${ARROW_URIPARSER_REQUIRED_VERSION})
+    endif()
+    if(NOT uriparser_FOUND AND NOT uriparserAlt_FOUND)
+      build_uriparser()
+    endif()
+  elseif(uriparser_SOURCE STREQUAL "BUNDLED")
+    build_rapidjson()
+  elseif(uriparser_SOURCE STREQUAL "SYSTEM")
+    # Debian does not ship cmake configs for uriparser
+    find_package(uriparser ${ARROW_URIPARSER_REQUIRED_VERSION} QUIET)
+    if(NOT uriparser_FOUND)
+      find_package(uriparserAlt ${ARROW_URIPARSER_REQUIRED_VERSION} REQUIRED)
+    endif()
   endif()
 
-  resolve_dependency(uriparser)
-
   get_target_property(URIPARSER_INCLUDE_DIRS uriparser::uriparser
                       INTERFACE_INCLUDE_DIRECTORIES)
   include_directories(SYSTEM ${URIPARSER_INCLUDE_DIRS})
diff --git a/run-cmake-format.py b/run-cmake-format.py
index d572432..331331a 100755
--- a/run-cmake-format.py
+++ b/run-cmake-format.py
@@ -50,6 +50,7 @@ patterns = [
     'cpp/cmake_modules/FindgRPCAlt.cmake',
     'cpp/cmake_modules/FindgflagsAlt.cmake',
     'cpp/cmake_modules/Findjemalloc.cmake',
+    'cpp/cmake_modules/FinduriparserAlt.cmake',
     'cpp/cmake_modules/SetupCxxFlags.cmake',
     'cpp/cmake_modules/ThirdpartyToolchain.cmake',
     'cpp/cmake_modules/san-config.cmake',