-
Notifications
You must be signed in to change notification settings - Fork 91
Description
If you aren't aware, a bit later than modern C++ happened, modern CMake happened. Just like C++ it also kept backwards compatibilty but there are much better ways to do things now. Specifically:
Lines 203 to 209 in a02d5a8
| IF(SDL2PP_STATIC) | |
| ADD_LIBRARY(SDL2pp STATIC ${LIBRARY_SOURCES} ${LIBRARY_HEADERS}) | |
| ELSE(SDL2PP_STATIC) | |
| ADD_LIBRARY(SDL2pp SHARED ${LIBRARY_SOURCES} ${LIBRARY_HEADERS}) | |
| TARGET_LINK_LIBRARIES(SDL2pp ${SDL2_ALL_LIBRARIES}) | |
| SET_TARGET_PROPERTIES(SDL2pp PROPERTIES VERSION 8.3.0 SOVERSION 8) | |
| ENDIF(SDL2PP_STATIC) |
This code currently uses old directory-based flags, which do not work well in modern CMake. As of now, any library should be used like this:
target_link_libraries(my_project PUBLIC SDL2pp)...and nothing more. In modern CMake this single command (with new keywords: PRIVATE (build requirement) /INTERFACE (usage requirement) / PUBLIC (both)) will forward all transitive requirements of the dependency: include paths, compiler options, linker flags and so on. The requirement for this is that the target is also defined this way:
add_library(SDL2pp ...)
target_sources(SDL2pp PRIVATE ...) # sources
target_sources(SDL2pp PUBLIC ...) # headers
target_include_directories(SDL2pp PUBLIC ...)
target_compile_options(SDL2pp PRIVATE ...)
target_*(SDL2pp PUBLIC/PRIVATE ...) # and so on...In other words, modern CMake projects do not want variables - they want targets they can link to. This is especially true for libraries that may want to use subrepository approach, which can not work in the old system.
I could make a PR for this but note: this will raise minimum required CMake version.