build: Pick up GeoModel v6 or v7 #3736
Conversation
CMakeLists.txt
Outdated
| foreach(_gm_ver 7.0.0;6.3.0) | ||
| message(STATUS "Trying to find GeoModel version ${_gm_ver}") | ||
| find_package(GeoModelCore ${_gm_ver} CONFIG) | ||
| find_package(GeoModelIO ${_gm_ver} CONFIG) |
There was a problem hiding this comment.
So, did the version range formalism not work then? I was hoping that this could've been written just as something like:
find_package(GeoModelCore 6.3...7.99)I'd really like to avoid such custom for-loops if at all possible... 🤔
There was a problem hiding this comment.
Yeah so the range formalism (for some reason) only allows ranges within a single MAJOR version it seems. The range version feature is also seemingly orthogonal to the compatibility check the package can do, which apparently still applies somehow?
There was a problem hiding this comment.
That's very disappointing. If we can't write a possibly quite wide range, then what's the point of this feature even? 😕
I do wonder that once we have an entire for-loop in our code to make all this happen, is the version check actually worth it? Then again, I was never really fond of such version checks to begin with... 🤔
There was a problem hiding this comment.
I have to admit I also hack the versions from time to time, so also not a big fan of it. Alternatively we could search without version constrain and then check the version manually afterwards
There was a problem hiding this comment.
My understanding is that if you supply a range, both ends of the range have to be deemed compatible by the target version, i.e. you can't span a range over MAJOR versions, which is indeed disappointing.
I really do want to have a version check, but indeed it might be better to manually check the version after finding without a constraint. I think this only makes a difference if multiple versions are found and CMake would otherwise pick a compatible version out of the available ones.
|
Allow picking up GeoModel v6.3.0 or v7.0.0.
See https://gitlab.cern.ch/atlas/athena/-/merge_requests/75025