Adsk contrib - Add support for minimum and recommended versions for dependencies#1777
Merged
Conversation
Implemention of minimum, maximum and recommended version. Refactor the FindOpenImageIO to be inline with the rest of the custom find module (OCIO_USE_OIIO_CMAKE_CONFIG is no longer needed). Added a new option called OCIO_VERBOSE. It allow the user to tell OCIO to display more information when searching and building the dependencies. Splitted the find and install part into two files - Find<pkg>.cmake and Install<pkg>.cmake. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
Setting policy CMP0042 when building ZLIB since that project is using an old CMake version as the cmake_minimum_required and that version has no knowledge of the policy. Fixing a potential issue in Installopenfx. Changing back the version to 1.4 as before. Ignoring warning from OpenImageIO for imageioapphelpers. The same warning are ignored for ociodisplay, ocioconvert and ociolutimage. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
…acOS. Renamed FindOpenShadingLanguage to FindOSL to match the project name used the OSL's CMakefile. Cleanup the code for FindOSL and removed duplicate warning about needed C++14. Updated the header comments for all Find and Install modules. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
Renamed ocio_install_package to ocio_install_dependency The "Installing [...]" message from Install module is now under OCIO_VERBOSE variable. Added a message in ocio_install_dependency instead Improve the colors usage in the logging Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
…ith the prefix of ocio_handle_dependency since they were the same. Removed FindOpenEXR.cmake and FindOpenImageIO.cmake since they are not needed anymore (for differente reason). Added PROMOTE_TARGET option for ocio_handle_dependency which promote the target to GLOBAL. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
Re-worded some of the comments Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
…ting OCIO_INSTALL_EXT_PACKAGES option correctly. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
…ib since the user can update their cmake in order to do that. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
…rsions-for-dependencies' into adsk_contrib/add-support-for-min-recommended-versions Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
cedrik-fuoco-adsk
changed the title
cedrik-fuoco-adsk
changed the title
Collaborator
|
Regarding our usual 2-week clock on these Autodesk PRs, this PR is identical to PR #1772 which had already been open for two weeks. So it is our intention to merge this one as soon as it is approved. Other work is gated by this, thanks. |
Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
Collaborator
remia
approved these changes
Mar 22, 2023
Collaborator
remia
left a comment
remia
left a comment
There was a problem hiding this comment.
Looks good to me, thanks for taking care of the rebase @cedrik-fuoco-adsk. Just a question about zlib, but that should not prevent merge and can be discussed in #1771. Great work!
| set(CMAKE_FIND_LIBRARY_SUFFIXES) | ||
| endif() | ||
| ocio_handle_dependency( ZLIB REQUIRED ALLOW_INSTALL | ||
| MIN_VERSION 1.2.10 |
Collaborator
There was a problem hiding this comment.
[Question] Is the 1.2.10 version the minimum compatible, how did we picked it?
Collaborator
There was a problem hiding this comment.
We basically just picked what seemed like a sufficiently old version, and did not invest the time to see what is actually the oldest that would work. If someone has a need for an older version, please post the requirement in issue #1771.
I will go ahead and merge this now since it is blocking another task. Thank you Remi!
doug-walker
approved these changes
Mar 22, 2023
doug-walker
deleted the
adsk_contrib/add-support-for-min-recommended-versions
branch
cedrik-fuoco-adsk
added a commit
to autodesk-forks/OpenColorIO
that referenced
this pull request
Mar 24, 2023
…ependencies (AcademySoftwareFoundation#1777) * Implementation of ocio_find_package and ocio_install_package macro. Implemention of minimum, maximum and recommended version. Refactor the FindOpenImageIO to be inline with the rest of the custom find module (OCIO_USE_OIIO_CMAKE_CONFIG is no longer needed). Added a new option called OCIO_VERBOSE. It allow the user to tell OCIO to display more information when searching and building the dependencies. Splitted the find and install part into two files - Find<pkg>.cmake and Install<pkg>.cmake. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com> * Removing a duplicate call to set_property in OpenImageIO find module. Setting policy CMP0042 when building ZLIB since that project is using an old CMake version as the cmake_minimum_required and that version has no knowledge of the policy. Fixing a potential issue in Installopenfx. Changing back the version to 1.4 as before. Ignoring warning from OpenImageIO for imageioapphelpers. The same warning are ignored for ociodisplay, ocioconvert and ociolutimage. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com> * Fixed an issue with FindOpenImageIO that was found while testing on macOS. Renamed FindOpenShadingLanguage to FindOSL to match the project name used the OSL's CMakefile. Cleanup the code for FindOSL and removed duplicate warning about needed C++14. Updated the header comments for all Find and Install modules. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com> * Fixing typo Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com> * Renamed ocio_find_package to ocio_handle_dependency Renamed ocio_install_package to ocio_install_dependency The "Installing [...]" message from Install module is now under OCIO_VERBOSE variable. Added a message in ocio_install_dependency instead Improve the colors usage in the logging Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com> * Changed prefix for ocio_install_dependency since it was conflicting with the prefix of ocio_handle_dependency since they were the same. Removed FindOpenEXR.cmake and FindOpenImageIO.cmake since they are not needed anymore (for differente reason). Added PROMOTE_TARGET option for ocio_handle_dependency which promote the target to GLOBAL. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com> * Mostly comments and documentations Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com> * Small update of the Existing Install Hints section Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com> * Changed RECOMMENDED_MIN_VERSION to RECOMMENDED_VERSION Re-worded some of the comments Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com> * Fixing issue with ocio_handle_dependency macro where it wasn't respecting OCIO_INSTALL_EXT_PACKAGES option correctly. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com> * Adding more documentations and dropping support to look for static zlib since the user can update their cmake in order to do that. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com> * Documentations Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com> * Documentations Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com> * fixing typo Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com> * Fix typo and fix issue when OCIO is installing ZLIB Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com> * Tentative fix for Linux CI failure Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com> * Ignoring specifics warnings on OpenImageIO target directly. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com> * Removing OCIO_USE_OIIO_CMAKE_CONFIG as it is not needed anymore. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com> --------- Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Something went wrong in the previous PR while trying to rebase the branch.
I created a fresh branch from main and merged my changes, which went much smoother.
Adding support for minimum, maximum and recommended versions for OCIO dependencies.
ocio_handle_dependencyandocio_install_dependency.share/cmake/modules/FindXYZ.cmakeandshare/cmake/modules/install/InstallXYZ.cmakeHere's a snippet of a build log: