Skip to content

Conversation

@vepadulano
Copy link
Member

No description provided.

@vepadulano vepadulano self-assigned this Jul 17, 2025
@vepadulano vepadulano added the skip ci Skip the full builds on the actions runners label Jul 17, 2025
# -- 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")
Copy link
Contributor

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"
Copy link
Contributor

@guitargeek guitargeek Jul 17, 2025

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
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"-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.
@vepadulano vepadulano force-pushed the experimental-pip-install-root branch from 71d4f3c to f2e88ac Compare July 17, 2025 11:46
@vepadulano vepadulano closed this Jul 17, 2025
@vepadulano vepadulano reopened this Jul 17, 2025
@vepadulano vepadulano closed this Jul 17, 2025
@vepadulano vepadulano reopened this Jul 17, 2025
@vepadulano
Copy link
Member Author

pull_request events do not trigger the workflow unless the workflow file is already in master. Closing the PR to avoid too much noise while the workflow is being worked upon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip ci Skip the full builds on the actions runners

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants