From 86d4e28f19849d69ca7c1babf858210f49dd1ecd Mon Sep 17 00:00:00 2001 From: Mike Taves Date: Wed, 10 Jul 2024 09:50:29 +1200 Subject: [PATCH] CI/RLS: upgrade to libspatialindex-2.0.0 (#316) * Upgrade libspatialindex from 1.9.3 to 2.0.0 * Remove the unneeded ci/CMakeLists.txt * Use cmake install, removing the unneeded lib subdirs * Remove the soversion number and symlinks for the library name (via CMAKE_PLATFORM_NO_VERSIONED_SONAME=ON) * It turns out the cibuildwheel tests on different python versions (via tox) was misleading, as it was not testing the built-wheel but the local source tree and sidx library build. This is fixed with `pytest --import-mode=importlib` * Consequently, the musllinux wheels on PyPI are "broken". This is resolved by setting rpath for libspatialindex described next. * Use `CMAKE_INSTALL_RPATH` only for macOS. On Linux, `$ORIGIN` seems to confuse auditwheel and prevents creation of `Rtree.libs`. It want's LD_LIBRARY_PATH set while repairing the wheel. But the `$ORIGIN` rpath is still needed for musllinux, so manually add this last step in repair_wheel.py * Simplify copying directory trees in setup.py with copy_tree --- .github/workflows/test.yml | 3 +- ci/CMakeLists.txt | 237 -------------------------------- ci/install_libspatialindex.bash | 63 ++++----- ci/install_libspatialindex.bat | 30 ++-- pyproject.toml | 8 +- scripts/repair_wheel.py | 16 ++- setup.py | 34 +---- tox.ini | 4 +- 8 files changed, 71 insertions(+), 324 deletions(-) delete mode 100644 ci/CMakeLists.txt diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index a852965a..0bd3baab 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -30,7 +30,8 @@ jobs: matrix: os: ['ubuntu-latest', 'macos-latest', 'windows-latest'] python-version: ['3.8', '3.9', '3.10', '3.11', '3.12'] - sidx-version: ['1.8.5', '1.9.3'] + # test oldesst and newest libspatialindex versions + sidx-version: ['1.8.5', '2.0.0'] exclude: - os: 'macos-latest' - sidx-version: '1.8.5' diff --git a/ci/CMakeLists.txt b/ci/CMakeLists.txt deleted file mode 100644 index aec6a080..00000000 --- a/ci/CMakeLists.txt +++ /dev/null @@ -1,237 +0,0 @@ -# -# top-level CMake configuration file for libspatialindex -# -# (based originally on the libLAS files copyright Mateusz Loskot) - -SET(MSVC_INCREMENTAL_DEFAULT OFF) -cmake_minimum_required(VERSION 3.5.0) -project(spatialindex) - -#------------------------------------------------------------------------------ -# internal cmake settings -#------------------------------------------------------------------------------ - -set(CMAKE_COLOR_MAKEFILE ON) - -# C++11 required -set (CMAKE_CXX_STANDARD 11) - -# Allow advanced users to generate Makefiles printing detailed commands -mark_as_advanced(CMAKE_VERBOSE_MAKEFILE) - -# Path to additional CMake modules -set(CMAKE_MODULE_PATH "${PROJECT_SOURCE_DIR}/cmake/modules" ${CMAKE_MODULE_PATH}) - -# Make string comparison in cmake behave like you'd expect -cmake_policy(SET CMP0054 NEW) - -if (WIN32) - if(${CMAKE_VERSION} VERSION_GREATER "3.14.5") - cmake_policy(SET CMP0092 NEW) # don't put /w3 in flags - endif() -endif() - -if (APPLE) - set(CMAKE_MACOSX_RPATH ON) -endif (APPLE) - -#------------------------------------------------------------------------------ -# libspatialindex general settings -#------------------------------------------------------------------------------ - -SET(SIDX_VERSION_MAJOR "1") -SET(SIDX_VERSION_MINOR "9") -SET(SIDX_VERSION_PATCH "3") -SET(SIDX_LIB_VERSION "6.1.1") -SET(SIDX_LIB_SOVERSION "6") -SET(BUILD_SHARED_LIBS ON) - - -set(SIDX_VERSION_STRING "${SIDX_VERSION_MAJOR}.${SIDX_VERSION_MINOR}.${SIDX_VERSION_PATCH}") - -#------------------------------------------------------------------------------ -# libspatialindex general cmake options -#------------------------------------------------------------------------------ - -option(SIDX_BUILD_TESTS "Enables integrated test suites" OFF) - - -# Name of C++ library - -set(SIDX_LIB_NAME spatialindex) -set(SIDX_C_LIB_NAME spatialindex_c) - -if(WIN32) - if (MSVC) - if( CMAKE_SIZEOF_VOID_P EQUAL 8 ) - set( SIDX_LIB_NAME "spatialindex-64" ) - set( SIDX_C_LIB_NAME "spatialindex_c-64" ) - else( CMAKE_SIZEOF_VOID_P EQUAL 8 ) - set( SIDX_LIB_NAME "spatialindex-32" ) - set( SIDX_C_LIB_NAME "spatialindex_c-32" ) - endif( CMAKE_SIZEOF_VOID_P EQUAL 8 ) - endif() -endif() - -set(CMAKE_INCLUDE_DIRECTORIES_PROJECT_BEFORE ON) - -include (CheckFunctionExists) - -check_function_exists(srand48 HAVE_SRAND48) -check_function_exists(gettimeofday HAVE_GETTIMEOFDAY) -check_function_exists(memset HAVE_MEMSET) -check_function_exists(memcpy HAVE_MEMCPY) -check_function_exists(bcopy HAVE_BCOPY) - - -INCLUDE (CheckIncludeFiles) - - -#------------------------------------------------------------------------------ -# General build settings -#------------------------------------------------------------------------------ - -# note we default to RelWithDebInfo mode if not set -if(NOT CMAKE_BUILD_TYPE) - set(CMAKE_BUILD_TYPE RelWithDebInfo CACHE STRING - "Choose the type of build, options are: None Debug Release RelWithDebInfo MinSizeRel" FORCE) -endif() - -# Always show which build type we have -message(STATUS "Setting libspatialindex build type - ${CMAKE_BUILD_TYPE}") - -set(SIDX_BUILD_TYPE ${CMAKE_BUILD_TYPE}) - -# TODO: Still testing the output paths --mloskot -set(SIDX_BUILD_OUTPUT_DIRECTORY "${PROJECT_BINARY_DIR}/bin") - -# Output directory in which to build RUNTIME target files. -set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${SIDX_BUILD_OUTPUT_DIRECTORY}) - -# Output directory in which to build LIBRARY target files -set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${SIDX_BUILD_OUTPUT_DIRECTORY}) - -# Output directory in which to build ARCHIVE target files. -set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${SIDX_BUILD_OUTPUT_DIRECTORY}) - - -#------------------------------------------------------------------------------ -# Platform and compiler specific settings -#------------------------------------------------------------------------------ - -if(NOT WIN32) - # Recommended C++ compilation flags - set(SIDX_COMMON_CXX_FLAGS - "-pedantic -Wall -Wpointer-arith -Wcast-align -Wcast-qual -Wredundant-decls -Wno-long-long -Wl --no-undefined") -endif(NOT WIN32) - -if (APPLE) - set(SO_EXT dylib) - set(CMAKE_FIND_FRAMEWORK "LAST") -elseif(WIN32) - set(SO_EXT dll) -else() - set(SO_EXT so) -endif(APPLE) - - -enable_testing() - -#------------------------------------------------------------------------------ -# installation path settings -#------------------------------------------------------------------------------ - -if(WIN32) - set(DEFAULT_LIB_SUBDIR lib) - set(DEFAULT_DATA_SUBDIR .) - set(DEFAULT_INCLUDE_SUBDIR include) - - if (MSVC) - set(DEFAULT_BIN_SUBDIR bin) - else() - set(DEFAULT_BIN_SUBDIR .) - endif() -else() - # Common locations for Unix and Mac OS X - set(DEFAULT_BIN_SUBDIR bin) - set(DEFAULT_LIB_SUBDIR lib${LIB_SUFFIX}) - set(DEFAULT_DATA_SUBDIR share/spatialindex) - set(DEFAULT_INCLUDE_SUBDIR include) -endif() - -# Locations are changeable by user to customize layout of SIDX installation -# (default values are platform-specific) -set(SIDX_BIN_SUBDIR ${DEFAULT_BIN_SUBDIR} CACHE STRING - "Subdirectory where executables will be installed") -set(SIDX_LIB_SUBDIR ${DEFAULT_LIB_SUBDIR} CACHE STRING - "Subdirectory where libraries will be installed") -set(SIDX_INCLUDE_SUBDIR ${DEFAULT_INCLUDE_SUBDIR} CACHE STRING - "Subdirectory where header files will be installed") -set(SIDX_DATA_SUBDIR ${DEFAULT_DATA_SUBDIR} CACHE STRING - "Subdirectory where data will be installed") - -# Mark *_SUBDIR variables as advanced and dedicated to use by power-users only. -mark_as_advanced(SIDX_BIN_SUBDIR - SIDX_LIB_SUBDIR SIDX_INCLUDE_SUBDIR SIDX_DATA_SUBDIR) - -# Full paths for the installation -set(SIDX_BIN_DIR ${SIDX_BIN_SUBDIR}) -set(SIDX_LIB_DIR ${SIDX_LIB_SUBDIR}) -set(SIDX_INCLUDE_DIR ${SIDX_INCLUDE_SUBDIR}) -set(SIDX_DATA_DIR ${SIDX_DATA_SUBDIR}) - -#------------------------------------------------------------------------------ -# subdirectory controls -#------------------------------------------------------------------------------ - -add_subdirectory(src) - -if(SIDX_BUILD_TESTS) - add_subdirectory(test) -endif() - -#------------------------------------------------------------------------------ -# CPACK controls -#------------------------------------------------------------------------------ - -SET(CPACK_PACKAGE_VERSION_MAJOR ${SIDX_VERSION_MAJOR}) -SET(CPACK_PACKAGE_VERSION_MINOR ${SIDX_VERSION_MINOR}) -SET(CPACK_PACKAGE_VERSION_PATCH ${SIDX_VERSION_MINOR}) -SET(CPACK_PACKAGE_NAME "libspatialindex") - -SET(CPACK_SOURCE_GENERATOR "TBZ2;TGZ") -SET(CPACK_PACKAGE_VENDOR "libspatialindex Development Team") -SET(CPACK_RESOURCE_FILE_LICENSE "${PROJECT_SOURCE_DIR}/COPYING") - -set(CPACK_SOURCE_PACKAGE_FILE_NAME - "${CMAKE_PROJECT_NAME}-src-${SIDX_VERSION_STRING}") - -set(CPACK_SOURCE_IGNORE_FILES -"/\\\\.gitattributes;/\\\\.vagrant;/\\\\.DS_Store;/CVS/;/\\\\.git/;\\\\.swp$;~$;\\\\.\\\\#;/\\\\#") - -list(APPEND CPACK_SOURCE_IGNORE_FILES "CMakeScripts/") -list(APPEND CPACK_SOURCE_IGNORE_FILES "_CPack_Packages") -list(APPEND CPACK_SOURCE_IGNORE_FILES "cmake_install.cmake") -list(APPEND CPACK_SOURCE_IGNORE_FILES "/bin/") -list(APPEND CPACK_SOURCE_IGNORE_FILES "/scripts/") -list(APPEND CPACK_SOURCE_IGNORE_FILES "/azure-pipelines.yml") -list(APPEND CPACK_SOURCE_IGNORE_FILES ".gitignore") -list(APPEND CPACK_SOURCE_IGNORE_FILES ".ninja*") -list(APPEND CPACK_SOURCE_IGNORE_FILES "HOWTORELEASE.txt") - -list(APPEND CPACK_SOURCE_IGNORE_FILES "README") -list(APPEND CPACK_SOURCE_IGNORE_FILES "build/") - -list(APPEND CPACK_SOURCE_IGNORE_FILES "CMakeFiles") -list(APPEND CPACK_SOURCE_IGNORE_FILES "CTestTestfile.cmake") -list(APPEND CPACK_SOURCE_IGNORE_FILES "/docs/build/") -list(APPEND CPACK_SOURCE_IGNORE_FILES "/doc/presentations/") -list(APPEND CPACK_SOURCE_IGNORE_FILES "package-release.sh") -list(APPEND CPACK_SOURCE_IGNORE_FILES "docker-package.sh") - -list(APPEND CPACK_SOURCE_IGNORE_FILES ".gz2") - -list(APPEND CPACK_SOURCE_IGNORE_FILES ".bz2") - -include(CPack) -add_custom_target(dist COMMAND ${CMAKE_MAKE_PROGRAM} package_source) diff --git a/ci/install_libspatialindex.bash b/ci/install_libspatialindex.bash index 4b38503e..50084035 100755 --- a/ci/install_libspatialindex.bash +++ b/ci/install_libspatialindex.bash @@ -2,26 +2,15 @@ set -xe # A simple script to install libspatialindex from a Github Release -VERSION=1.9.3 -SHA256=63a03bfb26aa65cf0159f925f6c3491b6ef79bc0e3db5a631d96772d6541187e +VERSION=2.0.0 +SHA256=8caa4564c4592824acbf63a2b883aa2d07e75ccd7e9bf64321c455388a560579 # where to copy resulting files # this has to be run before `cd`-ing anywhere -libtarget() { +install_prefix() { OURPWD=$PWD cd "$(dirname "$0")" - mkdir -p ../rtree/lib - cd ../rtree/lib - arr=$(pwd) - cd "$OURPWD" - echo $arr -} - -headertarget() { - OURPWD=$PWD - cd "$(dirname "$0")" - mkdir -p ../rtree/include - cd ../rtree/include + cd ../rtree arr=$(pwd) cd "$OURPWD" echo $arr @@ -36,48 +25,44 @@ scriptloc() { } # note that we're doing this convoluted thing to get # an absolute path so mac doesn't yell at us -LIBTARGET=`libtarget` -HEADERTARGET=`headertarget` +INSTALL_PREFIX=`install_prefix` SL=`scriptloc` -rm $VERSION.zip || true -curl -L -O https://github.com/libspatialindex/libspatialindex/archive/$VERSION.zip +rm -f $VERSION.zip +curl -LOs --retry 5 --retry-max-time 120 https://github.com/libspatialindex/libspatialindex/archive/${VERSION}.zip # check the file hash echo "${SHA256} ${VERSION}.zip" | sha256sum -c - -rm -rf "libspatialindex-${VERSION}" || true +rm -rf "libspatialindex-${VERSION}" unzip -q $VERSION cd libspatialindex-${VERSION} mkdir build cd build -cp "${SL}/CMakeLists.txt" .. - printenv if [ "$(uname)" == "Darwin" ]; then - CMAKE_ARGS="-DCMAKE_OSX_ARCHITECTURES=${ARCHFLAGS##* }" + CMAKE_ARGS="-D CMAKE_OSX_ARCHITECTURES=${ARCHFLAGS##* } \ + -D CMAKE_INSTALL_RPATH=@loader_path" fi -cmake -DCMAKE_BUILD_TYPE=Release ${CMAKE_ARGS} .. +cmake ${CMAKE_ARGS} \ + -D CMAKE_BUILD_TYPE=Release \ + -D BUILD_SHARED_LIBS=ON \ + -D CMAKE_INSTALL_PREFIX=${INSTALL_PREFIX} \ + -D CMAKE_INSTALL_LIBDIR=lib \ + -D CMAKE_PLATFORM_NO_VERSIONED_SONAME=ON \ + .. make -j 4 # copy built libraries relative to path of this script -# -d means copy links as links rather than duplicate files -# macos uses "bsd cp" and needs special handling -if [ "$(uname)" == "Darwin" ]; then - # change the rpath in the dylib to point to the same directory - install_name_tool -change @rpath/libspatialindex.6.dylib @loader_path/libspatialindex.dylib bin/libspatialindex_c.dylib - # copy the dylib files to the target director - cp bin/libspatialindex.dylib $LIBTARGET - cp bin/libspatialindex_c.dylib $LIBTARGET - cp -r ../include/* $HEADERTARGET -else - cp -L bin/* $LIBTARGET - cp -r ../include/* $HEADERTARGET -fi +make install + +# remove unneeded extras in lib +rm -rfv ${INSTALL_PREFIX}/lib/cmake +rm -rfv ${INSTALL_PREFIX}/lib/pkgconfig -ls $LIBTARGET -ls -R $HEADERTARGET +ls -R ${INSTALL_PREFIX}/lib +ls -R ${INSTALL_PREFIX}/include diff --git a/ci/install_libspatialindex.bat b/ci/install_libspatialindex.bat index a2ce9a95..3cb98791 100755 --- a/ci/install_libspatialindex.bat +++ b/ci/install_libspatialindex.bat @@ -1,13 +1,11 @@ python -c "import sys; print(sys.version)" -set SIDX_VERSION=1.9.3 +set SIDX_VERSION=2.0.0 -curl -OL "https://github.com/libspatialindex/libspatialindex/archive/%SIDX_VERSION%.zip" +curl -LO --retry 5 --retry-max-time 120 "https://github.com/libspatialindex/libspatialindex/archive/%SIDX_VERSION%.zip" tar xvf "%SIDX_VERSION%.zip" -REM unzip 1.9.3.zip -REM copy %~dp0\CMakeLists.txt libspatialindex-1.9.3\CMakeLists.txt cd libspatialindex-%SIDX_VERSION% mkdir build @@ -15,15 +13,21 @@ cd build pip install ninja -cmake -D CMAKE_BUILD_TYPE=Release -G Ninja .. +set INSTALL_PREFIX=%~dp0\..\rtree -ninja +cmake -G Ninja ^ + -D CMAKE_BUILD_TYPE=Release ^ + -D BUILD_SHARED_LIBS="ON" ^ + -D CMAKE_INSTALL_PREFIX="%INSTALL_PREFIX%" ^ + -D CMAKE_INSTALL_BINDIR=lib ^ + -D CMAKE_INSTALL_LIBDIR=libdir ^ + .. -mkdir %~dp0\..\rtree\lib -copy bin\*.dll %~dp0\..\rtree\lib -xcopy /S ..\include\* %~dp0\..\rtree\include\ -rmdir /Q /S bin +ninja install -dir %~dp0\..\rtree\ -dir %~dp0\..\rtree\lib -dir %~dp0\..\rtree\include +:: remove unneeded libdir +rmdir %INSTALL_PREFIX%\libdir /s /q + +dir %INSTALL_PREFIX% +dir %INSTALL_PREFIX%\lib +dir %INSTALL_PREFIX%\include /s diff --git a/pyproject.toml b/pyproject.toml index a1657e33..9fb47992 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -50,14 +50,17 @@ rtree = ["py.typed"] [tool.cibuildwheel] build = "cp38-*" -build-verbosity = "3" +build-verbosity = 3 repair-wheel-command = "python scripts/repair_wheel.py -w {dest_dir} {wheel}" test-requires = "tox" test-command = "tox --conf {project} --installpkg {wheel}" +test-skip = [ + "*aarch64", # slow! + "*-macosx_arm64", +] [tool.cibuildwheel.linux] archs = ["auto", "aarch64"] -test-skip = "*aarch64" # slow! before-build = [ "yum install -y cmake libffi-devel", "bash {project}/ci/install_libspatialindex.bash", @@ -72,7 +75,6 @@ before-build = [ [tool.cibuildwheel.macos] archs = ["x86_64", "arm64"] -test-skip = "*-macosx_arm64" environment = { MACOSX_DEPLOYMENT_TARGET="10.9" } before-build = [ "brew install coreutils cmake", diff --git a/scripts/repair_wheel.py b/scripts/repair_wheel.py index 1ae41fd5..d993ab7d 100755 --- a/scripts/repair_wheel.py +++ b/scripts/repair_wheel.py @@ -1,6 +1,7 @@ #!/usr/bin/env python3 import argparse +import os import shutil import subprocess import sys @@ -43,6 +44,13 @@ def main(): tmpdir = Path(tmpdir_) # use the platform specific repair tool first if os_ == "linux": + # use path from cibuildwheel which allows auditwheel to create + # Rtree.libs/libspatialindex-*.so.* + cibw_lib_path = "/project/rtree/lib" + if os.environ.get("LD_LIBRARY_PATH"): # append path + os.environ["LD_LIBRARY_PATH"] += f"{os.pathsep}{cibw_lib_path}" + else: + os.environ["LD_LIBRARY_PATH"] = cibw_lib_path subprocess.run( ["auditwheel", "repair", "-w", str(tmpdir), str(file)], check=True ) @@ -85,9 +93,15 @@ def main(): break else: raise RuntimeError("subdirectory not found") + if os_ == "linux": + # This is auditwheel's libs, which needs post-processing + libs_dir = unpackdir / "Rtree.libs" + lsidx_list = list(libs_dir.glob("libspatialindex*.so*")) + assert len(lsidx_list) == 1, list(libs_dir.iterdir()) + lsidx = lsidx_list[0] + subprocess.run(["patchelf", "--set-rpath", "$ORIGIN", lsidx], check=True) # remove duplicated dir - assert len(list((unpackdir / "Rtree.libs").glob("*.so*"))) >= 1 lib_dir = unpackdir / "rtree" / "lib" shutil.rmtree(lib_dir) # re-pack diff --git a/setup.py b/setup.py index a1fc40d9..a42bcd65 100755 --- a/setup.py +++ b/setup.py @@ -1,5 +1,4 @@ #!/usr/bin/env python3 -import sys from pathlib import Path from setuptools import setup @@ -43,38 +42,17 @@ def finalize_options(self) -> None: # destination for the files in the build directory target_dir = Path(self.build_lib) / "rtree" + # copy lib tree source_lib = source_dir / "lib" - target_lib = target_dir / "lib" if source_lib.is_dir(): - # what patterns represent shared libraries for supported platforms - if sys.platform.startswith("win"): - lib_pattern = "*.dll" - elif sys.platform.startswith("linux"): - lib_pattern = "*.so*" - elif sys.platform == "darwin": - lib_pattern = "libspatialindex*dylib" - else: - raise ValueError(f"unhandled platform {sys.platform!r}") - - target_lib.mkdir(parents=True, exist_ok=True) - for pth in source_lib.glob(lib_pattern): - # if the source isn't a file skip it - if not pth.is_file(): - continue - - # copy the source file to the target directory - self.copy_file(str(pth), str(target_lib / pth.name)) + target_lib = target_dir / "lib" + self.copy_tree(str(source_lib), str(target_lib)) + # copy include tree source_include = source_dir / "include" - target_include = target_dir / "include" if source_include.is_dir(): - for pth in source_include.rglob("*.h"): - rpth = pth.relative_to(source_include) - - # copy the source file to the target directory - target_subdir = target_include / rpth.parent - target_subdir.mkdir(parents=True, exist_ok=True) - self.copy_file(str(pth), str(target_subdir)) + target_include = target_dir / "include" + self.copy_tree(str(source_include), str(target_include)) # See pyproject.toml for other project metadata diff --git a/tox.ini b/tox.ini index 64e0be7e..8b81aa5d 100644 --- a/tox.ini +++ b/tox.ini @@ -6,11 +6,11 @@ env_list = py{38,39,310,311,312} [testenv] description = run unit tests deps = - pytest + pytest>=6 numpy install_command = python -I -m pip install --only-binary=:all: {opts} {packages} ignore_errors = True ignore_outcome = True commands = - pytest {posargs:tests} + pytest --import-mode=importlib {posargs:tests}