cmake: add missing SPIRV-Tools-opt dependency

[abouvier]

The generated glslang-targets.cmake file (for static glslang targets) contains this:

set_target_properties(glslang::SPIRV PROPERTIES
  INTERFACE_INCLUDE_DIRECTORIES "${_IMPORT_PREFIX}/include"
  INTERFACE_LINK_LIBRARIES "\$<LINK_ONLY:glslang::MachineIndependent>;\$<LINK_ONLY:SPIRV-Tools-opt>"
)

The SPIRV-Tools-opt here is as CMake target, but also the name of a real library file (libSPIRV-Tools-opt.so or libSPIRV-Tools-opt.a) which will be linked with (missing its dependency SPIRV-Tools) if the target is nonexistent. The find_dependency will import the target automatically.

REQUIRED is not needed in find_dependency, otherwise find_package(glslang) without REQUIRED will produce a fatal error if Threads is not found.

[CLAassistant]

All committers have signed the CLA.

[d235j]

I'm seeing this issue with both shared and static targets, so the patch is incomplete for this use case.

[d235j]

Removing AND NOT @BUILD_SHARED_LIBS@ fixes it for me.

[abouvier]

glslang compiled as a shared library generates this in glslang-targets.cmake:

# Create imported target glslang::SPIRV
add_library(glslang::SPIRV SHARED IMPORTED)

set_target_properties(glslang::SPIRV PROPERTIES
  INTERFACE_INCLUDE_DIRECTORIES "${_IMPORT_PREFIX}/include"
)

SPIRV-Tools-opt is not needed for the linking phase, only for the execution of the program using the shared glslang.

[d235j]

The target_link_libraries here (https://github.com/KhronosGroup/glslang/blob/main/SPIRV/CMakeLists.txt#L109) causes the linker directive -lSPIRV-Tools-opt to be generated regardless of shared or static, which causes link of the program to fail. Additionally, on macOS the binary must contain the dynamically linked libraries in its library paths which are viewable with otool -L, and if the symbols are not available at link time then link will fail.

Typically, a shared library is needed at linking time; an external module which is dlopen()ed by the calling library would not be, but that is not how SPIRV-Tools-opt is being used.

[d235j]

It looks like you are right; however, in this case the target_link_libraries referenced above will need to be changed to link it as PRIVATE instead of PUBLIC to prevent the -config.cmake from trying to link it anyway (which causes -lSPIRV-Tools-opt to be appended to the linker flags, which will blow up if the library is not in the path).

Additionally, what happens in this case if SPIRV-Tools-opt is not available at runtime?

[abouvier]

SPIRV-Tools-opt is indeed needed when linking glslang itself, but not when linking a program using glslang.

If a shared library is missing at runtime, your program will not start.

[d235j]

SPIRV-Tools-opt is indeed needed when linking glslang itself, but not when linking a program using glslang.

If a shared library is missing at runtime, your program will not start.

Makes sense... in that case, the target_link_libraries does need to be changed to PRIVATE for SPIRV-Tools-opt at https://github.com/KhronosGroup/glslang/blob/main/SPIRV/CMakeLists.txt#L109 — you can just drop the word PUBLIC from that entirely, or perhaps adding a conditional of some sort so this is only triggered for the static library?

Having PUBLIC there tells CMake that SPIRV-Tools-opt is needed when linking a program using glslang, so the generated glslang-targets.cmake still contains:

  INTERFACE_LINK_LIBRARIES "SPIRV-Tools-opt"

which causes CMake to place -lSPIRV-Tools-opt in the linker flags. And then if libSPIRV-Tools-Opt.dylib does not exist in the linker path, the program will not link.

[abouvier]

The reason for the public linking is explained here.

According to this find_dependency should be used unconditionally for everything.

But after analyzing some repos (like those of KDE) I guess it should be used for all public/private dependencies when building a static library, but only for public dependencies when building a shared library.

[arcady-lunarg]

Is it safe to remove this line?

[abouvier]

It's not removed, it's moved to the top of the file.