-
Notifications
You must be signed in to change notification settings - Fork 522
podio, edm4hep, dd4hep: conflicts ^python +freethreading #2907
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
Conversation
Add conflict for python free-threading requirement.
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.
Pull request overview
This PR adds explicit conflicts between the podio and edm4hep packages and Python with free-threading enabled (^python +freethreading). These conflicts prevent incompatible combinations until future releases of these packages add support for Python free-threading.
- Adds conflict declaration for
podio@:1.6with^python +freethreading, requiring version 1.7 or later - Adds conflict declaration for
edm4hep@:0.99.4with^python +freethreading, requiring version 0.99.5 or later
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| repos/spack_repo/builtin/packages/podio/package.py | Adds conflict preventing podio versions up to 1.6 from being used with Python free-threading |
| repos/spack_repo/builtin/packages/edm4hep/package.py | Adds conflict preventing edm4hep versions up to 0.99.4 from being used with Python free-threading |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Reformat conflict statement for better readability.
tmadlener
left a comment
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.
This comes a bit too late for now, but I think all the conflicts could be removed and replaced by explicitly injecting the python prefix via cmake, all the packages have corresponding cmake variables that should allow for this
-podio_PYTHON_INSTALLDIR
EDM4HEP_PYTHON_INSTALLDIRDD4HEP_PYTHON_INSTALL_DIR
All of them are populated to some more (or less as we have seen) reasonable default, but they can all be either specified as relative (to the CMAKE_INSTALL_PREFIX) or absolute paths and the corresponding python files will end up there.
True, but determining the target directory from spack is not trivial either and requires spinning up a subprocess of the target python (since the python being used by spack itself is not the relevant one). That complexity is avoided with a conflict that only restricts until the next versions are released. |
This PR marks explicit conflicts between
podio,edm4hep, anddd4hepand^python +freethreading. To be resolved in next releases (AIDASoft/podio#911, key4hep/EDM4hep#469, AIDASoft/DD4hep#1547).