Skip to content

Commit 5feafbc

Browse files
DavidWoortonDavidKeller
authored andcommitted
Add options to use system abseil, and lz4
This change solves #86, #99. Signed-off-by: David Keller <[email protected]>
1 parent ba90272 commit 5feafbc

40 files changed

+186
-92
lines changed

.github/workflows/linux.yml

+49-20
Original file line numberDiff line numberDiff line change
@@ -11,56 +11,85 @@ env:
1111
CH_SERVER_VERSION: 21.3.17.2
1212
jobs:
1313
build:
14-
runs-on: ubuntu-latest
1514
strategy:
15+
fail-fast: false
1616
matrix:
17-
compiler: [clang-6, gcc-7, gcc-8, gcc-9]
17+
os: [ubuntu-20.04]
18+
compiler: [clang-6, clang-10-libc++, gcc-7, gcc-8, gcc-9]
1819
ssl: [ssl_ON, ssl_OFF]
20+
dependencies: [dependencies_BUILT_IN]
21+
1922
include:
2023
- compiler: clang-6
21-
INSTALL: clang-6.0
24+
COMPILER_INSTALL: clang-6.0 libc++-dev
2225
C_COMPILER: clang-6.0
2326
CXX_COMPILER: clang++-6.0
2427

28+
- compiler: clang-10-libc++
29+
COMPILER_INSTALL: clang-10 libc++-dev
30+
C_COMPILER: clang-10
31+
CXX_COMPILER: clang++-10
32+
2533
- compiler: gcc-7
26-
INSTALL: gcc-7 g++-7
34+
COMPILER_INSTALL: gcc-7 g++-7
2735
C_COMPILER: gcc-7
2836
CXX_COMPILER: g++-7
2937

3038
- compiler: gcc-8
31-
INSTALL: gcc-8 g++-8
39+
COMPILER_INSTALL: gcc-8 g++-8
3240
C_COMPILER: gcc-8
3341
CXX_COMPILER: g++-8
3442

3543
- compiler: gcc-9
36-
INSTALL: gcc-9 g++-9
44+
COMPILER_INSTALL: gcc-9 g++-9
3745
C_COMPILER: gcc-9
3846
CXX_COMPILER: g++-9
3947

4048
- ssl: ssl_ON
41-
INSTALL_SSL: libssl-dev
42-
EXTRA_CMAKE_FLAGS: -DWITH_OPENSSL=ON
49+
SSL_CMAKE_OPTION: -D WITH_OPENSSL=ON
4350

44-
- ssl: ssl_OFF
45-
EXTRA_CMAKE_FLAGS: -DWITH_OPENSSL=OFF
51+
- dependencies: dependencies_SYSTEM
52+
compiler: compiler_SYSTEM
53+
os: ubuntu-22.04
54+
COMPILER_INSTALL: gcc g++
55+
C_COMPILER: gcc
56+
CXX_COMPILER: g++
57+
DEPENDENCIES_INSTALL: libabsl-dev liblz4-dev
58+
DEPENDENCIES_CMAKE_OPTIONS: >-
59+
-D WITH_SYSTEM_LZ4=ON
60+
-D WITH_SYSTEM_ABSEIL=ON
61+
62+
runs-on: ${{matrix.os}}
4663

4764
steps:
4865
- uses: actions/checkout@v2
4966

5067
- name: Install dependencies
51-
run: sudo apt-get install -y cmake ${{ matrix.INSTALL }} ${{ matrix.INSTALL_SSL }}
68+
run: |
69+
sudo apt-get update && \
70+
sudo apt-get install -y \
71+
cmake \
72+
${{matrix.COMPILER_INSTALL}} \
73+
${{matrix.DEPENDENCIES_INSTALL}}
74+
75+
- name: Configure project
76+
run: |
77+
cmake \
78+
-D CMAKE_C_COMPILER=${{matrix.C_COMPILER}} \
79+
-D CMAKE_CXX_COMPILER=${{matrix.CXX_COMPILER}} \
80+
-D CMAKE_BUILD_TYPE=${{env.BUILD_TYPE}} \
81+
-D BUILD_TESTS=ON \
82+
${{matrix.SSL_CMAKE_OPTION}} \
83+
${{matrix.DEPENDENCIES_CMAKE_OPTIONS}} \
84+
-S ${{github.workspace}} \
85+
-B ${{github.workspace}}/build
5286
53-
- name: Configure CMake
87+
- name: Build project
5488
run: |
5589
cmake \
56-
-DCMAKE_C_COMPILER=${{ matrix.C_COMPILER}} \
57-
-DCMAKE_CXX_COMPILER=${{ matrix.CXX_COMPILER}} \
58-
-B ${{github.workspace}}/build \
59-
-DCMAKE_BUILD_TYPE=${{env.BUILD_TYPE}} -DBUILD_TESTS=ON \
60-
${{ matrix.EXTRA_CMAKE_FLAGS }}
61-
62-
- name: Build
63-
run: cmake --build ${{github.workspace}}/build --config ${{env.BUILD_TYPE}} --target all
90+
--build ${{github.workspace}}/build \
91+
--config ${{env.BUILD_TYPE}} \
92+
--target all
6493
6594
- name: Start ClickHouse server
6695
run: |

