morganstanley / modern-cpp-kafka

A C++ API for Kafka clients (i.e. KafkaProducer, KafkaConsumer, AdminClient)
Apache License 2.0
358 stars 89 forks source link

Package modern-cpp-kafka with vcpkg #200

Open BenSleat opened 1 year ago

BenSleat commented 1 year ago

It would be great if you could create a vcpkg packaged version of modern-cpp-kafka.

I've tried creating one myself (I'm new to creating vcpkg packages) but I'm having problems with the CMakeLists.txt file. Specifically line 66 is complaining that it can't find the librdkafka headers, even though I've set librdkafka as a dependency in vcpkg.json. It looks like the CMakeLists.txt expects the headers location to be set through env vars, which isn't the right way to do it with a vcpkg package.

BenSleat commented 1 year ago

I also note that the HINTS in the CMakeLists.txt file seem to be Linux specific. Will this still work on Windows (I need to target both Linux and Windows):

        find_file(LIBRDKAFKA_HEADER
             NAMES rdkafka.h
             HINTS /usr/include/librdkafka /usr/local/include/librdkafka /opt/homebrew/include/librdkafka)
kenneth-jia commented 1 year ago

Hi, @BenSleat You could define the env variables LIBRDKAFKA_INCLUDE_DIR/LIBRDKAFKA_LIBRARY_DIR before running the cmake, -- it would be a simple workaround. E.g. https://github.com/morganstanley/modern-cpp-kafka/blob/main/.github/workflows/kafka_api_ci_tests.yml#L324

zlojvavan commented 1 year ago

see also [[New Port Request] <morganstanley/modern-cpp-kafka>](https://github.com/microsoft/vcpkg/issues/32897#top)

xiaozhuai commented 1 year ago

Note: I didn't use the CMakeLists.txt provide by this project, but write a new one from scratch. Just as @BenSleat said

It looks like the CMakeLists.txt expects the headers location to be set through env vars, which isn't the right way to do it with a vcpkg package.

Currentlly it export with unofficial:: namespace, see https://github.com/microsoft/vcpkg/pull/32903

install(TARGETS modern-cpp-kafka EXPORT unofficial-modern-cpp-kafka)

install(
    EXPORT unofficial-modern-cpp-kafka
    FILE unofficial-modern-cpp-kafka-config.cmake
    DESTINATION "${CMAKE_INSTALL_DATAROOTDIR}/unofficial-modern-cpp-kafka"
    NAMESPACE unofficial::modern-cpp-kafka::
)

install(
    DIRECTORY "${CMAKE_SOURCE_DIR}/include/kafka"
    DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}"
)

Allow follwing usage

find_package(unofficial-modern-cpp-kafka CONFIG REQUIRED)
target_link_libraries(main PRIVATE unofficial::modern-cpp-kafka::modern-cpp-kafka)

It would be better if we provide an official export so I can drop the unofficial:: namespace in vcpkg port. Then the usage will become

find_package(modern-cpp-kafka CONFIG REQUIRED)
target_link_libraries(main PRIVATE modern-cpp-kafka::modern-cpp-kafka)

@kenneth-jia If that look good for you, I can make a pr for this.