Add options to use system abseil, lz4 and cityhash

This change solves ClickHouse#86, ClickHouse#99.

Signed-off-by: David Keller <[email protected]>

Author: DavidWoorton

.github/workflows/linux.yml

@@ -12,55 +12,80 @@ env:
 jobs:
   build:
     runs-on: ubuntu-latest
+
     strategy:
+      fail-fast: false
       matrix:
-        compiler: [clang-6, gcc-7, gcc-8, gcc-9]
+        compiler: [clang-6, clang-10-libc++, gcc-7, gcc-8, gcc-9]
         ssl: [ssl_ON, ssl_OFF]
+        dependencies: [dependencies_SYSTEM, dependencies_BUILT_IN]
         include:
         - compiler: clang-6
-          INSTALL: clang-6.0
           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
+          DEPENDENCIES_INSTALL: libabsl-dev liblz4-dev libcityhash-dev
+          DEPENDENCIES_CMAKE_OPTIONS: >-
+            -D WITH_SYSTEM_LZ4=ON
+            -D WITH_SYSTEM_CITYHASH=ON
+            -D WITH_SYSTEM_ABSEIL=ON
+
+        - dependencies: dependencies_BUILT_IN
 
     steps:
     - uses: actions/checkout@v2
 
     - name: Install dependencies
-      run: sudo apt-get install -y cmake ${{ matrix.INSTALL }} ${{ matrix.INSTALL_SSL }}
+      run: |
+        sudo apt-get install -y \
+          cmake \
+          ${{matrix.COMPILER_INSTALL}} \
+          ${{matrix.DEPENDENCIES_INSTALL}}
+
+    - 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: Configure CMake
+    - 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: Start ClickHouse server
       run: |

.github/workflows/macos.yml

@@ -21,24 +21,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

@@ -7,6 +7,9 @@ INCLUDE (cmake/openssl.cmake)
 OPTION (BUILD_BENCHMARK "Build benchmark" OFF)
 OPTION (BUILD_TESTS "Build tests" 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)
 
@@ -26,14 +29,54 @@ PROJECT (CLICKHOUSE-CLIENT)
         SET (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wextra -Werror")
     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 ()
+
     INCLUDE_DIRECTORIES (.)
-    INCLUDE_DIRECTORIES (contrib)
+
+    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

@@ -39,46 +39,23 @@ ENDIF ()
 ADD_LIBRARY (clickhouse-cpp-lib SHARED ${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::absl
+    cityhash::cityhash
+    lz4::lz4
 )
 
 ADD_LIBRARY (clickhouse-cpp-lib-static STATIC ${clickhouse-cpp-lib-src})
 TARGET_LINK_LIBRARIES (clickhouse-cpp-lib-static
-    absl-lib
-    cityhash-lib
-    lz4-lib
+    absl::absl
+    cityhash::cityhash
+    lz4::lz4
 )
 
-IF (CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
-    INCLUDE (CheckCXXSourceCompiles)
-
-    CHECK_CXX_SOURCE_COMPILES("#include <bits/c++config.h>\nint main() { return __GLIBCXX__ != 0; }"
-                              BUILDING_WITH_LIB_STDCXX)
-
-    IF (BUILDING_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")
-
-        # 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
-        TARGET_LINK_LIBRARIES (clickhouse-cpp-lib gcc_s)
-        TARGET_LINK_LIBRARIES (clickhouse-cpp-lib-static gcc_s)
-    ENDIF ()
-ENDIF ()
-
 INSTALL (TARGETS clickhouse-cpp-lib clickhouse-cpp-lib-static
     ARCHIVE DESTINATION lib
     LIBRARY DESTINATION lib
 )
 
-
 # general
 INSTALL(FILES block.h DESTINATION include/clickhouse/)
 INSTALL(FILES client.h DESTINATION include/clickhouse/)

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>
 

contrib/absl/CMakeLists.txt

@@ -0,0 +1,5 @@
+ADD_LIBRARY (absl STATIC
+    numeric/int128.cc
+)
+
+ADD_LIBRARY (absl::absl ALIAS absl)

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-static
 )
-
-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

@@ -38,6 +38,3 @@ TARGET_LINK_LIBRARIES (clickhouse-cpp-ut
     clickhouse-cpp-lib-static
     gtest-lib
 )
-IF (CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
-    set (CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} --rtlib=compiler-rt")
-ENDIF ()