BLD Uses cibuildwheel for linux + osx wheels [cd build] by thomasjpfan · Pull Request #20102 · numpy/numpy

thomasjpfan · 2021-10-11T20:20:40Z

Related to MacPython/numpy-wheels#116

This PR:

Enables x86 linux+osx wheels to be built with cibuildwheel.
Allows PR to trigger the wheel build [cd build] is in the commit message, where "cd" means "continuous delivery"

I kept this PR small by only supporting two platforms. Follow up includes:

Windows support
Cross compile universal2 support for macOS
Upload to nightly index at https://pypi.anaconda.org/scipy-wheels-nightly/simple
Maybe linux arm support using QEMU depending on how fast it is

Note: The big diff comes from the licenses that was copied over from numpy-wheels.

mattip · 2021-10-11T20:58:35Z

Is there a reason you are doing this here rather than at Macpython/numpy-wheels ?

charris · 2021-10-12T01:32:20Z

.github/workflows/wheels.yml

+          NPY_USE_BLAS_ILP64: 1
+          CIBW_BUILD: cp${{ matrix.python }}-${{ matrix.platform }}
+          CIBW_MANYLINUX_X86_64_IMAGE: manylinux2014
+          CIBW_ENVIRONMENT_LINUX: CFLAGS='-std=c99 -fno-strict-aliasing'


Is -std=c99 definitely needed for the newer compilers? IIRC, there was some discussion that might disable some newer features.

It's only needed for GCC 4.x IIRC, and we should drop that now that we no longer need manylinux1.

Although we should separate dropping that, and not mix it with a CI reshuffle - so the flag is best kept as is here.

how will we remember to do this?

.github/workflows/wheels.yml

thomasjpfan · 2021-10-12T03:12:31Z

Is there a reason you are doing this here rather than at Macpython/numpy-wheels ?

There are some benefits of moving it to numpy/numpy:

Wheels can be built directly in PRs using a commit containing [cd build].
Do not need to keep numpy-wheels and numpy in sync anymore.

I am also okay with moving this over to MacPython/numpy-wheels. Overall, I think the move to cibuildwheel would make it easier to get wheels built when Python RCs are released.

CC @rgommers Do you have thoughts on moving wheel building to numpy/numpy?

This reverts commit 9b45d83.

rgommers · 2021-10-12T08:52:43Z

CC @rgommers Do you have thoughts on moving wheel building to numpy/numpy?

I think it's time for that. The one advantage of MacPython/numpy-wheels is that we have TravisCI credits there for aarch64/ppc64le builds. But other than that I think it mostly has downsides: hard to discover, more effort to maintain and search for issues because of the split over repos, not enough people paying attention to broken CI because it's a separate repo, etc. We also never intended the repo to be under MacPython, that's an accident of history.

There's a separate question about auto-uploading to PyPI - I know some other projects like it, but I'm still weary of that. Uploading to the Anaconda staging bucket, and having the release manager download those and re-upload when they're ready seems safer to me both release mechanics wise and security wise (e.g., recent codecov security breach exposed secrets).

rgommers · 2021-10-12T08:54:34Z

Thanks a lot for working on this @thomasjpfan! Moving to cibuildwheel will be valuable long-term.

rgommers · 2021-10-12T08:57:51Z

tools/wheels/cibw_before_build.sh

+
+# Install GFortran
+if [[ $UNAME == "Darwin" ]]; then
+    # same version of gfortran as the open-libs and numpy-wheel builds


typo: open-libs -> openblas-libs

rgommers · 2021-10-12T08:58:42Z

.github/workflows/wheels.yml

+          NPY_USE_BLAS_ILP64: 1
+          CIBW_BUILD: cp${{ matrix.python }}-${{ matrix.platform }}
+          CIBW_MANYLINUX_X86_64_IMAGE: manylinux2014
+          CIBW_ENVIRONMENT_LINUX: CFLAGS='-std=c99 -fno-strict-aliasing'


Although we should separate dropping that, and not mix it with a CI reshuffle - so the flag is best kept as is here.

rgommers

This looks like it will be a very pleasant change of pace, no more bash & submodules:)

tools/wheels/cibw_test_command.sh

rgommers · 2021-10-12T09:02:24Z

.github/workflows/wheels.yml

+
+on:
+  schedule:
+    # Nightly build at 1:42 A.M.


add time zone?

rgommers · 2021-10-12T09:04:21Z

.github/workflows/wheels.yml

+        with:
+          submodules: true
+          # The complete history is required for versioneer.py to work
+          fetch-depth: 0


I don't think it is - we just need enough commits to have the previous tag. In gitpod.Dockerfile it does git clone --shallow-since=2021-05-22

I think this is right. You need enough to get to the tag that started the current version. It is hard to get it right in a maintenance free way. Once could alternatively assume that NumPy will always have fewer than XXXX commits between releases, and then use that.

We tried the "guess the number of commits" and have been wrong before - so probably using the date and bumping it once a year is easier/safer.

I can't find a simple way to get the git describe (versioneer.py) to work with shallow clones:

git clone --shallow-since=2021-05-22 https://github.com/numpy/numpy cd numpy git describe # fatal: No names found, cannot describe anything.

Currently, a possible solution is:

git clone --depth=1 https://github.com/thomasjpfan/numpy numpy cd numpy # pulls in branch with tags git fetch --prune origin +d8922518c4ec66d91d16e5549bd9b9390cfee678:refs/remotes/origin/ci_buildwheel_rf_shallow # checkout branch git checkout --progress -B ci_buildwheel_rf_shallow refs/remotes/origin/ci_buildwheel_rf_shallow git describe # v1.22.0.dev0-1425-gd8922518c

I feel like that is a bit too complex compare to fetch-depth: 0.

Note: There is an open issue to get this workflow to work: actions/checkout#338

Agreed that that's too complex - so +1 for doing whatever is simplest here.

I can't find a simple way to get the git describe (versioneer.py) to work with shallow clones:

