Public flags, especially -Wxx (i.e. -Wno-XX) can hide some warnings,
that had been added in the main cmake rules of ClickHouse.
This patch had been tested manually with the following patch:
```patch
diff --git a/contrib/jemalloc-cmake/CMakeLists.txt b/contrib/jemalloc-cmake/CMakeLists.txt
index d5ea69d4926..7e79fba0c16 100644
--- a/contrib/jemalloc-cmake/CMakeLists.txt
+++ b/contrib/jemalloc-cmake/CMakeLists.txt
@@ -158,6 +158,7 @@ target_include_directories(_jemalloc SYSTEM PRIVATE
"${CMAKE_CURRENT_BINARY_DIR}/${JEMALLOC_INCLUDE_PREFIX}/jemalloc/internal")
target_compile_definitions(_jemalloc PRIVATE -DJEMALLOC_NO_PRIVATE_NAMESPACE)
+target_compile_options(_jemalloc INTERFACE -Wno-error) # also PUBLIC had been checked
if (CMAKE_BUILD_TYPE_UC STREQUAL "DEBUG")
target_compile_definitions(_jemalloc PRIVATE
```
And cmake gave:
CMake Error at cmake/sanitize_targets.cmake:69 (message):
_jemalloc set INTERFACE_COMPILE_OPTIONS to -Wno-error. This is forbidden.
Call Stack (most recent call first):
cmake/sanitize_targets.cmake:79 (sanitize_interface_flags)
CMakeLists.txt:595 (include)
Fixes: #12447
Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
We currently only log a compiler-generated "build id" at startup which
is different for each build. That makes it useless to determine the
exact source code state in tests (e.g. BC test) and from user log files
(e.g. if someone compiled an intermediate version of ClickHouse).
Current log message:
Starting ClickHouse 22.10.1.1 with revision 54467, build id: 6F35820328F89C9F36E91C447FF9E61CAF0EF019, PID 42633
New log message:
Starting ClickHouse 22.10.1.1 (revision 54467, git hash: b6b1f7f763f94ffa12133679a6f80342dd1c3afe, build id: 47B12BE61151926FBBD230DE42F3B7A6652AC482), PID 981813
Right now LINKER_NAME contains version so it does not work:
Aug 27 13:02:10 -- Using linker: /usr/bin/ld.lld-14
Without this patch:
$ time gdb ~/ch/clickhouse/.bld-release/programs/clickhouse -batch
real 1m20.564s
user 1m18.622s
sys 0m1.940s
$ time lldb-13 clickhouse -batch
real 0m9.353s
user 1m31.035s
sys 0m8.346s
With:
$ time gdb clickhouse -batch
real 0m4.769s
user 0m3.969s
sys 0m0.800s
$ time lldb clickhouse -batch
real 0m6.703s
user 0m45.383s
sys 0m4.934s
So by just adding 4.15MiB to the binary size, gdb start up time improved
17 times.
$ objdump -j .gdb_index -h .bld-release/programs/clickhouse
.bld-release/programs/clickhouse: file format elf64-x86-64
Sections:
Idx Name Size VMA LMA File off Algn
40 .gdb_index 00426d74 0000000000000000 0000000000000000 7fb43443 2**0
CONTENTS, READONLY, DEBUGGING
Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
Without ADDITIONAL_CLEAN_FILES it reports an error:
Cleaning... ninja: error: remove(contrib/llvm/llvm/NATIVE): Directory not empty
ninja: error: remove(/bld/contrib/llvm/llvm/NATIVE): Directory not empty
0 files.
Note, that ADDITIONAL_CLEAN_FILES had been added since cmake 3.15.
Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
Right now cmake add the following options only if USE_STATIC_LIBRARIES
is OFF:
- SPLIT_SHARED_LIBRARIES
- CLICKHOUSE_SPLIT_BINARY
And this breaks the following usage:
$ cmake ..
$ cat > debug-build-cache.cmake
set(USE_STATIC_LIBRARIES OFF CACHE BOOL "")
set(SPLIT_SHARED_LIBRARIES ON CACHE BOOL "")
set(CLICKHOUSE_SPLIT_BINARY ON CACHE BOOL "")
^D
$ cmake -C debug-build-cache.cmake ..
CMake Error at CMakeLists.txt:83 (message):
Defining SPLIT_SHARED_LIBRARIES=1 without USE_STATIC_LIBRARIES=0 has no
effect.
Since with this initial cache we have the following:
- USE_STATIC_LIBRARIES=OFF (because it was already set)
- SPLIT_SHARED_LIBRARIES=ON (was not set before, so new value)
- CLICKHOUSE_SPLIT_BINARY (was not set before, also new value)
Yes this is not the common usage, but it seems that it is pretty easy to
avoid.
Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
A simple HelloWorld program with zero includes except iostream triggers
a build of ca. 2000 source files. The reason is that ClickHouse's
top-level CMakeLists.txt overrides "add_executable()" to link all
binaries against "clickhouse_new_delete". This links against
"clickhouse_common_io", which in turn has lots of 3rd party library
dependencies ... Without linking "clickhouse_new_delete", the number of
compiled files for "HelloWorld" goes down to ca. 70.
As an example, the self-extracting-executable needs none of its current
dependencies but other programs may also benefit.
In order to restore access to the original "add_executable()", the
overriding version is now prefixed. There is precedence for a
"clickhouse_" prefix (as opposed to "ch_"), for example
"clickhouse_split_debug_symbols". In general prefixing makes sense also
because overriding CMake commands relies on undocumented behavior and is
considered not-so-great practice (*).
(*) https://crascit.com/2018/09/14/do-not-redefine-cmake-commands/
Sometimes it is useful to build contrib with debug symbols for further
debugging.
With everything turned ON (i.e. debug build) I got 3.3GB vs 3.0GB w/o
this patch, 9% bloat, thoughts about this is this OK or not for you, if
not STRIP_DEBUG_SYMBOLS_HEAVY_CONTRIB can be OFF by default (regardless
of build type).
P.S. aws debug symbols adds just 1.7%.
v2: rename STRIP_HEAVY_DEBUG_SYMBOLS
v3: OMIT_HEAVY_DEBUG_SYMBOLS
v4: documentation had been removed
Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
