Add Bazel build with BoringSSL support#501
Conversation
5ff5053 to
a2c545a
Compare
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds Bazel (bzlmod) build support for clickhouse-cpp and adapts TLS codepaths to work with BoringSSL.
Changes:
- Added
MODULE.bazelwith bzlmod deps (abseil, boringssl, lz4, zstd, rules_cc). - Added
BUILD.bazelwith:clickhousecc_library and vendored:cityhash. - Updated TLS implementation to compile under BoringSSL by disabling
SSL_CONF_*usage and usingSSL_readinstead ofSSL_read_ex.
Reviewed changes
Copilot reviewed 3 out of 5 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| clickhouse/base/sslsocket.cpp | Adds BoringSSL-specific branches for SSL configuration and reading APIs. |
| MODULE.bazel | Introduces a bzlmod module definition and external deps for Bazel builds. |
| BUILD.bazel | Adds Bazel targets to build/export the library and its vendored cityhash dependency. |
| .gitignore | Ignores Bazel output symlinks/directories. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Why BoringSSL (and not OpenSSL) in this PR A few people will probably ask why TLS goes through
The Follow-up: once OpenSSL's BCR story stabilizes, it's straightforward to add a Bazel |
|
@BYVoid, thank you for your contribution! However, before we proceed, I’d like to clarify two things:
|
|
Yes. I can help maintain the Bazel build support and probably submit it to https://registry.bazel.build/ in near future. I will try to add a github worklow to cover the Bazel build. |
Adds a Linux Bazel CI job that runs bazel build //... so the bzlmod build is exercised on push and pull requests, mirroring the existing CMake workflows.
|
@slabko I added a Github workflow. Can you grant me the permission to run it? |
|
Hi @BYVoid, I am trying to learn the basics of Bazel before we merge it. This is still in progress, but my plan is to merge it soon. I have a couple of new questions:
|
|
Great questions! They all trace back to Bazel's core idea: hermetic builds. A Bazel build should depend only on declared, version-pinned inputs, never on whatever happens to be installed on the host. Everything is built from pinned sources and (typically) statically linked, so the same commit produces the same binary on any machine. This is also the prevailing convention on the Bazel Central Registry: modules build their dependencies from source and avoid system packages whenever possible. The ecosystem takes this idea quite far, even with efforts like hermetic-llvm that make the compiler toolchain itself a hermetic part of the build. 1. System OpenSSL: Linking system libraries is possible in Bazel but goes against this philosophy, so I'd rather not make it the default. Your concern about security patching is valid, but the Bazel ecosystem solves it differently: instead of relying on the distro to swap a shared library at runtime, the usual workflow is to bump the pinned version in 2. zstd/lz4/Abseil: same treatment, all pinned BCR modules. Thanks for the heads-up on Abseil! Once you remove it, the Bazel side is just deleting one line from 3. Build options: Bazel has user-defined flags for exactly this. See ccronexpr's BUILD.bazel for an example. Options like Overall my suggestion: let the Bazel build stay hermetic. Since every dependency version is fully under control, many enterprise users actually prefer this model: the resulting binaries carry no expectations about what's installed on the host, which to some extent simplifies container (Docker) images, often down to a minimal base image plus the binary. For users who prefer system-installed dependencies, CMake remains the right tool. The two builds serve different philosophies, and that's fine. 🙂 |
Adds --@clickhouse_cpp//:tls=boringssl|openssl (default: boringssl). Both TLS implementations come from BCR modules and are linked statically, keeping the build hermetic. USE_BORINGSSL and the TLS deps are now switched via select() on the flag.
BoringSSL doesn't ship the SSL_CONF_* command API, so SSLParams::configuration cannot be applied. Print a one-time warning to stderr instead of silently dropping potentially security-relevant settings, and point users at the OpenSSL build option.
|
@slabko I added a |
abseil-cpp 20260107.1 -> 20260526.0 boringssl 0.20260508.0 -> 0.20260526.0 rules_cc 0.2.18 -> 0.2.19
cityhash 1.0.2 is now available on the Bazel Central Registry, so the Bazel build no longer compiles the vendored copy under contrib/cityhash/. The contrib/ tree is still used by the CMake build.
Build matrix: {ubuntu-24.04, macos-latest} x {boringssl, openssl}.
Summary
--@clickhouse_cpp//:tls=boringssl|openssl(default:boringssl). Both come from BCR modules and are linked statically, keeping the build hermetic.SSL_CONF_*command API andSSL_read_ex— are gated behind aUSE_BORINGSSLdefine inclickhouse/base/sslsocket.cpp, set only when building with the BoringSSL backend. WhenUSE_BORINGSSLis undefined (CMake, or Bazel withtls=openssl) the original code paths are used unchanged.What's added
MODULE.bazelabseil-cpp,bazel_skylib,boringssl,cityhash,lz4,openssl,rules_cc,zstd(all latest on BCR)MODULE.bazel.lockBUILD.bazel:tlsstring flag + config settings; the public:clickhousecc_library target.github/workflows/bazel.ymlbazel build //:clickhouseon Linux.gitignorebazel-*symlinksclickhouse/base/sslsocket.cppTLS backend selection
Implemented with a
bazel_skylibstring_flag+config_setting+select()ondefinesanddeps. BoringSSL is the default because it builds reliably across platforms on BCR today and matches what many Bazel workspaces (gRPC, Envoy) already link. The same mechanism is the natural place for future build options (e.g.CH_MAP_BOOL_TO_UINT8).sslsocket.cpp patch detail
Two
#ifdef USE_BORINGSSL ... #else ... #endifregions:configureSSL()body — BoringSSL has noSSL_CONF_*API, so the commands cannot be applied. IfSSLParams::configurationis non-empty, a one-time warning is printed to stderr pointing at the OpenSSL build option, instead of silently dropping potentially security-relevant settings.SSLSocketInput::DoRead()— underUSE_BORINGSSLfalls back toSSL_readwithlenclamped toINT_MAX.SSL_read_exis an OpenSSL 3.0+ API not in BoringSSL.Function signatures, declarations,
SSLParams::ConfigurationType,validateParams, etc. all stay intact, so the public API is unchanged regardless of which TLS backend is linked.Test plan
bazel build //:clickhouse(BoringSSL default) produceslibclickhouse.aon darwin-arm64 (Bazel 9.1.1) and Linux.bazel build //:clickhouse --@clickhouse_cpp//:tls=opensslbuilds OpenSSL 3.5.5 from source and links successfully.Notes for reviewers
cityhash@1.0.2(1.0.2 is the algorithm revision ClickHouse's wire checksums require). Thecontrib/trees are untouched and still used by the CMake build.bazel_dep.