Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Package modern-cpp-kafka with vcpkg #200

Open
BenSleat opened this issue Feb 7, 2023 · 4 comments
Open

Package modern-cpp-kafka with vcpkg #200

BenSleat opened this issue Feb 7, 2023 · 4 comments

Comments

@BenSleat
Copy link

BenSleat commented Feb 7, 2023

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
Copy link
Author

BenSleat commented Feb 7, 2023

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
Copy link
Contributor

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
Copy link

@xiaozhuai
Copy link

xiaozhuai commented Aug 2, 2023

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 microsoft/vcpkg#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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants