Update to librdkafka/1.5.0 #2380

sourcedelica · 2020-08-03T08:36:26Z

Specify library name and version: librdkafka/1.5.0

Fixes #2378

I've read the guidelines for contributing.
I've followed the PEP8 style guides for Python code in the recipes.
I've used the latest Conan client version.
I've tried at least one configuration locally with the
conan-center hook activated.

CLAassistant · 2020-08-03T08:36:30Z

All committers have signed the CLA.

conan-center-bot · 2020-08-04T08:44:36Z

All green in build 2 (73461a4f8c5a327ce13abd034d2644d2be4b7f03)! 😊

librdkafka/1.3.0: Generated 130 packages. All logs here
librdkafka/1.4.0: Generated 130 packages. All logs here

recipes/librdkafka/all/patches/0002-Change-library-targets-and-result-variables-1-5-0.patch

conan-center-bot · 2020-08-09T19:46:05Z

All green in build 3 (21d5687f15b759961eb607961ef75a01da8140f6)! 😊

librdkafka/1.3.0: Generated 130 packages. All logs here
librdkafka/1.4.0: Generated 130 packages. All logs here

SSE4 · 2020-08-11T08:40:28Z

...es/librdkafka/all/patches/0001-Remove-cmake-module-path-and-change-library-names-1-5-0.patch

+
+ # ZSTD {
+-find_package(Zstd QUIET)
+find_package(zstd QUIET)


is it to be fixed in zstd cpp_info.name?

SSE4 · 2020-08-11T08:40:36Z

...es/librdkafka/all/patches/0001-Remove-cmake-module-path-and-change-library-names-1-5-0.patch

+ set(WITH_LZ4_EXT OFF)
+ if(ENABLE_LZ4_EXT)
+-  find_package(LZ4)
+  find_package(lz4)


is it to be fixed in lz4 cpp_info.name?

Sounds reasonable ... There is no official Cmake target, but we have some insights:

The upstream doesn't export the target, but the project name is LZ4 (uppercased) in cmake file: https://github.com/lz4/lz4/blob/dev/contrib/cmake_unofficial/CMakeLists.txt

There was PR for CMake, including LZ4: https://gitlab.kitware.com/cmake/cmake/-/merge_requests/2087

Just force set(LZ4_FOUND TRUE) and use CONAN_PKG::lz4 target, it's more robust for upstream recipes without neat cpp_info (or uncertainty about official CMake names).
It's how c-blosc recipe handles lz4 (robust to breaking changes in lz4 recipes):

conan-center-index/recipes/c-blosc/all/patches/cmake-dependencies-1.20.0+.patch

Line 13 in 06963b0

+ set(LZ4_FOUND TRUE)

conan-center-index/recipes/c-blosc/all/patches/cmake-dependencies-1.20.0+.patch

Line 41 in 06963b0

+ set(LIBS ${LIBS} "CONAN_PKG::lz4")

SpaceIm · 2020-08-26T16:24:49Z

Could you also add components in package_info (or I can open another PR)?

CMake config file is: RdKafkaConfig.cmake
CMake imported targets: RdKafka::rdkafka and RdKafka::rdkafka++
pkg config files: rdkafka.pc and rdkafka++.pc

SpaceIm · 2020-08-26T19:25:47Z

recipes/librdkafka/all/patches/0002-Change-library-targets-and-result-variables-1-5-0.patch

+     add_dependencies(rdkafka bundled-ssl)
+   else()
+     find_package(OpenSSL REQUIRED)
+-    target_include_directories(rdkafka PUBLIC ${OPENSSL_INCLUDE_DIR})


No need to remove this line. If ${OPENSSL_INCLUDE_DIR} is not defined, this instruction is silently ignored.

SpaceIm · 2020-08-26T19:26:14Z

recipes/librdkafka/all/patches/0002-Change-library-targets-and-result-variables-1-5-0.patch

+ if(WITH_LZ4_EXT)
+-  target_include_directories(rdkafka PUBLIC ${LZ4_INCLUDE_DIRS})
+-  target_link_libraries(rdkafka PUBLIC LZ4::LZ4)
+  target_link_libraries(rdkafka PUBLIC lz4::lz4)


Suggested change

+ target_link_libraries(rdkafka PUBLIC lz4::lz4)

+ target_link_libraries(rdkafka PUBLIC CONAN_PKG::lz4)

SpaceIm · 2020-08-26T19:28:40Z

recipes/librdkafka/all/patches/0002-Change-library-targets-and-result-variables-1-5-0.patch

+ if(WITH_SASL_CYRUS)
+-  target_include_directories(rdkafka PUBLIC ${SASL_INCLUDE_DIRS})
+-  target_link_libraries(rdkafka PUBLIC ${SASL_LIBRARIES})
+  target_link_libraries(rdkafka PUBLIC cyrus-sasl::cyrus-sasl)


Suggested change

+ target_link_libraries(rdkafka PUBLIC cyrus-sasl::cyrus-sasl)

+ target_link_libraries(rdkafka PUBLIC CONAN_PKG::cyrus-sasl)

cyrus-sasl doesn't have official CMake module or config file, more robust to use CONAN_PKG::cyrus-sasl, because you can't predict that upstream will not create official config file with different CMake imported target, leading to change in upstream recipe and breaking this one.

madebr · 2020-09-11T18:53:45Z

This recipe has conflicts with current master.

conan-center-bot · 2020-09-15T09:50:18Z

Failure in build 4 (21d5687f15b759961eb607961ef75a01da8140f6):

librdkafka/1.3.0
- Hooks errors detected:
  - [HOOK - conan-center.py] pre_export(): ERROR: [CONFIG.YML HAS NEW VERSION (KB-H052)] The version "1.5.0" exists in "conandata.yml" but not in "../config.yml", so it will not be built. Please update "../config.yml" to include newly added version "1.5.0". (https://github.com/conan-io/conan-center-index/blob/master/docs/error_knowledge_base.md#KB-H052)
librdkafka/1.4.0
- Hooks errors detected:
  - [HOOK - conan-center.py] pre_export(): ERROR: [CONFIG.YML HAS NEW VERSION (KB-H052)] The version "1.5.0" exists in "conandata.yml" but not in "../config.yml", so it will not be built. Please update "../config.yml" to include newly added version "1.5.0". (https://github.com/conan-io/conan-center-index/blob/master/docs/error_knowledge_base.md#KB-H052)

prince-chrismc · 2020-10-01T02:13:17Z

recipes/librdkafka/all/conanfile.py

@@ -83,7 +83,7 @@ def _configure_cmake(self):
        self._cmake.definitions["RDKAFKA_BUILD_STATIC"] = not self.options.shared
        self._cmake.definitions["RDKAFKA_BUILD_EXAMPLES"] = False
        self._cmake.definitions["RDKAFKA_BUILD_TESTS"] = False
-        self._cmake.definitions["WITHOUT_WIN32_CONFIG"] = True
+        self._cmake.definitions["WITHOUT_WIN32_CONFIG"] = self.settings.os == "Windows"


If windows don't use windows config ¿¿¿

uilianries closed this Aug 3, 2020

uilianries reopened this Aug 3, 2020

uilianries previously approved these changes Aug 4, 2020

View reviewed changes

uilianries requested review from jgsogo and SSE4 August 4, 2020 21:23

SSE4 previously approved these changes Aug 5, 2020

View reviewed changes

jgsogo suggested changes Aug 5, 2020

View reviewed changes

sourcedelica dismissed stale reviews from SSE4 and uilianries via 21d5687 August 9, 2020 19:05

jgsogo approved these changes Aug 10, 2020

View reviewed changes

uilianries approved these changes Aug 10, 2020

View reviewed changes

uilianries requested review from danimtb and SSE4 August 10, 2020 17:27

SSE4 reviewed Aug 11, 2020

View reviewed changes

SpaceIm reviewed Aug 26, 2020

View reviewed changes

conan-center-bot added the Failed label Sep 15, 2020

prince-chrismc reviewed Oct 1, 2020

View reviewed changes

sourcedelica closed this Oct 19, 2020

sourcedelica force-pushed the master branch from fb9ce70 to eecc279 Compare October 19, 2020 16:19

prince-chrismc mentioned this pull request Nov 11, 2020

[request] librdkafka/1.5.2 #3327

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update to librdkafka/1.5.0 #2380

Update to librdkafka/1.5.0 #2380

sourcedelica commented Aug 3, 2020

CLAassistant commented Aug 3, 2020 •

edited

Loading

conan-center-bot commented Aug 4, 2020

conan-center-bot commented Aug 9, 2020

SSE4 Aug 11, 2020

SSE4 Aug 11, 2020

uilianries Aug 11, 2020

SpaceIm Aug 26, 2020 •

edited

Loading

SpaceIm commented Aug 26, 2020 •

edited

Loading

SpaceIm Aug 26, 2020 •

edited

Loading

SpaceIm Aug 26, 2020

SpaceIm Aug 26, 2020 •

edited

Loading

madebr commented Sep 11, 2020

conan-center-bot commented Sep 15, 2020

prince-chrismc Oct 1, 2020

	+ target_link_libraries(rdkafka PUBLIC lz4::lz4)
	+ target_link_libraries(rdkafka PUBLIC CONAN_PKG::lz4)

Update to librdkafka/1.5.0 #2380

Update to librdkafka/1.5.0 #2380

Conversation

sourcedelica commented Aug 3, 2020

CLAassistant commented Aug 3, 2020 • edited Loading

conan-center-bot commented Aug 4, 2020

conan-center-bot commented Aug 9, 2020

SSE4 Aug 11, 2020

Choose a reason for hiding this comment

SSE4 Aug 11, 2020

Choose a reason for hiding this comment

uilianries Aug 11, 2020

Choose a reason for hiding this comment

SpaceIm Aug 26, 2020 • edited Loading

Choose a reason for hiding this comment

SpaceIm commented Aug 26, 2020 • edited Loading

SpaceIm Aug 26, 2020 • edited Loading

Choose a reason for hiding this comment

SpaceIm Aug 26, 2020

Choose a reason for hiding this comment

SpaceIm Aug 26, 2020 • edited Loading

Choose a reason for hiding this comment

madebr commented Sep 11, 2020

conan-center-bot commented Sep 15, 2020

prince-chrismc Oct 1, 2020

Choose a reason for hiding this comment

CLAassistant commented Aug 3, 2020 •

edited

Loading

SpaceIm Aug 26, 2020 •

edited

Loading

SpaceIm commented Aug 26, 2020 •

edited

Loading

SpaceIm Aug 26, 2020 •

edited

Loading

SpaceIm Aug 26, 2020 •

edited

Loading