versioneer continues to be a pain:(

For this PR, I think we leave fetch-depth: 0 for now until actions/checkout#338 gets resolved. It currently takes ~ 14 seconds to fetch the entire history, so I think it is okay for the time being.

I note that --shallow-exclude=v1.21.0.dev0 works. Unfortunately, --shallow-exclude=v1.22.0.dev0^ doesn't.

rgommers · 2021-10-12T09:06:49Z

.github/workflows/wheels.yml

+          CIBW_BEFORE_TEST: pip install -r {project}/test_requirements.txt
+          # Installs libffi so that cffi can be built from source when needed
+          CIBW_BEFORE_TEST_LINUX: >
+            [[ "${{ matrix.install_libffi_devel }}" == "1" ]] && yum install -y libffi-devel;


What do we use libffi-devel for, just some optional tests or something more interesting?

libffi-devel is used to build cffi from source if wheels do not exists. cffi is required by test_requirements here:

numpy/test_requirements.txt

Lines 9 to 10 in e869222

# for numpy.random.test.test_extending

cffi

It would be fine to make cffi a soft requirement: if it is unavailable we could skip that test. Is there a way to do that without too much hassle? The test takes the failed import into account

numpy/numpy/random/tests/test_extending.py

Lines 9 to 13 in e869222

try:

import cffi

except ImportError:

cffi = None

Updated test_requirements.txt, to make cffi a soft requirement for python 3.10:

cffi; python_version < '3.10'

.github/workflows/wheels.yml

EpicWink · 2021-10-19T09:11:39Z

There's a separate question about auto-uploading to PyPI - I know some other projects like it, but I'm still weary of that. Uploading to the Anaconda staging bucket, and having the release manager download those and re-upload when they're ready seems safer to me both release mechanics wise and security wise (e.g., recent codecov security breach exposed secrets).

If PyPI implements draft releases (GitHub PR) (Discourse topic), you could switch to PyPI for this workflow

thomasjpfan · 2021-10-19T13:09:50Z

Uploading to the Anaconda staging bucket, and having the release manager download those and re-upload when they're ready seems safer to me both release mechanics wise and security wise

A follow up to this PR is to upload to an Anaconda staging bucket. From there you can follow your current release workflow of downloading and re-uploaded.

mattip

As far as I can tell there is only one nit about a future PR to remove compilation flags, and this looks good to go.

Edit: and the versioneer problem

.github/workflows/wheels.yml

mattip · 2021-10-19T14:34:26Z

.github/workflows/wheels.yml

+        with:
+          submodules: true
+          # The complete history is required for versioneer.py to work
+          fetch-depth: 0


mattip · 2021-10-19T14:34:56Z

.github/workflows/wheels.yml

+          NPY_USE_BLAS_ILP64: 1
+          CIBW_BUILD: cp${{ matrix.python }}-${{ matrix.platform }}
+          CIBW_MANYLINUX_X86_64_IMAGE: manylinux2014
+          CIBW_ENVIRONMENT_LINUX: CFLAGS='-std=c99 -fno-strict-aliasing'


how will we remember to do this?

tools/wheels/cibw_test_command.sh

.github/workflows/wheels.yml

mattip · 2021-10-20T06:08:47Z

One last thing: it would be good to document the [cd build] string somewhere. See also #18343 which is still open.

mattip · 2021-10-20T18:12:01Z

We discussed this at the weekly triage meeting. One workflow that is not clear is the desire to kick off all the different CI that will be used to build wheels against a commit (This PR uses github actions. Currently travis-ci is used for arm64, ppc64le, and s390x. We have concerns that the arm64 builds on travis are flaky and are looking for alternatives, so we may end up with a third CI service). Since we use at least two services, a button to trigger builds on github actions would not be sufficient to complete the task. Currently, the MacPython/numpy-wheels workflow allows us to make a commit to that repo, which then pulls the appropriate commit from here and builds a wheel. If we merge this PR to numpy/numpy, then we could continue with that idea: make a commit to trigger wheel building. But then that commit will become part of the permanent numpy git history. Is that the way other projects use cibuildwheel?

There is also some concern that a run of this CI job will push wheels to anaconda.org, these would be marked as the "latest dev wheels" and picked up by projects building off of latest NumPy wheels. It seems that is a bit too open. Is there a way to limit the ability to upload?

thomasjpfan · 2021-10-21T14:21:14Z

One workflow that is not clear is the desire to kick off all the different CI that will be used to build wheels against a commit

For triggering a build on another service for a specific PR, we can have a button to trigger builds in github actions that calls the other CI services' API to trigger the build. Another option is to manually trigger a build for a specific commit in the CI services' UI.

But then that commit will become part of the permanent numpy git history. Is that the way other projects use cibuildwheel?

I think every project is a little different on how they use cibuildwheel. For scikit-learn, we do have these "build" commit messages in the release/maintenance branches to trigger the build. This places the "which commit the wheel was built on" in the scikit-learn/scikit-learn repo. If numpy maintainers do not wish to have commits for wheel building in the main repo, then an option is to trigger different CI services as suggested above, which does not need a specific commit message.

There is also some concern that a run of this CI job will push wheels to anaconda.org, these would be marked as the "latest dev wheels" and picked up by projects building off of latest NumPy wheels. It seems that is a bit too open. Is there a way to limit the ability to upload?

There are ways to limit the upload. What kind of limits are you thinking of?

mattip · 2021-10-22T11:52:07Z

@charris - thoughts?

EpicWink · 2021-10-24T08:19:22Z

FYI cibuildwheel just released musllinux support

mattip · 2021-10-31T05:04:53Z

In recent community/triage discussions we decided to put this in. TBD:

upload weekly to https://anaconda.org/scipy-wheels-nightly/numpy and https://anaconda.org/multibuild-wheels-staging/numpy for releases
document the process at https://numpy.org/devdocs/dev/releasing.html

I would like to put this in as-is and do the rest in future PRs. Does that make sense?

mattip · 2021-11-01T09:31:53Z

Thanks @thomasjpfan

thomasjpfan · 2021-11-01T12:32:45Z

I would like to put this in as-is and do the rest in future PRs. Does that make sense?

Your plan looks good to me.

BLD Uses cibuildwheel for linux/osx wheels [cd build]

a83f0f2

github-actions bot added the 36 - Build Build related PR label Oct 11, 2021

thomasjpfan added 2 commits October 11, 2021 16:22

STY Lint [cd build]

c0e3231

MAINT Filter for workflow_dispatch [cd build]

500d4c6

ENH Adds python 3.10 [cd build]

cafe3cf

charris reviewed Oct 12, 2021

View reviewed changes

.github/workflows/wheels.yml Show resolved Hide resolved

thomasjpfan added 3 commits October 11, 2021 21:41

ENH Do not always install libffi-devel

cead9bf

ENH Only allow numpy/numpy to build [cd build]

126316e

REV Revert libffi flag [cd build]

9b45d83

Revert "REV Revert libffi flag [cd build]"

c9abce5

This reverts commit 9b45d83.

rgommers reviewed Oct 12, 2021

View reviewed changes

thomasjpfan added 5 commits October 13, 2021 16:47

CLN Address comments

8ce8489

CLN Remove restrictions for easier debugging on forks [cd build]

27308ef

WIP Restrict install cffi in test_requirements.txt

8b1e204

BLD [cd build]

54676b0

WIP Restrict install cffi in test_requirements.txt [cd build]

fdda3ca

jorisvandenbossche mentioned this pull request Oct 14, 2021

Consider cibuildwheel for building wheels? pandas-dev/pandas#44027

Closed

thomasjpfan added 3 commits October 14, 2021 08:13

DOC Improve versioneer.py comment

7dcc3c1

Merge remote-tracking branch 'upstream/main' into ci_buildwheel_rf

aae8453

BLD [cd build]

cb3d7ef

mattip reviewed Oct 19, 2021

View reviewed changes

mattip reviewed Oct 20, 2021

View reviewed changes

.github/workflows/wheels.yml Show resolved Hide resolved

mattip mentioned this pull request Oct 31, 2021

Move to github actions, cibuildwheel pyproj4/pyproj-wheels#42

Closed

mattip merged commit fa397e9 into numpy:main Nov 1, 2021

mattip mentioned this pull request Nov 1, 2021

BUILD: use cibuildwheel to upload wheels #20262

Closed

3 tasks

peterjc mentioned this pull request Nov 3, 2021

multibuild vs cibuildwheel multi-build/multibuild#416

Closed

moble mentioned this pull request Nov 4, 2021

Build musl aarch64 wheels moble/quaternion#188

Open

thomasjpfan mentioned this pull request Nov 9, 2021

MAINT: Rename commit trigger to "wheel build" for building wheels #20334

Merged

cphyc mentioned this pull request Dec 14, 2021

REL: update cibuildwheel, simplify wheel building workflow yt-project/yt#3715

Merged

h-vetinari mentioned this pull request Feb 10, 2022

CI: make sure CI stays on VS2019 unless changed explicitly #20524

Merged

skoudoro mentioned this pull request Mar 4, 2022

[Upcoming] Release 1.5.0 dipy/dipy#2462

Merged

23 tasks

mayeut mentioned this pull request Mar 5, 2022

BLD: move cibuildwheel configuration to pyproject.toml #21128

Merged

tgross35 mentioned this pull request Mar 15, 2022

ENH: upload musllinux wheels to PyPI #20089

Closed

h-vetinari mentioned this pull request Sep 11, 2022

TEST: status of cffi & python 3.10 #22244

Open

stefan6419846 mentioned this pull request Oct 26, 2023

-fno-strict-aliasing usage - is it needed, and if so do we default to it? #25004

Open

lorentzenchr mentioned this pull request Apr 9, 2024

Correctly document linked libraries scikit-learn/scikit-learn#27559

Closed

Uh oh!

Conversation

thomasjpfan commented Oct 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mattip commented Oct 11, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

thomasjpfan commented Oct 12, 2021

Uh oh!

rgommers commented Oct 12, 2021

Uh oh!

rgommers commented Oct 12, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rgommers left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

EpicWink commented Oct 19, 2021

Uh oh!

thomasjpfan commented Oct 19, 2021

Uh oh!

mattip left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mattip commented Oct 20, 2021

Uh oh!

mattip commented Oct 20, 2021

Uh oh!

thomasjpfan commented Oct 21, 2021

Uh oh!

mattip commented Oct 22, 2021

Uh oh!

thomasjpfan commented Oct 11, 2021 •

edited

Loading

mattip left a comment •

edited

Loading

EpicWink commented Oct 24, 2021 •

edited

Loading