Skip to content
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

feat: Python wheels workflow and build backend #4428

Open
wants to merge 155 commits into
base: main
Choose a base branch
from

Conversation

zachlewis
Copy link
Contributor

@zachlewis zachlewis commented Sep 16, 2024

Description

Summary

This PR is the spiritual successor to #4011. It implements a scikit-build-core-based python build-backend, making it possible to use pip install . to build from source; and it adds a Github workflow for building with cibuildwheel and publishing to pypi.org binary distributions (bdists) of the Python module / extensions / CLI tools for cpython 3.8-3.12, across major operating systems and architectures.

When you pip install OpenImageIO, pip attempts to retrieve an OpenImageIO bdist from pypi.org for the host's platform / architecture / Python interpreter. If it can't find something appropriate, pip will attempt to build locally from the OpenImageIO source distribution (sdist), downloading and temporarily installing cmake and ninja if necessary.

PEP-Compliant Packaging: pyproject.toml

The pyproject.toml file is organized in three parts:

  1. Package metadata: standard attributes identifying and describing the Python package, its run-time and build-time requirements, entry-points to executable scripts, and so forth.
  2. scikit-build-core options: governs how pip install ... interacts with cmake.
  3. cibuildwheel options: additional steps and considerations for building, packaging, "repairing", and testing relocatable wheel build artifacts in isolated environments.

Additions to __ init __.py

Previously, we were using a custom OpenImageIO/__ init__.py file to help Python-3.8+ on Windows load the shared libraries linked by the Python module (i.e., the .dll files that live alongside oiiotool.exe under $PATH).

This PR adds an additional method for loading the DLL path, necessitated by differences between pip-based and traditional CMake-based installs.

It also adds a mechanism for invoking binary executables found in the .../site-packages/OpenImageIO/bin directory. This provides a means for exposing Python script "shims" for each CLI tool, installed to platform-specific locations under $PATH, while keeping the actual binaries in a static location relative to the dynamic libraries. Upshot is, in pyproject.toml,
each item under [project.scripts] is turned into a Python script upon installation that behaves indistinguishably to the end user to the CLI binary executable of the same name.

Relocatable Binary Distributions with cibuildwheel + repairwheel

cibuildwheel is a widely-used tool for drastically streamlining the process of building, repairing, and testing Python wheels across platforms, architectures, interpreters, and interpreter versions.

Importantly, cibuildwheel allows for an arbitrary "repair-wheel-command" step, intended as a means for collecting and bundling any external shared libraries linked by the module (or the CLI tools) living outside the package, and subsequently patching RPATH entries to point to relative paths to these libraries inside the wheel. This ensures that compiled extensions have no runtime dependencies outside the pip toolchain.

In order to keep wheel file sizes to an absolute minimum, our repair-wheel-command is a multi-step process, implemented in a new tasks.py file living in the repo's root, invoked with invoke:

  1. Remove everything except for the python module and the binary executables under /bin.
  2. Use repairwheel to collect and bundle only and exactly the shared libraries needed -- i.e., missing dependencies built by OIIO's build system, as well as any other needed libraries found in the environment.

Additionally, the cibuildwheel-based builds set CMAKE_BUILD_TYPE to "MinSizeRel" to optimize for size (instead of speed) -- this seems to shave ~1.5MB off each .whl's size.

Note: the steps taken in tasks.py remove everything except that which is absolutely necessary for end users to import the Python modules and run the CLI tools, partly in order to determine a baseline minimum file size for the wheels. However, part of the motivation for setting things up the way I have is to make it easier to add things back into the wheels in the future; e.g., should we decide to package the headers and/or the fonts, generate or modify pkgconfig .pcs and/or CMake modules and configs, etc. Also worth noting, there's pyproject.toml metadata we can add to help downstream scikit-build-core-based builds find OpenImageIO.

"Wheels" Github workflow

I straight-up copied .github/workflows/wheel.yml from OpenColorIO and made a few OIIO-specific modifications. When pushing a commit tagged v2.6* or v3*, the workflow will invoke a platform-agnostic "build sdist" (source distribution) task, followed by a series of tasks for building OIIO wheels for cpython-3.8-3.13 on Windows, Linux (x86_64 + aarch64, new libstdc++), and MacOS (x86_64 + arm64, min deployment 10.15) and persisting build artifacts; followed finally by a task for publishing the build artifacts to pypi.org

Note: For the sake of simplicity and troubleshooting, I've made as few changes to OpenColorIO's wheel.yml as I could get away with; but in the future, we can also build wheels for the PyPy interpreter, and possibly pyodide.

Note: THE PUBLISH TASK WILL FAIL until a "trusted publisher" is set up on pypi.org. See https://docs.pypi.org/trusted-publishers/creating-a-project-through-oidc/

I've submitted separate PRs for these, since they fall outside the intended scope of this PR.

Note: One of the aims for this PR is to leverage OIIO's new dependency-self-building mechanisms as much as possible, so that users who pip install from source are guaranteed a nominally working installation. However, we can also use platform-specific package managers for the wheels uploaded to pypi.org (e.g., added as cibuildwheel "before-build" commands for each platform).

Other Changes

I made some minor adjustments to pythonutils.cmake and fancy_add_executable.cmake that only affect scikit-build-core-based installs -- namely, on Linux and macOS, I'm setting the INSTALL_RPATH property to point to the relative path to the dynamic libraries, for the Python module and CLI tools, respectively. This helps ensure that pip-based builds and installs from source (as opposed to installs from repaired, pre-built wheels) yield relocatable, importable packages, without needing to mess with $LD_LIBRARY_PATH etc.

Tests

The cibuildwheel "test-command" is oiiotool --help. If that command elicits code zero, it means the "oiiotool" Python script installed by the wheel is able to import OpenImageIO; that the actual binary executable oiiotool is properly packaged and exists in the expected location (e.g., at .../site-packages/OpenImageIO/bin); and that all runtime dependencies are found.

Inspiration, Credit, Prior Art

  • @aclark4life's and @JeanChristopheMorinPerso's efforts + direction + discussion + advice. See #3249, and #4011, as well as JCM's python_wheels and python_wheels_windows branches. This PR is an attempt to leverage OIIO-2.6+ self-building-dependency features with # 4011's minimalist and modern approach to packaging.
  • OpenColorIO -- I tried to copy as much as I could from @remia et al's fantastic work with all things wheels-related. The __init __.py modifications, the way we're wrapping the CLI tools, and the github Wheels workflow are lifted almost-verbatim from OCIO. Insert pun about reinventing the wheel here.
  • @joaovbs96's help and patience with testing stuff on Windows.

zachlewis and others added 30 commits September 16, 2024 10:55
The build system has been updated to specifically detect the Python3 Development.Module meta component, as opposed to the entire Development component. This allows for better compatibility with python distributions that do not provide the Development.Embed component, which is only required for projects that ship embedded Python interpreters. The changes have been made in CMakeLists.txt and pythonutils.cmake files.

Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
Scikit-build-core is used for collecting CMake and Ninja as needed, and for invoking the build. When invoked via cibuildwheels, `repairwheel` is used after each build to re-bundle and relink the shared library dependencies into properly redistributable whl archives. The command-line tools are exposed under the [project.scripts] section.
This commit incorporates or is otherwise inspired by similar efforts by @aclark4life and @JeanChristopheMorinPerso, as well as @remia's work on the OpenColorIO wheels.

Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
Use Apache-2.0 license identifier instead of BSD-3-Clause.
Add "OpenImageIO Contributors" / "oiio-dev@lists.aswf.io" as author.
I could not bring myself to remove Larry as an author and maintainer.

Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
…tic libdeflate

It's now possible to disable building of shared `libdeflate` libs.

