Skip to content

Conversation

@tmadlener
Copy link
Collaborator

@tmadlener tmadlener commented Dec 1, 2025

Fixes #892

BEGINRELEASENOTES

  • Make sure to generate the Collection headers to the front of the list of header file that is passed to root dictionary generation.

ENDRELEASENOTES

@wdconinc this fixes things for me. Not sure if this points to a more general "instability" in the code generation (e.g. missing headers, or something along those lines) or if this is just a quirk of ROOTs dictionary generation.

I am not entirely sure how to (or at least where) to add a unittest for this. I could not get the original failure reproduced locally within podio and its tests for some reason. We could add it to the downstream tests of EDM4hep, but that would imply some potentially longish and indirect feedback loop.

Without this it's not possible to load a generated datamodel via ROOTs
ipmlicit handling of library loading, i.e.

test.C:
    #include <edm4hep/MCParticle.h>
    int test() { return 0; }

will work without these changes when doing

   root test.C+

but will break when doing

    root test.C

See AIDASoft#892 for more details

Technically they would only have to appear before any of the other
datatype related headers, but sorting them to the beginning of the total
list is less logic and easier to do.
@wdconinc
Copy link
Contributor

wdconinc commented Dec 1, 2025

Would it make sense to flag this to the ROOT folks? Even if it's not related to a ROOT version upgrade, maybe they are interested in looking at this?

@jmcarcell
Copy link
Member

Doesn't clang-format modify the order?

@tmadlener
Copy link
Collaborator Author

tmadlener commented Dec 1, 2025

https://cern.zoom.us/j/98843051283?pwd=YVVZS29ZR2VRTDRibnd6aUVxdDkxQT09

Probably. Although that would require condensing it into a more minimal (and maybe podio free) reproducer(?). Not sure if I will be able to make that still this week.

Doesn't clang-format modify the order?

Depends on the configuration on the client (e.g. EDM4hep) side. In this case, however, the issue persists even if no clang-format is run on top of the generated files (as that is how I did this to speed up code gen). And the order that matters in this case is the order in which we pass it to the ROOT cmake macro that does the dictionary generation.

@tmadlener
Copy link
Collaborator Author

Unless there is something that speaks against it, I would merge this tomorrow. I would then try to follow up with ROOT (and a smaller example) afterwards.

@wdconinc
Copy link
Contributor

wdconinc commented Dec 1, 2025

Unless there is something that speaks against it, I would merge this tomorrow. I would then try to follow up with ROOT (and a smaller example) afterwards.

Sounds good to me.

@tmadlener tmadlener merged commit 1a678f5 into AIDASoft:master Dec 2, 2025
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Undeclared identifiers when including data model headers in cling

3 participants