Skip to content

Add Bazel build with BoringSSL support#501

Open
BYVoid wants to merge 1 commit into
ClickHouse:masterfrom
BYVoid:bazel-and-boringssl
Open

Add Bazel build with BoringSSL support#501
BYVoid wants to merge 1 commit into
ClickHouse:masterfrom
BYVoid:bazel-and-boringssl

Conversation

@BYVoid
Copy link
Copy Markdown

@BYVoid BYVoid commented May 27, 2026

Summary

  • Adds a Bazel (bzlmod) build alongside the existing CMake build, so downstream Bazel projects can depend on clickhouse-cpp via the Bazel Central Registry without standing up CMake.
  • TLS links against @boringssl (BCR ships BoringSSL natively, no OpenSSL module yet). The two OpenSSL-only surfaces — SSL_CONF_* command API and SSL_read_ex — are gated behind a new USE_BORINGSSL define in clickhouse/base/sslsocket.cpp. The patch is 17 lines, purely additive; when USE_BORINGSSL is undefined the file is byte-identical to before.
  • The CMake build is untouched.

What's added

File Purpose
MODULE.bazel Module declaration; deps: abseil-cpp, boringssl, lz4, rules_cc, zstd (all latest on BCR)
MODULE.bazel.lock Pinned dep graph
BUILD.bazel Two cc_library targets: :cityhash (private, from contrib/cityhash/cityhash/) and :clickhouse (public)
.gitignore Ignore bazel-* symlinks
clickhouse/base/sslsocket.cpp 17-line ifdef-guarded BoringSSL compat patch

sslsocket.cpp patch detail

Two #ifdef USE_BORINGSSL ... #else ... #endif regions:

  1. configureSSL() body — under USE_BORINGSSL becomes a no-op (the function is still defined and called, just does nothing). BoringSSL has no SSL_CONF_* API.
  2. SSLSocketInput::DoRead() — under USE_BORINGSSL falls back to SSL_read with len clamped to INT_MAX. SSL_read_ex is 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 produces libclickhouse.a on darwin-arm64 (Bazel 9.1.0) and Linux.

Notes for reviewers

  • cityhash stays vendored under contrib/cityhash/cityhash/ because it isn't on BCR. lz4/zstd/abseil are pulled from BCR; contrib/{lz4,zstd,absl} are still used by the CMake build.
  • Draft because I haven't wired up Bazel CI yet — happy to add a workflow if the direction is welcome.

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented May 27, 2026

CLA assistant check
All committers have signed the CLA.

@BYVoid BYVoid force-pushed the bazel-and-boringssl branch from 5ff5053 to a2c545a Compare May 27, 2026 04:26
@BYVoid BYVoid marked this pull request as ready for review May 27, 2026 04:29
Copilot AI review requested due to automatic review settings May 27, 2026 04:29
@BYVoid BYVoid requested review from mzitnik and slabko as code owners May 27, 2026 04:29
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.bazel with bzlmod deps (abseil, boringssl, lz4, zstd, rules_cc).
  • Added BUILD.bazel with :clickhouse cc_library and vendored :cityhash.
  • Updated TLS implementation to compile under BoringSSL by disabling SSL_CONF_* usage and using SSL_read instead of SSL_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.

Comment thread BUILD.bazel
Comment on lines +57 to +61
# Expose headers at their workspace path so both internal
# (`#include "client.h"` from clickhouse/*.cpp via quote-form
# source-dir search) and external (`#include <clickhouse/client.h>`)
# include forms resolve.
strip_include_prefix = "/",
Comment on lines 52 to +58
void configureSSL(const clickhouse::SSLParams::ConfigurationType & configuration, SSL * ssl, SSL_CTX * context = nullptr) {
#ifdef USE_BORINGSSL
// BoringSSL doesn't ship the SSL_CONF_* command API, so this becomes
// a no-op. The configuration vector is ignored.
(void)configuration;
(void)ssl;
(void)context;
Comment on lines +299 to +302
const int max_read = static_cast<int>(
std::min<std::size_t>(len, std::numeric_limits<int>::max()));
return static_cast<size_t>(
HANDLE_SSL_ERROR(ssl_, SSL_read(ssl_, buf, max_read)));
@BYVoid
Copy link
Copy Markdown
Author

BYVoid commented May 27, 2026

Why BoringSSL (and not OpenSSL) in this PR

A few people will probably ask why TLS goes through @boringssl rather than @openssl here. Some context:

  1. OpenSSL on BCR still has rough edges. The current OpenSSL modules don't build cleanly out of the box across all platforms/toolchains we tried, while @boringssl builds reliably with no extra rules.
  2. Single-TLS-in-process constraint. Many Bazel workspaces already link BoringSSL transitively (gRPC, Envoy, abseil-adjacent libs). Pulling in OpenSSL alongside leads to duplicate symbols, conflicting libssl/libcrypto initialization, and ODR violations. Picking BoringSSL keeps clickhouse-cpp compatible with that ecosystem without forcing consumers to fight their dep graph.

The USE_BORINGSSL define is intentionally a compile-time switch, not hardcoded behavior — the OpenSSL-only code paths are still present in sslsocket.cpp and active whenever USE_BORINGSSL is undefined (which is the case for the existing CMake build).

Follow-up: once OpenSSL's BCR story stabilizes, it's straightforward to add a Bazel config_setting (e.g. --@clickhouse_cpp//:tls=openssl|boringssl) that flips the defines and the deps between @openssl and @boringssl, letting consumers choose. Wanted to keep this PR scoped to a single working configuration first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants