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

build: Add support for pip install OpenImageIO #4011

Closed
wants to merge 7 commits into from
Closed

build: Add support for pip install OpenImageIO #4011

wants to merge 7 commits into from

Conversation

aclark4life
Copy link

@aclark4life aclark4life commented Oct 11, 2023

Description

Adds support forpip install OpenImageIO requested in #3249

Tests

Checklist:

  • I have read the contribution guidelines.
  • I have updated the documentation, if applicable.
  • I have ensured that the change is tested somewhere in the testsuite
    (adding new test cases if necessary).
  • If I added or modified a C++ API call, I have also amended the
    corresponding Python bindings (and if altering ImageBufAlgo functions, also
    exposed the new functionality as oiiotool options).
  • My code follows the prevailing code style of this project. If I haven't
    already run clang-format before submitting, I definitely will look at the CI
    test that runs clang-format and fix anything that it highlights as being
    nonconforming.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Oct 11, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

Copy link
Contributor

@jessey-git jessey-git left a comment

Choose a reason for hiding this comment

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

To fix the failing "DCO" check, the last time this happened to me I had to amend my commit and force-push my branch again (the PR will auto-update):

git commit --amend -s
git push --force origin <your branch>  # or similar...

The -s (which you could have used on any commit) appends the "Signed-off-by" line to your commit message.

I feel this could maybe have a better description too, since this typically ends up in git history in some form.

What is setup.py? That file doesn't exist in the repo at the moment. Who is Jean-Christophe? They don't seem to be any former contributor here; is that information important? I had to do a bit of internet searching to realize who this was :) Some of the results are, of course, interesting.

pyproject.toml Outdated
@@ -0,0 +1,52 @@
[project]
name = "OpenImageIO"
version = "2.4.0.dev1"
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this version match what OIIO itself declares? Saying 2.4 is weird since that's an old version now etc.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, I updated with 2.6.0.2 because that looked reasonably close. It doesn't really matter yet because even after this is merged, and we can see GitHub actions building wheels, we'll need to integrate GitHub Actions with Trusted Publishers. Or in the short term, if you wanted to publish to PyPI before that happens, we could make sure the version number is correct in the release branch then manually upload wheels with twine.

pyproject.toml Outdated
dependencies = [
"setuptools>=68.2.2",
]
requires-python = ">2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*, !=3.4.*, !=3.5.*, !=3.6.*, !=3.9.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a particular reason to exclude 3.9.0 exactly? Also, would attempting to use python 2.7.18 pass this check? If so, I have a meta question/comment around if we want to continue to support and encourage such nonsense :)

Copy link
Member

Choose a reason for hiding this comment

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

3.9.0 is known to be buggy in when used with Pybind11.

Choose a reason for hiding this comment

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

And no, I don't think we should support 2.7. We'll need at least Pybind11 2.10, which doesn't support python 2.7.

Copy link
Collaborator

Choose a reason for hiding this comment

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

VFX Platform 2022 uses Python 3.9, so it's definitely in the mix of things we support, and I don't recall any pybind11 problems (for OIIO anyway).

Copy link
Author

@aclark4life aclark4life Oct 11, 2023

Choose a reason for hiding this comment

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

Removed all Python version restrictions. Definitely not going to support 2.7 or anything older than 3.9 at this point, but I'm not sure if we need to be explicit about it here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This may help: https://docs.google.com/spreadsheets/d/1EwRlz5ZYObEOdBfIk8iTX5thlpTyEAfp3bxOgAfFOiU/edit#gid=1592527777

That shows (near the bottom) what VFX Platform years the different studios are using. Prior to 2020 is definitely not needed, and I think even 2020 (maybe 2021) is on the way to being phased out. This also matches with VFX Platform's own recommendation of trying to support up to 3 years back from the present.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'll put it in even more stark terms: my own studio will definitely be transitioned away from Python 2.7 by the end of this year, I hope. (And onto an assortment of 3.7, 3.9, 3.10, depending on the specific one of the several DCCs we are depending on.)

Copy link
Collaborator

Choose a reason for hiding this comment

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

