patrikhuber / eos

A lightweight 3D Morphable Face Model library in modern C++
Apache License 2.0
1.89k stars 596 forks source link

Adding cibuildwheel workflow to build and publish wheels #361

Closed omar-h-omar closed 5 months ago

omar-h-omar commented 9 months ago

@patrikhuber Here's a complete workflow. It builds wheels for all systems and supports all python versions. The only exceptions are Pypy and i686 as those are not fully supported by numpy. The following workflow will run a simple python test after building a wheel to confirm that it works as expected. It will also publish wheels to PyPi. As of now it is triggered by pushes to master but let me know if you'd want it to be enabled for all branches.

omar-h-omar commented 9 months ago

Test Resources Test PyPi: https://test.pypi.org/project/omar-eos-py/ Latest Successful Build: https://github.com/omar-h-omar/eos/actions/runs/6370010380

patrikhuber commented 8 months ago

Hi @omar-h-omar,

Thank you very much, that is all really amazing! I also gave this a try and tested it on https://test.pypi.org/, just so I'll know how to configure it, and it all worked.

Just a couple small questions:

CIBW_TEST_SKIP: "*-macosx_arm64 *-macosx_universal2:arm64"

I think this means that the tests are skipped on these two platforms, but I think we are building the wheels for them (?). What's the reason for skipping these?

And, you mentioned in your email that the test doesn't work yet for Python 3.12 because NumPy wasn't available yet, and I think you've then disabled Python 3.12? In my latest run, it looks like it did build and test the wheels on Python 3.12 successfully: https://github.com/patrikhuber/eos-testpypi/actions/runs/6517348994. As Python 3.12 final officially came out a couple weeks ago, this is probably all just automatically resolved now, correct?

I'll merge this in a moment - I think it's all good to go!

omar-h-omar commented 8 months ago

Hi @patrikhuber,

It's great to see this working!! I really think it would help eos a lot.

So the reason for skipping some tests is because cibuildwheel currently doesn't support testing arm64 macos wheels on github action runners. Thus, I've added the skip to avoid any warnings in the build logs. See point number 2 below:

Screenshot 2023-10-15 at 4 32 16 pm

As for Python 3.12, I think the email I sent was before the official release. I had forgotten to mention that the issue was resolved since then. The current build does support 3.12 like you saw.

Yeah I think we should good to go with this PR :))

patrikhuber commented 5 months ago

Hi @omar-h-omar,

Sorry I haven't gotten back to this - I think I tested it all back in Oct/Nov but then didn't actually do the final merge / setup. Let's merge this now, fingers crossed that it still works! (I don't see why not, but you never know.)