Also, we're checking for and aliasing `libdeflate` in `externalpackages.cmake`, just before checking for TIFF, as opposed to only doing so within `build_TIFF.cmake`. This change is necessary for certain build systems and pipelines that utilize cached dependency builds.

Specifically, when building wheels for multiple versions of cpython, `cibuildwheel` would complete the first build, and then throw an exception on the *second* build re: not being able to find `Deflate::Deflate`. Moving the aliasing above the check for TIFF ensures that the expected aliasing always takes place, whether or not TIFF needs to be built; whereas before, we were only creating the alias when initially building TIFF.

Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
Build and link missing libjpeg-turbo shared + static libs

Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
Instead of creating a separate OpenImageIO.OpenImageIO.command_line module for the CLI shims, move the CLI shim logic up to a "_command_line()" method in OpenImageIO.__init__.py.
(Maybe this method should still be called "main()" though?)

This also means the module is technically importable from OpenImageIO.OpenImageIO, but that's an improvement over OpenImageIO.OpenImageIO.OpenImageIO...!

Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
…oftwareFoundation#4358)

As suggested by Moritz Moeller

Signed-off-by: Larry Gritz <lg@larrygritz.com>
Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
…twareFoundation#4359)

* Get rid of some obsolete cmake code.

* Movement (but no change) to some parts of CMakeLists.txt, primarily to
make it closer to the corresponding file in OSL to make it easy for me
to diff them and port innovations back and forth between them.

* Some typo/etc fixes

* Remove unused OIIO_UNUSED_OK macro that's been deprecated since 2.0,
and OIIO_CONSTEXPR and OIIO_CONSTEXPR14, neither of which have been
needed for years.

Signed-off-by: Larry Gritz <lg@larrygritz.com>
Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
This seems to break builds under certain circumstances. Better to handle the problem with cached rebuilds another way, either in a FindLibdeflate.cmake, or by always locally-building libdeflate and TIFF.

Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
…directory, under module root

Added conditional logic to set relative RPATHs when building with scikit-build. This change ensures that the Python module and compiled cli tools correctly find all built dynamic libraries relative to a shared root, and keeps distributions self-contained and relocatable (i.e., without requiring a `repairwheel` step).

Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
* The build-directory is no longer hard-coded to a local "build_wheels" path.
The 'repairwheel' tool needs to know where it can find any dynamic libs compiled by the build system; and, frustratingly, doesn't seem to consider libraries already bundled in the whl. We can either point repairwheel to directory within an unzipped whl; or, we can point repairwheel back to where the compiled dependencies live inside the build-directory, under .../dist/deps/lib. There isn't a straightforward way of passing information from skbuild to repairwheel directly; but here, we're using cibw to set an environment variable dictating to where scikit-build-core builds, which we can also reference in the repair-wheel step.

* Always (re)build the TIFF dependency when building local wheels.
This is a workaround for an issue where "Deflate::Deflate" either can't be found, or can't be redeclared, under certain circumstances. A more robust solution might be to instead write a FindLibdeflate.cmake module that adds an alias for Deflate::Deflate as needed.

* Add "wheelhouse" directory created by cibuildwheel to .gitignore.

Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
We don't want to locally build TIFF when it already exists; but if we build static TIFF libs locally once, we have to rebuild every time (i.e., for subsequent builds), or else "Deflate::Deflate" is forgotten. This commit forces TIFF to be rebuilt every time, but only for cibuildwheel unix builds. Under normal circumstances, only missing dependencies will be locally built.

Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
Update pyproject.toml configuration

The project's TOML file has been updated to reflect changes in the build system, dependencies, and licensing. The scikit-build-core version requirement has been bumped up, and new tools have been added for wheel repair and invocation. The license text has also been simplified to only include Apache-2.0.

Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
The wheel repair command in the pyproject.toml file has been refactored to use an invoke task. he before-build step now also installs invoke. A new tasks.py file has been added with a 'wheel_repair' task that slims down and repairs the wheel file.

