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

v3.2.3 commits and release notes #1638

Merged
merged 33 commits into from
Feb 16, 2024

Conversation

cary-ilm
Copy link
Member

Cherry-picked commits for a v3.2.3 release, for your preview. Highlights include:

  • Fix bswap on NetBSD
  • Fix issue with decompressing fp32 dwa files
  • Support cmake config for libdeflate
  • Add uninstall cmake target
  • Updated security policy
  • Miscellaneous website improvements
  • python wheel building via scikit-build-core

This release will addresses:

  • OSS-fuzz 66612 Null-dereference in Imf_3_3::realloc_deepdata

Merged Pull Requests:

  • 1636 Do synk scans weekly on Sunday mornings
  • 1635 check and control reduceMemory and reduceTime in stream mode
  • 1634 adds a shortcut to avoid reconstructing every call
  • 1633 Fix install of symlink
  • 1631 Remove snyk-scan-pr.yml
  • 1629 Build python wheels via scikit-build-core
  • 1626 Bazel support: Bump Imath to 3.1.10
  • 1624 Add uninstall target
  • 1623 Document security expectations
  • 1622 Add a reference to building tools from source to the tools webpage.
  • 1621 Add explanation of distinction between OpenEXR/OpenEXRCore to API section
  • 1620 Make 'Hello, World' example reader/writer downloadable
  • 1615 Fix spelling of GitHub
  • 1613 Support cmake config for libdeflate
  • 1612 Fix bswap on NetBSD
  • 1611 Update MacPorts install instructions
  • 1608 CI/CD - Added Snyk C/C++ Scanning Job
  • 1605 Bump skylib in workspace approach
  • 1600 Release notes and news for v2.5.10
  • 1597 Account for duplicate emails with .mailmap
  • 1595 add deep id/manifest tools and doc
  • 1592 Remove some dead code when writing
  • 1591 Fix issue with decompressing fp32 dwa files
  • 1587 Fix formatting of sample exr file in OpenEXRFileLayout.rst - 3rd attempt
  • 1583 Converting code-blocks to literalincludes in ReadingAndWritingImageFiles.rst
  • 1579 python-wheels.yml - add arm64 builds for macOS
  • 1578 adding better error reporting for bin tests
  • 1577 Add tests for the Header class.
  • 1576 python-wheels.yml - bumps cibuildwheel version
  • 1575 fix typo in README.md
  • 1570 install.rst - update $ to % in the example shell prompts

kdt3rd and others added 30 commits February 12, 2024 13:07
…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]>
* fix typo in README.md

change make -> cmake

Signed-off-by: Barnaby Robson <[email protected]>

* removes mkdir.

Suggested by Milos in review. I tested it and it works.

Signed-off-by: Barnaby Robson <[email protected]>

---------

Signed-off-by: Barnaby Robson <[email protected]>
…tion#1579)

The CI platform that builds the wheels is Intel based and
by default only builds Intel wheels.

Modern Macs are all based on Apple Silicon so to support them
we need to also build arm64. This is done via cross compiling.

It is not possible to test when cross compiling according to:

https://cibuildwheel.readthedocs.io/en/stable/faq/#how-to-cross-compile

Signed-off-by: Barnaby Robson <[email protected]>
These are intended to exercise methods that are currently not covered by
other tests according to the coverage report.

Signed-off-by: Ben Grimes <[email protected]>
…n#1591)

There was an issue with the data type not passed to the DCT decompressor in the DWA decoder. That caused 32 bit DWA images to look squashed, as the decompressed 16 bit values were not getting expanded back to 32 bit.

Signed-off-by: Anton Dukhovnikov <[email protected]>
The file version is set to 2 above for writing files (it is impossible write version 1 data). Please refer to the history of history of ImfDwaCompressor.cpp or look at the reading code for details on the version 1 format.

Signed-off-by: Dave Sawyer <[email protected]>
* add deep id/manifest tools and docs

Signed-off-by: Peter Hillman <[email protected]>

* fix const-ness in deepidexample

Signed-off-by: Peter Hillman <[email protected]>

* fix link on doc, better variable name

Signed-off-by: Peter Hillman <[email protected]>

* add discussion of cryptomatte and deep compositing

Signed-off-by: Peter Hillman <[email protected]>

* fix names of cryptomatte ID channels

Signed-off-by: Peter Hillman <[email protected]>

---------

