From 5876499b3720136476a4736af7e66bcb604e580e Mon Sep 17 00:00:00 2001 From: Christian Rauch Date: Wed, 16 Oct 2024 22:49:02 +0200 Subject: [PATCH 01/13] simplify wheel file name formatting --- cmake/MakePythonWheel.cmake | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/cmake/MakePythonWheel.cmake b/cmake/MakePythonWheel.cmake index 8f6bcfdf0..26ec88c24 100644 --- a/cmake/MakePythonWheel.cmake +++ b/cmake/MakePythonWheel.cmake @@ -23,9 +23,9 @@ def wheel_name(**kwargs): # assemble wheel file name distname = bdist_wheel_cmd.wheel_dist_name tag = '-'.join(bdist_wheel_cmd.get_tag()) - return f'{distname};{tag}' + return f'{distname}-{tag}' -print(wheel_name(name='${python_module}', version='${version}', ext_modules=[Extension('dummy', ['summy.c'])])) +print(wheel_name(name='${python_module}', version='${version}', ext_modules=[Extension('dummy', ['dummy.c'])])) " OUTPUT_VARIABLE wheel_filename OUTPUT_STRIP_TRAILING_WHITESPACE @@ -33,14 +33,10 @@ print(wheel_name(name='${python_module}', version='${version}', ext_modules=[Ext ) if(NOT wheel_filename) message(STATUS "Python module `setuptools` required for correct wheel filename generation. Please install if needed.") - set(wheel_filename "unknown;unknown") + set(wheel_filename "unknown-unknown") endif() - list(GET wheel_filename 0 distname) - list(GET wheel_filename 1 platformtag) - set(complete_tag "${distname}-${platformtag}") - - set(wheel_filename "${CMAKE_BINARY_DIR}/${complete_tag}.whl") + set(wheel_filename "${CMAKE_BINARY_DIR}/${wheel_filename}.whl") set(wheel_distinfo "${CMAKE_BINARY_DIR}/${python_module}-${version}.dist-info") set(wheel_data "${CMAKE_BINARY_DIR}/${python_module}-${version}.data") set(wheel_generator_string "pango_wheelgen_${version}") From aa50cf180437f072eee9506d817174a371c4d184 Mon Sep 17 00:00:00 2001 From: Christian Rauch Date: Wed, 16 Oct 2024 23:15:10 +0200 Subject: [PATCH 02/13] install setuptools on macOS --- scripts/install_prerequisites.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/install_prerequisites.sh b/scripts/install_prerequisites.sh index 07c336c73..a0547afb1 100755 --- a/scripts/install_prerequisites.sh +++ b/scripts/install_prerequisites.sh @@ -159,7 +159,7 @@ elif [[ "$MANAGER" == "brew" ]]; then PKGS_OPTIONS+=(install) if ((VERBOSE > 0)); then PKGS_OPTIONS+=(--verbose); fi PKGS_REQUIRED+=(glew eigen cmake ninja) - PKGS_RECOMMENDED+=(libjpeg-turbo libpng openexr libtiff ffmpeg lz4 zstd catch2) + PKGS_RECOMMENDED+=(libjpeg-turbo libpng openexr libtiff ffmpeg lz4 zstd catch2 python-setuptools) # Brew doesn't have a dryrun option if ((DRYRUN > 0)); then MANAGER="echo $MANAGER" From 34dc5c79d06b3f21d4e19db498aaa763fdfa8626 Mon Sep 17 00:00:00 2001 From: Christian Rauch Date: Wed, 16 Oct 2024 22:49:51 +0200 Subject: [PATCH 03/13] make setuptools a hard requirement for MakeWheel macro --- cmake/MakePythonWheel.cmake | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/cmake/MakePythonWheel.cmake b/cmake/MakePythonWheel.cmake index 26ec88c24..7e2079b57 100644 --- a/cmake/MakePythonWheel.cmake +++ b/cmake/MakePythonWheel.cmake @@ -9,6 +9,11 @@ function( MakeWheel python_module) cmake_parse_arguments(MAKEWHEEL "PRINT_HELP" "MODULE;VERSION;SUMMARY;DESCRIPTION;HOMEPAGE;AUTHOR;EMAIL;LICENCE" "REQUIRES" ${ARGN} ) set(version ${MAKEWHEEL_VERSION}) + execute_process(COMMAND ${Python_EXECUTABLE} -c "import setuptools" ERROR_QUIET RESULT_VARIABLE has_setuptools) + if(has_setuptools EQUAL "1") + message(FATAL_ERROR "Python module `setuptools` required for correct wheel filename generation.") + endif() + execute_process( COMMAND ${Python_EXECUTABLE} -c " from setuptools.dist import Distribution @@ -31,10 +36,6 @@ print(wheel_name(name='${python_module}', version='${version}', ext_modules=[Ext OUTPUT_STRIP_TRAILING_WHITESPACE ERROR_QUIET ) - if(NOT wheel_filename) - message(STATUS "Python module `setuptools` required for correct wheel filename generation. Please install if needed.") - set(wheel_filename "unknown-unknown") - endif() set(wheel_filename "${CMAKE_BINARY_DIR}/${wheel_filename}.whl") set(wheel_distinfo "${CMAKE_BINARY_DIR}/${python_module}-${version}.dist-info") From 280578c711feb5b1a96f130ac944923d884ce25d Mon Sep 17 00:00:00 2001 From: Christian Rauch Date: Thu, 17 Oct 2024 22:37:36 +0200 Subject: [PATCH 04/13] switch to pybind11 2.13.6 --- components/pango_python/pybind11 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/pango_python/pybind11 b/components/pango_python/pybind11 index 3e9dfa286..a2e59f0e7 160000 --- a/components/pango_python/pybind11 +++ b/components/pango_python/pybind11 @@ -1 +1 @@ -Subproject commit 3e9dfa2866941655c56877882565e7577de6fc7b +Subproject commit a2e59f0e7065404b44dfe92a28aca47ba1378dc4 From 6e96d682ade6558f6b7c2c661ab5720126b12792 Mon Sep 17 00:00:00 2001 From: Christian Rauch Date: Wed, 16 Oct 2024 23:17:06 +0200 Subject: [PATCH 05/13] test wheel file installation on all systems --- .github/workflows/build.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index f62dc1607..004bb7db3 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -95,7 +95,6 @@ jobs: run: cmake --build build -t pypangolin_wheel - name: Install Python wheel - if: ${{ matrix.package_manager == 'apt' }} env: PIP_BREAK_SYSTEM_PACKAGES: 1 run: | From 4f915c1c8107c2e652f0bea66fcbf0b58b409f79 Mon Sep 17 00:00:00 2001 From: Christian Rauch Date: Thu, 17 Oct 2024 22:46:53 +0200 Subject: [PATCH 06/13] bump CMake version requirements --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 9dadf93e1..c91d2fa74 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,4 +1,4 @@ -cmake_minimum_required(VERSION 3.10) +cmake_minimum_required(VERSION 3.16) project("Pangolin") set(PANGOLIN_VERSION_MAJOR 0) From 61f1c94f1f8d5eb74b402b99cef36fa45a1b0f28 Mon Sep 17 00:00:00 2001 From: Christian Rauch Date: Thu, 17 Oct 2024 22:47:42 +0200 Subject: [PATCH 07/13] use FindPython3 module --- cmake/MakePythonWheel.cmake | 4 ++-- components/pango_python/CMakeLists.txt | 9 ++++++--- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/cmake/MakePythonWheel.cmake b/cmake/MakePythonWheel.cmake index 7e2079b57..4579bf2b5 100644 --- a/cmake/MakePythonWheel.cmake +++ b/cmake/MakePythonWheel.cmake @@ -9,13 +9,13 @@ function( MakeWheel python_module) cmake_parse_arguments(MAKEWHEEL "PRINT_HELP" "MODULE;VERSION;SUMMARY;DESCRIPTION;HOMEPAGE;AUTHOR;EMAIL;LICENCE" "REQUIRES" ${ARGN} ) set(version ${MAKEWHEEL_VERSION}) - execute_process(COMMAND ${Python_EXECUTABLE} -c "import setuptools" ERROR_QUIET RESULT_VARIABLE has_setuptools) + execute_process(COMMAND ${Python3_EXECUTABLE} -c "import setuptools" ERROR_QUIET RESULT_VARIABLE has_setuptools) if(has_setuptools EQUAL "1") message(FATAL_ERROR "Python module `setuptools` required for correct wheel filename generation.") endif() execute_process( - COMMAND ${Python_EXECUTABLE} -c " + COMMAND ${Python3_EXECUTABLE} -c " from setuptools.dist import Distribution from setuptools import Extension diff --git a/components/pango_python/CMakeLists.txt b/components/pango_python/CMakeLists.txt index 945ae3c86..0eb6bc419 100644 --- a/components/pango_python/CMakeLists.txt +++ b/components/pango_python/CMakeLists.txt @@ -3,10 +3,13 @@ get_filename_component(COMPONENT ${CMAKE_CURRENT_LIST_DIR} NAME) option(BUILD_PANGOLIN_PYTHON "Build support for Pangolin Interactive Console" ON) if(BUILD_PANGOLIN_PYTHON) - find_package(Python COMPONENTS Interpreter Development QUIET) + if(POLICY CMP0148) + cmake_policy(SET CMP0148 NEW) + endif() + find_package(Python3 COMPONENTS Interpreter Development REQUIRED) endif() -if(BUILD_PANGOLIN_PYTHON AND Python_FOUND) +if(BUILD_PANGOLIN_PYTHON AND Python3_FOUND) # If we've inited the submodule, use that if(EXISTS "${CMAKE_CURRENT_LIST_DIR}/pybind11/CMakeLists.txt") add_subdirectory(${CMAKE_CURRENT_LIST_DIR}/pybind11) @@ -15,7 +18,7 @@ if(BUILD_PANGOLIN_PYTHON AND Python_FOUND) endif() endif() -if(BUILD_PANGOLIN_PYTHON AND Python_FOUND AND pybind11_FOUND) +if(BUILD_PANGOLIN_PYTHON AND Python3_FOUND AND pybind11_FOUND) set(SRC_BINDINGS ${CMAKE_CURRENT_LIST_DIR}/src/pypangolin/attach.cpp ${CMAKE_CURRENT_LIST_DIR}/src/pypangolin/colour.cpp From 85642e85e328a41500e9c83de00e15918a24bb5b Mon Sep 17 00:00:00 2001 From: Christian Rauch Date: Sat, 19 Oct 2024 10:03:56 +0200 Subject: [PATCH 08/13] details about module search path on failure --- cmake/MakePythonWheel.cmake | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/cmake/MakePythonWheel.cmake b/cmake/MakePythonWheel.cmake index 4579bf2b5..200ba1e15 100644 --- a/cmake/MakePythonWheel.cmake +++ b/cmake/MakePythonWheel.cmake @@ -9,7 +9,19 @@ function( MakeWheel python_module) cmake_parse_arguments(MAKEWHEEL "PRINT_HELP" "MODULE;VERSION;SUMMARY;DESCRIPTION;HOMEPAGE;AUTHOR;EMAIL;LICENCE" "REQUIRES" ${ARGN} ) set(version ${MAKEWHEEL_VERSION}) - execute_process(COMMAND ${Python3_EXECUTABLE} -c "import setuptools" ERROR_QUIET RESULT_VARIABLE has_setuptools) + execute_process( + COMMAND ${Python3_EXECUTABLE} -c " +import sys +try: + import setuptools + sys.exit(0) +except ImportError as e: + print(f'{e}. Search paths:', file=sys.stderr) + for p in sys.path: print(f' {p}', file=sys.stderr) + sys.exit(1) +" + RESULT_VARIABLE has_setuptools) + if(has_setuptools EQUAL "1") message(FATAL_ERROR "Python module `setuptools` required for correct wheel filename generation.") endif() From 8fbc323829b0c16f3ced2d2bb5f22fdb2633454c Mon Sep 17 00:00:00 2001 From: Christian Rauch Date: Thu, 17 Oct 2024 23:09:07 +0200 Subject: [PATCH 09/13] skip Python bindings for Emscripten --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 004bb7db3..d7b9e5e66 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -120,7 +120,7 @@ jobs: sudo apt install -y emscripten ninja-build libeigen3-dev catch2 - name: Configure Pangolin - run: emcmake cmake -G Ninja -B pangolin-build -D CMAKE_BUILD_TYPE=$BUILD_TYPE -D Eigen3_DIR=/usr/share/eigen3/cmake/ + run: emcmake cmake -G Ninja -B pangolin-build -D CMAKE_BUILD_TYPE=$BUILD_TYPE -D BUILD_PANGOLIN_PYTHON=OFF -D Eigen3_DIR=/usr/share/eigen3/cmake/ - name: Build Pangolin run: cmake --build pangolin-build From 215e89628724c5dddcf3d6291a30c65583e6cad5 Mon Sep 17 00:00:00 2001 From: Christian Rauch Date: Sat, 19 Oct 2024 14:54:50 +0200 Subject: [PATCH 10/13] replace vcpkg installed python --- .github/workflows/build.yml | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index d7b9e5e66..0aea743ad 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -80,6 +80,21 @@ jobs: echo "CMake toolchain file: $TOOLCHAIN_FILE" $GITHUB_WORKSPACE/scripts/install_prerequisites.sh -v -u -m ${{matrix.package_manager}} all + - name: "(vcpkg only) remove python" + if: ${{ matrix.package_manager == 'vcpkg' }} + run: | + vcpkg remove python3 + + - uses: actions/setup-python@v5 + if: ${{ matrix.package_manager == 'vcpkg' }} + with: + python-version: '3.12' + + - name: "(vcpkg only) install setuptools" + if: ${{ matrix.package_manager == 'vcpkg' }} + run: | + pip install setuptools + - name: Configure CMake run: > cmake -G Ninja -B build From 53142fd7a8f08662cdbf1943449f3b4e48d32a0d Mon Sep 17 00:00:00 2001 From: Christian Rauch Date: Sat, 19 Oct 2024 16:21:10 +0200 Subject: [PATCH 11/13] list wheel file content --- .github/workflows/build.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 0aea743ad..bf89ef66c 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -113,6 +113,7 @@ jobs: env: PIP_BREAK_SYSTEM_PACKAGES: 1 run: | + python -m zipfile --list build/pypangolin-*.whl pip install build/pypangolin-*.whl pip show pypangolin python -c "import pypangolin" From 8ac92bd547adf7cbca73ad714fa1842e5d6dda10 Mon Sep 17 00:00:00 2001 From: Christian Rauch Date: Sat, 19 Oct 2024 23:19:09 +0200 Subject: [PATCH 12/13] do not run wheel test with shared libs on Windows --- .github/workflows/build.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index bf89ef66c..39d77f081 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -116,6 +116,10 @@ jobs: python -m zipfile --list build/pypangolin-*.whl pip install build/pypangolin-*.whl pip show pypangolin + + - name: Test Python wheel + if: ${{ !(matrix.package_manager == 'vcpkg' && matrix.shared_libs == 'ON') }} + run: | python -c "import pypangolin" - name: Run all tests From 44d64ae24b30bc977e463083a7bb0bfd2f816afc Mon Sep 17 00:00:00 2001 From: Christian Rauch Date: Sat, 19 Oct 2024 23:21:53 +0200 Subject: [PATCH 13/13] run tests on all platforms --- .github/workflows/build.yml | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 39d77f081..eb050caab 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -21,19 +21,15 @@ jobs: include: - os: ubuntu-22.04 package_manager: "apt" - test: "ON" - os: ubuntu-24.04 package_manager: "apt" - test: "ON" - os: macos-14 package_manager: "brew" - test: "OFF" - os: windows-2022 package_manager: "vcpkg" - test: "OFF" steps: - name: Checkout Pangolin @@ -101,7 +97,7 @@ jobs: -D CMAKE_BUILD_TYPE=$BUILD_TYPE -D CMAKE_TOOLCHAIN_FILE="$TOOLCHAIN_FILE" -D BUILD_SHARED_LIBS=${{ matrix.shared_libs }} - -D BUILD_TESTS=${{ matrix.test }} + -D BUILD_TESTS=ON - name: Build run: cmake --build build --config $BUILD_TYPE @@ -123,7 +119,6 @@ jobs: python -c "import pypangolin" - name: Run all tests - if: ${{ matrix.test == 'ON' }} run: | cmake --build build --target test