Remove unbundled support by azat · Pull Request #33690 · ClickHouse/ClickHouse

azat · 2022-01-16T20:42:46Z

Changelog category (leave one):

Build/Testing/Packaging Improvement

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Remove unbundled support

Detailed description / Documentation draft:
Recently I found one more issues with unbundled builds, and instead of fixing it in particular I decided to do a global cleanup.
Note, that right now (since d604cf5), to use UNBUNDLED build you need to manually set some variables, in other words there is no more one common switch for this (so it is unlikely that someone uses it, even I don't already)

Implementation:

each library is in a separate commit (more or less)
each commit may include some refactoring, since there was some issues while I was looking at this, one of common mistakes are:
- not using internal bundled library (zlib)
- use of non-existing variables (X_LIBRARIES instead of X_LIBRARY)
- remove quirks for library types (statis/shared)
- use ENABLE_LIBRARIES as default for contrib
- move -DUSE_XXX to config file (configure_file)
original library name hidden with a prefix (to avoid accidentally using it)
alias that starts with ch_contrib:: is used as a public interface (that should include all include directories and compile definitions)
add alias libraries even for header-only libraries
add a style check to avoid using find_path/find_libraries (as a marker for non-bundled/system-wide libraries)

Checklist:

cleanup all cmake/find/*.cmake
cleanup docs
cleanup USE_INTERNAL_
introduce helper to include folder and check that submodule cloned

Fixes: #30598 (cc @alexey-milovidov )
Fixes: #9226

alexey-milovidov

This is amazing!

azat · 2022-01-19T06:20:32Z

Resolved lots of conflicts after merging #33695 (@amosbird some contrib headers does not requires SYSTEM, but I left them for now, since it is a minor thing (and I would say that it is a matter of choice) and it is important to finish this ASAP).

azat · 2022-01-19T06:21:21Z

The only thing that is left for now is compatibility check, by some reason fscanf is used from 2.7

I missed some bits while converting libuv-cmake, fixed now.

amosbird · 2022-01-19T06:31:34Z

Resolved lots of conflicts after merging #33695 (@amosbird some contrib headers does not requires SYSTEM, but I left them for now, since it is a minor thing (and I would say that it is a matter of choice) and it is important to finish this ASAP).

It can be useful. System headers will appear later than normal headers which helps when some thirdparty's include dir introduces common header files, like libpq's config.h.

contrib/arrow-cmake/CMakeLists.txt

amosbird · 2022-01-19T07:14:14Z

contrib/arrow-cmake/CMakeLists.txt

Why do these two libs not populate their only INTERFACE_INCLUDE_DIRECTORIES so that we can use modern cmake usage requirement directly? (e.g. target_link_libraries( _arrow PRIVATE ch_contrib::flatbuffers ch_contrib::hdfs) )

Why do these two libs not populate their only INTERFACE_INCLUDE_DIRECTORIES

They populate PUBLIC part (which is PRIVATE + INTERFACE), and only those paths, which is required for public interface.

so that we can use modern cmake usage requirement directly? (e.g. target_link_libraries( _arrow PRIVATE ch_contrib::flatbuffers ch_contrib::hdfs) )

This is indeed what is used later.

Oh, you are talking about doing the same thing for flatbuffers, for now it is important not to use ${LIB_INCLUDE_DIR} outside of contrib since this is important (because it is easy to miss spell when variables in different files, while if everything is in one file it is more easy to do this right), everything other was leaved more or less as-is.

azat · 2022-01-19T08:09:59Z

Rebased to fix conflicts with #33742

alesapin · 2022-01-19T08:31:23Z

@Mergifyio update

mergify · 2022-01-19T08:31:33Z

update

✅ Branch has been successfully updated

azat · 2022-01-19T11:43:29Z

PullRequestCI / UnitTestsReleaseClang (pull_request) Failing after 14s — UnitTestsReleaseClang

INFO:root:Found build report json build_urls_binary_release.json
['https://s3.amazonaws.com/clickhouse-builds/33690/c397932ad4699abe58cdbb78ab56d76f4bdcbfaf/binary_release/clickhouse']
...
FileNotFoundError: [Errno 2] No such file or directory: '/home/ubuntu/actions-runner/_work/_temp/unit_tests_asan/unit_tests_dbms'

Apparently it tries to download unit tests too early, or what? @alesapin any clue?

Worth adding an exception if filter_fn does not match anything I guess?

ClickHouse/tests/ci/build_download_helper.py

Lines 67 to 72 in dfd8d40

    
           def download_builds(result_path, build_urls, filter_fn): 
        
               for url in build_urls: 
        
                   if filter_fn(url): 
        
                       fname = os.path.basename(url.replace('%2B', '+').replace('%20', ' ')) 
        
                       logging.info("Will download %s to %s", fname, result_path) 
        
                       dowload_build_with_progress(url, os.path.join(result_path, fname))

UPD: nevermind, fixed

azat · 2022-01-20T08:53:34Z

utils/check-style/check-style

 fi

+# Check that there is no system-wide libraries/headers in use.
+if git grep -e find_path -e find_library -- :**CMakeLists.txt; then


#33690 (comment)

azat · 2022-01-20T09:02:08Z

PVS Studio (actions) — Total errors 39

PVS passed.

qoega · 2022-01-27T14:58:30Z

contrib/llvm-cmake/CMakeLists.txt

+set (LLVM_ENABLE_EH 1 CACHE INTERNAL "")
+set (LLVM_ENABLE_RTTI 1 CACHE INTERNAL "")
+set (LLVM_ENABLE_PIC 0 CACHE INTERNAL "")
+set (LLVM_TARGETS_TO_BUILD "X86;AArch64" CACHE STRING "")


Why you build both x86 and aarch64? As I understand we have information about target and host architecture.

Algunenano · 2026-02-23T17:53:40Z

contrib/arrow-cmake/CMakeLists.txt

+# We dont use arrow's cmakefiles because they uses too many depends and download some libs in compile time
+# But you can update auto-generated parquet files manually:
+# cd {BUILD_DIR}/contrib/arrow/cpp/src/parquet && mkdir -p build && cd build
+# cmake .. -DARROW_COMPUTE=ON -DARROW_PARQUET=ON -DARROW_SIMD_LEVEL=NONE -DARROW_VERBOSE_THIRDPARTY_BUILD=ON


Do you happen to remember why we disable SIMD completely? Seems like it could be useful and we could enable it in x86 and in ARM

No, I don't remember.
One hypothesis is that it enabled too much, another hypothesis is that it didn't build.

robot-clickhouse added the pr-not-for-changelog This PR should not be mentioned in the changelog label Jan 16, 2022

alexey-milovidov approved these changes Jan 16, 2022

View reviewed changes

alexey-milovidov self-assigned this Jan 16, 2022

azat force-pushed the remove-unbundled-support branch 3 times, most recently from ef7afef to a07ae4e Compare January 18, 2022 12:49

robot-clickhouse added pr-build Pull request with build/testing/packaging improvement and removed pr-not-for-changelog This PR should not be mentioned in the changelog labels Jan 18, 2022

azat force-pushed the remove-unbundled-support branch 10 times, most recently from 081c5bb to 96746c6 Compare January 19, 2022 06:20

amosbird reviewed Jan 19, 2022

View reviewed changes

contrib/arrow-cmake/CMakeLists.txt Outdated Show resolved Hide resolved

amosbird reviewed Jan 19, 2022

View reviewed changes

azat force-pushed the remove-unbundled-support branch 3 times, most recently from 9ba3065 to 150c10e Compare January 19, 2022 08:09

azat added 19 commits January 20, 2022 10:02

docs/en/development/build-cross-riscv.md: cleanup USE_INTERNAL_XXX

c3ac1f9

tests/instructions/sanitizers.md: cleanup USE_INTERNAL_XXX

eeb0c83

Add add_contrib() macro

c6f95bc

Add alias library for magic_enum

ad67aa5

Add alias library for fmt

e3b140a

Remove extra cmake modules from libhdfs3-cmake (everything is bundled)

eda7182

check-style: add a check that there is no system-wide library in use

7dcc037

Remove USE_OPENCL (removed in ClickHouse#31744)

e160a20

Fix USE_XXHASH in StorageSystemBuildOptions.generated.cpp.in

9426e63

Improve add_contrib() (teach it about other dependencies)

70b3f1d

LZMA/xz cannot be disabled

e05a75f

Fix w/o ODBC build

e0e81b3

Fix build w/o hyperscan

66a2104

Fix build w/o replxx

3e58094

Remove USE_XXHASH (reduntant)

b23053d

Cleanup cpuid contrib

9926f33

Cleanup jemalloc contrib

a6d482d

Fix build hive w/o hdfs

97f9cf9

Fix librdkafka/hdfs that depends on sasl2/krb5

78858f9

azat force-pushed the remove-unbundled-support branch from 209871e to 78858f9 Compare January 20, 2022 07:03

azat commented Jan 20, 2022

View reviewed changes

alexey-milovidov merged commit 28a9d56 into ClickHouse:master Jan 20, 2022

azat deleted the remove-unbundled-support branch January 20, 2022 13:57

azat mentioned this pull request Jan 20, 2022

More cmake external modules cleanups #33842

Merged

qoega reviewed Jan 27, 2022

View reviewed changes

azat mentioned this pull request Jan 31, 2022

Cmake leftovers cleanup #34167

Merged

azat mentioned this pull request Feb 23, 2022

build_options stopped to show that some options are enabled #34812

Closed

seanlaff mentioned this pull request Mar 21, 2022

ArrowStream inputData compression over gRPC broken in v22.2.2.1+ #35283

Closed

Algunenano reviewed Feb 23, 2026

View reviewed changes

Conversation

azat commented Jan 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alexey-milovidov left a comment

Choose a reason for hiding this comment

Uh oh!

azat commented Jan 19, 2022

Uh oh!

azat commented Jan 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

amosbird commented Jan 19, 2022

Uh oh!

Uh oh!

amosbird Jan 19, 2022

Choose a reason for hiding this comment

Uh oh!

azat Jan 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

azat Jan 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

azat commented Jan 19, 2022

Uh oh!

alesapin commented Jan 19, 2022

Uh oh!

mergify bot commented Jan 19, 2022

✅ Branch has been successfully updated

Uh oh!

azat commented Jan 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

azat Jan 20, 2022

Choose a reason for hiding this comment

Uh oh!

azat commented Jan 20, 2022

Uh oh!

qoega Jan 27, 2022

Choose a reason for hiding this comment

Uh oh!

Algunenano Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

alexey-milovidov Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

azat commented Jan 16, 2022 •

edited

Loading

azat commented Jan 19, 2022 •

edited

Loading

azat Jan 19, 2022 •

edited

Loading

azat Jan 19, 2022 •

edited

Loading

azat commented Jan 19, 2022 •

edited

Loading

alexey-milovidov Feb 24, 2026 •

edited

Loading