Issue-5369-Fix CMake Oauthbearer OIDC on Windows#5370
Issue-5369-Fix CMake Oauthbearer OIDC on Windows#5370Marc Gutman (icanhasmath) wants to merge 2 commits intoconfluentinc:masterfrom
Conversation
WITH_CURL and WITH_OAUTHBEARER_OIDC were missing from Windows CMAKE setup, causing them to be treated as 0 by the preprocessor regardless of the CMake option values, as cmake does not use win32_config.h.
Use WITH_SSL and WITH_CURL only.
|
Please sign the Contributor License Agreement here before this PR can be approved. |
There was a problem hiding this comment.
Pull request overview
This PR addresses #5369 by aligning Windows CMake builds (when WITHOUT_WIN32_CONFIG is enabled) with the top-level CMakeLists.txt feature gating so that WITH_OAUTHBEARER_OIDC is reflected correctly at compile time when WITH_CURL and WITH_SSL are enabled.
Changes:
- Add
WITH_CURLcompile definitions (1/0) under theWITHOUT_WIN32_CONFIGpath. - Add
WITH_OAUTHBEARER_OIDCcompile definitions gated onWITH_SSL AND WITH_CURLunder theWITHOUT_WIN32_CONFIGpath.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| else() | ||
| list(APPEND rdkafka_compile_definitions WITH_OAUTHBEARER_OIDC=0) |
There was a problem hiding this comment.
Defining WITH_OAUTHBEARER_OIDC=0 still makes the macro defined, which can unintentionally enable #ifdef WITH_OAUTHBEARER_OIDC code paths. In particular, src/rdkafka_conf.c includes <curl/curl.h> under #ifdef WITH_OAUTHBEARER_OIDC, so this change can force a curl header dependency even when WITH_CURL/OIDC are disabled and break builds where libcurl headers aren't present. Prefer only defining WITH_OAUTHBEARER_OIDC when it is enabled (omit the =0 definition), or switch the C code to #if WITH_OAUTHBEARER_OIDC so 0 behaves as disabled.
| else() | |
| list(APPEND rdkafka_compile_definitions WITH_OAUTHBEARER_OIDC=0) |
| if(WITH_SSL AND WITH_CURL) | ||
| list(APPEND rdkafka_compile_definitions WITH_OAUTHBEARER_OIDC=1) | ||
| else() | ||
| list(APPEND rdkafka_compile_definitions WITH_OAUTHBEARER_OIDC=0) | ||
| endif() |
There was a problem hiding this comment.
For maintainability and to avoid condition drift, consider basing the compile definition on the already-derived CMake variable (WITH_OAUTHBEARER_OIDC) rather than duplicating the WITH_SSL AND WITH_CURL condition here.
| if(WITH_SSL AND WITH_CURL) | |
| list(APPEND rdkafka_compile_definitions WITH_OAUTHBEARER_OIDC=1) | |
| else() | |
| list(APPEND rdkafka_compile_definitions WITH_OAUTHBEARER_OIDC=0) | |
| endif() | |
| if(WITH_OAUTHBEARER_OIDC) | |
| list(APPEND rdkafka_compile_definitions WITH_OAUTHBEARER_OIDC=1) | |
| else() | |
| list(APPEND rdkafka_compile_definitions WITH_OAUTHBEARER_OIDC=0) | |
| endif(WITH_OAUTHBEARER_OIDC) |
| list(APPEND rdkafka_compile_definitions WITH_OAUTHBEARER_OIDC=1) | ||
| else() | ||
| list(APPEND rdkafka_compile_definitions WITH_OAUTHBEARER_OIDC=0) | ||
| endif() |
There was a problem hiding this comment.
Minor consistency: the surrounding blocks in this same WITHOUT_WIN32_CONFIG section use endif(<condition>) (e.g., endif(WITH_CURL) / endif(WITH_PLUGINS)). Using endif(WITH_SSL AND WITH_CURL) here as well would keep the style consistent and reduce ambiguity when scanning the file.
| endif() | |
| endif(WITH_SSL AND WITH_CURL) |
Define the same conditions for WITH_OAUTHBEARER_OIDC in Cmake as are defined at the top level CMakelists.txt
Closes #5369