Step 1: Remove `lib`, `include`, `share` directories from wheel
Step 2: Let `repairwheel` fix the wheel with freshly-built libraries found in {build_dir}/lib and {build_dir}/deps/dist/lib.

Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
Also, tiny bit of tidying.

Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
It's an OCIO dependency.

Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
Apparently MSVC is having trouble linking Python3::Python otherwise...

Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
- Lowered the required Python version from 3.9 to 3.7
- Added numpy as a new dependency
- Refined DLL loading for Windows in Python 3.8+
- Adjusted build verbosity settings
- Moved CMAKE_INSTALL_LIBDIR setting into platform-specific overrides for Linux and macOS only
- On Windows, do not make adjustments to the INSTALL_RPATH.
- Reorganized variables in cibuildwheel configuration for better readability
- Refactored variable names in __init__._call_program function to follow PEP8 guidelines

Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
Stolen nearly line-for-line from OpenColorIO's wheel workflow.

Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
…cademySoftwareFoundation#4365)

Fixes a simple copy/paste error in a copy constructor where the y
coordinate gets initialised twice instead of y and z.

Signed-off-by: Anton Dukhovnikov <antond@wetafx.co.nz>
Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
…emySoftwareFoundation#4348)

Additional stack manipulation commands:
* `--popbottom` discards the bottom-of-stack image
* `--stackreverse` reverses the order of the whole stack
* `--stackclear` fully empties the stack
* `--stackextract <index>` moves the indexed item from the stack (index
0 means the top) to the top.

Make `--for` work correctly in both directions:
* Correct behavior if `--for` has a negative step value.
* If the end value is less than the begin value and no step is supplied,
assume -1 (analogous to how we usually assueme step=1 under ordinary
circumstances).
* Error if step is 0 (presume it will make an infinite loop).

