librdkafka
librdkafka copied to clipboard
Support finding LZ4 from installed package
When built with ENABLE_LZ4_EXT=ON, consuming RdKafka via find_package also requires lz4. Since lz4 doesn't bring an LZ4Config.cmake, this package is found using RdKafka's custom FindLZ4.cmake. But this "find module" isn't searched in RdKafka's install target by default 1.
Attempting to do so results in By not providing "FindLZ4.cmake" in CMAKE_MODULE_PATH this project has asked CMake to find a package configuration file provided by "LZ4", but CMake did not find one. [...] with current releases.
This is why extending the CMAKE_MODULE_PATH to the RdKafka install target is necessary. This is actually common practice e.g. 2.
I might need some help with Doozer CI errors:
Status: Temporary failure: GIT: Unable to create GIT repo -- failed to make directory './(null)': Permission denied because Unable to create heap repo.BxCgKo6DQq.0 -- Read-only file system, No more retries
Doozer had some issues unrelated to your PR, but the cmake job passed so that's good :+1:
Pinging @Oxymoron79 @neptoess @benesch @snar for review
Pinging @Oxymoron79 @neptoess @benesch @snar for review
Looks good to me. A nice part about the way this is done is that, if LZ4 ever starts shipping their own Find.cmake, this will automatically use that instead of the rdkafka one.
Pinging @Oxymoron79 @neptoess @benesch @snar for review
Looks good to me as well.
I just wonder if the chosen installation directory for the FindLZ4.cmake file (see https://github.com/edenhill/librdkafka/blob/master/CMakeLists.txt#L250) should be adjusted to install it to a directory that CMake searches by default?
Pinging @Oxymoron79 @neptoess @benesch @snar for review
Looks good to me as well.
I just wonder if the chosen installation directory for the FindLZ4.cmake file (see https://github.com/edenhill/librdkafka/blob/master/CMakeLists.txt#L250) should be adjusted to install it to a directory that CMake searches by default?
Definitely a little bit of a tricky decision. If this were the CMake install command for the LZ4 library, I would 100% agree that FindLZ4 should end up in a directory CMake searches by default. But this is librdkafka. Not sure it makes sense for this library to distribute CMake Find* scripts for other libraries.
I just wonder if the chosen installation directory for the FindLZ4.cmake file (see https://github.com/edenhill/librdkafka/blob/master/CMakeLists.txt#L250) should be adjusted to install it to a directory that CMake searches by default?
Probably complicated: According to the cmake find procedure documentation, this requires writing files into some folder named lz4. This likely raises concerns of LZ4, e.g. might cause conflicts if they ever were to distribute their own cmake-config.
@neptoess, @m8mble Good points.Thanks for your explanations.
Is there anything I can do to push this PR over the line?
For my future self: the issue can be worked around by adding -DCMAKE_MODULE_PATH=/usr/lib64/cmake/RdKafka at CMake configure time.
Since this might affect the vcpkg, can you review this @myd7349 ?
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.
@emasab, still have the same problem with
FetchContent_Declare(
RdKafka
GIT_REPOSITORY https://github.com/confluentinc/librdkafka.git
GIT_TAG v2.4.0
FIND_PACKAGE_ARGS
)
when the lib is installed