-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Test Python wheel build github workflow #19391
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
| # -- ROOT/ | ||
| # With most of the ROOT libraries being stored in `ROOT/lib`. The RPATH must then be updated for the CPython extension | ||
| # shared libraries. | ||
| set_target_properties(${libname} PROPERTIES INSTALL_RPATH "${CMAKE_INSTALL_RPATH}:$ORIGIN/ROOT/lib") |
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.
I think all of these hardcoded rpaths should now not be necessary if CMAKE_INSTALL_PYTHONDIR and CMAKE_INSTALL_LIBDIR are set correctly, at least relative to each other.
There are still some quirks with unneeded artifacts when these directories are not identical, which I'm trying to figure out:
#19044
But that should not be a problem for the functionality.
| BUILD_RPATH "$ORIGIN/${pymoduledir_to_libdir_build}" | ||
| INSTALL_RPATH "$ORIGIN/${pymoduledir_to_libdir_install}" | ||
| # ATTENTION: NEXT PART IS CRUCIAL SEE COMMENT ABOVE | ||
| INSTALL_RPATH "$ORIGIN/${pymoduledir_to_libdir_install}:${CMAKE_INSTALL_RPATH}:$ORIGIN/lib" |
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.
Here you actually see the new mechanism that I introduced so make arbitrary CMAKE_INSTALL_PYTHONDIR and CMAKE_INSTALL_LIBDIR work. Could you try -DCMAKE_INSTALL_PYTHONDIR=site-packages/ and -DCMAKE_INSTALL_LIBDIR=site-packages/ROOT/lib?
CMAKE_INSTALL_BINDIR also needs to be set to site-packages/ROOT/bin then, so that the relative rpaths between lib and bin are not broken.
To summarize: I think if you avoid having the move around site-packages/, site-packages/ROOT/bin and site-packages/ROOT/lib relative to each other manually after the install but instead use the right CMake flags, it should work out of the box.
| mode_opts = shlex.split( | ||
| "-Dbuiltin_nlohmannjson=ON -Dbuiltin_tbb=ON -Dbuiltin_xrootd=ON " # builtins | ||
| "-Dbuiltin_lz4=ON -Dbuiltin_lzma=ON -Dbuiltin_zstd=ON -Dbuiltin_xxhash=ON" # builtins | ||
| "-Druntime_cxxmodules=ON -Drpath=ON -Dfail-on-missing=ON " # Generic build configuration |
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.
| "-Druntime_cxxmodules=ON -Drpath=ON -Dfail-on-missing=ON " # Generic build configuration | |
| "-Druntime_cxxmodules=ON -Drpath=ON -Dfail-on-missing=ON -Dthisroot_scripts=OFF" # Generic build configuration |
We don't need the thisroot scripts (in fact they would be broken, because they assume CMAKE_INSTALL_PYTHONDIR equals CMAKE_INSTALL_LIBDIR), and they can be excluded from the build now:
Removing the need to link against libPython is a requirement for manylinux. A run of `cibuildwheel` with this configuration produces a wheel, which can be reinstalled on another Python virtual environment in the same machine. The problem is that the wheel is recognized as a pure Python wheel and the C++ libraries are just copied verbatim over. We need to understand how to build according to the Python version.
Apparently just declaring a C extension module in the setuptools setup function is enough. Even if that extension has nothing to do with the project and doesn't exist anywhere.
See pypa/distutils#284 , the issue is marked as solved but it is not really.
71d4f3c to
f2e88ac
Compare
|
|
No description provided.