Use cibuildwheel; build MacOS universal

libgit2 / pygit2

Python bindings for libgit2

https://www.pygit2.org/

Other

1.58k stars 382 forks source link

Use cibuildwheel; build MacOS universal #1109

Closed simoncozens closed 2 years ago

simoncozens commented 2 years ago

This does two things:

Uses the cibuildwheel library and action to build the wheels, which simplifies the GitHub action somewhat.
Compiles universal builds of openssl, libssh2, libgit2 and pygit2 on macOS. (Fixes #1102)

jdavid commented 2 years ago

Great job! I didn't know about cibuildwheel

There's quite some duplication in the build.sh, install-deps.sh and install-deps-mac.sh scripts. I'd prefer to have only one build.sh. So we will need a section:

if [ -n "$OPENSSL_VERSION" ]; then
    [...]
fi

It's okay if it only works with macOS for now.

Some other things that are now done unconditionally should be in such blocks. For example:

if [ "$1" = "wheel" ]; then
    [...]
else
    [...]
fi

May become:

if [ "$1" = "wheel" ]; then
    [...]
elif [ "$1" = "inplace" ]; then
    [...]
fi

So at the end we can have just 1 script.

simoncozens commented 2 years ago

Ah, yes, fair comment. I will have a look at merging them back into one script on Monday.

simoncozens commented 2 years ago

OK, I think this is done - inplace tests and CI builds both working.

jdavid commented 2 years ago

Looks good!

Just a few comments:

Now the artifacts are uploaded with the name artifact while the download step uses the name wheels. Whatever names used, they mast match, otherwise the pypi job will fail.
The misc/build-all-manylinux-wheels.sh and misc/build-manylinux-wheels.sh are not used anymore, so they can be removed.
Please update to cibuildwheel v2.3.0