Merge pull request #281 from Jihadist/opt_deps

Add options to use system abseil, lz4 and cityhash

Author: Enmk

.github/workflows/linux.yml

@@ -14,44 +14,67 @@ env:
 
 jobs:
   build:
-    runs-on: ubuntu-20.04
     strategy:
+      fail-fast: false
       matrix:
-        compiler: [clang-6, gcc-7, gcc-8, gcc-9]
+        os: [ubuntu-20.04]
+        compiler: [clang-6, clang-10-libc++, gcc-7, gcc-8, gcc-9]
         ssl: [ssl_ON, ssl_OFF]
+        dependencies: [dependencies_BUILT_IN]
+
         include:
         - compiler: clang-6
-          INSTALL: clang-6.0
+          COMPILER_INSTALL: clang-6.0 libc++-dev
           C_COMPILER: clang-6.0
           CXX_COMPILER: clang++-6.0
 
+        - compiler: clang-10-libc++
+          COMPILER_INSTALL: clang-10 libc++-dev
+          C_COMPILER: clang-10
+          CXX_COMPILER: clang++-10
+
         - compiler: gcc-7
-          INSTALL: gcc-7 g++-7
+          COMPILER_INSTALL: gcc-7 g++-7
           C_COMPILER: gcc-7
           CXX_COMPILER: g++-7
 
         - compiler: gcc-8
-          INSTALL: gcc-8 g++-8
+          COMPILER_INSTALL: gcc-8 g++-8
           C_COMPILER: gcc-8
           CXX_COMPILER: g++-8
 
         - compiler: gcc-9
-          INSTALL: gcc-9 g++-9
+          COMPILER_INSTALL: gcc-9 g++-9
           C_COMPILER: gcc-9
           CXX_COMPILER: g++-9
 
         - ssl: ssl_ON
-          INSTALL_SSL: libssl-dev
-          EXTRA_CMAKE_FLAGS: -DWITH_OPENSSL=ON
+          SSL_CMAKE_OPTION: -D WITH_OPENSSL=ON
 
-        - ssl: ssl_OFF
-          EXTRA_CMAKE_FLAGS: -DWITH_OPENSSL=OFF
+        - dependencies: dependencies_SYSTEM
+          compiler: compiler_SYSTEM
+          os: ubuntu-22.04
+          COMPILER_INSTALL: gcc g++
+          C_COMPILER: gcc
+          CXX_COMPILER: g++
+          DEPENDENCIES_INSTALL: libabsl-dev liblz4-dev
+          DEPENDENCIES_CMAKE_OPTIONS: >-
+            -D WITH_SYSTEM_LZ4=ON
+            -D WITH_SYSTEM_ABSEIL=ON
+
+    runs-on: ${{matrix.os}}
 
     steps:
     - uses: actions/checkout@v2
 
     - name: Install dependencies
-      run: sudo apt-get install -y docker cmake ${{ matrix.INSTALL }} ${{ matrix.INSTALL_SSL }}
+      run: |
+        sudo apt-get update && \
+        sudo apt-get install -y \
+          docker \
+          cmake \
+          ${{matrix.COMPILER_INSTALL}} \
+          ${{matrix.DEPENDENCIES_INSTALL}}
 
     - name: Install dependencies - Docker
       run: |
@@ -62,17 +85,24 @@ jobs:
         sudo apt update -q
         sudo apt install docker-ce docker-ce-cli containerd.io
 
-    - name: Configure CMake
+    - name: Configure project
+      run: |
+        cmake \
+          -D CMAKE_C_COMPILER=${{matrix.C_COMPILER}} \
+          -D CMAKE_CXX_COMPILER=${{matrix.CXX_COMPILER}} \
+          -D CMAKE_BUILD_TYPE=${{env.BUILD_TYPE}} \
+          -D BUILD_TESTS=ON \
+          ${{matrix.SSL_CMAKE_OPTION}} \
+          ${{matrix.DEPENDENCIES_CMAKE_OPTIONS}} \
+          -S ${{github.workspace}} \
+          -B ${{github.workspace}}/build 
+
+    - name: Build project
       run: |
         cmake \
-          -DCMAKE_C_COMPILER=${{ matrix.C_COMPILER}} \
-          -DCMAKE_CXX_COMPILER=${{ matrix.CXX_COMPILER}} \
-          -B ${{github.workspace}}/build \
-          -DCMAKE_BUILD_TYPE=${{env.BUILD_TYPE}} -DBUILD_TESTS=ON \
-          ${{ matrix.EXTRA_CMAKE_FLAGS }}
-
-    - name: Build
-      run: cmake --build ${{github.workspace}}/build --config ${{env.BUILD_TYPE}} --target all
+          --build ${{github.workspace}}/build \
+          --config ${{env.BUILD_TYPE}} \
+          --target all
 
     - name: Test - Start ClickHouse server in background
       run: |

.github/workflows/macos.yml

@@ -23,24 +23,34 @@ jobs:
         build: [nossl, ssl]
         include:
         - build: nossl
-          extra_cmake_flags: -DWITH_OPENSSL=OFF
-          extra_install: 
 
         - build: ssl
-          extra_cmake_flags: -DWITH_OPENSSL=ON -DOPENSSL_ROOT_DIR=/usr/local/opt/openssl/
-          extra_install: openssl
+          SSL_CMAKE_OPTION: >-
+            -D WITH_OPENSSL=ON
+            -D OPENSSL_ROOT_DIR=/usr/local/opt/openssl/
+          SSL_INSTALL: openssl
 
     steps:
     - uses: actions/checkout@v2
 
     - name: Install dependencies
-      run: brew install cmake ${{matrix.extra_install}}
+      run: brew install cmake ${{matrix.SSL_INSTALL}}
 
     - name: Configure CMake
-      run: cmake -B ${{github.workspace}}/build -DCMAKE_BUILD_TYPE=${{env.BUILD_TYPE}} -DBUILD_TESTS=ON ${{matrix.extra_cmake_flags}}
+      run: |
+        cmake \
+          -D CMAKE_BUILD_TYPE=${{env.BUILD_TYPE}} \
+          -D BUILD_TESTS=ON \
+          ${{matrix.SSL_CMAKE_OPTION}} \
+          -S ${{github.workspace}} \
+          -B ${{github.workspace}}/build
 
     - name: Build
-      run: cmake --build ${{github.workspace}}/build --config ${{env.BUILD_TYPE}} --target all
+      run: |
+        cmake \
+          --build ${{github.workspace}}/build \
+          --config ${{env.BUILD_TYPE}} \
+          --target all
 
     - name: Start tls offoader proxy
       # that mimics non-secure clickhouse running on localhost

CMakeLists.txt

@@ -1,13 +1,18 @@
 CMAKE_MINIMUM_REQUIRED (VERSION 3.0.2)
 
-INCLUDE (cmake/cpp17.cmake)
-INCLUDE (cmake/subdirs.cmake)
-INCLUDE (cmake/openssl.cmake)
+LIST (APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake")
+
+INCLUDE (cpp17)
+INCLUDE (subdirs)
+INCLUDE (openssl)
 
 OPTION (BUILD_BENCHMARK "Build benchmark" OFF)
 OPTION (BUILD_TESTS "Build tests" OFF)
 OPTION (BUILD_SHARED_LIBS "Build shared libs" OFF)
 OPTION (WITH_OPENSSL "Use OpenSSL for TLS connections" OFF)
+OPTION (WITH_SYSTEM_ABSEIL "Use system ABSEIL" OFF)
+OPTION (WITH_SYSTEM_LZ4 "Use system LZ4" OFF)
+OPTION (WITH_SYSTEM_CITYHASH "Use system cityhash" OFF)
 
 PROJECT (CLICKHOUSE-CLIENT)
 
@@ -34,11 +39,52 @@ PROJECT (CLICKHOUSE-CLIENT)
         ENDIF()
     ENDIF()
 
+    IF (CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
+        INCLUDE (CheckCXXSourceCompiles)
+
+        CHECK_CXX_SOURCE_COMPILES("#include <bits/c++config.h>\nint main() { return __GLIBCXX__ != 0; }"
+                                  CLANG_WITH_LIB_STDCXX)
+    ENDIF ()
+
+    IF (CLANG_WITH_LIB_STDCXX)
+        # there is a problem with __builtin_mul_overflow call at link time
+        # the error looks like: ... undefined reference to `__muloti4' ...
+        # caused by clang bug https://bugs.llvm.org/show_bug.cgi?id=16404
+        # explicit linking to compiler-rt allows to workaround the problem
+        SET (CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} --rtlib=compiler-rt")
+        SET (CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} --rtlib=compiler-rt")
+
+        # some workaround for linking issues on linux:
+        # /usr/bin/ld: CMakeFiles/simple-test.dir/main.cpp.o: undefined reference to symbol '_Unwind_Resume@@GCC_3.0'
+        # /usr/bin/ld: /lib/x86_64-linux-gnu/libgcc_s.so.1: error adding symbols: DSO missing from command line
+        # FIXME: that workaround breaks clang build on mingw
+        SET (CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} -lgcc_s")
+        SET (CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -lgcc_s")
+    ENDIF ()
+
+    IF (WITH_SYSTEM_ABSEIL)
+        FIND_PACKAGE(absl REQUIRED)
+    ELSE ()
+        INCLUDE_DIRECTORIES (contrib/absl)
+        SUBDIRS (contrib/absl/absl)
+    ENDIF ()
+
+    IF (WITH_SYSTEM_LZ4)
+        FIND_PACKAGE(lz4 REQUIRED)
+    ELSE ()
+        INCLUDE_DIRECTORIES (contrib/lz4/lz4)
+        SUBDIRS (contrib/lz4/lz4)
+    ENDIF ()
+
+    IF (WITH_SYSTEM_CITYHASH)
+        FIND_PACKAGE(cityhash REQUIRED)
+    ELSE ()
+        INCLUDE_DIRECTORIES (contrib/cityhash/cityhash)
+        SUBDIRS (contrib/cityhash/cityhash)
+    ENDIF ()
+
     SUBDIRS (
         clickhouse
-        contrib/absl
-        contrib/cityhash
-        contrib/lz4
     )
 
     IF (BUILD_BENCHMARK)

clickhouse/CMakeLists.txt

@@ -41,9 +41,9 @@ ENDIF ()
 ADD_LIBRARY (clickhouse-cpp-lib ${clickhouse-cpp-lib-src})
 SET_TARGET_PROPERTIES (clickhouse-cpp-lib PROPERTIES LINKER_LANGUAGE CXX)
 TARGET_LINK_LIBRARIES (clickhouse-cpp-lib
-    absl-lib
-    cityhash-lib
-    lz4-lib
+    absl::int128
+    cityhash::cityhash
+    lz4::lz4
 )
 TARGET_INCLUDE_DIRECTORIES (clickhouse-cpp-lib
     PUBLIC ${PROJECT_SOURCE_DIR}

clickhouse/base/compressed.cpp

@@ -3,8 +3,8 @@
 #include "output.h"
 #include "../exceptions.h"
 
-#include <cityhash/city.h>
-#include <lz4/lz4.h>
+#include <city.h>
+#include <lz4.h>
 #include <stdexcept>
 #include <system_error>
 

clickhouse/columns/lowcardinality.cpp

@@ -4,7 +4,7 @@
 #include "nullable.h"
 #include "../base/wire_format.h"
 
-#include <cityhash/city.h>
+#include <city.h>
 
 #include <functional>
 #include <string_view>

clickhouse/types/types.cpp

@@ -2,7 +2,7 @@
 
 #include "../exceptions.h"
 
-#include <cityhash/city.h>
+#include <city.h>
 
 #include <stdexcept>
 

cmake/Findlz4.cmake

@@ -0,0 +1,38 @@
+find_path(lz4_INCLUDE_DIR
+  NAMES lz4.h
+  DOC "lz4 include directory")
+mark_as_advanced(lz4_INCLUDE_DIR)
+find_library(lz4_LIBRARY
+  NAMES lz4 liblz4
+  DOC "lz4 library")
+mark_as_advanced(lz4_LIBRARY)
+
+if (lz4_INCLUDE_DIR)
+  file(STRINGS "${lz4_INCLUDE_DIR}/lz4.h" _lz4_version_lines
+    REGEX "#define[ \t]+LZ4_VERSION_(MAJOR|MINOR|RELEASE)")
+  string(REGEX REPLACE ".*LZ4_VERSION_MAJOR *\([0-9]*\).*" "\\1" _lz4_version_major "${_lz4_version_lines}")
+  string(REGEX REPLACE ".*LZ4_VERSION_MINOR *\([0-9]*\).*" "\\1" _lz4_version_minor "${_lz4_version_lines}")
+  string(REGEX REPLACE ".*LZ4_VERSION_RELEASE *\([0-9]*\).*" "\\1" _lz4_version_release "${_lz4_version_lines}")
+  set(lz4_VERSION "${_lz4_version_major}.${_lz4_version_minor}.${_lz4_version_release}")
+  unset(_lz4_version_major)
+  unset(_lz4_version_minor)
+  unset(_lz4_version_release)
+  unset(_lz4_version_lines)
+endif ()
+
+include(FindPackageHandleStandardArgs)
+find_package_handle_standard_args(lz4
+  REQUIRED_VARS lz4_LIBRARY lz4_INCLUDE_DIR
+  VERSION_VAR lz4_VERSION)
+
+if (lz4_FOUND)
+  set(lz4_INCLUDE_DIRS "${lz4_INCLUDE_DIR}")
+  set(lz4_LIBRARIES "${lz4_LIBRARY}")
+
+  if (NOT TARGET lz4::lz4)
+    add_library(lz4::lz4 UNKNOWN IMPORTED)
+    set_target_properties(lz4::lz4 PROPERTIES
+      IMPORTED_LOCATION "${lz4_LIBRARY}"
+      INTERFACE_INCLUDE_DIRECTORIES "${lz4_INCLUDE_DIR}")
+  endif ()
+endif ()

contrib/absl/CMakeLists.txt

@@ -0,0 +1,9 @@
+ADD_LIBRARY (absl_int128 STATIC
+    numeric/int128.cc
+)
+
+TARGET_INCLUDE_DIRECTORIES (absl_int128
+    PUBLIC ${PROJECT_SOURCE_DIR}/contrib/absl
+)
+
+ADD_LIBRARY (absl::int128 ALIAS absl_int128)

contrib/absl/absl/CMakeLists.txt

@@ -0,0 +1,7 @@
+ADD_LIBRARY (cityhash STATIC
+    city.cc
+)
+
+set_property(TARGET cityhash PROPERTY POSITION_INDEPENDENT_CODE ON)
+
+ADD_LIBRARY (cityhash::cityhash ALIAS cityhash)

contrib/absl/base/attributes.h renamed to contrib/absl/absl/base/attributes.h

@@ -0,0 +1,8 @@
+ADD_LIBRARY (lz4 STATIC
+    lz4.c
+    lz4hc.c
+)
+
+set_property(TARGET lz4 PROPERTY POSITION_INDEPENDENT_CODE ON)
+
+ADD_LIBRARY(lz4::lz4 ALIAS lz4)

contrib/absl/base/config.h renamed to contrib/absl/absl/base/config.h

@@ -6,11 +6,3 @@ ADD_EXECUTABLE (simple-test
 TARGET_LINK_LIBRARIES (simple-test
     clickhouse-cpp-lib
 )
-
-IF (CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
-    # there is a problem with __builtin_mul_overflow call at link time
-    # the error looks like: ... undefined reference to `__muloti4' ... 
-    # caused by clang bug https://bugs.llvm.org/show_bug.cgi?id=16404
-    # explicit linking to compiler-rt allows to workaround the problem
-    set (CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} --rtlib=compiler-rt")
-ENDIF ()

contrib/absl/base/internal/bits.h renamed to contrib/absl/absl/base/internal/bits.h

@@ -39,6 +39,7 @@ TARGET_LINK_LIBRARIES (clickhouse-cpp-ut
     clickhouse-cpp-lib
     gtest-lib
 )
-IF (CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
-    set (CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} --rtlib=compiler-rt")
-ENDIF ()
+
+IF (MSVC)
+    TARGET_COMPILE_OPTIONS(clickhouse-cpp-ut PRIVATE /bigobj)
+ENDIF()