Skip to content

Commit

Permalink
Build python wheels via scikit-build-core (AcademySoftwareFoundation#…
Browse files Browse the repository at this point in the history
…1629)

* Build python wheels via scikit-build-core

This converts the setuptools configuration for building python wheels
to use scikit-build-core, which has better support for CMake. There is
no more setup.py; the configuration is entirely in `pyproject.toml`
and the compile/link is done exclusively via cmake.

The build/publish policy is:

* A PR that changes any of the python wheel source/configuration
  (src/wrappers/python/* or .github/workflows/python-wheels.yml)
  triggers a build as a check.

* PRs that change other library source do *not* trigger a build of the
  python wheels. Note that the primary CI workflow does build and test
  the bindings, although only for a single python version on a single
  arch for Linux/macOS/Windows. The wheel building validates multiple
  python versions and architectures, but involves signifant
  computation/time.  Currently, the python wheels are a thin wrapper
  about basic read/write functions that don't add significant
  additional functionality to the library. Any potential problem will
  almost certainly get caught by the primary CI.

* A tag of the form `v3.[0-9]+.[0-9]+-rc*` (e.g. `v3.2.4-rc`) triggers
  a full build of the wheels followed by a publish to
  `test.pypi.org`. This validates release candidates.

* Publishing a release triggers a full build of the wheels followed by
  a publish to `pypi.org`.

Signed-off-by: Cary Phillips <[email protected]>

* Add custom README.md for pypi.org

Signed-off-by: Cary Phillips <[email protected]>

* fix typo

Signed-off-by: Cary Phillips <[email protected]>

* reference src/wrappers/python/README.md in pyproject.toml

Signed-off-by: Cary Phillips <[email protected]>

* Add copyright notice

Signed-off-by: Cary Phillips <[email protected]>

* Update pyproject.toml

Co-authored-by: Jean-Christophe Morin <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* Update pyproject.toml

Co-authored-by: Jean-Christophe Morin <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* Update src/wrappers/python/CMakeLists.txt

Co-authored-by: Jean-Christophe Morin <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* Add uninstall target (AcademySoftwareFoundation#1624)

* Add uninstall target

Satisfy the OpenSSF Best Practices Badge requirement for an
insta/uninstall process:
https://www.bestpractices.dev/en/criteria/1#1.installation_common

CMake does not support a standard "uninstall" target, but the
community recommends implementing an "uninstal" target that remove files named in the
`install_manifest.txt`:
https://gitlab.kitware.com/cmake/community/-/wikis/FAQ#can-i-do-make-uninstall-with-cmake

However, our existing process of installing the symlink to the "bare"
library, i.e. the symlink from libImath-3_2.so to libImath.so, fails
to add the symlink to the manifest, so "make uninstall" misses the
symlink. The existing mechanism use "install(CODE execute_process(cmake -E create_symlink))".

This changes that to use a simpler "file(CREATE_LINK)" and
"install(FILES)" to accomplish the same thing while also registering
the symlink the the manifest.

Also, this fixes an issue where `OpenEXRConfig.h` was passed to
`install()` twice, producing two entries in `install_manifest.txt`.

Signed-off-by: Cary Phillips <[email protected]>

* mention uninstall in install instructions

Signed-off-by: Cary Phillips <[email protected]>

* poke

Signed-off-by: Cary Phillips <[email protected]>

* COPY_ON_ERROR

Signed-off-by: Cary Phillips <[email protected]>

* clarify the uninstall instructions

Signed-off-by: Cary Phillips <[email protected]>

---------

Signed-off-by: Cary Phillips <[email protected]>

* Add cmake.targets and OPENEXR_INSTALL=OFF

Signed-off-by: Cary Phillips <[email protected]>

* INSTALL_TOOLS=OFF

Signed-off-by: Cary Phillips <[email protected]>

* propogate OPENEXR_INSTALL to Imath

Signed-off-by: Cary Phillips <[email protected]>

* test1

Signed-off-by: Cary Phillips <[email protected]>

* OPENEXR_INSTALL_PKG_CONFIG

Signed-off-by: Cary Phillips <[email protected]>

* Fix CVE 2023 5841 (AcademySoftwareFoundation#1627)

* enable deep file checks for core

Signed-off-by: Kimball Thurston <[email protected]>

* fix possible int overflow

Signed-off-by: Kimball Thurston <[email protected]>

* fix validation of deep sample counts

Addresses CVE-2023-5841, fixing sample count check to not only check
against 0 but previous sample as well.

Signed-off-by: Kimball Thurston <[email protected]>

* add clarifying comment

Signed-off-by: Kimball Thurston <[email protected]>

---------

Signed-off-by: Kimball Thurston <[email protected]>

* Bazel support: Bump Imath to 3.1.10 (AcademySoftwareFoundation#1626)

Signed-off-by: Vertexwahn <[email protected]>

* Document security expectations (AcademySoftwareFoundation#1623)

* Document security expectations

Signed-off-by: Cary Phillips <[email protected]>

* Menion Imath as a dependency

Signed-off-by: Cary Phillips <[email protected]>

* Update SECURITY.md

Co-authored-by: Nick Porcino <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* change 'Threat Model' to 'Potential Vulnerabilties'

Signed-off-by: Cary Phillips <[email protected]>

* Mention GitHub issue as fallback security contact

Signed-off-by: Cary Phillips <[email protected]>

* github security advisory

Signed-off-by: Cary Phillips <[email protected]>

* mention exrcheck

Signed-off-by: Cary Phillips <[email protected]>

---------

Signed-off-by: Cary Phillips <[email protected]>
Co-authored-by: Nick Porcino <[email protected]>

* Add uninstall target (AcademySoftwareFoundation#1624)

* Add uninstall target

Satisfy the OpenSSF Best Practices Badge requirement for an
insta/uninstall process:
https://www.bestpractices.dev/en/criteria/1#1.installation_common

CMake does not support a standard "uninstall" target, but the
community recommends implementing an "uninstal" target that remove files named in the
`install_manifest.txt`:
https://gitlab.kitware.com/cmake/community/-/wikis/FAQ#can-i-do-make-uninstall-with-cmake

However, our existing process of installing the symlink to the "bare"
library, i.e. the symlink from libImath-3_2.so to libImath.so, fails
to add the symlink to the manifest, so "make uninstall" misses the
symlink. The existing mechanism use "install(CODE execute_process(cmake -E create_symlink))".

This changes that to use a simpler "file(CREATE_LINK)" and
"install(FILES)" to accomplish the same thing while also registering
the symlink the the manifest.

Also, this fixes an issue where `OpenEXRConfig.h` was passed to
`install()` twice, producing two entries in `install_manifest.txt`.

Signed-off-by: Cary Phillips <[email protected]>

* mention uninstall in install instructions

Signed-off-by: Cary Phillips <[email protected]>

* poke

Signed-off-by: Cary Phillips <[email protected]>

* COPY_ON_ERROR

Signed-off-by: Cary Phillips <[email protected]>

* clarify the uninstall instructions

Signed-off-by: Cary Phillips <[email protected]>

---------

Signed-off-by: Cary Phillips <[email protected]>

* Remove snyk-scan-pr.yml (AcademySoftwareFoundation#1631)

This workflow is causing errors on each PR:

  Snyk is missing auth token in order to run inside CI. You must include your API token as an environment value: `SNYK_TOKEN=12345678`
  Error: Process completed with exit code 2.

As discussed on AcademySoftwareFoundation#1608, the preferred workflow will run weekly, not on PR.

Signed-off-by: Cary Phillips <[email protected]>

* fix issue with unpacking sample counts (AcademySoftwareFoundation#1630)

When unpacking sample counts as "individual" counts (no longer
monotonic), it writes the total sample count to a value 1 past the
individual sample counts, but this is not in the packed data, so do not
expect to unpack that many values. The buffer just needs to be allocated
one value larger to avoid write past end of buffer which is taken care
of in the update_pack_unpack_ptrs function

Signed-off-by: Kimball Thurston <[email protected]>

* adjust checks for core to better match c++ checks (AcademySoftwareFoundation#1632)

The core checks were not setting the same image / tile size limits and
not disabling reads at quite the same level.

Note: the core check does not read the entire image into a contiguous
slice, so does not replicate the maximum deep sample checks in the same
way, this is a source of potential false-negative failures

This should address OSS-Fuzz 66491 and 66489 (different forms of the
same failure where a large sample size allocation was happening), and
are only constrained memory (2.5Gb) issues.

Signed-off-by: Kimball Thurston <[email protected]>

* Fix install of symlink (AcademySoftwareFoundation#1633)

PR AcademySoftwareFoundation#1624 caused the .so symlink without the `OPENEXR_LIB_SUFFIX`
(e.g. libOpenEXR.so which links to libOpenEXR-3_2.so) to get created
in the wrong directory. This caused certain invocations of cmake to
fail, even though the invocation in the CI succeeded. It's not at all
clear why.

This also changes the CI to invoke cmake in the way that previously
failed (e.g. from the top-level directory with `-B` and `-S`), as an additional check.

Signed-off-by: Cary Phillips <[email protected]>

* adds a shortcut to avoid reconstructing every call (AcademySoftwareFoundation#1634)

When there is a loop trying to get scan / tile info that is ignoring
return values, add a shortcut to avoid trying to reconstruct the chunk
table every time. This will still respect the strict header flag, either
returning an error immediately (strict), or (non-strict) enabling a
multi-part file with only partially corrupt parts to work.

Signed-off-by: Kimball Thurston <[email protected]>

* check and control reduceMemory and reduceTime in stream mode (AcademySoftwareFoundation#1635)

exrcheck by default uses file mode, but the fuzzer and exrcheck -s use
stream mode, need to respect the memory and time flags consistently on
that path as well.

Will address OSS-Fuzz 66612, although real fix underlying is in AcademySoftwareFoundation#1634

Signed-off-by: Kimball Thurston <[email protected]>

* Update .github/workflows/python-wheels-publish-test.yml

Co-authored-by: Jean-Christophe Morin <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* Add sdist

Signed-off-by: Cary Phillips <[email protected]>

* Update .github/workflows/python-wheels-publish-test.yml

Co-authored-by: Jean-Christophe Morin <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* fix sdist; remove debugging

Signed-off-by: Cary Phillips <[email protected]>

---------

Signed-off-by: Cary Phillips <[email protected]>
Signed-off-by: Kimball Thurston <[email protected]>
Signed-off-by: Vertexwahn <[email protected]>
Co-authored-by: Jean-Christophe Morin <[email protected]>
Co-authored-by: Kimball Thurston <[email protected]>
Co-authored-by: Vertexwahn <[email protected]>
Co-authored-by: Nick Porcino <[email protected]>
  • Loading branch information
5 people committed Feb 13, 2024
1 parent 231ab0b commit 7d5ca5f
Show file tree
Hide file tree
Showing 19 changed files with 679 additions and 208 deletions.
2 changes: 2 additions & 0 deletions .github/workflows/ci_workflow.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ on:
- 'website/src/**'
- '!bazel/**'
- '!src/wrappers/**'
- '!.github/workflows/python-**.yml'
pull_request:
branches-ignore:
- RB-2.*
Expand All @@ -38,6 +39,7 @@ on:
- 'website/src/**'
- '!bazel/**'
- '!src/wrappers/**'
- '!.github/workflows/python-**.yml'

permissions:
contents: read
Expand Down
102 changes: 102 additions & 0 deletions .github/workflows/python-wheels-publish-test.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
# SPDX-License-Identifier: BSD-3-Clause
# Copyright (c) Contributors to the OpenEXR Project.

name: Publish python distribution 📦 to TestPyPI

on:

# Publish python wheels to test.pypi when a release candidate is tagged,
# e.g. v3.4.5-rc, v3.4.5-rc6, etc.

push:
tags:
- v3.[0-9]+.[0-9]+-rc*
workflow_dispatch:

permissions:
contents: read

jobs:
build:
name: Python Wheels - ${{ matrix.os }}
runs-on: ${{ matrix.os }}
strategy:
matrix:
os: [ubuntu-latest, macos-latest, windows-latest]

environment:
name: testpypi
url: https://test.pypi.org/p/openexr

permissions:
id-token: write

steps:
- name: Checkout
uses: actions/checkout@v4

- name: Install Python
uses: actions/setup-python@v5
with:
python-version: '3.x'

- name: Create sdist
# Only create it once.
if: ${{ matrix.os == 'ubuntu-latest' }}
run: pipx run build --sdist . --outdir wheelhouse

- name: Build wheel
uses: pypa/[email protected]
with:
output-dir: wheelhouse
env:
CIBW_ARCHS_LINUX: x86_64
CIBW_ARCHS_MACOS: x86_64 arm64 universal2
# Skip python 3.6 since scikit-build-core requires 3.7+
# Skip 32-bit wheels builds on Windows
# Also skip the PyPy builds, since they fail the unit tests
CIBW_SKIP: cp36-* *-win32 *_i686 pp*
CIBW_TEST_SKIP: "*-macosx_universal2:arm64"
CIBW_ENVIRONMENT: OPENEXR_RELEASE_CANDIDATE_TAG="${{ github.ref_name }}"

- name: Upload artifact
uses: actions/[email protected]
with:
name: wheels-${{ matrix.os }}
path: |
./wheelhouse/*.whl
./wheelhouse/*.tar.gz
publish-to-testpypi:
name: Publish Python 🐍 distribution 📦 to TestPyPI
needs:
- build
runs-on: ubuntu-latest

environment:
name: testpypi
url: https://test.pypi.org/p/openexr

permissions:
id-token: write

steps:
- name: Download Linux artifacts
uses: actions/[email protected]
with:
name: wheels-ubuntu-latest
path: dist
- name: Download macOS artifacts
uses: actions/[email protected]
with:
name: wheels-macos-latest
path: dist
- name: Download Windows artifacts
uses: actions/[email protected]
with:
name: wheels-windows-latest
path: dist
- name: Publish distribution 📦 to TestPyPI
uses: pypa/gh-action-pypi-publish@release/v1
with:
repository-url: https://test.pypi.org/legacy/
96 changes: 96 additions & 0 deletions .github/workflows/python-wheels-publish.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
# SPDX-License-Identifier: BSD-3-Clause
# Copyright (c) Contributors to the OpenEXR Project.

name: Publish python distribution 📦 to PyPI

on:
# Publish wheels to pypi on release
release:
types: [published]
workflow_dispatch:

permissions:
contents: read

jobs:
build:
name: Python Wheels - ${{ matrix.os }}
runs-on: ${{ matrix.os }}
strategy:
matrix:
os: [ubuntu-latest, macos-latest, windows-latest]

environment:
name: pypi
url: https://pypi.org/p/openexr

permissions:
id-token: write

steps:
- name: Checkout
uses: actions/checkout@v4

- name: Install Python
uses: actions/setup-python@v5
with:
python-version: '3.x'

- name: Create sdist
# Only create it once.
if: ${{ matrix.os == 'ubuntu-latest' }}
run: pipx run build --sdist . --outdir wheelhouse

- name: Build wheel
uses: pypa/[email protected]
with:
output-dir: wheelhouse
env:
CIBW_BUILD: cp312-*
CIBW_ARCHS_LINUX: x86_64
CIBW_ARCHS_MACOS: x86_64 arm64 universal2
# Skip python 3.6 since scikit-build-core requires 3.7+
# Skip 32-bit wheels builds on Windows
# Also skip the PyPy builds, since they fail the unit tests
CIBW_SKIP: cp36-* *-win32 *_i686 pp*
CIBW_TEST_SKIP: "*arm64"

- name: Upload artifact
uses: actions/[email protected]
with:
name: wheels-${{ matrix.os }}
path: |
./wheelhouse/*.whl
./wheelhouse/*.tar.gz
publish-to-pypi:
name: Publish Python 🐍 distribution 📦 to PyPI
needs:
- build
runs-on: ubuntu-latest

environment:
name: pypi
url: https://pypi.org/p/openexr

permissions:
id-token: write

steps:
- name: Download Linux artifacts
uses: actions/[email protected]
with:
name: wheels-ubuntu-latest
path: dist
- name: Download macOS artifacts
uses: actions/[email protected]
with:
name: wheels-macos-latest
path: dist
- name: Download Windows artifacts
uses: actions/[email protected]
with:
name: wheels-windows-latest
path: dist
- name: Publish distribution 📦 to PyPI
uses: pypa/gh-action-pypi-publish@release/v1
109 changes: 46 additions & 63 deletions .github/workflows/python-wheels.yml
Original file line number Diff line number Diff line change
@@ -1,91 +1,74 @@
# SPDX-License-Identifier: BSD-3-Clause
# Copyright (c) Contributors to the OpenEXR Project.
#

name: Python wheels
name: Python Wheels

on:

# Run on all changes (PR and push) to the python binding
# source/configuration files, except on the release branches, which
# have their own workflow, which also publish to pypi/test.pypi.
# Note that changes to the core libraries will *not*
# trigger building the wheels. However, the main ci workflow does
# build and test the bindings (for a single python version on a
# single arch)

push:
branches-ignore:
- RB-2.*
tags-ignore:
- v1.*
- v2.*
- RB-*
paths:
- '**'
- '!**.md'
- '!website/**'
- 'website/src/**'
- '!bazel/**'
- 'src/wrappers/python/**'
- 'pyproject.toml'
- '.github/workflows/python-wheels.yml'
pull_request:
branches-ignore:
- RB-2.*
tags-ignore:
- v1.*
- v2.*
- RB-*
paths:
- '**'
- '!**.md'
- '!website/**'
- 'website/src/**'
- '!bazel/**'
- 'src/wrappers/python/**'
- 'pyproject.toml'
- '.github/workflows/python-wheels.yml'

permissions:
contents: read

jobs:
build_wheels:
name: Build Python wheels
name: Python Wheels - ${{ matrix.os }}
runs-on: ${{ matrix.os }}
strategy:
matrix:
os: [ubuntu-22.04, windows-latest, macOS-latest]
env:
# On macOS we build both x86 and arm to support Intel and Apple Silicon.
CIBW_ARCHS_MACOS: x86_64 arm64
# Skip 32-bit wheels builds on Windows.
# Also skip the PyPy builds, since they fail the unittests
CIBW_SKIP: "*-win32 *_i686 pp*"
# The CI platform is Intel based so we are doing cross compilation
# for arm64. It is not currently possible to test arm64 when cross
# compiling.
CIBW_TEST_SKIP: "*_arm64"
CIBW_BEFORE_BUILD: >
echo "Installing OpenEXR..." &&
cd openexr.build &&
cmake -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=../openexr.install -DCMAKE_VERBOSE_MAKEFILE:BOOL=ON -DCMAKE_PREFIX_PATH=../openexr.install -DCMAKE_INSTALL_LIBDIR=lib -DBUILD_TESTING=OFF -DOPENEXR_INSTALL_EXAMPLES=OFF -DOPENEXR_BUILD_TOOLS=OFF -DBUILD_SHARED_LIBS=OFF -DOPENEXR_FORCE_INTERNAL_DEFLATE=ON -DOPENEXR_FORCE_INTERNAL_IMATH=ON -DCMAKE_POSITION_INDEPENDENT_CODE=ON ../ &&
cmake --build ./ --config Release --clean-first &&
cmake --install ./ --config Release &&
cd ..
CIBW_TEST_REQUIRES: pytest
CIBW_TEST_COMMAND: pytest {project}/src/wrappers/python/tests/
os: [ubuntu-latest, macos-latest, windows-latest]

steps:
- uses: actions/checkout@v3

# Used to host cibuildwheel
- uses: actions/setup-python@v4
with:
python-version: '3.x'

- name: Install cibuildwheel
run: python -m pip install cibuildwheel==2.16.2
- name: Checkout
uses: actions/checkout@v4

- name: Create setup.py
run: |
mv ${{github.workspace}}/src/wrappers/python/setup.py ${{github.workspace}}/setup.py
mv ${{github.workspace}}/src/wrappers/python/Imath.py ${{github.workspace}}/Imath.py
mv ${{github.workspace}}/src/wrappers/python/OpenEXR.cpp ${{github.workspace}}/OpenEXR.cpp
- name: Install Python
uses: actions/setup-python@v5
with:
python-version: '3.x'

- name: Create folders
run: |
mkdir -p ${{github.workspace}}/openexr.build
mkdir -p ${{github.workspace}}/openexr.install
- name: Create sdist
# Only create it once.
if: ${{ matrix.os == 'ubuntu-latest' }}
run: pipx run build --sdist . --outdir wheelhouse

- name: Build wheels
run: python -m cibuildwheel --output-dir wheelhouse
- name: Build wheel
uses: pypa/[email protected]
env:
CIBW_ARCHS_MACOS: x86_64 arm64 universal2
# Skip python 3.6 since scikit-build-core requires 3.7+
# Skip 32-bit wheels builds on Windows
# Also skip the PyPy builds, since they fail the unit tests
CIBW_SKIP: cp36-* *-win32 *_i686 pp*
CIBW_TEST_SKIP: "*-macosx*arm64"

- uses: actions/upload-artifact@v3
- name: Upload artifact
uses: actions/upload-artifact@v4
with:
name: "Python wheels"
path: ./wheelhouse/*.whl
name: wheels-${{ matrix.os }}
path: |
./wheelhouse/*.whl
./wheelhouse/*.tar.gz
4 changes: 4 additions & 0 deletions .github/workflows/website_workflow.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,17 @@ on:
branches:-ignore:
- RB-2.*
- RB-3.*
tags-ignore:
- v3.[0-9]+.[0-9]+-rc*
paths:
- 'website/**'

pull_request:
branches:-ignore:
- RB-2.*
- RB-3.*
tags-ignore:
- v3.[0-9]+.[0-9]+-rc*
paths:
- 'website/**'

Expand Down
Loading

0 comments on commit 7d5ca5f

Please sign in to comment.