CMake and ext package upgrades

[michdolan]

A variety of updates to ext package versions and CMake find modules:

• Upgrade to CMake 3.12
• Favor third-party CMake configs for finding packages
• Adopt standard CMake <package>_ROOT variables for overriding package locations
• Upgrade Half version to match VFX Reference Platform CY2020
• Upgrade other packages to take advantage of upstream CMake upgrades and configs
• Match find module variable namespace to target name, and target names to third-party CMake configs
• Move ext find package calls to project root to guarantee imported targets are global

Signed-off-by: Michael Dolan [email protected]

[hodoulp]

@michdolan Thanks for modernizing & improving the external library integration.

But, could you explain the rational of the first two bullets (i.e. Upgrade to CMake 3.12 &
Favor third-party CMake configs) ?

[michdolan]

As for the rationale for preferring third-party CMake configs:

Most of our upstream dependencies provide CMake configs with their installs (in the new versions I proposed), which allows us to greatly reduce dependence on our CMake find modules for knowing where to find packages or how to check versions. Eventually, we can rely on the configs alone, and have no custom logic. The only thing stopping us from doing that right now is the handling of OCIO_INSTALL_EXT_PACKAGES, which complicates things a bit. We should also be shipping our own CMake config with OCIO for other projects to use for finding OCIO. We could eventually pull our package install logic out of the find modules, and rely on CMake configs when no install is needed.

[lgritz]

This all looks good from my point of view. For a while now, I've had my projects using CMake 3.12 as a minimum for the same reasons, I export cmake config files, and am slowly retiring my own FindFoo.cmake modules as my dependencies reliably have their own exported config files.

I still have a couple places where I do automatic downloads, but enough people/sites have trouble with that to cause me to slowly remove them one by one, making my builds assume that all dependencies are present, but supplying scripts that download/build/install any of the dependencies that are not extremely common in all the distros. So I've come to think the best policy is to leave it up to the users to install the dependencies, but I give them tools to at least make it a single easy command when they need to do it.

[hodoulp]

By checking what is done in OpenImageIO, I agree that having a findXX from third-party libraries is extremely useful to simplify any cmake integration. By default, it could provide the version, and a standard way to search for the library. However, Windows platform support is still weak for the search part. So, the OCIO_INSTALL_EXT_PACKAGES is then essential for Windows platform support.

On the other hand, the automatic OpenColorIO library build is also mandatory (whatever is the platform) for most of our devs/users i.e. search for third-party libraries and if not found, download & compile, to guaranty a successful build without worrying about dependencies.

With the current work, the only missing piece is now the generated findOpenColorIO() and perhaps adjustments between findXX() vs. OCIO_INSTALL_EXT_PACKAGES.

Other aspects about using installed third-party libraries is the low control on the version, the dynamic vs. static library, and sometime the namespace. From the various integrations I did, I now always strongly recommend to generate & link with static libraries for all mandatory third-party dependencies, and hides private symbols (including third-party library ones). When integrating a library in a larger program (Maya, Flame, etc) there is a great chance that some third-party libraries be already used by the app or by another third-party dependency (out of your control) and that versions be different. The proposed approach avoids symbol conflicts at link time, weird crashes on client site at runtime.
So OCIO_INSTALL_EXT_PACKAGES is critical to 1) allow the bypass of installed third-party libraries, 2) build static libraries for dependencies and 3) optionally pass some extra compilation and/or link flags.

[hodoulp]

When looking at the CI builds, I notice that:

• [Question] Could NOT find OpenImageIO (missing: OpenImageIO_DIR), should that be OpenImageIO_ROOT ?
• [Minor] (found suitable version "3.7.7", minimum required is "2.7") when detecting Python 3 the required version must be 3.x

[hodoulp]

When some third-party libraries are missing, the code could provide a additional one-line explanation on how to add the support:

• Could NOT find GLEW (missing: GLEW_INCLUDE_DIRS GLEW_LIBRARIES)
  or Could NOT find GLUT (missing: GLUT_glut_LIBRARY)
  Keep the warning with an additional line explaining to use package_ROOT.

