CMake: Guard inclusion of VULKAN_HEADERS and SPIRV-Headers

[sergeiepopov]

Don't include the VULKAN_HEADERS and SPIRV-Headers submodule if they are already included.
Fix for the #7898.

[CLAassistant]

All committers have signed the CLA.

[jkwak-work]

@expipiplus1 and @jarcherNV for second opinion.

[jkwak-work]

I think this should be an error so that the user can fix it right away.

And from the naming perspective, "override" sounds like it could/should override SLANG_USE_SYSTEM_SPIRV_HEADERS; not the other way around.
But it wouldn't matter if it is an error and the build stops here.

[jarcherNV]

Looking at the original issue, are you sure this change is needed? If I understand correctly, the problem you are having is that Slang is attempting to use its own spirv and vulkan headers from the external submodules rather than yours? It is posible to override this behavior though, eg:

-DSLANG_OVERRIDE_VULKAN_HEADERS_PATH=/path/to/vulkan/headers
-DSLANG_OVERRIDE_SPIRV_HEADERS_PATH=/path/to/spirv/headers

Does this not work for your project?

[jkwak-work]

Looking at the original issue, are you sure this change is needed? If I understand correctly, the problem you are having is that Slang is attempting to use its own spirv and vulkan headers from the external submodules rather than yours? It is posible to override this behavior though, eg:

-DSLANG_OVERRIDE_VULKAN_HEADERS_PATH=/path/to/vulkan/headers -DSLANG_OVERRIDE_SPIRV_HEADERS_PATH=/path/to/spirv/headers

Does this not work for your project?

I think the intention is to use ones installed on the system.

So I guess there are three ways to use SPIRV-HEADER/TOOLS now.

• external/spirv-{header,tools}
• any directory with SLANG_OVERRIDE_SPIRV_HEADERS_PATH
• /usr/lib/... somewhere with SLANG_USE_SYSTEM_SPIRV_HEADERS

[jarcherNV]

Ok that makes sense. If that third option is currently broken and this fixes it then that sounds reasonable to me.

[sergeiepopov]

-DSLANG_OVERRIDE_VULKAN_HEADERS_PATH=/path/to/vulkan/headers
-DSLANG_OVERRIDE_SPIRV_HEADERS_PATH=/path/to/spirv/headers

Does this not work for your project?

This does not work in my project because it has its own Vulkan and SPIR-V header libraries, which are included first. However, with the above command, Slang also tries to include them, resulting in the following error:
CMake Error at core/external/spirv-headers/CMakeLists.txt:17 (add_library): add_library cannot create target "SPIRV-Headers" because another target with the same name already exists. The existing target is an interface library created in source directory "C:/source/engine/core/external/spirv-headers". See documentation for policy CMP0002 for more details.

Ok that makes sense. If that third option is currently broken and this fixes it then that sounds reasonable to me.

No, I don't use the third option. As I mentioned, my project uses its own instances of SPIR-V and Vulkan headers, which are not the system ones (i.e., they are not added via find_package(SPIRV-Headers REQUIRED) but rather through add_subdirectory("external/vulkan-headers")). This fix prevents double inclusion of those libraries by Slang, which now checks if a library was already included by the top-level project and skips the second inclusion.

[jkwak-work]

/format

[slangbot]

🌈 Formatted, please merge the changes from this PR

[sergeiepopov]

It requires workflow approval. @expipiplus1, @jarcherNV, @jkwak-work, could someone approve it and tick auto-mere if all looks good?