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 python wheels via scikit-build-core #1629

Merged
merged 28 commits into from
Feb 13, 2024

Conversation

cary-ilm
Copy link
Member

@cary-ilm cary-ilm commented Feb 8, 2024

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.

Other changes:

  • Consolidate CMake BUILD/INSTALL options in a central place in cmake/OpenEXRSetup.cmake
    and make the pairing of BUILD and INSTALL consistent. The options are:
  option(OPENEXR_BUILD_LIBS "Enables building of main libraries" ON)
  option(OPENEXR_BUILD_TOOLS "Enables building of utility programs" ON)
  option(OPENEXR_INSTALL_TOOLS "Install OpenEXR tools" ON)
  option(OPENEXR_BUILD_EXAMPLES "Build OpenEXR examples" ON)
  option(OPENEXR_INSTALL_EXAMPLES "Install OpenEXR examples" ON)
  option(OPENEXR_BUILD_PYTHON "Build python bindings" OFF)
  option(OPENEXR_TEST_LIBRARIES "Run library tests" ON)
  option(OPENEXR_TEST_TOOLS "Run tool tests" ON)
  option(OPENEXR_TEST_PYTHON "Run python binding tests" ON)
  • The wheel build needs the software version, and rather than manual parsing of the openexr_version.h header, it gets it via an invocation of cmake. The new BUILD_LIBS option tells cmake to ignore the libraries entirely in this case (otherwise it would fetch and download libdeflate and Imath).

  • The OPENEXR_TEST_LIBRARIES option disables the library tests, which isn't necessary for the python tests during wheel building.

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

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]>
Signed-off-by: Cary Phillips <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>
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.

This looks great!

I left some notes to show how you can fix the problem where you can't import OpenEXR after pip installing it.

I also noticed that some things should be pruned from the wheel:

include/Imath
include/OpenEXR
lib/cmake
lib/libIex-3_3.a
lib/libIlmThread-3_3.a
lib/libImath-3_2.a
lib/libOpenEXR-3_3.a
lib/libOpenEXRCore-3_3.a
lib/libOpenEXRUtil-3_3.a
lib/pkgconfig

These would either pollute site-packages or outright be installed in "random" places.

Little trick, a wheel is just a zip file. So after building it locally, you can inspect it with unzip -l <path.whl>.

I expect the final wheel to only contain these files:

Imath.py
OpenEXR.so
openexr-3.3.0.dist-info/METADATA
openexr-3.3.0.dist-info/WHEEL
openexr-3.3.0.dist-info/entry_points.txt
openexr-3.3.0.dist-info/licenses/LICENSE.md
openexr-3.3.0.dist-info/RECORD

Let me know if you have questions!

pyproject.toml Outdated Show resolved Hide resolved
src/wrappers/python/CMakeLists.txt Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
cary-ilm and others added 2 commits February 9, 2024 09:16
Co-authored-by: Jean-Christophe Morin <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>
Co-authored-by: Jean-Christophe Morin <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>
@cary-ilm
Copy link
Member Author

cary-ilm commented Feb 9, 2024

Thanks for the guidance and the fixes! For the files you mentioned above that should be pruned, what's the mechanism to do that?

Co-authored-by: Jean-Christophe Morin <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>
@JeanChristopheMorinPerso
Copy link
Member

Thanks for the guidance and the fixes! For the files you mentioned above that should be pruned, what's the mechanism to do that?

I tried setting cmake.targets = ["PyOpenEXR"] thinking that only the PyOpenEXR target would be installed, but it's also installing the includes and static libs. So I guess putting the installation of includes, libs and cmake files in an if block or something? Or maybe there is a cleaner way to tell cmake to not install these when cmake --install <build dir> --targets PyOpenEXR is run...

Maybe you could use CMAKE_SKIP_INSTALL_ALL_DEPENDENCY like

if(SKBUILD)
  set(CMAKE_SKIP_INSTALL_ALL_DEPENDENCY true)
endif()

in the main CMakeLists.txt or in src/wrappers/python/CMakeLists.txt and set cmake.targets = ["PyOpenEXR"] in pyproject.toml?

@JeanChristopheMorinPerso
Copy link
Member

