-
Notifications
You must be signed in to change notification settings - Fork 178
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
fix: update the DD4hep requirements when the ODD is built #3330
fix: update the DD4hep requirements when the ODD is built #3330
Conversation
|
Maybe we should add this requirement on the ODD side? That way, the ODD build internally will refuse to proceed when the versions don't match up. |
That is actually a much better way to do it indeed. I will see what I can do that way tomorrow. We will also probably need to update the images in the CI. Since we have been using DD4Hep v1.25, we are ignoring part of the ODD in the physmon. |
CMakeLists.txt
Outdated
@@ -201,6 +201,7 @@ set(_acts_actsvg_version 0.4.40) | |||
set(_acts_autodiff_version 0.6) | |||
set(_acts_boost_version 1.71.0) | |||
set(_acts_dd4hep_version 1.21) | |||
set(_acts_dd4hep4ODD_version 1.27) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could it make sense to set the DD4hep version to 1.28
? Version 1.27
is incompatible with python 3.12
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I am doing now is directly putting the min version in the ODD (see : https://gitlab.cern.ch/acts/OpenDataDetector/-/merge_requests/92) I will then bump the ODD version in this PR and remove the extra dd4hep requirement. I think the CI images we use have at most 1.27 so I would prefer to keep the CI in line with the min version to avoid unexpected issue
In this PR, I introduce an extra constraint on the DD4HEP version when building the ODD (v1.27).
I recently discovered that when trying to run the full chain with the ODD and an older version of the DD4Hep, a parsing error occurs when trying to retrieve the
OpenDataDetectorPlugins.xml
file (it looks in the current directory instead of the ODD XML one). This behaviour was fixed in this DD4Hep PR: AIDASoft/DD4hep#1181 added to release 1.27.For now, I have added a separate minimum version for when people want to build the ODD to avoid affecting users that don’t use it but use DD4Hep for their own detector, but if people prefer, I could also bump ``_acts_dd4hep_version` directly.