Modernize CMake #606
Merged
Modernize CMake #606
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mikke89
added
enhancement
Closed
Closed
Partial rewrite originally written by @hobyst in #446 and #551. Remaining code and changes written by @mikke89. These changes should generally be feature-complete with the previous CMake code, with some exceptions to legacy and certain Apple-specific behavior. Please see the changelog for breaking changes, in particular, several options, CMake targets, and file names have been changed. The following lists some noteworthy changes: - CMake presets now available. - Major changes to CI automation and tests. - Most tests on Appveyor are moved to Github actions. This includes a completely new packaging workflow on GitHub. - Packaging now uses Visual Studio 2019 instead of 2017. Keep Appveyor only for testing with Visual Studio 2017. - Use CMake scripts for some packaging tasks to avoid avoid duplicate workflow code. - `RmlUi::RmlUi` is now an include-all library target, while components like `RmlUi::Core` and `RmlUi::Debugger` can be chosen individually. - Add `contributing.md` guidelines for CMake. - Backends can now be changed without recompiling the whole library. - Set CMake `policy_max` to a recent version. This opts in to a number of CMake improvements for users that can take advantage of that. - Sample bitmapfont now available without FreeType. - Runtime outputs are now placed in the binary root directory. - Runtime dependencies can now be installed automatically, this includes dependencies from vcpkg. - Update .editorconfig and format all CMake files consistently. - Rename `harfbuzzshaping` sample to `harfbuzz`. - Fix harfbuzz compatibility for older versions. - Remove CMake warning on FreeType version. In the following, some working notes and lessons learned are written down (authored by @mikke89): - Avoid separate libraries for subparts of Core Originally, parts of core such as Elements, Layout and FontEngineDefault, were added as interface libraries. Defining these as interface libraries cause issues during export. I believe we would have to export them as separate targets, but that doesn't relly make sense from a client standpoint and causes new issues with exported source files and dependencies. It seems to be a lot less troublesome to use a single CMake library and set properties and attach sources to that directly. - Avoid interface libraries for plugins and Lua dependencies Using interface libraries to add plugins causes problems when installing targets. Even trying to split the interface libraries into private and public parts did not work suitably. An issue arises when compiling the project as static libraries. Consider when we are linking targets to rmlui_core. Even when they are added as private dependencies, CMake adds it as a transient link-time dependency. This even applies to interface dependencies, which causes problems for our private interface libraries representing the plugins. In particular, CMake wants us to export these libraries, as now they are referenced from rmlui_core, but we don't want to export them, as that was the whole point of making them private and would cause new issues. Instead, add the SVG and Lottie source files and imported dependencies directly to Core. This simplifies things quite a bit, and seems to work reliably. For similar reasons, use imported interface targets instead of a pure interface library for the custom Lua target. - Common options for CMake targets Using CMake presets to specify compiler flags, warnings in particular, is not a great experience. Presets require one to set all the flags at the same time (in a single variable). For example, we can't easily set just /WX in one profile and /MP in another. There are workarounds, but being more pragmatic, setting these flags in a cmake file is much more straightforward with proper conditionals. - On CMake presets I also experimented with more detailed, platform-dependent presets, but found that it is not really that useful. In particular, we want users themselves to use whichever compilers and generators they want to rather than to constrain the choices. We strive to be agnostict toward both generators and platforms. This way, we we can also give more common build instructions regardless of platform. Also tested with making presets for e.g. vcpkg and mingw toolchains. However, this doesn't really make sense in presets. First of all, the choice of such a toolchain is not mutually exclusive with the current presets, so we would have to replicate most of them for each toolchain we wanted to support. Further, the exact specifics of these toolcahins, like version, platform, install location, and so on, is very specific to each case. Thus, if we wanted to handle these it would create additional dimensions to the presets, and we'd end up with a huge list of similar presets we would have to mainatain and which users would have to choose from. Or alternatively, users would have to provide a lot of their setup in any case, thereby dismissing some of the initially considered usefulness. Instead, many tools already allow you to setup a toolchain first which can then be used with a given preset. Thus, making the choice of toolchains and cmake presets orthogonal makes a lot more sense. One unfortunate issue from a user-friendliness perspective is that the cmake gui doesn't allow you to select a preset without a specified generator. There's an issue for this here: https://gitlab.kitware.com/cmake/cmake/-/issues/23341 I figured accepting this for now is better than duplicating all our presets to specify different generators. But the situations is not ideal, hopefully it will be fixed soon. - Recommended compiler flags The compiler flags are enabled by default to ensure users get a good experience when building the library without fiddling with options. Especially on MSVC, the default compiler flags means really slow builds (no /MP), and we also risk exposing more warnings. Co-authored-by: Michael Ragazzon <michael.ragazzon@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The CMake code has been fully rewritten with modern practices. Special thanks to @hobyst who laid the groundwork for this change, with a solid foundation and great guidelines (#446).
I am making a pull request to gather any feedback before merging into master. In particular, because this is quite a significant breaking change in terms of build setups. However, I believe the benefits are quite clear, this will make it a lot easier to work with the library.
Please let me know if you have any feedback. It would be very helpful to see this tested in as many environments as possible before merging to master, so feel free to test it in your environment to identify any issues.
The changes should be close to feature-complete with what we currently have in master. I consider this todo-list, written by @hobyst, to be pretty much completed. One thing that we are missing is any apple-specific behavior such as frameworks and universal binaries.
I will have to squash most of these commits before merging. However I kept the individual commits around so that one can lookup the motivation behind some of the changes. For the most part I would recommend looking at the full changes together.
Changes
While modernizing our CMake code, it was clear that we also needed to change our naming conventions. This leads to quite significant breaking changes for building and linking, but the result should make the library a lot easier to work with and link with.
We now try to support all setups including:
It should be a lot easier now to simply point to the built library or sources, and have everything link correctly.
And naturally, we will continue to support package managers however we can, and that is still considered the default recommendation. However, for the most part we rely on contributors to keep supporting this. Please help out with your favorite package manager if you see missing versions, or room for improvements.
Large parts of the CI workflows have also been rewritten to accommodate these changes. Most of the Windows building and packaging procedures have been moved from Appveyor to GitHub Actions, which streamlines our testing and also helps speed up the CI builds.
New target names
We now export the following targets:
When including RmlUi as a subdirectory, the targets are constructed as aliases. When using pre-built or installed binaries, they are constructed using imported targets, which are available through the exported build targets.
The internal target names have also been changed, although they are typically only needed when exploring or developing the library. They are all lowercase and contain the prefix
rmlui_to avoid colliding with names in any parent projects. Some examples are:rmlui_core,rmlui_debugger,rmlui_sample_invaders,rmlui_tutorial_drag,rmlui_unit_tests, andrmlui_visual_tests.New library filenames
The library binaries have also changed names. These names would be suffixed by e.g.
.dllon Windows, and so on.rmluirmlui_debuggerrmlui_luaNew option names
We have a new set of CMake naming conventions for the library:
RMLUI_prefix to make all options specific to this project easily identifiable, and avoid colliding with any parent project variables.The following table lists all the new option names.
recommended compiler flags
flags -only enables RmlUi's
custom RTTI solution so that
the user can disable language
RTTI and exceptions
the options:
none,freetypedependencies on supported
platforms (e.g. DLLs)
the options:
lua,lua_as_cxx,luajitFor reference, the following options have not changed names, as these are standard options used by CMake.
CMake presets
We now have CMake presets:
samplesEnable samples but only those without extra dependencies.samples-allEnable all samples, also those with extra dependencies.standaloneBuild the library completely without any dependencies (even freetype). The idea is that it could be useful to take a quick look without dealing with dependencies, the only sample that works isbitmapfont.devEnable testing and samples.dev-allEnable testing and all samples, including those that require extra dependencies.One would still typically need to set options like
CMAKE_BUILD_TYPEwhen using these. I also experimented with more detailed, platform-dependent presets, but found that it is not really that useful, see the following commit message for reasoning: 27cf1df.