Skip to content
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

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

Open
LecrisUT opened this issue Aug 12, 2024 · 5 comments

Comments

@LecrisUT
Copy link
Collaborator

LecrisUT commented Aug 12, 2024

I have started to work on spglib/spglib#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:

  • Limit python binding installation to only pip install environment (too fragile to allow standalone cmake --install without populating .dist-info metadata)
  • Switch to using INSTALL_RPATH on the python bindings file pointing to $ORIGIN/${CMAKE_INSTALL_LIBDIR}
  • Install all files in a folder structure compliant with GNUInstallDirs (except for the python bindings which point to .), with the install prefix pointing to the python package
  • Make the configuration as close to a vanilla CMake project as possible

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

  • For windows environments, there is no INSTALL_RPATH equivalent, and usually it relies on all the .dll files being in the same folder, but this conflicts with GNUInstallDirs where it would put them in bin, but on the other hand, we need the python bindings to be under the path dictated by import ....
    Is there a way to reconcile these? I was thinking maybe putting the python bindings under the GNUInstallDirs structure as well, but that is not predictable since it would be ${CMAKE_INSTALL_LIBDIR} on unix, but ${CMAKE_INSTALL_BINDIR} on windows
  • macos repair-wheel commands are failing with rather cryptic issue:
      + delocate-wheel --require-archs arm64 -w /private/var/folders/m_/cksx93ys47x4621g0zbw_m4m0000gn/T/cibw-run-88asjxr1/cp311-macosx_arm64/repaired_wheel -v /private/var/folders/m_/cksx93ys47x4621g0zbw_m4m0000gn/T/cibw-run-88asjxr1/cp311-macosx_arm64/built_wheel/spglib-2.5.1.dev19+ged0599c-cp311-cp311-macosx_11_0_arm64.whl
    ERROR:delocate.libsana:
    @rpath/libsymspg.2.dylib not found:
      Needed by: /private/var/folders/m_/cksx93ys47x4621g0zbw_m4m0000gn/T/tmpzdzcou3j/wheel/spglib/_spglib.cpython-311-darwin.so
      Search path:
        $ORIGIN/lib
    ERROR:delocate.libsana:
    @rpath/libsymspg.2.dylib not found:
      Needed by: /private/var/folders/m_/cksx93ys47x4621g0zbw_m4m0000gn/T/tmpzdzcou3j/wheel/spglib/_spglib.cpython-311-darwin.so
      Search path:
        $ORIGIN/lib
    ERROR:delocate.libsana:@rpath/libsymspg.2.dylib not found
    
@henryiii
Copy link
Collaborator

henryiii commented Aug 13, 2024

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
Copy link
Collaborator Author

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

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 cda-tum/mqt-core#662

@LecrisUT
Copy link
Collaborator Author

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

@LecrisUT
Copy link
Collaborator Author

LecrisUT commented Aug 13, 2024

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.

# 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 "$<IF:$<BOOL:${APPLE}>,@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
        $<TARGET_FILE:Spglib_python>
        ${CMAKE_CURRENT_BINARY_DIR}/spglib/$<TARGET_FILE_NAME:Spglib_python>
        COMMAND ${CMAKE_COMMAND} -E copy -t ${CMAKE_CURRENT_BINARY_DIR}/spglib/ $<TARGET_RUNTIME_DLLS:Spglib_python>
        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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants