Skip to content

Commit 04abf44

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 4a186a8 commit 04abf44

40 files changed

+193
-94
lines changed

.github/workflows/linux.yml

+50-20
Original file line numberDiff line numberDiff line change
@@ -12,44 +12,67 @@ env:
1212

1313
jobs:
1414
build:
15-
runs-on: ubuntu-latest
1615
strategy:
16+
fail-fast: false
1717
matrix:
18-
compiler: [clang-6, gcc-7, gcc-8, gcc-9]
18+
os: [ubuntu-20.04]
19+
compiler: [clang-6, clang-10-libc++, gcc-7, gcc-8, gcc-9]
1920
ssl: [ssl_ON, ssl_OFF]
21+
dependencies: [dependencies_BUILT_IN]
22+
2023
include:
2124
- compiler: clang-6
22-
INSTALL: clang-6.0
25+
COMPILER_INSTALL: clang-6.0 libc++-dev
2326
C_COMPILER: clang-6.0
2427
CXX_COMPILER: clang++-6.0
2528

29+
- compiler: clang-10-libc++
30+
COMPILER_INSTALL: clang-10 libc++-dev
31+
C_COMPILER: clang-10
32+
CXX_COMPILER: clang++-10
33+
2634
- compiler: gcc-7
27-
INSTALL: gcc-7 g++-7
35+
COMPILER_INSTALL: gcc-7 g++-7
2836
C_COMPILER: gcc-7
2937
CXX_COMPILER: g++-7
3038

3139
- compiler: gcc-8
32-
INSTALL: gcc-8 g++-8
40+
COMPILER_INSTALL: gcc-8 g++-8
3341
C_COMPILER: gcc-8
3442
CXX_COMPILER: g++-8
3543

3644
- compiler: gcc-9
37-
INSTALL: gcc-9 g++-9
45+
COMPILER_INSTALL: gcc-9 g++-9
3846
C_COMPILER: gcc-9
3947
CXX_COMPILER: g++-9
4048

4149
- ssl: ssl_ON
42-
INSTALL_SSL: libssl-dev
43-
EXTRA_CMAKE_FLAGS: -DWITH_OPENSSL=ON
50+
SSL_CMAKE_OPTION: -D WITH_OPENSSL=ON
4451

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

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

5168
- name: Install dependencies
52-
run: sudo apt-get install -y docker cmake ${{ matrix.INSTALL }} ${{ matrix.INSTALL_SSL }}
69+
run: |
70+
sudo apt-get update && \
71+
sudo apt-get install -y \
72+
docker \
73+
cmake \
74+
${{matrix.COMPILER_INSTALL}} \
75+
${{matrix.DEPENDENCIES_INSTALL}}
5376
5477
- name: Install dependencies - Docker
5578
run: |
@@ -60,17 +83,24 @@ jobs:
6083
sudo apt update -q
6184
sudo apt install docker-ce docker-ce-cli containerd.io
6285
63-
- name: Configure CMake
86+
- name: Configure project
87+
run: |
88+
cmake \
89+
-D CMAKE_C_COMPILER=${{matrix.C_COMPILER}} \
90+
-D CMAKE_CXX_COMPILER=${{matrix.CXX_COMPILER}} \
91+
-D CMAKE_BUILD_TYPE=${{env.BUILD_TYPE}} \
92+
-D BUILD_TESTS=ON \
93+
${{matrix.SSL_CMAKE_OPTION}} \
94+
${{matrix.DEPENDENCIES_CMAKE_OPTIONS}} \
95+
-S ${{github.workspace}} \
96+
-B ${{github.workspace}}/build
97+
98+
- name: Build project
6499
run: |
65100
cmake \
66-
-DCMAKE_C_COMPILER=${{ matrix.C_COMPILER}} \
67-
-DCMAKE_CXX_COMPILER=${{ matrix.CXX_COMPILER}} \
68-
-B ${{github.workspace}}/build \
69-
-DCMAKE_BUILD_TYPE=${{env.BUILD_TYPE}} -DBUILD_TESTS=ON \
70-
${{ matrix.EXTRA_CMAKE_FLAGS }}
71-
72-
- name: Build
73-
run: cmake --build ${{github.workspace}}/build --config ${{env.BUILD_TYPE}} --target all
101+
--build ${{github.workspace}}/build \
102+
--config ${{env.BUILD_TYPE}} \
103+
--target all
74104
75105
- name: Test - Start ClickHouse server in background
76106
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

+54-6
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

@@ -27,11 +32,54 @@ PROJECT (CLICKHOUSE-CLIENT)
2732
SET (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wextra -Werror -Wno-deprecated-declarations")
2833
ENDIF ()
2934

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

3785
IF (BUILD_BENCHMARK)

clickhouse/CMakeLists.txt

+6-29
Original file line numberDiff line numberDiff line change
@@ -39,52 +39,29 @@ 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::int128
43+
cityhash::cityhash
44+
lz4::lz4
4545
)
4646
TARGET_INCLUDE_DIRECTORIES (clickhouse-cpp-lib
4747
PUBLIC ${PROJECT_SOURCE_DIR}
4848
)
4949

5050
ADD_LIBRARY (clickhouse-cpp-lib-static STATIC ${clickhouse-cpp-lib-src})
5151
TARGET_LINK_LIBRARIES (clickhouse-cpp-lib-static
52-
absl-lib
53-
cityhash-lib
54-
lz4-lib
52+
absl::int128
53+
cityhash::cityhash
54+
lz4::lz4
5555
)
5656
TARGET_INCLUDE_DIRECTORIES (clickhouse-cpp-lib-static
5757
PUBLIC ${PROJECT_SOURCE_DIR}
5858
)
5959

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

87-
8865
# general
8966
INSTALL(FILES block.h DESTINATION include/clickhouse/)
9067
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

-6
This file was deleted.

contrib/absl/absl/CMakeLists.txt

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
ADD_LIBRARY (absl_int128 STATIC
2+
numeric/int128.cc
3+
)
4+
5+
TARGET_INCLUDE_DIRECTORIES (absl_int128
6+
PUBLIC ${PROJECT_SOURCE_DIR}/contrib/absl
7+
)
8+
9+
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.

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.

0 commit comments

Comments
 (0)