WIP: Change the way dune module versions are checked in cmake
Summary
In CMake we parse the dune.module
file, extract the dependencies and their versions and perform a manual version check for find_package
. This is the second time versions of modules are checked and the way it is implemented is not compatible to dunecontrol
, see #173. This MR proposes a different workflow:
- A dune module knows best to which version it is compatible. Typically, this compatibility is
SameMinorVersion
. - We let cmake automatically generate
dune-<module>-config-version.cmake
files usingwrite_basic_package_version_file
with a specified version compatibility. - When configuring a module and when searching for its dependencies, we use
find_package(<module> <version>)
with just the version string we want to be compatible with.
Closes #173
Merge request reports
Activity
Check pipeline in https://gitlab.dune-project.org/infrastructure/dune-nightly-test/-/pipelines/35358
Edited by Simon PraetoriusThe actual question is, should we remove the version check from CMake or from dunecontrol? dunecontrol has some bugs like #112 that might be easier get fixed within CMake.
From a historic point of view, we started with the CMake modules without dunecontrol and added it later. And on Windows I worked without dunecontrol. That are reasons, but no arguments to not change it today.
added 1 commit
- 0dfba6d0 - Extract the >= version from the version relation string
Is the
dune.module
version format specified anywhere? Or is it again just guessing which format should be supported? Is it compatible to debian package format version requirements? Are there all logical operators allowed? Are brackets allowed? Do we need to implement a full logical expression parser? I don't know. Thus, neither dunecontrol nor cmake is wrong nor correct. They just parse the version string differently.I have simplified the version check in cmake to a simple compatibility definition. Request version x.y.z and allow all version that are compatible w.r.t. some contract that is properly defined. The default contract is "SameMinorVersion", but each dune module could choose its own compatibility contract, either from the list of default cmake compatibility formats or by providing a version file template itself.
The problem with current version specification in
dune.module
is that we have to translate this to multiple other formats, e.g. debian package format, pkg-config format, cmakefind_package
format and maybe more. All these formats are possibly different and allow different logical relations.
An example, where my approach behaves differently to the old one, is the pdelab pipeline: The master branch of pdelab requests versions
>= 2.7
, while the pdelab version is 2.8-git and all the other modules also have a 2.8 developer version. This is by our versioning scheme typically not classified as compatible. Thus, the cmake configuration crashes with an error of incompatible version:Could not find a configuration file for package "dune-grid" that is compatible with requested version "2.7".
Is pdelab really compatible to
2.7
or is this just an error in the version specification?So, one question remains: How to specify a version requirement that violates the compatibility statement of a module? E.g. you have tested your module with dune 2.6, 2.7, and master. This needs to be investigated.
Is pdelab really compatible to
2.7
or is this just an error in the version specification?The curreent PDELab works with core 2.7 and the development branch.
To be totally correct, we would have to specify something like "dune-grid >=2.7 && <2.9". But this is something that didn't work up to now and also didn't cause many problems. Still to specify a minimal version is something done by many downsteam modules and often people try to stay compatible to a range of versions
We should not break this behavior in cmake.
I disagree. Having multiple conditions make the code complicated for little gain. Speaking from a CMake perspective, if = and >= is not enough, you should manually write the code you need to process the version number.
Edit I think we don't disagree, you want to have = and >= and that's all, right?!Edited by Christoph Grüninger@christi you are right, we should not break behavior in cmake that is introduced in other places. That is why this situation needs further investigation.
My proposal is just an experiment. Maybe it can not work with our more involved version constraints. After thinking about it a bit more, I think that having the version contract defined by the module itself covers only half the story. Your restriction of a wider version range could not be covered by this approach easily. The module can only specify that it tries to have a stable interface within the minor version number, e.g. 2.7 and 2.7.1 and 2.7.9 should be interface compatible. This is what the "contract" specifies. So if you ask for a package version compatible to, e.g., 2.7. CMake accepts everything of the version 2.7.x if
SameMinorVersion
is specified.But this does not cover the requirement that we want to support a wider version range, e.g., 2.6-2.8, for some packages - because we have tested this or have implement version switches or use only a subset of functions or ...
A solution could be, that we do not test for compatible version in
find_package
of CMake directly, but perform a version compatibility check afterwards. This, however, needs to be a compatible check to whatdunecontrol
performs, maybe even using the same code for the check.Hi Simon, I didn't want to sound harsh.
I think it good to clean up this part of the buildsystem, but it is a bit more involved.
I suggest, that we
- first write down a proper definition of the supported version strings
- we should try to stay compatible with the current
dunecontrol
parser - ideally we should use the same code to do the version check in cmake and in dunecontrol
- lastly we should then also add support to read these version strings and transform them to version strings of other systems, e.g. pkg-config.
mentioned in merge request !1249 (merged)
added buildsystem label
changed milestone to %CMake Modernization