-
Notifications
You must be signed in to change notification settings - Fork 34
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
Scikit-build-core build system rework #1971
Changes from all commits
678f50d
7c20a47
6f8c182
b5d8bce
bc166f7
f3ec205
51723fd
cff8080
042f320
5fa6f72
76da30d
210a75e
f73dcb8
a456a6d
cac2204
4e145ba
0b429c4
8ed0840
7156bdc
982d1e5
dfb32b8
e160a89
56f7b76
7e7099f
7f1c3cd
9c13ff8
09ac7d6
2c1699e
c9f5b73
b2f8f14
2b4e5d2
a1f8578
78a09d9
666458e
bc12c26
39ef85a
aa17c85
113cc03
97e8ff9
1be328a
d434cf3
d8ae95b
c95606e
6456ad5
294bc1e
6cdc40f
27a3855
83f439f
2c4e236
c4275f6
9901082
b787fda
80a8d18
9dcfd3a
9bd795e
3dd210a
7eaa19a
707da3b
2dd5f4a
320c02c
c62733a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,146 @@ | ||
name: Build and test wheels | ||
|
||
on: | ||
workflow_dispatch: | ||
push: | ||
branches: | ||
- release-* | ||
- "*wheel*" # must quote since "*" is a YAML reserved character; we want a string | ||
tags: | ||
- "*" | ||
pull_request: | ||
branches: | ||
- "*wheel*" # must quote since "*" is a YAML reserved character; we want a string | ||
|
||
jobs: | ||
build_wheels: | ||
name: Wheel ${{ matrix.buildplat[0] }}-${{ matrix.buildplat[1] }}-${{ matrix.python }} | ||
runs-on: ${{ matrix.buildplat[0] }} | ||
strategy: | ||
matrix: | ||
buildplat: | ||
- [ubuntu-22.04, manylinux_x86_64] | ||
- [macos-13, macosx_x86_64] | ||
- [macos-14, macosx_arm64] | ||
- [windows-2022, win_amd64] | ||
python: ["cp39", "cp310", "cp311", "cp312"] | ||
|
||
steps: | ||
- uses: actions/checkout@v4 | ||
|
||
- name: "Brew setup on macOS" # x-ref c8e49ba8f8b9ce | ||
if: ${{ startsWith(matrix.os, 'macos-') == true }} | ||
run: | | ||
set -e pipefail | ||
brew update | ||
brew install automake pkg-config ninja llvm | ||
|
||
- name: Build wheels | ||
uses: pypa/[email protected] | ||
env: | ||
CIBW_BUILD_VERBOSITY: 3 | ||
CIBW_ENVIRONMENT_MACOS: > | ||
CC=clang | ||
CXX=clang++ | ||
MACOSX_DEPLOYMENT_TARGET: "11.0" | ||
CIBW_ARCHS: all | ||
CIBW_PRERELEASE_PYTHONS: True | ||
CIBW_BUILD: ${{ matrix.python }}-${{ matrix.buildplat[1] }} | ||
# __init__.py interferes with the tests and is included as local file instead of | ||
# used from wheels. To be honest, tests should not be in the source folder at all. | ||
CIBW_BEFORE_TEST: rm {project}/tiledb/__init__.py | ||
with: | ||
output-dir: wheelhouse | ||
|
||
- uses: actions/upload-artifact@v4 | ||
with: | ||
name: cibw-wheels-${{ matrix.buildplat[1] }}-${{ strategy.job-index }} | ||
path: "./wheelhouse/*.whl" | ||
|
||
build_sdist: | ||
name: Build source distribution | ||
runs-on: ubuntu-latest | ||
outputs: | ||
sdist_name: ${{ steps.get_sdist_name.outputs.sdist_name }} | ||
steps: | ||
- uses: actions/checkout@v4 | ||
|
||
- name: Build sdist | ||
run: pipx run build --sdist | ||
|
||
- name: Get sdist package name | ||
id: get_sdist_name | ||
run: | | ||
echo "sdist_name=$(ls dist/ | head -n 1)" >> "$GITHUB_OUTPUT" | ||
|
||
- uses: actions/upload-artifact@v4 | ||
with: | ||
name: sdist | ||
path: dist/*.tar.gz | ||
|
||
test_sdist: | ||
name: Test source distribution package | ||
needs: [build_sdist] | ||
strategy: | ||
matrix: | ||
os: | ||
- macos-13 | ||
- macos-14 | ||
- windows-2022 | ||
- ubuntu-22.04 | ||
python: ["3.8", "3.9", "3.10", "3.11", "3.12"] | ||
runs-on: ${{ matrix.os }} | ||
steps: | ||
- name: Set up Python ${{ matrix.python }} | ||
uses: actions/setup-python@v4 | ||
with: | ||
python-version: ${{ matrix.python }} | ||
|
||
- name: Download sdist artifact | ||
uses: actions/download-artifact@v4 | ||
with: | ||
name: sdist | ||
path: dist | ||
|
||
- name: Install sdist artifact | ||
run: pip install --verbose dist/${{ needs.build_sdist.outputs.sdist_name }} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not blocking but we might want to change this later to run in parallel with the wheel builds, because this will have to build everything from source so it will take some time at the end that could be amortized. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not sure if I understand correctly, but this does run in parallel right now. All jobs (around 40) run in parallel, half is building wheels and half is building |
||
|
||
- uses: actions/checkout@v4 | ||
|
||
- name: Install test dependencies | ||
run: pip install pytest hypothesis psutil pyarrow | ||
|
||
- name: Run tests | ||
shell: bash | ||
run: | | ||
PROJECT_CWD=$PWD | ||
rm tiledb/__init__.py | ||
cd .. | ||
pytest -vv --showlocals $PROJECT_CWD | ||
|
||
upload_pypi: | ||
needs: [build_wheels, test_sdist] | ||
runs-on: ubuntu-latest | ||
environment: pypi | ||
permissions: | ||
id-token: write | ||
outputs: | ||
package_version: ${{ steps.get_package_version.outputs.package_version }} | ||
steps: | ||
- uses: actions/download-artifact@v4 | ||
with: | ||
path: dist | ||
merge-multiple: true | ||
|
||
- id: get_package_version | ||
run: | | ||
echo "package_version=$(ls dist/ | head -n 1 | cut -d - -f 2)" >> "$GITHUB_OUTPUT" | ||
|
||
- name: Upload to test-pypi | ||
uses: pypa/gh-action-pypi-publish@release/v1 | ||
with: | ||
repository-url: https://test.pypi.org/legacy/ | ||
|
||
- name: Upload to pypi | ||
if: startsWith(github.ref, 'refs/tags/') | ||
uses: pypa/gh-action-pypi-publish@release/v1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is relying on the file ordering, seems like it could be brittle? Can we construct the sdist name from the version? Or at least it should be
ls *.tar.gz
to be more specific.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There should be only one file in
/dist
since this is completely clean job. But adding*.tar.gz
is a good idea.