[hodoulp]

[Warning already present i.e. not from this PR]
On Windows platform, yaml-cpp and pystring have the following warning:

CMake Warning at share/cmake/modules/Findyaml-cpp.cmake:180 (message):
  Building STATIC libOpenColorIO using the in-built yaml-cpp.  yaml-cpp
  symbols are NOT included in the output binary!
Call Stack (most recent call first):
  share/cmake/modules/FindExtPackages.cmake:15 (find_package)
  CMakeLists.txt:216 (include)

The change could be part of another PR.

[scoopxyz]

Hey, was just about to put in a PR around this. It may be isolated to my environment, however find_package(PythonInterp 2.7 QUIET) wasn't able to find my local Python3 interpreter or libraries. Swapping this with the now recommended (>=3.12) find_package(Python COMPONENTS Interpreter) however did work.

This would have to be updated in the python bindings CMakeLists.txt as well:
and find_package(Python COMPONENTS Interpreter Development)

I'll make an updated PR to your branch.
michdolan#3

[hodoulp]

@michdolan I have the same problem on macos with Python 2.7.16 & cmake 3.16.5.
@scoopxyz Your fix (in src/bindings/python/CMakeLists.txt) solves the problem on my machine.

[michdolan]

I will address the remaining comments once I merge Sean's PR into my branch.

[hodoulp]

[Suggestions]

• The search of the installed python is done several times, the code could centralize it, and enable/disable some builds i.e. docs, bindings, etc. For example, it could be moved in the root CMakeLists.txt, and if Python is not found, disable docs, Python bindings & Python unit tests with a warning message.
• As the VFX Reference platform now requests Python 3, the code could check for version 3 (use of FindPython3 instead of FindPython) and if not found, fall back to Python 2 with a warning.

[linux-foundation-easycla]

The committers are authorized under a signed CLA.

• ✅ Michael Dolan (0a8b86b, 5a99141, f52d975, 21f8bf0)
• ✅ Sean Cooper (78ca2d6, a08ab68, 28c7549)

[michdolan]

[Question] Could NOT find OpenImageIO (missing: OpenImageIO_DIR), should that be OpenImageIO_ROOT ?

CMake supports both. <package>_ROOT is searched first, and later <package>_DIR. I'm not sure why the CMake message calls out the later, but it's coming from find_package internals.

[Minor] (found suitable version "3.7.7", minimum required is "2.7") when detecting Python 3 the required version must be 3.x

I agree that 3.x is preferred, but technically 2.7 will still work too. If I bump the minimum we will be dropping support for Python 2 bindings.

[Warning already present i.e. not from this PR]
On Windows platform, yaml-cpp and pystring have the following warning:

These warnings are coming from our find modules. Is your suggestion to address the TODO comments about finding a way to merge in the static libs? On that point, do we want the yaml-cpp or pystring symbols to be exposed?

When some third-party libraries are missing, the code could provide a additional one-line explanation on how to add the support

I added a new macro to print a standard message about using <package>_ROOT when any non-REQUIRED package is not found. For REQUIRED packages, a fatal error is raised before I have the chance to add an additional line. We could customize the fail message for REQUIRED packages to address this, but CMake's documentation recommends against it.

@hodoulp on a somewhat related point, should oiiohelpers depend on OpenGL and GLEW? OCIO CMake configuration will currently fail if OpenGL or GLEW are not found and OCIO_BUILD_APPS=ON, due to the REQUIRED find_package calls in src/libutils/oiiohelpers/CMakeLists.txt. It feels like this should not fail, since some apps could still be built.

[michdolan]

@scoopxyz thanks for resolving the CLA issue. The DCO is now failing as it thinks your sign-off name should be your github handle. See: https://github.com/AcademySoftwareFoundation/OpenColorIO/pull/1016/checks?check_run_id=815283666

[hodoulp]

@hodoulp on a somewhat related point, should oiiohelpers depend on OpenGL and GLEW?

No, That's a false dependency. To be removed.