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.

Sign up for GitHub

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/BLD: move CIBW commands to pyproject.toml, move scripts #328

Merged
merged 2 commits into from
Jul 13, 2024
Merged

CI/BLD: move CIBW commands to pyproject.toml, move scripts #328

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0148a4d
CI/BLD: move CIBW commands to pyproject.toml, move scripts
mwtoews Jul 12, 2024
45f7c51
Use shasum on macOS, use = for POSIX compliance
adamjstewart Jul 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 0 additions & 9 deletions .github/workflows/deploy.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,18 +34,9 @@ jobs:
with:
python-version: '3.11'

- name: Setup
run: pip install wheel

- uses: ilammy/msvc-dev-cmd@v1
if: startsWith(matrix.os, 'windows')

- name: Run Windows Preinstall Build
if: startsWith(matrix.os, 'windows')
run: |
choco install vcpython27 -f -y
ci\install_libspatialindex.bat

- name: Build wheels
uses: pypa/[email protected]

Expand Down
10 changes: 7 additions & 3 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ rtree = ["py.typed"]
[tool.cibuildwheel]
build = "cp39-*"
build-verbosity = 3
before-all = "pip install wheel"
repair-wheel-command = "python scripts/repair_wheel.py -w {dest_dir} {wheel}"
test-requires = "tox"
test-command = "tox --conf {project} --installpkg {wheel}"
Expand All @@ -62,26 +63,29 @@ test-skip = [
archs = ["auto", "aarch64"]
before-build = [
"yum install -y cmake libffi-devel",
"bash {project}/ci/install_libspatialindex.bash",
"sh {project}/scripts/install_libspatialindex.sh",
]

[[tool.cibuildwheel.overrides]]
select = "*-musllinux*"
before-build = [
"apk add cmake libffi-dev",
"bash {project}/ci/install_libspatialindex.bash",
"sh {project}/scripts/install_libspatialindex.sh",
]

[tool.cibuildwheel.macos]
archs = ["x86_64", "arm64"]
environment = { MACOSX_DEPLOYMENT_TARGET="10.9" }
before-build = [
"brew install coreutils cmake",
"bash {project}/ci/install_libspatialindex.bash",
"sh {project}/scripts/install_libspatialindex.sh",
]

[tool.cibuildwheel.windows]
archs = ["AMD64"]
before-build = [
"call {project}\\scripts\\install_libspatialindex.bat",
]

[tool.coverage.report]
# Ignore warnings for overloads
Expand Down
0 ci/install_libspatialindex.bat → scripts/install_libspatialindex.bat
100755 → 100644
View file
Open in desktop
File renamed without changes.
11 changes: 8 additions & 3 deletions ci/install_libspatialindex.bash → scripts/install_libspatialindex.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#!/bin/bash
#!/bin/sh
set -xe
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For fun, I tried running this script with dash on macOS. I confirmed that it's POSIX compliant. The only issue I encountered is that sha256sum does not exist on macOS, you have to use shasum -a 256 instead. Might be able to check uname in an if-statement if you want to fix this. I can also submit a PR after this one is merged if you want.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for checking! It looks like the macos image has sha256sum, probably via brew's coreutils package (the internet tells me at least). But I won't mind if you want to complete this aspect. You can push commits on this PR, if you know how.

Copy link
Collaborator

@adamjstewart adamjstewart Jul 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I confirmed that it's POSIX compliant.

I take this back. == is not POSIX compliant, we should be using = instead. I'll fix this in my commit.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw, https://www.shellcheck.net/ is handy if you want to check for POSIX compliance and other sh gotchas.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Handy resource! The cli version does nice work too.


# A simple script to install libspatialindex from a Github Release
Expand Down Expand Up @@ -32,7 +32,12 @@ 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 -
if [ "$(uname)" = "Darwin" ]
then
echo "${SHA256} ${VERSION}.zip" | shasum -a 256 -c -
else
echo "${SHA256} ${VERSION}.zip" | sha256sum -c -
fi

rm -rf "libspatialindex-${VERSION}"
unzip -q $VERSION
Expand All @@ -43,7 +48,7 @@ cd build

printenv

if [ "$(uname)" == "Darwin" ]; then
if [ "$(uname)" = "Darwin" ]; then
CMAKE_ARGS="-D CMAKE_OSX_ARCHITECTURES=${ARCHFLAGS##* } \
-D CMAKE_INSTALL_RPATH=@loader_path"
fi
Expand Down
Loading