feat: add option to use system-installed jwt-cpp #4596
Conversation
Add ZM_USE_SYSTEM_JWT_CPP CMake option (default OFF) to allow using system-installed jwt-cpp instead of the vendored version. When enabled, CMake will search for jwt-cpp via find_package(jwt-cpp CONFIG). The vendored jwt-cpp in dep/ is only built when not using system version. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR adds a new CMake option ZM_USE_SYSTEM_JWT_CPP (default OFF) to allow using a system-installed jwt-cpp library instead of the vendored version in dep/jwt-cpp. When enabled, CMake searches for jwt-cpp via find_package(jwt-cpp CONFIG). The vendored jwt-cpp is only built when not using the system version.
Changes:
- Added
ZM_USE_SYSTEM_JWT_CPPCMake cache variable to control whether to use system or vendored jwt-cpp - Added logic to find and validate system jwt-cpp when the option is enabled
- Modified dep/CMakeLists.txt to conditionally skip building vendored jwt-cpp when system version is used
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| CMakeLists.txt | Adds ZM_USE_SYSTEM_JWT_CPP option and logic to find system jwt-cpp package when jwt_cpp backend is selected |
| dep/CMakeLists.txt | Conditionally excludes vendored jwt-cpp subdirectory based on HAVE_SYSTEM_JWT_CPP variable |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # jwt-cpp (system or vendored) | ||
| if (${ZM_JWT_BACKEND} STREQUAL "jwt_cpp") | ||
| if(ZM_USE_SYSTEM_JWT_CPP) | ||
| find_package(jwt-cpp CONFIG REQUIRED) | ||
| if(TARGET jwt-cpp::jwt-cpp) | ||
| set(HAVE_SYSTEM_JWT_CPP 1) | ||
| set(optlibsfound "${optlibsfound} jwt-cpp(system)") | ||
| message(STATUS "Using system-installed jwt-cpp") | ||
| else() | ||
| message(FATAL_ERROR "System jwt-cpp requested but jwt-cpp::jwt-cpp target not found") | ||
| endif() | ||
| else() | ||
| set(HAVE_SYSTEM_JWT_CPP 0) | ||
| set(optlibsfound "${optlibsfound} jwt-cpp(vendored)") | ||
| message(STATUS "Using vendored jwt-cpp") | ||
| endif() | ||
| endif() |
There was a problem hiding this comment.
The variable HAVE_SYSTEM_JWT_CPP is only set when ZM_JWT_BACKEND equals "jwt_cpp", but it's used unconditionally in dep/CMakeLists.txt. When ZM_JWT_BACKEND is set to "libjwt", HAVE_SYSTEM_JWT_CPP will be undefined, which could cause CMake to either error or treat it as false.
To fix this, initialize HAVE_SYSTEM_JWT_CPP to 0 before the jwt-cpp backend check. Add this line before line 404:
set(HAVE_SYSTEM_JWT_CPP 0)
This ensures the variable is always defined regardless of which JWT backend is selected.
| # Option to use system-installed jwt-cpp instead of vendored version | ||
| set(ZM_USE_SYSTEM_JWT_CPP "OFF" CACHE BOOL | ||
| "Set to ON to use system-installed jwt-cpp instead of the vendored version. default: OFF") | ||
|
|
There was a problem hiding this comment.
The ZM_USE_SYSTEM_JWT_CPP option is only meaningful when ZM_JWT_BACKEND is set to "jwt_cpp". If a user sets ZM_USE_SYSTEM_JWT_CPP to ON but uses the libjwt backend, the option is silently ignored, which could be confusing.
Consider adding a warning message after line 252 to inform users about this:
if(ZM_USE_SYSTEM_JWT_CPP AND NOT (ZM_JWT_BACKEND STREQUAL "jwt_cpp"))
message(WARNING "ZM_USE_SYSTEM_JWT_CPP is set but will be ignored because ZM_JWT_BACKEND is not jwt_cpp")
endif()
This helps users understand the relationship between these options.
| if(ZM_USE_SYSTEM_JWT_CPP AND NOT (ZM_JWT_BACKEND STREQUAL "jwt_cpp")) | |
| message(WARNING "ZM_USE_SYSTEM_JWT_CPP is set but will be ignored because ZM_JWT_BACKEND is not jwt_cpp") | |
| endif() |
1 participant
Add ZM_USE_SYSTEM_JWT_CPP CMake option (default OFF) to allow using system-installed jwt-cpp instead of the vendored version. When enabled, CMake will search for jwt-cpp via find_package(jwt-cpp CONFIG).
The vendored jwt-cpp in dep/ is only built when not using system version.
Maybe fixes #4950