-
Notifications
You must be signed in to change notification settings - Fork 123
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
CI/RLS: upgrade to libspatialindex-2.0.0 #316
Conversation
Waiting on you to merge this @mwtoews. Should we fire a release afterward? |
there one thing that I'm sorting out before merge, as I want to keep the linux lib dir in Rtree.libs, since I see downstream packages rely on this. |
25210a2
to
14fa6fb
Compare
There is yet another big list of changes on the build-side, updated in the PR description. This is ready to review by anyone, I'll probably merge it in another day after I've had a chance to carefully review the artefacts. |
This looks like an amazing clean-up! One thing I noticed, is that in the build CI vcpython27 is installed on Windows: rtree/.github/workflows/deploy.yml Lines 43 to 47 in d42650c
Is this still needed, or can it be updated? |
It looks like vcpython27 was added in 2020 with #174, at the time when MSBuild was used. Perhaps the existing ilammy/msvc-dev-cmd is sufficient, since cl.exe is needed for |
Towards #315.
This PR does the following:
CMAKE_PLATFORM_NO_VERSIONED_SONAME=ON
)pytest --import-mode=importlib
docker run -it --rm alpine:3.18 /bin/sh --login
thenapk add py3-pip
,pip install rtree
,python -c "import rtree"
to observe error. This is resolved by setting rpath for libspatialindex described next.CMAKE_INSTALL_RPATH
only for macOS. On Linux,$ORIGIN
seems to confuse auditwheel and prevents creation ofRtree.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