RmlUi
RmlUi copied to clipboard
Modernize CMake
Our CMakeLists.txt is really a patchwork on top of something written using legacy CMake conventions. It could really need a rewrite or at least a larger review. I've seen several users writing their own CMakelists to consume the library, which is understandable but not an ideal situation.
Some ways to consume the library that should ideally be supported without too much trouble for users:
- FetchContent_Declare (and related)
- add_subdirectory()
- Exporting and installing.
-
find_package(RmlUi CONFIG REQUIRED)
followed bytarget_link_libraries(main PRIVATE RmlCore RmlDebugger)
. OrRmlUi::Core RmlUi::Debugger
? Both from build directory and install directory.
Questions:
- What is the current state of CMake on iOS/Apple targets? Currently we have a lot of logic for building a framework / universal libraries / Apple targets. Are there more elegant solutions today?
Some issue that have been reported by users:
- Issues finding dependencies (Freetype in particular, eg. when using CPM.cmake as a package manager)
- Issues linking to RmlUi, eg.
target_link_libraries (... RmlCore)
not declaring all dependencies. - Consumers required to write export rules, eg. error message
export called with target "RmlCore" which requires target that is not in any export set
.
What do you think about all of this? Please do contribute with your own use cases, requirements, and issues. Generally, we would like to keep our current CMake options, but we can break some of them if it leads to improvements. I'm all for bumping the minimum CMake version to something more recent.
I am not a CMake expert myself, so I'd be very happy if a volunteer who is more comfortable with the task could lend us a helping hand.
I write my ideas down and some informations:
- I include the CMakeLists.txt file with add_subdirectory and comment out the Config.cmake part.
- I would use add_library(Rml::Core ALIAS RmlCore) for creating an alias name, many other projects do this on this way.
- I would use target_include_directories(RmlCore INTERFACE Include) for setting up the include path of an added library
- I would use target_link_libraries(RmlDebugger [INTERFACE|PUBLIC|PRIVATE] RmlCore) adding dependencies,
- Minimum CMake Version should be 3.10, because the current Android NDK Version use this. I think that will changes soon, to a higher version. Hopefully! target_sources is a nice new feature.
- My build run currently on MacOS, but I didn't tested it on iOS.
I hope this helps a bit for the discussion. I am also not a CMake expert.
Thank you, those are valuable considerations! They all sound reasonable to me.
I took a look at the main CMakeLists.txt, here's my feedback.
First, note that the minimum CMake version is 3.1.
There are a few CMake policies that are set to NEW. If the minimum CMake version is newer than the version the policy was introduced in the explicit setting can be removed.
- CMP0015 was added in 2.8.0
- CMP0042 was added in 3.0
- CMP0072 was added in 3.11
- CMP0074 was added in 3.12
Notes:
- Various
end*
commands still use the old syntax that required the expression in the opening command to be repeated. This requirement was fully removed in CMake 2.6.0 and can be removed. - RMLUI_VERSION_RELEASE is set to
false
. Although CMake does treat this as boolean false CMake documents boolean as usingON
andOFF
. You may want to useOFF
here since it allows the variable to be made into a cache variable without having to change the value. - The three
Environment tests
includes are not actually used for anything. Including them prints whether the features are available and set a boolean variable, but the variables are never checked. The features involved are all standard C++ and since the library requires C++14 they must all exist, so these can be removed. - CMAKE_BUILD_TYPE requires one of a set of options, so they should be set as the list of available options using set_property. This only matters to GUI users since the value can still be set to anything.
- Some commands have spaces around the expression contained within parentheses. Using consistent formatting makes for a better reading experience. It's a minor issue though.
- Some variable names use a
NO_
prefix which could be confusing and lead to double negatives. - When included directly in another CMake project all of the cache variables will be visible to that other project, so it may be a good idea to prefix all cache variables with
RML_
orRMLUI_
. - CMake version checks can be removed if the minimum version is raised to that version or greater.
- Prefer using
target_sources
to add header and source files to a target. It was added in CMake 3.1 so it's already available. Starting with 3.11 you can create targets without sources and populate the source list later on. This allows you to rework target creation from using variables to usingtarget_sources
. - Avoid using non-target versions of functions like
include_directories
. If you need to share properties between multiple targets you can use interface libraries to share them. Theadd_common_target_options
can be partially converted into an interface target as well. - The
rlottie
library seems to export a target that can be used instead of checking the old style CMake variables. You can useif (NOT TARGET rlottie::rlottie)
to test for success instead. The inclusion ofrlottie_INCLUDE_DIR
is probably redundant. - The special case logic for Mac framework builds can probably be simplified by relying on PUBLIC include directories, link libraries, etc for the RmlCore library.
- Avoid constructing target and variable names dynamically, this makes finding references harder.
- The samples install commands seem overly verbose, maybe consider restructuring the source files to allow installing everything in a single command?
You should figure out what the minimum CMake version you want to use is so you can decide what to do about these things.
I looked up which CMake versions are used by various platforms:
- Windows can use any version, CMake is installed by the user so you can assume a fairly recent version (3.16 or newer i'd say) but that all depends on your users. C++14 requirements mean you need a fairly recent version of Visual Studio so you'll need a CMake version that can generate projects for that version (unless you rely on VS to upgrade the project). VS 2017 is the first version to fully support C++14, but 2015 may also be able to compile this library. CMake added support for 2015 in 3.1, and experimental support for 2017 in 3.7. It looks like 2017 support already worked properly because the next mention of it is in 3.10's release notes about supporting ARM64.
- Linux provides CMake through package managers. This website tracks package versions across a number of them: https://repology.org/project/cmake/versions Looking at Ubuntu, the oldest LTS still in support (18.04) uses 3.10.2, the next LTS (20.04) uses 3.16.3
- Mac OS users can use homebrew to install CMake. The website lists 3.24.1: https://formulae.brew.sh/formula/cmake#default I suppose iOS does the same
- Android has a forked version 3.10.2, but it is possible to use a custom installation. See https://developer.android.com/studio/projects/install-ndk
- Emscripten relies on whatever platform you're compiling on
So you can safely switch to 3.10.2 and still support every platform listed, unless there are platforms supported that i'm not aware of.
If you're ok with requiring users to install a newer version then you can probably pick any version, but you should probably check with users to see what version they're limited to.
Thank you for the detailed feedback and investigation! This is very helpful.
So I think based on this I would choose 3.10.2 as minimum. I don't want compilation to require any more steps than necessary, so avoiding custom CMake installs would be preferable on systems that already provide an older version.
I guess then we're just missing out on some nice, modern CMake techniques. Maybe in a year or two we should be able to go for 3.16, and perhaps also move towards C++17.
All of your notes sound reasonable to me, I think this is a very good outline of what needs to be done. I do agree with renaming the variable names, they're not consistent right now. It would be a pretty big breaking change so we would have to merge that during a new major version.
Hi friends. What I'm want to say is that if you want to partition your projects in Modern Cmake, don't use static libraries to partition your projects. This bring problems with dynamic library symbol export. A more reasonable approach is to use the OBJECT library to divide submodules. However, this requires upgrading the CMake version.
Another problem is dependency lookup related. I think dependent lookups should follow:
if find_package(freetype) then
link_target(freetype)
include_target(freetype)
else if try_add_subdirectory(freetype) then
-- ....
else fetch_content(freetype) then
-- ...
end
Hi friends. What I'm want to say is that if you want to partition your projects in Modern Cmake, don't use static libraries to partition your projects. This bring problems with dynamic library symbol export. A more reasonable approach is to use the OBJECT library to divide submodules. However, this requires upgrading the CMake version.
@cathaysia What issues does it cause? I've heard about issues that occur when compiling a library as a static library and such library uses the MSVC-specific __declspec(dllimport)
and __declspec(dllexport)
preprocessor macros, but these are more about not properly guarding them to make the libary compatible with both static and dynamic linking than a CMake issue.
To modernize the CMake script entirely, I would rather start from scratch with a new script that would follow Modern CMake as much as possible, then port functionality (options, compilation flags...) over the new one as soon as we are able to get it compiling for desktop platforms (maybe create a cmake
branch so people can contribute to it without breaking master
). This way we could re-make the script taking advantage of modern features of CMake without having to care about how the old script was structured.
As time went on, many CMake scripts of open-source many projects that were made before CMake 3.0 accumulated a lot of boilerplate code that might not be needed for the newer CMake versions, and that could be the case of RmlUi as well.
My advice would be to:
-
Avoid setting compile flags as much as possible: These do not only constitute a large part of the code of older CMake scripts but in many cases are cross-compilation related, for which toolchain files should be used. If there's an actual need to set them, set them either on a target-by-target basis using
target_compile_options()
or directory-wise basis usingadd_directory_options()
. Setting compiler flags withCMAKE_<LANG>_FLAGS
is considered to be a bad practice as those are meant to be set by the user compiling the project and theCOMPILE_FLAGS
target property has been deprecated in favor ofCOMPILE_OPTIONS
- Do not set stuff in the script that is meant to be set by the developer compiling RmlUi: If you want to share or automatically set certain options like compiler, architecture or toolchain file to use or other variables at configure time, CMake presets were made with that purpose in mind and can be implemented without breaking compatibility with older CMake versions. However, CMake 3.19+ will be needed to use them. For older versions, all these options will need to be manually set by the consumer of the library as that's the way they are meant to be used.
-
Investigate a more flexible approach to how dependencies are required by the project: While the technique @cathaysia proposed would be the ideal approach, that isn't possible on CMake due to how
find_package()
works internally. Once a package hasn't been detected, no further attempts to locate it will happen unless the CMake cache is completely deleted and re-generated from scratch. An actual solution should be investigated based on the information about consuming dependencies on CMake projects. The integration betweenFetchContent
andfind_package()
looks promising but unfortunately is only available on CMake 3.24+.
dllexport is not necessary for dynamic lib, cmake has this to export all symbols. What I say is this case:
add_library(rmlui_lua STATIC rmlui_lua.cpp)
add_library(rmlui SHARED core.cpp)
target_link_library(rmlui PRIVATE rmlui_lua)
in this case, I split rmlui to two libraries, a static, a dynamic, then it should produce a rmlui.so. In this case, rmlui_lua is not a dependency of rmlui, and it is written just to divide sub-projects. In other words, I just simply hope that rml_lua is merged into the dynamic library rml
Then we use rmlui.so like this:
add_executable(my_prog main.cpp)
target_link_library(my_prog PRIVATE rmlui.so)
#include <rmlui_lua.h>
//....
then you will find all symbols of rml_lua are missing, this is because the compiler will only export the symbols used by rmlui in rmlui_lua.
emm, maybe you just giva an eyes on https://stackoverflow.com/questions/11429055/cmake-how-create-a-single-shared-library-from-all-static-libraries-of-subprojec , it should has a more clear content.
Investigate a more flexible approach to how dependencies are required by the project: While the technique @cathaysia proposed would be the ideal approach, that isn't possible on CMake due to how find_package() works internally. Once a package hasn't been detected, no further attempts to locate it will happen unless the CMake cache is completely deleted and re-generated from scratch. An actual solution should be investigated based on the information about consuming dependencies on CMake projects. The integration between FetchContent and find_package() looks promising but unfortunately is only available on CMake 3.24+.
may we can use this cmake script:
function(add_deps target_name pkg_name) # such as params ares freetype::freetype and freetype
if(TARGET ${target_name}) # if already has freetype::freetype
set(${target_name}_LIBRARY ${target_name})
return
endif()
if(EXISTS ${PROJECT_DIR}/Deps/${pkg_name}/CMakeLists.txt) # check rmlui/Deps/freetype dir
add_subdirectory(${PROJECT_DIR}/Deps/${pkg_name})
set(${target_name}_LIBRARY ${target_name})
return
endif()
# we has no freetype::freetype and freetype
FetchContent() # .......
endfunction()
then we can add deps by:
add_deps(freetype::freetype freetype)
target_link_library(rmlui PRIVATE freetype_LIBRARY)
dllexport is not necessary for dynamic lib, cmake has this to export all symbols. What I say is this case:
add_library(rmlui_lua STATIC rmlui_lua.cpp) add_library(rmlui SHARED core.cpp) target_link_library(rmlui PRIVATE rmlui_lua)
in this case, I split rmlui to two libraries, a static, a dynamic, then it should produce a rmlui.so. In this case, rmlui_lua is not a dependency of rmlui, and it is written just to divide sub-projects. In other words, I just simply hope that rml_lua is merged into the dynamic library rml
Then we use rmlui.so like this:
add_executable(my_prog main.cpp) target_link_library(my_prog PRIVATE rmlui.so)
#include <rmlui_lua.h> //....
then you will find all symbols of rml_lua are missing, this is because the compiler will only export the symbols used by rmlui in rmlui_lua.
emm, maybe you just giva an eyes on https://stackoverflow.com/questions/11429055/cmake-how-create-a-single-shared-library-from-all-static-libraries-of-subprojec , it should has a more clear content.
In that case, if the RmlUi Lua header is meant to be used by the library consumer as well, that means that consumers can also call to code defined in rmlui_lua
and therefore the linkage is wrongly set up. rmlui_lua
should be linked publicly so that projects consuming RmlUi can also call code found in rmlui_lua
.
One way to do so would be to make rmlui_lua
static and make the link public:
add_library(rmlui_lua STATIC rmlui_lua.cpp)
add_library(rmlui SHARED core.cpp)
target_link_library(rmlui PUBLIC rmlui_lua)
And the other way, which should be the default one since rmlui_lua
is meant to be used by several foreign targets at the same time, is to make rmlui_lua
a shared library. This way it would be much easier for consumers to tell when an RmlUi build includes the Lua bindings:
add_library(rmlui_lua SHARED rmlui_lua.cpp)
add_library(rmlui SHARED core.cpp)
target_link_library(rmlui PUBLIC rmlui_lua)
may we can use this cmake script:
function(add_deps target_name pkg_name) # such as params ares freetype::freetype and freetype if(TARGET ${target_name}) # if already has freetype::freetype set(${target_name}_LIBRARY ${target_name}) return endif() if(EXISTS ${PROJECT_DIR}/Deps/${pkg_name}/CMakeLists.txt) # check rmlui/Deps/freetype dir add_subdirectory(${PROJECT_DIR}/Deps/${pkg_name}) set(${target_name}_LIBRARY ${target_name}) return endif() # we has no freetype::freetype and freetype FetchContent() # ....... endfunction()
then we can add deps by:
add_deps(freetype::freetype freetype) target_link_library(rmlui PRIVATE freetype_LIBRARY)
That approach could actually work!
I've been investigating a bit and found that using CMAKE_PREFIX_PATH
(CMake variable) and CMAKE_PREFIX_PATH
(CMake environment variable) could be used by both the project and consumers to set up lookup paths for every find_...()
function. With this, all it should take for dependencies to be detected is:
-
Consumers and/or developers: If they already have some of their dependencies installed in some place other than the
CMAKE_SYSTEM_PREFIX_PATH
or the in-sourceDependencies
folder, they can setCMAKE_PREFIX_PATH
(CMake environment variable) in their working environment before configuring the RmlUi CMake project andfind_package()
should work just fine. Since paths for found packages are saved as cached variables, this should only be required when configuring RmlUi from scratch. -
The RmlUi CMake project: Since
CMAKE_PREFIX_PATH
(CMake variable) doesn't seem to be cached, the project can just append the path to theDependencies
folder to it andfind_package()
will have in mind both the user-specified path(s) and the project-specified path(s). Because the project will be appending its preferred paths, the user-specified paths will take priority. For example, for Freetype:# Because CMAKE_PREFIX_PATH is a semicolon-separated list, list() can be used to easily add elements to it # https://cmake.org/cmake/help/latest/command/list.html list(APPEND CMAKE_PREFIX_PATH "${CMAKE_CURRENT_SOURCE_DIR}/Dependencies") find_package(Freetype) add_library(rmlui SHARED core.cpp) target_link_library(rmlui PRIVATE Freetype::Freetype)
Much simpler approach that should work regardless of the conditions and can also be integrated with FetchContent(...)
with a simple check to <package_name>_FOUND
. However, special attention must be taken with target names for the different packages that use a find module as well as all the find modules provided by RmlUi itself, some of which have an official CMake counterpart and for which may be better to use the official modules.