Skip to content

Commit 17811af

Browse files
cary-ilmJeanChristopheMorinPersokdt3rdVertexwahnmeshula
committed
Build python wheels via scikit-build-core (#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 (#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 (#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 (#1626) Signed-off-by: Vertexwahn <[email protected]> * Document security expectations (#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 (#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 (#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 #1608, the preferred workflow will run weekly, not on PR. Signed-off-by: Cary Phillips <[email protected]> * fix issue with unpacking sample counts (#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 (#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 (#1633) PR #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 (#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 (#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 #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]>
1 parent 2d3fce4 commit 17811af

19 files changed

+679
-208
lines changed

.github/workflows/ci_workflow.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ on:
2525
- 'website/src/**'
2626
- '!bazel/**'
2727
- '!src/wrappers/**'
28+
- '!.github/workflows/python-**.yml'
2829
pull_request:
2930
branches-ignore:
3031
- RB-2.*
@@ -38,6 +39,7 @@ on:
3839
- 'website/src/**'
3940
- '!bazel/**'
4041
- '!src/wrappers/**'
42+
- '!.github/workflows/python-**.yml'
4143

4244
permissions:
4345
contents: read
Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
# SPDX-License-Identifier: BSD-3-Clause
2+
# Copyright (c) Contributors to the OpenEXR Project.
3+
4+
name: Publish python distribution 📦 to TestPyPI
5+
6+
on:
7+
8+
# Publish python wheels to test.pypi when a release candidate is tagged,
9+
# e.g. v3.4.5-rc, v3.4.5-rc6, etc.
10+
11+
push:
12+
tags:
13+
- v3.[0-9]+.[0-9]+-rc*
14+
workflow_dispatch:
15+
16+
permissions:
17+
contents: read
18+
19+
jobs:
20+
build:
21+
name: Python Wheels - ${{ matrix.os }}
22+
runs-on: ${{ matrix.os }}
23+
strategy:
24+
matrix:
25+
os: [ubuntu-latest, macos-latest, windows-latest]
26+
27+
environment:
28+
name: testpypi
29+
url: https://test.pypi.org/p/openexr
30+
31+
permissions:
32+
id-token: write
33+
34+
steps:
35+
- name: Checkout
36+
uses: actions/checkout@v4
37+
38+
- name: Install Python
39+
uses: actions/setup-python@v5
40+
with:
41+
python-version: '3.x'
42+
43+
- name: Create sdist
44+
# Only create it once.
45+
if: ${{ matrix.os == 'ubuntu-latest' }}
46+
run: pipx run build --sdist . --outdir wheelhouse
47+
48+
- name: Build wheel
49+
uses: pypa/[email protected]
50+
with:
51+
output-dir: wheelhouse
52+
env:
53+
CIBW_ARCHS_LINUX: x86_64
54+
CIBW_ARCHS_MACOS: x86_64 arm64 universal2
55+
# Skip python 3.6 since scikit-build-core requires 3.7+
56+
# Skip 32-bit wheels builds on Windows
57+
# Also skip the PyPy builds, since they fail the unit tests
58+
CIBW_SKIP: cp36-* *-win32 *_i686 pp*
59+
CIBW_TEST_SKIP: "*-macosx_universal2:arm64"
60+
CIBW_ENVIRONMENT: OPENEXR_RELEASE_CANDIDATE_TAG="${{ github.ref_name }}"
61+
62+
- name: Upload artifact
63+
uses: actions/[email protected]
64+
with:
65+
name: wheels-${{ matrix.os }}
66+
path: |
67+
./wheelhouse/*.whl
68+
./wheelhouse/*.tar.gz
69+
70+
publish-to-testpypi:
71+
name: Publish Python 🐍 distribution 📦 to TestPyPI
72+
needs:
73+
- build
74+
runs-on: ubuntu-latest
75+
76+
environment:
77+
name: testpypi
78+
url: https://test.pypi.org/p/openexr
79+
80+
permissions:
81+
id-token: write
82+
83+
steps:
84+
- name: Download Linux artifacts
85+
uses: actions/[email protected]
86+
with:
87+
name: wheels-ubuntu-latest
88+
path: dist
89+
- name: Download macOS artifacts
90+
uses: actions/[email protected]
91+
with:
92+
name: wheels-macos-latest
93+
path: dist
94+
- name: Download Windows artifacts
95+
uses: actions/[email protected]
96+
with:
97+
name: wheels-windows-latest
98+
path: dist
99+
- name: Publish distribution 📦 to TestPyPI
100+
uses: pypa/gh-action-pypi-publish@release/v1
101+
with:
102+
repository-url: https://test.pypi.org/legacy/
Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
# SPDX-License-Identifier: BSD-3-Clause
2+
# Copyright (c) Contributors to the OpenEXR Project.
3+
4+
name: Publish python distribution 📦 to PyPI
5+
6+
on:
7+
# Publish wheels to pypi on release
8+
release:
9+
types: [published]
10+
workflow_dispatch:
11+
12+
permissions:
13+
contents: read
14+
15+
jobs:
16+
build:
17+
name: Python Wheels - ${{ matrix.os }}
18+
runs-on: ${{ matrix.os }}
19+
strategy:
20+
matrix:
21+
os: [ubuntu-latest, macos-latest, windows-latest]
22+
23+
environment:
24+
name: pypi
25+
url: https://pypi.org/p/openexr
26+
27+
permissions:
28+
id-token: write
29+
30+
steps:
31+
- name: Checkout
32+
uses: actions/checkout@v4
33+
34+
- name: Install Python
35+
uses: actions/setup-python@v5
36+
with:
37+
python-version: '3.x'
38+
39+
- name: Create sdist
40+
# Only create it once.
41+
if: ${{ matrix.os == 'ubuntu-latest' }}
42+
run: pipx run build --sdist . --outdir wheelhouse
43+
44+
- name: Build wheel
45+
uses: pypa/[email protected]
46+
with:
47+
output-dir: wheelhouse
48+
env:
49+
CIBW_BUILD: cp312-*
50+
CIBW_ARCHS_LINUX: x86_64
51+
CIBW_ARCHS_MACOS: x86_64 arm64 universal2
52+
# Skip python 3.6 since scikit-build-core requires 3.7+
53+
# Skip 32-bit wheels builds on Windows
54+
# Also skip the PyPy builds, since they fail the unit tests
55+
CIBW_SKIP: cp36-* *-win32 *_i686 pp*
56+
CIBW_TEST_SKIP: "*arm64"
57+
58+
- name: Upload artifact
59+
uses: actions/[email protected]
60+
with:
61+
name: wheels-${{ matrix.os }}
62+
path: |
63+
./wheelhouse/*.whl
64+
./wheelhouse/*.tar.gz
65+
66+
publish-to-pypi:
67+
name: Publish Python 🐍 distribution 📦 to PyPI
68+
needs:
69+
- build
70+
runs-on: ubuntu-latest
71+
72+
environment:
73+
name: pypi
74+
url: https://pypi.org/p/openexr
75+
76+
permissions:
77+
id-token: write
78+
79+
steps:
80+
- name: Download Linux artifacts
81+
uses: actions/[email protected]
82+
with:
83+
name: wheels-ubuntu-latest
84+
path: dist
85+
- name: Download macOS artifacts
86+
uses: actions/[email protected]
87+
with:
88+
name: wheels-macos-latest
89+
path: dist
90+
- name: Download Windows artifacts
91+
uses: actions/[email protected]
92+
with:
93+
name: wheels-windows-latest
94+
path: dist
95+
- name: Publish distribution 📦 to PyPI
96+
uses: pypa/gh-action-pypi-publish@release/v1

.github/workflows/python-wheels.yml

Lines changed: 46 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -1,91 +1,74 @@
11
# SPDX-License-Identifier: BSD-3-Clause
22
# Copyright (c) Contributors to the OpenEXR Project.
3-
#
43

5-
name: Python wheels
4+
name: Python Wheels
65

76
on:
7+
8+
# Run on all changes (PR and push) to the python binding
9+
# source/configuration files, except on the release branches, which
10+
# have their own workflow, which also publish to pypi/test.pypi.
11+
# Note that changes to the core libraries will *not*
12+
# trigger building the wheels. However, the main ci workflow does
13+
# build and test the bindings (for a single python version on a
14+
# single arch)
15+
816
push:
917
branches-ignore:
10-
- RB-2.*
11-
tags-ignore:
12-
- v1.*
13-
- v2.*
18+
- RB-*
1419
paths:
15-
- '**'
16-
- '!**.md'
17-
- '!website/**'
18-
- 'website/src/**'
19-
- '!bazel/**'
20+
- 'src/wrappers/python/**'
21+
- 'pyproject.toml'
22+
- '.github/workflows/python-wheels.yml'
2023
pull_request:
2124
branches-ignore:
22-
- RB-2.*
23-
tags-ignore:
24-
- v1.*
25-
- v2.*
25+
- RB-*
2626
paths:
27-
- '**'
28-
- '!**.md'
29-
- '!website/**'
30-
- 'website/src/**'
31-
- '!bazel/**'
27+
- 'src/wrappers/python/**'
28+
- 'pyproject.toml'
29+
- '.github/workflows/python-wheels.yml'
3230

3331
permissions:
3432
contents: read
3533

3634
jobs:
3735
build_wheels:
38-
name: Build Python wheels
36+
name: Python Wheels - ${{ matrix.os }}
3937
runs-on: ${{ matrix.os }}
4038
strategy:
4139
matrix:
42-
os: [ubuntu-22.04, windows-latest, macOS-latest]
43-
env:
44-
# On macOS we build both x86 and arm to support Intel and Apple Silicon.
45-
CIBW_ARCHS_MACOS: x86_64 arm64
46-
# Skip 32-bit wheels builds on Windows.
47-
# Also skip the PyPy builds, since they fail the unittests
48-
CIBW_SKIP: "*-win32 *_i686 pp*"
49-
# The CI platform is Intel based so we are doing cross compilation
50-
# for arm64. It is not currently possible to test arm64 when cross
51-
# compiling.
52-
CIBW_TEST_SKIP: "*_arm64"
53-
CIBW_BEFORE_BUILD: >
54-
echo "Installing OpenEXR..." &&
55-
cd openexr.build &&
56-
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 ../ &&
57-
cmake --build ./ --config Release --clean-first &&
58-
cmake --install ./ --config Release &&
59-
cd ..
60-
CIBW_TEST_REQUIRES: pytest
61-
CIBW_TEST_COMMAND: pytest {project}/src/wrappers/python/tests/
40+
os: [ubuntu-latest, macos-latest, windows-latest]
6241

6342
steps:
64-
- uses: actions/checkout@v3
65-
66-
# Used to host cibuildwheel
67-
- uses: actions/setup-python@v4
68-
with:
69-
python-version: '3.x'
7043

71-
- name: Install cibuildwheel
72-
run: python -m pip install cibuildwheel==2.16.2
44+
- name: Checkout
45+
uses: actions/checkout@v4
7346

74-
- name: Create setup.py
75-
run: |
76-
mv ${{github.workspace}}/src/wrappers/python/setup.py ${{github.workspace}}/setup.py
77-
mv ${{github.workspace}}/src/wrappers/python/Imath.py ${{github.workspace}}/Imath.py
78-
mv ${{github.workspace}}/src/wrappers/python/OpenEXR.cpp ${{github.workspace}}/OpenEXR.cpp
47+
- name: Install Python
48+
uses: actions/setup-python@v5
49+
with:
50+
python-version: '3.x'
7951

80-
- name: Create folders
81-
run: |
82-
mkdir -p ${{github.workspace}}/openexr.build
83-
mkdir -p ${{github.workspace}}/openexr.install
52+
- name: Create sdist
53+
# Only create it once.
54+
if: ${{ matrix.os == 'ubuntu-latest' }}
55+
run: pipx run build --sdist . --outdir wheelhouse
8456

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

88-
- uses: actions/upload-artifact@v3
67+
- name: Upload artifact
68+
uses: actions/upload-artifact@v4
8969
with:
90-
name: "Python wheels"
91-
path: ./wheelhouse/*.whl
70+
name: wheels-${{ matrix.os }}
71+
path: |
72+
./wheelhouse/*.whl
73+
./wheelhouse/*.tar.gz
74+

.github/workflows/website_workflow.yml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,17 @@ on:
1414
branches:-ignore:
1515
- RB-2.*
1616
- RB-3.*
17+
tags-ignore:
18+
- v3.[0-9]+.[0-9]+-rc*
1719
paths:
1820
- 'website/**'
1921

2022
pull_request:
2123
branches:-ignore:
2224
- RB-2.*
2325
- RB-3.*
26+
tags-ignore:
27+
- v3.[0-9]+.[0-9]+-rc*
2428
paths:
2529
- 'website/**'
2630

0 commit comments

Comments
 (0)