add recipe for cassandra-cpp-driver #3110
Conversation
|
|
|
Some configurations of 'cassandra-cpp-driver/2.15.3' failed in build 1 (
|
|
All green in build 2 (
|
| if self.settings.os == "Macos": | ||
| raise ConanInvalidConfiguration("Macos is unsuported") |
There was a problem hiding this comment.
This can be moved to configure
| # Compilation error on Linux with clang | ||
| # cmake.definitions["CASS_USE_BOOST_ATOMIC"] = True | ||
| # cmake.definitions["CASS_USE_STD_ATOMIC"] = False | ||
| raise ConanInvalidConfiguration( | ||
| "Boost.Atomic is not supported at the moment") |
There was a problem hiding this comment.
Does it work for gcc@Linux?
If so, you can move the throw to configure (and only if use_atomic=True and compiler=clang) and set the cmake definitions here unconditionally.
There was a problem hiding this comment.
I don't think. Bincrafters has a similar recipe, Linux is excluded: https://github.com/bincrafters/conan-cassandra-driver
There was a problem hiding this comment.
Tested with GCC and Clang on Linux - compilation failed. Boost.Atomic is switched off for Linux. But I switched it on for Windows.
Done at
| cmake.definitions["CASS_USE_OPENSSL"] = self.options.with_openssl | ||
| cmake.definitions["CASS_USE_ZLIB"] = self.options.with_zlib |
There was a problem hiding this comment.
Is this indentation correct?
| -project(cassandra C CXX) | ||
| +# project(cassandra C CXX) | ||
| + | ||
| +# include(${CMAKE_BINARY_DIR}/conanbuildinfo.cmake) | ||
| +# conan_basic_setup() |
There was a problem hiding this comment.
I think these lines are harmless. Or aren't they?
| -option(CASS_BUILD_EXAMPLES "Build examples" OFF) | ||
| -option(CASS_BUILD_INTEGRATION_TESTS "Build integration tests" OFF) | ||
| -option(CASS_BUILD_SHARED "Build shared library" ON) | ||
| -option(CASS_BUILD_STATIC "Build static library" OFF) | ||
| -option(CASS_BUILD_TESTS "Build tests" OFF) | ||
| -option(CASS_BUILD_UNIT_TESTS "Build unit tests" OFF) | ||
| -option(CASS_DEBUG_CUSTOM_ALLOC "Debug custom allocator" OFF) |
There was a problem hiding this comment.
Instead of removing these options and carrying a heavy patch,
I think you can set the definitions in the conanfile.
e.g.
cmake.definitions["CASS_BUILD_EXAMPLES"] = FalseThere was a problem hiding this comment.
Setting it this way will make the patch much lighter, I think.
Because you don't have to re-do the add_library for shared/static thing.
There was a problem hiding this comment.
Please notice: I did not revert the part where I can choose which lib to build shared/static.
| + | ||
| +if(WIN32) | ||
| + target_link_libraries(${PROJECT_LIB_NAME_TARGET} | ||
| + iphlpapi |
There was a problem hiding this comment.
These system libraries should also be added to the conanfile.
| diff --git a/src/third_party/minizip/CMakeLists.txt b/src/third_party/minizip/CMakeLists.txt | ||
| index 569bf91f6..b2e189360 100644 | ||
| --- a/src/third_party/minizip/CMakeLists.txt | ||
| +++ b/src/third_party/minizip/CMakeLists.txt |
There was a problem hiding this comment.
Can we use the minizip from conan instead of the vendored minizip here?
This will avoid a linker error when linking to a static minizip recipe and cassandra-cpp-driver.
| --- a/src/third_party/http-parser/CMakeLists.txt | ||
| +++ b/src/third_party/http-parser/CMakeLists.txt | ||
| @@ -3,15 +3,8 @@ file(GLOB SOURCES http_parser.c http_parser.h) | ||
| source_group("Source Files\\" REGULAR_EXPRESSION ".*\\.c(pp)?") |
There was a problem hiding this comment.
Can we use a conan http-parser?
| --- a/src/third_party/hdr_histogram/CMakeLists.txt | ||
| +++ b/src/third_party/hdr_histogram/CMakeLists.txt | ||
| @@ -3,15 +3,8 @@ file(GLOB SOURCES *.cpp *.hpp) | ||
| source_group("Source Files\\" REGULAR_EXPRESSION ".*\\.c(pp)?") |
There was a problem hiding this comment.
Can we use a conan hdr_histogram?
There was a problem hiding this comment.
No we can not. Cassandra uses patched hdr_histogram version.
| --- a/src/third_party/curl/CMakeLists.txt | ||
| +++ b/src/third_party/curl/CMakeLists.txt | ||
| @@ -3,16 +3,8 @@ file(GLOB SOURCES *.cpp *.hpp) | ||
| source_group("Source Files\\" REGULAR_EXPRESSION ".*\\.c(pp)?") |
There was a problem hiding this comment.
We should really use the conan curl recipe/package.
(unless this directory only contains the code to use curl instead of the curl source tree itself)
There was a problem hiding this comment.
Unfortunately curl implementation that uses Cassandra represents a private piece of code from original sources.
There was a problem hiding this comment.
Does replacing these sources with a link to CONAN_PKG::libcurl work?
If libcurl is built as a static library, all symbols are visible.
There was a problem hiding this comment.
@madebr But what about headers? https://github.com/curl/curl/blob/master/lib/hostcheck.h seems to be private as well. Anyway, such breaking through the public library API looks tricky to me.
There was a problem hiding this comment.
Indeed, you're right. No hostcheck.h is installed by cci's libcurl.
Can you try the following?
Add libcurl/7.72.0 to the requirements of the test_package and see whether libcurl and cassandra cause problems.
Either way, you cannot use cci's libcurl.
But this may cause a static libcurl being incompatible with cassandra-cpp-driver.
There was a problem hiding this comment.
Are you talking about violating of ODR for Curl_cert_hostcheck?
There was a problem hiding this comment.
Yes, symbol of static libcurl.a <-> this one.
There was a problem hiding this comment.
So, it seems that the code of cassandra driver needs to be patched, but is this a good way to solve this problem? In comparison to adding a package to CCI with some usage limitations.
There was a problem hiding this comment.
@vaerizk
Limitations are totally fine.
We're still in the process of determining what those limitations are.
If a patch can fix the issues, maybe these can be upstreamed and others can benefit from them.
There was a problem hiding this comment.
@madebr We have tested the test_package with cassandra-cpp-driver and libcurl dependencies included at the same time. Seems it works by good fortune 😄 .
We have included the following examples, using SSL - this one for cassandra driver, and this one for libcurl - into one executable to make sure both Curl_cert_hostcheck-s are invoked and preserved into final executable after linking. The package with test executable is built using -o shared=false to use static variants of dependencies and -s build_type=Debug to keep readable names in the output PE binary.
And now we have two symbols in the executable:
?Curl_cert_hostcheck@@YAHPEBD0@Z(unmangled:int __cdecl Curl_cert_hostcheck(char const * __ptr64,char const * __ptr64)) - from https://github.com/datastax/cpp-driver/blob/master/src/third_party/curl/hostcheck.cpp , compiled as C++ code;Curl_cert_hostcheck- from https://github.com/curl/curl/blob/master/lib/hostcheck.c#L128 , compiled as C code.
Co-authored-by: Anonymous Maarten <madebr@users.noreply.github.com>
Co-authored-by: Anonymous Maarten <madebr@users.noreply.github.com>
…e to use from conanfile.py
|
All green in build 14 (
|
| - base_path: "source_subfolder" | ||
| patch_file: "patches/2.15.3/fix-rapidjson.patch" | ||
| patch_file: "patches/2.15.3/fix-cmake.patch" |
There was a problem hiding this comment.
| - base_path: "source_subfolder" | |
| patch_file: "patches/2.15.3/fix-rapidjson.patch" | |
| patch_file: "patches/2.15.3/fix-cmake.patch" | |
| - base_path: "source_subfolder" | |
| patch_file: "patches/2.15.3/fix-rapidjson.patch" | |
| - base_path: "source_subfolder" | |
| patch_file: "patches/2.15.3/fix-cmake.patch" |
There was a problem hiding this comment.
| "fPIC": [True, False], | ||
| "install_header_in_subdir": [True, False], | ||
| "use_atomic": [None, "boost", "std"], | ||
| "with_openssl": [True, False], |
There was a problem hiding this comment.
Is openssl the only tls provider?
Some packages offer wolfssl/libretls/... support.
There was a problem hiding this comment.
Yep openssl is the only tls provider in this package.
Co-authored-by: Anonymous Maarten <madebr@users.noreply.github.com>
|
Some configurations of 'cassandra-cpp-driver/2.15.3' failed in build 15 (
|
|
Some configurations of 'cassandra-cpp-driver/2.15.3' failed in build 16 (
|
|
All green in build 17 (
|
| self._cmake.definitions["CASS_USE_ZLIB"] = self.options.with_zlib | ||
| self._cmake.definitions["CASS_USE_LIBSSH2"] = False | ||
|
|
||
| # FIXME |
There was a problem hiding this comment.
| # FIXME | |
| # FIXME |
FIXME what? Please, add more detail on that tag.
|
All green in build 18 (
|
|
All green in build 19 (
|
Specify library name and version: cassandra-cpp-driver/2.15.3
conan-center hook activated.