Please see cmake_minimum_required() for details.
This PR does not change the miminum required version of CMake; it just changes the policy version, which would otherwise be identical to the minimum required version (3.8).
A policy version of less than 3.10 causes a warning when using CMake 3.31. The policy version should always be set to the latest version that a project has been tested with.
The policy version should always be set to the latest version that a project has been tested with.
Doing so will make the build system’s behaviour dependent on the CMake version.
Doing so will make the build system’s behaviour dependent on the CMake version.
IIUC, that does seem like a possible downside to this change. Before 300278daa77e861564ae2f342dd995dfa8f8d171 with cmake_minimum_required(VERSION 3.8) specified everybody who is using this build will see cmake 3.8 behaviors no matter what version of cmake they are using. But after 300278daa77e861564ae2f342dd995dfa8f8d171 with cmake_minimum_required(VERSION 3.8...3.31) people who are using cmake 3.8 will get cmake 3.8 behaviors, people who are using cmake 3.12 will get 3.12 behaviors, 3.20 will see 3.20 behaviors, etc. Cmake will stop trying to be compatible with version 3.8 and only try to be compatible with version 3.31, so people using cmake versions after 3.31 will still see 3.31 behaviors.
On the other hand, it does seem better to use 3.31 behaviors instead of 3.8 behaviors as long as it doesn’t cause bugs. Because if it does cause bugs we will need to fix them eventually anyway and it would be better to fix them sooner rather than later. Also in the worst case if any bugs are difficult to fix properly or difficult to fix while maintaining compatibility we can always set cmake_policy SET to restore specific old behaviors that we want.
So overall this change seems good, even if it might expose some problems in the short run.
I guess another possible downside of this change is it increases the chances that we accidentally break compatibility with old versions of cmake without knowing about it.
For example right now maybe we can be reasonably assumed that we are not depending on cmake 3.20 features because our cmake policy version is 3.8 so those features aren’t enabled for us. But if we increase our policy version to 3.31 then new features may become available and we may rely on them without knowing about it. I’m not sure if cmake can warn us when we are relying on a new feature without checking for it and need to increase our minimum version.
Whenever a new version of CMake comes out, you should test your project with that new version without changing the policy version.
For all new policies, cmake will execute two code paths: the one with the old behavior and the one with the new behavior. If the results mismatch, cmake will print a warning that your project is affected by the policy and then will use the old result. You can silence the warning by setting the policy to OLD, but this should be temporarily only, because the old behavior is deprecated by definition.
Once you fixed a policy warning by avoiding ambiguous cmake code, you should set the policy to NEW. Once you fixed all policy warnings, you may increase the policy version. You can then be sure that behaviour does not dependent on the CMake version.
Note that cmake_minimum_required(VERSION 3.8) does not prevent using features that are introduced later. The risk of accidental breakage is real, but it is not increased with this change. The only way to actually guarantee compatibility with version 3.8 is to use that version in CI.
As a part of the Bitcoin Core project, libmultiprocess should ensure a stable build environment.
Testing all available policies introduces unneeded burden for developers.
Testing all available policies introduces unneeded burden for developers.
You do that multiple times a day. Every time you run cmake, all available policies are tested. If no policy warnings are printed, it means that you are not affected by the breaking changes those policies are introduced for. You should trust cmake and increase the policy version in this case.
You don’t turn cmake into a hermetic buildsystem by keeping policies unset. You should use them the way it is intended by their developers.
So if my understanding in the previous comment is correct, I presume a possible alternative fixes would be to simply change 3.8 into 3.10 to avoid the cmake warning. As part of reviewing of that fix, one would check all policy changes between 3.8 and 3.10 and confirm that they do not affect the build, or are already accounted for. Given that a build with cmake 3.10 (or any version up to 3.30) did not issue any warnings, there are likely no policy changes affecting the project and the change should be fine. Also, there should be no need to support 3.8 at all, because the project here requires a C++20 compiler. Any system that comes with a C++20 compiler should also support running a more recent cmake than 3.8.
Going further, if someone tried to actually use cmake 3.8 (up to cmake 3.11), it should already fail because the value 20 isn’t defined.
So I submitted this fix in https://github.com/chaincodelabs/libmultiprocess/pull/164
For now I merged #164 which raises minimum version to 3.12, since that seems like the most obvious fix for the warning.
I am still interested in setting in specifying a max version to use a higher default policy version though. It seems to me like there are different risks to whichever policy version you chose (though the risks are minor and theoretical in all cases). If you choose an old policy version, you are opting into old deprecated behaviors and relying on cmake’s ability to emulate old versions of itself. If you set a new policy version, you put developers using older cmake versions at a disadvantage because you aren’t testing the old policies they might be using.
IMO the best thing to do would be to disable application of old cmake policies and use non-deprecrated policies in our release build, which uses cmake 3.24.2 (it is also 3.24.2 according to guix time-machine --commit=53396a22afc04536ddf75d8f82ad2eafa5082725 -- shell cmake-minimal -- cmake --version). So IMO it would be good to set 3.24 as max version, so developers are free to use older or new versions of cmake and their builds will always match the release build as closely as possible.
According to the cmake documentation, the <min>...<max> syntax means that the project is expected to work with both the OLD and NEW behaviors of policies introduced between those versions.
For earlier versions than <min>, only the NEW behavior is supported and for later versions than <max> only the OLD behavior is supported.
Once a new version of cmake introduces a new deprecation, it is recommended to update affected code as soon as possible. It does not matter what version is used to build releases. Once we know that a certain feature of cmake is going to be removed in the future, we should stop using that particular feature, not continue using it until official releases are built with a version that no longer has it.
Sure, CI should be checking the minimum version, but it can’t cover all possible user systems.
It does not have to cover all possible user systems; it has to cover all possible policies. This is archived by testing both <min> (where all policies in the <min>...<max> range are OLD) and <max> (where all policies in that range are NEW). With only two CI builds, you are covering both OLD and NEW for all possible policies. That is no burden.
Sure, CI should be checking the minimum version, but it can’t cover all possible user systems.
It does not have to cover all possible user systems; it has to cover all possible policies. This is archived by testing both
<min>(where all policies in the<min>...<max>range areOLD) and<max>(where all policies in that range areNEW). With only two CI builds, you are covering bothOLDandNEWfor all possible policies. That is no burden.
According to the cmake documentation, there are policies that only take effect on some systems, so I don’t think testing on a single system the min and max cmake version is sufficient.
For example, https://cmake.org/cmake/help/latest/policy/CMP0141.html says: “The policy setting takes effect […] whose compiler targets the MSVC ABI.”
IMO the best thing to do would be to disable application of old cmake policies
Isn’t this what is happening in current master already? (Pin the cmake version and address any policy warnings by dropping support for (deprecated or) old cmake policies)
release build
If this project is tied to Bitcoin Core, it could make sense to just take over the min version from there: https://github.com/bitcoin/bitcoin/blob/e486597f9a57903600656fb5106858941885852f/CMakeLists.txt#L10
Since this project has no CI and I presume no one is testing on ancient versions of Cmake anyway, so the only testing is on recent versions of cmake, or the versions of cmake used in the CI of bitcoin core.
re: #163 (comment)
According to the cmake documentation, there are policies that only take effect on some systems, so I don’t think testing on a single system the min and max cmake version is sufficient.
This is true, but I don’t see it as a goal to have test coverage for esoteric options on all systems. I think best strategy is just to prefer modern defaults over deprecated defaults, and explicitly document whatever defaults we tested and have most confidence in.
IMO the best thing to do would be to disable application of old cmake policies
Isn’t this what is happening in current master already? (Pin the cmake version and address any policy warnings by dropping support for (deprecated or) old cmake policies)
No, current master is using policies from 3.12 which enables policies up to CMP0075. The latest cmake release is cmake 3.31 which enables policies up to CMP0180. So we are opting into 180-75=105 outdated policies which could make builds slower and less reliable and worse for users, and which are deprecated and less well tested upstream.
If this project is tied to Bitcoin Core, it could make sense to just take over the min version from there: https://github.com/bitcoin/bitcoin/blob/e486597f9a57903600656fb5106858941885852f/CMakeLists.txt#L10
This doesn’t make a lot of sense to me. IMO, purpose of the min version should just be to document the minimum version that should work, and not to preemptively break compatibility with older versions of cmake when there is no reason to believe they have any problems.
i do think it could be reasonable to set max version here to the policy version used by bitcoin core to avoid opting into newer cmake policies than are used by bitcoin core releases. This could be 3.22 since that’s the policy version used in the cmake file you linked to, or 3.24 since that’s the version of cmake actually used to build releases mentioned earlier. But it also seems reasonable to opt into new all new policies like this PR is doing, set the max version to 3.31 and just document the fact that building with 3.31 is known to work.
I don’t think any of this is critically important, and I doubt it actually matters what max version we set here in practice. But it seems pretty wonky to me to intentionally disable improvements that have been made to cmake since 3.12, so I think this PR is better than the status quo.
If there are objections to this PR that 3.31 is too new, those could be reasonable, and maybe we should go with 3.22 or 3.24 as the max version instead of 3.31. But IMO it would not be good to set 3.22 or 3.24 min version if there’s no real reason to require those versions, and I don’t think the 3.12 policy set is the one we should be opting into, so I agree with purpleKarrot that we should be setting a max version to opt into newer policies.
Code review ACK f507c50ee5fcf86c666b4046d4f9bac4dcd33b56. Seems like an improvement to me to opt into new, non-deprecated policies by default if we’ve done a little testing with them and believe the build should work. A theoretical downside of this change is that that it could put users of older cmake versions at a disadvantage, because we will no longer be emulating old versions of cmake in our own builds and might unintentionally break things for them by relying on newer policies. But I don’t believe this will happen in practice and if it does happen I think it would be useful to have bug reports so we can know if we are relying on particular policies.
Would also be happy with a different change that sets max version to something less than 3.31 to be more conservative and not use the latest policies, but I don’t think it makes sense to use 3.12 policies.
If this PR is merged, what actions will be taken when a new CMake release comes out?
What are criteria to bump the <policy_max> option?
If this PR is merged, what actions will be taken when a new CMake release comes out?
My plan would be to wait for someone to open another PR, wait for it to get acks, and merge it. If someone is interested in using newer cmake policies, they should have motivation to submit a PR. If no one is interested, it should also be ok to stick with oldier policies we are using. But we should not intentionally prefer old and deprecated policies.
I think I will probably merge this PR soon. It seems like a safe, practical change to opt the build into more widely used, modern recommended cmake default behaviors instead of enabling dozens of backwards compatibility policies that cause modern cmake versions try to emulate a cmake release that is 7 years old.
I do think statement in the PR description “The policy version should always be set to the latest version that a project has been tested with” is probably a little too strong, because this change comes with some downside of making it possible for the build to unintentionally rely on new cmake policies without us realizing it (which is also possible without this change, but less likely). In the end it’s probably best to set the max version to a recent version that we think some number of users and developers are using, without needing it to be the very latest version.
4.0.2.
If this project is tied to Bitcoin Core, it could make sense to just take over the min version from there: https://github.com/bitcoin/bitcoin/blob/e486597f9a57903600656fb5106858941885852f/CMakeLists.txt#L10
This doesn’t make a lot of sense to me. IMO, purpose of the min version should just be to document the minimum version that should work, and not to preemptively break compatibility with older versions of cmake when there is no reason to believe they have any problems.
i do think it could be reasonable to set max version here to the policy version used by bitcoin core to avoid opting into newer cmake policies than are used by bitcoin core releases. This could be 3.22 since that’s the policy version used
Why does it not make sense?
This repo has no CI, and no developers that are testing with such an old cmake version. (As evident from #164). Also, it is unclear what the goal would be to even try to support such ancient versions. libmultiprocess is currently only used by Bitcoin Core, which requires 3.22, so any developer or users will use that version (or more likely a later one) anyway.
Maybe it is fine to derive from that in the future, when there is an actual need or use-case, but absent that it just seems simple and consistent to follow Bitcoin Core.
Why does it not make sense?
The cmake_minimum_required call does two different things:
1 - It enables & disables various cmake compatibility policies 2 - It enables build errors when you are using a version of cmake that is too old to work.
The focus of this PR is (1) and this PR only changes (1) not (2).
But with regard to (2), I am saying in that comment that it does not make sense to me to change minimum required version of cmake from 3.12 to something else when the cmake build here is minimal and only uses rudimentary cmake features.
I’m not saying we should try to support old versions of cmake or go out of our way to test old versions of cmake. (Though if someone wants to open a PR testing an old version of cmake in CI, it’d be more than welcome). All I’m saying is that we should not trigger a fatal error preemptively without a factual reason to believe there is some problem.
Maybe it is fine to derive from that in the future, when there is an actual need or use-case, but absent that it just seems simple and consistent to follow Bitcoin Core.
This would seem to create a dependency doesn’t need to exist. Like in the future if bitcoin core starts relying on a new cmake feature an needs to raise its minimum version, I don’t see what the point would be of opening a separate PR to raise the minimum version here. It seems better to just document what we think the actual minimum version here is and throw errors if an older version is used. It does not seem good to tie the repositories together with unnecessary requirements.
That said, if you are concerned that versions of cmake before 3.22 are bad or broken or scary in some way and want to trigger a fatal error on use, I wouldn’t object to a PR that raised the minimum version here. I just think it would be misleading and not practically helpful since bitcoin core already requires 3.22. It should also be unrelated to this PR since this PR is leaving the minimum version alone and only raising the policy version.
Here are a couple of examples of policies that change the build system’s behaviour depending on the CMake version in use:
That is the main reason I’m reluctant on use the ...<policy_max> option.
That is the main reason I’m reluctant on use the
...<policy_max>option.
CMP0117 stops specifying a redundant rtti flag CMP0141 stops specifying harmful debug information flags
You didn’t say what your reasoning is, but it seems like from these examples you don’t trust new cmake releases to set good default policies and want to opt into old policies cmake developers do not recommend… unless those policies are so old that they are below some minimum version of cmake we can’t support or are choosing not to support.
This seems like precarious situation to opt into, where you want to allow running new versions of cmake with old policies from some arbitrary version instead the actual default policies they ship with and recommend.
I do understand your previously stated concern that enabling recent policies will make the “build system’s behaviour dependent on the CMake version”. But that concern exists regardless, not just with cmake but with all the tools we are using. It’s a legitimate thing to be worried that the compilation behavior may be dependent on on the version of the compiler that is used. Maybe you are concerned GCC 14 will have some problems that GCC 13 doesn’t have. But the ideal solution to that is to use GCC 13, not use GCC 14 with a bunch of flags to emulate old behaviors. That’s a reason our official builds use a fixed set of dependencies, including a fixed cmake dependency. But letting version of cmake runtime and library vary up the the bleeding edge while statically setting a much older policy version seems like a dubious practice, IMO.
But with regard to (2), I am saying in that comment that it does not make sense to me to change minimum required version of cmake from 3.12 to something else when the cmake build here is minimal and only uses rudimentary cmake features.
The current CMake code in this project is a bit more complex than necessary. It kind of abuses the PUBLIC_HEADER property for something that it is not intended for and where there is a simper alternative in later versions. It uses complex generator expressions in target_include_directories that also have a simpler alternative in later versions. There is nothing we cannot do with CMake version 3.12, but we can greatly simplify the code if we increase the minimum version.
Note that “which version is required to make those simplifications” is the wrong question. The correct approach is to set the minimum according to outside requirements (like the version that is available in DebianTesting) and then make simplifications based on that.
but we can greatly simplify the code if we increase the minimum version.
for reference, a bump is done in https://github.com/bitcoin-core/libmultiprocess/pull/175
There is nothing we cannot do with CMake version 3.12, but we can greatly simplify the code if we increase the minimum version.
That is great to know. I would love to see a PR simplifying the cmake code here and bumping the minimum version!
And that is another reason I would like to merge this PR. Because currently, if a PR simplifying the code and bumping the minimum version was created, it would not only change which cmake versions show fatal minimum version errors, but it would also change a bunch of entirely unrelated build behaviors by altering the policy version as well.
This PR prevents that and decouples the policy version from the minimum version. It makes the build use recommended policies instead of deprecated ones, and lets the minimum required version be determined and set independently.
Here is another example of how new policies might be controversial: https://github.com/bitcoin/bitcoin/pull/31504.
Thanks for linking https://github.com/bitcoin/bitcoin/pull/31504. I think it provides another example of why this PR is a good idea. The change in https://github.com/bitcoin/bitcoin/pull/31504 enabling one specific policy turning on link-line deduplication, while leaving all other recommended policies turned off would have added a maintenance burden, and is exactly the type of complexity this PR would avoid.
Old linkers may require libraries to be specified multiple lines when linking. Newer linkers do not. Cmake has logic to deal with this complexity and policies which control that logic. The logic was introduced in 3.29 and improved in cmake 3.31. By setting a recent policy version and letting cmake releases actually apply the policies they were designed to use, we stop putting ourselves in a strange no-mans-land where we are using new versions of cmake with old policies.
It is true that different versions of cmake can have problems, just like different versions of libraries can have problems, and different versions of compilers can have problems. We should deal with that by pinning dependencies (which we effectively do in guix), not by spending time trying to second-guess cmake’s policy decisions and force our users into dozens of deprecated and no longer recommended behaviors.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| Stale ACK | ryanofsky |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
Reviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
1@@ -2,7 +2,7 @@
2 # Distributed under the MIT software license, see the accompanying
3 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
4
5-cmake_minimum_required(VERSION 3.12)
6+cmake_minimum_required(VERSION 3.12...3.31)
I think I’d like to merge this PR soon if there’s not a strong objection. Josibake pointed out that cmake documentation explicitly says “Projects should be updated to use the NEW behaviors of the policies as soon as possible,” and I think it just makes sense to not opt into deprecated policies without a specific reason for doing that.
I don’t think it matters too much which exact policy version is used, but it would make sense to specify policies that have some testing in CI. Currently the nix CI jobs are using cmake-3.31.6, the macos CI job is probably using 4.1.1, and it’s unclear what versions of cmake the openbsd and netbsd jobs are using. (Should probably update the cmake script to print the cmake version.) (EDIT: This is done in #207) Bitcoin core is still using policy version 3.22 with different runtime cmake versions in CI, and using pinned cmake runtime version 3.24 to build binary releases.
This PR changes the cmake policy version from 3.12 to 3.31 without changing the minimum cmake version, which is still 3.12. I think it’s good this PR is leaving the minimum version alone, because the policy version and minimum version are different concepts, and effects of changing them are not the same. There’s a separate PR #175 that does change the minimum version, and I also started working on a change to add CI coverage to the olddeps job for the current minimum version. (EDIT: This is done in #208)
An alternative to this PR could choose to use a policy version of 3.22 instead of 3.31 like bitcoin core does. This would go against what cmake documentation recommends and opt into deprecated behavior in builds, but it could reduce variance between cmake versions and might catch bugs that would be unnoticed if users are using older versions of cmake and developers are using newer versions of cmake, as discussed above.
Another alternative or possible followup, if we are worried about not using a consistent set of policies in the bitcoin core build, might be to wrap the cmake_minimum_required call in if (NOT DEFINED CMAKE_MINIMUM_REQUIRED_VERSION) to let bitcoin core policies take precedence.
I’m generally fine with focussing support on modern environments and only adding support for older stuff if there’s a need for it. Since Bitcoin Core requires 3.22 it would make little sense to support anything older.
If CI passes with 4.1 that’s fine too.
If CI passes with 4.1 that’s fine too.
Does CI have any job that uses CMake 4.1?
Does CI have any job that uses CMake 4.1?
Looks like not currently. I though mac os ci job might be using it because 4.1.1 is currently stable in homebrew, but in #207 the macos ci job https://github.com/bitcoin-core/libmultiprocess/actions/runs/17552747815/job/49848966387?pr=207 outputs 3.31.6
Josibake pointed out that cmake documentation explicitly says “Projects should be updated to use the NEW behaviors of the policies as soon as possible,” and I think it just makes sense to not opt into deprecated policies without a specific reason for doing that.
At some point, I stopped treating the CMake docs as a reference for best practices.
I still insist that Bitcoin Core’s build system should have fixed effective policies, independent of the CMake version in use.
Another point is that none of the reviewers of this PR mentioned whether they actually analysed the effects of the new policies up to CMP0180.
As a subproject of Bitcoin Core, the most straightforward way to enforce a fixed policy is not to use a version range. This approach is already used in other subprojects, such as libsecp256k1 and minisketch.
An alternative to this PR could choose to use a policy version of 3.22 instead of 3.31 like bitcoin core does.
Why not simply close this one in favour of #175?
Another alternative or possible followup, if we are worried about not using a consistent set of policies in the bitcoin core build, might be to wrap the
cmake_minimum_requiredcall inif (NOT DEFINED CMAKE_MINIMUM_REQUIRED_VERSION)to let bitcoin core policies take precedence.
This would be my second choice, if people agree that the extra complexity is worth it.
@hebasto can you clarify what your concern is exactly? If you want old cmake behaviors, you should use old versions of cmake. If you want new cmake behaviors you should use new versions of cmake. It makes no sense to use new versions of cmake with policy settings causing them to emulate bugs and quirks of older versions released 7 or 4 years ago. This is not what cmake policies were designed for or what cmake documentation recommends for good reason. Cmake policies are a tool for controlling rollout of new changes. They are not a guarantee of stability and if anything they will make builds less stable as policy and runtime versions diverge and receive less testing.
I still insist that Bitcoin Core’s build system should have fixed effective policies, independent of the CMake version in use.
Bitcoin core’s build system pins its cmake version via guix similar to how it pins other dependencies. There is nothing special about cmake in this respect. I can understand why it’s important to pin the cmake version, but it’s doesn’t make sense to insist that cmake behavior be the same “independent of the cmake version.” The point of different versions is to have different behavior.
Another point is that none of the reviewers of this PR mentioned whether they actually analysed the effects of the new policies up to CMP0180.
Not sure what you are looking for here, but happy to review or test anything you’d like me to.
Why not simply close this one in favour of #175?
#175 is setting the minimum version incorrectly, but if it can be updated to just change the policy version without changing the minimum version it seems fine.
This would be my second choice, if people agree that the extra complexity is worth it.
If @purpleKarrot can add if (NOT DEFINED CMAKE_MINIMUM_REQUIRED_VERSION) and it would unblock this PR, I’d be happy to see that. Would also be happy to see this added in a separate PR or to open a PR which adds this. I do think it would be an improvement to use policies that are 4 years old instead of 7 years old in the bitcoin build at least.
@hebasto can you clarify what your concern is exactly? If you want old cmake behaviors, you should use old versions of cmake. If you want new cmake behaviors you should use new versions of cmake.
None of these are my intentions. As I wrote earlier, Bitcoin Core’s build system behavior should not be affected by the CMake version in use.
It makes no sense to use new versions of cmake with policy settings causing them to emulate bugs and quirks of older versions released 7 or 4 years ago. This is not what cmake policies were designed for or what cmake documentation recommends for good reason. Cmake policies are a tool for controlling rollout of new changes. They are not a guarantee of stability and if anything they will make builds less stable as policy and runtime versions diverge and receive less testing.
The opposite. This is exactly what policies were designed for. From Professional CMake: A Practical Guide 21st Edition, Section 13.1:
This gives projects confidence that developers should be able to update to a newer version of CMake at their convenience, and the project will still build as it did before.
Again, I would strongly advise against treating the official docs as best practices. CMake has a history of introducing questionable features aimed at paid customers.
I still insist that Bitcoin Core’s build system should have fixed effective policies, independent of the CMake version in use.
Bitcoin core’s build system pins its cmake version via guix similar to how it pins other dependencies. There is nothing special about cmake in this respect. I can understand why it’s important to pin the cmake version, but it’s doesn’t make sense to insist that cmake behavior be the same “independent of the cmake version.” The point of different versions is to have different behavior.
Following this logic, Bitcoin Core should have CI jobs that running the CMake version pinned in Guix, and some developers would be expected to do the same, which seems rather awkward.
Why not simply close this one in favour of #175?
#175 is setting the minimum version incorrectly…
My apologies, but I must disagree.
None of these are my intentions. As I wrote earlier, Bitcoin Core’s build system behavior should not be affected by the CMake version in use.
Sorry, I don’t understand. This seems analogous to saying the compiler behavior should not be affected by the gcc version in use. The reason different cmake versions exist is to have different behavior. Newer versions should have newer features, fewer bugs, faster builds, etc. Again it would help if you can explain how cmake is different from other dependencies like gcc or sqlite where different versions have different behaviors and we don’t insist on there being unnecessary flags to make new versions act like old versions.
The opposite. This is exactly what policies were designed for. From Professional CMake: A Practical Guide 21st Edition, Section 13.1:
This gives projects confidence that developers should be able to update to a newer version of CMake at their convenience, and the project will still build as it did before.
This quote is saying the same thing I said: “Cmake policies are a tool for controlling rollout of new changes.” The quote says “a newer version at their convenience” not “new versions 7 years from now.” Cmake policies are a tool for smoother upgrades, not form of pinning.
Following this logic, Bitcoin Core should have CI jobs that running the CMake version pinned in Guix, and some developers would be expected to do the same, which seems rather awkward.
Yes, it would make sense for there to be a CI job that tests the same cmake version as guix. I don’t think this job would be particularly likely to break and detect issues, but it’d be natural to have and doesn’t seem like it would be more awkward to set up than any other CI job. Of course many developers do guix builds as well. I think I don’t understand what is bad or awkward about either of these things.
#175 is setting the minimum version incorrectly…
My apologies, but I must disagree.
To be specific, #175 would result in error messages which are not accurate like “CMake 3.22 or higher is required. You are running version […].” These errors would be incorrect because this project doesn’t rely on any new cmake features and versions back to 3.12 work well, even with some CI coverage in #208.
I’d be happy with a PR that set cmake_minimum_required(VERSION 3.12...3.22) accurately representing the required minimum version as 3.12 and increasing the policy version to 3.22. I would even favor that change if I were concerned that “CMake has a history of introducing questionable features aimed at paid customers.” Since that is not a concern I actually have, I am also happy with any other change that increases the policy version and without changing and misrepresenting the minimum required version.
Maybe as a possible way forward you could clarify which of the following variants you might dislike the least?
0cmake_minimum_required(VERSION 3.12...3.31)
0cmake_minimum_required(VERSION 3.12...3.22)
0if(NOT DEFINED CMAKE_MINIMUM_REQUIRED_VERSION)
1 cmake_minimum_required(VERSION 3.12...3.31)
2endif()
0if(NOT DEFINED CMAKE_MINIMUM_REQUIRED_VERSION)
1 cmake_minimum_required(VERSION 3.12...3.22)
2endif()
Maybe as a possible way forward you could clarify which of the following variants you might dislike the least?
0cmake_minimum_required(VERSION 3.12...3.31)0cmake_minimum_required(VERSION 3.12...3.22)0if(NOT DEFINED CMAKE_MINIMUM_REQUIRED_VERSION) 1 cmake_minimum_required(VERSION 3.12...3.31) 2endif()0if(NOT DEFINED CMAKE_MINIMUM_REQUIRED_VERSION) 1 cmake_minimum_required(VERSION 3.12...3.22) 2endif()
0if(DEFINED CMAKE_MINIMUM_REQUIRED_VERSION)
1 cmake_minimum_required(${CMAKE_MINIMUM_REQUIRED_VERSION})
2else()
3 cmake_minimum_required(VERSION 3.12...4.1)
4endif()
A new version of CMake is released. Project author tries it out and gets a warning that the project is affected by a new policy. Project author fixes the project so that it is no longer affected by the policy (eg by fixing an invalid regex, by properly quoting a string, by replacing a deprecated command invocation, by searching for a packages in config mode only, or similar). After making the change, CI verifies that the change did not raise the minimum requirement: Using an old versions of CMake still works. Project author increases the policy version to prevent regressions (like the re-introduction of invalid regexes, wrongly quoted strings, deprecated commands, deprecated find-modules, etc).
0if(...) 1 cmake_minimum_required(...) 2endif()
Setting the policy version is a statement of the project author that the project is not affected by policy changes. Why should this be set from outside?
Setting the policy version is a statement of the project author that the project is not affected by policy changes. Why should this be set from outside? @purpleKarrot it makes sense to me to use the policy version the way you describe, as an upgrade mechanism and way of documenting compatibility with new cmake versions as they are released.
However, the bitcoin core project is not using it this way, because the bitcoin core policy version is hardcoded to 3.22 even though most developers are using newer versions of cmake. Hebasto and marco also have expressed a preference for #175 instead of this PR, setting the libmultiprocess policy version to 3.22 to match bitcoin’s policies.
As far as I can tell, hebasto and maybe marco are not looking at the cmake policy version setting as an upgrade mechanism and way to provide a smooth transition to new cmake versions as you describe. Instead they see the cmake policy version as a pinning mechanism: a way to guarantee old cmake behaviors are used regardless of the user’s environment and version of cmake they have installed. This way for example, if developers are using new versions of cmake, and users are using old versions of cmake, everybody will be be using old cmake policies, and if there are any problems with those policies, developers will know about them and fix them.
If hebasto wants the bitcoin core policy to be set to 3.22 and for libmultiprocess policy to match this when it is compiled as part of bitcoin core, I don’t have a problem with supporting that even though I feel the approach has more downsides than upsides. (Downsides being disabling cmake features and bugfixes, not respecting user preferences, using cmake in less common configuration against cmake developer recommendations. Upside being a form of pinning where there is less variance between builds in different environments.)
Setting the policy version is a statement of the project author that the project is not affected by policy changes. Why should this be set from outside?
It’s not about setting the policy version from outside. The downstream project is simply asking the project author not to break its currently pinned policy behaviour. The project author chooses a pinned policy version for such a use case at their own discretion.
If hebasto wants the bitcoin core policy to be set to 3.22 and for libmultiprocess policy to match this when it is compiled as part of bitcoin core…
The code snippet I suggested does indeed align the policies for Bitcoin Core and libmultiprocess, but that’s just a (reasonable) simplification. All I’m asking is that a pinned policy, chosen in some way, be used when building as a subproject of Bitcoin Core.
Thanks for clarification hebasto. That all makes sense to me and explains what you are looking for, except I am confused by the last sentence:
All I’m asking is that a pinned policy, chosen in some way, be used when building as a subproject of Bitcoin Core.
This last sentence seems to imply that you don’t care how the pinned policy is chosen. But you do have a clear preference for pinning the policy to an old version of cmake, not a current version of cmake like this PR.
All I’m asking is that a pinned policy, chosen in some way, be used when building as a subproject of Bitcoin Core.
This last sentence seems to imply that you don’t care how the pinned policy is chosen.
That’s correct. However, it should be a value within the [3.12..3.22] range.
But you do have a clear preference for pinning the policy to an old version of cmake, not a current version of cmake like this PR.
How would you suggest implementing policy version pinning in the latter case?
Thanks I think I see the discrepancy. I am using the word “pinned” to mean “fixed.” This PR is setting a fixed policy version, so I would say this PR is pinning the policy version.
But I think you are using “pinned” to not simply mean “fixed” but “fixed to the minimum version of cmake that does not trigger a build error.” You want it to be an error to build with any version of cmake below the specified policy version.
All I’m asking is that a pinned policy, chosen in some way, be used when building as a subproject of Bitcoin Core.
But why? If project A was written to support the NEW behavior for policy X, why would project B build project A as subproject with policy X unset? The only usecase I can imagine where a superproject may want to set the policy version of a subproject is this:
Project A was not updated in many years, and therefore still sets a very old policy version, like 2.8.12. It is however very stable; there are no issues building it with the latest policy version of CMake and the latest compilers. The only issue is that CMake version 4.1 treats it as an error to set a policy version below 3.10. Project B wants to build project A as subproject without modifying A’s source code. It can do that by setting CMAKE_POLICY_VERSION_MINIMUM right before the add_subdirectory() call.
@hebasto, I think you are asking for a feature that CMake does not provide. There is no way to tell CMake to behave as version 3.22. The policy mechanism may look like such a feature, but it is something different. CMake actually does provide feature flags for features that are currently under development. But the policies are not feature flags.
@hebasto, I think you are asking for a feature that CMake does not provide. There is no way to tell CMake to behave as version 3.22. The policy mechanism may look like such a feature, but it is something different. CMake actually does provide feature flags for features that are currently under development. But the policies are not feature flags.
Hmm, I’m confused. The quote I mentioned earlier states that “the project will still build as it did before”.
I’d like to avoid, for example, treating a compiler flag differently depending on the policy version. @purpleKarrot
Mind elaborating your point please? Or demonstrate it?
The quote I mentioned #163 (comment) states that “the project will still build as it did before”.
Yes, in the sense that it will not suddenly start failing or introduce other incompatible changes. It however makes no promise to get exactly the same behavior. If you need exactly the same behavior for reproducibility, you need to pin down the executable.
To demonstrate, I have installed CMake version 3.22.6 and configured bitcoin core. I have also configured it with CMake version 4.0.3 and compared the results. You can see that there are a number of changes both in the CMake cache and in the generated build system:
0CMakeCache.txt | 121 +-
1CMakeFiles/rules.ninja | 1438 ++++++++--
2build.ninja | 3867 ++++++++++++++++++---------
I then changed the policy version to 4.0 and configured again. The following changes are not surprising; we expect different behavior when we set a different policy. But the output above should convince you that pinning a policy will not guarantee exact behavior across different versions of CMake.
0build.ninja | 6466 ++++++++++++++++++++++++++++++++++++++----
The quote I mentioned #163 (comment) states that “the project will still build as it did before”.
Yes, in the sense that it will not suddenly start failing or introduce other incompatible changes. It however makes no promise to get exactly the same behavior. If you need exactly the same behavior for reproducibility, you need to pin down the executable.
To demonstrate, I have installed CMake version 3.22.6 and configured bitcoin core. I have also configured it with CMake version 4.0.3 and compared the results. You can see that there are a number of changes both in the CMake cache and in the generated build system:
0CMakeCache.txt | 121 +- 1CMakeFiles/rules.ninja | 1438 ++++++++-- 2build.ninja | 3867 ++++++++++++++++++---------I then changed the policy version to 4.0 and configured again. The following changes are not surprising; we expect different behavior when we set a different policy. But the output above should convince you that pinning a policy will not guarantee exact behavior across different versions of CMake.
0build.ninja | 6466 ++++++++++++++++++++++++++++++++++++++----
Thanks for this demonstration.
So, a CMakeLists.txt with cmake_minimum_required(VERSION 3.22) generates different underlying build system code depending on the CMake binary version being used.
What about the resulting compiler, linker or archiver invocation strings?
So, a CMakeLists.txt with cmake_minimum_required(VERSION 3.22) generates different underlying build system code depending on the CMake binary version being used.
Yes, definitely.
What about the resulting compiler, linker or archiver invocation strings?
No change can be observed in the compile_commands.json file.
It’s good to continue discussion here, but I think ultimately this PR could be closed in favor of #209, since #209 includes fixes for things that break with newer versions and adds more CI coverage.
That’s just to say that I think the current version of this PR breaks CI, but if it doesn’t do that or if the problems are fixed, I think it would be fine to merge this PR instead of #209. Will close and reopen this to see what the situation with CI actually is.
the current version of this PR breaks CI @hebasto, can you reproduce the issue with a minimal example on freebsd?
0cmake_minimum_required(VERSION 3.12)
1project(Dummy CXX)
2find_package(Threads REQUIRED)
Does this start failing when ...4.1 is added?
the current version of this PR breaks CI
@hebasto, can you reproduce the issue with a minimal example on freebsd?
0cmake_minimum_required(VERSION 3.12) 1project(Dummy CXX) 2find_package(Threads REQUIRED)
https://github.com/hebasto/github-actions/actions/runs/17740332314/job/50412442287
Does this start failing when
...4.1is added?
https://github.com/hebasto/github-actions/actions/runs/17740344810/job/50412485438
the current version of this PR breaks CI
I think an easy to fix for this would be to cherry pick e416d45528faa7d4a15dc6e46e265778e611a26c from #209. You could also add 6ba1050bef0b5ea5df35d3623c9e101cceb82677 here, or just cat CMakeFiles/CMakeConfigureLog.yaml here to help debug
the current version of this PR breaks CI
@hebasto, can you reproduce the issue with a minimal example on freebsd?
0cmake_minimum_required(VERSION 3.12) 1project(Dummy CXX) 2find_package(Threads REQUIRED)Does this start failing when
...4.1is added?
Here is an MRE for FreeBSD 14.3 without the llvm19 package installed (no clang-scan-deps):
0cmake_minimum_required(VERSION 3.12...4.1)
1project(Dummy CXX)
2set(CMAKE_CXX_STANDARD 20)
3find_package(Threads REQUIRED)
Run as follows:
0cmake -B build -G Ninja
Here is an MRE for FreeBSD 14.3 without the
llvm19package installed (noclang-scan-deps):
Am pretty sure these problem’s aren’t bsd-specific and I can reproduce something similar on linux.
The find_package(Threads REQUIRED) implementation in general seems to be good at failing and hiding the reasons for the failure. For example while I was implementing #209, I set CMAKE_TRY_COMPILE_NO_PLATFORM_VARIABLES to revert effects of CMP0137 and find_package(Threads REQUIRED) started failing there with no information as well. Interestingly this only happened with the ninja backend, not the make backend. Minimal CMakeLists.txt to reproduce is:
0cmake_minimum_required(VERSION 3.12...4.1 FATAL_ERROR)
1
2project("Libmultiprocess" CXX)
3
4set(CMAKE_CXX_STANDARD 20)
5set(CMAKE_CXX_STANDARD_REQUIRED YES)
6
7set(CMAKE_TRY_COMPILE_NO_PLATFORM_VARIABLES TRUE)
8
9find_package(Threads REQUIRED)
and command line is:
0CXX=clang++ cmake -B build -G Ninja
or for a pure environment:
0nix-shell --pure -p cmake -p ninja -p clang --run "CXX=clang++ cmake -B build -G Ninja"
This only fails when CMAKE_TRY_COMPILE_NO_PLATFORM_VARIABLES is true, CXX=clang++ is specified, and the Ninja backend is used. Otherwise there is no problem. The CMakeConfigureLog.yaml shows the build trying to call clang-scan-deps with a broken command line and "" where the clang-scan-deps command should be:
FAILED: CMakeFiles/cmTC_c7096.dir/CheckForPthreads.cxx.o.ddi
"" -format=p1689 -- /nix/store/fmysfy9gl5d59yir4fksi29wz10maljg-clang-wrapper-19.1.7/bin/clang++ -std=gnu++20 -x c++ /home/russ/work/min/build/CMakeFiles/CMakeScratch/TryCompile-t01yCJ/CheckForPthreads.cxx -c -o CMakeFiles/cmTC_c7096.dir/CheckForPthreads.cxx.o -resource-dir "/nix/store/fmysfy9gl5d59yir4fksi29wz10maljg-clang-wrapper-19.1.7/resource-root" -MT CMakeFiles/cmTC_c7096.dir/CheckForPthreads.cxx.o.ddi -MD -MF CMakeFiles/cmTC_c7096.dir/CheckForPthreads.cxx.o.ddi.d > CMakeFiles/cmTC_c7096.dir/CheckForPthreads.cxx.o.ddi.tmp && mv CMakeFiles/cmTC_c7096.dir/CheckForPthreads.cxx.o.ddi.tmp CMakeFiles/cmTC_c7096.dir/CheckForPthreads.cxx.o.ddi
/bin/sh: line 1: : command not found
ninja: build stopped: subcommand failed.
You can tell this was supposed to be a clang-scan-deps command line due to the -format=p1689 parameter. Also adding cmake_policy(SET CMP0155 OLD) fixes the problem. Though adding set(CMAKE_CXX_SCAN_FOR_MODULES OFF) does not fix it. Also interestingly replacing set(CMAKE_TRY_COMPILE_NO_PLATFORM_VARIABLES TRUE) with cmake_policy(SET CMP0137 OLD) is not enough to cause the problem to happen, even thought I might expect those two things to be equivalent.
All of this just makes me think find_package(Threads REQUIRED) is very murky territory and find_package(Threads) is a more correct and less fragile alternative.
Here is the upstream issue: https://gitlab.kitware.com/cmake/cmake/-/issues/27228
All of this just makes me think find_package(Threads REQUIRED) is very murky territory and find_package(Threads) is a more correct and less fragile alternative.
It seams that FindThreads (and other modules that rely on CheckCXXSourceCompiles) have issues with policy CMP0155 when clang-scan-deps is not avaliable.
However, if that module has issues, invoking it without REQUIRED is not more correct. In fact, it is probably more fragile, because it just ignores the underlying problem.
However, if that module has issues, invoking it without
REQUIREDis not more correct. In fact, it is probably more fragile, because it just ignores the underlying problem.
I am not saying invoking it without required is more correct because the module has issues. I am saying invoking it without required is correct because it is not required.
invoking it without required is correct because it is not required.
Even if you just want to use it optionally, you want a reliable result whether it is available.
Even if you just want to use it optionally, you want a reliable result whether it is available.
Yes, and thanks for creating the upstream issue!