search

key4hep / EDM4hep

Generic event data model for HEP collider experiments
https://cern.ch/edm4hep
Apache License 2.0
24 stars 35 forks source link

Use 'declarationFile' instead of #include in CovMatrix component #308

Closed m-fila closed 1 month ago

m-fila commented 2 months ago

BEGINRELEASENOTES

ENDRELEASENOTES

Closes #296

Declarations added with #include were not appearing in a documentation generated with doxygen. New podio direcative 'declarationFile' (https://github.com/AIDASoft/podio/pull/601) includes these declaration during file generation without relaying on pre-processor and make them visible to doxygen Old New
Screenshot_2024-05-01_14-27-21 covmatrix_fix
tmadlener commented 1 month ago

I think we also need to install the extra_code directory into <prefix>/share (or wherever we put the yaml file exactly). Otherwise downstream models cannot use EDM4hep as an upstream. See, e.g. https://github.com/key4hep/EDM4hep/actions/runs/9205295127/job/25320679012?pr=308#step:4:1220

m-fila commented 1 month ago

The extra_code is installed in <prefix>/share/edm4hep/edm4hep/extra_code to mimic the path in source tree. Or in the source tree it could be move from edm4hep/extra_code to the top level and then installed in <prefix>/share/edm4hep/extra_code

hegner commented 1 month ago

@m-fila this PR destroyed all automatic testing because the keyword DEPENDS is interpreted as value of PODIO_IO_HANDLERS. Can you prepare a fix? ping @tmadlener

tmadlener commented 1 month ago

As far as I can tell only those workflows which build on an old(ish) version of podio are currently broken. However, there are multiple small changes in EDM4hep that depend on features of podio that came after v00.99

m-fila commented 1 month ago

this should break only the Key4hep build / build (release, ...) checks. Do you mean there are some other checks failing or should I fix the build (release, ...) checks? If the second is the case I don't really see any solutions other than reverting this, but then these test will fail due to some other changes (failing since the introduction of interfaces)

tmadlener commented 1 month ago

I don't think it's worth fixing the release builds. They effectively just have a podio version that is no longer sufficient. We could put a version requirement into the top level CMakeLists.txt if we wanted to, to make the tests fail earlier. However, since podio v1.0 is very close, I think we just wait with that until we have that.