Signed-off-by: Peter Hillman <[email protected]>
…penEXRFileLayout.rst - 3rd attempt (AcademySoftwareFoundation#1587)

* Replaced the sample exr file code blocks with a grid table formatted with bytes of the sample file in one column. The value in the second column and the third column being the description of the bytes and value.

Signed-off-by: An Nguyen <[email protected]>

* * Updated so the ascii values for the bytes are aligned to each byte.
* Gave the table a title and reduce the width to 50%

Signed-off-by: An Nguyen <[email protected]>

* Added missing 4 bytes missing from the dataWindow value and displayWindow value

Signed-off-by: An Nguyen <[email protected]>

* Added the sample.exr file

Signed-off-by: An Nguyen <[email protected]>

* Annotated the attribute values more for the header. Should be a bit clearer.

Signed-off-by: An Nguyen <[email protected]>

* Adding the sample.exr file was documented.

Signed-off-by: An Nguyen <[email protected]>

* Added download link to the sample.exr file.

Signed-off-by: An Nguyen <[email protected]>

* Move the download link above the table. Updated the paragraph before the table to describe the table

Signed-off-by: An Nguyen <[email protected]>

* Move the sample.exr image into the downloads directory to match the target destination naming scheme to avoid confusion where the files go when built

Signed-off-by: An Nguyen <[email protected]>

* Updated the table to annotate the type and literal as type.

Signed-off-by: An Nguyen <[email protected]>

---------

Signed-off-by: An Nguyen <[email protected]>
…les.rst (AcademySoftwareFoundation#1583)

* Converting code-blocks to literalincludes in ReadingAndWritingImageFiles.rst

Signed-off-by: Megha S <[email protected]>

* Addressed review comments

Signed-off-by: Megha S <[email protected]>

* Changing from std::clamp to Imath::clamp

Signed-off-by: Megha S <[email protected]>

---------

Signed-off-by: Megha S <[email protected]>
…#1597)

The .mailmap file maps author/committer names and emails to canonical
real names and email addresses.

This also updates the CONTRIBUTORS.md file with missing names from the
git logs.

This information is to the best of our knowledge, but please submit
any corrections if you prefer.

Signed-off-by: Cary Phillips <[email protected]>
* Release notes for v2.5.10

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

* News for v2.5.10

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

---------

Signed-off-by: Cary Phillips <[email protected]>
Default behavior enable_bzlmod was switch (bazelbuild/bazel#18958)

Bump Bazel version to 7.0.0

Update Bazel modules

Apply buildifier on MODULE.bazel

Signed-off-by: Vertexwahn <[email protected]>
- added example C/C++ Code scanner using the Snyk GitHub Action.
  The `--unmanaged` flag indicates this is for a C/C++ codebase. In this
  example, it currently scans on a new pull request to the 'main'
  branch. The repository administrator should set both the SNYK_ORG and
  SNYK_TOKEN environment variables before merging this PR. The
  environment variables can be obtained from the LFX Security team.
- added *.h, *.c, *.cpp filter to only run the scan when source files
  are changed

Signed-off-by: David Deal <[email protected]>
The #includes were incorrect.

Signed-off-by: Cary Phillips <[email protected]>
* Update MacPorts install instructions

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

* remove sudo

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

* mention vcpkg for Windows

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

---------

Signed-off-by: Cary Phillips <[email protected]>
* Support cmake config and shared library for libdeflate

When not using the internal deflate, the only option was pkgconfig which
is not well supported on Windows. This adds support for using the
libdeflate cmake config files when available.

This change also affects the generated OpenEXR cmake config files. For
a static OpenEXR library, INTERFACE_LINK_LIBRARIES now appropriately
includes libdeflate along with the existing dependencies like Imath.

It also turns the internal libdeflate into a target for consistency.

Resolves AcademySoftwareFoundation#1588

Signed-off-by: Brecht Van Lommel <[email protected]>

* Back out of adding a target for internal libdeflate

Signed-off-by: Brecht Van Lommel <[email protected]>

* Use Requires.private for generated pkgconfig file

Signed-off-by: Brecht Van Lommel <[email protected]>

---------

Signed-off-by: Brecht Van Lommel <[email protected]>
…tion (AcademySoftwareFoundation#1621)

* Add explanation of the distinction between OpenEXR/OpenEXRCore to API section

This was described in the release notes but was never addressed in the
actual online documentation, where it was not, in fact, obvious.

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

* Update website/API.rst

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

---------

Signed-off-by: Cary Phillips <[email protected]>
Co-authored-by: Nick Porcino <[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]>
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]>
Signed-off-by: Cary Phillips <[email protected]>
cary-ilm and others added 3 commits February 12, 2024 15:29
Signed-off-by: Cary Phillips <[email protected]>
…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]>
Copy link
Contributor

@meshula meshula left a comment

Choose a reason for hiding this comment

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

AFAICT the diff matches the label on the tin

@cary-ilm cary-ilm merged commit 17811af into AcademySoftwareFoundation:RB-3.2 Feb 16, 2024
27 checks passed
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.