-
-
Notifications
You must be signed in to change notification settings - Fork 12.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
clickhouse-cpp 2.4.0 #128575
clickhouse-cpp 2.4.0 #128575
Conversation
|
Thank you @ZhongRuoyu 🙏 |
inreplace "CMakeLists.txt", "FIND_PACKAGE(cityhash REQUIRED)", | ||
"FIND_LIBRARY(CITYHASH NAMES cityhash REQUIRED)" | ||
inreplace "clickhouse/CMakeLists.txt", "cityhash::cityhash", "cityhash" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we upstream this? Or has upstream forked cityhash to provide a CMake config file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Upstream has written a simple CMake config file for cityhash
: https://github.com/ClickHouse/clickhouse-cpp/blob/master/contrib/cityhash/cityhash/CMakeLists.txt.
But since cityhash
does not provide any CMake modules, the setup with WITH_SYSTEM_CITYHASH=ON
is basically broken. I will upstream a proper fix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Upstream has written a simple CMake config file for
cityhash
: https://github.com/ClickHouse/clickhouse-cpp/blob/master/contrib/cityhash/cityhash/CMakeLists.txt.
That's what I thought. This project likes to just fork all their dependencies.
🤖 A scheduled task has requested bottles to be published to this PR. |
* clickhouse-cpp 2.4.0 * clickhouse-cpp: fix build Also: 1. Add deps that would otherwise be vendored. 2. Eliminate CMake usage in test. Closes #128575. Co-authored-by: Ruoyu Zhong <[email protected]> Signed-off-by: Rui Chen <[email protected]> Signed-off-by: BrewTestBot <[email protected]>
Created by
brew bump
Created with
brew bump-formula-pr
.release notes
ColumnTuple::At()
. (#268 by @huyphams)New options (
clickhouse::ClientOptions
)Other
CI/CD
system.query_logs
-dependent unit-tests (#267 by @Enmk)New Contributors
Full Changelog: ClickHouse/clickhouse-cpp@v2.3.0...v2.4.0