Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add version file compatible with other Key4hep packages #152

Merged
merged 7 commits into from
May 3, 2023

Conversation

tmadlener
Copy link
Contributor

BEGINRELEASENOTES

  • Add a EDM4hepVersion.h file that has the same basic structure and functionality as other Key4hep packages.

ENDRELEASENOTES

@vvolkl vvolkl enabled auto-merge (squash) May 24, 2022 06:52
CMakeLists.txt Outdated Show resolved Hide resolved
@vvolkl vvolkl disabled auto-merge May 24, 2022 09:31
@vvolkl
Copy link
Collaborator

vvolkl commented May 24, 2022

@tmadlener Is there a way to write this version to the metadata as well? Would probably need to be done in podio or in the user code?

@tmadlener
Copy link
Contributor Author

For podio we are already writing the build version of podio. However, there is no "easy" way yet to do it for a generated datamodel like EDM4hep. What could be done at the moment is to write the 64 bit encoded unsigned as user data, or potentially also writing it as 3 ints into the meta data. None of these are ideal, I think and this should be done automatically without having to do it explicitly.

Since writing the version is necessary in any case for schema evolution, this has to be solved in any case rather soon. With that in mind, maybe the current header has to be considered as not completely stable yet.

@tmadlener tmadlener force-pushed the add-version branch 2 times, most recently from ae1f668 to 220d434 Compare January 12, 2023 13:56
@tmadlener
Copy link
Contributor Author

Merging this as it is at the moment, and decide how to deal with the schema version once that is actually available.

// necessary

/// Define a version to be used in edm4hep.
#define EDM4HEP_VERSION(major, minor, patch) (((unsigned long)(major) << 32) | ((unsigned long)(minor) << 16) | ((unsigned long)(patch)))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not work when used in the preprocessor. See AIDASoft/podio#374

EDM4hepVersion.h.in Outdated Show resolved Hide resolved
EDM4hepVersion.h.in Outdated Show resolved Hide resolved
EDM4hepVersion.h.in Show resolved Hide resolved
EDM4hepVersion.h.in Outdated Show resolved Hide resolved
@tmadlener
Copy link
Contributor Author

Meeting: Podio could generate this file (and add schema information into it)

@hegner
Copy link
Contributor

hegner commented Feb 28, 2023

I think that would be a slightly bigger thing to discuss. feature creep is a big risk there...

@tmadlener tmadlener enabled auto-merge (squash) May 3, 2023 07:44
@tmadlener tmadlener merged commit 1145e20 into key4hep:master May 3, 2023
wdconinc added a commit to eic/EICrecon that referenced this pull request Aug 15, 2023
wdconinc added a commit to eic/EICrecon that referenced this pull request Aug 16, 2023
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.

3 participants