Signed-off-by: Larry Gritz <lg@larrygritz.com>
Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
…TORY (AcademySoftwareFoundation#4368)

It's not just in oiiotool. This seems clearer and adheres to the env
variable naming convention we chose.

Reminder: This controls whether command line history gets written to
output image metadata by default by oiiotool and maketx. We historically
did it, but recently stopped because of security concerns.

Signed-off-by: Larry Gritz <lg@larrygritz.com>
Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
JPEG output configuration hint "jpeg:iptc" (default: 1), if set to 0,
will suppress IPTC block output to the file.

In the process, we changed the return type of utility function
encode_iptc_iim() to return true if anything was successfully encoded,
false otherwise.

Signed-off-by: Larry Gritz <lg@larrygritz.com>
Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
…ion#4347)

Signed-off-by: Larry Gritz <lg@larrygritz.com>
Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
zachlewis and others added 5 commits September 30, 2024 11:32
Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
…oundation#4459)

This fixes OIIO's CI "bleeding edge" test that broke when OCIO pushed
to their main the new config files that will be in OCIO 2.4. (Yay!)

Signed-off-by: Larry Gritz <lg@larrygritz.com>
Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
On the macos runners, the static freetype library is linked against a x86_64 PNG lib.

Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
@zachlewis zachlewis force-pushed the python_cibuildwheels branch 2 times, most recently from a09e835 to 10fbf03 Compare September 30, 2024 16:27
Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
@zachlewis
Copy link
Contributor Author

Okay, I think I was able to prevent OIIO from linking x265. It seems the system TIFF was built against WebP... so now we're always building all missing dependencies (on MacOS), and LINKSTATIC is enabled, in order to force the build system to pick up our locally-built static dependencies, where possible. We're also forcibly uninstalling the runner image's Freetype, (installed with homebrew) in order to avoid linking some of its dependencies.

The x86_64 .dylibs directory looks like this:

.
├── libbrotlicommon.1.1.0.dylib
├── libbrotlidec.1.1.0.dylib
├── libbrotlienc.1.1.0.dylib
├── libhwy.1.2.0.dylib
├── libIex-3_2.31.3.2.4.dylib
├── libIlmThread-3_2.31.3.2.4.dylib
├── libImath-3_1.29.11.0.dylib
├── libjpeg.8.3.2.dylib
├── libjxl.0.11.0.dylib
├── libjxl_cms.0.11.0.dylib
├── libjxl_threads.0.11.0.dylib
├── liblcms2.2.dylib
├── libOpenColorIO_v_2_3_2_OIIO.2.3.2.dylib
├── libOpenEXR-3_2.31.3.2.4.dylib
├── libOpenEXRCore-3_2.31.3.2.4.dylib
├── libOpenImageIO.2.6.7.dylib
├── libOpenImageIO_Util.2.6.7.dylib
├── libpng16.16.dylib
├── libsharpyuv.0.1.0.dylib
├── libwebp.7.1.9.dylib
└── libwebpdemux.2.0.15.dylib

This is still larger than the ARM64 wheels, and some of these dependencies seem to be coming from the system level (libwebp, ...), and others from homebrew (libpng16, jpeg i think). Interestingly, OCIO is linking the shared lcms2 library, and probably the shared IMath library as well, both of which are provided by the runner image.

As long as there are no GPL-licensed dependencies, I'm cool with leaving this as-is.

I believe at this point, the only failing check is the CI / bleeding edge run.

It's an old flavor not well supported by homebrew any longer, so
what's happening is that most of the dependency packages we ask to
install have to build from source, which means it takes a long long
time. Just get rid of it, we have two other Mac test cases -- a newer
MacOS 13 Intel one, and a MacOS 14 ARM one. That's enough coverage.

Signed-off-by: Larry Gritz <lg@larrygritz.com>
Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
…or it

Signed-off-by: Larry Gritz <lg@larrygritz.com>
Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
It's an old flavor not well supported by homebrew any longer, so what's
happening is that some of the dependency packages we ask to install have
to build from source, which means it takes a long long time.

By disabling OpenCV and Qt homebrew installation for it, we can make it
fast again. Neither the minimal OpenCV interop, nor osltoy (the only
thing that needs Qt) are exercised by the automated testsuite anyway, so
I don't think there's any loss of testing functionality in practice.

Signed-off-by: Larry Gritz <lg@larrygritz.com>
OpenEXR 3.3 was just released and its main branch has bumped its
advertised version to 4.0 (the latter breaking our "bleeding edge" CI
test).

Raise our "latest releases" test to use 3.3.

Our "bleeding edge" test picking up the now-called-4.0 OpenEXR main
branch required some minor logic changes to not reject the version as
too new to be compatible.

Signed-off-by: Larry Gritz <lg@larrygritz.com>
Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
zachlewis and others added 8 commits October 2, 2024 13:42
Instead of defining the brew uninstall command at the pyproject.toml config level, define using an env var during the workflow run. This makes things more predictable when testing cibuildwheel locally

Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
OCIO recently made the decision to disable the "Load DLLs from $PATH" hack for Python > 3.7 that they had copied from OIIO.
We should follow suit -- this behavior was removed from Python for a reason (it was a security vulnerability).

This PR warns users that this behavior is deprecated.

Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Signed-off-by: zachlewis <zachlewis@users.noreply.github.com>
Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scorecard found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

…n_cibuildwheels

Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
…wheels

Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
 - Remove "numpy" dependency -- technically, numpy is only needed if using `ImageBuf.get_data()` (I believe). Previously, we had set this to require numpy-2; but that caused the Python 3.8 wheels not to build.

 - Remove MacOS architecture CMake defines -- since we're using both x86_64 and ARM64 runners to build the ARM and intel wheels separately, we shouldn't have to worry about the ARM64 python modules trying to link x86_64 dependencies.

- Test with  "oiiotool --buildinfo" instead of "oiiotool --help"

Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
Turns out, /opt/homebrew/lib/libfreetype.a links a libPNG that wasn't built for arm64.

Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
Now that numpy-2 isn't a requirement, we can re-enable Python 3.8 wheel builds.

Also, I've added top-level permissions to the Workflow, and pinned the Action dependencies by hash to pass / improve the advanced security checks / scorecard

Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.