Skip to content

Conversation

@tillrohrmann
Copy link

Instead of relying only on the pkg-config to find the SASL2 library, we should also respect the CMAKE_PREFIX_PATH by using find_package in case sasl2 should be linked statically.

Instead of relying only on the pkg-config to find the SASL2 library, we should also respect
the CMAKE_PREFIX_PATH by using find_package in case sasl2 should be linked statically.
Copilot AI review requested due to automatic review settings November 13, 2025 15:17
@tillrohrmann tillrohrmann requested a review from a team as a code owner November 13, 2025 15:17
@confluent-cla-assistant
Copy link

confluent-cla-assistant bot commented Nov 13, 2025

🎉 All Contributor License Agreements have been signed. Ready to merge.
✅ tillrohrmann
Please push an empty commit if you would like to re-run the checks to verify CLA status for all contributors.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR enhances the SASL2 library detection mechanism in CMake by adding find_package support alongside the existing pkg-config method. This allows the build system to respect CMAKE_PREFIX_PATH for static linking scenarios.

Key changes:

  • Added find_package(SASL2) as the primary detection method before falling back to pkg-config
  • Restructured the conditional logic to check for SASL2 via find_package first

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +131 to +146
if(PkgConfig_FOUND)
pkg_check_modules(SASL libsasl2)
if(SASL_FOUND)
set(with_sasl_default ON)
set(SASL_LIBRARIES "-lsasl2")
else()
set(with_sasl_default OFF)
endif()
else()
try_compile(
WITH_SASL_CYRUS_BOOL
"${CMAKE_CURRENT_BINARY_DIR}/try_compile"
"${TRYCOMPILE_SRC_DIR}/libsasl2_test.c"
LINK_LIBRARIES "-lsasl2"
)
if(WITH_SASL_CYRUS_BOOL)
set(with_sasl_default ON)
set(SASL_LIBRARIES "-lsasl2")
else()
set(with_sasl_default OFF)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure whether we still need this if we are using find_package(SASL2).

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

Successfully merging this pull request may close these issues.

1 participant