From 69f81e5aebc262c4dc3229914c0f67b8b19b3f57 Mon Sep 17 00:00:00 2001 From: Paul Baksic <30337881+bakpaul@users.noreply.github.com> Date: Wed, 13 Mar 2024 16:59:31 +0100 Subject: [PATCH 1/9] fix Versioning to allow patches (#404) --- CMakeLists.txt | 3 +++ SofaPython3Config.cmake.in | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index a388fc9e..3774e67a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -109,6 +109,7 @@ if(EXISTS "${Python_INCLUDE_DIR}") elseif(EXISTS "${Python_INCLUDE_DIRS}") set(PYTHON_INCLUDE_DIR "${Python_INCLUDE_DIRS}" CACHE INTERNAL "" FORCE) endif() +string(REGEX MATCH "[0-9]+\.[0-9]+" PythonMAJMIN "${Python_VERSION}") # Set the minimum pybind11 version to 2.3 (before that the pybind11::embed target did not exist) find_package(pybind11 2.3 CONFIG QUIET REQUIRED) @@ -145,6 +146,8 @@ message(STATUS "Python: Libraries: ${Python_LIBRARIES} User site: ${PYTHON_USER_SITE}" ) + + message(STATUS "pybind11: Version: ${pybind11_VERSION} Config: ${pybind11_CONFIG}" diff --git a/SofaPython3Config.cmake.in b/SofaPython3Config.cmake.in index b8b94de3..2c3410a4 100644 --- a/SofaPython3Config.cmake.in +++ b/SofaPython3Config.cmake.in @@ -10,7 +10,7 @@ include(SofaPython3Tools) # Find Python3 if(NOT Python_FOUND) - find_package(Python @Python_VERSION@ QUIET REQUIRED COMPONENTS Interpreter Development) + find_package(Python @PythonMAJMIN@ EXACT QUIET REQUIRED COMPONENTS Interpreter Development) endif() # Find pybind11 From a390afdcc5edad2bfbfb1f9507d6e3a68035b2df Mon Sep 17 00:00:00 2001 From: Paul Baksic <30337881+bakpaul@users.noreply.github.com> Date: Wed, 6 Mar 2024 12:47:39 +0100 Subject: [PATCH 2/9] Update ci.yml (#402) * Update ci.yml * Update ci.yml * Update ci.yml * Add pybind_DIR to CMAKE_PREFIX_PATH --- .github/workflows/ci.yml | 23 ++++------------------- 1 file changed, 4 insertions(+), 19 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a349fbeb..3b8a0a6a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -18,29 +18,14 @@ jobs: strategy: fail-fast: false matrix: - os: [ubuntu-20.04, macos-11, windows-2019] + os: [ubuntu-22.04, macos-13, windows-2022] sofa_branch: [master] - python_version: ['3.8'] + python_version: ['3.10'] steps: - - name: (Mac) Workaround for homebrew - shell: bash - if: runner.os == 'macOS' - run: | - rm -f /usr/local/bin/2to3 - rm -f /usr/local/bin/idle3 - rm -f /usr/local/bin/pydoc3 - rm -f /usr/local/bin/python3 - rm -f /usr/local/bin/python3-config - rm -f /usr/local/bin/2to3-3.11 - rm -f /usr/local/bin/idle3.11 - rm -f /usr/local/bin/pydoc3.11 - rm -f /usr/local/bin/python3.11 - rm -f /usr/local/bin/python3.11-config - - name: Setup SOFA and environment id: sofa - uses: sofa-framework/sofa-setup-action@v4 + uses: sofa-framework/sofa-setup-action@v5 with: sofa_root: ${{ github.workspace }}/sofa sofa_version: ${{ matrix.sofa_branch }} @@ -66,7 +51,7 @@ jobs: cmake_options="-GNinja \ -DCMAKE_BUILD_TYPE=Release \ -DCMAKE_INSTALL_PREFIX="$WORKSPACE_INSTALL_PATH" \ - -DCMAKE_PREFIX_PATH="$SOFA_ROOT/lib/cmake" \ + -DCMAKE_PREFIX_PATH="$SOFA_ROOT/lib/cmake:$pybind11_DIR" \ -DPYTHON_ROOT=$PYTHON_ROOT -DPython_ROOT=$PYTHON_ROOT \ -DPYTHON_EXECUTABLE=$PYTHON_EXE -DPython_EXECUTABLE=$PYTHON_EXE" if [ -e "$(command -v ccache)" ]; then From 2b02599571831dbfa7e93a551d9e2573a8ab876a Mon Sep 17 00:00:00 2001 From: bakpaul Date: Tue, 26 Mar 2024 16:33:17 +0100 Subject: [PATCH 3/9] Update ci script branch name --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3b8a0a6a..95675a7b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -19,7 +19,7 @@ jobs: fail-fast: false matrix: os: [ubuntu-22.04, macos-13, windows-2022] - sofa_branch: [master] + sofa_branch: [v23.12] python_version: ['3.10'] steps: From fd199b16dff8fb26747d59a00b1fbc27bff2912a Mon Sep 17 00:00:00 2001 From: Paul Baksic Date: Thu, 28 Mar 2024 20:34:13 +0100 Subject: [PATCH 4/9] Update ci script to generate release for tags --- .github/workflows/ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 95675a7b..8796c845 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -83,7 +83,7 @@ jobs: if [[ "${{ github.event.inputs.is_nightly }}" == "true" ]]; then ARTIFACT_VERSION="${ARTIFACT_VERSION}-nightly" fi - ARTIFACT_NAME="${PROJECT_NAME}_${ARTIFACT_VERSION}_python-${{ matrix.python_version }}_for-SOFA-${{ matrix.sofa_branch }}_${{ runner.os }}" + ARTIFACT_NAME="${PROJECT_NAME}_${ARTIFACT_VERSION}_python-${{ matrix.python_version }}_for-SOFA-${{ steps.sofa.outputs.sofa_version }}_${{ runner.os }}" echo "ARTIFACT_NAME=$ARTIFACT_NAME" | tee -a $GITHUB_ENV - name: Create artifact @@ -162,7 +162,7 @@ jobs: deploy: name: Deploy artifacts - if: always() && startsWith(github.ref, 'refs/heads/') # we are on a branch (not a PR) + if: always() && startsWith(github.repository, 'sofa-framework') && (startsWith(github.ref, 'refs/heads/') || startsWith(github.ref, 'refs/tags/')) # we are not on a fork and on a branch or a tag (not a PR) needs: [build-and-test] runs-on: ubuntu-latest continue-on-error: true From 724b49f1f93c0834d93ffb5c9b2abc4fcc887672 Mon Sep 17 00:00:00 2001 From: Olivier Roussel Date: Tue, 9 Jan 2024 10:22:34 +0100 Subject: [PATCH 5/9] Fix pybind add module segv on macos. --- CMake/SofaPython3Tools.cmake | 37 +----------------------------------- 1 file changed, 1 insertion(+), 36 deletions(-) diff --git a/CMake/SofaPython3Tools.cmake b/CMake/SofaPython3Tools.cmake index aa9d5878..7099de86 100644 --- a/CMake/SofaPython3Tools.cmake +++ b/CMake/SofaPython3Tools.cmake @@ -114,44 +114,9 @@ function(SP3_add_python_module) find_package(pybind11 CONFIG QUIET REQUIRED) - # We are doing manually what's usually done with pybind11_add_module(${A_TARGET} SHARED "${A_SOURCES}") - # since we got some problems on MacOS using recent versions of pybind11 where the SHARED argument wasn't taken - # into account - python_add_library(${A_TARGET} SHARED "${A_SOURCES}") + pybind11_add_module(${A_TARGET} SHARED "${A_SOURCES}") add_library(SofaPython3::${A_TARGET} ALIAS ${A_TARGET}) - if ("${pybind11_VERSION}" VERSION_GREATER_EQUAL "2.6.0") - target_link_libraries(${A_TARGET} PRIVATE pybind11::headers) - target_link_libraries(${A_TARGET} PRIVATE pybind11::embed) - target_link_libraries(${A_TARGET} PRIVATE pybind11::lto) - if(MSVC) - target_link_libraries(${A_TARGET} PRIVATE pybind11::windows_extras) - endif() - - pybind11_extension(${A_TARGET}) - pybind11_strip(${A_TARGET}) - else() - target_link_libraries(${A_TARGET} PRIVATE pybind11::module) - - # Equivalent to pybind11_extension(${A_TARGET}) which doesn't exists on pybind11 versions < 5 - set_target_properties(${A_TARGET} PROPERTIES PREFIX "" SUFFIX "${PYTHON_MODULE_EXTENSION}") - - if(NOT MSVC AND NOT ${CMAKE_BUILD_TYPE} MATCHES Debug|RelWithDebInfo) - # Equivalent to pybind11_strip(${A_TARGET}) which doesn't exists on pybind11 versions < 5 - # Strip unnecessary sections of the binary on Linux/macOS - if(CMAKE_STRIP) - if(APPLE) - set(x_opt -x) - endif() - - add_custom_command( - TARGET ${A_TARGET} - POST_BUILD - COMMAND ${CMAKE_STRIP} ${x_opt} $) - endif() - endif() - endif() - set_target_properties(${A_TARGET} PROPERTIES CXX_VISIBILITY_PRESET "hidden" From 6837b10203fdedac382e7a437c7fb8706cf91826 Mon Sep 17 00:00:00 2001 From: Olivier Roussel Date: Thu, 11 Jan 2024 15:11:09 +0100 Subject: [PATCH 6/9] Do not look for python (handled by pybind & invalid links on macOS) --- CMakeLists.txt | 39 -------------------------------------- SofaPython3Config.cmake.in | 37 ------------------------------------ 2 files changed, 76 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 3774e67a..4902809e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -82,35 +82,6 @@ set_property(GLOBAL PROPERTY USE_FOLDERS ON) # Set the minimum python version to 3.7 set(PYBIND11_PYTHON_VERSION 3.7) -# Find Python3 -find_package(Python ${PYBIND11_PYTHON_VERSION} COMPONENTS Interpreter Development REQUIRED) - -# Save PYTHON_* vars -set(PYTHON_VERSION_RESET "${PYTHON_VERSION}") -set(PYTHON_EXECUTABLE_RESET "${PYTHON_EXECUTABLE}") -set(PYTHON_LIBRARIES_RESET "${PYTHON_LIBRARIES}") -set(PYTHON_INCLUDE_DIRS_RESET "${PYTHON_INCLUDE_DIRS}") -set(PYTHON_LIBRARY_RESET "${PYTHON_LIBRARY}") -set(PYTHON_INCLUDE_DIR_RESET "${PYTHON_INCLUDE_DIR}") - -# Change PYTHON_* vars before pybind11 find_package -# to be sure that pybind11 relies on the right Python version -set(PYTHON_VERSION "${Python_VERSION}" CACHE STRING "" FORCE) -set(PYTHON_EXECUTABLE "${Python_EXECUTABLE}" CACHE FILEPATH "" FORCE) -set(PYTHON_LIBRARIES "${Python_LIBRARIES}" CACHE STRING "" FORCE) -set(PYTHON_INCLUDE_DIRS "${Python_INCLUDE_DIRS}" CACHE STRING "" FORCE) -if(EXISTS "${Python_LIBRARY}") - set(PYTHON_LIBRARY "${Python_LIBRARY}" CACHE INTERNAL "" FORCE) -elseif(EXISTS "${Python_LIBRARIES}") - set(PYTHON_LIBRARY "${Python_LIBRARIES}" CACHE INTERNAL "" FORCE) -endif() -if(EXISTS "${Python_INCLUDE_DIR}") - set(PYTHON_INCLUDE_DIR "${Python_INCLUDE_DIR}" CACHE INTERNAL "" FORCE) -elseif(EXISTS "${Python_INCLUDE_DIRS}") - set(PYTHON_INCLUDE_DIR "${Python_INCLUDE_DIRS}" CACHE INTERNAL "" FORCE) -endif() -string(REGEX MATCH "[0-9]+\.[0-9]+" PythonMAJMIN "${Python_VERSION}") - # Set the minimum pybind11 version to 2.3 (before that the pybind11::embed target did not exist) find_package(pybind11 2.3 CONFIG QUIET REQUIRED) @@ -146,8 +117,6 @@ message(STATUS "Python: Libraries: ${Python_LIBRARIES} User site: ${PYTHON_USER_SITE}" ) - - message(STATUS "pybind11: Version: ${pybind11_VERSION} Config: ${pybind11_CONFIG}" @@ -246,11 +215,3 @@ if (SP3_LINK_TO_USER_SITE AND SP3_PYTHON_PACKAGES_LINK_DIRECTORY) endif() endforeach() endif() - -# Reset PYTHON_* vars -set(PYTHON_VERSION "${PYTHON_VERSION_RESET}" CACHE STRING "" FORCE) -set(PYTHON_EXECUTABLE "${PYTHON_EXECUTABLE_RESET}" CACHE FILEPATH "" FORCE) -set(PYTHON_LIBRARIES "${PYTHON_LIBRARIES_RESET}" CACHE STRING "" FORCE) -set(PYTHON_INCLUDE_DIRS "${PYTHON_INCLUDE_DIRS_RESET}" CACHE STRING "" FORCE) -set(PYTHON_LIBRARY "${PYTHON_LIBRARY_RESET}" CACHE INTERNAL "" FORCE) -set(PYTHON_INCLUDE_DIR "${PYTHON_INCLUDE_DIR_RESET}" CACHE INTERNAL "" FORCE) diff --git a/SofaPython3Config.cmake.in b/SofaPython3Config.cmake.in index 2c3410a4..89221331 100644 --- a/SofaPython3Config.cmake.in +++ b/SofaPython3Config.cmake.in @@ -8,47 +8,10 @@ set(SP3_PYTHON_PACKAGES_DIRECTORY @SP3_PYTHON_PACKAGES_DIRECTORY@) list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_LIST_DIR}") include(SofaPython3Tools) -# Find Python3 -if(NOT Python_FOUND) - find_package(Python @PythonMAJMIN@ EXACT QUIET REQUIRED COMPONENTS Interpreter Development) -endif() # Find pybind11 if(NOT pybind11_FOUND) - # Save PYTHON_* vars - set(PYTHON_VERSION_RESET "${PYTHON_VERSION}") - set(PYTHON_EXECUTABLE_RESET "${PYTHON_EXECUTABLE}") - set(PYTHON_LIBRARIES_RESET "${PYTHON_LIBRARIES}") - set(PYTHON_INCLUDE_DIRS_RESET "${PYTHON_INCLUDE_DIRS}") - set(PYTHON_LIBRARY_RESET "${PYTHON_LIBRARY}") - set(PYTHON_INCLUDE_DIR_RESET "${PYTHON_INCLUDE_DIR}") - - # Change PYTHON_* vars before pybind11 find_package - # to be sure that pybind11 relies on the right Python version - set(PYTHON_VERSION "${Python_VERSION}" CACHE STRING "" FORCE) - set(PYTHON_EXECUTABLE "${Python_EXECUTABLE}" CACHE FILEPATH "" FORCE) - set(PYTHON_LIBRARIES "${Python_LIBRARIES}" CACHE STRING "" FORCE) - set(PYTHON_INCLUDE_DIRS "${Python_INCLUDE_DIRS}" CACHE STRING "" FORCE) - if(EXISTS "${Python_LIBRARY}") - set(PYTHON_LIBRARY "${Python_LIBRARY}" CACHE INTERNAL "" FORCE) - elseif(EXISTS "${Python_LIBRARIES}") - set(PYTHON_LIBRARY "${Python_LIBRARIES}" CACHE INTERNAL "" FORCE) - endif() - if(EXISTS "${Python_INCLUDE_DIR}") - set(PYTHON_INCLUDE_DIR "${Python_INCLUDE_DIR}" CACHE INTERNAL "" FORCE) - elseif(EXISTS "${Python_INCLUDE_DIRS}") - set(PYTHON_INCLUDE_DIR "${Python_INCLUDE_DIRS}" CACHE INTERNAL "" FORCE) - endif() - find_package(pybind11 @pybind11_VERSION@ QUIET REQUIRED CONFIG) - - # Reset PYTHON_* vars - set(PYTHON_VERSION "${PYTHON_VERSION_RESET}" CACHE STRING "" FORCE) - set(PYTHON_EXECUTABLE "${PYTHON_EXECUTABLE_RESET}" CACHE FILEPATH "" FORCE) - set(PYTHON_LIBRARIES "${PYTHON_LIBRARIES_RESET}" CACHE STRING "" FORCE) - set(PYTHON_INCLUDE_DIRS "${PYTHON_INCLUDE_DIRS_RESET}" CACHE STRING "" FORCE) - set(PYTHON_LIBRARY "${PYTHON_LIBRARY_RESET}" CACHE INTERNAL "" FORCE) - set(PYTHON_INCLUDE_DIR "${PYTHON_INCLUDE_DIR_RESET}" CACHE INTERNAL "" FORCE) endif() # Find SofaPython3::XXXXX From 624fb11c5e2556cd539d348e6477d6dcc71be50a Mon Sep 17 00:00:00 2001 From: Olivier Roussel Date: Thu, 11 Jan 2024 17:10:43 +0100 Subject: [PATCH 7/9] Avoid python lib linking propation on macOS (segv w/ conda) --- Plugin/CMakeLists.txt | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Plugin/CMakeLists.txt b/Plugin/CMakeLists.txt index f53e987a..549865df 100644 --- a/Plugin/CMakeLists.txt +++ b/Plugin/CMakeLists.txt @@ -35,7 +35,12 @@ add_library(SofaPython3::${PROJECT_NAME} ALIAS ${PROJECT_NAME}) target_compile_definitions(${PROJECT_NAME} PRIVATE "-DSOFA_BUILD_SOFAPYTHON3") target_link_libraries(${PROJECT_NAME} PUBLIC Sofa.Simulation.Graph) -target_link_libraries(${PROJECT_NAME} PUBLIC pybind11::module pybind11::embed) + +if(CMAKE_SYSTEM_NAME STREQUAL Darwin) + target_link_libraries(${PROJECT_NAME} PRIVATE pybind11::embed) +else() + target_link_libraries(${PROJECT_NAME} PUBLIC pybind11::embed) +endif() set_target_properties(${PROJECT_NAME} PROPERTIES OUTPUT_NAME SofaPython3) From 28b539bf50e8034ea572176458bb75e3cf388929 Mon Sep 17 00:00:00 2001 From: Olivier Roussel Date: Thu, 11 Jan 2024 17:32:13 +0100 Subject: [PATCH 8/9] Fix sofapython3 tests linking w/ python. --- Plugin/CMakeLists.txt | 5 +++++ Testing/CMakeLists.txt | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/Plugin/CMakeLists.txt b/Plugin/CMakeLists.txt index 549865df..97c4d386 100644 --- a/Plugin/CMakeLists.txt +++ b/Plugin/CMakeLists.txt @@ -36,6 +36,11 @@ target_compile_definitions(${PROJECT_NAME} PRIVATE "-DSOFA_BUILD_SOFAPYTHON3") target_link_libraries(${PROJECT_NAME} PUBLIC Sofa.Simulation.Graph) +# The public linking with pybind lead to have a link with libpython which +# propagates in the python module .so. On macOS, this extra link with libpython +# lead to segv when importing the python module in versions of python that don't +# have a dynamic link with libpython (such as the one provided by conda), but works +# fine with versions that have such link. if(CMAKE_SYSTEM_NAME STREQUAL Darwin) target_link_libraries(${PROJECT_NAME} PRIVATE pybind11::embed) else() diff --git a/Testing/CMakeLists.txt b/Testing/CMakeLists.txt index 193e3a19..a44d93a4 100644 --- a/Testing/CMakeLists.txt +++ b/Testing/CMakeLists.txt @@ -16,6 +16,11 @@ set(SOURCE_FILES add_library(${PROJECT_NAME} SHARED ${HEADER_FILES} ${SOURCE_FILES}) target_link_libraries(${PROJECT_NAME} PUBLIC Sofa.Testing SofaPython3::Plugin) +# We wan't the pybind11 dependency to propagate to SofaPython3Testing consumers, +# here Bindings.*.Tests +if(CMAKE_SYSTEM_NAME STREQUAL Darwin) + target_link_libraries(${PROJECT_NAME} PUBLIC pybind11::embed) +endif() target_compile_definitions(${PROJECT_NAME} PRIVATE "-DSOFA_BUILD_SOFAPYTHON3") set_target_properties(${PROJECT_NAME} PROPERTIES FOLDER Testing) From 8cd3a9b9e5fecccafd2447cfdebe74861fbd8b5b Mon Sep 17 00:00:00 2001 From: Olivier Roussel Date: Mon, 22 Jan 2024 12:28:51 +0100 Subject: [PATCH 9/9] Do not link with pybind11 embed for libSofaPython on macOS --- Plugin/CMakeLists.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Plugin/CMakeLists.txt b/Plugin/CMakeLists.txt index 97c4d386..7cd08e49 100644 --- a/Plugin/CMakeLists.txt +++ b/Plugin/CMakeLists.txt @@ -39,10 +39,10 @@ target_link_libraries(${PROJECT_NAME} PUBLIC Sofa.Simulation.Graph) # The public linking with pybind lead to have a link with libpython which # propagates in the python module .so. On macOS, this extra link with libpython # lead to segv when importing the python module in versions of python that don't -# have a dynamic link with libpython (such as the one provided by conda), but works -# fine with versions that have such link. +# have a dynamic link with libpython (such as the one provided by conda which is linked +# statically), but works fine with versions that have such link. if(CMAKE_SYSTEM_NAME STREQUAL Darwin) - target_link_libraries(${PROJECT_NAME} PRIVATE pybind11::embed) + target_link_libraries(${PROJECT_NAME} PRIVATE pybind11::pybind11 pybind11::python_link_helper) else() target_link_libraries(${PROJECT_NAME} PUBLIC pybind11::embed) endif()