-
Notifications
You must be signed in to change notification settings - Fork 649
ci(wheels): unbreak wheel release + other enhancements pt 1 #4937
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
ci(wheels): unbreak wheel release + other enhancements pt 1 #4937
Conversation
Signed-off-by: Zach Lewis <[email protected]>
…ux wheels Signed-off-by: Zach Lewis <[email protected]>
Updates the default OpenColorIO build version to 2.5.0, and reconcile dependency required minimum versions accordingly: - expat-2.5 --> 2.6 - yaml-cpp-0.6 --> 0.8 - minizip-ng --> 4.0.10 Also: ci(wheels): macos-15-intel: brew uninstall minizip-ng and expat Signed-off-by: Zach Lewis <[email protected]>
per OCIO-2.5.0 Signed-off-by: Zach Lewis <[email protected]>
Signed-off-by: Zach Lewis <[email protected]>
fixes issue with wheels not getting released to pypi.org! Signed-off-by: Zach Lewis <[email protected]>
For some reason, the build script seems to scream in agony when trying to use ld.gold to link ccache. No matter -- having fixed a typo in build_ccache.bash, we can download a pre-built ccache and be on our way. Signed-off-by: Zach Lewis <[email protected]>
Signed-off-by: Zach Lewis <[email protected]>
also add the -f flag to force homebrew to try to uninstall stuff even if it isn't installed Signed-off-by: Zach Lewis <[email protected]>
Signed-off-by: Zach Lewis <[email protected]>
This is just to silence warnings in the workflow summary view. It shouldn't affect anything about the builds themselves. Signed-off-by: Zach Lewis <[email protected]>
A little while ago, we'd switched to forcing cmake to generate "Unix Makefiles" -- this commit reverts that change, in an effort to remove superstitious settings in our workflow file! Signed-off-by: Zach Lewis <[email protected]>
This reverts commit 9c02b73. Signed-off-by: Zach Lewis <[email protected]>
|
You seem to be making a lot of adjustments. Let us know when it's ready to review. |
|
This is ready for review -- I don't intend to make any more adjustments. |
| macos: | ||
| name: Build wheels on macOS | ||
| runs-on: macos-13 | ||
| runs-on: macos-15-intel |
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.
Oops, my bad, I totally didn't think about the wheels when I did this very change on the regular CI.
| echo "ARCH=$ARCH" | ||
|
|
||
| CCACHE_PREBULT=${CCACHE_PREBULT:=1} | ||
| CCACHE_PREBUILT=${CCACHE_PREBUILT:=1} |
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.
Oh boy, that one is embarrassing
src/cmake/build_OpenColorIO.cmake
Outdated
| ###################################################################### | ||
|
|
||
| set_cache (OpenColorIO_BUILD_VERSION 2.4.2 "OpenColorIO version for local builds") | ||
| set_cache (OpenColorIO_BUILD_VERSION 2.5.0 "OpenColorIO version for local builds") |
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 feels like an orthogonal decision to anything to do with the wheels, because it will change the version of OCIO that anyone will get for the auto-builder. Are we confident enough in 2.5.0 to make it the default?
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.
Good point -- I had only been considering that users locally pip-installing (building) OIIO would want the same OCIO behavior as the available prebuilt wheels. It totally skipped my mind that this affects the default auto-building behavior everywhere!
I am confident enough in 2.5.0 to make it the default, in terms of stuff not breaking; but I agree with you that this change is way out of scope for this PR! I'll revert it here, and open a new PR just for this sucker so we can further discuss.
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.
I feel like without a serious beta/RC cycle, OCIO is treating 2.5.0 as their release candidate and a future 2.5.1 as the one where stuff actually gets fixed. I think 2.5 hasn't actually had a chance to prove itself in production quite yet, so I'm cautious about making it the default. Anybody can override by setting either cmake or env variable OpenColorIO_BUILD_VERSION to 2.5.0, but the default should be playing it safe with a well-tested version we are 100% confident in.
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.
My thinking was that we'd want the version of OCIO available on pypi to match what the version of OIIO available on pypi was built against.
But I think that's a wise decision, to regard 2.5.0 as a release candidate, and I agree with you about holding off until 2.5.1 rolls out. Let's do that.
And if folks really want, they can locally build the OIIO bindings against OCIO 2.5 with:
$ pip install OpenImageIO --config-settings=cmake.define.OpenColorIO_BUILD_VERSION="2.5.0"
keep these commits clean! Signed-off-by: Zach Lewis <[email protected]>
|
Nice speed bump from not having to build ccache from source on Linux Intel. Is that just because you eliminated the jobs that involved the older glibc? Are there pre-built ccache we can use for the linux arm wheels? |
e1422ca to
e606d70
Compare
We can address updating to 2.5.0 in another PR Signed-off-by: Zach Lewis <[email protected]>
I hadn't considered that, but... yeah, having eliminated the manylinux2014 (glibc-2.17) builds, we're able to use the pre-built ccache for the remaining manylinux_2_28 (glibc-2.28) builds for the time being.
Not officially, as far as I can tell. The developers have only recently (as of ccache-4.12.0, I think) started to release aarch64-compatible pre-built binary distributions, and those seem to require a newer version of glibc than we'd like to support (w.r.t. current vfx reference platform recommendations). One option we could investigate is building ccache in a custom docker image based on the manylinux_2_28 image. Another option is to try switching from ccache to sccache, which seems to be a more purpose-built solution for what we're trying to do. |
lgritz
left a comment
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.
LGTM, if this is ready to go for you, @zachlewis
|
Thanks Larry! Seems good to go to me too. |
…oftwareFoundation#4937) ## Description This PR fixes and improves a handful of wheels-related items: - Fix an issue where wheels produced with v3.1 releases were not getting uploaded to pypi - Update cibuildwheel to latest version (v3.2.1) - Remove the manylinux2014 builds, replace with manylinux_2_28 builds where applicable - Add CPython 3.14 wheels - Update macos-x86 runner to `macos-15-intel` - Update Python typestubs ## Notes - This PR ends support for pre-built wheels for glibc < 2.27 (CentOS-7 etc). However, the module can still be pip-installed locally, provided devtoolset-7+ (e.g., gcc-9+) is available. - Unless we arrive at a more elegant solution, we must remember to change the conditional in the "upload_pypi" step for every major OIIO release. --------- Signed-off-by: Zach Lewis <[email protected]>
Description
This PR fixes and improves a handful of wheels-related items:
macos-15-intelNotes