Don't request a version of podio in find_package(podio)

[jmcarcell]

cmake has decided that the version 1.0.0 of podio is not compatible with 0.16.3 and fails at find_package(podio).

BEGINRELEASENOTES

• Try to find podio 1.0 if the version found is not compatible

ENDRELEASENOTES

[tmadlener]

Why not just bump it to 1? I would like to have this information also in the package sources directly, not only in spack.

[jmcarcell]

I see a few reasons why I don't like having the version after seeing what happens. One is that we end up never updating it, because probably this won't build with 0.16.3 but no one is building with that so in the end it's doing nothing. Then, we can bump it to 1 but it builds fine with previous versions so we are introducing a version dependency when there isn't one. And also cmake decides when these are not compatible so we may have the same issue when going from version 1 to 2 multiplied by the number of packages that require version 1.

[tmadlener]

We could define a version range find_package(podio minVersion...maxVersion), but AFAIU (documentation) that would require exact versions on both ends, and there is no way to define only a lower bound. Also at least in this case, I think it would be OK to require podio v1, even if things still build with older version.

[jmcarcell]

We could define a version range find_package(podio minVersion...maxVersion), but AFAIU (documentation) that would require exact versions on both ends, and there is no way to define only a lower bound.

The lower bound is the current way, or am I not understanding something?

I still think we should not use the version numbers as we do in most places. The only gain is that it will fail fast when building older versions and the downside is having to modify in every package the version requirement when there are (or cmake decides there are) incompatibilities (every major version bump?). Then the spack recipes should be the only source of truth for versions, as in the end it becomes a stale number (so useless, and wrong).

[tmadlener]

The lower bound is the current way, or am I not understanding something?

No, I don't think so (but I might just be misunderstanding CMake documentation once again). IIUC, currently it goes into its default search mode which is: same major version, and >= minor and patch version. However, I can't find that in the documentation at the moment.

I think the range would allow us to specify a range of versions to work with, but then we would probably also have to change the compatibility mode in podio. (As per this documentation).

Having said all that, I think the real issue in this case is probably the podio config:
https://github.com/AIDASoft/podio/blob/072c7dce699a2dc90f225cb0a75a4f2ce9677cf8/cmake/podioCreateConfig.cmake#L8

Technically that is correct though from a podio perspective. Not sure there is an easy solution in this case. It looks like the whole thing is not completely designed to easily handle 0.x.y versions.

Either we drop it, or we bump it to 1.0 (even if things build with older versions) would be my suggestions. Bumping it to 1.0 would have the benefit of making people move to that if they build this outside of spack.

[jmcarcell]

Technically, based on that documentation and since typically our packages have been so far packages that don't guarantee backwards compatibility ExactVersion should be used, and that would render the version requirement very annoying to work with of course. I don't think we have to do that, we can keep it as it is.

I see the point to make people to move to 1.0, however a setup in which you keep updating k4FWCore but not podio is unlikely I think. The fact that no one complains about the version numbers makes me think they don't have that much effect (and will end up annoying us like now). My claim is that if we remove those version numbers no one will complain (currently only for podio and EDM4hep I think). Maybe @andresailer wants to comment since we should take a decision. For EDM4hep it will happen the same thing and there are a few repos that have a version requirement which we would have to update to 1.0 even if it's not necessary and some of them are outside of Key4hep (so no nightlies until they get updated...).

[wdconinc]

I'd like to (strongly) suggest keeping the lower version (as I've expressed on the DD4hep issue as well; I think the solution there is one what would work here as well).

Removing any minimum version is a bit of a lazy approach on the part of the developers, and it leaves it up to software librarians and packagers (in spack upstream, but also in key4hep-spack) to figure out what works and what doesn't. Those versions in spack do not appear there just by accident but based on what is in the CMakeLists.txt :-)

We also aren't all able to upgrade the same components at the same time. At EIC we can't switch to podio v1.0 yet, but we do benefit from updates to the other packages as they are updated.

One is that we end up never updating it, [...] but no one is building with that so in the end it's doing nothing

I think that's an actionable part here: it would make sense to build and unit test against the minimum versions in CI.

[tmadlener]

I think in this case the only thing that we can really do

find_package(podio)
if (NOT podio_VERSION VERSION_GREATER_EQUAL 0.16.3)
  message(FATAL "Need at least podio v0.16.3)
endif()

[andresailer]

Maybe like in DD4hep AIDASoft/DD4hep#1280

[jmcarcell]

Removing any minimum version is a bit of a lazy approach on the part of the developers, and it leaves it up to software librarians and packagers (in spack upstream, but also in key4hep-spack) to figure out what works and what doesn't. Those versions in spack do not appear there just by accident but based on what is in the CMakeLists.txt :-)

But this is exactly what has been happening, 0.16.3 is from more than one year ago so having it there hasn't helped at all in the last year. I'm not against having the numbers but breaking builds; if what people really want is to use it as a lower bound then I think the solution presented by Thomas is fine, fail if lower than the bound but don't fail just because podio upgraded to 1.0. Then we can just simply change the CMakeLists.txt to require 1.0 when we see fit.

I'll merge this PR today so that we can have nightlies tomorrow.

[tmadlener]

I think we can do as it is proposed in this PR, but before the next release / tag we should probably either figure out which minimal version of podio still works (because I doubt 0.16.3 is it) or simply bump to 1.0 before that.

[jmcarcell]

I found that these changes are more convenient like in DD4hep, because then multiple repos can be built at the same time with CMake in case we want to support that in the future...

[andresailer]

@jmcarcell Please update the release notes to reflect the actual change.