[cmake] Produce python .dist-info metadata#22688
Conversation
There was a problem hiding this comment.
Very nice work! I have a couple of comments:
- We would require a follow-up PR as the
try/exceptblock we added for thePackageNotFoundErrorshould in principle no longer be needed here thanks to your addition in this PR - I have a small doubt about how to handle the synchronization between the metadata in this
METADATA.intemplate and thepyproject.tomlsince it's hard coded here, say for exampleRequires-Distevolves to not only includenumpy, there is no mechanism that enforces applying changes in both files, thus metadata could quietly diverge, do we want to document that changes should be always applied to both files? Do we want to think of a way to generate the template by parsingpyproject.toml? ...
Test Results 22 files 22 suites 3d 11h 23m 51s ⏱️ For more details on these failures, see this check. Results for commit 6a5c4cc. ♻️ This comment has been updated with latest results. |
jase231
force-pushed
the
jserwe_provide_python_metadata
branch
4 times, most recently
from
8eebeba to
a05a820
Compare
jase231
force-pushed
the
jserwe_provide_python_metadata
branch
from
a05a820 to
6a5c4cc
Compare
vepadulano
left a comment
vepadulano
left a comment
There was a problem hiding this comment.
Thank you! A couple of suggestions for improvement before running the CI
|
|
||
| ROOT_ADD_PYUNITTEST(regression_18441 regression_18441.py) | ||
|
|
||
| ROOT_ADD_PYUNITTEST(package_metadata package_metadata.py PYTHON_DEPS importlib) |
There was a problem hiding this comment.
| ROOT_ADD_PYUNITTEST(package_metadata package_metadata.py PYTHON_DEPS importlib) | |
| ROOT_ADD_PYUNITTEST(package_metadata package_metadata.py) |
importlib is part of the standard Python libraries, so we don't need to signal the dependency explicitly since it will always be present with a Python installation
| @@ -0,0 +1,3 @@ | |||
| Metadata-Version: 2.2 | |||
| Name: root | |||
| Version: @ROOT_VERSION@ No newline at end of file | |||
| # See: https://packaging.python.org/en/latest/specifications/recording-installed-packages/ | ||
|
|
||
| # scikit-build-core handles metadata so only do this for non-wheel builds to avoid conflict | ||
| if(NOT _wheel_build) |
There was a problem hiding this comment.
Here we are missing more checks: pyroot option must be on, as well as Python3_FOUND which signals that there was actually a Python installation found for this build
3 participants
This Pull request:
Makes the ROOT Python import package compatible with
importlib.metadata's metadata system.Changes or fixes:
Adds a cmake module that places a METADATA and INSTALLER files inside a
root-<version>.dist-infofolder on thePYTHONPATH, which importlib.metadata can access to query e.g. distribution package version.This metadata is already present in the wheel distribution courtesy of the build backend, but is absent in all other ROOT distributions.
importlib.metadata.version()will throw an exception if called on a ROOT source build, but works just fine for wheels. This PR's change standardizes this behavior.See https://packaging.python.org/en/latest/specifications/recording-installed-packages/ for python's official spec.
Checklist: