-
Notifications
You must be signed in to change notification settings - Fork 596
Backport changes to increase the CMake minimum version to 3.8 to support 2.13 support branch #10625
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
It's on by default since CMake version `3.0`
CMake 3.4 introduced a new policy [^1] which prevents from automatically adding the compiler flags needed for exporting the symbols of the executables and libraries without the `ENABLE_EXPORTS` property. So, by defining this variable, CMake will restore the previous behaviour by automatically adding the `ENABLE_EXPORTS` properties to all targets. [1]: https://cmake.org/cmake/help/latest/policy/CMP0065.html
Al2Klimov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CMake version `< 3.5` is no longer supported, so the new CMake minimum policy version is set to `3.8` to support C++17 unconditionally. After checking all the policies that might affect Icinga 2 in any way, CMake `3.17` is used as a max supported CMake policy. Anything above that may work but we didn't explicitly verify the policies introduced with CMake 3.18 and later and may or may not affect Icinga 2.
CMake 3.15 introduced the `MSVC_RUNTIME_LIBRARY` property as a way to specify which MSVC runtime library is used and how it is linked. Also with that release, policy CMP0091 was introduced where the new behavior no longer adds the corresponding compile flags to the `CMAKE_<LANG>_FLAGS_<CONFIG>` variables. The new policy was enabled by 7f164bd, resulting in the MSI no longer working on previous Windows Server versions (2019 for example) as the CMake configuration replaced those compile flags (lines 3-9 in the same file) which no longer works when they are no longer part of the string. This commit fixes this regression by setting the `MSVC_RUNTIME_LIBRARY` property on the icinga-installer target. I've also added a comment stating my understanding of why this is necessary. Unfortunately, I couldn't find an explanation of why replacing the /MD with the /MT flag was done originally in the project history, so it's a bit of guesswork. https://cmake.org/cmake/help/latest/policy/CMP0091.html https://cmake.org/cmake/help/latest/prop_tgt/MSVC_RUNTIME_LIBRARY.html
b060f1d to
c59cd55
Compare
julianbrost
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also survived the compatibility test on the ancient Windows Server 2012 R2, so no apparent compatibility issues from the change here as well.
|
#10663 (starting to add the missing checks now required by the branch rules) has failed checks because of CMake 4 compatibility errors, which is just what this PR fixes. So without combining all required fixes into a single PR, you have to start merging somewhere bypassing the rules, so I'll go ahead and do this here. At least this PR only has missing checks not failed ones. |
|
Oops, just wanted to click on the link but GitHub moved the content unexpectedly. Do Fedora ship boost < 1.87? If not you'll have to backport the boost 1.87 support PR as well (you will actually have to if you're going to backport Ubuntu 25.10 as well since it's also required by the branch protection rules as well). |
|
I haven't checked that yet, checking that is part of #10663 (and other's that will follow). But I started creating that PR because I had preferred merging this PR without having to bypass the branch rules, but as soon as that showed failed jobs due to CMake 4, it was clear that this won't work. |
These are the changes from #10402 backported to the 2.13 support branch as requested by @julianbrost.
This adds compatibility with CMake 4, required to continue building this branch on newer systems.
backport of #10402
backport of #10478