I just did a quick test, and setting cmake.targets = ['PyOpenEXR'] and adding OPENEXR_INSTALL = 'OFF' in tool.scikit-build.cmake.define gets us close. We are left with the Imath stuff. CMAKE_SKIP_INSTALL_ALL_DEPENDENCY doesn't seem to have the desired effect...

I'm not sure how we could get rid of the Imath stuff (cmake configs, includes, pkgconfig and lib). But I'm guessing you'll probably know how to do that.

* 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]>
Signed-off-by: Cary Phillips <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>
@cary-ilm
Copy link
Member Author

Imath needs an IMATH_INSTALL option so that OpenEXR can propagate OPENEXR_INSTALL to it: AcademySoftwareFoundation/Imath#366

With that change, the contents of the wheel looks good:

./Imath.py
./OpenEXR.so
./openexr.libs
./openexr-3.3.0.dist-info
./openexr-3.3.0.dist-info/RECORD
./openexr-3.3.0.dist-info/licenses
./openexr-3.3.0.dist-info/licenses/LICENSE.md
./openexr-3.3.0.dist-info/WHEEL
./openexr-3.3.0.dist-info/entry_points.txt
./openexr-3.3.0.dist-info/METADATA

@JeanChristopheMorinPerso
Copy link
Member

Great! What's is ./openexr.libs in the wheel?

@cary-ilm
Copy link
Member Author

It's an empty directory. I'm not sure where it's coming from.

kdt3rd and others added 9 commits February 11, 2024 09:52
* 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]>
* 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

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]>
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]>
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]>
…ndation#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]>
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]>
…undation#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]>
…SoftwareFoundation#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]>
@JeanChristopheMorinPerso
Copy link
Member

Ok thanks. I guess we'll have to start a hunt to find out where it's coming from.

@JeanChristopheMorinPerso
Copy link
Member

Umm, openexr.libs is not in thw eheel when I build locally with your latest changes:

Archive:  dist/openexr-3.3.0-cp311-cp311-manylinux_2_38_x86_64.whl
  Length      Date    Time    Name
---------  ---------- -----   ----
     9236  2024-02-09 01:08   Imath.py
  3860984  2024-02-11 18:18   OpenEXR.so
     6815  2024-02-11 18:18   openexr-3.3.0.dist-info/METADATA
      117  2024-02-11 18:18   openexr-3.3.0.dist-info/WHEEL
        0  2024-02-11 18:18   openexr-3.3.0.dist-info/entry_points.txt
     1501  2024-02-11 18:18   openexr-3.3.0.dist-info/licenses/LICENSE.md
      536  2024-02-11 18:18   openexr-3.3.0.dist-info/RECORD
---------                     -------
  3879189                     7 files

I'll wait for the workflows to finish before looking further.

@JeanChristopheMorinPerso
Copy link
Member

Ah, it's there in the CI generated wheels. I think I know what it is. cibuildwheel runs auditwheel (auditwheel repair) to make sure that the wheel will really work and it's auditwheel that creates this folder. See https://github.com/pypa/auditwheel/blob/db7962149a3e971eba3974a58edcb2226c787f8d/src/auditwheel/main_repair.py#L54-L62.

So it's all good.

cary-ilm and others added 2 commits February 12, 2024 12:11
Co-authored-by: Jean-Christophe Morin <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>
@cary-ilm
Copy link
Member Author

@JeanChristopheMorinPerso, I added the sdist step but the wheels don't see to have any additional sourced components. I download an unzip the unbunt-latest artifact, shouldn't it include source? Did I miss a step?

cary-ilm and others added 2 commits February 12, 2024 17:13
Co-authored-by: Jean-Christophe Morin <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>
@cary-ilm
Copy link
Member Author

This looks like it's in good shape now, I'm going to merge it and address any further issues separately. @JeanChristopheMorinPerso, thanks for your copious help!

@cary-ilm cary-ilm merged commit 6905396 into AcademySoftwareFoundation:main Feb 13, 2024
30 checks passed
cary-ilm added a commit to cary-ilm/openexr that referenced this pull request Feb 13, 2024
…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]>
cary-ilm added a commit that referenced this pull request Feb 16, 2024
* 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]>
cary-ilm added a commit to cary-ilm/openexr that referenced this pull request Mar 3, 2024
…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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants