Modernize CMake

[mikke89]

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 by target_link_libraries(main PRIVATE RmlCore RmlDebugger). Or RmlUi::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.

[andreasschultes]

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.

[mikke89]

Thank you, those are valuable considerations! They all sound reasonable to me.

[SamVanheer]

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 using ON and OFF. You may want to use OFF 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_ or RMLUI_.
• 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 using target_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. The add_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 use if (NOT TARGET rlottie::rlottie) to test for success instead. The inclusion of rlottie_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.

[SamVanheer]

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.

[mikke89]

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.

[cathaysia]

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]

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

[hobyst]

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.

[hobyst]

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 using add_directory_options(). Setting compiler flags with CMAKE_<LANG>_FLAGS is considered to be a bad practice as those are meant to be set by the user compiling the project and the COMPILE_FLAGS target property has been deprecated in favor of COMPILE_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 between FetchContent and find_package() looks promising but unfortunately is only available on CMake 3.24+.

[cathaysia]

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.

[cathaysia]

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)

[hobyst]

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)

[hobyst]

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-source Dependencies folder, they can set CMAKE_PREFIX_PATH (CMake environment variable) in their working environment before configuring the RmlUi CMake project and find_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 the Dependencies folder to it and find_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.

[hobyst]

@mikke89 I've successfully re-written the CMake project up to some extent using modern CMake and is currently able to build the all-rounder "demo" sample as well as the "lottie" sample. Could you create a cmake branch on the repo for me to PR to it? That way more people will be able to contribute to it without completely breaking the master branch.

[mikke89]

Very cool, a cmake branch sounds like a good idea, I just pushed it now.

[aquawicket]

As a side note...
CMakeLists.txt:310 target_compile_features(${NAME} PUBLIC cxx_std_14)
Fails if "cxx_std_14" is not listed in CMAKE_CXX_COMPILE_FEATURES

I encounter this issue while targeting earlier Android NDK 23.1.7779620 due to c++14 being flagged as -std=c++1z instead.
Maybe a check against CMAKE_CXX_COMPILE_FEATURES would work as a pitfall.

It may be beneficial to target against platforms that don't yet introduce the c++14 flag, yet can be obtained by c++1z.
Especially if you're looking for backward compatibility.

P.S. Besides that.. The newest CMakeLists.txt is working for me across all platforms amazingly.

[hobyst]

@aquawicket That looks as if CMake wasn't even detecting the Android compiler. Usually in between the brackets CMake would show the friendly display name of the detected compiler. On top of that, the message itself means CMake doesn't have proper integration with the Android compiler. Are you using a standalone version of CMake or the CMake that ships as a part of the Android NDK? If you haven't already, you should try again using the CMake that ships with the NDK.

[aquawicket]

Standard CMake..

against
android\23.1.7779620\build\cmake\android.toolchain.cmake

I get around this by commenting out target_compile_features(${NAME} PUBLIC cxx_std_14)
and using cmake compiler flags to ask for -std=c++1z instead.

then everything works

[mikke89]

If the compiler isn't listed as supporting C++14, then we don't "officially" support it, so some workarounds should be expected. I'm hoping to move towards C++17 eventually, so my suggestion is that we don't specifically handle this case.

P.S. Besides that.. The newest CMakeLists.txt is working for me across all platforms amazingly.

Just to clarify, did you mean the one in the pull request, or the one on master? In any case, that is good to hear!

[aquawicket]

@mikke89, I'm using the master branch. :) Everything is working for me with my workaround, so false alarm. Thanks.

[mikke89]

Hey, just letting everyone who contributed here know that the pull request is now up: #606 😈