Add unified `glslang` CMake config collecting `glslang-targets` targets #2989

theblackunknown · 2022-07-30T22:18:00Z

This PR introduce a new and unified glslang CMake config package, all glslang targets are exposed through glslang:: CMake namespace.

Prior to this PR there were several exported CMake target under various name, no package.
With those changes, and when set properly, any CMake project should be able to consume glslang by simply calling find_package(glslang CONFIG) and then pick whatever targets they need (e.g. glslang::glslang, glslang::SPIRV, etc.)

Note that as I am not really sure which existing projects uses existing exported CMake targets via direct includes, I still provide a layer of backward compatibility and issue a warning.

I see that there are a number of pending PR/issues already (#1978, #2751, #2570, #2778 and maybe more ?) but they seems to be on hold as I do not see any progress over last months so here is my attempt to fix the issue.

CLAassistant · 2022-07-30T22:18:04Z

All committers have signed the CLA.

theblackunknown · 2022-07-30T22:21:19Z

Last but not least, it is that the current CMake setup to install CMake targets does not work anymore with recent CMake version (e.g. 3.23)
CMake now verifies that when we install a target in an export set (i.e. install(TARGETS foo EXPORT export_set ...) we also include the target dependencies in it, which is not the case currently.

Here is a configure output I get when using CMake 3.23.0 on Windows with VS generator

C:\devel-personal\glslang\build>cmake ..
-- Building for: Visual Studio 17 2022
-- Selecting Windows SDK version 10.0.19041.0 to target Windows 10.0.22000.
-- The CXX compiler identification is MSVC 19.32.31332.0
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: C:/Program Files/Microsoft Visual Studio/2022/Community/VC/Tools/MSVC/14.32.31326/bin/Hostx64/x64/cl.exe - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- No build type selected, default to Debug
-- Using Debug VC++ CRT: MDd
-- Using Release VC++ CRT: MD
-- Using MinSizeRel VC++ CRT: MD
-- Using RelWithDebInfo VC++ CRT: MD
-- Using Debug VC++ CRT: MDd
-- Found PythonInterp: C:/Users/[redacted]/AppData/Local/Programs/Python/Python310/python.exe (found suitable version "3.10.2", minimum required is "3")
-- Google Mock was not found - tests based on that will not build
-- spirv-tools not linked - illegal SPIRV may be generated for HLSL
-- Configuring done
CMake Error: install(EXPORT "glslangTargets" ...) includes target "glslang" which requires target "OGLCompiler" that is not in this export set, but in multiple other export sets: lib/cmake/OGLCompilerTargets.cmake, share/glslang/glslang-targets.cmake.
An exported target cannot depend upon another target which is exported multiple times. Consider consolidating the exports of the "OGLCompiler" target to a single export.
CMake Error: install(EXPORT "glslangTargets" ...) includes target "glslang" which requires target "OSDependent" that is not in this export set, but in multiple other export sets: lib/cmake/OSDependentTargets.cmake, share/glslang/glslang-targets.cmake.
An exported target cannot depend upon another target which is exported multiple times. Consider consolidating the exports of the "OSDependent" target to a single export.
CMake Error: install(EXPORT "glslangTargets" ...) includes target "MachineIndependent" which requires target "OGLCompiler" that is not in this export set, but in multiple other export sets: lib/cmake/OGLCompilerTargets.cmake, share/glslang/glslang-targets.cmake.
An exported target cannot depend upon another target which is exported multiple times. Consider consolidating the exports of the "OGLCompiler" target to a single export.
CMake Error: install(EXPORT "glslangTargets" ...) includes target "MachineIndependent" which requires target "OSDependent" that is not in this export set, but in multiple other export sets: lib/cmake/OSDependentTargets.cmake, share/glslang/glslang-targets.cmake.
An exported target cannot depend upon another target which is exported multiple times. Consider consolidating the exports of the "OSDependent" target to a single export.
CMake Error: install(EXPORT "SPIRVTargets" ...) includes target "SPIRV" which requires target "MachineIndependent" that is not in this export set, but in multiple other export sets: share/glslang/glslang-targets.cmake, lib/cmake/glslangTargets.cmake.
An exported target cannot depend upon another target which is exported multiple times. Consider consolidating the exports of the "MachineIndependent" target to a single export.
-- Generating done
CMake Generate step failed.  Build files cannot be regenerated correctly.

ncesario-lunarg

This generally LGTM. I tried this out in the Vulkan-ValidationLayers repo without changing any cmake, and also tried using these newly added targets (ncesario-lunarg/Vulkan-ValidationLayers@63e14e4), and both the old and new methods worked fine.

FYI @lunarpapillo. I don't think this will have any effect on SDK packaging, but if the SDK uses the installation artifacts of glslang, we'll probably want to include these new *.cmake files.

The last time I did this was a few years ago, and I was kind of hoping it would be more streamlined, but cmake is cmake :)

ncesario-lunarg · 2022-08-02T21:22:23Z

Last but not least, it is that the current CMake setup to install CMake targets does not work anymore with recent CMake version (e.g. 3.23)
CMake now verifies that when we install a target in an export set (i.e. install(TARGETS foo EXPORT export_set ...) we also include the target dependencies in it, which is not the case currently.

@theblackunknown are you saying that an error should be expected with the current cmake on cmake versions >= 3.23.0? If so, I'm not seeing that.

theblackunknown · 2022-08-02T21:37:54Z

the SDK uses the installation artifacts of glslang, we'll probably want to include these new *.cmake files.

That is exactly my hope with this PR :)

theblackunknown · 2022-08-02T21:39:14Z

Last but not least, it is that the current CMake setup to install CMake targets does not work anymore with recent CMake version (e.g. 3.23)
CMake now verifies that when we install a target in an export set (i.e. install(TARGETS foo EXPORT export_set ...) we also include the target dependencies in it, which is not the case currently.

@theblackunknown are you saying that an error should be expected with the current cmake on cmake versions >= 3.23.0? If so, I'm not seeing that.

Good to know ! I just randomly run into this while developing this PR, I can try again to understand the cause but given your test this is likely specific to my dev setup.
In any case, not a blocker for this PR.

EDIT Just to double-check, I experienced an error with the current repo HEAD, not with this PR.

ncesario-lunarg · 2022-08-02T23:34:36Z

I experienced an error with the current repo HEAD, not with this PR.

Right, I'm using cmake 3.23.2 (Windows), and not getting any error when building glslang on ToT master.

greg-lunarg · 2022-08-03T00:15:48Z

@theblackunknown Thanks for the code!
@ncesario-lunarg Thanks for the review!

Tachi107 · 2022-08-25T14:39:57Z

CMakeLists.txt

+            "${CMAKE_CURRENT_BINARY_DIR}/glslang-config.cmake"
+            "${CMAKE_CURRENT_BINARY_DIR}/glslang-config-version.cmake"
+        DESTINATION
+            "${CMAKE_INSTALL_DATADIR}/${PROJECT_NAME}"


Why is this installed to CMAKE_INSTALL_DATADIR? glslang ships architecture-dependent libraries, right?

Also, write_basic_package_version_file() produces an architecture-dependent output unless called with the ARCH_INDEPENDENT option,

Why is this installed to CMAKE_INSTALL_DATADIR? glslang ships architecture-dependent libraries, right?

Good remark, I used this path out of habit using vcpkg a lot.
I know there are other paths but I do not have experience using them, do you have any suggestions written in cmake portable way ?
Also I am curious on which issues you ran into issues using this arch independent path ?

write_basic_package_version_file() produces an architecture-dependent output unless called with the ARCH_INDEPENDENT option,

Kind of echoing the previous comment, I do not have experience on how to correctly write this in a portable cmake way, can you suggest a good alternative here ?
I know that there are platform dependent libraries in glslang, but as we compiled binaries I guess we will always ends up with arch dependent libraries isn't it ?

The issue is that on Linux and most Unix-like platforms, share/ (datadir) is used to store architecture-independent data, while lib/ (libdir) is used to store architecture-specific files like compiled libraries.

Given this conventions, some distributions like Debian extend this concept to improve co-installability of different architecture variants of the same library (Debian and Ubuntu call this MultiArch), to make things like cross-compilation way easier.

In our case, as glslang-config.cmake is in share/, the library would be considered architecture independent and would be used when cross compiling to any architecture, while in fact glslang-config.cmake points to libraries only usable on the build machine's architecture, breaking the whole process.

Now, in practise I don't know if this is done by CMake, what I described above is what pkg-config does (if I recall correctly), but in any case installing arch-dependent stuff in share/ is always wrong and can lead to issues. Installing arch-independent stuff to lib/ is less severe, but that's not what we have here.

Hope I made this clear enough :)

Would it suffice to use CMAKE_INSTALL_LIBDIR?

Yeah I think it would

Edit: I mean, not only that, but also all the related ones. I can submit a PR if you prefer

Please submit a PR. I'll happily review it. Thanks!

Opened #3009

As glslang ships architecture dependant files, the Config file should be installed to libdir, not datadir. See KhronosGroup#2989 (comment) for more details. Here's the diff between the install tree before and after this patch: $ diff <(tree install-datadir) <(tree install) 1c1 < install-datadir --- > install 74,99c74,98 < ├── lib < │ ├── cmake < │ │ ├── glslang-default-resource-limitsTargets.cmake < │ │ ├── glslangTargets.cmake < │ │ ├── glslangValidatorTargets.cmake < │ │ ├── HLSLTargets.cmake < │ │ ├── OGLCompilerTargets.cmake < │ │ ├── OSDependentTargets.cmake < │ │ ├── spirv-remapTargets.cmake < │ │ ├── SPIRVTargets.cmake < │ │ └── SPVRemapperTargets.cmake < │ ├── libGenericCodeGen.a < │ ├── libglslang.a < │ ├── libglslang-default-resource-limits.a < │ ├── libHLSL.a < │ ├── libMachineIndependent.a < │ ├── libOGLCompiler.a < │ ├── libOSDependent.a < │ ├── libSPIRV.a < │ └── libSPVRemapper.a < └── share < └── glslang < ├── glslang-config.cmake < ├── glslang-config-version.cmake < ├── glslang-targets.cmake < └── glslang-targets-debug.cmake --- > └── lib > ├── cmake > │ ├── glslang-default-resource-limitsTargets.cmake > │ ├── glslangTargets.cmake > │ ├── glslangValidatorTargets.cmake > │ ├── HLSLTargets.cmake > │ ├── OGLCompilerTargets.cmake > │ ├── OSDependentTargets.cmake > │ ├── spirv-remapTargets.cmake > │ ├── SPIRVTargets.cmake > │ └── SPVRemapperTargets.cmake > ├── glslang > │ ├── glslang-config.cmake > │ ├── glslang-config-version.cmake > │ ├── glslang-targets.cmake > │ └── glslang-targets-debug.cmake > ├── libGenericCodeGen.a > ├── libglslang.a > ├── libglslang-default-resource-limits.a > ├── libHLSL.a > ├── libMachineIndependent.a > ├── libOGLCompiler.a > ├── libOSDependent.a > ├── libSPIRV.a > └── libSPVRemapper.a 101c100 < 15 directories, 83 files --- > 14 directories, 83 files

Also enforce target names in CMakeDeps. Since KhronosGroup/glslang#2989, glslang provides glslang-config.cmake file with CMake targets under glslang:: namespace.

* partial conan v2 support Also enforce target names in CMakeDeps. Since KhronosGroup/glslang#2989, glslang provides glslang-config.cmake file with CMake targets under glslang:: namespace. * add glslang/1.3.224.0

sl1pkn07 · 2022-11-17T20:25:06Z

all my reviews fixed with #3063

As glslang ships architecture dependant files, the Config file should be installed to libdir, not datadir. See KhronosGroup#2989 (comment) for more details. Here's the diff between the install tree before and after this patch: $ diff <(tree install-datadir) <(tree install) 1c1 < install-datadir --- > install 74,99c74,98 < ├── lib < │ ├── cmake < │ │ ├── glslang-default-resource-limitsTargets.cmake < │ │ ├── glslangTargets.cmake < │ │ ├── glslangValidatorTargets.cmake < │ │ ├── HLSLTargets.cmake < │ │ ├── OGLCompilerTargets.cmake < │ │ ├── OSDependentTargets.cmake < │ │ ├── spirv-remapTargets.cmake < │ │ ├── SPIRVTargets.cmake < │ │ └── SPVRemapperTargets.cmake < │ ├── libGenericCodeGen.a < │ ├── libglslang.a < │ ├── libglslang-default-resource-limits.a < │ ├── libHLSL.a < │ ├── libMachineIndependent.a < │ ├── libOGLCompiler.a < │ ├── libOSDependent.a < │ ├── libSPIRV.a < │ └── libSPVRemapper.a < └── share < └── glslang < ├── glslang-config.cmake < ├── glslang-config-version.cmake < ├── glslang-targets.cmake < └── glslang-targets-debug.cmake --- > └── lib > ├── cmake > │ ├── glslang-default-resource-limitsTargets.cmake > │ ├── glslangTargets.cmake > │ ├── glslangValidatorTargets.cmake > │ ├── HLSLTargets.cmake > │ ├── OGLCompilerTargets.cmake > │ ├── OSDependentTargets.cmake > │ ├── spirv-remapTargets.cmake > │ ├── SPIRVTargets.cmake > │ └── SPVRemapperTargets.cmake > ├── glslang > │ ├── glslang-config.cmake > │ ├── glslang-config-version.cmake > │ ├── glslang-targets.cmake > │ └── glslang-targets-debug.cmake > ├── libGenericCodeGen.a > ├── libglslang.a > ├── libglslang-default-resource-limits.a > ├── libHLSL.a > ├── libMachineIndependent.a > ├── libOGLCompiler.a > ├── libOSDependent.a > ├── libSPIRV.a > └── libSPVRemapper.a 101c100 < 15 directories, 83 files --- > 14 directories, 83 files

Add unified glslang CMake config collecting glslang-targets targets

950022a

theblackunknown force-pushed the unified-cmake-config branch from e83b80a to 950022a Compare July 30, 2022 22:36

greg-lunarg added the kokoro:run Trigger Google bot runs label Aug 1, 2022

kokoro-team removed the kokoro:run Trigger Google bot runs label Aug 1, 2022

ncesario-lunarg reviewed Aug 2, 2022

View reviewed changes

greg-lunarg requested review from greg-lunarg and removed request for greg-lunarg August 3, 2022 00:12

greg-lunarg merged commit fb64704 into KhronosGroup:master Aug 3, 2022

dneto0 mentioned this pull request Aug 11, 2022

update deps, finalize v2022.2 google/shaderc#1261

Merged

Tachi107 reviewed Aug 25, 2022

View reviewed changes

Tachi107 mentioned this pull request Aug 25, 2022

build: install glslang-config.cmake to libdir #3009

Merged

theblackunknown deleted the unified-cmake-config branch August 28, 2022 15:06

rhabacker mentioned this pull request Sep 6, 2022

Fix support to generate cmake config files #2778

Closed

svenstaro mentioned this pull request Nov 14, 2022

Fix locations of cmake files in side compat shims #3063

Merged

akien-mga mentioned this pull request Nov 14, 2022

Fix export configurations paths for Unixes #1978

Closed

rhabacker mentioned this pull request Jul 15, 2023

Incomplete cmake find_package support #2751

Closed

arcady-lunarg mentioned this pull request Nov 28, 2023

Add glslangConfig.cmake or glslang-config.cmake file to facilitate cmake package interaction #2570

Closed

robertosfield mentioned this pull request Nov 29, 2023

Inability to compile vsg with gslang support in build environments without network access vsg-dev/VulkanSceneGraph#1035

Closed

theblackunknown mentioned this pull request Jan 8, 2024

Create a single CMake config package for all SPIRV-Cross targets KhronosGroup/SPIRV-Cross#2253

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add unified `glslang` CMake config collecting `glslang-targets` targets #2989

Add unified `glslang` CMake config collecting `glslang-targets` targets #2989

theblackunknown commented Jul 30, 2022 •

edited by arcady-lunarg

Loading

CLAassistant commented Jul 30, 2022 •

edited

Loading

theblackunknown commented Jul 30, 2022

ncesario-lunarg left a comment

ncesario-lunarg commented Aug 2, 2022

theblackunknown commented Aug 2, 2022

theblackunknown commented Aug 2, 2022 •

edited

Loading

ncesario-lunarg commented Aug 2, 2022

greg-lunarg commented Aug 3, 2022

Tachi107 Aug 25, 2022

Tachi107 Aug 25, 2022

theblackunknown Aug 25, 2022

theblackunknown Aug 25, 2022

Tachi107 Aug 25, 2022

jeremy-lunarg Aug 25, 2022

Tachi107 Aug 25, 2022 •

edited

Loading

jeremy-lunarg Aug 25, 2022

Tachi107 Aug 25, 2022

sl1pkn07 commented Nov 17, 2022 •

edited

Loading

Add unified glslang CMake config collecting glslang-targets targets #2989

Add unified glslang CMake config collecting glslang-targets targets #2989

Conversation

theblackunknown commented Jul 30, 2022 • edited by arcady-lunarg Loading

CLAassistant commented Jul 30, 2022 • edited Loading

theblackunknown commented Jul 30, 2022

ncesario-lunarg left a comment

Choose a reason for hiding this comment

ncesario-lunarg commented Aug 2, 2022

theblackunknown commented Aug 2, 2022

theblackunknown commented Aug 2, 2022 • edited Loading

ncesario-lunarg commented Aug 2, 2022

greg-lunarg commented Aug 3, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Tachi107 Aug 25, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sl1pkn07 commented Nov 17, 2022 • edited Loading

Add unified `glslang` CMake config collecting `glslang-targets` targets #2989

Add unified `glslang` CMake config collecting `glslang-targets` targets #2989

theblackunknown commented Jul 30, 2022 •

edited by arcady-lunarg

Loading

CLAassistant commented Jul 30, 2022 •

edited

Loading

theblackunknown commented Aug 2, 2022 •

edited

Loading

Tachi107 Aug 25, 2022 •

edited

Loading

sl1pkn07 commented Nov 17, 2022 •

edited

Loading