feat: Python wheels workflow and build backend #4428
Conversation
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 <[email protected]>
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 <[email protected]>
Use Apache-2.0 license identifier instead of BSD-3-Clause. Add "OpenImageIO Contributors" / "[email protected]" as author. I could not bring myself to remove Larry as an author and maintainer. Signed-off-by: Zach Lewis <[email protected]>
…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 <[email protected]>
Build and link missing libjpeg-turbo shared + static libs Signed-off-by: Zach Lewis <[email protected]>
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 <[email protected]>
…oftwareFoundation#4358) As suggested by Moritz Moeller Signed-off-by: Larry Gritz <[email protected]> Signed-off-by: Zach Lewis <[email protected]>
…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 <[email protected]> Signed-off-by: Zach Lewis <[email protected]>
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 <[email protected]>
…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 <[email protected]>
* 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 <[email protected]>
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 <[email protected]>
Signed-off-by: Zach Lewis <[email protected]>
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 <[email protected]>
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 <[email protected]>
Signed-off-by: Zach Lewis <[email protected]>
Signed-off-by: Zach Lewis <[email protected]>
Also, tiny bit of tidying. Signed-off-by: Zach Lewis <[email protected]>
Signed-off-by: Zach Lewis <[email protected]>
It's an OCIO dependency. Signed-off-by: Zach Lewis <[email protected]>
Apparently MSVC is having trouble linking Python3::Python otherwise... Signed-off-by: Zach Lewis <[email protected]>
Signed-off-by: Zach Lewis <[email protected]>
- 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 <[email protected]>
Signed-off-by: Zach Lewis <[email protected]>
Stolen nearly line-for-line from OpenColorIO's wheel workflow. Signed-off-by: Zach Lewis <[email protected]>
…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 <[email protected]> Signed-off-by: Zach Lewis <[email protected]>
…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 <[email protected]> Signed-off-by: Zach Lewis <[email protected]>
…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 <[email protected]> Signed-off-by: Zach Lewis <[email protected]>
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 <[email protected]> Signed-off-by: Zach Lewis <[email protected]>
…ion#4347) Signed-off-by: Larry Gritz <[email protected]> Signed-off-by: Zach Lewis <[email protected]>
tasks.py
Outdated
There was a problem hiding this comment.
ha -- we'll see... what I've been trying has apparently broken everything. I'm not sure if I have enough time to debug this myself. We may have to split off the attempt to get rid of tasks.py into its own PR. Right now, it's solving a few problems at once.
I've dedicated an entire "remove_prefixes_from_variable" cmake function to omitting prefixes from environment and cmake variables to help me debug and fine-tune dependency building. I believe this would be useful for tricky build recipes; but I think it would be better to pare this down for the immediate future, pending CI checks... Signed-off-by: Zach Lewis <[email protected]>
This reverts commit 34d7e3a. Signed-off-by: Zach Lewis <[email protected]>
Signed-off-by: Zach Lewis <[email protected]>
|
Great work with the latest changes @zachlewis! I like how the extract step to fix the wheels are not needed anymore. I started to review the generated artifacts. I see that the versioned libraries are not yet fully removed: I also see Are these really needed? The OIIO libs have an absolute RPATH: Right now I think the dynamic linker will do its job only because
(emphasis mine) Though, the tests in CI seem to say that there is a problem with RPATHs since it can't find OCIO. Lastly, the wheel filenames are wrong on Linux. |
|
About the rpath, if I had to take a guess, I would guess that the culprit is OpenImageIO/src/cmake/compiler.cmake Lines 657 to 658 in c3c6b1e |
Optionally disable creation of multiple versioned copies / symlinks of installed libraries. In other words, create libOpenImageIO.so.3.1.0; but not libOpenImageIO.so or libOpenImageIO.so.3.1. Signed-off-by: Zach Lewis <[email protected]>
Signed-off-by: Zach Lewis <[email protected]>
Signed-off-by: Zach Lewis <[email protected]>
|
Hey @JeanChristopheMorinPerso -- once again, I can't tell you how much I appreciate you having a look! I apologize, I left things in an inconsistent state over the weekend. Good news is, I do believe I've fixed remaining issues...! Major changes:
You were totally right about this -- now I'm doing this: if (NOT CMAKE_INSTALL_RPATH)
if(APPLE)
set(BASEPOINT @loader_path)
else()
set(BASEPOINT $ORIGIN)
endif()
set (CMAKE_INSTALL_RPATH ${BASEPOINT} ${BASEPOINT}/${CMAKE_INSTALL_LIBDIR})
endif ()And now the OIIO libs have a relative RPATH: 🤘 I've gotten rid of the docs. For the fonts, actually, what I'd like to do adjust __init __.py and append the relative dir to the env var "OPENIMAGEIO_FONTS". I like the idea of a cross-platform default font shipping with the module... |
| @@ -655,7 +655,12 @@ if (CMAKE_SKIP_RPATH) | |||
| unset (CMAKE_INSTALL_RPATH) | |||
| else () | |||
| if (NOT CMAKE_INSTALL_RPATH) | |||
| set (CMAKE_INSTALL_RPATH "${CMAKE_INSTALL_FULL_LIBDIR}") | |||
| if(APPLE) | |||
| set(BASEPOINT @loader_path) | |||
There was a problem hiding this comment.
This is kind of reverting what was made before. You could pass -DCMAKE_INSTALL_RPATH on a per platform basis instead.
There was a problem hiding this comment.
Well, before, I think CMAKE_INSTALL_RPATH was defaulting to an absolute path; whereas now, it uses loader-path-relative defaults. We could still pass the option on a per-platform basis, but I'm not sure how that would be advantageous...?
|
Thanks @zachlewis. I'm always happy to help! |
Assuming the wheel bundles fonts under <platlib>/OpenImageIO/share/fonts, setting the OpenImageIO_ROOT env var should help OIIO automatically find the fonts. As a bonus, pip-installed CLI tools will be aware of OpenImageIO_ROOT. Signed-off-by: Zach Lewis <[email protected]>
|
Hey there, By curiosity I wanted to test early some those wheels so I downloaded one from the last action's artefacts. However, once installed I do notice that the whell doesn't include support for Looking at the previous comments I see that there was some trouble with png so I assume its removal is intentional for now. However, I see those comments seems to be for MacOS and I'm currently testing it on Windows. Did I just misunderstand and png is not yet available for all platform ? Job I used: https://github.com/AcademySoftwareFoundation/OpenImageIO/actions/runs/11464854677/job/31902028113 Feel free to ignore as I fully understand that this PR is still wip and I'm not supposed to use those wheels yet. |
Feedback welcome! @zachlewis or @JeanChristopheMorinPerso can probably provide more detail but yeah I think PNG is a WIP and FWIW here's Pillow's PNG build: https://github.com/python-pillow/Pillow/blob/main/.github/workflows/wheels-dependencies.sh |
"These go to eleven!" Signed-off-by: Zach Lewis <[email protected]>
zachlewis
force-pushed
the
python_cibuildwheels
branch
from
2ee184b
to
8fdc0d5
Compare
|
Heya -- This PR doesn't intend to do anything specific with PNG (or any other dependency) per se -- it assumes that all dependencies are provided by OIIO's own dependency self-building mechanisms. The reason why PNG isn't included yet is because we're still working on getting the PNG self-builds to work (see #4423). Once a working solution is merged, the Python wheels will automatically inherit PNG support. (It's true, not long ago, we were disabling PNG for the MacOS ARM wheels earlier here, but that was just a temporary workaround for handling conflicting homebrew-managed libraries. Now we have a more robust CMake option As for the status of this PR, I don't really intend to change anything, unless there's something specific you guys would like me to change. |
Signed-off-by: zachlewis <[email protected]>
The installation components have been limited to 'user' and 'fonts'. No development headers or config files are included. Also, only `oiiotool` and `maketx` are built and exposed as Python scripts (when building with pip). Signed-off-by: Zach Lewis <[email protected]>
Simplified the DLL loading process for Windows platform by removing version-specific conditions. Also, removed unnecessary metadata such as status, author, email, license, and copyright from the file. All that stuff can be retrieved with importlib.metadata. Signed-off-by: Zach Lewis <[email protected]>
Signed-off-by: Zach Lewis <[email protected]>
Signed-off-by: Zach Lewis <[email protected]>
lgritz
added
build / testing / port / CI
Description
Summary
This PR is the spiritual successor to #4011. It implements a
scikit-build-core-based python build-backend, making it possible to usepip install .to build from source; and it adds a Github workflow for building withcibuildwheeland publishing to pypi.org binary distributions (bdists) of the Python module / extensions / CLI tools for cpython 3.8-3.13, 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.tomlThe
pyproject.tomlfile is organized in three parts:pip install ...interacts withcmake.Additions to
__ init __.pyPreviously, 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+repairwheelcibuildwheel is a widely-used tool for drastically streamlining the process of building, repairing, and testing Python wheels across platforms, architectures, interpreters, and interpreter versions.
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, compared to "Release"
"Wheels" Github workflow
I straight-up copied
.github/workflows/wheel.ymlfrom OpenColorIO and made a few OIIO-specific modifications. When pushing a commit tagged 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) and persisting build artifacts; followed finally by a task for publishing the build artifacts to pypi.orgNote: 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: A "trusted publisher" must be set up on pypi.org. See https://docs.pypi.org/trusted-publishers/creating-a-project-through-oidc/
Other Changes
I made some minor adjustments to
pythonutils.cmakeandfancy_add_executable.cmakethat only affect scikit-build-core-based installs:Tests
cibuildwheeltests ifoiiotool --buildinforuns. If that command elicits code zero, it means the "oiiotool" Python script installed by the wheel is able toimport OpenImageIO; that the actual binary executableoiiotoolis 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