Conversation
"System deps" might be used not only when the corresponding packages are installed by the OS package manager, but also when compiling them separately. In either case, it would be useful to allow selecting which system/external dependencies shoud be used and which should be built within SFML more granularly. Add the corresponding options. Freetype and Harfbuzz are combined into one option due to their circular dependencies, same with FLAC and Vorbis, and MbedTLS and libssh. Allow using external SheenBidi. The default behaves just like before the change, and the global `SFML_USE_SYSTEM_DEPS` switch is still here. Signed-off-by: Alexander Lobakin <alobakin@mailbox.org>
|
I'm not in favor of adding so many new options. There likely will only ever be very few users that really need this kind of granularity. In those cases it's much simpler to patch SFML's CMake files, than to have SFML cover every potential configuration option. I'd even argue that if someone does need to instrument the dependency discovery as much, they likely would also need to patch other SFML CMake code. It's the route Debian and Vcpkg packages go. It's also the route we pick when building dependencies ourselves. As for SheenBidi, we can treat it as system package, as soon as it lands in popular distro package managers. |
| if(SFML_USE_SYSTEM_LIBSSH2_MBEDTLS) | ||
| find_package(MbedTLS REQUIRED) | ||
| find_package(Libssh2 REQUIRED) | ||
| else() |
There was a problem hiding this comment.
Would it make more sense to keep SFML_USE_SYSTEM_DEPS and instead do something like this?
sfml_set_option(SFML_USE_SYSTEM_MBEDTLS ${SFML_USE_SYSTEM_DEPS} BOOL "ON to use system MbedTLS, OFF to use the bundled ones.")
sfml_set_option(SFML_USE_SYSTEM_LIBSSH2 ${SFML_USE_SYSTEM_DEPS} BOOL "ON to use system Libssh2, OFF to use the bundled ones.")
include(FetchContent)
if(SFML_USE_SYSTEM_MBEDTLS)
find_package(MbedTLS REQUIRED)
else()
...
endif()
if(SFML_USE_SYSTEM_LIBSSH2)
find_package(Libssh2 REQUIRED)
else()
...
endif()This would allow a top-level CMake to do something like this
set(SFML_USE_SYSTEM_DEPS OFF)
set(SFML_USE_SYSTEM_MBEDTLS ON)
set(CMAKE_DISABLE_FIND_PACKAGE_MBEDTLS TRUE)
set(MBEDTLS_FOUND TRUE)
set(MBEDTLS_LIBRARY MbedTLS::mbedtls)
set(MBEDX509_LIBRARY MbedTLS::mbedx509)
set(MBEDCRYPTO_LIBRARY MbedTLS::mbedcrypto)
get_target_property(MBEDTLS_INCLUDE_DIR ${MBEDTLS_LIBRARY} INTERFACE_INCLUDE_DIRECTORIES)
add_subdirectory(SFML)
Description
"System deps" might be used not only when the corresponding packages are installed by the OS package manager, but also when compiling them separately. In either case, it would be useful to allow selecting which system/external dependencies shoud be used and which should be built within SFML more granularly.
Add the corresponding options. Freetype and Harfbuzz are combined into one option due to their circular dependencies, same with FLAC and Vorbis, and MbedTLS and libssh. Allow using external SheenBidi. The default behaves just like before the change, and the global
SFML_USE_SYSTEM_DEPSswitch is still here.Tasks
How to test this PR?
Compile test on different OSes and different combinations of the system dependencies options.