fix(cmake): Make the cmake config files always pull in the required libraries

[DarkDefender]

Without these changes, we could get link time errors as openimageio doesn't pull in the libraries correctly when using modern cmake targets. It would work when libraries have been installed globally on the system. But I don't think we should rely on the linker automagically finding the libraries in /usr/lib for example.

Especially because we can make this a lot better by warning/erroring out in the cmake setup step when we can't find all required libraries.

I added the libraries that I saw show up in OpenImageIOTargets-release.cmake under the OpenImageIO::OpenImageIO target properties on my system.

Minimal test example

Create a text project with this C++ file called main.cxx:

#include <OpenImageIO/imagebuf.h>

int main() {
  OIIO::ImageBuf image("test.tiff");
  image.write("output.exr");
  return 0;
}

And this CMakeLists.txt file:

cmake_minimum_required(VERSION 3.23)

project(test_oiio_proj)

set(Imath_ROOT ${CMAKE_SOURCE_DIR}/libs/imath)
set(OpenEXR_ROOT ${CMAKE_SOURCE_DIR}/libs/openexr)
set(openjph_ROOT ${CMAKE_SOURCE_DIR}/libs/openjph)
set(TBB_ROOT ${CMAKE_SOURCE_DIR}/libs/tbb)
set(OpenColorIO_ROOT ${CMAKE_SOURCE_DIR}/libs/opencolorio)
set(OpenImageIO_ROOT ${CMAKE_SOURCE_DIR}/libs/openimageio)
find_package(OpenImageIO REQUIRED)

add_executable(test_oiio)

target_sources(test_oiio
  PRIVATE
    main.cxx
)

target_link_libraries(test_oiio OpenImageIO::OpenImageIO)

ENSURE that you do not have OpenColorIO or OpenEXR installed globally on your system.

Make sure that you have git-lfs installed and configured on your system and then clone the precompiled linux libraries from blender into a libs directory next to the other files:
git clone https://projects.blender.org/blender/lib-linux_x64.git libs

Now if you run cmake and then try to build you will get a lot of errors like the ones below at link time:

/usr/x86_64-pc-linux-gnu/binutils-bin/2.46.0/ld: libOpenImageIO.so.3.1.13: undefined reference to `ojph::get_error()'
/usr/x86_64-pc-linux-gnu/binutils-bin/2.46.0/ld: libOpenImageIO.so.3.1.13: undefined reference to `Imf_3_4::Header::dwaCompressionLevel()
/usr/x86_64-pc-linux-gnu/binutils-bin/2.46.0/ld: libOpenImageIO.so.3.1.13: undefined reference to `exr_get_user_data'

Go and edit the libs/openimageio/lib/cmake/OpenImageIO/OpenImageIOConfig.cmake file and add the following on line 42:

find_dependency(OpenColorIO)
find_dependency(OpenEXR)
if (TRUE)
    find_dependency(TBB)
endif ()

Clean out the cmake build files and rerun cmake and try to build again.

Notice that the build finishes without any linker errors.

I don't know if there is any easy way to add tests for this in the test suite, so that is why I left that box unchecked.

Checklist:

• I have read the guidelines on contributions and code review procedures.
• I have read the Policy on AI Coding Assistants
  and if I used AI coding assistants, I have an Assisted-by: TOOL / MODEL
  line in the pull request description above.
• I have updated the documentation if my PR adds features or changes
  behavior.
• I am sure that this PR's changes are tested in the testsuite.
• I have run and passed the testsuite in CI before submitting the
  PR, by pushing the changes to my fork and seeing that the automated CI
  passed there. (Exceptions: If most tests pass and you can't figure out why
  the remaining ones fail, it's ok to submit the PR and ask for help. Or if
  any failures seem entirely unrelated to your change; sometimes things break
  on the GitHub runners.)
• My code follows the prevailing code style of this project and I
  fixed any problems reported by the clang-format CI test.
• If I added or modified a public C++ API call, I have also amended the
  corresponding Python bindings. If altering ImageBufAlgo functions, I also
  exposed the new functionality as oiiotool options.

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: DarkDefender / name: Sebastian Parborg (61368b8)

[lgritz]

I'm a little confused about the circumstances we're talking about.

These libraries are not exposed in the public headers of OIIO, so projects downstream do not need to know where to find the include files, etc.

The theory was that for static library libOpenImageIO.a, static libs don't inherently know about their link needs, so the downstream app using OIIO needs to know about those targets in order to know to link the libraries to the final app.

For dynamic library libOpenImageIO.so, it knows where which libraries it in turn needs, so our belief was that this takes care of it all and the downstream didn't need to know about the targets. So we only did the find_dependency when building static libs.

But you're saying: no, libOpenImageIO.so might know it needs to link against libtbb.so (say), but it doesn't know where to find it if it's not in a standard place. So even for dynamic libOpenImageIO, it would be better for the exported cmake config to establish and use the targets for all the dependencies.

OK, I can buy that. But in that case, I think the solution is to just get rid of the if (NOT @BUILD_SHARED_LIBS@) test itself, rather than to move only some of the dependencies out of that if clause. There's probably no point moving just the couple libraries you have in nonstandard places, but leaving others inside that if. Better to remove the if and do them all unconditionally. Or am I misunderstanding?

[lgritz]

Oh, and it's probably worse than that, all the CI is failing.

How does this happen? Did you not run CI at all prior to submitting the PR?

[DarkDefender]

Oh, and it's probably worse than that, all the CI is failing.

How does this happen? Did you not run CI at all prior to submitting the PR?

Not all of the CI pipeline is failing. Out of all 30 checks 8 is failing. If I read the logs correctly it seems like it fails because it can't find some image files? I looked at the contribution guidelines and there it says that if the majority for the CI tests pass (which I think 22 passing out of 30 is the majority), then it is OK to submit a PR and figure out what is wrong in the PR.

I'm not really sure why this cmake change would make the tests fail they way they are. I would think that there would be at least some cmake output hinting that it can't find the required libraries if that were the case. Do you have any clue what is going on?

But you're saying: no, libOpenImageIO.so might know it needs to link against libtbb.so (say), but it doesn't know where to find it if it's not in a standard place. So even for dynamic libOpenImageIO, it would be better for the exported cmake config to establish and use the targets for all the dependencies.

Some of the libraries have exposed symbols in the libOpenImageIO.so library yes. So you could in theory only link to OpenImageIO but then use openexr functions or definitions directly. Because of this, the linker needs to know where these are defined at link time.

These libraries are not exposed in the public headers of OIIO, so projects downstream do not need to know where to find the include files, etc.

Ah, I see! I thought this was on purpose. I guess then perhaps the correct solution is to not expose these symbols in the OpenImageIO library and only expose OpenImageIO specific APIs?

[DarkDefender]

You can check the exported symbols on linux with nm -D libOpenImageIO.so.

For example you can see that the libOpenImageIO.so does export OpenEXR functions:

 nm -D libOpenImageIO.so | grep exr
                 U exr_attr_get_envmap
                 U exr_decoding_choose_default_routines
                 U exr_decoding_destroy
                 U exr_decoding_initialize
                 U exr_decoding_run
                 U exr_decoding_update
                 U exr_finish
                 U exr_get_attribute_by_index
                 U exr_get_attribute_count
                 U exr_get_channels
                 U exr_get_compression
                 U exr_get_count
                 U exr_get_data_window
                 U exr_get_default_error_message
                 U exr_get_display_window
                 U exr_get_error_code_as_string
                 U exr_get_level_sizes
                 U exr_get_name
                 U exr_get_scanlines_per_chunk
                 U exr_get_storage
                 U exr_get_tile_descriptor
                 U exr_get_tile_levels
                 U exr_get_user_data
                 U exr_read_scanline_chunk_info
                 U exr_read_tile_chunk_info
                 U exr_start_read
                 U exr_test_file_header

There is also opencolorio, imath, and other library symbols in there.

[lgritz]

nm -D libOpenImageIO.so | grep exr
                  U exr_attr_get_envmap
                  U exr_decoding_choose_default_routines

U means "undefined." These are not symbols that the OpenImageIO library is exposing or providing; they are the symbols that the library needs to get dynamically from other libraries at run time. There is not a way to hide that, and there is also not a way that downstream projects can use it.

[lgritz]

If I read the logs correctly it seems like it fails because it can't find some image files?

No, that's just the downstream symptom.

The tests that are failing are cmake-consumer and docs-examples-cpp. These are the two tests that try to build something as if it were a downstream project, and thus consuming the exported cmake config that you altered. (Whereas all the 200+ other tests are just using the OIIO build components directly, within the OIIO build system so to speak, rather than acting like a separate project that doesn't have access to our own build scripts.)

If you download the saved artifacts from those jobs, it also contains the build logs from those secondary builds, and for example cmake-consumer looks like:

-- The CXX compiler identification is GNU 12.3.0
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/lib/ccache/g++-12 - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Building consumer  - Release
CMake Error at /usr/local/share/cmake-3.31/Modules/CMakeFindDependencyMacro.cmake:76 (find_package):
  By not providing "FindOpenColorIO.cmake" in CMAKE_MODULE_PATH this project
  has asked CMake to find a package configuration file provided by
  "OpenColorIO", but CMake did not find one.

  Could not find a package configuration file provided by "OpenColorIO" with
  any of the following names:

    OpenColorIOConfig.cmake
    opencolorio-config.cmake

  Add the installation prefix of "OpenColorIO" to CMAKE_PREFIX_PATH or set
  "OpenColorIO_DIR" to a directory containing one of the above files.  If
  "OpenColorIO" provides a separate development package or SDK, be sure it
  has been installed.
Call Stack (most recent call first):
  /home/runner/work/OpenImageIO/OpenImageIO/dist/lib/cmake/OpenImageIO/OpenImageIOConfig.cmake:43 (find_dependency)
  CMakeLists.txt:22 (find_package)

So we can make sure the export contains find_dependency(OpenColorIO), but if the downstream process doesn't know where to find OpenColorIO's cmake exports, it will be a build error for the downstream project, even though no OpenImageIO .h files contain any references to OpenColorIO's API.

Some of the libraries have exposed symbols in the libOpenImageIO.so library yes.

I believe they do not. They only have recorded OpenImageIO's need for those symbols to be resolved. They are not exported symbols.

So you could in theory only link to OpenImageIO but then use openexr functions or definitions directly.
...
Ah, I see! I thought this was on purpose. I guess then perhaps the correct solution is to not expose these symbols in the OpenImageIO library and only expose OpenImageIO specific APIs?

We already do that.

I think you cannot use that from downstream apps, because there are no function declarations for those OpenEXR functions anywhere in OIIO's header files, and OpenImageIO library doesn't provide the implementations.

But the fact that a downstream project can't use those definitions directly is not an important detail here, because it's still true that libOpenImageIO needs those symbols to be resolved at runtime so that IT can call the functions internally. It needs to be found at load time in order to execute.

The libOpenImageIO.so library itself knows it needs libOpenColorIO.so (whereas libOpenImageIO.a, as a static library, would not), but it still assumes that there are enough clues at runtime to FIND that library.

This is the function of the LD_LIBRARY_PATH environment variable.

I believe that is the key here. I think that changing the cmake exports is not really going to fully solve the problem (and may create new ones). I think the real solution is that if you are going to store required libraries in a nonstandard place, you should expect to need to put those nonstandard locations in your LD_LIBRARY_PATH.