Closed
Conversation
Contributor
|
Workflow [PR], commit [97d48e1] Summary: ❌
|
clickhouse-gh
bot
added
pr-improvement
Fix the CityHash128 linkage issue in the vendored clickhouse-rs-cityhash-sys crate by adding `extern "C"` to the declaration in city.h and fixing the signature to use `const char*`. The issue was that CityHashCrc128 called CityHash128 using C++ linkage (looking for mangled symbol `__Z11CityHash128PKcm`), but CityHash128 was defined with C linkage (`extern "C"`). Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
alexey-milovidov
force-pushed
the
enable-chdig-on-mac
branch
from
7575ee0 to
6df3f93
Compare
Fix CityHash128 linkage for macOS compatibility by adding extern "C" to the declaration in city.h. This fixes undefined symbol error on x86_64 Darwin where CityHashCrc128 (compiled only with SSE4.2) calls CityHash128. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This adds a forward declaration of CityHash128 with extern "C" linkage directly in the SSE4.2 code block to ensure the correct symbol is called regardless of header include order. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
azat
reviewed
Feb 5, 2026
Member
There was a problem hiding this comment.
👍
But changes to clickhosue-rs-cityhash-sys will be overwritten by the next rust/vendor.sh, we need to clone it and change the dependencies, and I doubt that it can be done outside of chdig, so chdig/Cargo.toml should be updated
P.S. interesting why it works using cargo directly, probably due to default linker?
Member
|
Maybe the build hit some cache, or similar issue But anyway I've fixed it here - azat/chdig#220 But now we have new issues DetailsI will take a look |
Member
Author
|
I think we explicitly need to link certain "System Framework" libraries on Mac. |
Member
|
Will be done as part of upgrade to newer version - #96113 That includes proper fixes for build system for MacOS |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix the CityHash128 linkage issue in the vendored clickhouse-rs-cityhash-sys crate by adding
extern "C"to the declaration in city.h and fixing the signature to useconst char*.The issue was that CityHashCrc128 called CityHash128 using C++ linkage (looking for mangled symbol
__Z11CityHash128PKcm), but CityHash128 was defined with C linkage (extern "C").Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):
The chdig tool is available on macOS.
See ClickHouse/rust_vendor#55