Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add options to use system abseil, lz4 and cityhash #168

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 50 additions & 20 deletions .github/workflows/linux.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,44 +12,67 @@ env:

jobs:
build:
runs-on: ubuntu-latest
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: |
Expand All @@ -60,17 +83,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: |
DavidKeller marked this conversation as resolved.
Show resolved Hide resolved
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: |
Expand Down
24 changes: 17 additions & 7 deletions .github/workflows/macos.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
60 changes: 54 additions & 6 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@
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 (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)

Expand All @@ -27,11 +32,54 @@ PROJECT (CLICKHOUSE-CLIENT)
SET (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wextra -Werror -Wno-deprecated-declarations")
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 (.)

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)
Expand Down
35 changes: 6 additions & 29 deletions clickhouse/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -39,52 +39,29 @@ 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::int128
cityhash::cityhash
lz4::lz4
)
TARGET_INCLUDE_DIRECTORIES (clickhouse-cpp-lib
PUBLIC ${PROJECT_SOURCE_DIR}
)

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::int128
cityhash::cityhash
lz4::lz4
)
TARGET_INCLUDE_DIRECTORIES (clickhouse-cpp-lib-static
PUBLIC ${PROJECT_SOURCE_DIR}
)

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/)
Expand Down
4 changes: 2 additions & 2 deletions clickhouse/base/compressed.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>

Expand Down
2 changes: 1 addition & 1 deletion clickhouse/columns/lowcardinality.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
#include "nullable.h"
#include "../base/wire_format.h"

#include <cityhash/city.h>
#include <city.h>

#include <functional>
#include <string_view>
Expand Down
2 changes: 1 addition & 1 deletion clickhouse/types/types.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

#include "../exceptions.h"

#include <cityhash/city.h>
#include <city.h>

#include <stdexcept>

Expand Down
38 changes: 38 additions & 0 deletions cmake/Findlz4.cmake
Original file line number Diff line number Diff line change
@@ -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 ()
6 changes: 0 additions & 6 deletions contrib/absl/CMakeLists.txt

This file was deleted.

9 changes: 9 additions & 0 deletions contrib/absl/absl/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -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)
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
5 changes: 0 additions & 5 deletions contrib/cityhash/CMakeLists.txt

This file was deleted.

File renamed without changes.
7 changes: 7 additions & 0 deletions contrib/cityhash/cityhash/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -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)
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
6 changes: 0 additions & 6 deletions contrib/lz4/CMakeLists.txt

This file was deleted.

File renamed without changes.
Loading