In terms of priority, it's likely that the users who need a simple "pip install" approach most desperately (i.e., want to avoid building anything from source) are very likely on a very recent python. So that's probably top priority. But I'm playing devil's advocate here to make sure we don't back ourselves into a corner, design wise, that will make it difficult to eventually support python versions as far back as the VFX years we claim to support.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I do agree that we should definitely not support python older than 3.7. Whether 3.7 or 3.9 is the minimum we want to support for "pip install" is reasonable for debate, and I don't have a firm opinion on it, other than that we should choose among them for a principled reason.

Copy link
Member

Choose a reason for hiding this comment

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

The code base already supports 3.7 and up thanks to Pybind11, so building for 3.7 isn't more difficult than building for 3.11. 3.12 is a different story though because it's still pretty recent (it's only one week old). In other words, once you get a build running and working wheels for one version, you get the other versions for free without any additional work (in most cases, since Pybind11 abstracts everything for us).

@JeanChristopheMorinPerso
Copy link
Member

Hi @jessey-git 👋, I'm the Jean-Christophe mentioned in the description. I volunteered to help with #3249, mainly to review PRs, but I'll be happy to help in other things ways too if there is a need. Obviously, that is if you want me to review PRs.

@jessey-git
Copy link
Contributor

@JeanChristopheMorinPerso Hi! I did eventually find you within the ASWF umbrella after searching and you're of course more than welcome to help out in whatever capacity you'd like :) And ah! The description was missing a link back to the issue which would have made things a lot more clear where this was coming from.

@aclark4life
Copy link
Author

@jessey-git Sorry, this is a WIP I created to get the CLA signed before tomorrow!

@aclark4life
Copy link
Author

aclark4life commented Oct 11, 2023

@JeanChristopheMorinPerso Can you please identify which of the commits listed here are most important to cherry-pick? For example, we don't need to build with setup.py so not going to cherry-pick that one. Thanks

@aclark4life aclark4life changed the title build: Add pyproject.toml build: Add support for pip install OpenImageIO Oct 11, 2023
@aclark4life
Copy link
Author

Happy sprint day 1 folks!

Please correct me if any of these assumptions are wrong:

  • VFX industry uses OpenImageIO as part of the VFX Platform and ASWF supports development of the VFX Platform
  • There's currently no formal binary distribution of OpenImageIO from ASWF, thus leaving the community with ad hoc attempts to provide them e.g.:
  • DCCs may include binary distributions of OpenImageIO thus lowering the burden of installation for VFX industry
    • Binary distributions may include the oiiotool command line tool as well as "much of its functionality" available via Python bindings
    • Python wheels would further reduce OpenImageIO installation burden by making its Python API available via pip install OpenImageIO

Most importantly, if I understand correctly, the oiiotool and related runtime libraries are not needed for Python API usage when OpenImageIO is installed via pip install OpenImageIO because all of the dependencies will be included in the wheel.

@lgritz
Copy link
Collaborator

lgritz commented Oct 12, 2023

VFX industry uses OpenImageIO as part of the VFX Platform and ASWF supports development of the VFX Platform

OIIO is not in the VFX Platform. ASWF is not the originator of the VFX Platform. But the VFX Platform largely specifies an important subset of the permutations of versions that we (OIIO and the other ASWF projects) definitely need to be able to build against.

There's currently no formal binary distribution of OpenImageIO from ASWF, thus leaving the community with ad hoc attempts to provide them e.g.:

Correct.

DCCs may include binary distributions of OpenImageIO thus lowering the burden of installation for VFX industry

Many DCCs and other applications use OIIO internally. The OIIO components are thus generally compiled in statically, or put in libraries with custom symbol namespaces so as to not interfere with any other OIIO used in the environment. I'm not aware of any DCC that ships OIIO in a form that's exposed to and useful to the users of the DCC.

Python wheels would further reduce OpenImageIO installation burden

Yes.

Most importantly, if I understand correctly, the oiiotool and related runtime libraries are not needed for Python API usage when OpenImageIO is installed via pip install OpenImageIO because all of the dependencies will be included in the wheel.

I would disagree with this. I think there is one set of users who are only after the Python APIs and thus view OIIO like any other Python library that they want to import. But a second set of users merely want "pip install openimageio" as the simplest and least painful way to get a full binary install of the entire package and its dependencies. Those people want python, C++, oiiotool, the whole schmear. Also, I think that somebody who starts out only wanting/needing the python interface could easily find themselves in a position of later needing oiiotool, and I wouldn't want them to suddenly be stuck without any way to get it without slogging through a full set of builds from source.

@lgritz
Copy link
Collaborator

lgritz commented Oct 12, 2023

A crucial thing to understand about VFX Platform is that it is not a list of recommendations, nor is it a list of all the things needed in a VFX studio. It's a list of packages that have historically been a such a compatibility versionitis nightmare (often because of deficiencies in their design or packaging) that we needed to get the major DCC vendors to agree to all use the same version in the same year, and this is the list of which versions they negotiated. Nothing more and nothing less.

@aclark4life
Copy link
Author

Copy that, thanks!

@JeanChristopheMorinPerso
Copy link
Member

I would disagree with this. I think there is one set of users who are only after the Python APIs and thus view OIIO like any other Python library that they want to import. But a second set of users merely want "pip install openimageio" as the simplest and least painful way to get a full binary install of the entire package and its dependencies. Those people want python, C++, oiiotool, the whole schmear. Also, I think that somebody who starts out only wanting/needing the python interface could easily find themselves in a position of later needing oiiotool, and I wouldn't want them to suddenly be stuck without any way to get it without slogging through a full set of builds from source.

Indeed @lgritz. The work I did last year included adding the tools into the wheels for this exact reason (because people need th tools or the API or both).

@aclark4life
Copy link
Author

OK so in Python lingo, we have console_scripts = oiiotool so that folks have access to the same command line they'd have available if they compiled the C++?

@JeanChristopheMorinPerso
Copy link
Member

Yep, and it's already there, see https://github.com/AcademySoftwareFoundation/OpenImageIO/pull/4011/files#diff-50c86b7ed8ac2cf95bd48334961bf0530cdc77b5a56f852c5c61b89d735fd711R37-R44

@lgritz
Copy link
Collaborator

lgritz commented Oct 12, 2023

We should probably talk at some point about which are important enough to include in the python wheel. Definitely oiiotool and maketx. Probably not testshade, which I think is only used in the testsuite for unit tests but isn't used by users (am I right about that?). Technically, iinfo, iconvert, and idiff don't do anything that oiiotool can't do (they exist mostly because they predate oiiotool), though some people find them useful because they each do one simple thing well without having to look up the oiiotool commands needed to do the same thing. I honestly don't know if anybody uses igrep (it seemed cool on the day I wrote it, but I also can't recall the last time I needed it). And iv... maybe? (Though also maybe it should be renamed, it's so short and generic it does scare me a little with it being confused with or hiding other things.)

@@ -73,4 +73,4 @@ elif [[ "${RUNNER_OS}" == "macOS" ]] ; then
fi

# Save the env for use by other stages
src/build-scripts/save-env.bash
# src/build-scripts/save-env.bash
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this line is going to turn out to be important.

Copy link
Author

Choose a reason for hiding this comment

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

Fixed

@aclark4life
Copy link
Author

OK rebased and fixed issues raised by @lgritz and it looks like @JeanChristopheMorinPerso signed DCO 🚀 so today I will continue with cherry-picking remainder of applicable @JeanChristopheMorinPerso commits.

@lgritz
Copy link
Collaborator

lgritz commented Oct 13, 2023

@aclark4life Can you please mark this as a draft PR, and then remove the draft status when you think it's ready to go? I'm afraid that my knowledge about what you're trying to do is so thin that I'm not sure it will be obvious to me when this is ready to review or complete.

Alternately, if you know how to break it into tasks or milestones, you could amend the description to have that checklist and tick them off as they get done, then it'll be easy to see which pieces are done and which are still in progress.

@aclark4life aclark4life marked this pull request as draft October 13, 2023 17:41
@aclark4life
Copy link
Author

@lgritz Almost done with something that can get merged!

In #3249 you'll see a list of commits from @JeanChristopheMorinPerso that I've cherry-picked and fixed conflicts. A few more to go, then I'll add the trusted publisher configuration and we'll call it a sprint.

Copy link
Member

@JeanChristopheMorinPerso JeanChristopheMorinPerso left a comment

Choose a reason for hiding this comment

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

Great start @aclark4life!

For others following, I think the next steps would be to finish that and get a working build, and then make sure that we can build using python -m build -w (ie make sure that scikit-build-core can be used). Then I guess we'll need a GitHub Action workflow that does the whole end to end build with cibuildwheel. After that, we'll know if the theory works.

pyproject.toml Outdated Show resolved Hide resolved
src/build-scripts/build_pugixml.bash Outdated Show resolved Hide resolved
src/build-scripts/docker-centos.bash Outdated Show resolved Hide resolved
src/build-scripts/docker-centos.bash Outdated Show resolved Hide resolved
src/cmake/compiler.cmake Outdated Show resolved Hide resolved
src/cmake/externalpackages.cmake Outdated Show resolved Hide resolved
src/cmake/externalpackages.cmake Outdated Show resolved Hide resolved
src/libOpenImageIO/CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
INSTALL.md Outdated Show resolved Hide resolved
Comment on lines 50 to 64
if [[ ${OPENEXR_VERSION} == "v2.3.0" ]] ; then
# Simplified setup for 2.3+
cd ${OPENEXR_BUILD_DIR}
cmake -DCMAKE_BUILD_TYPE=${OPENEXR_BUILD_TYPE} \
-DBUILD_SHARED_LIBS=${OPENEXR_BUILD_SHARED_LIBS} \
-DCMAKE_INSTALL_PREFIX="${OPENEXR_INSTALL_DIR}" \
-DCMAKE_PREFIX_PATH="${CMAKE_PREFIX_PATH}" \
-DILMBASE_PACKAGE_PREFIX="${OPENEXR_INSTALL_DIR}" \
-DOPENEXR_BUILD_UTILS=0 \
-DOPENEXR_BUILD_TESTS=0 \
-DOPENEXR_BUILD_PYTHON_LIBS=0 \
-DCMAKE_CXX_FLAGS="${OPENEXR_CXX_FLAGS}" \
${OPENEXR_CMAKE_FLAGS} ${OPENEXR_SOURCE_DIR}
time cmake --build . --target install --config ${OPENEXR_BUILD_TYPE}
else
Copy link
Collaborator

Choose a reason for hiding this comment

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

I feel like maybe you forward-ported something from a while back. We recently simplified this code to no longer need the "v2.3.0" clause because OpenEXR 2.4 is now the oldest version we support.

@lgritz
Copy link
Collaborator

lgritz commented Mar 20, 2024

Wondering what the progress on this is, or what roadblocks exist that we can help with. It might be nice to have this solved once and for all by the time of the 3.0 release this fall.

@aclark4life
Copy link
Author

@lgritz Sorry, got sidetracked but still interested in completing, and fall is super reasonable. I'm pitching Pillow project to TAC today during which time I'll mention my status:

@lgritz
Copy link
Collaborator

lgritz commented Mar 20, 2024

Yes, I agree that the only sensible path is to use the same mechanisms as other ASWF projects that are publishing python wheels, unless there are serious deficiencies in their methodologies (in which case we should fix that, then copy what they've done).

@aclark4life
Copy link
Author

Just compare it directly to the current master. I understand that you added a few of these build scripts for dependencies where we lacked them entirely. But for the ones that we already had, nothing should change except the absolute minimum additions you need to get the python wheel build, right?

I think I fixed these

@aclark4life
Copy link
Author

Not sure how much actual progress I've made but I just rebased and fixed a bunch of issues raised in comments (or at least tried to!)

@lgritz
Copy link
Collaborator

lgritz commented Jun 2, 2024

What's the status of this PR?

Is this waiting for me specifically?

It's still marked as a draft, but also it seems out of date with many merge conflicts if it were to be merged now.

And most importantly, I should not be the reviewer here. I don't know anything about python wheels. Ideally, this should be reviewed by (a) somebody who has made python wheels before and will be consuming them; and also (b) somebody who set up the python wheels for another ASWF project and therefore knows if this is conforming to a common methodology.

@aclark4life
Copy link
Author

What's the status of this PR?

Just lazy-rebased by resolving conflicts with master using master branch files. That means we've lost changes made to those files for this PR because they are now identical to the master branch files. We could just close this for now, however there are still some changes that could be reviewed by anyone interested helping with this issue. Not to mention this is an awesome learning process for me! So I'm going to leave it open for the time being if that's OK with you.

Is this waiting for me specifically?

No, not especially, unless you are interested in helping with this issue.

It's still marked as a draft, but also it seems out of date with many merge conflicts if it were to be merged now.

Right this is a fork of JC's branch in which I've tried to finish what JC started but have not managed to do so yet. Since then, the issue of "how do we do this consistently across all projects" was raised.

And most importantly, I should not be the reviewer here. I don't know anything about python wheels.

I would not feel obligated to review, but also note wheels are just zip files and they are installed by unzipping the contents to Python's site-packages directory. When Python is run, >>> import OpenImageIO "just works" via Python's import mechanism. In the case of libraries compiled against C++, apparently that mechanism includes loading so files e.g.

Screenshot 2024-06-02 at 9 57 54 AM

Ideally, this should be reviewed by (a) somebody who has made python wheels before and will be consuming them; and also (b) somebody who set up the python wheels for another ASWF project and therefore knows if this is conforming to a common methodology.

The general consensus from various discussions I've had and observed IIUC is "big studios don't need this" and "many small studios do". So, we probably need more input from "many small studios" on #3249 particularly when it comes time to testing. You mentioned timing this with the fall release and I'm still tentatively planning to do that. However I'm mostly preoccupied with job search now, hoping the need to search will subside by beginning of summer.🤞

@JGoldstone
Copy link
Contributor

I'm not qualified to review this PR either but … just wanted to note that OpenVPCal installation instructions say that they'd be able to make that project available on PyPi as soon as OIIO is on PyPi.

https://github.com/Netflix/OpenVPCal/tree/main?tab=readme-ov-file#why-not-pypi

@JeanChristopheMorinPerso
Copy link
Member

Right this is a fork of JC's branch in which I've tried to finish what JC started but have not managed to do so yet. Since then, the issue of "how do we do this consistently across all projects" was raised.

I'm not sure to get what you mean by "how do we do this consistently across all projects". Build backends (setuptools, scikit-build-core, etc) are an implementation detail that users and the tooling will never see and building wheels for bindings nowadays is a well defined process and standardized process. All ASWF projects that have bindings do approximately the same thing which is to use cibuildwheel. So I don't know what consistency means here since our projects are already consistent in how they build their wheels (minus build backends, but again that doesn't matter).

I would not feel obligated to review, but also note wheels are just zip files and they are installed by unzipping the contents to Python's site-packages directory. When Python is run, >>> import OpenImageIO "just works" via Python's import mechanism. In the case of libraries compiled against C++, apparently that mechanism includes loading so files e.g.

Reviewing wheels is more involved than just looking at if importing the library works. A reviewer has to check what files are included, licenses bundled, the linking of libraries (static vs shared), the size of the wheels, etc. The method of how the wheels are uploaded, the validity and accuracy of the metadata also has to be reviewed. And then there are per project things to check that will differ from project to project.

As I said in the past, I'll be happy to review once this PR is complete and ready to review. Right now what's missing is the glue. There is a disconnect between the actual build and where dependencies are fetched from. There is also no CI to test the wheels and test that the builds actually work.

The general consensus from various discussions I've had and observed IIUC is "big studios don't need this" and "many small studios do". So, we probably need more input from "many small studios" on #3249 particularly when it comes time to testing.

The question is not really big studios vs small studios. The main argument of creating wheels and more importantly making wheels available on PyPI is to make projects easily accessible for all users, including other projects (like @JGoldstone highlighted above).

@lgritz
Copy link
Collaborator

lgritz commented Jun 2, 2024

This needs DCOs on all commits

@lgritz
Copy link
Collaborator

lgritz commented Jun 2, 2024

"how do we do this consistently across all projects".

The goal is not a blind consistency across all ASWF projects for its own sake, but rather, we observe that some projects have already worked this task out and have been making wheels long enough to have shaken out most of the rookie mistakes. Therefore, using their methodology as a starting point seems like a way to greatly reduce the risk of unforced errors on our part, and also increases the chances that process improvements in one project can be ported to the others, and that anyone who understands the wheel creation in one project is already mostly up to speed on the others.

There is also no CI to test the wheels and test that the builds actually work.

It seems vital to have a comprehensive and fully automated test of the wheel as a gating factor for publishing it to PyPI.

aclark4life and others added 7 commits June 6, 2024 13:29
- Add build scripts for wheel dependencies.
- Fix OpenColorIO.
- Working wheel build.
- Remove unused scripts.
- Cleanup CentOS build.
- Require Python Development.Module instead of Development since the libpython is not required to build the extension.
- Remove unnecessary changes.
- Working windows build!
- Build from pyproject.toml with scikit.build

Signed-off-by: Alex Clark <[email protected]>
@aclark4life
Copy link
Author

@aclark4life aclark4life closed this by deleting the head repository Jul 21, 2024
lgritz added a commit that referenced this pull request Jan 21, 2025
### 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.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.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, 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](https://github.com/pypa/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.

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.yml` from 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.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: 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.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

`cibuildwheel` tests if `oiiotool --buildinfo` runs. 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](#3249),
and
[#4011](#4011),
as well as JCM's
[python_wheels](https://github.com/JeanChristopheMorinPerso/oiio/tree/python_wheels)
and
[python_wheels_windows](https://github.com/JeanChristopheMorinPerso/oiio/tree/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.

---------

Signed-off-by: Zach Lewis <[email protected]>
Signed-off-by: Larry Gritz <[email protected]>
Signed-off-by: Anton Dukhovnikov <[email protected]>
Signed-off-by: Basile Fraboni <[email protected]>
Signed-off-by: Vlad (Kuzmin) Erium <[email protected]>
Signed-off-by: Joseph Goldstone <[email protected]>
Signed-off-by: Darby Johnston <[email protected]>
Signed-off-by: Jeremy Retailleau <[email protected]>
Signed-off-by: zachlewis <[email protected]>
Co-authored-by: Larry Gritz <[email protected]>
Co-authored-by: Anton Dukhovnikov <[email protected]>
Co-authored-by: Basile Fraboni <[email protected]>
Co-authored-by: Vlad (Kuzmin) Erium <[email protected]>
Co-authored-by: Joseph Goldstone <[email protected]>
Co-authored-by: Darby Johnston <[email protected]>
Co-authored-by: Jeremy Retailleau <[email protected]>
Co-authored-by: Jean-Christophe Morin <38703886+JeanChristopheMorinPerso@users.noreply.github.com>
lgritz added a commit to lgritz/OpenImageIO that referenced this pull request Jan 21, 2025
…eFoundation#4428)

### Summary
This PR is the spiritual successor to AcademySoftwareFoundation#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.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.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, 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](https://github.com/pypa/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.

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.yml` from 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.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: 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.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

`cibuildwheel` tests if `oiiotool --buildinfo` runs. 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
[AcademySoftwareFoundation#3249](AcademySoftwareFoundation#3249),
and
[AcademySoftwareFoundation#4011](AcademySoftwareFoundation#4011),
as well as JCM's
[python_wheels](https://github.com/JeanChristopheMorinPerso/oiio/tree/python_wheels)
and
[python_wheels_windows](https://github.com/JeanChristopheMorinPerso/oiio/tree/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.

---------

Signed-off-by: Zach Lewis <[email protected]>
Signed-off-by: Larry Gritz <[email protected]>
Signed-off-by: Anton Dukhovnikov <[email protected]>
Signed-off-by: Basile Fraboni <[email protected]>
Signed-off-by: Vlad (Kuzmin) Erium <[email protected]>
Signed-off-by: Joseph Goldstone <[email protected]>
Signed-off-by: Darby Johnston <[email protected]>
Signed-off-by: Jeremy Retailleau <[email protected]>
Signed-off-by: zachlewis <[email protected]>
Co-authored-by: Larry Gritz <[email protected]>
Co-authored-by: Anton Dukhovnikov <[email protected]>
Co-authored-by: Basile Fraboni <[email protected]>
Co-authored-by: Vlad (Kuzmin) Erium <[email protected]>
Co-authored-by: Joseph Goldstone <[email protected]>
Co-authored-by: Darby Johnston <[email protected]>
Co-authored-by: Jeremy Retailleau <[email protected]>
Co-authored-by: Jean-Christophe Morin <38703886+JeanChristopheMorinPerso@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
python Python APIs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants