AcademySoftwareFoundation / OpenTimelineIO

Open Source API and interchange format for editorial timeline information.
http://opentimeline.io
Apache License 2.0
1.4k stars 276 forks source link

Updated cibuildwheel settings to generate apple silicon wheels #1673

Open reinecke opened 8 months ago

reinecke commented 8 months ago

Fixes #1672

This is a WIP PR for enabling Apple Silicon Support.

Currently, this PR simply enables cibuildwheel for x86_64 and arm64.

TODO:

codecov-commenter commented 8 months ago

Codecov Report

Merging #1673 (dc3ff98) into main (745e614) will not change coverage. The diff coverage is n/a.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1673/graphs/tree.svg?width=650&height=150&src=pr&token=mfnfUB4h7u&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation)](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1673?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation) ```diff @@ Coverage Diff @@ ## main #1673 +/- ## ======================================= Coverage 79.84% 79.84% ======================================= Files 197 197 Lines 21767 21767 Branches 4354 4354 ======================================= Hits 17379 17379 Misses 2234 2234 Partials 2154 2154 ``` | [Flag](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1673/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation) | Coverage Δ | | |---|---|---| | [py-unittests](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1673/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation) | `79.84% <ø> (ø)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation#carryforward-flags-in-the-pull-request-comment) to find out more. ------ [Continue to review full report in Codecov by Sentry](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1673?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1673?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation). Last update [745e614...dc3ff98](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1673?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation).
JeanChristopheMorinPerso commented 8 months ago

Decide which wheels we feel like we need (is there a demand for universal2 or are the optimized wheels fine?)

Maybe look at https://cibuildwheel.readthedocs.io/en/stable/faq/#what-to-provide?

Figure out how to do testing on Apple Silicon since arm64 wheels can't be tested on x86_64

I hear through the branches that it's coming. See https://github.blog/2023-10-02-introducing-the-new-apple-silicon-powered-m1-macos-larger-runner-for-github-actions/#how-to-use-the-runner and https://github.com/AcademySoftwareFoundation/wg-ci/blob/main/meetings/2023-10-11.md?plain=1#L89. I'll check to see if we could have access.

reinecke commented 8 months ago

I spot checked the Apple Silicon python 3.11 wheel from the first build.

I unzipped that dir into /tmp/otiowheels and installed from it using the following command:

python -m pip install --find-links /tmp/otiowheels opentimelineio==0.16.0.dev1

I was then able to use make ci-postbuild to run tests against the installed wheel.

Our CI currently has a py_build_test job and a package_wheels job. These redundantly build OTIO on all the platforms but py_build_test just does a pip install .[dev] -v whereas package_wheels uses cibuildwheel to make the final artifacts.

I think a good refactor might be to use have one job that just runs the lint and check-manifest in a single platform (like we do with package_sdist), then a dependent job that runs package_wheels to generate the artifacts, and finally make a dependent job that pulls down the artifacts, installs the right one for the platform, and runs tests (basically use these steps, put a step in to pull down the built wheels and install from them, and then run these steps).

This would have some specific benefits:

The one downside I see is that cibuildwheel seems to take ~5m to build whereas pip install . is taking closer to ~2m in many instances. I suspect this is because cibuildwheel downloads and installs it’s own python. My personal feeling is that the benefits outweigh the extra few minutes.

reinecke commented 8 months ago

Another consideration - universal binaries are a bit slower to import possibly - what's the tax of this?

From TSC meeting: let's go with x86_64 + universal2 archs