Add uninstall target #1624
Merged
Add uninstall target #1624
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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]>
Signed-off-by: Cary Phillips <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>
meshula
approved these changes
Feb 2, 2024
lgritz
reviewed
Feb 2, 2024
cmake/LibraryDefine.cmake
Outdated
| @@ -93,7 +93,8 @@ function(OPENEXR_DEFINE_LIBRARY libname) | |||
| string(TOUPPER "${CMAKE_BUILD_TYPE}" uppercase_CMAKE_BUILD_TYPE) | |||
| set(verlibname ${CMAKE_SHARED_LIBRARY_PREFIX}${libname}${OPENEXR_LIB_SUFFIX}${CMAKE_${uppercase_CMAKE_BUILD_TYPE}_POSTFIX}${CMAKE_SHARED_LIBRARY_SUFFIX}) | |||
| set(baselibname ${CMAKE_SHARED_LIBRARY_PREFIX}${libname}${CMAKE_${uppercase_CMAKE_BUILD_TYPE}_POSTFIX}${CMAKE_SHARED_LIBRARY_SUFFIX}) | |||
| install(CODE "execute_process(COMMAND ${CMAKE_COMMAND} -E chdir \"\$ENV\{DESTDIR\}${CMAKE_INSTALL_FULL_LIBDIR}\" ${CMAKE_COMMAND} -E create_symlink ${verlibname} ${baselibname})") | |||
| file(CREATE_LINK ${verlibname} ${baselibname} SYMBOLIC) | |||
There was a problem hiding this comment.
Does this work right on Windows? I recall that some link thinks don't work quite the same way as you'd expect from Unix.
There was a problem hiding this comment.
Could someone with a Window please confirm the symlinks are still valid?
There was a problem hiding this comment.
Based on https://cmake.org/cmake/help/latest/command/file.html#create-link, we could pass COPY_ON_ERROR so that cmake will copy instead of symlinking if it fails?
There was a problem hiding this comment.
Regarding Windows, the entire logic here is inside if (... AND NOT WIN32), so hopefully it's not an issue. And hopefully cmake -E create_simlink has a similar effect to file(CREATE_LINK ... SYMBOLIC).
I've added COPY_ON_ERROR as a failsafe anyway.
lgritz
reviewed
Feb 2, 2024
website/install.rst
Outdated
Comment on lines
280
to
295
| Uninstall | ||
| ~~~~~~~~~ | ||
|
|
||
| If you have installed from source, you can uninstall via: | ||
|
|
||
| .. code-block:: | ||
|
|
||
| % cmake --build $builddir --target uninstall | ||
|
|
||
| or if using ``make``: | ||
|
|
||
| .. code-block:: | ||
|
|
||
| % make uninstall | ||
|
|
||
|
|
There was a problem hiding this comment.
This implies that if you still have the exact working source and build tree that built your installation, then running this would remove the installed components.
But if you don't have source, or a build tree, or a build tree that exactly matches the commit and build-time options of your installed OpenEXR, this might be unreliable or entirely unavailable. It's not at all like having an "openexr-uninstall" script installed with the rest of the binary components.
I'm not sure if this satisfies the general requirement to have an uninstall command. Is that sufficient from an OpenSSF standpoint? Do they mean "the person who built it can reverse an install if they catch it before they blow away their build tree?" Or do they mean "any user with permissions to alter the install area can reliably remove all installed components at any time?"
There was a problem hiding this comment.
@jmertic mentioned on slack a while back that:
For source builds, just a note saying "files are created at x, y, z that would need to be removed for uninstall" is sufficent
So the uninstall target might not be absolutely necessary to satisfy the badge requirements. But I do see this construct adopted by other respectable projects like pybind11 and c-blosc and libjpSo the uninstall target might not be absolutely necessary to satisfy the badge requirements. But I do see this construct adopted by other projects like pybind11 and c-blosc and libjpeg-turbo
eg-turbo, so it seems reasonable to follow that convention.
I can also add a mention to the obvious - if you installed via a package manager, uninstall with the package manager. This applies only if you've built from source (and still have the configuration files).
There was a problem hiding this comment.
I think it has value in the sense that if a contributor accidentally installs into its root filesystem, at least there is an uninstall target to undo the damage. When I started compiling stuff a couple of years ago, I did this mistake quite often and make uninstall or equivalents (when available) saved me a lot of trouble on multiple occasions.
There was a problem hiding this comment.
I added a bit more clarifying wording.
Signed-off-by: Cary Phillips <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>
cary-ilm
added a commit
to cary-ilm/openexr
that referenced
this pull request
Feb 9, 2024
* 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]>
cary-ilm
added a commit
to cary-ilm/openexr
that referenced
this pull request
Feb 10, 2024
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]>
cary-ilm
added a commit
that referenced
this pull request
Feb 10, 2024
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]>
cary-ilm
added a commit
to cary-ilm/openexr
that referenced
this pull request
Feb 11, 2024
* 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]>
cary-ilm
added a commit
to cary-ilm/openexr
that referenced
this pull request
Feb 11, 2024
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]>
cary-ilm
added a commit
that referenced
this pull request
Feb 13, 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
Feb 13, 2024
* 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]>
cary-ilm
added a commit
to cary-ilm/openexr
that referenced
this pull request
Feb 13, 2024
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]>
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
* 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]>
cary-ilm
added a commit
that referenced
this pull request
Feb 16, 2024
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]>
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
Feb 22, 2024
This should have gone in to AcademySoftwareFoundation#1624 since `file(CREATE_LINK)` requires CMake 3.14. Signed-off-by: Cary Phillips <[email protected]>
cary-ilm
added a commit
that referenced
this pull request
Feb 29, 2024
This should have gone in to #1624 since `file(CREATE_LINK)` requires CMake 3.14. Signed-off-by: Cary Phillips <[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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.hwas passed toinstall()twice, producing two entries ininstall_manifest.txt.