scikit-build / scikit-build-core

A next generation Python CMake adaptor and Python API for plugins
https://scikit-build-core.readthedocs.io
Apache License 2.0
223 stars 47 forks source link

CMake design: How to make vanilla CMake installs play nice with python packaging #856

Open LecrisUT opened 1 month ago

LecrisUT commented 1 month ago

I have started to work on https://github.com/spglib/spglib/pull/520, where I redesign the build process to be more compatible with audit-wheel and make things simpler overall. The problems that I am trying to tackle there:

That design works relatively well but I am hitting 2 issues:

henryiii commented 4 weeks ago

CMake strips RPaths that are outside the install tree. You probably need:

set(CMAKE_INSTALL_RPATH_USE_LINK_PATH ON)

That will leave the proper $ORIGIN rpaths and fix the issue above for macOS. I see Windows mentioned at https://cmake.org/cmake/help/latest/prop_tgt/INSTALL_RPATH.html, so worth trying this on Windows too.

LecrisUT commented 4 weeks ago

CMAKE_INSTALL_RPATH_USE_LINK_PATH relates to out-of-tree dependencies and it doesn't apply to the refactored code. Otherwise I am using INSTALL_RPATH and it works on linux and in the macos issue can see that it is still there in Search path error message. Does macos not support $origin? One clue I've found is a commit that changed it to @loader_path.

For windows it points to TARGET_RUNTIME_DLLS genex, which might work for install(FILES) I guess.

burgholzer commented 4 weeks ago

CMAKE_INSTALL_RPATH_USE_LINK_PATH relates to out-of-tree dependencies and it doesn't apply to the refactored code. Otherwise I am using INSTALL_RPATH and it works on linux and in the macos issue can see that it is still there in Search path error message. Does macos not support $origin? One clue I've found is a commit that changed it to @loader_path.

For windows it points to TARGET_RUNTIME_DLLS genex, which might work for install(FILES) I guess.

In our project (https://github.com/mqt-core) we use the following snippet


if(APPLE)
    set(BASEPOINT @loader_path)
else()
    set(BASEPOINT $ORIGIN)
endif()
set(CMAKE_INSTALL_RPATH ${BASEPOINT} ${BASEPOINT}/${CMAKE_INSTALL_LIBDIR})
set(CMAKE_INSTALL_RPATH_USE_LINK_PATH TRUE)
set(CMAKE_BUILD_WITH_INSTALL_RPATH TRUE)

The BASEPOINT is exactly for what you are alluding to above. CMAKE_BUILD_WITH_INSTALL_RPATH avoids a code signing issue on macOS when installing the libraries as part of the wheel.

As for the windows problems: you can emulate something similar to what delvewheel is doing and patch your __init__.py


import sys

# under Windows, make sure to add the appropriate DLL directory to the PATH
if sys.platform == "win32":

    def _dll_patch() -> None:
        """Add the DLL directory to the PATH."""
        import os
        import sysconfig
        from pathlib import Path

        bin_dir = Path(sysconfig.get_paths()["purelib"]) / "mqt" / "core" / "bin"
        os.add_dll_directory(str(bin_dir))

    _dll_patch()
    del _dll_patch

At least that's our current way of trying to make it work in https://github.com/cda-tum/mqt-core/pull/662

LecrisUT commented 4 weeks ago

Awesome, thanks for sharing that. Let me try them out and I'll share my version as well for reference

LecrisUT commented 4 weeks ago

Ok, here's my version which addresses various quirks. Some parts are due to having the C library be the main CMake recipe, but all sub-projects are independently buildable.

```cmake # Try to find system installed library, otherwise use the bundled version if (NOT TARGET Spglib::symspg) find_package(Spglib CONFIG) if (NOT Spglib_FOUND) message(STATUS "Using bundled spglib sources") add_subdirectory(${PROJECT_SOURCE_DIR}/.. _deps/spglib-build) endif () endif () # Set appropriate RPATH. If it's using the system one, it should point to empty folders and not have any effect set_target_properties(Spglib_python PROPERTIES INSTALL_RPATH "$,@loader_path,$ORIGIN>/${CMAKE_INSTALL_LIBDIR}" ) # Make the python package runnable within the build tree file(COPY spglib DESTINATION ${CMAKE_CURRENT_BINARY_DIR}/ ) add_custom_command(TARGET Spglib_python POST_BUILD COMMAND ${CMAKE_COMMAND} -E create_symlink $ ${CMAKE_CURRENT_BINARY_DIR}/spglib/$ COMMAND ${CMAKE_COMMAND} -E copy -t ${CMAKE_CURRENT_BINARY_DIR}/spglib/ $ COMMAND_EXPAND_LISTS ) # Installation stuff using `wheel.install-dir="spglib"` for the `.` destination if (NOT SKBUILD AND SPGLIB_INSTALL) message(WARNING "Installing the python bindings outside of scikit-build-core environment is not supported.") elseif (SPGLIB_INSTALL) if (TARGET Spglib_symspg) # For windows systems we need to also install a copy of the dll files, this has no effect otherwise install(TARGETS Spglib_symspg RUNTIME DESTINATION . COMPONENT Spglib_Runtime ) endif () install(TARGETS Spglib_python LIBRARY DESTINATION . COMPONENT Spglib_Runtime ) endif () ```

The last part I'm not fond of because it creates a copy of the .dll file instead of making a symlink. Another caveat is that if the C/C++ library has other external dependencies, it would need RUNTIME_DEPENDENCIES added, but I couldn't find a way to filter out the default system ones, e.g. libc