.github/workflows/macos.yml

+17-7
Original file line numberDiff line numberDiff line change
@@ -21,24 +21,34 @@ jobs:
2121
build: [nossl, ssl]
2222
include:
2323
- build: nossl
24-
extra_cmake_flags: -DWITH_OPENSSL=OFF
25-
extra_install:
2624

2725
- build: ssl
28-
extra_cmake_flags: -DWITH_OPENSSL=ON -DOPENSSL_ROOT_DIR=/usr/local/opt/openssl/
29-
extra_install: openssl
26+
SSL_CMAKE_OPTION: >-
27+
-D WITH_OPENSSL=ON
28+
-D OPENSSL_ROOT_DIR=/usr/local/opt/openssl/
29+
SSL_INSTALL: openssl
3030

3131
steps:
3232
- uses: actions/checkout@v2
3333

3434
- name: Install dependencies
35-
run: brew install cmake ${{matrix.extra_install}}
35+
run: brew install cmake ${{matrix.SSL_INSTALL}}
3636

3737
- name: Configure CMake
38-
run: cmake -B ${{github.workspace}}/build -DCMAKE_BUILD_TYPE=${{env.BUILD_TYPE}} -DBUILD_TESTS=ON ${{matrix.extra_cmake_flags}}
38+
run: |
39+
cmake \
40+
-D CMAKE_BUILD_TYPE=${{env.BUILD_TYPE}} \
41+
-D BUILD_TESTS=ON \
42+
${{matrix.SSL_CMAKE_OPTION}} \
43+
-S ${{github.workspace}} \
44+
-B ${{github.workspace}}/build
3945
4046
- name: Build
41-
run: cmake --build ${{github.workspace}}/build --config ${{env.BUILD_TYPE}} --target all
47+
run: |
48+
cmake \
49+
--build ${{github.workspace}}/build \
50+
--config ${{env.BUILD_TYPE}} \
51+
--target all
4252
4353
- name: Start tls offoader proxy
4454
# that mimics non-secure clickhouse running on localhost

CMakeLists.txt

+52-7
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,17 @@
11
CMAKE_MINIMUM_REQUIRED (VERSION 3.0.2)
22

3-
INCLUDE (cmake/cpp17.cmake)
4-
INCLUDE (cmake/subdirs.cmake)
5-
INCLUDE (cmake/openssl.cmake)
3+
LIST (APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake")
4+
5+
INCLUDE (cpp17)
6+
INCLUDE (subdirs)
7+
INCLUDE (openssl)
68

79
OPTION (BUILD_BENCHMARK "Build benchmark" OFF)
810
OPTION (BUILD_TESTS "Build tests" OFF)
911
OPTION (WITH_OPENSSL "Use OpenSSL for TLS connections" OFF)
12+
OPTION (WITH_SYSTEM_ABSEIL "Use system ABSEIL" OFF)
13+
OPTION (WITH_SYSTEM_LZ4 "Use system LZ4" OFF)
14+
OPTION (WITH_SYSTEM_CITYHASH "Use system cityhash" OFF)
1015

1116
PROJECT (CLICKHOUSE-CLIENT)
1217

@@ -26,14 +31,54 @@ PROJECT (CLICKHOUSE-CLIENT)
2631
SET (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wextra -Werror")
2732
ENDIF ()
2833

34+
IF (CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
35+
INCLUDE (CheckCXXSourceCompiles)
36+
37+
CHECK_CXX_SOURCE_COMPILES("#include <bits/c++config.h>\nint main() { return __GLIBCXX__ != 0; }"
38+
CLANG_WITH_LIB_STDCXX)
39+
ENDIF ()
40+
41+
IF (CLANG_WITH_LIB_STDCXX)
42+
# there is a problem with __builtin_mul_overflow call at link time
43+
# the error looks like: ... undefined reference to `__muloti4' ...
44+
# caused by clang bug https://bugs.llvm.org/show_bug.cgi?id=16404
45+
# explicit linking to compiler-rt allows to workaround the problem
46+
SET (CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} --rtlib=compiler-rt")
47+
SET (CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} --rtlib=compiler-rt")
48+
49+
# some workaround for linking issues on linux:
50+
# /usr/bin/ld: CMakeFiles/simple-test.dir/main.cpp.o: undefined reference to symbol '_Unwind_Resume@@GCC_3.0'
51+
# /usr/bin/ld: /lib/x86_64-linux-gnu/libgcc_s.so.1: error adding symbols: DSO missing from command line
52+
# FIXME: that workaround breaks clang build on mingw
53+
SET (CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} -lgcc_s")
54+
SET (CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -lgcc_s")
55+
ENDIF ()
56+
2957
INCLUDE_DIRECTORIES (.)
30-
INCLUDE_DIRECTORIES (contrib)
58+
59+
IF (WITH_SYSTEM_ABSEIL)
60+
FIND_PACKAGE(absl REQUIRED)
61+
ELSE ()
62+
INCLUDE_DIRECTORIES (contrib/absl)
63+
SUBDIRS (contrib/absl/absl)
64+
ENDIF ()
65+
66+
IF (WITH_SYSTEM_LZ4)
67+
FIND_PACKAGE(lz4 REQUIRED)
68+
ELSE ()
69+
INCLUDE_DIRECTORIES (contrib/lz4/lz4)
70+
SUBDIRS (contrib/lz4/lz4)
71+
ENDIF ()
72+
73+
IF (WITH_SYSTEM_CITYHASH)
74+
FIND_PACKAGE(cityhash REQUIRED)
75+
ELSE ()
76+
INCLUDE_DIRECTORIES (contrib/cityhash/cityhash)
77+
SUBDIRS (contrib/cityhash/cityhash)
78+
ENDIF ()
3179

3280
SUBDIRS (
3381
clickhouse
34-
contrib/absl
35-
contrib/cityhash
36-
contrib/lz4
3782
)
3883

3984
IF (BUILD_BENCHMARK)

clickhouse/CMakeLists.txt

+6-29
Original file line numberDiff line numberDiff line change
@@ -39,46 +39,23 @@ ENDIF ()
3939
ADD_LIBRARY (clickhouse-cpp-lib SHARED ${clickhouse-cpp-lib-src})
4040
SET_TARGET_PROPERTIES(clickhouse-cpp-lib PROPERTIES LINKER_LANGUAGE CXX)
4141
TARGET_LINK_LIBRARIES (clickhouse-cpp-lib
42-
absl-lib
43-
cityhash-lib
44-
lz4-lib
42+
absl::absl
43+
cityhash::cityhash
44+
lz4::lz4
4545
)
4646

4747
ADD_LIBRARY (clickhouse-cpp-lib-static STATIC ${clickhouse-cpp-lib-src})
4848
TARGET_LINK_LIBRARIES (clickhouse-cpp-lib-static
49-
absl-lib
50-
cityhash-lib
51-
lz4-lib
49+
absl::absl
50+
cityhash::cityhash
51+
lz4::lz4
5252
)
5353

54-
IF (CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
55-
INCLUDE (CheckCXXSourceCompiles)
56-
57-
CHECK_CXX_SOURCE_COMPILES("#include <bits/c++config.h>\nint main() { return __GLIBCXX__ != 0; }"
58-
BUILDING_WITH_LIB_STDCXX)
59-
60-
IF (BUILDING_WITH_LIB_STDCXX)
61-
# there is a problem with __builtin_mul_overflow call at link time
62-
# the error looks like: ... undefined reference to `__muloti4' ...
63-
# caused by clang bug https://bugs.llvm.org/show_bug.cgi?id=16404
64-
# explicit linking to compiler-rt allows to workaround the problem
65-
SET (CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} --rtlib=compiler-rt")
66-
67-
# some workaround for linking issues on linux:
68-
# /usr/bin/ld: CMakeFiles/simple-test.dir/main.cpp.o: undefined reference to symbol '_Unwind_Resume@@GCC_3.0'
69-
# /usr/bin/ld: /lib/x86_64-linux-gnu/libgcc_s.so.1: error adding symbols: DSO missing from command line
70-
# FIXME: that workaround breaks clang build on mingw
71-
TARGET_LINK_LIBRARIES (clickhouse-cpp-lib gcc_s)
72-
TARGET_LINK_LIBRARIES (clickhouse-cpp-lib-static gcc_s)
73-
ENDIF ()
74-
ENDIF ()
75-
7654
INSTALL (TARGETS clickhouse-cpp-lib clickhouse-cpp-lib-static
7755
ARCHIVE DESTINATION lib
7856
LIBRARY DESTINATION lib
7957
)
8058

81-
8259
# general
8360
INSTALL(FILES block.h DESTINATION include/clickhouse/)
8461
INSTALL(FILES client.h DESTINATION include/clickhouse/)

clickhouse/base/compressed.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33
#include "output.h"
44
#include "../exceptions.h"
55

6-
#include <cityhash/city.h>
7-
#include <lz4/lz4.h>
6+
#include <city.h>
7+
#include <lz4.h>
88
#include <stdexcept>
99
#include <system_error>
1010

clickhouse/columns/lowcardinality.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
#include "nullable.h"
55
#include "../base/wire_format.h"
66

7-
#include <cityhash/city.h>
7+
#include <city.h>
88

99
#include <functional>
1010
#include <string_view>

clickhouse/types/types.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
#include "../exceptions.h"
44

5-
#include <cityhash/city.h>
5+
#include <city.h>
66

77
#include <stdexcept>
88

cmake/Findlz4.cmake

+38
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
find_path(lz4_INCLUDE_DIR
2+
NAMES lz4.h
3+
DOC "lz4 include directory")
4+
mark_as_advanced(lz4_INCLUDE_DIR)
5+
find_library(lz4_LIBRARY
6+
NAMES lz4 liblz4
7+
DOC "lz4 library")
8+
mark_as_advanced(lz4_LIBRARY)
9+
10+
if (lz4_INCLUDE_DIR)
11+
file(STRINGS "${lz4_INCLUDE_DIR}/lz4.h" _lz4_version_lines
12+
REGEX "#define[ \t]+lz4_VERSION_(MAJOR|MINOR|RELEASE)")
13+
string(REGEX REPLACE ".*lz4_VERSION_MAJOR *\([0-9]*\).*" "\\1" _lz4_version_major "${_lz4_version_lines}")
14+
string(REGEX REPLACE ".*lz4_VERSION_MINOR *\([0-9]*\).*" "\\1" _lz4_version_minor "${_lz4_version_lines}")
15+
string(REGEX REPLACE ".*lz4_VERSION_RELEASE *\([0-9]*\).*" "\\1" _lz4_version_release "${_lz4_version_lines}")
16+
set(lz4_VERSION "${_lz4_version_major}.${_lz4_version_minor}.${_lz4_version_release}")
17+
unset(_lz4_version_major)
18+
unset(_lz4_version_minor)
19+
unset(_lz4_version_release)
20+
unset(_lz4_version_lines)
21+
endif ()
22+
23+
include(FindPackageHandleStandardArgs)
24+
find_package_handle_standard_args(lz4
25+
REQUIRED_VARS lz4_LIBRARY lz4_INCLUDE_DIR
26+
VERSION_VAR lz4_VERSION)
27+
28+
if (lz4_FOUND)
29+
set(lz4_INCLUDE_DIRS "${lz4_INCLUDE_DIR}")
30+
set(lz4_LIBRARIES "${lz4_LIBRARY}")
31+
32+
if (NOT TARGET lz4::lz4)
33+
add_library(lz4::lz4 UNKNOWN IMPORTED)
34+
set_target_properties(lz4::lz4 PROPERTIES
35+
IMPORTED_LOCATION "${lz4_LIBRARY}"
36+
INTERFACE_INCLUDE_DIRECTORIES "${lz4_INCLUDE_DIR}")
37+
endif ()
38+
endif ()

contrib/absl/CMakeLists.txt

-3
This file was deleted.

contrib/absl/absl/CMakeLists.txt

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
ADD_LIBRARY (absl STATIC
2+
numeric/int128.cc
3+
)
4+
5+
ADD_LIBRARY (absl::absl ALIAS absl)
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.

contrib/cityhash/CMakeLists.txt

-5
This file was deleted.
File renamed without changes.
+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
ADD_LIBRARY (cityhash STATIC
2+
city.cc
3+
)
4+
5+
set_property(TARGET cityhash PROPERTY POSITION_INDEPENDENT_CODE ON)
6+
7+
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.

contrib/lz4/CMakeLists.txt

-6
This file was deleted.
File renamed without changes.

contrib/lz4/lz4/CMakeLists.txt

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
ADD_LIBRARY (lz4 STATIC
2+
lz4.c
3+
lz4hc.c
4+
)
5+
6+
set_property(TARGET lz4 PROPERTY POSITION_INDEPENDENT_CODE ON)
7+
8+
ADD_LIBRARY(lz4::lz4 ALIAS lz4)
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.

0 commit comments

Comments
 (0)