Conversation
|
|
| VERSION ${CLICKHOUSE_CPP_VERSION} | ||
| ) | ||
| TARGET_LINK_LIBRARIES (clickhouse-cpp-lib | ||
| absl::int128 |
There was a problem hiding this comment.
Why did you mark dependencies private?
There was a problem hiding this comment.
Since all dependencies are static libraries the exported target should not propagate these dependencies further downstream.
There was a problem hiding this comment.
Clickhouse-cpp-lib can be built as shared library
There was a problem hiding this comment.
Yes, this is how I build it for packaging. The dependencies are still static though.
There was a problem hiding this comment.
Yes, this is how I build it for packaging. The dependencies are still static though.
Dependencies may be shared, for example, conan packaging. In this case, looks like package will be broken.
There was a problem hiding this comment.
The package should not be broken, but the exported CMake target config will be missing dependencies. Is clickhouse-cpp built with conan? I don't see any configuration for it anywhere.
There was a problem hiding this comment.
https://conan.io/center/recipes/clickhouse-cpp?version=2.5.1 and here is the example of usage https://github.com/userver-framework/userver/blob/develop/conanfile.py#L162
There was a problem hiding this comment.
Okay, it seems like conan generates its own CMake config file, so shouldn't be affected.
https://github.com/conan-io/conan-center-index/blob/master/recipes/clickhouse-cpp/all/conanfile.py#L106
I know nothing about conan though, so I'd be grateful if you verify this.
There was a problem hiding this comment.
it seems like conan generates its own CMake config file, so shouldn't be affected
Okay, I forgot about this. From this point of view, everything is ok. Thank you :)
There was a problem hiding this comment.
There is of course some room for improvement in how I generate the config file (it's probably not really how you are supposed to do it) and it will most likely not cover all possible usecases. I figured out that it might still be useful as is, after all it's an improvement from not having one at all.
|
Hey, guys, do you have any updates? I wrote almost similar patch for my own packaging 😂 I think I could contribute, but found this more corect PR ( |
|
I'd need to update it to 2.6.0 (which I already did on my debian branch, it's just a matter of porting the changes here). |
4 participants
I started working on Debian packaging for the clickhouse-cpp library (not officialy, I need the packages for internal use) and I made some improvements to CMake setup that makes it play nicely with packaging tools.
I thought some of them can be useful for the project.
I tried to split my changes into logical commits so that it's easy to cherry-pick a subset of the changes.
Apparently also closes #105