Add missing find_dependency calls.#203
Conversation
This was detected in reviewing microsoft/vcpkg#52901 by GPT 5.6 Sol. (Also the file VERSION in this repo still claims 5.4.0 rather than 5.5.0 but I have not tried to fix that in this PR as I'm not sure what you want your next version to be)
| endif() | ||
|
|
||
| if (@OMATH_ENABLE_LUA@) | ||
| find_dependency(Lua) |
| endif() | ||
|
|
||
| if (@OMATH_ENABLE_HOOKING@) | ||
| find_dependency(safetyhook CONFIG) |
There was a problem hiding this comment.
Code Review
This pull request updates the CMake configuration template to find the Lua, safetyhook, and OpenGL dependencies when their respective features are enabled. The reviewer points out that because these are private dependencies, they should only be required by consumers when building as a static library. It is recommended to wrap these dependency checks in a conditional block to avoid forcing shared library consumers to install them.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if (@OMATH_ENABLE_LUA@) | ||
| find_dependency(Lua) | ||
| endif() | ||
|
|
||
| if (@OMATH_ENABLE_HOOKING@) | ||
| find_dependency(safetyhook CONFIG) | ||
| if (NOT WIN32 AND (UNIX AND NOT APPLE)) | ||
| find_dependency(OpenGL) | ||
| endif() | ||
| endif() |
There was a problem hiding this comment.
Since Lua, safetyhook, and OpenGL are linked as PRIVATE dependencies in CMakeLists.txt, they are only required by consumers when omath is built as a static library. If omath is built as a shared library (OMATH_BUILD_AS_SHARED_LIBRARY is ON), these dependencies are already linked into the shared library binary, and forcing consumers to find them via find_dependency is unnecessary and can cause configuration failures if those packages are not installed on the consumer's system.
We should wrap these find_dependency calls in a check for NOT @OMATH_BUILD_AS_SHARED_LIBRARY@ to avoid forcing shared library consumers to install these private dependencies.
if (NOT @OMATH_BUILD_AS_SHARED_LIBRARY@)
if (@OMATH_ENABLE_LUA@)
find_dependency(Lua)
endif()
if (@OMATH_ENABLE_HOOKING@)
find_dependency(safetyhook CONFIG)
if (NOT WIN32 AND (UNIX AND NOT APPLE))
find_dependency(OpenGL)
endif()
endif()
endif()
There was a problem hiding this comment.
I think there can be header dependencies and similar still present even if PRIVATE?
|
thx for fix! |
This was detected in reviewing microsoft/vcpkg#52901 by GPT 5.6 Sol.
(Also the file VERSION in this repo still claims 5.4.0 rather than 5.5.0 but I have not tried to fix that in this PR as I'm not sure what you want your next version to be)